diff --git a/redisinsight/api/src/common/interceptors/timeout.interceptor.ts b/redisinsight/api/src/common/interceptors/timeout.interceptor.ts index 04857ef2cb..f726898d0f 100644 --- a/redisinsight/api/src/common/interceptors/timeout.interceptor.ts +++ b/redisinsight/api/src/common/interceptors/timeout.interceptor.ts @@ -18,7 +18,7 @@ export class TimeoutInterceptor implements NestInterceptor { private readonly message: string; - constructor(message?: string) { + constructor(message: string = 'Request timeout') { this.message = message; } diff --git a/redisinsight/api/src/constants/telemetry-events.ts b/redisinsight/api/src/constants/telemetry-events.ts index 307a74ea47..75385fbef0 100644 --- a/redisinsight/api/src/constants/telemetry-events.ts +++ b/redisinsight/api/src/constants/telemetry-events.ts @@ -61,3 +61,5 @@ export enum CommandType { Core = 'core', Module = 'module', } + +export const unknownCommand = 'unknown'; diff --git a/redisinsight/api/src/modules/analytics/telemetry.base.service.spec.ts b/redisinsight/api/src/modules/analytics/telemetry.base.service.spec.ts index 26cb3f12db..0d0e5302ae 100644 --- a/redisinsight/api/src/modules/analytics/telemetry.base.service.spec.ts +++ b/redisinsight/api/src/modules/analytics/telemetry.base.service.spec.ts @@ -33,11 +33,11 @@ describe('TelemetryBaseService', () => { describe('sendEvent', () => { it('should emit event', () => { - service.sendEvent(TelemetryEvents.RedisInstanceAdded, { data: 'Some data' }); + service.sendEvent(TelemetryEvents.RedisInstanceAdded, { data: 'Some data', command: 'lowercase' }); expect(eventEmitter.emit).toHaveBeenCalledWith(AppAnalyticsEvents.Track, { event: TelemetryEvents.RedisInstanceAdded, - eventData: { data: 'Some data' }, + eventData: { data: 'Some data', command: 'LOWERCASE' }, }); }); it('should emit event with empty event data', () => { @@ -87,13 +87,18 @@ describe('TelemetryBaseService', () => { }); }); it('should emit event with additional event data', () => { - service.sendFailedEvent(TelemetryEvents.RedisInstanceAddFailed, httpException, { data: 'Some data' }); + service.sendFailedEvent( + TelemetryEvents.RedisInstanceAddFailed, + httpException, + { data: 'Some data', command: 'lowercase' }, + ); expect(eventEmitter.emit).toHaveBeenCalledWith(AppAnalyticsEvents.Track, { event: TelemetryEvents.RedisInstanceAddFailed, eventData: { error: 'Internal Server Error', data: 'Some data', + command: 'LOWERCASE', }, }); }); diff --git a/redisinsight/api/src/modules/analytics/telemetry.base.service.ts b/redisinsight/api/src/modules/analytics/telemetry.base.service.ts index 211e7f2a25..5f1dc54973 100644 --- a/redisinsight/api/src/modules/analytics/telemetry.base.service.ts +++ b/redisinsight/api/src/modules/analytics/telemetry.base.service.ts @@ -1,3 +1,4 @@ +import { isString } from 'lodash'; import { EventEmitter2 } from '@nestjs/event-emitter'; import { HttpException } from '@nestjs/common'; import { AppAnalyticsEvents } from 'src/constants'; @@ -13,7 +14,10 @@ export abstract class TelemetryBaseService { try { this.eventEmitter.emit(AppAnalyticsEvents.Track, { event, - eventData, + eventData: { + ...eventData, + command: isString(eventData['command']) ? eventData['command'].toUpperCase() : eventData['command'], + }, }); } catch (e) { // continue regardless of error @@ -27,6 +31,7 @@ export abstract class TelemetryBaseService { eventData: { error: exception.getResponse()['error'] || exception.message, ...eventData, + command: isString(eventData['command']) ? eventData['command'].toUpperCase() : eventData['command'], }, }); } catch (e) { diff --git a/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/cluster.strategy.spec.ts b/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/cluster.strategy.spec.ts index fef3dd2f95..d8a120ac12 100644 --- a/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/cluster.strategy.spec.ts +++ b/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/cluster.strategy.spec.ts @@ -943,7 +943,7 @@ describe('Cluster Scanner Strategy', () => { scanned: 10, }, ]); - expect(strategy.getKeyInfo).toHaveBeenCalledWith(clusterClient, key); + expect(strategy.getKeyInfo).toHaveBeenCalledWith(clusterClient, Buffer.from(key)); expect(strategy.scanNodes).not.toHaveBeenCalled(); }); it('should find exact key when match is escaped glob patter', async () => { @@ -973,7 +973,7 @@ describe('Cluster Scanner Strategy', () => { scanned: 10, }, ]); - expect(strategy.getKeyInfo).toHaveBeenCalledWith(clusterClient, searchPattern); + expect(strategy.getKeyInfo).toHaveBeenCalledWith(clusterClient, Buffer.from(searchPattern)); expect(strategy.scanNodes).not.toHaveBeenCalled(); }); it('should find exact key with correct type', async () => { diff --git a/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/cluster.strategy.ts b/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/cluster.strategy.ts index a83efd19c4..75742eb0a5 100644 --- a/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/cluster.strategy.ts +++ b/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/cluster.strategy.ts @@ -49,7 +49,7 @@ export class ClusterStrategy extends AbstractStrategy { await this.calculateNodesTotalKeys(clientOptions, currentDbIndex, nodes); if (!isGlob(match, { strict: false })) { - const keyName = unescapeGlob(match); + const keyName = Buffer.from(unescapeGlob(match)); nodes.forEach((node) => { // eslint-disable-next-line no-param-reassign node.cursor = 0; diff --git a/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/standalone.strategy.spec.ts b/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/standalone.strategy.spec.ts index b82c98b8a0..8501a10b59 100644 --- a/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/standalone.strategy.spec.ts +++ b/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/standalone.strategy.spec.ts @@ -476,7 +476,7 @@ describe('Standalone Scanner Strategy', () => { }, ]); expect(strategy.getKeysInfo).toHaveBeenCalledWith(nodeClient, [ - key, + Buffer.from(key), ]); expect(strategy.scan).not.toHaveBeenCalled(); }); @@ -497,7 +497,7 @@ describe('Standalone Scanner Strategy', () => { keys: [{ ...getKeyInfoResponse, name: mockSearchPattern }], }, ]); - expect(strategy.getKeysInfo).toHaveBeenCalledWith(nodeClient, [mockSearchPattern]); + expect(strategy.getKeysInfo).toHaveBeenCalledWith(nodeClient, [Buffer.from(mockSearchPattern)]); expect(strategy.scan).not.toHaveBeenCalled(); }); it('should find exact key with correct type', async () => { diff --git a/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/standalone.strategy.ts b/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/standalone.strategy.ts index 76a1f31e6a..9ffe268a37 100644 --- a/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/standalone.strategy.ts +++ b/redisinsight/api/src/modules/browser/services/keys-business/scanner/strategies/standalone.strategy.ts @@ -62,7 +62,7 @@ export class StandaloneStrategy extends AbstractStrategy { } if (!isGlob(match, { strict: false })) { - const keyName = unescapeGlob(match); + const keyName = Buffer.from(unescapeGlob(match)); node.cursor = 0; node.scanned = isNull(node.total) ? 1 : node.total; node.keys = await this.getKeysInfo(client, [keyName]); diff --git a/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.spec.ts b/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.spec.ts index 49d02c430b..175276afb0 100644 --- a/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.spec.ts +++ b/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.spec.ts @@ -240,7 +240,7 @@ describe('RedisearchService', () => { 'LIMIT', `${mockSearchRedisearchDto.offset}`, `${mockSearchRedisearchDto.limit}`, ], })); - expect(nodeClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining( { + expect(nodeClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining({ name: 'FT.CONFIG', args: [ 'GET', @@ -277,7 +277,7 @@ describe('RedisearchService', () => { 'LIMIT', `${mockSearchRedisearchDto.offset}`, `${mockSearchRedisearchDto.limit}`, ], })); - expect(clusterClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining( { + expect(clusterClient.sendCommand).toHaveBeenCalledWith(jasmine.objectContaining({ name: 'FT.CONFIG', args: [ 'GET', diff --git a/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts b/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts index 2e23d053de..9776cb99d3 100644 --- a/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts +++ b/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts @@ -3,6 +3,7 @@ import { toNumber, uniq } from 'lodash'; import { BadRequestException, ConflictException, + HttpException, Injectable, Logger, } from '@nestjs/common'; @@ -147,11 +148,6 @@ export class RedisearchService { const client = await this.browserTool.getRedisClient(clientOptions); - const [total, ...keyNames] = await client.sendCommand( - new Command('FT.SEARCH', [index, query, 'NOCONTENT', 'LIMIT', offset, limit]), - ); - - try { const [[, maxSearchResults]] = await client.sendCommand( // response: [ [ 'MAXSEARCHRESULTS', '10000' ] ] @@ -165,6 +161,16 @@ export class RedisearchService { maxResults = null; } + // Workaround: recalculate limit to not query more then MAXSEARCHRESULTS + let safeLimit = limit; + if (maxResults && offset + limit > maxResults) { + safeLimit = offset <= maxResults ? maxResults - offset : limit; + } + + const [total, ...keyNames] = await client.sendCommand( + new Command('FT.SEARCH', [index, query, 'NOCONTENT', 'LIMIT', offset, safeLimit]), + ); + return plainToClass(GetKeysWithDetailsResponse, { cursor: limit + offset, total, @@ -172,12 +178,17 @@ export class RedisearchService { keys: keyNames.map((name) => ({ name })), maxResults, }); - } catch (error) { - this.logger.error('Failed to search keys using redisearch index', error); - if (error.message?.includes(RedisErrorCodes.RedisearchLimit)) { + } catch (e) { + this.logger.error('Failed to search keys using redisearch index', e); + + if (e instanceof HttpException) { + throw e; + } + + if (e.message?.includes(RedisErrorCodes.RedisearchLimit)) { throw new BadRequestException(ERROR_MESSAGES.INCREASE_MINIMUM_LIMIT(numberWithSpaces(dto.limit))); } - throw catchAclError(error); + throw catchAclError(e); } } diff --git a/redisinsight/api/src/modules/cli/services/cli-analytics/cli-analytics.service.spec.ts b/redisinsight/api/src/modules/cli/services/cli-analytics/cli-analytics.service.spec.ts index 94250cd88c..6ffd5ca0d9 100644 --- a/redisinsight/api/src/modules/cli/services/cli-analytics/cli-analytics.service.spec.ts +++ b/redisinsight/api/src/modules/cli/services/cli-analytics/cli-analytics.service.spec.ts @@ -203,7 +203,7 @@ describe('CliAnalyticsService', () => { TelemetryEvents.CliCommandExecuted, { databaseId, - command: mockAdditionalData.command.toUpperCase(), + command: mockAdditionalData.command, commandType: CommandType.Core, moduleName: 'n/a', capability: 'string', @@ -231,7 +231,7 @@ describe('CliAnalyticsService', () => { { databaseId, error: ReplyError.name, - command: mockAdditionalData.command.toUpperCase(), + command: mockAdditionalData.command, commandType: CommandType.Core, moduleName: 'n/a', capability: 'string', @@ -246,7 +246,7 @@ describe('CliAnalyticsService', () => { { databaseId, error: ReplyError.name, - command: 'sadd'.toUpperCase(), + command: 'sadd', }, ); }); @@ -259,7 +259,7 @@ describe('CliAnalyticsService', () => { { databaseId, error: CommandParsingError.name, - command: mockAdditionalData.command.toUpperCase(), + command: mockAdditionalData.command, commandType: CommandType.Core, moduleName: 'n/a', capability: 'string', @@ -312,7 +312,7 @@ describe('CliAnalyticsService', () => { TelemetryEvents.CliClusterNodeCommandExecuted, { databaseId, - command: mockAdditionalData.command.toUpperCase(), + command: mockAdditionalData.command, commandType: CommandType.Core, moduleName: 'n/a', capability: 'string', @@ -335,7 +335,7 @@ describe('CliAnalyticsService', () => { { databaseId, error: redisReplyError.name, - command: 'sadd'.toUpperCase(), + command: 'sadd', }, ); }); diff --git a/redisinsight/api/src/modules/cli/services/cli-analytics/cli-analytics.service.ts b/redisinsight/api/src/modules/cli/services/cli-analytics/cli-analytics.service.ts index 268ef4f578..7da6a5e8b3 100644 --- a/redisinsight/api/src/modules/cli/services/cli-analytics/cli-analytics.service.ts +++ b/redisinsight/api/src/modules/cli/services/cli-analytics/cli-analytics.service.ts @@ -88,7 +88,6 @@ export class CliAnalyticsService extends CommandTelemetryBaseService { databaseId, ...(await this.getCommandAdditionalInfo(additionalData['command'])), ...additionalData, - command: additionalData['command']?.toUpperCase() || undefined, }, ); } catch (e) { @@ -102,15 +101,14 @@ export class CliAnalyticsService extends CommandTelemetryBaseService { additionalData: object = {}, ): Promise { try { - const commandFromError = error?.command?.name?.toUpperCase() || undefined; this.sendEvent( TelemetryEvents.CliCommandErrorReceived, { databaseId, error: error?.name, + command: error?.command?.name, ...(await this.getCommandAdditionalInfo(additionalData['command'])), ...additionalData, - command: additionalData['command']?.toUpperCase() || commandFromError, }, ); } catch (e) { @@ -132,20 +130,18 @@ export class CliAnalyticsService extends CommandTelemetryBaseService { databaseId, ...(await this.getCommandAdditionalInfo(additionalData['command'])), ...additionalData, - command: additionalData['command']?.toUpperCase() || undefined, }, ); } if (status === CommandExecutionStatus.Fail) { - const commandFromError = error?.command?.name?.toUpperCase() || undefined; this.sendEvent( TelemetryEvents.CliCommandErrorReceived, { databaseId, error: error.name, + command: error?.command?.name, ...(await this.getCommandAdditionalInfo(additionalData['command'])), ...additionalData, - command: additionalData['command']?.toUpperCase() || commandFromError, }, ); } diff --git a/redisinsight/api/src/modules/cli/services/cli-business/cli-business.service.spec.ts b/redisinsight/api/src/modules/cli/services/cli-business/cli-business.service.spec.ts index bd55f83855..078f193ae1 100644 --- a/redisinsight/api/src/modules/cli/services/cli-business/cli-business.service.spec.ts +++ b/redisinsight/api/src/modules/cli/services/cli-business/cli-business.service.spec.ts @@ -28,6 +28,7 @@ import { CommandParsingError, WrongDatabaseTypeError, } from 'src/modules/cli/constants/errors'; +import { unknownCommand } from 'src/constants'; import { CliAnalyticsService } from 'src/modules/cli/services/cli-analytics/cli-analytics.service'; import { KeytarUnavailableException } from 'src/modules/encryption/exceptions'; import { RedisToolService } from 'src/modules/redis/redis-tool.service'; @@ -257,7 +258,7 @@ describe('CliBusinessService', () => { expect(analyticsService.sendCommandExecutedEvent).toHaveBeenCalledWith( mockClientOptions.instanceId, { - command: 'memory'.toUpperCase(), + command: 'memory', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -283,7 +284,7 @@ describe('CliBusinessService', () => { expect(analyticsService.sendCommandExecutedEvent).toHaveBeenCalledWith( mockClientOptions.instanceId, { - command: 'memory'.toUpperCase(), + command: 'memory', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -307,7 +308,7 @@ describe('CliBusinessService', () => { ERROR_MESSAGES.CLI_COMMAND_NOT_SUPPORTED(command.toUpperCase()), ), { - command: 'script'.toUpperCase(), + command: 'script', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -330,6 +331,7 @@ describe('CliBusinessService', () => { ERROR_MESSAGES.CLI_UNTERMINATED_QUOTES(), ), { + command: unknownCommand, outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -355,7 +357,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, replyError, { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -374,7 +376,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new Error(mockENotFoundMessage), { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -394,7 +396,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new KeytarUnavailableException(), { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -416,7 +418,7 @@ describe('CliBusinessService', () => { expect(analyticsService.sendCommandExecutedEvent).toHaveBeenCalledWith( mockClientOptions.instanceId, { - command: 'info'.toUpperCase(), + command: 'info', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -491,7 +493,7 @@ describe('CliBusinessService', () => { ...mockNode, }, { - command: 'memory'.toUpperCase(), + command: 'memory', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -535,7 +537,7 @@ describe('CliBusinessService', () => { ...mockNode, }, { - command: 'info'.toUpperCase(), + command: 'info', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -565,7 +567,7 @@ describe('CliBusinessService', () => { command.toUpperCase(), )), { - command: 'script'.toUpperCase(), + command: 'script', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -591,6 +593,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new CommandParsingError(ERROR_MESSAGES.CLI_UNTERMINATED_QUOTES()), { + command: unknownCommand, outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -699,7 +702,7 @@ describe('CliBusinessService', () => { status: CommandExecutionStatus.Success, }, { - command: 'memory'.toUpperCase(), + command: 'memory', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -740,7 +743,7 @@ describe('CliBusinessService', () => { status: CommandExecutionStatus.Success, }, { - command: 'info'.toUpperCase(), + command: 'info', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -786,7 +789,7 @@ describe('CliBusinessService', () => { status: CommandExecutionStatus.Success, }, { - command: 'set'.toUpperCase(), + command: 'set', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -831,7 +834,7 @@ describe('CliBusinessService', () => { status: CommandExecutionStatus.Success, }, { - command: 'set'.toUpperCase(), + command: 'set', outputFormat: CliOutputFormatterTypes.Text, }, ); @@ -868,7 +871,7 @@ describe('CliBusinessService', () => { status: CommandExecutionStatus.Fail, }, { - command: 'set'.toUpperCase(), + command: 'set', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -896,7 +899,7 @@ describe('CliBusinessService', () => { command.toUpperCase(), )), { - command: 'script'.toUpperCase(), + command: 'script', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -920,6 +923,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new CommandParsingError(ERROR_MESSAGES.CLI_UNTERMINATED_QUOTES()), { + command: unknownCommand, outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -945,7 +949,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new WrongDatabaseTypeError(ERROR_MESSAGES.WRONG_DATABASE_TYPE), { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -975,7 +979,7 @@ describe('CliBusinessService', () => { ERROR_MESSAGES.CLUSTER_NODE_NOT_FOUND('127.0.0.1:7002'), ), { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -999,7 +1003,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new Error(mockENotFoundMessage), { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -1023,7 +1027,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new KeytarUnavailableException(), { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); diff --git a/redisinsight/api/src/modules/cli/services/cli-business/cli-business.service.ts b/redisinsight/api/src/modules/cli/services/cli-business/cli-business.service.ts index 77a4e8bab1..4ebf383e9f 100644 --- a/redisinsight/api/src/modules/cli/services/cli-business/cli-business.service.ts +++ b/redisinsight/api/src/modules/cli/services/cli-business/cli-business.service.ts @@ -5,6 +5,7 @@ import { Logger, } from '@nestjs/common'; import { IFindRedisClientInstanceByOptions } from 'src/modules/redis/redis.service'; +import { unknownCommand } from 'src/constants'; import { CommandsService } from 'src/modules/commands/commands.service'; import { ClusterNodeRole, @@ -143,7 +144,7 @@ export class CliBusinessService { ): Promise { this.logger.log('Executing redis CLI command.'); const { command: commandLine } = dto; - let command: string; + let command: string = unknownCommand; let args: string[] = []; const outputFormat = dto.outputFormat || CliOutputFormatterTypes.Raw; @@ -160,7 +161,7 @@ export class CliBusinessService { this.cliAnalyticsService.sendCommandExecutedEvent( clientOptions.instanceId, { - command: command?.toUpperCase(), + command, outputFormat, }, ); @@ -177,14 +178,14 @@ export class CliBusinessService { || error?.name === 'ReplyError' ) { this.cliAnalyticsService.sendCommandErrorEvent(clientOptions.instanceId, error, { - command: command?.toUpperCase(), + command, outputFormat, }); return { response: error.message, status: CommandExecutionStatus.Fail }; } this.cliAnalyticsService.sendConnectionErrorEvent(clientOptions.instanceId, error, { - command: command?.toUpperCase(), + command, outputFormat, }); @@ -229,7 +230,7 @@ export class CliBusinessService { outputFormat: CliOutputFormatterTypes = CliOutputFormatterTypes.Raw, ): Promise { this.logger.log(`Executing redis.cluster CLI command for [${role}] nodes.`); - let command: string; + let command: string = unknownCommand; let args: string[] = []; try { @@ -250,7 +251,7 @@ export class CliBusinessService { this.cliAnalyticsService.sendClusterCommandExecutedEvent( clientOptions.instanceId, nodeExecReply, - { command: command?.toUpperCase(), outputFormat }, + { command, outputFormat }, ); const { response, status, host, port, @@ -266,7 +267,7 @@ export class CliBusinessService { if (error instanceof CommandParsingError || error instanceof CommandNotSupportedError) { this.cliAnalyticsService.sendCommandErrorEvent(clientOptions.instanceId, error, { - command: command?.toUpperCase(), + command, outputFormat, }); return [ @@ -298,7 +299,7 @@ export class CliBusinessService { outputFormat: CliOutputFormatterTypes = CliOutputFormatterTypes.Raw, ): Promise { this.logger.log(`Executing redis.cluster CLI command for single node ${JSON.stringify(nodeOptions)}`); - let command: string; + let command: string = unknownCommand; let args: string[] = []; try { @@ -333,7 +334,7 @@ export class CliBusinessService { this.cliAnalyticsService.sendClusterCommandExecutedEvent( clientOptions.instanceId, result, - { command: command?.toUpperCase(), outputFormat }, + { command, outputFormat }, ); const { host, port, error, slot, ...rest @@ -344,14 +345,14 @@ export class CliBusinessService { if (error instanceof CommandParsingError || error instanceof CommandNotSupportedError) { this.cliAnalyticsService.sendCommandErrorEvent(clientOptions.instanceId, error, { - command: command?.toUpperCase(), + command, outputFormat, }); return { response: error.message, status: CommandExecutionStatus.Fail }; } this.cliAnalyticsService.sendConnectionErrorEvent(clientOptions.instanceId, error, { - command: command?.toUpperCase(), + command, outputFormat, }); diff --git a/redisinsight/api/src/modules/database/database.controller.ts b/redisinsight/api/src/modules/database/database.controller.ts index 4e650ea50e..af19aa4f0f 100644 --- a/redisinsight/api/src/modules/database/database.controller.ts +++ b/redisinsight/api/src/modules/database/database.controller.ts @@ -147,7 +147,7 @@ export class DatabaseController { } @Get(':id/connect') - @UseInterceptors(new TimeoutInterceptor()) + @UseInterceptors(new TimeoutInterceptor(ERROR_MESSAGES.CONNECTION_TIMEOUT)) @ApiEndpoint({ description: 'Connect to database instance by id', statusCode: 200, diff --git a/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.spec.ts b/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.spec.ts index e380dc7810..0edbe8c0a8 100644 --- a/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.spec.ts +++ b/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.spec.ts @@ -5,6 +5,8 @@ import { mockDatabase, mockWorkbenchAnalyticsService, } from 'src/__mocks__'; +import ERROR_MESSAGES from 'src/constants/error-messages'; +import { unknownCommand } from 'src/constants'; import { IFindRedisClientInstanceByOptions } from 'src/modules/redis/redis.service'; import { WorkbenchCommandsExecutor } from 'src/modules/workbench/providers/workbench-commands.executor'; import { @@ -17,6 +19,7 @@ import { CommandExecutionStatus } from 'src/modules/cli/dto/cli.dto'; import { BadRequestException, InternalServerErrorException, ServiceUnavailableException } from '@nestjs/common'; import { CommandNotSupportedError, + CommandParsingError, ClusterNodeNotFoundError, WrongDatabaseTypeError, } from 'src/modules/cli/constants/errors'; @@ -49,6 +52,7 @@ const mockCliNodeResponse: ICliExecResultFromNode = { }; const mockSetCommand = 'set'; +const mockGetEscapedKeyCommand = 'get "\\\\key'; const mockCreateCommandExecutionDto: CreateCommandExecutionDto = { command: `${mockSetCommand} foo bar`, nodeOptions: { @@ -131,7 +135,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -157,7 +161,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -188,7 +192,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -218,7 +222,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -248,7 +252,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: true, }, ); @@ -274,7 +278,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -299,7 +303,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -330,7 +334,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -364,7 +368,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -387,7 +391,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -410,7 +414,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -434,7 +438,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -475,7 +479,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -502,7 +506,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -529,7 +533,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -557,12 +561,42 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); } }); }); + describe('CommandParsingError', () => { + it('should return response with [CLI_UNTERMINATED_QUOTES] error for sendCommandForNodes', async () => { + const mockResult = [ + { + response: ERROR_MESSAGES.CLI_UNTERMINATED_QUOTES(), + status: CommandExecutionStatus.Fail, + }, + ]; + + const result = await service.sendCommand(mockClientOptions, { + command: mockGetEscapedKeyCommand, + role: mockCreateCommandExecutionDto.role, + mode: RunQueryMode.ASCII, + }); + + expect(result).toEqual(mockResult); + expect(mockAnalyticsService.sendCommandExecutedEvent).toHaveBeenCalledWith( + mockClientOptions.instanceId, + { + response: ERROR_MESSAGES.CLI_UNTERMINATED_QUOTES(), + error: new CommandParsingError(ERROR_MESSAGES.CLI_UNTERMINATED_QUOTES()), + status: CommandExecutionStatus.Fail, + }, + { + command: unknownCommand, + rawMode: false, + }, + ); + }); + }); }); }); diff --git a/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.ts b/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.ts index bfcb4a5809..53ba13104a 100644 --- a/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.ts +++ b/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.ts @@ -18,6 +18,7 @@ import { ClusterNodeNotFoundError, WrongDatabaseTypeError, } from 'src/modules/cli/constants/errors'; +import { unknownCommand } from 'src/constants'; import { CommandExecutionResult } from 'src/modules/workbench/models/command-execution-result'; import { CreateCommandExecutionDto, RunQueryMode } from 'src/modules/workbench/dto/create-command-execution.dto'; import { RedisToolService } from 'src/modules/redis/redis-tool.service'; @@ -62,6 +63,8 @@ export class WorkbenchCommandsExecutor { dto: CreateCommandExecutionDto, ): Promise { let result; + let command = unknownCommand; + let commandArgs: string[] = []; const { command: commandLine, @@ -69,10 +72,9 @@ export class WorkbenchCommandsExecutor { nodeOptions, mode, } = dto; - - const [command, ...commandArgs] = splitCliCommandLine(commandLine); - try { + [command, ...commandArgs] = splitCliCommandLine(commandLine); + if (nodeOptions) { result = [await this.sendCommandForSingleNode( clientOptions, @@ -102,7 +104,7 @@ export class WorkbenchCommandsExecutor { this.analyticsService.sendCommandExecutedEvents( clientOptions.instanceId, result, - { command: command.toUpperCase(), rawMode: mode === RunQueryMode.Raw }, + { command, rawMode: mode === RunQueryMode.Raw }, ); return result; @@ -113,7 +115,7 @@ export class WorkbenchCommandsExecutor { this.analyticsService.sendCommandExecutedEvent( clientOptions.instanceId, { ...errorResult, error }, - { command: command.toUpperCase(), rawMode: dto.mode === RunQueryMode.Raw }, + { command, rawMode: dto.mode === RunQueryMode.Raw }, ); if ( diff --git a/redisinsight/api/src/modules/workbench/services/workbench-analytics/workbench-analytics.service.spec.ts b/redisinsight/api/src/modules/workbench/services/workbench-analytics/workbench-analytics.service.spec.ts index bada330c1d..e109b58a6d 100644 --- a/redisinsight/api/src/modules/workbench/services/workbench-analytics/workbench-analytics.service.spec.ts +++ b/redisinsight/api/src/modules/workbench/services/workbench-analytics/workbench-analytics.service.spec.ts @@ -98,7 +98,7 @@ describe('WorkbenchAnalyticsService', () => { TelemetryEvents.WorkbenchCommandExecuted, { databaseId: instanceId, - command: 'set'.toUpperCase(), + command: 'set', commandType: CommandType.Core, moduleName: 'n/a', capability: 'string', @@ -118,7 +118,7 @@ describe('WorkbenchAnalyticsService', () => { TelemetryEvents.WorkbenchCommandExecuted, { databaseId: instanceId, - command: 'set'.toUpperCase(), + command: 'set', commandType: CommandType.Core, moduleName: 'n/a', capability: 'string', @@ -138,7 +138,7 @@ describe('WorkbenchAnalyticsService', () => { TelemetryEvents.WorkbenchCommandExecuted, { databaseId: instanceId, - command: 'set'.toUpperCase(), + command: 'set', }, ); }); @@ -153,7 +153,7 @@ describe('WorkbenchAnalyticsService', () => { TelemetryEvents.WorkbenchCommandExecuted, { databaseId: instanceId, - command: 'bF.rEsErvE'.toUpperCase(), + command: 'bF.rEsErvE', commandType: CommandType.Module, moduleName: 'redisbloom', capability: 'bf', @@ -171,7 +171,7 @@ describe('WorkbenchAnalyticsService', () => { TelemetryEvents.WorkbenchCommandExecuted, { databaseId: instanceId, - command: 'CUSTOM.COMMAnd'.toUpperCase(), + command: 'CUSTOM.COMMAnd', commandType: CommandType.Module, moduleName: 'custommodule', capability: 'n/a', @@ -189,7 +189,7 @@ describe('WorkbenchAnalyticsService', () => { TelemetryEvents.WorkbenchCommandExecuted, { databaseId: instanceId, - command: 'some.command'.toUpperCase(), + command: 'some.command', commandType: CommandType.Module, moduleName: 'custom', capability: 'n/a', @@ -221,7 +221,7 @@ describe('WorkbenchAnalyticsService', () => { { databaseId: instanceId, error: ReplyError.name, - command: 'set'.toUpperCase(), + command: 'set', commandType: CommandType.Core, moduleName: 'n/a', capability: 'string', @@ -240,7 +240,7 @@ describe('WorkbenchAnalyticsService', () => { { databaseId: instanceId, error: ReplyError.name, - command: 'sadd'.toUpperCase(), + command: 'sadd', }, ); }); diff --git a/redisinsight/api/src/modules/workbench/services/workbench-analytics/workbench-analytics.service.ts b/redisinsight/api/src/modules/workbench/services/workbench-analytics/workbench-analytics.service.ts index 447376325c..ad01e6086d 100644 --- a/redisinsight/api/src/modules/workbench/services/workbench-analytics/workbench-analytics.service.ts +++ b/redisinsight/api/src/modules/workbench/services/workbench-analytics/workbench-analytics.service.ts @@ -51,7 +51,6 @@ export class WorkbenchAnalyticsService extends CommandTelemetryBaseService { databaseId, ...(await this.getCommandAdditionalInfo(additionalData['command'])), ...additionalData, - command: additionalData['command']?.toUpperCase() || undefined, }, ); } @@ -85,18 +84,16 @@ export class WorkbenchAnalyticsService extends CommandTelemetryBaseService { { databaseId, ...additionalData, - command: additionalData['command']?.toUpperCase() || undefined, }, ); } else { - const commandFromError = error?.command?.name?.toUpperCase() || undefined; this.sendEvent( TelemetryEvents.WorkbenchCommandErrorReceived, { databaseId, error: error.name, + command: error?.command?.name, ...additionalData, - command: additionalData['command']?.toUpperCase() || commandFromError, }, ); } diff --git a/redisinsight/api/src/modules/workbench/workbench.service.spec.ts b/redisinsight/api/src/modules/workbench/workbench.service.spec.ts index bd1d640ab9..0a7d3a5944 100644 --- a/redisinsight/api/src/modules/workbench/workbench.service.spec.ts +++ b/redisinsight/api/src/modules/workbench/workbench.service.spec.ts @@ -1,7 +1,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { v4 as uuidv4 } from 'uuid'; import { when } from 'jest-when'; -import { mockDatabase, mockWorkbenchAnalyticsService } from 'src/__mocks__'; +import { mockDatabase, mockDatabaseConnectionService, mockWorkbenchAnalyticsService } from 'src/__mocks__'; import { IFindRedisClientInstanceByOptions } from 'src/modules/redis/redis.service'; import { WorkbenchService } from 'src/modules/workbench/workbench.service'; import { WorkbenchCommandsExecutor } from 'src/modules/workbench/providers/workbench-commands.executor'; @@ -17,6 +17,7 @@ import { CommandExecutionResult } from 'src/modules/workbench/models/command-exe import { CommandExecutionStatus } from 'src/modules/cli/dto/cli.dto'; import { BadRequestException, InternalServerErrorException } from '@nestjs/common'; import ERROR_MESSAGES from 'src/constants/error-messages'; +import { DatabaseConnectionService } from 'src/modules/database/database-connection.service'; import { CreateCommandExecutionsDto } from 'src/modules/workbench/dto/create-command-executions.dto'; import { WorkbenchAnalyticsService } from './services/workbench-analytics/workbench-analytics.service'; @@ -126,6 +127,10 @@ describe('WorkbenchService', () => { provide: CommandExecutionProvider, useFactory: mockCommandExecutionProvider, }, + { + provide: DatabaseConnectionService, + useFactory: mockDatabaseConnectionService, + }, ], }).compile(); diff --git a/redisinsight/api/src/modules/workbench/workbench.service.ts b/redisinsight/api/src/modules/workbench/workbench.service.ts index bcfac4cf33..8623c353a7 100644 --- a/redisinsight/api/src/modules/workbench/workbench.service.ts +++ b/redisinsight/api/src/modules/workbench/workbench.service.ts @@ -10,12 +10,15 @@ import { getBlockingCommands, multilineCommandToOneLine } from 'src/utils/cli-he import ERROR_MESSAGES from 'src/constants/error-messages'; import { ShortCommandExecution } from 'src/modules/workbench/models/short-command-execution'; import { CommandExecutionStatus } from 'src/modules/cli/dto/cli.dto'; +import { DatabaseConnectionService } from 'src/modules/database/database-connection.service'; +import { AppTool } from 'src/models'; import { getUnsupportedCommands } from './utils/getUnsupportedCommands'; import { WorkbenchAnalyticsService } from './services/workbench-analytics/workbench-analytics.service'; @Injectable() export class WorkbenchService { constructor( + private readonly databaseConnectionService: DatabaseConnectionService, private commandsExecutor: WorkbenchCommandsExecutor, private commandExecutionProvider: CommandExecutionProvider, private analyticsService: WorkbenchAnalyticsService, @@ -127,6 +130,13 @@ export class WorkbenchService { clientOptions: IFindRedisClientInstanceByOptions, dto: CreateCommandExecutionsDto, ): Promise { + // todo: handle concurrent client creation on RedisModule side + // temporary workaround. Just create client before any command execution precess + await this.databaseConnectionService.getOrCreateClient({ + databaseId: clientOptions.instanceId, + namespace: AppTool.Workbench, + }); + if (dto.resultsMode === ResultsMode.GroupMode) { return this.commandExecutionProvider.createMany( [await this.createCommandsExecution(clientOptions, dto, dto.commands)], diff --git a/redisinsight/api/src/utils/cli-helper.spec.ts b/redisinsight/api/src/utils/cli-helper.spec.ts index acfd6c0091..2f24ed4b38 100644 --- a/redisinsight/api/src/utils/cli-helper.spec.ts +++ b/redisinsight/api/src/utils/cli-helper.spec.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/quotes */ import { randomBytes } from 'crypto'; import ERROR_MESSAGES from 'src/constants/error-messages'; import { CommandParsingError, RedirectionParsingError } from 'src/modules/cli/constants/errors'; @@ -20,64 +21,59 @@ import { describe('Cli helper', () => { describe('splitCliCommandLine', () => { - it('should correctly split simple command with args', () => { - const input = 'memory usage key'; - - const output = splitCliCommandLine(input); - - expect(output).toEqual(['memory', 'usage', 'key']); - }); - it('should correctly split command with special symbols in the args in the double quotes', () => { - const input = 'set test "—"'; - - const output = splitCliCommandLine(input); - const buffer = Buffer.from('e28094', 'hex'); - expect(output).toEqual(['set', 'test', buffer]); - }); - // todo: enable after review splitCliCommandLine functionality - xit('should correctly split command with special symbols in the args in the single quotes', () => { - const input = "set test '—'"; - - const output = splitCliCommandLine(input); - - const buffer = Buffer.from('e28094', 'hex'); - expect(output).toEqual(['set', 'test', buffer]); - }); - it('should correctly split simple command without args', () => { - const input = 'info'; - - const output = splitCliCommandLine(input); - - expect(output).toEqual(['info']); - }); - it('should correctly split command with double quotes', () => { - const input = 'get "key name"'; - - const output = splitCliCommandLine(input); - expect(output).toEqual(['get', Buffer.from('key name')]); - }); - it('should correctly split command with single quotes', () => { - const input = "get 'key name'"; - - const output = splitCliCommandLine(input); - - expect(output).toEqual(['get', 'key name']); - }); - it('should correctly handle special character', () => { - const input = 'set key "\\a\\b\\t\\n\\r"'; - const output = splitCliCommandLine(input); - - expect(output).toEqual([ - 'set', - 'key', - Buffer.alloc(5, String.fromCharCode(7, 8, 9, 10, 13)), - ]); - }); - it('should correctly handle hexadecimal', () => { - const input = 'set key "\\xac\\xed"'; - const output = splitCliCommandLine(input); - - expect(output).toEqual(['set', 'key', Buffer.from([172, 237])]); + [ + { + input: 'memory usage key', + output: ['memory', 'usage', 'key'], + }, + { + input: 'set test "—"', + output: ['set', 'test', '—'], + }, + { + input: "set test '—'", + output: ['set', 'test', '—'], + }, + { + input: 'info', + output: ['info'], + }, + { + input: 'get "key name"', + output: ['get', 'key name'], + }, + { + input: `get "key ' name"`, + output: ['get', `key ' name`], + }, + { + input: `get "key \\" name"`, + output: ['get', `key " name`], + }, + { + input: "get 'key name'", + output: ['get', 'key name'], + }, + { + input: `s"et" ~\\'\\nk"k "ey' 1`, + output: ['set', `~\\\\nk"k "ey`, '1'], + }, + { + input: 'set key "\\a\\b\\t\\n\\r"', + output: ['set', 'key', `\u0007\u0008\u0009\n\r`], + }, + { + input: 'set key "\\xac\\xed"', + output: ['set', 'key', Buffer.from([172, 237])], + }, + { + input: `ACL SETUSER t on nopass ~'\\x00' &* +@all`, + output: ['ACL', 'SETUSER', 't', 'on', 'nopass', '~\\x00', '&*', '+@all'], + }, + ].forEach((tc) => { + it(`should return ${JSON.stringify(tc.output)} for command ${tc.input}`, async () => { + expect(splitCliCommandLine(tc.input)).toEqual(tc.output); + }); }); it('should throw [CLI_INVALID_QUOTES_CLOSING] error for command with double quotes', () => { const input = 'get "key"a'; diff --git a/redisinsight/api/src/utils/cli-helper.ts b/redisinsight/api/src/utils/cli-helper.ts index 43c1e96108..0c7b3b54b2 100644 --- a/redisinsight/api/src/utils/cli-helper.ts +++ b/redisinsight/api/src/utils/cli-helper.ts @@ -63,6 +63,17 @@ function getSpecChar(str: string): string { return char; } +export const convertToStringIfPossible = (data: any) => { + if (data instanceof Buffer) { + const str = data.toString(); + if (Buffer.compare(data, Buffer.from(str)) === 0) { + return str; + } + } + + return data; +}; + // todo: review/rewrite this function. Pay attention on handling data inside '' vs "" // todo: rethink implementation. set key {value} where {value} is string ~500KB take ~15s export const splitCliCommandLine = (line: string): string[] => { @@ -70,7 +81,7 @@ export const splitCliCommandLine = (line: string): string[] => { // Ported from sdssplitargs() function in sds.c from Redis source code. // This is the function redis-cli uses to parse command lines. let i = 0; - let currentArg = null; + let currentArg: any = ''; const args = []; while (i < line.length) { /* skip blanks */ @@ -141,19 +152,19 @@ export const splitCliCommandLine = (line: string): string[] => { } else if ([' ', '\n', '\r', '\t', '\0'].includes(line[i])) { done = true; } else if (line[i] === '"') { - currentArg = Buffer.alloc(0); + currentArg = Buffer.from(currentArg); inq = true; } else if (line[i] === "'") { - currentArg = ''; insq = true; } else { currentArg = `${currentArg || ''}${line[i]}`; } if (i < line.length) i += 1; } - args.push(currentArg); - currentArg = null; + args.push(convertToStringIfPossible(currentArg)); + currentArg = ''; } + return args; }; diff --git a/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts b/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts index e58bdd3e09..f4fa0dfa30 100644 --- a/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts +++ b/redisinsight/api/test/api/redisearch/POST-databases-id-redisearch-search.test.ts @@ -45,6 +45,7 @@ const mainCheckFn = getMainCheckFn(endpoint); describe('POST /databases/:id/redisearch/search', () => { requirements('!rte.bigData', 'rte.modules.search'); before(async () => rte.data.generateRedisearchIndexes(true)); + beforeEach(() => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '10000')); describe('Main', () => { describe('Validation', () => { @@ -64,7 +65,7 @@ describe('POST /databases/:id/redisearch/search', () => { expect(body.cursor).to.eq(10); expect(body.scanned).to.eq(10); expect(body.total).to.eq(2000); - expect(body.maxResults).to.eq(10000); + expect(body.maxResults).to.gte(10000); }, }, { @@ -80,12 +81,32 @@ describe('POST /databases/:id/redisearch/search', () => { expect(body.cursor).to.eq(110); expect(body.scanned).to.eq(110); expect(body.total).to.eq(2000); - expect(body.maxResults).to.eq(10000); + expect(body.maxResults).to.gte(10000); }, }, + { + name: 'Should modify limit to not exceed available search limitation', + data: { + ...validInputData, + offset: 0, + limit: 10, + }, + checkFn: async ({ body }) => { + expect(body.keys.length).to.eq(1); + expect(body.cursor).to.eq(10); + expect(body.scanned).to.eq(1); + expect(body.total).to.eq(2000); + expect(body.maxResults).to.gte(1); + }, + before: () => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '1'), + }, { name: 'Should return custom error message if MAXSEARCHRESULTS less than request.limit', - data: validInputData, + data: { + ...validInputData, + offset: 10, + limit: 10, + }, statusCode: 400, responseBody: { statusCode: 400, @@ -93,7 +114,6 @@ describe('POST /databases/:id/redisearch/search', () => { message: `Set MAXSEARCHRESULTS to at least ${numberWithSpaces(validInputData.limit)}.`, }, before: () => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '1'), - after: () => rte.data.setRedisearchConfig('MAXSEARCHRESULTS', '10000'), }, ].map(mainCheckFn); }); diff --git a/redisinsight/ui/src/components/query-card/QueryCard.tsx b/redisinsight/ui/src/components/query-card/QueryCard.tsx index 5d444d716b..64e1f8869c 100644 --- a/redisinsight/ui/src/components/query-card/QueryCard.tsx +++ b/redisinsight/ui/src/components/query-card/QueryCard.tsx @@ -179,7 +179,7 @@ const QueryCard = (props: Props) => { /> {isOpen && ( <> - {React.isValidElement(commonError) + {React.isValidElement(commonError) && !isGroupMode(resultsMode) ? : ( <> diff --git a/redisinsight/ui/src/components/query-card/QueryCardCliGroupResult/QueryCardCliGroupResult.tsx b/redisinsight/ui/src/components/query-card/QueryCardCliGroupResult/QueryCardCliGroupResult.tsx index f53b5bc077..858e43cf33 100644 --- a/redisinsight/ui/src/components/query-card/QueryCardCliGroupResult/QueryCardCliGroupResult.tsx +++ b/redisinsight/ui/src/components/query-card/QueryCardCliGroupResult/QueryCardCliGroupResult.tsx @@ -2,8 +2,9 @@ import { flatten } from 'lodash' import React from 'react' import { CommandExecutionResult } from 'uiSrc/slices/interfaces' -import { cliParseCommandsGroupResult, Maybe } from 'uiSrc/utils' +import { cliParseCommandsGroupResult, wbSummaryCommand, Maybe } from 'uiSrc/utils' import QueryCardCliDefaultResult from '../QueryCardCliDefaultResult' +import { CommonErrorResponse } from '../QueryCardCommonResult' export interface Props { result?: Maybe @@ -12,12 +13,18 @@ export interface Props { const QueryCardCliGroupResult = (props: Props) => { const { result = [], isFullScreen } = props + return (
- flatten(cliParseCommandsGroupResult(item))))} + items={flatten(result?.[0]?.response.map((item: any) => { + const commonError = CommonErrorResponse(item.command, item.response) + if (React.isValidElement(commonError)) { + return ([wbSummaryCommand(item.command), commonError]) + } + return flatten(cliParseCommandsGroupResult(item)) + }))} />
) diff --git a/redisinsight/ui/src/components/query-card/QueryCardHeader/QueryCardHeader.tsx b/redisinsight/ui/src/components/query-card/QueryCardHeader/QueryCardHeader.tsx index f4fbfcad02..b10d6a8aec 100644 --- a/redisinsight/ui/src/components/query-card/QueryCardHeader/QueryCardHeader.tsx +++ b/redisinsight/ui/src/components/query-card/QueryCardHeader/QueryCardHeader.tsx @@ -292,7 +292,7 @@ const QueryCardHeader = (props: Props) => { {isNumber(executionTime) && ( { history.push(Pages.browser(instanceId)) } + if (!data?.topMemoryNsp || data?.totalKeys?.total === 0) { + return null + } + if (!data?.topMemoryNsp?.length && !data?.topKeysNsp?.length) { return (
diff --git a/redisinsight/ui/src/slices/browser/redisearch.ts b/redisinsight/ui/src/slices/browser/redisearch.ts index e779e3261b..eec5b69454 100644 --- a/redisinsight/ui/src/slices/browser/redisearch.ts +++ b/redisinsight/ui/src/slices/browser/redisearch.ts @@ -5,7 +5,7 @@ import { remove } from 'lodash' import successMessages from 'uiSrc/components/notifications/success-messages' import { ApiEndpoints } from 'uiSrc/constants' import { apiService } from 'uiSrc/services' -import { bufferToString, getApiErrorMessage, getUrl, isEqualBuffers, isStatusSuccessful, Nullable } from 'uiSrc/utils' +import { bufferToString, getApiErrorMessage, getUrl, isEqualBuffers, isStatusSuccessful, Maybe, Nullable } from 'uiSrc/utils' import { DEFAULT_SEARCH_MATCH } from 'uiSrc/constants/api' import { IKeyPropTypes } from 'uiSrc/constants/prop-types/keys' import ApiErrors from 'uiSrc/constants/apiErrors' @@ -67,8 +67,10 @@ const redisearchSlice = createSlice({ state.isSearched = isSearched state.data.lastRefreshTime = Date.now() }, - loadKeysFailure: (state, { payload }) => { - state.error = payload + loadKeysFailure: (state, { payload }: PayloadAction>) => { + if (payload) { + state.error = payload + } state.loading = false }, @@ -86,9 +88,11 @@ const redisearchSlice = createSlice({ state.loading = false }, - loadMoreKeysFailure: (state, { payload }) => { + loadMoreKeysFailure: (state, { payload }: PayloadAction>) => { + if (payload) { + state.error = payload + } state.loading = false - state.error = payload }, // load list of indexes @@ -278,6 +282,8 @@ export function fetchRedisearchKeysAction( dispatch(fetchRedisearchListAction()) } onFailed?.() + } else { + dispatch(loadKeysFailure()) } } } @@ -326,6 +332,8 @@ export function fetchMoreRedisearchKeysAction( const errorMessage = getApiErrorMessage(error) dispatch(addErrorNotification(error)) dispatch(loadMoreKeysFailure(errorMessage)) + } else { + dispatch(loadMoreKeysFailure()) } } } diff --git a/redisinsight/ui/src/slices/tests/browser/redisearch.spec.ts b/redisinsight/ui/src/slices/tests/browser/redisearch.spec.ts index 9a2efd7585..671a483be4 100644 --- a/redisinsight/ui/src/slices/tests/browser/redisearch.spec.ts +++ b/redisinsight/ui/src/slices/tests/browser/redisearch.spec.ts @@ -171,6 +171,33 @@ describe('redisearch slice', () => { }) expect(redisearchSelector(rootState)).toEqual(state) }) + + it('should not properly set the error if no payload', () => { + // Arrange + const state = { + ...initialState, + loading: false, + error: initialState.error, + data: { + ...initialState.data, + keys: [], + nextCursor: '0', + total: 0, + scanned: 0, + shardsMeta: {}, + previousResultCount: 0, + }, + } + + // Act + const nextState = reducer(initialState, loadKeysFailure()) + + // Assert + const rootState = Object.assign(initialStateDefault, { + browser: { redisearch: nextState }, + }) + expect(redisearchSelector(rootState)).toEqual(state) + }) }) describe('loadMoreKeys', () => { @@ -298,6 +325,33 @@ describe('redisearch slice', () => { }) expect(redisearchSelector(rootState)).toEqual(state) }) + + it('should not properly set the error if no payload', () => { + // Arrange + const state = { + ...initialState, + loading: false, + error: initialState.error, + data: { + ...initialState.data, + keys: [], + nextCursor: '0', + total: 0, + scanned: 0, + shardsMeta: {}, + previousResultCount: 0, + }, + } + + // Act + const nextState = reducer(initialState, loadMoreKeysFailure()) + + // Assert + const rootState = Object.assign(initialStateDefault, { + browser: { redisearch: nextState }, + }) + expect(redisearchSelector(rootState)).toEqual(state) + }) }) describe('loadList', () => { diff --git a/redisinsight/ui/src/slices/tests/workbench/wb-results.spec.ts b/redisinsight/ui/src/slices/tests/workbench/wb-results.spec.ts index 0addd27b5c..a1627987bb 100644 --- a/redisinsight/ui/src/slices/tests/workbench/wb-results.spec.ts +++ b/redisinsight/ui/src/slices/tests/workbench/wb-results.spec.ts @@ -17,6 +17,7 @@ import reducer, { sendWBCommand, sendWBCommandSuccess, processWBCommandFailure, + processWBCommandsFailure, workbenchResultsSelector, sendWBCommandAction, sendWBCommandClusterAction, @@ -207,6 +208,41 @@ describe('workbench results slice', () => { }) }) + describe('processWBCommandsFailure', () => { + it('should properly remove from items', () => { + // Arrange + const data = 'error' + const mockCommandExecution = { + commandsId: [mockItemId + 0], + error: data, + } + const state = { + ...initialStateWithItems, + items: [{ + id: mockItemId + 0, + loading: false, + isOpen: true, + error: '', + result: [{ + response: data, + status: 'fail', + }] + }] + } + + // Act + const nextState = reducer(initialStateWithItems, processWBCommandsFailure(mockCommandExecution)) + + // Assert + const rootState = Object.assign(initialStateDefault, { + workbench: { + results: nextState, + }, + }) + expect(workbenchResultsSelector(rootState)).toEqual(state) + }) + }) + describe('loadWBHistorySuccess', () => { it('should properly set history items', () => { // Arrange @@ -323,7 +359,7 @@ describe('workbench results slice', () => { expect(clearStoreActions(store.getActions())).toEqual(clearStoreActions(expectedActions)) }) - it('call both sendWBCommandAction and processWBCommandFailure when fetch is fail', async () => { + it('call both sendWBCommandAction and processWBCommandsFailure when fetch is fail', async () => { // Arrange const commands = ['keys *'] const commandId = `${Date.now()}` @@ -344,7 +380,10 @@ describe('workbench results slice', () => { const expectedActions = [ sendWBCommand({ commands, commandId }), addErrorNotification(responsePayload as AxiosError), - processWBCommandFailure({ id: commandId, error: responsePayload.response.data.message }), + processWBCommandsFailure({ + commandsId: commands.map((_, i) => commandId + i), + error: responsePayload.response.data.message + }), ] expect(clearStoreActions(store.getActions())).toEqual(clearStoreActions(expectedActions)) }) @@ -438,7 +477,10 @@ describe('workbench results slice', () => { const expectedActions = [ sendWBCommand({ commands, commandId }), addErrorNotification(responsePayload as AxiosError), - processWBCommandFailure({ id: commandId, error: responsePayload.response.data.message }), + processWBCommandsFailure({ + commandsId: commands.map((_, i) => commandId + i), + error: responsePayload.response.data.message + }), ] expect(clearStoreActions(store.getActions())).toEqual(clearStoreActions(expectedActions)) }) diff --git a/redisinsight/ui/src/slices/workbench/wb-results.ts b/redisinsight/ui/src/slices/workbench/wb-results.ts index 06437f67a5..75674e3c52 100644 --- a/redisinsight/ui/src/slices/workbench/wb-results.ts +++ b/redisinsight/ui/src/slices/workbench/wb-results.ts @@ -13,6 +13,7 @@ import { isStatusSuccessful, } from 'uiSrc/utils' import { WORKBENCH_HISTORY_MAX_LENGTH } from 'uiSrc/pages/workbench/constants' +import { CommandExecutionStatus } from 'uiSrc/slices/interfaces/cli' import { CreateCommandExecutionsDto } from 'apiSrc/modules/workbench/dto/create-command-executions.dto' import { AppDispatch, RootState } from '../store' @@ -63,6 +64,29 @@ const workbenchResultsSlice = createSlice({ }) }, + processWBCommandsFailure: (state, { payload }: { payload: { commandsId: string[], error: string } }) => { + state.items = [...state.items].map((item) => { + let newItem = item + payload.commandsId.forEach(() => { + if (payload.commandsId.indexOf(item?.id as string) !== -1) { + newItem = { + ...item, + result: [{ + response: payload.error, + status: CommandExecutionStatus.Fail, + }], + loading: false, + isOpen: true, + error: '', + } + } + }) + return newItem + }) + state.loading = false + state.processing = false + }, + processWBCommandFailure: (state, { payload }: { payload: { id: string, error: string } }) => { state.items = [...state.items].map((item) => { if (item.id === payload.id) { @@ -155,6 +179,7 @@ export const { processWBCommand, fetchWBCommandSuccess, processWBCommandFailure, + processWBCommandsFailure, sendWBCommand, sendWBCommandSuccess, toggleOpenWBResult, @@ -242,7 +267,10 @@ export function sendWBCommandAction({ const error = _err as AxiosError const errorMessage = getApiErrorMessage(error) dispatch(addErrorNotification(error)) - dispatch(processWBCommandFailure({ id: commandId, error: errorMessage })) + dispatch(processWBCommandsFailure({ + commandsId: commands.map((_, i) => commandId + i), + error: errorMessage + })) onFailAction?.() } } @@ -300,7 +328,10 @@ export function sendWBCommandClusterAction({ const error = _err as AxiosError const errorMessage = getApiErrorMessage(error) dispatch(addErrorNotification(error)) - dispatch(processWBCommandFailure({ id: commandId, error: errorMessage })) + dispatch(processWBCommandsFailure({ + commandsId: commands.map((_, i) => commandId + i), + error: errorMessage + })) onFailAction?.() } } diff --git a/redisinsight/ui/src/utils/cliHelper.tsx b/redisinsight/ui/src/utils/cliHelper.tsx index 10fc584d48..2776fd7c4f 100644 --- a/redisinsight/ui/src/utils/cliHelper.tsx +++ b/redisinsight/ui/src/utils/cliHelper.tsx @@ -197,5 +197,6 @@ export { checkBlockingCommand, checkUnsupportedModuleCommand, getDbIndexFromSelectQuery, - getCommandNameFromQuery + getCommandNameFromQuery, + wbSummaryCommand }