Skip to content

Commit 757e5c2

Browse files
Merge pull request #2605 from RedisInsight/feature/RI-4813_not_duplicate_cloud_database
#RI-4813 - [BE] Do not duplicate Cloud databases
2 parents 51f930c + fdf9eda commit 757e5c2

File tree

27 files changed

+465
-61
lines changed

27 files changed

+465
-61
lines changed

redisinsight/api/src/constants/custom-error-codes.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,4 +33,7 @@ export enum CustomErrorCodes {
3333
CloudTaskNotFound = 11_112,
3434
CloudJobNotFound = 11_113,
3535
CloudSubscriptionAlreadyExistsFree = 11_114,
36+
37+
// General database errors [11200, 11299]
38+
DatabaseAlreadyExists = 11_200,
3639
}

redisinsight/api/src/constants/error-messages.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export default {
4848
REDIS_CLOUD_FORBIDDEN: 'Error fetching account details.',
4949

5050
DATABASE_IS_INACTIVE: 'The database is inactive.',
51+
DATABASE_ALREADY_EXISTS: 'The database already exists.',
5152

5253
INCORRECT_CLUSTER_CURSOR_FORMAT: 'Incorrect cluster cursor format.',
5354
REMOVING_MULTIPLE_ELEMENTS_NOT_SUPPORT: () => 'Removing multiple elements is available for Redis databases v. 6.2 or later.',

redisinsight/api/src/modules/database-import/database-import.service.spec.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ describe('DatabaseImportService', () => {
168168
...pick(mockDatabase, ['host', 'port', 'name', 'connectionType', 'timeout', 'compressor', 'modules']),
169169
provider: 'RE_CLOUD',
170170
new: true,
171-
});
171+
}, false);
172172
});
173173
it('should create standalone with created name', async () => {
174174
await service['createDatabase']({
@@ -180,7 +180,7 @@ describe('DatabaseImportService', () => {
180180
...pick(mockDatabase, ['host', 'port', 'name', 'connectionType', 'timeout', 'compressor', 'modules']),
181181
name: `${mockDatabase.host}:${mockDatabase.port}`,
182182
new: true,
183-
});
183+
}, false);
184184
});
185185
it('should create standalone with none compressor', async () => {
186186
await service['createDatabase']({
@@ -192,7 +192,7 @@ describe('DatabaseImportService', () => {
192192
...pick(mockDatabase, ['host', 'port', 'name', 'connectionType', 'timeout', 'compressor', 'modules']),
193193
compressor: Compressor.NONE,
194194
new: true,
195-
});
195+
}, false);
196196
});
197197
it('should create standalone with compressor', async () => {
198198
await service['createDatabase']({
@@ -204,7 +204,7 @@ describe('DatabaseImportService', () => {
204204
...pick(mockDatabase, ['host', 'port', 'name', 'connectionType', 'timeout', 'compressor', 'modules']),
205205
compressor: Compressor.GZIP,
206206
new: true,
207-
});
207+
}, false);
208208
});
209209
it('should create cluster database', async () => {
210210
await service['createDatabase']({
@@ -217,7 +217,7 @@ describe('DatabaseImportService', () => {
217217
...pick(mockDatabase, ['host', 'port', 'name', 'timeout', 'compressor', 'modules']),
218218
connectionType: ConnectionType.CLUSTER,
219219
new: true,
220-
});
220+
}, false);
221221
});
222222
});
223223

redisinsight/api/src/modules/database-import/database-import.service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ export class DatabaseImportService {
243243

244244
const database = classToClass(Database, dto);
245245

246-
await this.databaseRepository.create(database);
246+
await this.databaseRepository.create(database, false);
247247

248248
return {
249249
index,

redisinsight/api/src/modules/database/database.controller.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ export class DatabaseController {
9898
async create(
9999
@Body() dto: CreateDatabaseDto,
100100
): Promise<Database> {
101-
return await this.service.create(dto);
101+
return await this.service.create(dto, true);
102102
}
103103

104104
@UseInterceptors(ClassSerializerInterceptor)

redisinsight/api/src/modules/database/database.service.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,16 @@ export class DatabaseService {
9696
/**
9797
* Create new database with auto-detection of database type, modules, etc.
9898
* @param dto
99+
* @param uniqueCheck
99100
*/
100-
async create(dto: CreateDatabaseDto): Promise<Database> {
101+
async create(dto: CreateDatabaseDto, uniqueCheck = false): Promise<Database> {
101102
try {
102103
this.logger.log('Creating new database.');
103104

104105
const database = await this.repository.create({
105106
...await this.databaseFactory.createDatabaseModel(classToClass(Database, dto)),
106107
new: true,
107-
});
108+
}, uniqueCheck);
108109

109110
// todo: clarify if we need this and if yes - rethink implementation
110111
try {
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { HttpException, HttpExceptionOptions, HttpStatus } from '@nestjs/common';
2+
import { CustomErrorCodes } from 'src/constants';
3+
import ERROR_MESSAGES from 'src/constants/error-messages';
4+
5+
export class DatabaseAlreadyExistsException extends HttpException {
6+
constructor(databaseId: string, message = ERROR_MESSAGES.DATABASE_ALREADY_EXISTS, options?: HttpExceptionOptions) {
7+
const response = {
8+
message,
9+
statusCode: HttpStatus.CONFLICT,
10+
error: 'DatabaseAlreadyExists',
11+
errorCode: CustomErrorCodes.DatabaseAlreadyExists,
12+
resource: {
13+
databaseId,
14+
},
15+
};
16+
17+
super(response, response.statusCode, options);
18+
}
19+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export * from './database-already-exists.exception';

redisinsight/api/src/modules/database/repositories/database.repository.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ export abstract class DatabaseRepository {
2727
/**
2828
* Create database
2929
* @param database
30+
* @param uniqueCheck
3031
*/
31-
abstract create(database: Database): Promise<Database>;
32+
abstract create(database: Database, uniqueCheck: boolean): Promise<Database>;
3233

3334
/**
3435
* Update database with new data

redisinsight/api/src/modules/database/repositories/local.database.repository.spec.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ import { CaCertificateRepository } from 'src/modules/certificate/repositories/ca
4545
import { ClientCertificateRepository } from 'src/modules/certificate/repositories/client-certificate.repository';
4646
import { cloneClassInstance } from 'src/utils';
4747
import { SshOptionsEntity } from 'src/modules/ssh/entities/ssh-options.entity';
48+
import ERROR_MESSAGES from 'src/constants/error-messages';
49+
import { DatabaseAlreadyExistsException } from 'src/modules/database/exeptions';
4850

4951
const listFields = [
5052
'id', 'name', 'host', 'port', 'db', 'timeout',
@@ -251,7 +253,7 @@ describe('LocalDatabaseRepository', () => {
251253

252254
describe('create', () => {
253255
it('should create standalone database', async () => {
254-
const result = await service.create(mockDatabase);
256+
const result = await service.create(mockDatabase, false);
255257

256258
expect(result).toEqual(mockDatabase);
257259
expect(caCertRepository.create).not.toHaveBeenCalled();
@@ -261,7 +263,7 @@ describe('LocalDatabaseRepository', () => {
261263
it('should create standalone database with cloud details', async () => {
262264
repository.save.mockResolvedValue(mockDatabaseEntityWithCloudDetails);
263265

264-
const result = await service.create(mockDatabaseWithCloudDetails);
266+
const result = await service.create(mockDatabaseWithCloudDetails, false);
265267

266268
expect(result).toEqual(mockDatabaseWithCloudDetails);
267269
expect(caCertRepository.create).not.toHaveBeenCalled();
@@ -271,7 +273,7 @@ describe('LocalDatabaseRepository', () => {
271273
it('should create standalone database (with existing certificates)', async () => {
272274
repository.save.mockResolvedValueOnce(mockDatabaseWithTlsAuthEntity);
273275

274-
const result = await service.create(mockDatabaseWithTlsAuth);
276+
const result = await service.create(mockDatabaseWithTlsAuth, false);
275277

276278
expect(result).toEqual(mockDatabaseWithTlsAuth);
277279
expect(caCertRepository.create).not.toHaveBeenCalled();
@@ -283,12 +285,26 @@ describe('LocalDatabaseRepository', () => {
283285

284286
const result = await service.create(
285287
omit(cloneClassInstance(mockDatabaseWithTlsAuth), 'caCert.id', 'clientCert.id'),
288+
false,
286289
);
287290

288291
expect(result).toEqual(mockDatabaseWithTlsAuth);
289292
expect(caCertRepository.create).toHaveBeenCalled();
290293
expect(clientCertRepository.create).toHaveBeenCalled();
291294
});
295+
296+
it('should throw an error if create called with cloud details and have the same entity', async () => {
297+
repository.findOneBy.mockResolvedValueOnce(mockDatabaseEntity);
298+
try {
299+
await service.create(mockDatabaseEntityWithCloudDetails, true);
300+
fail();
301+
} catch (e) {
302+
expect(e).toBeInstanceOf(DatabaseAlreadyExistsException);
303+
expect(e.message).toEqual(ERROR_MESSAGES.DATABASE_ALREADY_EXISTS);
304+
expect(e.response?.resource?.databaseId).toEqual(mockDatabaseEntity.id);
305+
expect(repository.save).not.toHaveBeenCalled();
306+
}
307+
});
292308
});
293309

294310
describe('update', () => {

0 commit comments

Comments
 (0)