From 8608dbec86c4b1425b75c508222d016b563389a4 Mon Sep 17 00:00:00 2001 From: Artem Date: Fri, 18 Nov 2022 18:52:29 +0300 Subject: [PATCH 01/20] #RI-3816 prevent creating redis client for each command in the pipeline on "cold" run --- .../src/modules/workbench/workbench.service.spec.ts | 7 ++++++- .../api/src/modules/workbench/workbench.service.ts | 10 ++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) 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)], From 3ae485aeddad069b117272ede37a47fe980845e9 Mon Sep 17 00:00:00 2001 From: Artem Date: Fri, 18 Nov 2022 18:55:44 +0300 Subject: [PATCH 02/20] #RI-3790 Fix key name returned for keys list with search w\o glob pattern --- .../keys-business/scanner/strategies/cluster.strategy.spec.ts | 4 ++-- .../keys-business/scanner/strategies/cluster.strategy.ts | 2 +- .../scanner/strategies/standalone.strategy.spec.ts | 4 ++-- .../keys-business/scanner/strategies/standalone.strategy.ts | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) 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]); From 4ba32f881b8bccfb4876035e2840efaef2bfba6a Mon Sep 17 00:00:00 2001 From: Artem Date: Fri, 18 Nov 2022 21:06:01 +0300 Subject: [PATCH 03/20] #RI-3736 rollback implementation --- .../cli-analytics.service.spec.ts | 12 +++--- .../cli-analytics/cli-analytics.service.ts | 8 +--- .../cli-business/cli-business.service.spec.ts | 40 +++++++++---------- .../cli-business/cli-business.service.ts | 16 ++++---- .../workbench-commands.executor.spec.ts | 32 +++++++-------- .../providers/workbench-commands.executor.ts | 4 +- .../workbench-analytics.service.spec.ts | 16 ++++---- .../workbench-analytics.service.ts | 5 +-- 8 files changed, 63 insertions(+), 70 deletions(-) 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..bd1652de9c 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 @@ -257,7 +257,7 @@ describe('CliBusinessService', () => { expect(analyticsService.sendCommandExecutedEvent).toHaveBeenCalledWith( mockClientOptions.instanceId, { - command: 'memory'.toUpperCase(), + command: 'memory', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -283,7 +283,7 @@ describe('CliBusinessService', () => { expect(analyticsService.sendCommandExecutedEvent).toHaveBeenCalledWith( mockClientOptions.instanceId, { - command: 'memory'.toUpperCase(), + command: 'memory', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -307,7 +307,7 @@ describe('CliBusinessService', () => { ERROR_MESSAGES.CLI_COMMAND_NOT_SUPPORTED(command.toUpperCase()), ), { - command: 'script'.toUpperCase(), + command: 'script', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -355,7 +355,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, replyError, { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -374,7 +374,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new Error(mockENotFoundMessage), { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -394,7 +394,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new KeytarUnavailableException(), { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -416,7 +416,7 @@ describe('CliBusinessService', () => { expect(analyticsService.sendCommandExecutedEvent).toHaveBeenCalledWith( mockClientOptions.instanceId, { - command: 'info'.toUpperCase(), + command: 'info', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -491,7 +491,7 @@ describe('CliBusinessService', () => { ...mockNode, }, { - command: 'memory'.toUpperCase(), + command: 'memory', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -535,7 +535,7 @@ describe('CliBusinessService', () => { ...mockNode, }, { - command: 'info'.toUpperCase(), + command: 'info', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -565,7 +565,7 @@ describe('CliBusinessService', () => { command.toUpperCase(), )), { - command: 'script'.toUpperCase(), + command: 'script', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -699,7 +699,7 @@ describe('CliBusinessService', () => { status: CommandExecutionStatus.Success, }, { - command: 'memory'.toUpperCase(), + command: 'memory', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -740,7 +740,7 @@ describe('CliBusinessService', () => { status: CommandExecutionStatus.Success, }, { - command: 'info'.toUpperCase(), + command: 'info', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -786,7 +786,7 @@ describe('CliBusinessService', () => { status: CommandExecutionStatus.Success, }, { - command: 'set'.toUpperCase(), + command: 'set', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -831,7 +831,7 @@ describe('CliBusinessService', () => { status: CommandExecutionStatus.Success, }, { - command: 'set'.toUpperCase(), + command: 'set', outputFormat: CliOutputFormatterTypes.Text, }, ); @@ -868,7 +868,7 @@ describe('CliBusinessService', () => { status: CommandExecutionStatus.Fail, }, { - command: 'set'.toUpperCase(), + command: 'set', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -896,7 +896,7 @@ describe('CliBusinessService', () => { command.toUpperCase(), )), { - command: 'script'.toUpperCase(), + command: 'script', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -945,7 +945,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new WrongDatabaseTypeError(ERROR_MESSAGES.WRONG_DATABASE_TYPE), { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -975,7 +975,7 @@ describe('CliBusinessService', () => { ERROR_MESSAGES.CLUSTER_NODE_NOT_FOUND('127.0.0.1:7002'), ), { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -999,7 +999,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new Error(mockENotFoundMessage), { - command: 'get'.toUpperCase(), + command: 'get', outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -1023,7 +1023,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..b8a7be8f02 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 @@ -160,7 +160,7 @@ export class CliBusinessService { this.cliAnalyticsService.sendCommandExecutedEvent( clientOptions.instanceId, { - command: command?.toUpperCase(), + command, outputFormat, }, ); @@ -177,14 +177,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, }); @@ -250,7 +250,7 @@ export class CliBusinessService { this.cliAnalyticsService.sendClusterCommandExecutedEvent( clientOptions.instanceId, nodeExecReply, - { command: command?.toUpperCase(), outputFormat }, + { command, outputFormat }, ); const { response, status, host, port, @@ -266,7 +266,7 @@ export class CliBusinessService { if (error instanceof CommandParsingError || error instanceof CommandNotSupportedError) { this.cliAnalyticsService.sendCommandErrorEvent(clientOptions.instanceId, error, { - command: command?.toUpperCase(), + command, outputFormat, }); return [ @@ -333,7 +333,7 @@ export class CliBusinessService { this.cliAnalyticsService.sendClusterCommandExecutedEvent( clientOptions.instanceId, result, - { command: command?.toUpperCase(), outputFormat }, + { command, outputFormat }, ); const { host, port, error, slot, ...rest @@ -344,14 +344,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/workbench/providers/workbench-commands.executor.spec.ts b/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.spec.ts index e380dc7810..5f47d96b3d 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 @@ -131,7 +131,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -157,7 +157,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -188,7 +188,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -218,7 +218,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -248,7 +248,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: true, }, ); @@ -274,7 +274,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -299,7 +299,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -330,7 +330,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -364,7 +364,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -387,7 +387,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -410,7 +410,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -434,7 +434,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -475,7 +475,7 @@ describe('WorkbenchCommandsExecutor', () => { }, ], { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -502,7 +502,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -529,7 +529,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, rawMode: false, }, ); @@ -557,7 +557,7 @@ describe('WorkbenchCommandsExecutor', () => { status: CommandExecutionStatus.Fail, }, { - command: mockSetCommand.toUpperCase(), + command: mockSetCommand, 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..08b5207d82 100644 --- a/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.ts +++ b/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.ts @@ -102,7 +102,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 +113,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, }, ); } From c607512173a28f8849425579d6c90e58c5d09c67 Mon Sep 17 00:00:00 2001 From: Artem Date: Fri, 18 Nov 2022 21:37:43 +0300 Subject: [PATCH 04/20] #RI-3736 reimplement functionality to send all commands in uppercase --- .../modules/analytics/telemetry.base.service.spec.ts | 11 ++++++++--- .../src/modules/analytics/telemetry.base.service.ts | 7 ++++++- 2 files changed, 14 insertions(+), 4 deletions(-) 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) { From dbaaea250a25c1d317f48ce1c9880ee26eb8b002 Mon Sep 17 00:00:00 2001 From: Amir Allayarov Date: Mon, 21 Nov 2022 11:37:18 +0400 Subject: [PATCH 05/20] #RI-3822,3742,3788_fixed --- .../src/components/query-card/QueryCard.tsx | 2 +- .../QueryCardHeader/QueryCardHeader.tsx | 2 +- .../components/top-namespace/TopNamespace.tsx | 5 +++ .../slices/tests/workbench/wb-results.spec.ts | 39 +++++++++++++++++-- .../ui/src/slices/workbench/wb-results.ts | 18 ++++++++- 5 files changed, 59 insertions(+), 7 deletions(-) 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/QueryCardHeader/QueryCardHeader.tsx b/redisinsight/ui/src/components/query-card/QueryCardHeader/QueryCardHeader.tsx index fa1a001808..f830190ef1 100644 --- a/redisinsight/ui/src/components/query-card/QueryCardHeader/QueryCardHeader.tsx +++ b/redisinsight/ui/src/components/query-card/QueryCardHeader/QueryCardHeader.tsx @@ -291,7 +291,7 @@ const QueryCardHeader = (props: Props) => { {isNumber(executionTime) && ( { history.push(Pages.browser(instanceId)) } + if (isNull(data)) { + return null + } + if (!data?.topMemoryNsp?.length && !data?.topKeysNsp?.length) { return (
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..1225e411fe 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,32 @@ 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: [] + } + + // 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 +350,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 +371,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 +468,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..e2d40a3420 100644 --- a/redisinsight/ui/src/slices/workbench/wb-results.ts +++ b/redisinsight/ui/src/slices/workbench/wb-results.ts @@ -63,6 +63,13 @@ const workbenchResultsSlice = createSlice({ }) }, + // temporary solution, need improve on BE how we handle errors + processWBCommandsFailure: (state, { payload }: { payload: { commandsId: string[], error: string } }) => { + state.items = [...state.items].filter((item) => payload.commandsId.indexOf(item?.id as string) === -1) + 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 +162,7 @@ export const { processWBCommand, fetchWBCommandSuccess, processWBCommandFailure, + processWBCommandsFailure, sendWBCommand, sendWBCommandSuccess, toggleOpenWBResult, @@ -242,7 +250,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 +311,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?.() } } From abb87765be45b3c0835ba315ec966ed83fdc1539 Mon Sep 17 00:00:00 2001 From: Amir Allayarov Date: Mon, 21 Nov 2022 12:10:04 +0400 Subject: [PATCH 06/20] #RI-3822-fix check topnamespace --- .../databaseAnalysis/components/top-namespace/TopNamespace.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/redisinsight/ui/src/pages/databaseAnalysis/components/top-namespace/TopNamespace.tsx b/redisinsight/ui/src/pages/databaseAnalysis/components/top-namespace/TopNamespace.tsx index 811a2280ea..a5244a0cbe 100644 --- a/redisinsight/ui/src/pages/databaseAnalysis/components/top-namespace/TopNamespace.tsx +++ b/redisinsight/ui/src/pages/databaseAnalysis/components/top-namespace/TopNamespace.tsx @@ -1,6 +1,5 @@ import { EuiButton, EuiLink, EuiSwitch, EuiTitle } from '@elastic/eui' import cx from 'classnames' -import { isNull } from 'lodash' import React, { useEffect, useState } from 'react' import { useDispatch } from 'react-redux' import { useHistory, useParams } from 'react-router-dom' @@ -47,7 +46,7 @@ const TopNamespace = (props: Props) => { history.push(Pages.browser(instanceId)) } - if (isNull(data)) { + if (!data?.topMemoryNsp) { return null } From 18808890cacccb073033c90e3ba6626c4a92cbe0 Mon Sep 17 00:00:00 2001 From: Artem Date: Mon, 21 Nov 2022 17:17:29 +0300 Subject: [PATCH 07/20] #RI-3844 throw gateway timeout error and close the client if no response in 45 seconds for ft.search command + add error message for global request timeout handler --- redisinsight/api/config/default.ts | 1 + redisinsight/api/config/test.ts | 1 + .../interceptors/timeout.interceptor.ts | 2 +- .../api/src/constants/error-messages.ts | 1 + .../redisearch/redisearch.service.spec.ts | 17 ++++++++++++ .../services/redisearch/redisearch.service.ts | 27 ++++++++++++++++--- .../modules/database/database.controller.ts | 2 +- 7 files changed, 46 insertions(+), 5 deletions(-) diff --git a/redisinsight/api/config/default.ts b/redisinsight/api/config/default.ts index 7f089d358f..2454533097 100644 --- a/redisinsight/api/config/default.ts +++ b/redisinsight/api/config/default.ts @@ -56,6 +56,7 @@ export default { buildType: process.env.BUILD_TYPE || 'ELECTRON', appVersion: process.env.APP_VERSION || '2.0.0', requestTimeout: parseInt(process.env.REQUEST_TIMEOUT, 10) || 10000, + ftSearchRequestTimeout: parseInt(process.env.REQUEST_TIMEOUT, 10) || 45_000, excludeRoutes: [], }, sockets: { diff --git a/redisinsight/api/config/test.ts b/redisinsight/api/config/test.ts index 7b087a949c..5dd78ac8db 100644 --- a/redisinsight/api/config/test.ts +++ b/redisinsight/api/config/test.ts @@ -2,6 +2,7 @@ export default { server: { env: 'test', requestTimeout: 1000, + ftSearchRequestTimeout: 1000, }, profiler: { logFileIdleThreshold: parseInt(process.env.PROFILER_LOG_FILE_IDLE_THRESHOLD, 10) || 1000 * 2, // 3sec 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/error-messages.ts b/redisinsight/api/src/constants/error-messages.ts index 77d0b3ef96..9048b4eb0b 100644 --- a/redisinsight/api/src/constants/error-messages.ts +++ b/redisinsight/api/src/constants/error-messages.ts @@ -11,6 +11,7 @@ export default { WRONG_DATABASE_TYPE: 'Wrong database type.', CONNECTION_TIMEOUT: 'The connection has timed out, please check the connection details.', + FT_SEARCH_COMMAND_TIMED_OUT: 'Command timed out.', AUTHENTICATION_FAILED: () => 'Failed to authenticate, please check the username or password.', INCORRECT_DATABASE_URL: (url) => `Could not connect to ${url}, please check the connection details.`, INCORRECT_CERTIFICATES: (url) => `Could not connect to ${url}, please check the CA or Client certificate.`, 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 38a7f41a16..2c4679c0ad 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 @@ -2,7 +2,9 @@ import { Test, TestingModule } from '@nestjs/testing'; import { ConflictException, ForbiddenException, + GatewayTimeoutException, } from '@nestjs/common'; +import ERROR_MESSAGES from 'src/constants/error-messages'; import { when } from 'jest-when'; import { mockDatabase, @@ -281,5 +283,20 @@ describe('RedisearchService', () => { expect(e).toBeInstanceOf(ForbiddenException); } }); + it('should handle produce BadGateway issue due to no response from ft.search command', async () => { + when(nodeClient.sendCommand) + .calledWith(jasmine.objectContaining({ name: 'FT.SEARCH' })) + .mockResolvedValue(new Promise((res) => { + setTimeout(() => res([100, keyName1, keyName2]), 1100); + })); + + try { + await service.search(mockClientOptions, mockSearchRedisearchDto); + fail(); + } catch (e) { + expect(e).toBeInstanceOf(GatewayTimeoutException); + expect(e.message).toEqual(ERROR_MESSAGES.FT_SEARCH_COMMAND_TIMED_OUT); + } + }); }); }); 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 7bef02c2cb..f27ec20332 100644 --- a/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts +++ b/redisinsight/api/src/modules/browser/services/redisearch/redisearch.service.ts @@ -2,6 +2,8 @@ import { Cluster, Command, Redis } from 'ioredis'; import { uniq } from 'lodash'; import { ConflictException, + GatewayTimeoutException, + HttpException, Injectable, Logger, } from '@nestjs/common'; @@ -15,8 +17,11 @@ import { } from 'src/modules/browser/dto/redisearch'; import { GetKeysWithDetailsResponse } from 'src/modules/browser/dto'; import { plainToClass } from 'class-transformer'; +import config from 'src/utils/config'; import { BrowserToolService } from '../browser-tool/browser-tool.service'; +const serverConfig = config.get('server'); + @Injectable() export class RedisearchService { private logger = new Logger('RedisearchService'); @@ -143,9 +148,21 @@ 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]), - ); + // special workaround to avoid blocking client with ft.search command + // due to RediSearch issue + const [total, ...keyNames] = await Promise.race([ + client.sendCommand( + new Command('FT.SEARCH', [index, query, 'NOCONTENT', 'LIMIT', offset, limit]), + ), + new Promise((res, rej) => setTimeout(() => { + try { + client.disconnect(); + } catch (e) { + // ignore any error related to disconnect client + } + rej(new GatewayTimeoutException(ERROR_MESSAGES.FT_SEARCH_COMMAND_TIMED_OUT)); + }, serverConfig.ftSearchRequestTimeout)), + ]); return plainToClass(GetKeysWithDetailsResponse, { cursor: limit + offset, @@ -156,6 +173,10 @@ export class RedisearchService { } catch (e) { this.logger.error('Failed to search keys using redisearch index', e); + if (e instanceof HttpException) { + throw e; + } + throw catchAclError(e); } } 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, From 92e278581448a8d6a8d81460ff75fda12377414e Mon Sep 17 00:00:00 2001 From: Amir Allayarov Date: Tue, 22 Nov 2022 04:30:26 +0400 Subject: [PATCH 08/20] #RI-3822-fix topnamespace when total = 0 --- .../databaseAnalysis/components/top-namespace/TopNamespace.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redisinsight/ui/src/pages/databaseAnalysis/components/top-namespace/TopNamespace.tsx b/redisinsight/ui/src/pages/databaseAnalysis/components/top-namespace/TopNamespace.tsx index a5244a0cbe..ca7520b5b2 100644 --- a/redisinsight/ui/src/pages/databaseAnalysis/components/top-namespace/TopNamespace.tsx +++ b/redisinsight/ui/src/pages/databaseAnalysis/components/top-namespace/TopNamespace.tsx @@ -46,7 +46,7 @@ const TopNamespace = (props: Props) => { history.push(Pages.browser(instanceId)) } - if (!data?.topMemoryNsp) { + if (!data?.topMemoryNsp || data?.totalKeys?.total === 0) { return null } From 1aa596fbc0cc594026243d6d3da99d516aad1406 Mon Sep 17 00:00:00 2001 From: Amir Allayarov Date: Tue, 22 Nov 2022 11:47:30 +0400 Subject: [PATCH 09/20] #RI-3788-fix monitor command in group mode --- .../QueryCardCliGroupResult.tsx | 13 ++++++++++--- redisinsight/ui/src/utils/cliHelper.tsx | 3 ++- 2 files changed, 12 insertions(+), 4 deletions(-) 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/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 } From e7163b08dafb5230d3c7c2194280b07d5065ede9 Mon Sep 17 00:00:00 2001 From: Artem Date: Tue, 22 Nov 2022 10:59:58 +0300 Subject: [PATCH 10/20] fix ITests --- .../redisearch/POST-databases-id-redisearch-search.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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..347020e11e 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 @@ -64,7 +64,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,7 +80,7 @@ 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); }, }, { From d7ae94a9f73ad1165d839b2c14ccd1948cb4336d Mon Sep 17 00:00:00 2001 From: Artem Date: Tue, 22 Nov 2022 11:01:28 +0300 Subject: [PATCH 11/20] #RI-3789 fix cli command parsing --- redisinsight/api/src/utils/cli-helper.spec.ts | 112 +++++++++--------- redisinsight/api/src/utils/cli-helper.ts | 22 +++- 2 files changed, 71 insertions(+), 63 deletions(-) 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..e7b70fc23c 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,20 @@ 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)); + // args.push(currentArg); + currentArg = ''; } + // return [convertToStringIfPossible(args.pop()), ...args]; return args; }; From 9f939ade36a43271ed0a7665d755569b4047bec3 Mon Sep 17 00:00:00 2001 From: Artem Date: Tue, 22 Nov 2022 11:02:21 +0300 Subject: [PATCH 12/20] remove comments --- redisinsight/api/src/utils/cli-helper.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/redisinsight/api/src/utils/cli-helper.ts b/redisinsight/api/src/utils/cli-helper.ts index e7b70fc23c..0c7b3b54b2 100644 --- a/redisinsight/api/src/utils/cli-helper.ts +++ b/redisinsight/api/src/utils/cli-helper.ts @@ -162,10 +162,9 @@ export const splitCliCommandLine = (line: string): string[] => { if (i < line.length) i += 1; } args.push(convertToStringIfPossible(currentArg)); - // args.push(currentArg); currentArg = ''; } - // return [convertToStringIfPossible(args.pop()), ...args]; + return args; }; From e12358892f8b0504975752b42a6f8c0f7ca258c1 Mon Sep 17 00:00:00 2001 From: Amir Allayarov Date: Tue, 22 Nov 2022 14:22:48 +0400 Subject: [PATCH 13/20] #RI-3742-parse commands --- .../workbench/providers/workbench-commands.executor.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 08b5207d82..df9f7a3f10 100644 --- a/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.ts +++ b/redisinsight/api/src/modules/workbench/providers/workbench-commands.executor.ts @@ -69,10 +69,12 @@ export class WorkbenchCommandsExecutor { nodeOptions, mode, } = dto; - - const [command, ...commandArgs] = splitCliCommandLine(commandLine); + let command = commandLine; try { + const [parsedCommand, ...commandArgs] = splitCliCommandLine(commandLine); + command = parsedCommand; + if (nodeOptions) { result = [await this.sendCommandForSingleNode( clientOptions, From 972e8540c1ba6050ae02e22a9b6732cc828be1d1 Mon Sep 17 00:00:00 2001 From: Amir Allayarov Date: Tue, 22 Nov 2022 18:12:45 +0400 Subject: [PATCH 14/20] #RI-3742-resolve comments --- .../api/src/constants/telemetry-events.ts | 2 ++ .../cli-business/cli-business.service.spec.ts | 4 +++ .../cli-business/cli-business.service.ts | 7 ++-- .../workbench-commands.executor.spec.ts | 34 +++++++++++++++++++ .../providers/workbench-commands.executor.ts | 8 ++--- 5 files changed, 48 insertions(+), 7 deletions(-) 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/cli/services/cli-business/cli-business.service.spec.ts b/redisinsight/api/src/modules/cli/services/cli-business/cli-business.service.spec.ts index bd1652de9c..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'; @@ -330,6 +331,7 @@ describe('CliBusinessService', () => { ERROR_MESSAGES.CLI_UNTERMINATED_QUOTES(), ), { + command: unknownCommand, outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -591,6 +593,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new CommandParsingError(ERROR_MESSAGES.CLI_UNTERMINATED_QUOTES()), { + command: unknownCommand, outputFormat: CliOutputFormatterTypes.Raw, }, ); @@ -920,6 +923,7 @@ describe('CliBusinessService', () => { mockClientOptions.instanceId, new CommandParsingError(ERROR_MESSAGES.CLI_UNTERMINATED_QUOTES()), { + command: unknownCommand, 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 b8a7be8f02..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; @@ -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 { @@ -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 { 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 5f47d96b3d..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: { @@ -564,5 +568,35 @@ describe('WorkbenchCommandsExecutor', () => { } }); }); + 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 df9f7a3f10..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,11 +72,8 @@ export class WorkbenchCommandsExecutor { nodeOptions, mode, } = dto; - let command = commandLine; - try { - const [parsedCommand, ...commandArgs] = splitCliCommandLine(commandLine); - command = parsedCommand; + [command, ...commandArgs] = splitCliCommandLine(commandLine); if (nodeOptions) { result = [await this.sendCommandForSingleNode( From 622aeaa64503fe4d8a778431a450c4296588fdb6 Mon Sep 17 00:00:00 2001 From: Amir Allayarov Date: Tue, 22 Nov 2022 19:52:25 +0400 Subject: [PATCH 15/20] #RI-3742-update styles and error handling in worbench --- .../QueryCardHeader/styles.module.scss | 20 ++++++++---------- .../components/query-card/styles.module.scss | 11 +++++++++- .../ui/src/slices/workbench/wb-results.ts | 21 +++++++++++++++++-- 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss b/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss index c860423b40..5e52dd641f 100644 --- a/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss +++ b/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss @@ -41,13 +41,16 @@ $marginIcon: 12px; .titleWrapper { width: calc(100% - 490px); + min-width: calc(100% - 490px); - @media (min-width: $breakpoint-s) { - width: calc(100% - 529px); + @media (min-width: $breakpoint-m) { + width: calc(100% - 582px); + min-width: calc(100% - 582px); } - @media (min-width: $breakpoint-m) { - width: calc(100% - 651px); + @media (min-width: $breakpoint-l) { + width: calc(100% - 633px); + min-width: calc(100% - 633px); } } @@ -146,21 +149,16 @@ $marginIcon: 12px; min-width: 13px; width: 13px; - @media (min-width: $breakpoint-s) { + @media (min-width: $breakpoint-l) { min-width: 58px; width: 58px; } - - @media (min-width: $breakpoint-m) { - min-width: 70px; - width: 70px; - } } .executionTimeValue { display: none; - @media (min-width: $breakpoint-s) { + @media (min-width: $breakpoint-l) { display: initial; overflow: hidden; text-overflow: ellipsis; diff --git a/redisinsight/ui/src/components/query-card/styles.module.scss b/redisinsight/ui/src/components/query-card/styles.module.scss index f7171847b7..917ce443a6 100644 --- a/redisinsight/ui/src/components/query-card/styles.module.scss +++ b/redisinsight/ui/src/components/query-card/styles.module.scss @@ -2,8 +2,17 @@ @import "@elastic/eui/src/components/table/mixins"; @import "@elastic/eui/src/global_styling/index"; +$breakpoint-l: 1300px; +$breakpoint-m: 1050px; + .containerWrapper { - min-width: 590px; + min-width: 560px; + @media (min-width: $breakpoint-m) { + min-width: 650px; + } + @media (min-width: $breakpoint-l) { + min-width: 720px; + } &:nth-of-type(even) { background-color: var(--euiColorEmptyShade) !important; } diff --git a/redisinsight/ui/src/slices/workbench/wb-results.ts b/redisinsight/ui/src/slices/workbench/wb-results.ts index e2d40a3420..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,9 +64,25 @@ const workbenchResultsSlice = createSlice({ }) }, - // temporary solution, need improve on BE how we handle errors processWBCommandsFailure: (state, { payload }: { payload: { commandsId: string[], error: string } }) => { - state.items = [...state.items].filter((item) => payload.commandsId.indexOf(item?.id as string) === -1) + 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 }, From 093ca433689752b19c47915ca0e9c856eb2cbc87 Mon Sep 17 00:00:00 2001 From: Amir Allayarov Date: Tue, 22 Nov 2022 23:36:48 +0400 Subject: [PATCH 16/20] #RI-3822-fix tests --- .../query-card/QueryCardHeader/styles.module.scss | 6 +++--- .../ui/src/slices/tests/workbench/wb-results.spec.ts | 11 ++++++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss b/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss index 5e52dd641f..fd1599fba6 100644 --- a/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss +++ b/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss @@ -57,11 +57,11 @@ $marginIcon: 12px; :global(.fullscreen) .titleWrapper { width: calc(100% - 402px); - @media (min-width: $breakpoint-s) { - width: calc(100% - 447px); + @media (min-width: $breakpoint-m) { + width: calc(100% - 474px); } - @media (min-width: $breakpoint-m) { + @media (min-width: $breakpoint-l) { width: calc(100% - 519px); } } 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 1225e411fe..a1627987bb 100644 --- a/redisinsight/ui/src/slices/tests/workbench/wb-results.spec.ts +++ b/redisinsight/ui/src/slices/tests/workbench/wb-results.spec.ts @@ -218,7 +218,16 @@ describe('workbench results slice', () => { } const state = { ...initialStateWithItems, - items: [] + items: [{ + id: mockItemId + 0, + loading: false, + isOpen: true, + error: '', + result: [{ + response: data, + status: 'fail', + }] + }] } // Act From 07dbaeed1265ead80e94f145103572a560e72dab Mon Sep 17 00:00:00 2001 From: zalenskiSofteq Date: Wed, 23 Nov 2022 12:01:37 +0400 Subject: [PATCH 17/20] #RI-3844 - loading after canceling redisearch request --- .../ui/src/slices/browser/redisearch.ts | 18 +++++-- .../slices/tests/browser/redisearch.spec.ts | 54 +++++++++++++++++++ 2 files changed, 67 insertions(+), 5 deletions(-) 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', () => { From a1a0ee04d05dc746c66029493689b4933bef6c69 Mon Sep 17 00:00:00 2001 From: Amir Allayarov Date: Wed, 23 Nov 2022 14:50:28 +0400 Subject: [PATCH 18/20] #RI-3822-update styles --- .../components/query-card/QueryCardHeader/styles.module.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss b/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss index fd1599fba6..9b27e8c57c 100644 --- a/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss +++ b/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss @@ -158,7 +158,7 @@ $marginIcon: 12px; .executionTimeValue { display: none; - @media (min-width: $breakpoint-l) { + @media (min-width: $breakpoint-m) { display: initial; overflow: hidden; text-overflow: ellipsis; From a8d16c5f992e98358cb2133b699d48864896fb53 Mon Sep 17 00:00:00 2001 From: Amir Allayarov Date: Wed, 23 Nov 2022 17:44:43 +0400 Subject: [PATCH 19/20] #RI-3822-update styles --- .../query-card/QueryCardHeader/styles.module.scss | 11 +++-------- .../ui/src/components/query-card/styles.module.scss | 5 +---- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss b/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss index 9b27e8c57c..86cfc9a6dd 100644 --- a/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss +++ b/redisinsight/ui/src/components/query-card/QueryCardHeader/styles.module.scss @@ -44,13 +44,8 @@ $marginIcon: 12px; min-width: calc(100% - 490px); @media (min-width: $breakpoint-m) { - width: calc(100% - 582px); - min-width: calc(100% - 582px); - } - - @media (min-width: $breakpoint-l) { - width: calc(100% - 633px); - min-width: calc(100% - 633px); + width: calc(100% - 627px); + min-width: calc(100% - 627px); } } @@ -149,7 +144,7 @@ $marginIcon: 12px; min-width: 13px; width: 13px; - @media (min-width: $breakpoint-l) { + @media (min-width: $breakpoint-m) { min-width: 58px; width: 58px; } diff --git a/redisinsight/ui/src/components/query-card/styles.module.scss b/redisinsight/ui/src/components/query-card/styles.module.scss index 917ce443a6..443f49170f 100644 --- a/redisinsight/ui/src/components/query-card/styles.module.scss +++ b/redisinsight/ui/src/components/query-card/styles.module.scss @@ -8,10 +8,7 @@ $breakpoint-m: 1050px; .containerWrapper { min-width: 560px; @media (min-width: $breakpoint-m) { - min-width: 650px; - } - @media (min-width: $breakpoint-l) { - min-width: 720px; + min-width: 700px; } &:nth-of-type(even) { background-color: var(--euiColorEmptyShade) !important; From 028f47f95728c881e5f78508b61adeaf142f7308 Mon Sep 17 00:00:00 2001 From: Artem Date: Wed, 23 Nov 2022 16:27:09 +0300 Subject: [PATCH 20/20] #RI-3844 rework workaround for blocked client due to ft.search command --- redisinsight/api/config/default.ts | 1 - redisinsight/api/config/test.ts | 1 - .../api/src/constants/error-messages.ts | 1 - .../redisearch/redisearch.service.spec.ts | 21 ++----------- .../services/redisearch/redisearch.service.ts | 30 +++++++------------ ...OST-databases-id-redisearch-search.test.ts | 24 +++++++++++++-- 6 files changed, 34 insertions(+), 44 deletions(-) diff --git a/redisinsight/api/config/default.ts b/redisinsight/api/config/default.ts index 2454533097..7f089d358f 100644 --- a/redisinsight/api/config/default.ts +++ b/redisinsight/api/config/default.ts @@ -56,7 +56,6 @@ export default { buildType: process.env.BUILD_TYPE || 'ELECTRON', appVersion: process.env.APP_VERSION || '2.0.0', requestTimeout: parseInt(process.env.REQUEST_TIMEOUT, 10) || 10000, - ftSearchRequestTimeout: parseInt(process.env.REQUEST_TIMEOUT, 10) || 45_000, excludeRoutes: [], }, sockets: { diff --git a/redisinsight/api/config/test.ts b/redisinsight/api/config/test.ts index 5dd78ac8db..7b087a949c 100644 --- a/redisinsight/api/config/test.ts +++ b/redisinsight/api/config/test.ts @@ -2,7 +2,6 @@ export default { server: { env: 'test', requestTimeout: 1000, - ftSearchRequestTimeout: 1000, }, profiler: { logFileIdleThreshold: parseInt(process.env.PROFILER_LOG_FILE_IDLE_THRESHOLD, 10) || 1000 * 2, // 3sec diff --git a/redisinsight/api/src/constants/error-messages.ts b/redisinsight/api/src/constants/error-messages.ts index 2be9fa75fa..afd6aa4340 100644 --- a/redisinsight/api/src/constants/error-messages.ts +++ b/redisinsight/api/src/constants/error-messages.ts @@ -11,7 +11,6 @@ export default { WRONG_DATABASE_TYPE: 'Wrong database type.', CONNECTION_TIMEOUT: 'The connection has timed out, please check the connection details.', - FT_SEARCH_COMMAND_TIMED_OUT: 'Command timed out.', AUTHENTICATION_FAILED: () => 'Failed to authenticate, please check the username or password.', INCORRECT_DATABASE_URL: (url) => `Could not connect to ${url}, please check the connection details.`, INCORRECT_CERTIFICATES: (url) => `Could not connect to ${url}, please check the CA or Client certificate.`, 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 54dcbffb85..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 @@ -2,9 +2,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { ConflictException, ForbiddenException, - GatewayTimeoutException, } from '@nestjs/common'; -import ERROR_MESSAGES from 'src/constants/error-messages'; import { when } from 'jest-when'; import { mockDatabase, @@ -242,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', @@ -279,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', @@ -299,20 +297,5 @@ describe('RedisearchService', () => { expect(e).toBeInstanceOf(ForbiddenException); } }); - it('should handle produce BadGateway issue due to no response from ft.search command', async () => { - when(nodeClient.sendCommand) - .calledWith(jasmine.objectContaining({ name: 'FT.SEARCH' })) - .mockResolvedValue(new Promise((res) => { - setTimeout(() => res([100, keyName1, keyName2]), 1100); - })); - - try { - await service.search(mockClientOptions, mockSearchRedisearchDto); - fail(); - } catch (e) { - expect(e).toBeInstanceOf(GatewayTimeoutException); - expect(e.message).toEqual(ERROR_MESSAGES.FT_SEARCH_COMMAND_TIMED_OUT); - } - }); }); }); 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 ec8fb0bd36..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,7 +3,6 @@ import { toNumber, uniq } from 'lodash'; import { BadRequestException, ConflictException, - GatewayTimeoutException, HttpException, Injectable, Logger, @@ -20,11 +19,8 @@ import { GetKeysWithDetailsResponse } from 'src/modules/browser/dto'; import { RedisErrorCodes } from 'src/constants'; import { plainToClass } from 'class-transformer'; import { numberWithSpaces } from 'src/utils/base.helper'; -import config from 'src/utils/config'; import { BrowserToolService } from '../browser-tool/browser-tool.service'; -const serverConfig = config.get('server'); - @Injectable() export class RedisearchService { private logger = new Logger('RedisearchService'); @@ -152,22 +148,6 @@ export class RedisearchService { const client = await this.browserTool.getRedisClient(clientOptions); - // special workaround to avoid blocking client with ft.search command - // due to RediSearch issue - const [total, ...keyNames] = await Promise.race([ - client.sendCommand( - new Command('FT.SEARCH', [index, query, 'NOCONTENT', 'LIMIT', offset, limit]), - ), - new Promise((res, rej) => setTimeout(() => { - try { - client.disconnect(); - } catch (e) { - // ignore any error related to disconnect client - } - rej(new GatewayTimeoutException(ERROR_MESSAGES.FT_SEARCH_COMMAND_TIMED_OUT)); - }, serverConfig.ftSearchRequestTimeout)), - ]); - try { const [[, maxSearchResults]] = await client.sendCommand( // response: [ [ 'MAXSEARCHRESULTS', '10000' ] ] @@ -181,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, 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 347020e11e..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', () => { @@ -83,9 +84,29 @@ describe('POST /databases/:id/redisearch/search', () => { 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); });