Skip to content

Commit ce231de

Browse files
author
Artem
committed
Fix validation + add telemetry
1 parent 7bd548f commit ce231de

12 files changed

+169
-44
lines changed

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ export enum TelemetryEvents {
1414
RedisInstanceConnectionFailed = 'DATABASE_CONNECTION_FAILED',
1515
RedisInstanceListReceived = 'CONFIG_DATABASES_DATABASE_LIST_DISPLAYED',
1616

17+
// Databases import
18+
DatabaseImportParseFailed = 'CONFIG_DATABASES_REDIS_IMPORT_PARSE_FAILED',
19+
DatabaseImportFailed = 'CONFIG_DATABASES_REDIS_IMPORT_FAILED',
20+
DatabaseImportSucceeded = 'CONFIG_DATABASES_REDIS_IMPORT_SUCCEEDED',
21+
1722
// Events for autodiscovery flows
1823
REClusterDiscoverySucceed = 'CONFIG_DATABASES_RE_CLUSTER_AUTODISCOVERY_SUCCEEDED',
1924
REClusterDiscoveryFailed = 'CONFIG_DATABASES_RE_CLUSTER_AUTODISCOVERY_FAILED',
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { Injectable } from '@nestjs/common';
2+
import { TelemetryBaseService } from 'src/modules/analytics/telemetry.base.service';
3+
import { EventEmitter2 } from '@nestjs/event-emitter';
4+
import { TelemetryEvents } from 'src/constants';
5+
import { DatabaseImportResponse } from 'src/modules/database-import/dto/database-import.response';
6+
7+
@Injectable()
8+
export class DatabaseImportAnalytics extends TelemetryBaseService {
9+
constructor(protected eventEmitter: EventEmitter2) {
10+
super(eventEmitter);
11+
}
12+
13+
sendImportResults(importResult: DatabaseImportResponse): void {
14+
if (importResult.success) {
15+
this.sendEvent(
16+
TelemetryEvents.DatabaseImportSucceeded,
17+
{
18+
succeed: importResult.success,
19+
},
20+
);
21+
}
22+
23+
if (importResult.errors?.length) {
24+
this.sendEvent(
25+
TelemetryEvents.DatabaseImportFailed,
26+
{
27+
failed: importResult.errors.length,
28+
errors: importResult.errors.map((e) => (e?.constructor?.name || 'UncaughtError')),
29+
},
30+
);
31+
}
32+
}
33+
34+
sendImportFailed(e: Error): void {
35+
this.sendEvent(
36+
TelemetryEvents.DatabaseImportParseFailed,
37+
{
38+
error: e?.constructor?.name || 'UncaughtError',
39+
},
40+
);
41+
}
42+
}

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import {
2-
BadRequestException,
32
Controller, HttpCode, Post, UploadedFile,
43
UseInterceptors, UsePipes, ValidationPipe,
54
} from '@nestjs/common';
@@ -35,14 +34,6 @@ export class DatabaseImportController {
3534
async import(
3635
@UploadedFile() file: any,
3736
): Promise<DatabaseImportResponse> {
38-
// todo: create FileValidation class
39-
if (!file) {
40-
throw new BadRequestException('No import file provided');
41-
}
42-
if (file?.size > 1024 * 1024 * 10) {
43-
throw new BadRequestException('Import file is too big. Maximum 10mb allowed');
44-
}
45-
4637
return this.service.import(file);
4738
}
4839
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import { Module } from '@nestjs/common';
22
import { DatabaseImportController } from 'src/modules/database-import/database-import.controller';
33
import { DatabaseImportService } from 'src/modules/database-import/database-import.service';
4+
import { DatabaseImportAnalytics } from 'src/modules/database-import/database-import.analytics';
45

56
@Module({
67
controllers: [DatabaseImportController],
78
providers: [
89
DatabaseImportService,
10+
DatabaseImportAnalytics,
911
],
1012
})
1113
export class DatabaseImportModule {}

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

Lines changed: 56 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { BadRequestException, Injectable, Logger } from '@nestjs/common';
1+
import { Injectable, Logger } from '@nestjs/common';
22
import { isArray, get, set } from 'lodash';
33
import { Database } from 'src/modules/database/models/database';
44
import { plainToClass } from 'class-transformer';
@@ -8,6 +8,11 @@ import { DatabaseImportResponse } from 'src/modules/database-import/dto/database
88
import { Validator } from 'class-validator';
99
import { ImportDatabaseDto } from 'src/modules/database-import/dto/import.database.dto';
1010
import { classToClass } from 'src/utils';
11+
import { DatabaseImportAnalytics } from 'src/modules/database-import/database-import.analytics';
12+
import {
13+
SizeLimitExceededDatabaseImportFileException,
14+
NoDatabaseImportFileProvidedException, UnableToParseDatabaseImportFileException,
15+
} from 'src/modules/database-import/exceptions';
1116

1217
@Injectable()
1318
export class DatabaseImportService {
@@ -27,36 +32,65 @@ export class DatabaseImportService {
2732

2833
constructor(
2934
private readonly databaseRepository: DatabaseRepository,
35+
private readonly analytics: DatabaseImportAnalytics,
3036
) {}
3137

3238
/**
3339
* Import databases from the file
3440
* @param file
3541
*/
3642
public async import(file): Promise<DatabaseImportResponse> {
37-
const items = DatabaseImportService.parseFile(file);
43+
try {
44+
// todo: create FileValidation class
45+
if (!file) {
46+
throw new NoDatabaseImportFileProvidedException('No import file provided');
47+
}
48+
if (file?.size > 1024 * 1024 * 10) {
49+
throw new SizeLimitExceededDatabaseImportFileException('Import file is too big. Maximum 10mb allowed');
50+
}
3851

39-
if (!isArray(items) || !items?.length) {
40-
let filename = file?.originalname || 'import file';
41-
if (filename.length > 50) {
42-
filename = `${filename.slice(0, 50)}...`;
52+
const items = DatabaseImportService.parseFile(file);
53+
54+
if (!isArray(items) || !items?.length) {
55+
let filename = file?.originalname || 'import file';
56+
if (filename.length > 50) {
57+
filename = `${filename.slice(0, 50)}...`;
58+
}
59+
throw new UnableToParseDatabaseImportFileException(`Unable to parse ${filename}`);
4360
}
44-
return Promise.reject(new BadRequestException(`Unable to parse ${filename}`));
45-
}
4661

47-
const response = {
48-
total: items.length,
49-
success: 0,
50-
};
62+
let response = {
63+
total: items.length,
64+
success: 0,
65+
errors: [],
66+
};
67+
68+
// 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;
77+
}
78+
this.logger.warn(`Unable to import database: ${error?.constructor?.name || 'UncaughtError'}`, error);
79+
response.errors.push(error);
80+
})), Promise.resolve());
81+
82+
this.analytics.sendImportResults(response);
83+
84+
response = plainToClass(DatabaseImportResponse, response);
85+
86+
return response;
87+
} catch (e) {
88+
this.logger.warn(`Unable to import databases: ${e?.constructor?.name || 'UncaughtError'}`, e);
5189

52-
// it is very important to insert databases on-by-one to avoid db constraint errors
53-
await items.reduce((prev, item) => prev.finally(() => this.createDatabase(item)
54-
.then(() => {
55-
response.success += 1;
56-
})
57-
.catch(() => { /* just ignore errors */ })), Promise.resolve());
90+
this.analytics.sendImportFailed(e);
5891

59-
return plainToClass(DatabaseImportResponse, response);
92+
throw e;
93+
}
6094
}
6195

6296
/**
@@ -101,14 +135,9 @@ export class DatabaseImportService {
101135
}, {}),
102136
);
103137

104-
try {
105-
await this.validator.validateOrReject(dto, {
106-
whitelist: true,
107-
});
108-
} catch (e) {
109-
this.logger.warn('Invalid data for database import entry', e);
110-
return Promise.reject(e);
111-
}
138+
await this.validator.validateOrReject(dto, {
139+
whitelist: true,
140+
});
112141

113142
const database = classToClass(Database, dto);
114143

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
11
import { ApiProperty } from '@nestjs/swagger';
2+
import { Exclude, Expose } from 'class-transformer';
23

34
export class DatabaseImportResponse {
45
@ApiProperty({
56
description: 'Total elements processed from the import file',
67
type: Number,
78
})
9+
@Expose()
810
total: number;
911

1012
@ApiProperty({
1113
description: 'Number of imported database',
1214
type: Number,
1315
})
16+
@Expose()
1417
success: number;
18+
19+
@Exclude()
20+
errors: Error[];
1521
}
Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
11
import { PickType } from '@nestjs/swagger';
22
import { Database } from 'src/modules/database/models/database';
3+
import { Expose, Type } from 'class-transformer';
4+
import { IsInt, IsNotEmpty } from 'class-validator';
35

46
export class ImportDatabaseDto extends PickType(Database, [
57
'host', 'port', 'name', 'db', 'username', 'password',
68
'connectionType',
7-
] as const) {}
9+
] as const) {
10+
@Expose()
11+
@IsNotEmpty()
12+
@IsInt({ always: true })
13+
@Type(() => Number)
14+
port: number;
15+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export * from './size-limit-exceeded-database-import-file.exception';
2+
export * from './no-database-import-file-provided.exception';
3+
export * from './unable-to-parse-database-import-file.exception';
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { HttpException } from '@nestjs/common';
2+
3+
export class NoDatabaseImportFileProvidedException extends HttpException {
4+
constructor(message: string = 'No import file provided') {
5+
const response = {
6+
message,
7+
statusCode: 400,
8+
error: 'No Database Import File Provided',
9+
};
10+
11+
super(response, 400);
12+
}
13+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { HttpException } from '@nestjs/common';
2+
3+
export class SizeLimitExceededDatabaseImportFileException extends HttpException {
4+
constructor(message: string = 'Invalid import file') {
5+
const response = {
6+
message,
7+
statusCode: 400,
8+
error: 'Invalid Database Import File',
9+
};
10+
11+
super(response, 400);
12+
}
13+
}

0 commit comments

Comments
 (0)