Skip to content

Commit f945ff1

Browse files
author
Artem
committed
#RI-3902 import result with statuses
1 parent 4f4b62b commit f945ff1

File tree

10 files changed

+254
-79
lines changed

10 files changed

+254
-79
lines changed

‎redisinsight/api/src/__mocks__/database-import.ts‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { DatabaseImportResponse } from 'src/modules/database-import/dto/database-import.response';
1+
import { DatabaseImportResponse, DatabaseImportStatus } from 'src/modules/database-import/dto/database-import.response';
22
import { BadRequestException, ForbiddenException } from '@nestjs/common';
33
import { mockDatabase } from 'src/__mocks__/databases';
4-
import { ValidationError } from 'class-validator';
4+
import { ValidationException } from 'src/common/exceptions';
55

66
export const mockDatabasesToImportArray = new Array(10).fill(mockDatabase);
77

@@ -12,23 +12,46 @@ export const mockDatabaseImportFile = {
1212
buffer: Buffer.from(JSON.stringify(mockDatabasesToImportArray)),
1313
};
1414

15+
export const mockDatabaseImportResultSuccess = {
16+
index: 0,
17+
status: DatabaseImportStatus.Success,
18+
host: mockDatabase.host,
19+
port: mockDatabase.port,
20+
};
21+
22+
export const mockDatabaseImportResultFail = {
23+
index: 0,
24+
status: DatabaseImportStatus.Fail,
25+
host: mockDatabase.host,
26+
port: mockDatabase.port,
27+
error: new BadRequestException(),
28+
};
29+
1530
export const mockDatabaseImportResponse = Object.assign(new DatabaseImportResponse(), {
1631
total: 10,
17-
success: 7,
18-
errors: [new ValidationError(), new BadRequestException(), new ForbiddenException()],
32+
success: (new Array(7).fill(mockDatabaseImportResultSuccess)).map((v, index) => ({
33+
...v,
34+
index: index + 3,
35+
})),
36+
partial: [],
37+
fail: [new ValidationException([]), new BadRequestException(), new ForbiddenException()].map((error, index) => ({
38+
...mockDatabaseImportResultFail,
39+
index,
40+
error,
41+
})),
1942
});
2043

2144
export const mockDatabaseImportParseFailedAnalyticsPayload = {
2245

2346
};
2447

2548
export const mockDatabaseImportFailedAnalyticsPayload = {
26-
failed: mockDatabaseImportResponse.errors.length,
27-
errors: ['ValidationError', 'BadRequestException', 'ForbiddenException'],
49+
failed: mockDatabaseImportResponse.fail.length,
50+
errors: ['ValidationException', 'BadRequestException', 'ForbiddenException'],
2851
};
2952

3053
export const mockDatabaseImportSucceededAnalyticsPayload = {
31-
succeed: mockDatabaseImportResponse.success,
54+
succeed: mockDatabaseImportResponse.success.length,
3255
};
3356

3457
export const mockDatabaseImportAnalytics = jest.fn(() => ({

‎redisinsight/api/src/common/exceptions/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export * from './validation.exception';

‎redisinsight/api/src/common/exceptions/validation.exception.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { BadRequestException } from '@nestjs/common';
2+
3+
export class ValidationException extends BadRequestException {}

‎redisinsight/api/src/constants/telemetry-events.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export enum TelemetryEvents {
1818
DatabaseImportParseFailed = 'CONFIG_DATABASES_REDIS_IMPORT_PARSE_FAILED',
1919
DatabaseImportFailed = 'CONFIG_DATABASES_REDIS_IMPORT_FAILED',
2020
DatabaseImportSucceeded = 'CONFIG_DATABASES_REDIS_IMPORT_SUCCEEDED',
21+
DatabaseImportPartiallySucceeded = 'CONFIG_DATABASES_REDIS_IMPORT_PARTIALLY_SUCCEEDED',
2122

2223
// Events for autodiscovery flows
2324
REClusterDiscoverySucceed = 'CONFIG_DATABASES_RE_CLUSTER_AUTODISCOVERY_SUCCEEDED',

‎redisinsight/api/src/modules/database-import/database-import.analytics.ts‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,31 @@ export class DatabaseImportAnalytics extends TelemetryBaseService {
1111
}
1212

1313
sendImportResults(importResult: DatabaseImportResponse): void {
14-
if (importResult.success) {
14+
if (importResult.success?.length) {
1515
this.sendEvent(
1616
TelemetryEvents.DatabaseImportSucceeded,
1717
{
18-
succeed: importResult.success,
18+
succeed: importResult.success.length,
1919
},
2020
);
2121
}
2222

23-
if (importResult.errors?.length) {
23+
if (importResult.fail?.length) {
2424
this.sendEvent(
2525
TelemetryEvents.DatabaseImportFailed,
2626
{
27-
failed: importResult.errors.length,
28-
errors: importResult.errors.map((e) => (e?.constructor?.name || 'UncaughtError')),
27+
failed: importResult.fail.length,
28+
errors: importResult.fail.map((res) => (res?.error?.constructor?.name || 'UncaughtError')),
29+
},
30+
);
31+
}
32+
33+
if (importResult.partial?.length) {
34+
this.sendEvent(
35+
TelemetryEvents.DatabaseImportPartiallySucceeded,
36+
{
37+
partially: importResult.partial.length,
38+
errors: importResult.partial.map((res) => (res?.error?.constructor?.name || 'UncaughtError')),
2939
},
3040
);
3141
}

‎redisinsight/api/src/modules/database-import/database-import.controller.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
2+
ClassSerializerInterceptor,
23
Controller, HttpCode, Post, UploadedFile,
3-
UseInterceptors, UsePipes, ValidationPipe,
4+
UseInterceptors, UsePipes, ValidationPipe
45
} from '@nestjs/common';
56
import {
67
ApiBody, ApiConsumes, ApiResponse, ApiTags,
@@ -10,6 +11,7 @@ import { FileInterceptor } from '@nestjs/platform-express';
1011
import { DatabaseImportResponse } from 'src/modules/database-import/dto/database-import.response';
1112

1213
@UsePipes(new ValidationPipe({ transform: true }))
14+
@UseInterceptors(ClassSerializerInterceptor)
1315
@ApiTags('Database')
1416
@Controller('/databases')
1517
export class DatabaseImportController {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ describe('DatabaseImportService', () => {
138138
it('should create standalone database', async () => {
139139
await service['createDatabase']({
140140
...mockDatabase,
141-
});
141+
}, 0);
142142

143143
expect(databaseRepository.create).toHaveBeenCalledWith({
144144
...pick(mockDatabase, ['host', 'port', 'name', 'connectionType']),
@@ -148,7 +148,7 @@ describe('DatabaseImportService', () => {
148148
await service['createDatabase']({
149149
...mockDatabase,
150150
name: undefined,
151-
});
151+
}, 0);
152152

153153
expect(databaseRepository.create).toHaveBeenCalledWith({
154154
...pick(mockDatabase, ['host', 'port', 'name', 'connectionType']),
@@ -159,7 +159,7 @@ describe('DatabaseImportService', () => {
159159
await service['createDatabase']({
160160
...mockDatabase,
161161
cluster: true,
162-
});
162+
}, 0);
163163

164164
expect(databaseRepository.create).toHaveBeenCalledWith({
165165
...pick(mockDatabase, ['host', 'port', 'name']),

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

Lines changed: 96 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,24 @@
1-
import { Injectable, Logger } from '@nestjs/common';
2-
import { isArray, get, set } from 'lodash';
1+
import { HttpException, Injectable, InternalServerErrorException, Logger } from '@nestjs/common';
2+
import { get, isArray, set } from 'lodash';
33
import { Database } from 'src/modules/database/models/database';
44
import { plainToClass } from 'class-transformer';
55
import { ConnectionType } from 'src/modules/database/entities/database.entity';
66
import { DatabaseRepository } from 'src/modules/database/repositories/database.repository';
7-
import { DatabaseImportResponse } from 'src/modules/database-import/dto/database-import.response';
8-
import { Validator } from 'class-validator';
7+
import {
8+
DatabaseImportResponse,
9+
DatabaseImportResult,
10+
DatabaseImportStatus,
11+
} from 'src/modules/database-import/dto/database-import.response';
12+
import { ValidationError, Validator } from 'class-validator';
913
import { ImportDatabaseDto } from 'src/modules/database-import/dto/import.database.dto';
1014
import { classToClass } from 'src/utils';
1115
import { DatabaseImportAnalytics } from 'src/modules/database-import/database-import.analytics';
1216
import {
17+
NoDatabaseImportFileProvidedException,
1318
SizeLimitExceededDatabaseImportFileException,
14-
NoDatabaseImportFileProvidedException, UnableToParseDatabaseImportFileException,
19+
UnableToParseDatabaseImportFileException,
1520
} from 'src/modules/database-import/exceptions';
21+
import { ValidationException } from 'src/common/exceptions';
1622

1723
@Injectable()
1824
export class DatabaseImportService {
@@ -61,28 +67,33 @@ export class DatabaseImportService {
6167

6268
let response = {
6369
total: items.length,
64-
success: 0,
65-
errors: [],
70+
success: [],
71+
partial: [],
72+
fail: [],
6673
};
6774

6875
// it is very important to insert databases on-by-one to avoid db constraint errors
69-
await items.reduce((prev, item) => prev.finally(() => this.createDatabase(item)
70-
.then(() => {
71-
response.success += 1;
72-
})
73-
.catch((e) => {
74-
let error = e;
75-
if (isArray(e)) {
76-
[error] = e;
76+
await items.reduce((prev, item, index) => prev.finally(() => this.createDatabase(item, index)
77+
.then((result) => {
78+
switch (result.status) {
79+
case DatabaseImportStatus.Fail:
80+
response.fail.push(result);
81+
break;
82+
case DatabaseImportStatus.Partial:
83+
response.partial.push(result);
84+
break;
85+
case DatabaseImportStatus.Success:
86+
response.success.push(result);
87+
break;
88+
default:
89+
// do not include into repost, since some unexpected behaviour
7790
}
78-
this.logger.warn(`Unable to import database: ${error?.constructor?.name || 'UncaughtError'}`, error);
79-
response.errors.push(error);
8091
})), Promise.resolve());
8192

82-
this.analytics.sendImportResults(response);
83-
8493
response = plainToClass(DatabaseImportResponse, response);
8594

95+
this.analytics.sendImportResults(response);
96+
8697
return response;
8798
} catch (e) {
8899
this.logger.warn(`Unable to import databases: ${e?.constructor?.name || 'UncaughtError'}`, e);
@@ -97,51 +108,84 @@ export class DatabaseImportService {
97108
* Map data to known model, validate it and create database if possible
98109
* Note: will not create connection, simply create database
99110
* @param item
111+
* @param index
100112
* @private
101113
*/
102-
private async createDatabase(item: any): Promise<Database> {
103-
const data: any = {};
114+
private async createDatabase(item: any, index: number): Promise<DatabaseImportResult> {
115+
try {
116+
const data: any = {};
104117

105-
this.fieldsMapSchema.forEach(([field, paths]) => {
106-
let value;
118+
this.fieldsMapSchema.forEach(([field, paths]) => {
119+
let value;
107120

108-
paths.every((path) => {
109-
value = get(item, path);
110-
return value === undefined;
121+
paths.every((path) => {
122+
value = get(item, path);
123+
return value === undefined;
124+
});
125+
126+
set(data, field, value);
111127
});
112128

113-
set(data, field, value);
114-
});
129+
// set database name if needed
130+
if (!data.name) {
131+
data.name = `${data.host}:${data.port}`;
132+
}
115133

116-
// set database name if needed
117-
if (!data.name) {
118-
data.name = `${data.host}:${data.port}`;
119-
}
134+
// determine database type
135+
if (data.isCluster) {
136+
data.connectionType = ConnectionType.CLUSTER;
137+
} else {
138+
data.connectionType = ConnectionType.STANDALONE;
139+
}
120140

121-
// determine database type
122-
if (data.isCluster) {
123-
data.connectionType = ConnectionType.CLUSTER;
124-
} else {
125-
data.connectionType = ConnectionType.STANDALONE;
126-
}
141+
const dto = plainToClass(
142+
ImportDatabaseDto,
143+
// additionally replace empty strings ("") with null
144+
Object.keys(data)
145+
.reduce((acc, key) => {
146+
acc[key] = data[key] === '' ? null : data[key];
147+
return acc;
148+
}, {}),
149+
);
150+
151+
await this.validator.validateOrReject(dto, {
152+
whitelist: true,
153+
});
154+
155+
const database = classToClass(Database, dto);
127156

128-
const dto = plainToClass(
129-
ImportDatabaseDto,
130-
// additionally replace empty strings ("") with null
131-
Object.keys(data)
132-
.reduce((acc, key) => {
133-
acc[key] = data[key] === '' ? null : data[key];
134-
return acc;
135-
}, {}),
136-
);
157+
await this.databaseRepository.create(database);
137158

138-
await this.validator.validateOrReject(dto, {
139-
whitelist: true,
140-
});
159+
return {
160+
index,
161+
status: DatabaseImportStatus.Success,
162+
host: database.host,
163+
port: database.port,
164+
};
165+
} catch (e) {
166+
let error = e;
167+
if (isArray(e)) {
168+
[error] = e;
169+
}
141170

142-
const database = classToClass(Database, dto);
171+
if (error instanceof ValidationError) {
172+
error = new ValidationException(Object.values(error?.constraints || {}) || 'Bad request');
173+
}
143174

144-
return this.databaseRepository.create(database);
175+
if (!(error instanceof HttpException)) {
176+
error = new InternalServerErrorException(error?.message);
177+
}
178+
179+
this.logger.warn(`Unable to import database: ${error?.constructor?.name || 'UncaughtError'}`, error);
180+
181+
return {
182+
index,
183+
status: DatabaseImportStatus.Fail,
184+
host: item?.host,
185+
port: item?.port,
186+
error,
187+
};
188+
}
145189
}
146190

147191
/**

0 commit comments

Comments
 (0)