From 937b69a85c6cb30c7fc0bbe2eb655906352e2322 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Mon, 17 Feb 2025 18:29:19 +0200 Subject: [PATCH 01/15] RI-6849: fallback to hello command when connecting to db --- .../api/src/constants/error-messages.ts | 1 + .../modules/database/dto/redis-info.dto.ts | 45 +++++++++++++++++++ .../providers/database-info.provider.spec.ts | 29 +++++++++++- .../providers/database-info.provider.ts | 28 +++++++++++- redisinsight/api/src/utils/converter.ts | 18 ++++++++ 5 files changed, 119 insertions(+), 2 deletions(-) diff --git a/redisinsight/api/src/constants/error-messages.ts b/redisinsight/api/src/constants/error-messages.ts index a2f729a5a7..1df215ccf1 100644 --- a/redisinsight/api/src/constants/error-messages.ts +++ b/redisinsight/api/src/constants/error-messages.ts @@ -59,6 +59,7 @@ export default { 'Key with this name does not exist or does not have an associated timeout.', SERVER_NOT_AVAILABLE: 'Server is not available. Please try again later.', REDIS_CLOUD_FORBIDDEN: 'Error fetching account details.', + NO_INFO_COMMAND_PERMISSION: 'has no permissions to run the \'info\' command', DATABASE_IS_INACTIVE: 'The database is inactive.', DATABASE_ALREADY_EXISTS: 'The database already exists.', diff --git a/redisinsight/api/src/modules/database/dto/redis-info.dto.ts b/redisinsight/api/src/modules/database/dto/redis-info.dto.ts index 8951034bac..57c71bbe85 100644 --- a/redisinsight/api/src/modules/database/dto/redis-info.dto.ts +++ b/redisinsight/api/src/modules/database/dto/redis-info.dto.ts @@ -96,3 +96,48 @@ export class RedisDatabaseModuleDto { }) ver?: number; } + +export class RedisDatabaseHelloResponse { + @ApiProperty({ + description: 'Redis database id', + type: Number, + }) + id: number; + + @ApiProperty({ + description: 'Redis database server name', + type: String, + }) + server: string; + + @ApiProperty({ + description: 'Redis database version', + type: String, + }) + version: string; + + @ApiProperty({ + description: 'Redis database proto', + type: Number, + }) + proto: number; + + @ApiProperty({ + description: 'Redis database mode', + type: String, + }) + mode: "standalone" | "sentinel" | "cluster"; + + @ApiProperty({ + description: 'Redis database role', + type: String, + }) + role: 'master' | 'slave'; + + @ApiProperty({ + description: 'Redis database modules', + type: RedisDatabaseModuleDto, + isArray: true, + }) + modules: RedisDatabaseModuleDto[] +} diff --git a/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts b/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts index 2ccbd343c9..89719bdd7e 100644 --- a/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts +++ b/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts @@ -379,13 +379,40 @@ describe('DatabaseInfoProvider', () => { nodes: [mockRedisGeneralInfo, mockRedisGeneralInfo], }); }); - it('should throw an error if no permission to run \'info\' command', async () => { + it('should get info from hello command when info command is not available', async () => { when(standaloneClient.sendCommand) .calledWith(['info'], { replyEncoding: 'utf8' }) .mockRejectedValue({ message: 'NOPERM this user has no permissions to run the \'info\' command', }); + when(standaloneClient.sendCommand) + .calledWith(['hello'], { replyEncoding: 'utf8' }) + .mockResolvedValue([ + 'version', mockRedisGeneralInfo.version, + 'server', mockRedisServerInfoDto, + ]); + + const result = await service.getRedisGeneralInfo(standaloneClient); + + expect(result).toEqual({ + version: mockRedisGeneralInfo.version, + server: mockRedisServerInfoDto, + }); + }); + it('should throw an error if no permission to run \'info\' and \'hello\' commands', async () => { + when(standaloneClient.sendCommand) + .calledWith(['info'], { replyEncoding: 'utf8' }) + .mockRejectedValue({ + message: 'NOPERM this user has no permissions to run the \'info\' command', + }); + + when(standaloneClient.sendCommand) + .calledWith(['hello'], { replyEncoding: 'utf8' }) + .mockRejectedValue({ + message: 'NOPERM this user has no permissions to run the \'hello\' command', + }); + try { await service.getRedisGeneralInfo(standaloneClient); fail('Should throw an error'); diff --git a/redisinsight/api/src/modules/database/providers/database-info.provider.ts b/redisinsight/api/src/modules/database/providers/database-info.provider.ts index 0703e14baa..319abfc9e5 100644 --- a/redisinsight/api/src/modules/database/providers/database-info.provider.ts +++ b/redisinsight/api/src/modules/database/providers/database-info.provider.ts @@ -2,18 +2,21 @@ import { Injectable } from '@nestjs/common'; import { calculateRedisHitRatio, catchAclError, + convertArrayOfKeyValuePairsToObject, convertIntToSemanticVersion, convertRedisInfoReplyToObject, } from 'src/utils'; import { AdditionalRedisModule } from 'src/modules/database/models/additional.redis.module'; import { REDIS_MODULES_COMMANDS, SUPPORTED_REDIS_MODULES } from 'src/constants'; import { get, isNil } from 'lodash'; -import { RedisDatabaseInfoResponse } from 'src/modules/database/dto/redis-info.dto'; +import { RedisDatabaseHelloResponse, RedisDatabaseInfoResponse } from 'src/modules/database/dto/redis-info.dto'; import { FeatureService } from 'src/modules/feature/feature.service'; import { KnownFeatures } from 'src/modules/feature/constants'; import { convertArrayReplyToObject, convertMultilineReplyToObject } from 'src/modules/redis/utils'; import { RedisClient, RedisClientConnectionType } from 'src/modules/redis/client'; +import ERROR_MESSAGES from 'src/constants/error-messages'; import { SessionMetadata } from 'src/common/models'; +import { plainToClass } from 'class-transformer'; @Injectable() export class DatabaseInfoProvider { @@ -161,6 +164,16 @@ export class DatabaseInfoProvider { server: serverInfo, }; } catch (error) { + if (error.message.includes(ERROR_MESSAGES.NO_INFO_COMMAND_PERMISSION)) { + // Fallback to hello command + const { version, server } = await this.getRedisHelloInfo(client); + + return { + version, + server, + }; + } + throw catchAclError(error); } } @@ -261,4 +274,17 @@ export class DatabaseInfoProvider { throw catchAclError(e); } } + + private async getRedisHelloInfo(client: RedisClient): Promise { + try { + const helloResponse = convertArrayOfKeyValuePairsToObject(await client.sendCommand( + ['hello'], + { replyEncoding: 'utf8' }, + ) as any[]); + + return plainToClass(RedisDatabaseHelloResponse, helloResponse) + } catch (e) { + throw catchAclError(e); + } + } } diff --git a/redisinsight/api/src/utils/converter.ts b/redisinsight/api/src/utils/converter.ts index a98a98bb06..774c9b4f6d 100644 --- a/redisinsight/api/src/utils/converter.ts +++ b/redisinsight/api/src/utils/converter.ts @@ -36,3 +36,21 @@ export const convertStringToNumber = (value: any, defaultValue?: number): number return num; }; + +export const convertArrayOfKeyValuePairsToObject = ( + array: any[], +): Record => + array.reduce( + (memo, current, index, array) => + index % 2 === 1 + ? memo + : { + ...memo, + [current]: Array.isArray(array[index + 1]?.[0]) + ? array[index + 1].map((element) => + convertArrayOfKeyValuePairsToObject(element), + ) + : array[index + 1], + }, + {}, + ); From 3dcffeb30499f037ee855f29599acd5047f0dfbf Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Tue, 18 Feb 2025 13:13:30 +0200 Subject: [PATCH 02/15] RI-6849: refactor and test convertArrayOfKeyValuePairsToObject --- .../providers/database-info.provider.ts | 15 ++++++---- redisinsight/api/src/utils/converter.spec.ts | 30 +++++++++++++++++++ redisinsight/api/src/utils/converter.ts | 28 +++++++---------- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/redisinsight/api/src/modules/database/providers/database-info.provider.ts b/redisinsight/api/src/modules/database/providers/database-info.provider.ts index 319abfc9e5..d65d0981af 100644 --- a/redisinsight/api/src/modules/database/providers/database-info.provider.ts +++ b/redisinsight/api/src/modules/database/providers/database-info.provider.ts @@ -277,12 +277,17 @@ export class DatabaseInfoProvider { private async getRedisHelloInfo(client: RedisClient): Promise { try { - const helloResponse = convertArrayOfKeyValuePairsToObject(await client.sendCommand( - ['hello'], - { replyEncoding: 'utf8' }, - ) as any[]); + const helloResponse = (await client.sendCommand(['hello'], { + replyEncoding: 'utf8', + })) as any[]; + + const helloInfoResponse = convertArrayOfKeyValuePairsToObject(helloResponse); + + if (helloInfoResponse.modules?.length) { + helloInfoResponse.modules = helloInfoResponse.modules.map(convertArrayOfKeyValuePairsToObject); + } - return plainToClass(RedisDatabaseHelloResponse, helloResponse) + return plainToClass(RedisDatabaseHelloResponse, helloInfoResponse) } catch (e) { throw catchAclError(e); } diff --git a/redisinsight/api/src/utils/converter.spec.ts b/redisinsight/api/src/utils/converter.spec.ts index a58a3f0082..1226527721 100644 --- a/redisinsight/api/src/utils/converter.spec.ts +++ b/redisinsight/api/src/utils/converter.spec.ts @@ -1,4 +1,5 @@ import { + convertArrayOfKeyValuePairsToObject, convertIntToSemanticVersion, convertStringToNumber, } from './converter'; @@ -49,3 +50,32 @@ describe('convertStringToNumber', () => { }); }); }); + +describe('convertArrayOfKeyValuePairsToObject', () => { + it('should convert array of key value pairs to object', () => { + const input = ['key1', 'value1', 'key2', 'value2']; + const output = { key1: 'value1', key2: 'value2' }; + + const result = convertArrayOfKeyValuePairsToObject(input); + + expect(result).toEqual(output); + }); + + it('should convert array of key value pairs to object with odd number of elements', () => { + const input = ['key1', 'value1', 'key2']; + const output = { key1: 'value1' }; + + const result = convertArrayOfKeyValuePairsToObject(input); + + expect(result).toEqual(output); + }); + + it('should convert empty array to empty object', () => { + const input: any[] = []; + const output = {}; + + const result = convertArrayOfKeyValuePairsToObject(input); + + expect(result).toEqual(output); + }); +}); diff --git a/redisinsight/api/src/utils/converter.ts b/redisinsight/api/src/utils/converter.ts index 774c9b4f6d..485f933560 100644 --- a/redisinsight/api/src/utils/converter.ts +++ b/redisinsight/api/src/utils/converter.ts @@ -37,20 +37,14 @@ export const convertStringToNumber = (value: any, defaultValue?: number): number return num; }; -export const convertArrayOfKeyValuePairsToObject = ( - array: any[], -): Record => - array.reduce( - (memo, current, index, array) => - index % 2 === 1 - ? memo - : { - ...memo, - [current]: Array.isArray(array[index + 1]?.[0]) - ? array[index + 1].map((element) => - convertArrayOfKeyValuePairsToObject(element), - ) - : array[index + 1], - }, - {}, - ); +export const convertArrayOfKeyValuePairsToObject = (array: any[]) => { + const result: Record = {}; + + for (let i = 0; i + 1 < array.length; i += 2) { + const key = array[i]; + const value = array[i + 1]; + result[key] = value; + } + + return result; +}; From 2ae9be07c9fa1a04f76015ce6d984aeac68b49e9 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Wed, 19 Feb 2025 13:00:56 +0200 Subject: [PATCH 03/15] RI-6849: expose getRedisInfo method --- .../providers/database-info.provider.spec.ts | 22 ++++++-- .../providers/database-info.provider.ts | 54 ++++++++++++------- 2 files changed, 52 insertions(+), 24 deletions(-) diff --git a/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts b/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts index 89719bdd7e..9b1576f5aa 100644 --- a/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts +++ b/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts @@ -298,8 +298,8 @@ describe('DatabaseInfoProvider', () => { describe('determineDatabaseServer', () => { it('get modules by using MODULE LIST command', async () => { - when(standaloneClient.call) - .calledWith(['info', 'server'], expect.anything()) + when(standaloneClient.sendCommand) + .calledWith(['info'], expect.anything()) .mockResolvedValue(mockRedisServerInfoResponse); const result = await service.determineDatabaseServer(standaloneClient); @@ -390,14 +390,26 @@ describe('DatabaseInfoProvider', () => { .calledWith(['hello'], { replyEncoding: 'utf8' }) .mockResolvedValue([ 'version', mockRedisGeneralInfo.version, - 'server', mockRedisServerInfoDto, + 'mode', mockRedisServerInfoDto.redis_mode, + 'role', mockRedisGeneralInfo.role, + 'server', 'redis', ]); const result = await service.getRedisGeneralInfo(standaloneClient); expect(result).toEqual({ - version: mockRedisGeneralInfo.version, - server: mockRedisServerInfoDto, + ...mockRedisGeneralInfo, + server: { + redis_mode: mockRedisServerInfoDto.redis_mode, + redis_version: mockRedisGeneralInfo.version, + server_name: 'redis', + }, + uptimeInSeconds: undefined, + totalKeys: undefined, + usedMemory: undefined, + hitRatio: undefined, + connectedClients: undefined, + cashedScripts: undefined, }); }); it('should throw an error if no permission to run \'info\' and \'hello\' commands', async () => { diff --git a/redisinsight/api/src/modules/database/providers/database-info.provider.ts b/redisinsight/api/src/modules/database/providers/database-info.provider.ts index d65d0981af..f30aa22523 100644 --- a/redisinsight/api/src/modules/database/providers/database-info.provider.ts +++ b/redisinsight/api/src/modules/database/providers/database-info.provider.ts @@ -79,10 +79,7 @@ export class DatabaseInfoProvider { */ public async determineDatabaseServer(client: RedisClient): Promise { try { - const reply = convertRedisInfoReplyToObject(await client.call( - ['info', 'server'], - { replyEncoding: 'utf8' }, - ) as string); + const reply = await this.getRedisInfo(client); return reply['server']?.redis_version; } catch (e) { // continue regardless of error @@ -138,10 +135,7 @@ export class DatabaseInfoProvider { client: RedisClient, ): Promise { try { - const info = convertRedisInfoReplyToObject(await client.sendCommand( - ['info'], - { replyEncoding: 'utf8' }, - ) as string); + const info = await this.getRedisInfo(client); const serverInfo = info['server']; const memoryInfo = info['memory']; const keyspaceInfo = info['keyspace']; @@ -164,16 +158,6 @@ export class DatabaseInfoProvider { server: serverInfo, }; } catch (error) { - if (error.message.includes(ERROR_MESSAGES.NO_INFO_COMMAND_PERMISSION)) { - // Fallback to hello command - const { version, server } = await this.getRedisHelloInfo(client); - - return { - version, - server, - }; - } - throw catchAclError(error); } } @@ -275,7 +259,39 @@ export class DatabaseInfoProvider { } } - private async getRedisHelloInfo(client: RedisClient): Promise { + public async getRedisInfo(client: RedisClient) { + try { + return convertRedisInfoReplyToObject( + (await client.sendCommand(['info'], { + replyEncoding: 'utf8', + })) as string, + ); + } catch (error) { + if (error.message.includes(ERROR_MESSAGES.NO_INFO_COMMAND_PERMISSION)) { + // Fallback to getting basic information from `hello` command + return this.getRedisHelloInfo(client); + } + + throw error; + } + } + + private async getRedisHelloInfo(client: RedisClient) { + const helloResponse = await this.getRedisHelloResponse(client); + + return { + replication: { + role: helloResponse.role, + }, + server: { + server_name: helloResponse.server, + redis_version: helloResponse.version, + redis_mode: helloResponse.mode, + }, + }; + } + + private async getRedisHelloResponse(client: RedisClient): Promise { try { const helloResponse = (await client.sendCommand(['hello'], { replyEncoding: 'utf8', From c5bb1e2a4b5c8f16219862d9bb21fcb548162839 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Thu, 20 Feb 2025 14:14:15 +0200 Subject: [PATCH 04/15] RI-6849: refactor getInfo method, expose from client --- .../autodiscovery.service.spec.ts | 18 +++-- .../autodiscovery/autodiscovery.service.ts | 8 +-- .../cluster-monitor.service.ts | 7 +- .../strategies/abstract.info.strategy.ts | 7 +- .../providers/database-info.provider.spec.ts | 54 ++++++--------- .../providers/database-info.provider.ts | 60 +--------------- .../database-overview.provider.spec.ts | 13 ++-- .../providers/database-overview.provider.ts | 12 +--- .../providers/recommendation.provider.spec.ts | 64 +++++++---------- .../providers/recommendation.provider.ts | 33 ++------- .../src/modules/redis/client/redis.client.ts | 68 ++++++++++++++++--- .../src/modules/redis/utils/keys.util.spec.ts | 16 +++-- .../api/src/modules/redis/utils/keys.util.ts | 7 +- 13 files changed, 150 insertions(+), 217 deletions(-) diff --git a/redisinsight/api/src/modules/autodiscovery/autodiscovery.service.spec.ts b/redisinsight/api/src/modules/autodiscovery/autodiscovery.service.spec.ts index caa0376475..2e966d15a8 100644 --- a/redisinsight/api/src/modules/autodiscovery/autodiscovery.service.spec.ts +++ b/redisinsight/api/src/modules/autodiscovery/autodiscovery.service.spec.ts @@ -180,6 +180,14 @@ describe('AutodiscoveryService', () => { describe('addRedisDatabase', () => { it('should create database if redis_mode is standalone', async () => { + redisClientFactory.createClient.mockResolvedValue({ + getInfo: async () => ({ + server: { + redis_mode: 'standalone', + }, + }) + }); + await service['addRedisDatabase'](mockSessionMetadata, mockAutodiscoveryEndpoint); expect(databaseService.create).toHaveBeenCalledTimes(1); @@ -193,10 +201,12 @@ describe('AutodiscoveryService', () => { }); it('should not create database if redis_mode is not standalone', async () => { - (utils.convertRedisInfoReplyToObject as jest.Mock).mockReturnValueOnce({ - server: { - redis_mode: 'cluster', - }, + redisClientFactory.createClient.mockResolvedValue({ + getInfo: async () => ({ + server: { + redis_mode: 'cluster', + }, + }) }); await service['addRedisDatabase'](mockSessionMetadata, mockAutodiscoveryEndpoint); diff --git a/redisinsight/api/src/modules/autodiscovery/autodiscovery.service.ts b/redisinsight/api/src/modules/autodiscovery/autodiscovery.service.ts index 4d2d599141..0b5c708aa9 100644 --- a/redisinsight/api/src/modules/autodiscovery/autodiscovery.service.ts +++ b/redisinsight/api/src/modules/autodiscovery/autodiscovery.service.ts @@ -1,7 +1,6 @@ import { v4 as uuidv4 } from 'uuid'; import { Injectable, Logger } from '@nestjs/common'; import { getAvailableEndpoints } from 'src/modules/autodiscovery/utils/autodiscovery.util'; -import { convertRedisInfoReplyToObject } from 'src/utils'; import config, { Config } from 'src/utils/config'; import { SettingsService } from 'src/modules/settings/settings.service'; import { Database } from 'src/modules/database/models/database'; @@ -91,12 +90,7 @@ export class AutodiscoveryService { { useRetry: false, connectionName: 'redisinsight-auto-discovery' }, ); - const info = convertRedisInfoReplyToObject( - await client.sendCommand( - ['info'], - { replyEncoding: 'utf8' }, - ) as string, - ); + const info = await client.getInfo(); if (info?.server?.redis_mode === 'standalone') { await this.databaseService.create( diff --git a/redisinsight/api/src/modules/cluster-monitor/cluster-monitor.service.ts b/redisinsight/api/src/modules/cluster-monitor/cluster-monitor.service.ts index d2b6edefd5..8f64398a14 100644 --- a/redisinsight/api/src/modules/cluster-monitor/cluster-monitor.service.ts +++ b/redisinsight/api/src/modules/cluster-monitor/cluster-monitor.service.ts @@ -2,7 +2,7 @@ import { get } from 'lodash'; import { BadRequestException, HttpException, Injectable, Logger, } from '@nestjs/common'; -import { catchAclError, convertRedisInfoReplyToObject } from 'src/utils'; +import { catchAclError } from 'src/utils'; import { IClusterInfo } from 'src/modules/cluster-monitor/strategies/cluster.info.interface'; import { ClusterNodesInfoStrategy } from 'src/modules/cluster-monitor/strategies/cluster-nodes.info.strategy'; import { ClusterShardsInfoStrategy } from 'src/modules/cluster-monitor/strategies/cluster-shards.info.strategy'; @@ -41,10 +41,7 @@ export class ClusterMonitorService { return Promise.reject(new BadRequestException('Current database is not in a cluster mode')); } - const info = convertRedisInfoReplyToObject(await client.sendCommand( - ['info', 'server'], - { replyEncoding: 'utf8' }, - ) as string); + const info = await client.getInfo(true, 'server'); const strategy = this.getClusterInfoStrategy(get(info, 'server.redis_version')); diff --git a/redisinsight/api/src/modules/cluster-monitor/strategies/abstract.info.strategy.ts b/redisinsight/api/src/modules/cluster-monitor/strategies/abstract.info.strategy.ts index ea395c341e..a4e2c0eb30 100644 --- a/redisinsight/api/src/modules/cluster-monitor/strategies/abstract.info.strategy.ts +++ b/redisinsight/api/src/modules/cluster-monitor/strategies/abstract.info.strategy.ts @@ -1,5 +1,5 @@ import { IClusterInfo } from 'src/modules/cluster-monitor/strategies/cluster.info.interface'; -import { convertRedisInfoReplyToObject, convertStringToNumber } from 'src/utils'; +import { convertStringToNumber } from 'src/utils'; import { get, map, sum } from 'lodash'; import { ClusterDetails, ClusterNodeDetails } from 'src/modules/cluster-monitor/models'; import { plainToClass } from 'class-transformer'; @@ -63,10 +63,7 @@ export abstract class AbstractInfoStrategy implements IClusterInfo { * @private */ private async getClusterNodeInfo(nodeClient: RedisClient, node): Promise { - const info = convertRedisInfoReplyToObject(await nodeClient.sendCommand( - ['info'], - { replyEncoding: 'utf8' }, - ) as string); + const info = await nodeClient.getInfo(); return { ...node, diff --git a/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts b/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts index 9b1576f5aa..9991558b88 100644 --- a/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts +++ b/redisinsight/api/src/modules/database/providers/database-info.provider.spec.ts @@ -16,6 +16,7 @@ import { RedisDatabaseInfoResponse } from 'src/modules/database/dto/redis-info.d import { ForbiddenException, InternalServerErrorException } from '@nestjs/common'; import { FeatureService } from 'src/modules/feature/feature.service'; import { DatabaseInfoProvider } from 'src/modules/database/providers/database-info.provider'; +import { convertRedisInfoReplyToObject } from 'src/utils'; const mockRedisServerInfoDto = { redis_version: '6.0.5', @@ -298,9 +299,8 @@ describe('DatabaseInfoProvider', () => { describe('determineDatabaseServer', () => { it('get modules by using MODULE LIST command', async () => { - when(standaloneClient.sendCommand) - .calledWith(['info'], expect.anything()) - .mockResolvedValue(mockRedisServerInfoResponse); + when(standaloneClient.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockRedisServerInfoResponse)); const result = await service.determineDatabaseServer(standaloneClient); @@ -336,9 +336,8 @@ describe('DatabaseInfoProvider', () => { service.getDatabasesCount = jest.fn().mockResolvedValue(16); }); it('get general info for redis standalone', async () => { - when(standaloneClient.sendCommand) - .calledWith(['info'], { replyEncoding: 'utf8' }) - .mockResolvedValue(mockStandaloneRedisInfoReply); + when(standaloneClient.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockStandaloneRedisInfoReply)); const result = await service.getRedisGeneralInfo(standaloneClient); @@ -349,9 +348,8 @@ describe('DatabaseInfoProvider', () => { }\r\n${ mockRedisClientsInfoResponse }\r\n`; - when(standaloneClient.sendCommand) - .calledWith(['info'], { replyEncoding: 'utf8' }) - .mockResolvedValue(reply); + when(standaloneClient.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(reply)); const result = await service.getRedisGeneralInfo(standaloneClient); @@ -365,10 +363,8 @@ describe('DatabaseInfoProvider', () => { }); it('get general info for redis cluster', async () => { clusterClient.nodes.mockResolvedValueOnce([standaloneClient, standaloneClient]); - when(standaloneClient.sendCommand) - .calledWith(['info'], { replyEncoding: 'utf8' }) - .mockResolvedValueOnce(mockStandaloneRedisInfoReply) - .mockResolvedValueOnce(mockStandaloneRedisInfoReply); + when(standaloneClient.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockStandaloneRedisInfoReply)) const result = await service.getRedisGeneralInfo(clusterClient); @@ -380,21 +376,18 @@ describe('DatabaseInfoProvider', () => { }); }); it('should get info from hello command when info command is not available', async () => { - when(standaloneClient.sendCommand) - .calledWith(['info'], { replyEncoding: 'utf8' }) - .mockRejectedValue({ - message: 'NOPERM this user has no permissions to run the \'info\' command', + when(standaloneClient.getInfo) + .mockResolvedValue({ + replication: { + role: mockRedisGeneralInfo.role, + }, + server: { + redis_mode: mockRedisServerInfoDto.redis_mode, + redis_version: mockRedisGeneralInfo.version, + server_name: 'redis', + }, }); - when(standaloneClient.sendCommand) - .calledWith(['hello'], { replyEncoding: 'utf8' }) - .mockResolvedValue([ - 'version', mockRedisGeneralInfo.version, - 'mode', mockRedisServerInfoDto.redis_mode, - 'role', mockRedisGeneralInfo.role, - 'server', 'redis', - ]); - const result = await service.getRedisGeneralInfo(standaloneClient); expect(result).toEqual({ @@ -413,14 +406,7 @@ describe('DatabaseInfoProvider', () => { }); }); it('should throw an error if no permission to run \'info\' and \'hello\' commands', async () => { - when(standaloneClient.sendCommand) - .calledWith(['info'], { replyEncoding: 'utf8' }) - .mockRejectedValue({ - message: 'NOPERM this user has no permissions to run the \'info\' command', - }); - - when(standaloneClient.sendCommand) - .calledWith(['hello'], { replyEncoding: 'utf8' }) + when(standaloneClient.getInfo) .mockRejectedValue({ message: 'NOPERM this user has no permissions to run the \'hello\' command', }); diff --git a/redisinsight/api/src/modules/database/providers/database-info.provider.ts b/redisinsight/api/src/modules/database/providers/database-info.provider.ts index f30aa22523..014f6e0421 100644 --- a/redisinsight/api/src/modules/database/providers/database-info.provider.ts +++ b/redisinsight/api/src/modules/database/providers/database-info.provider.ts @@ -2,21 +2,17 @@ import { Injectable } from '@nestjs/common'; import { calculateRedisHitRatio, catchAclError, - convertArrayOfKeyValuePairsToObject, convertIntToSemanticVersion, - convertRedisInfoReplyToObject, } from 'src/utils'; import { AdditionalRedisModule } from 'src/modules/database/models/additional.redis.module'; import { REDIS_MODULES_COMMANDS, SUPPORTED_REDIS_MODULES } from 'src/constants'; import { get, isNil } from 'lodash'; -import { RedisDatabaseHelloResponse, RedisDatabaseInfoResponse } from 'src/modules/database/dto/redis-info.dto'; +import { RedisDatabaseInfoResponse } from 'src/modules/database/dto/redis-info.dto'; import { FeatureService } from 'src/modules/feature/feature.service'; import { KnownFeatures } from 'src/modules/feature/constants'; import { convertArrayReplyToObject, convertMultilineReplyToObject } from 'src/modules/redis/utils'; import { RedisClient, RedisClientConnectionType } from 'src/modules/redis/client'; -import ERROR_MESSAGES from 'src/constants/error-messages'; import { SessionMetadata } from 'src/common/models'; -import { plainToClass } from 'class-transformer'; @Injectable() export class DatabaseInfoProvider { @@ -79,7 +75,7 @@ export class DatabaseInfoProvider { */ public async determineDatabaseServer(client: RedisClient): Promise { try { - const reply = await this.getRedisInfo(client); + const reply = await client.getInfo(); return reply['server']?.redis_version; } catch (e) { // continue regardless of error @@ -135,7 +131,7 @@ export class DatabaseInfoProvider { client: RedisClient, ): Promise { try { - const info = await this.getRedisInfo(client); + const info = await client.getInfo(); const serverInfo = info['server']; const memoryInfo = info['memory']; const keyspaceInfo = info['keyspace']; @@ -258,54 +254,4 @@ export class DatabaseInfoProvider { throw catchAclError(e); } } - - public async getRedisInfo(client: RedisClient) { - try { - return convertRedisInfoReplyToObject( - (await client.sendCommand(['info'], { - replyEncoding: 'utf8', - })) as string, - ); - } catch (error) { - if (error.message.includes(ERROR_MESSAGES.NO_INFO_COMMAND_PERMISSION)) { - // Fallback to getting basic information from `hello` command - return this.getRedisHelloInfo(client); - } - - throw error; - } - } - - private async getRedisHelloInfo(client: RedisClient) { - const helloResponse = await this.getRedisHelloResponse(client); - - return { - replication: { - role: helloResponse.role, - }, - server: { - server_name: helloResponse.server, - redis_version: helloResponse.version, - redis_mode: helloResponse.mode, - }, - }; - } - - private async getRedisHelloResponse(client: RedisClient): Promise { - try { - const helloResponse = (await client.sendCommand(['hello'], { - replyEncoding: 'utf8', - })) as any[]; - - const helloInfoResponse = convertArrayOfKeyValuePairsToObject(helloResponse); - - if (helloInfoResponse.modules?.length) { - helloInfoResponse.modules = helloInfoResponse.modules.map(convertArrayOfKeyValuePairsToObject); - } - - return plainToClass(RedisDatabaseHelloResponse, helloInfoResponse) - } catch (e) { - throw catchAclError(e); - } - } } diff --git a/redisinsight/api/src/modules/database/providers/database-overview.provider.spec.ts b/redisinsight/api/src/modules/database/providers/database-overview.provider.spec.ts index 1da998495c..8a334db2ad 100644 --- a/redisinsight/api/src/modules/database/providers/database-overview.provider.spec.ts +++ b/redisinsight/api/src/modules/database/providers/database-overview.provider.spec.ts @@ -10,6 +10,7 @@ import { DatabaseOverview } from 'src/modules/database/models/database-overview' import { DatabaseOverviewProvider } from 'src/modules/database/providers/database-overview.provider'; import * as Utils from 'src/modules/redis/utils/keys.util'; import { DatabaseOverviewKeyspace } from 'src/modules/database/constants/overview'; +import { convertRedisInfoReplyToObject } from 'src/utils'; const mockServerInfo = { redis_version: '6.2.4', @@ -92,10 +93,8 @@ describe('OverviewService', () => { describe('getOverview', () => { describe('Standalone', () => { it('should return proper overview', async () => { - when(standaloneClient.sendCommand) - .calledWith(['info'], { replyEncoding: 'utf8' }) - .mockResolvedValue(mockStandaloneRedisInfoReply); - + when(standaloneClient.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockStandaloneRedisInfoReply)); const result = await service.getOverview(mockClientMetadata, standaloneClient, mockCurrentKeyspace); expect(result).toEqual({ @@ -113,10 +112,8 @@ describe('OverviewService', () => { }); it('should return overview with serverName if server_name is present in redis info', async () => { const redisInfoReplyWithServerName = `${mockStandaloneRedisInfoReply.slice(0, 11)}server_name:valkey\r\n${mockStandaloneRedisInfoReply.slice(11)}`; - when(standaloneClient.sendCommand) - .calledWith(['info'], { replyEncoding: 'utf8' }) - .mockResolvedValue(redisInfoReplyWithServerName); - + when(standaloneClient.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(redisInfoReplyWithServerName)); const result = await service.getOverview(mockClientMetadata, standaloneClient, mockCurrentKeyspace); expect(result).toEqual({ diff --git a/redisinsight/api/src/modules/database/providers/database-overview.provider.ts b/redisinsight/api/src/modules/database/providers/database-overview.provider.ts index 431ad8a979..b1ccdcc3ef 100644 --- a/redisinsight/api/src/modules/database/providers/database-overview.provider.ts +++ b/redisinsight/api/src/modules/database/providers/database-overview.provider.ts @@ -8,9 +8,6 @@ import { sumBy, isNumber, } from 'lodash'; -import { - convertRedisInfoReplyToObject, -} from 'src/utils'; import { getTotalKeys, convertMultilineReplyToObject } from 'src/modules/redis/utils'; import { DatabaseOverview } from 'src/modules/database/models/database-overview'; import { ClientMetadata } from 'src/common/models'; @@ -74,13 +71,10 @@ export class DatabaseOverviewProvider { */ private async getNodeInfo(client: RedisClient) { const { host, port } = client.options; + const infoData = await client.getInfo(); + return { - ...convertRedisInfoReplyToObject( - await client.sendCommand( - ['info'], - { replyEncoding: 'utf8' }, - ) as string, - ), + ...infoData, host, port, }; diff --git a/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.spec.ts b/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.spec.ts index e5d01c7161..2d75ccdbd8 100644 --- a/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.spec.ts +++ b/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.spec.ts @@ -3,6 +3,7 @@ import { RECOMMENDATION_NAMES } from 'src/constants'; import { mockRedisNoAuthError, mockRedisNoPasswordError, mockStandaloneRedisClient } from 'src/__mocks__'; import { RecommendationProvider } from 'src/modules/recommendation/providers/recommendation.provider'; import { RedisClientConnectionType } from 'src/modules/redis/client'; +import { convertRedisInfoReplyToObject } from 'src/utils'; const mockRedisMemoryInfoResponse1: string = '# Memory\r\nnumber_of_cached_scripts:10\r\n'; const mockRedisMemoryInfoResponse2: string = '# Memory\r\nnumber_of_cached_scripts:11\r\n'; @@ -150,26 +151,22 @@ describe('RecommendationProvider', () => { describe('determineLuaScriptRecommendation', () => { it('should not return luaScript recommendation', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) - .mockResolvedValue(mockRedisMemoryInfoResponse1); + when(client.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockRedisMemoryInfoResponse1)); const luaScriptRecommendation = await service.determineLuaScriptRecommendation(client); expect(luaScriptRecommendation).toEqual(null); }); it('should return luaScript recommendation', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) - .mockResolvedValue(mockRedisMemoryInfoResponse2); - + when(client.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockRedisMemoryInfoResponse2)); const luaScriptRecommendation = await service.determineLuaScriptRecommendation(client); expect(luaScriptRecommendation).toEqual({ name: RECOMMENDATION_NAMES.LUA_SCRIPT }); }); it('should not return luaScript recommendation when info command executed with error', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) + when(client.getInfo) .mockRejectedValue('some error'); const luaScriptRecommendation = await service.determineLuaScriptRecommendation(client); @@ -204,35 +201,31 @@ describe('RecommendationProvider', () => { describe('determineLogicalDatabasesRecommendation', () => { it('should not return avoidLogicalDatabases recommendation when only one logical db', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) - .mockResolvedValue(mockRedisKeyspaceInfoResponse1); + when(client.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockRedisKeyspaceInfoResponse1)); const avoidLogicalDatabasesRecommendation = await service.determineLogicalDatabasesRecommendation(client); expect(avoidLogicalDatabasesRecommendation).toEqual(null); }); it('should not return avoidLogicalDatabases recommendation when only on logical db with keys', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) - .mockResolvedValue(mockRedisKeyspaceInfoResponse2); + when(client.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockRedisKeyspaceInfoResponse2)); const avoidLogicalDatabasesRecommendation = await service.determineLogicalDatabasesRecommendation(client); expect(avoidLogicalDatabasesRecommendation).toEqual(null); }); it('should return avoidLogicalDatabases recommendation', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) - .mockResolvedValue(mockRedisKeyspaceInfoResponse3); + when(client.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockRedisKeyspaceInfoResponse3)); const avoidLogicalDatabasesRecommendation = await service.determineLogicalDatabasesRecommendation(client); expect(avoidLogicalDatabasesRecommendation).toEqual({ name: 'avoidLogicalDatabases' }); }); it('should not return avoidLogicalDatabases recommendation when info command executed with error', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) + when(client.getInfo) .mockRejectedValue('some error'); const avoidLogicalDatabasesRecommendation = await service.determineLogicalDatabasesRecommendation(client); @@ -241,9 +234,8 @@ describe('RecommendationProvider', () => { it('should not return avoidLogicalDatabases recommendation when isCluster', async () => { client.getConnectionType = jest.fn().mockReturnValueOnce(RedisClientConnectionType.CLUSTER); - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) - .mockResolvedValue(mockRedisKeyspaceInfoResponse3); + when(client.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockRedisKeyspaceInfoResponse3)); const avoidLogicalDatabasesRecommendation = await service.determineLogicalDatabasesRecommendation(client); expect(avoidLogicalDatabasesRecommendation).toEqual(null); @@ -441,9 +433,8 @@ describe('RecommendationProvider', () => { describe('determineConnectionClientsRecommendation', () => { it('should not return connectionClients recommendation', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) - .mockResolvedValue(mockRedisClientsResponse1); + when(client.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockRedisClientsResponse1)); const connectionClientsRecommendation = await service .determineConnectionClientsRecommendation(client); @@ -451,9 +442,8 @@ describe('RecommendationProvider', () => { }); it('should return connectionClients recommendation', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) - .mockResolvedValue(mockRedisClientsResponse2); + when(client.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockRedisClientsResponse2)); const connectionClientsRecommendation = await service .determineConnectionClientsRecommendation(client); @@ -463,8 +453,7 @@ describe('RecommendationProvider', () => { it('should not return connectionClients recommendation when info command executed with error', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) + when(client.getInfo) .mockRejectedValue('some error'); const connectionClientsRecommendation = await service @@ -519,9 +508,8 @@ describe('RecommendationProvider', () => { describe('determineRedisVersionRecommendation', () => { it('should not return redis version recommendation', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) - .mockResolvedValue(mockRedisServerResponse1); + when(client.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockRedisServerResponse1)); const redisVersionRecommendation = await service .determineRedisVersionRecommendation(client); @@ -529,9 +517,8 @@ describe('RecommendationProvider', () => { }); it('should return redis version recommendation', async () => { - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) - .mockResolvedValueOnce(mockRedisServerResponse2); + when(client.getInfo) + .mockResolvedValue(convertRedisInfoReplyToObject(mockRedisServerResponse2)); const redisVersionRecommendation = await service .determineRedisVersionRecommendation(client); @@ -541,8 +528,7 @@ describe('RecommendationProvider', () => { it('should not return redis version recommendation when info command executed with error', async () => { resetAllWhenMocks(); - when(client.sendCommand) - .calledWith(expect.arrayContaining(['info']), expect.anything()) + when(client.getInfo) .mockRejectedValue('some error'); const redisVersionRecommendation = await service diff --git a/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.ts b/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.ts index 95d77087e1..18c062acb6 100644 --- a/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.ts +++ b/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.ts @@ -1,9 +1,7 @@ import { Injectable, Logger } from '@nestjs/common'; import { get } from 'lodash'; import * as semverCompare from 'node-version-compare'; -import { - convertRedisInfoReplyToObject, checkTimestamp, checkKeyspaceNotification, -} from 'src/utils'; +import { checkTimestamp } from 'src/utils'; import { RECOMMENDATION_NAMES } from 'src/constants'; import { RedisDataType } from 'src/modules/browser/keys/dto'; import { Recommendation } from 'src/modules/database-analysis/models/recommendation'; @@ -41,13 +39,7 @@ export class RecommendationProvider { redisClient: RedisClient, ): Promise { try { - const info = convertRedisInfoReplyToObject( - await redisClient.sendCommand( - ['info', 'memory'], - { replyEncoding: 'utf8' }, - ) as string, - ); - + const info = await redisClient.getInfo(true, 'memory'); const nodesNumbersOfCachedScripts = get(info, 'memory.number_of_cached_scripts'); return parseInt(nodesNumbersOfCachedScripts, 10) > LUA_SCRIPT_RECOMMENDATION_COUNT @@ -98,12 +90,7 @@ export class RecommendationProvider { return null; } try { - const info = convertRedisInfoReplyToObject( - await redisClient.sendCommand( - ['info', 'keyspace'], - { replyEncoding: 'utf8' }, - ) as string, - ); + const info = await redisClient.getInfo(true, 'keyspace'); const keyspace = get(info, 'keyspace', {}); const databasesWithKeys = Object.values(keyspace).filter((db) => { const { keys } = convertMultilineReplyToObject(db as string, ',', '='); @@ -296,12 +283,7 @@ export class RecommendationProvider { redisClient: RedisClient, ): Promise { try { - const info = convertRedisInfoReplyToObject( - await redisClient.sendCommand( - ['info', 'clients'], - { replyEncoding: 'utf8' }, - ) as string, - ); + const info = await redisClient.getInfo(true, 'clients'); const connectedClients = parseInt(get(info, 'clients.connected_clients'), 10); return connectedClients > BIG_AMOUNT_OF_CONNECTED_CLIENTS_RECOMMENDATION_CLIENTS @@ -343,12 +325,7 @@ export class RecommendationProvider { redisClient: RedisClient, ): Promise { try { - const info = convertRedisInfoReplyToObject( - await redisClient.sendCommand( - ['info', 'server'], - { replyEncoding: 'utf8' }, - ) as string, - ); + const info = await redisClient.getInfo(true, 'server'); const version = get(info, 'server.redis_version'); return semverCompare(version, REDIS_VERSION_RECOMMENDATION_VERSION) >= 0 ? null diff --git a/redisinsight/api/src/modules/redis/client/redis.client.ts b/redisinsight/api/src/modules/redis/client/redis.client.ts index 55f8fead87..437dd8a36e 100644 --- a/redisinsight/api/src/modules/redis/client/redis.client.ts +++ b/redisinsight/api/src/modules/redis/client/redis.client.ts @@ -3,8 +3,11 @@ import { isNumber } from 'lodash'; import { RedisString } from 'src/common/constants'; import apiConfig from 'src/utils/config'; import { EventEmitter2 } from '@nestjs/event-emitter'; -import { convertRedisInfoReplyToObject } from 'src/utils'; +import { convertArrayOfKeyValuePairsToObject, convertRedisInfoReplyToObject } from 'src/utils'; import * as semverCompare from 'node-version-compare'; +import { RedisDatabaseHelloResponse } from 'src/modules/database/dto/redis-info.dto'; +import ERROR_MESSAGES from 'src/constants/error-messages'; +import { plainToClass } from 'class-transformer'; const REDIS_CLIENTS_CONFIG = apiConfig.get('redis_clients'); @@ -47,7 +50,7 @@ export enum RedisFeature { export abstract class RedisClient extends EventEmitter2 { public readonly id: string; - protected info: object; + protected info: any; protected lastTimeUsed: number; @@ -143,7 +146,7 @@ export abstract class RedisClient extends EventEmitter2 { switch (feature) { case RedisFeature.HashFieldsExpiration: try { - const info = await this.getInfo(); + const info = await this.getInfo(false); return info?.['server']?.['redis_version'] && semverCompare('7.3', info['server']['redis_version']) < 1; } catch (e) { return false; @@ -157,18 +160,67 @@ export abstract class RedisClient extends EventEmitter2 { * Get redis database info * Uses cache by default * @param force + * @param infoSection - e.g. server, clients, memory, etc. */ - public async getInfo(force = false): Promise { + public async getInfo(force = true, infoSection?: string) { if (force || !this.info) { - this.info = convertRedisInfoReplyToObject(await this.call( - ['info'], - { replyEncoding: 'utf8' }, - ) as string); + try { + const infoData = convertRedisInfoReplyToObject(await this.call( + infoSection ? ['info', infoSection] : ['info'], + { replyEncoding: 'utf8' }, + ) as string); + + this.info = { + ...this.info, + ...infoData, + } + } catch (error) { + if (error.message.includes(ERROR_MESSAGES.NO_INFO_COMMAND_PERMISSION)) { + try { + // Fallback to getting basic information from `hello` command + this.info = await this.getRedisHelloInfo(); + } catch (_error) { + this.info = {}; + } + } else { + this.info = {}; + } + } } return this.info; } + private async getRedisHelloInfo() { + const helloResponse = await this.getRedisHelloResponse(); + + return { + replication: { + role: helloResponse.role, + }, + server: { + server_name: helloResponse.server, + redis_version: helloResponse.version, + redis_mode: helloResponse.mode, + }, + modules: helloResponse.modules, + }; + } + + private async getRedisHelloResponse(): Promise { + const helloResponse = (await this.sendCommand(['hello'], { + replyEncoding: 'utf8', + })) as any[]; + + const helloInfoResponse = convertArrayOfKeyValuePairsToObject(helloResponse); + + if (helloInfoResponse.modules?.length) { + helloInfoResponse.modules = helloInfoResponse.modules.map(convertArrayOfKeyValuePairsToObject); + } + + return plainToClass(RedisDatabaseHelloResponse, helloInfoResponse); + } + /** * Prepare clientMetadata to be used for generating id and other operations with clients * like: find, remove many, etc. diff --git a/redisinsight/api/src/modules/redis/utils/keys.util.spec.ts b/redisinsight/api/src/modules/redis/utils/keys.util.spec.ts index 57ef909f0a..4a75425e5c 100644 --- a/redisinsight/api/src/modules/redis/utils/keys.util.spec.ts +++ b/redisinsight/api/src/modules/redis/utils/keys.util.spec.ts @@ -4,6 +4,7 @@ import { mockRedisKeyspaceInfoResponseNoKeyspaceData, mockStandaloneRedisClient, } from 'src/__mocks__'; +import { convertRedisInfoReplyToObject } from 'src/utils'; describe('getTotalKeys', () => { beforeEach(() => { @@ -19,25 +20,26 @@ describe('getTotalKeys', () => { it('Should return total from info (when dbsize returned error)', async () => { mockStandaloneRedisClient.sendCommand.mockRejectedValueOnce(new Error('some error')); - mockStandaloneRedisClient.sendCommand.mockResolvedValueOnce(mockRedisKeyspaceInfoResponse); + mockStandaloneRedisClient.getInfo.mockResolvedValueOnce(convertRedisInfoReplyToObject(mockRedisKeyspaceInfoResponse)); expect(await getTotalKeys(mockStandaloneRedisClient)).toEqual(1); - expect(mockStandaloneRedisClient.sendCommand).toHaveBeenCalledTimes(2); + expect(mockStandaloneRedisClient.sendCommand).toHaveBeenCalledTimes(1); expect(mockStandaloneRedisClient.sendCommand).toHaveBeenNthCalledWith(1, ['dbsize'], { replyEncoding: 'utf8' }); - expect(mockStandaloneRedisClient.sendCommand) - .toHaveBeenNthCalledWith(2, ['info', 'keyspace'], { replyEncoding: 'utf8' }); + expect(mockStandaloneRedisClient.getInfo) + .toHaveBeenNthCalledWith(1, true, 'keyspace'); }); it('Should return 0 since info keyspace hasn\'t keys values', async () => { mockStandaloneRedisClient.sendCommand.mockRejectedValueOnce(new Error('some error')); - mockStandaloneRedisClient.sendCommand.mockResolvedValueOnce(mockRedisKeyspaceInfoResponseNoKeyspaceData); + mockStandaloneRedisClient.getInfo.mockResolvedValueOnce(convertRedisInfoReplyToObject(mockRedisKeyspaceInfoResponseNoKeyspaceData)); expect(await getTotalKeys(mockStandaloneRedisClient)).toEqual(0); }); it('Should return 0 since info returned empty string', async () => { mockStandaloneRedisClient.sendCommand.mockRejectedValueOnce(new Error('some error')); - mockStandaloneRedisClient.sendCommand.mockResolvedValueOnce(''); + mockStandaloneRedisClient.getInfo.mockResolvedValueOnce(convertRedisInfoReplyToObject('')); expect(await getTotalKeys(mockStandaloneRedisClient)).toEqual(0); }); it('Should return -1 when dbsize and info returned error', async () => { - mockStandaloneRedisClient.sendCommand.mockRejectedValue(new Error('some error')); + mockStandaloneRedisClient.sendCommand.mockRejectedValueOnce(new Error('some error')); + mockStandaloneRedisClient.getInfo.mockRejectedValue(new Error('some error')); expect(await getTotalKeys(mockStandaloneRedisClient)).toEqual(-1); }); }); diff --git a/redisinsight/api/src/modules/redis/utils/keys.util.ts b/redisinsight/api/src/modules/redis/utils/keys.util.ts index 5bd3d1ab73..bc603fdac0 100644 --- a/redisinsight/api/src/modules/redis/utils/keys.util.ts +++ b/redisinsight/api/src/modules/redis/utils/keys.util.ts @@ -1,16 +1,11 @@ import { get } from 'lodash'; -import { convertRedisInfoReplyToObject } from 'src/utils'; import { RedisClient } from 'src/modules/redis/client'; import { convertMultilineReplyToObject } from 'src/modules/redis/utils/reply.util'; export const getTotalKeysFromInfo = async (client: RedisClient) => { try { const currentDbIndex = await client.getCurrentDbIndex(); - const info = convertRedisInfoReplyToObject( - await client.sendCommand(['info', 'keyspace'], { - replyEncoding: 'utf8', - }) as string, - ); + const info = await client.getInfo(true, 'keyspace'); const dbInfo = get(info, 'keyspace', {}); if (!dbInfo[`db${currentDbIndex}`]) { From 5f4230fbf27e79193139fafd960bbec8b742edaf Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Thu, 20 Feb 2025 14:25:13 +0200 Subject: [PATCH 05/15] remove convertArrayOfKeyValuePairsToObject --- .../src/modules/redis/client/redis.client.ts | 7 +++-- redisinsight/api/src/utils/converter.spec.ts | 30 ------------------- redisinsight/api/src/utils/converter.ts | 12 -------- 3 files changed, 4 insertions(+), 45 deletions(-) diff --git a/redisinsight/api/src/modules/redis/client/redis.client.ts b/redisinsight/api/src/modules/redis/client/redis.client.ts index 437dd8a36e..4d71614335 100644 --- a/redisinsight/api/src/modules/redis/client/redis.client.ts +++ b/redisinsight/api/src/modules/redis/client/redis.client.ts @@ -3,7 +3,8 @@ import { isNumber } from 'lodash'; import { RedisString } from 'src/common/constants'; import apiConfig from 'src/utils/config'; import { EventEmitter2 } from '@nestjs/event-emitter'; -import { convertArrayOfKeyValuePairsToObject, convertRedisInfoReplyToObject } from 'src/utils'; +import { convertRedisInfoReplyToObject } from 'src/utils'; +import { convertArrayReplyToObject } from '../utils'; import * as semverCompare from 'node-version-compare'; import { RedisDatabaseHelloResponse } from 'src/modules/database/dto/redis-info.dto'; import ERROR_MESSAGES from 'src/constants/error-messages'; @@ -212,10 +213,10 @@ export abstract class RedisClient extends EventEmitter2 { replyEncoding: 'utf8', })) as any[]; - const helloInfoResponse = convertArrayOfKeyValuePairsToObject(helloResponse); + const helloInfoResponse = convertArrayReplyToObject(helloResponse); if (helloInfoResponse.modules?.length) { - helloInfoResponse.modules = helloInfoResponse.modules.map(convertArrayOfKeyValuePairsToObject); + helloInfoResponse.modules = helloInfoResponse.modules.map(convertArrayReplyToObject); } return plainToClass(RedisDatabaseHelloResponse, helloInfoResponse); diff --git a/redisinsight/api/src/utils/converter.spec.ts b/redisinsight/api/src/utils/converter.spec.ts index 1226527721..a58a3f0082 100644 --- a/redisinsight/api/src/utils/converter.spec.ts +++ b/redisinsight/api/src/utils/converter.spec.ts @@ -1,5 +1,4 @@ import { - convertArrayOfKeyValuePairsToObject, convertIntToSemanticVersion, convertStringToNumber, } from './converter'; @@ -50,32 +49,3 @@ describe('convertStringToNumber', () => { }); }); }); - -describe('convertArrayOfKeyValuePairsToObject', () => { - it('should convert array of key value pairs to object', () => { - const input = ['key1', 'value1', 'key2', 'value2']; - const output = { key1: 'value1', key2: 'value2' }; - - const result = convertArrayOfKeyValuePairsToObject(input); - - expect(result).toEqual(output); - }); - - it('should convert array of key value pairs to object with odd number of elements', () => { - const input = ['key1', 'value1', 'key2']; - const output = { key1: 'value1' }; - - const result = convertArrayOfKeyValuePairsToObject(input); - - expect(result).toEqual(output); - }); - - it('should convert empty array to empty object', () => { - const input: any[] = []; - const output = {}; - - const result = convertArrayOfKeyValuePairsToObject(input); - - expect(result).toEqual(output); - }); -}); diff --git a/redisinsight/api/src/utils/converter.ts b/redisinsight/api/src/utils/converter.ts index 485f933560..a98a98bb06 100644 --- a/redisinsight/api/src/utils/converter.ts +++ b/redisinsight/api/src/utils/converter.ts @@ -36,15 +36,3 @@ export const convertStringToNumber = (value: any, defaultValue?: number): number return num; }; - -export const convertArrayOfKeyValuePairsToObject = (array: any[]) => { - const result: Record = {}; - - for (let i = 0; i + 1 < array.length; i += 2) { - const key = array[i]; - const value = array[i + 1]; - result[key] = value; - } - - return result; -}; From 819e2a4732eeafd55ba3a78d2af0b05f1d6f7a6c Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Thu, 20 Feb 2025 15:24:07 +0200 Subject: [PATCH 06/15] fix failing unit test --- .../strategies/abstract.info.strategy.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/redisinsight/api/src/modules/cluster-monitor/strategies/abstract.info.strategy.spec.ts b/redisinsight/api/src/modules/cluster-monitor/strategies/abstract.info.strategy.spec.ts index c4c6a5a7c3..1a3db404c9 100644 --- a/redisinsight/api/src/modules/cluster-monitor/strategies/abstract.info.strategy.spec.ts +++ b/redisinsight/api/src/modules/cluster-monitor/strategies/abstract.info.strategy.spec.ts @@ -4,6 +4,7 @@ import { set } from 'lodash'; import { ClusterNodesInfoStrategy } from 'src/modules/cluster-monitor/strategies/cluster-nodes.info.strategy'; import { ClusterDetails, ClusterNodeDetails } from 'src/modules/cluster-monitor/models'; import { mockClusterRedisClient, mockStandaloneRedisClient, mockStandaloneRedisInfoReply } from 'src/__mocks__'; +import { convertRedisInfoReplyToObject } from 'src/utils'; const m1 = { id: 'm1', @@ -136,8 +137,8 @@ describe('AbstractInfoStrategy', () => { describe('getClusterDetails', () => { beforeEach(() => { clusterClient.sendCommand.mockResolvedValue(mockClusterInfoReply); - node1.sendCommand.mockResolvedValue(mockStandaloneRedisInfoReply); - node2.sendCommand.mockResolvedValue(mockStandaloneRedisInfoReply); + node1.getInfo.mockResolvedValue(convertRedisInfoReplyToObject(mockStandaloneRedisInfoReply)); + node2.getInfo.mockResolvedValue(convertRedisInfoReplyToObject(mockStandaloneRedisInfoReply)); }); it('should return cluster info', async () => { const info = await service.getClusterDetails(clusterClient); From e1c4e1b32bdc428c0fbab4698536923f4fb41a1c Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Thu, 20 Feb 2025 16:48:15 +0200 Subject: [PATCH 07/15] update test --- .../GET-databases-id-cluster_details.test.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/redisinsight/api/test/api/cluster-monitor/GET-databases-id-cluster_details.test.ts b/redisinsight/api/test/api/cluster-monitor/GET-databases-id-cluster_details.test.ts index 4e734e2c63..d0266054d5 100644 --- a/redisinsight/api/test/api/cluster-monitor/GET-databases-id-cluster_details.test.ts +++ b/redisinsight/api/test/api/cluster-monitor/GET-databases-id-cluster_details.test.ts @@ -130,12 +130,11 @@ describe('GET /databases/:id/cluster-details', () => { }, { before: () => rte.data.setAclUserRules('~* +@all -info'), - name: 'Should throw error if no permissions for "info" command', + name: 'Should not throw error if no permissions for "info" command', endpoint: () => endpoint(constants.TEST_INSTANCE_ACL_ID), - statusCode: 403, - responseBody: { - statusCode: 403, - error: 'Forbidden', + responseSchema, + checkFn: ({body}) => { + expect(body.state).to.eql('ok'); }, }, ].map(mainCheckFn); From 7032266bde144d11b30c0b588646b06279e754d6 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Thu, 20 Feb 2025 17:04:53 +0200 Subject: [PATCH 08/15] add default redis info --- redisinsight/api/src/common/constants/general.ts | 4 ++++ redisinsight/api/src/modules/redis/client/redis.client.ts | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/redisinsight/api/src/common/constants/general.ts b/redisinsight/api/src/common/constants/general.ts index 8812509fa5..173cc71768 100644 --- a/redisinsight/api/src/common/constants/general.ts +++ b/redisinsight/api/src/common/constants/general.ts @@ -1,3 +1,7 @@ export enum TransformGroup { Secure = 'security', } + +export const UNKNOWN_REDIS_INFO = { + version: 'unknown', +}; diff --git a/redisinsight/api/src/modules/redis/client/redis.client.ts b/redisinsight/api/src/modules/redis/client/redis.client.ts index 4d71614335..6b917682a6 100644 --- a/redisinsight/api/src/modules/redis/client/redis.client.ts +++ b/redisinsight/api/src/modules/redis/client/redis.client.ts @@ -1,6 +1,6 @@ import { ClientContext, ClientMetadata } from 'src/common/models'; import { isNumber } from 'lodash'; -import { RedisString } from 'src/common/constants'; +import { RedisString, UNKNOWN_REDIS_INFO } from 'src/common/constants'; import apiConfig from 'src/utils/config'; import { EventEmitter2 } from '@nestjs/event-emitter'; import { convertRedisInfoReplyToObject } from 'src/utils'; @@ -181,10 +181,10 @@ export abstract class RedisClient extends EventEmitter2 { // Fallback to getting basic information from `hello` command this.info = await this.getRedisHelloInfo(); } catch (_error) { - this.info = {}; + this.info = UNKNOWN_REDIS_INFO; } } else { - this.info = {}; + this.info = UNKNOWN_REDIS_INFO; } } } From 9e842b9effd902f0e97b267947878fd15bb994f3 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Thu, 20 Feb 2025 18:26:26 +0200 Subject: [PATCH 09/15] update static value --- redisinsight/api/src/common/constants/general.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/redisinsight/api/src/common/constants/general.ts b/redisinsight/api/src/common/constants/general.ts index 173cc71768..6b9ea13557 100644 --- a/redisinsight/api/src/common/constants/general.ts +++ b/redisinsight/api/src/common/constants/general.ts @@ -3,5 +3,7 @@ export enum TransformGroup { } export const UNKNOWN_REDIS_INFO = { - version: 'unknown', + server: { + version: 'unknown', + }, }; From af075db5e608402f333baaca62090f201e5cadb3 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Fri, 21 Feb 2025 09:56:30 +0200 Subject: [PATCH 10/15] update default info --- redisinsight/api/src/common/constants/general.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redisinsight/api/src/common/constants/general.ts b/redisinsight/api/src/common/constants/general.ts index 6b9ea13557..35c9aa84b2 100644 --- a/redisinsight/api/src/common/constants/general.ts +++ b/redisinsight/api/src/common/constants/general.ts @@ -4,6 +4,6 @@ export enum TransformGroup { export const UNKNOWN_REDIS_INFO = { server: { - version: 'unknown', + redis_version: 'unknown', }, }; From 021f24b95044ab3579989b27764051d8e075a5c2 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Fri, 21 Feb 2025 10:11:29 +0200 Subject: [PATCH 11/15] update default info --- redisinsight/api/src/common/constants/general.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/redisinsight/api/src/common/constants/general.ts b/redisinsight/api/src/common/constants/general.ts index 35c9aa84b2..be50ef84a6 100644 --- a/redisinsight/api/src/common/constants/general.ts +++ b/redisinsight/api/src/common/constants/general.ts @@ -5,5 +5,6 @@ export enum TransformGroup { export const UNKNOWN_REDIS_INFO = { server: { redis_version: 'unknown', + redis_mode: 'standalone', }, }; From aa1755575a7e8bdc21fa8342138c52a1f21156e3 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Fri, 21 Feb 2025 15:06:44 +0200 Subject: [PATCH 12/15] refactor getInfo --- .../cluster-monitor.service.ts | 2 +- .../providers/recommendation.provider.ts | 8 +-- .../src/modules/redis/client/redis.client.ts | 52 +++++++++---------- .../api/src/modules/redis/utils/keys.util.ts | 2 +- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/redisinsight/api/src/modules/cluster-monitor/cluster-monitor.service.ts b/redisinsight/api/src/modules/cluster-monitor/cluster-monitor.service.ts index 8f64398a14..e30ac50fcd 100644 --- a/redisinsight/api/src/modules/cluster-monitor/cluster-monitor.service.ts +++ b/redisinsight/api/src/modules/cluster-monitor/cluster-monitor.service.ts @@ -41,7 +41,7 @@ export class ClusterMonitorService { return Promise.reject(new BadRequestException('Current database is not in a cluster mode')); } - const info = await client.getInfo(true, 'server'); + const info = await client.getInfo('server'); const strategy = this.getClusterInfoStrategy(get(info, 'server.redis_version')); diff --git a/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.ts b/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.ts index 18c062acb6..28d3ff63cf 100644 --- a/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.ts +++ b/redisinsight/api/src/modules/recommendation/providers/recommendation.provider.ts @@ -39,7 +39,7 @@ export class RecommendationProvider { redisClient: RedisClient, ): Promise { try { - const info = await redisClient.getInfo(true, 'memory'); + const info = await redisClient.getInfo('memory'); const nodesNumbersOfCachedScripts = get(info, 'memory.number_of_cached_scripts'); return parseInt(nodesNumbersOfCachedScripts, 10) > LUA_SCRIPT_RECOMMENDATION_COUNT @@ -90,7 +90,7 @@ export class RecommendationProvider { return null; } try { - const info = await redisClient.getInfo(true, 'keyspace'); + const info = await redisClient.getInfo('keyspace'); const keyspace = get(info, 'keyspace', {}); const databasesWithKeys = Object.values(keyspace).filter((db) => { const { keys } = convertMultilineReplyToObject(db as string, ',', '='); @@ -283,7 +283,7 @@ export class RecommendationProvider { redisClient: RedisClient, ): Promise { try { - const info = await redisClient.getInfo(true, 'clients'); + const info = await redisClient.getInfo('clients'); const connectedClients = parseInt(get(info, 'clients.connected_clients'), 10); return connectedClients > BIG_AMOUNT_OF_CONNECTED_CLIENTS_RECOMMENDATION_CLIENTS @@ -325,7 +325,7 @@ export class RecommendationProvider { redisClient: RedisClient, ): Promise { try { - const info = await redisClient.getInfo(true, 'server'); + const info = await redisClient.getInfo('server'); const version = get(info, 'server.redis_version'); return semverCompare(version, REDIS_VERSION_RECOMMENDATION_VERSION) >= 0 ? null diff --git a/redisinsight/api/src/modules/redis/client/redis.client.ts b/redisinsight/api/src/modules/redis/client/redis.client.ts index 6b917682a6..9b36928c77 100644 --- a/redisinsight/api/src/modules/redis/client/redis.client.ts +++ b/redisinsight/api/src/modules/redis/client/redis.client.ts @@ -51,7 +51,7 @@ export enum RedisFeature { export abstract class RedisClient extends EventEmitter2 { public readonly id: string; - protected info: any; + protected _redisVersion: string | undefined; protected lastTimeUsed: number; @@ -147,8 +147,8 @@ export abstract class RedisClient extends EventEmitter2 { switch (feature) { case RedisFeature.HashFieldsExpiration: try { - const info = await this.getInfo(false); - return info?.['server']?.['redis_version'] && semverCompare('7.3', info['server']['redis_version']) < 1; + const redisVersion = await this.getRedisVersion(); + return redisVersion && semverCompare('7.3', redisVersion) < 1; } catch (e) { return false; } @@ -157,39 +157,39 @@ export abstract class RedisClient extends EventEmitter2 { } } + private async getRedisVersion(): Promise { + if (!this._redisVersion) { + const infoData = await this.getInfo('server'); + this._redisVersion = infoData?.server?.redis_version; + } + + return this._redisVersion; + } + /** * Get redis database info * Uses cache by default * @param force * @param infoSection - e.g. server, clients, memory, etc. */ - public async getInfo(force = true, infoSection?: string) { - if (force || !this.info) { - try { - const infoData = convertRedisInfoReplyToObject(await this.call( - infoSection ? ['info', infoSection] : ['info'], - { replyEncoding: 'utf8' }, - ) as string); - - this.info = { - ...this.info, - ...infoData, - } - } catch (error) { - if (error.message.includes(ERROR_MESSAGES.NO_INFO_COMMAND_PERMISSION)) { - try { - // Fallback to getting basic information from `hello` command - this.info = await this.getRedisHelloInfo(); - } catch (_error) { - this.info = UNKNOWN_REDIS_INFO; - } - } else { - this.info = UNKNOWN_REDIS_INFO; + public async getInfo(infoSection?: string) { + try { + return convertRedisInfoReplyToObject(await this.call( + infoSection ? ['info', infoSection] : ['info'], + { replyEncoding: 'utf8' }, + ) as string); + } catch (error) { + if (error.message.includes(ERROR_MESSAGES.NO_INFO_COMMAND_PERMISSION)) { + try { + // Fallback to getting basic information from `hello` command + return await this.getRedisHelloInfo(); + } catch (_error) { + // Ignore: hello is not available pre redis version 6 } } } - return this.info; + return UNKNOWN_REDIS_INFO; } private async getRedisHelloInfo() { diff --git a/redisinsight/api/src/modules/redis/utils/keys.util.ts b/redisinsight/api/src/modules/redis/utils/keys.util.ts index bc603fdac0..5254adfea9 100644 --- a/redisinsight/api/src/modules/redis/utils/keys.util.ts +++ b/redisinsight/api/src/modules/redis/utils/keys.util.ts @@ -5,7 +5,7 @@ import { convertMultilineReplyToObject } from 'src/modules/redis/utils/reply.uti export const getTotalKeysFromInfo = async (client: RedisClient) => { try { const currentDbIndex = await client.getCurrentDbIndex(); - const info = await client.getInfo(true, 'keyspace'); + const info = await client.getInfo('keyspace'); const dbInfo = get(info, 'keyspace', {}); if (!dbInfo[`db${currentDbIndex}`]) { From 6ee35d656fcc1370492b042721580af629b97c45 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Fri, 21 Feb 2025 15:18:50 +0200 Subject: [PATCH 13/15] fix failing test --- redisinsight/api/src/modules/redis/utils/keys.util.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redisinsight/api/src/modules/redis/utils/keys.util.spec.ts b/redisinsight/api/src/modules/redis/utils/keys.util.spec.ts index 4a75425e5c..5790cae8fb 100644 --- a/redisinsight/api/src/modules/redis/utils/keys.util.spec.ts +++ b/redisinsight/api/src/modules/redis/utils/keys.util.spec.ts @@ -25,7 +25,7 @@ describe('getTotalKeys', () => { expect(mockStandaloneRedisClient.sendCommand).toHaveBeenCalledTimes(1); expect(mockStandaloneRedisClient.sendCommand).toHaveBeenNthCalledWith(1, ['dbsize'], { replyEncoding: 'utf8' }); expect(mockStandaloneRedisClient.getInfo) - .toHaveBeenNthCalledWith(1, true, 'keyspace'); + .toHaveBeenNthCalledWith(1, 'keyspace'); }); it('Should return 0 since info keyspace hasn\'t keys values', async () => { mockStandaloneRedisClient.sendCommand.mockRejectedValueOnce(new Error('some error')); From 62dbbe78266fb8d55a68c3382033837c4e456845 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Tue, 25 Feb 2025 13:32:15 +0200 Subject: [PATCH 14/15] RI-6844: hide db overview metrics when data is not available (#4383) --- .../components/OverviewMetrics/OverviewMetrics.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/redisinsight/ui/src/components/database-overview/components/OverviewMetrics/OverviewMetrics.tsx b/redisinsight/ui/src/components/database-overview/components/OverviewMetrics/OverviewMetrics.tsx index c107a8b540..839b4e6ea3 100644 --- a/redisinsight/ui/src/components/database-overview/components/OverviewMetrics/OverviewMetrics.tsx +++ b/redisinsight/ui/src/components/database-overview/components/OverviewMetrics/OverviewMetrics.tsx @@ -199,7 +199,7 @@ export const getOverviewMetrics = ({ theme, items, db = 0 }: Props): Array (Number.isInteger(connectedClients) ? connectedClients : `~${Math.round(connectedClients)}`) // Connected clients - availableItems.push({ + connectedClients !== undefined && availableItems.push({ id: 'overview-connected-clients', value: connectedClients, unavailableText: 'Connected Clients are not available', From cc9693704c8fcff15fc0d26037771e24d236e557 Mon Sep 17 00:00:00 2001 From: Krum Tyukenov Date: Tue, 25 Feb 2025 13:32:45 +0200 Subject: [PATCH 15/15] RI-6848: add info_command_is_disabled to analytics when INFO is disabled via ACL (#4390) * RI-6848: add info_command_is_disabled to analytics when INFO is disabled via ACL * add isInfoCommandDisabled prop to RedisClient * preserve function signature * set default value for isInfoCommandDisabled * refactor getInfo * update: always call HELLO if INFO fails --- .../api/src/constants/error-messages.ts | 1 - .../database-connection.service.spec.ts | 2 +- .../database/database-connection.service.ts | 3 +- .../database/database.analytics.spec.ts | 2 +- .../modules/database/database.analytics.ts | 6 +--- .../src/modules/redis/client/redis.client.ts | 29 ++++++++++++------- 6 files changed, 23 insertions(+), 20 deletions(-) diff --git a/redisinsight/api/src/constants/error-messages.ts b/redisinsight/api/src/constants/error-messages.ts index 1df215ccf1..a2f729a5a7 100644 --- a/redisinsight/api/src/constants/error-messages.ts +++ b/redisinsight/api/src/constants/error-messages.ts @@ -59,7 +59,6 @@ export default { 'Key with this name does not exist or does not have an associated timeout.', SERVER_NOT_AVAILABLE: 'Server is not available. Please try again later.', REDIS_CLOUD_FORBIDDEN: 'Error fetching account details.', - NO_INFO_COMMAND_PERMISSION: 'has no permissions to run the \'info\' command', DATABASE_IS_INACTIVE: 'The database is inactive.', DATABASE_ALREADY_EXISTS: 'The database already exists.', diff --git a/redisinsight/api/src/modules/database/database-connection.service.spec.ts b/redisinsight/api/src/modules/database/database-connection.service.spec.ts index f75aef4349..018e939fac 100644 --- a/redisinsight/api/src/modules/database/database-connection.service.spec.ts +++ b/redisinsight/api/src/modules/database/database-connection.service.spec.ts @@ -143,8 +143,8 @@ describe('DatabaseConnectionService', () => { expect(databaseInfoProvider.getClientListInfo).toHaveBeenCalled(); expect(analytics.sendDatabaseConnectedClientListEvent).toHaveBeenCalledWith( mockSessionMetadata, - mockDatabase.id, { + databaseId: mockDatabase.id, clients: mockRedisClientListResult.map((c) => ({ version: mockRedisGeneralInfo.version, resp: get(c, 'resp', 'n/a'), diff --git a/redisinsight/api/src/modules/database/database-connection.service.ts b/redisinsight/api/src/modules/database/database-connection.service.ts index 627d061444..11788501aa 100644 --- a/redisinsight/api/src/modules/database/database-connection.service.ts +++ b/redisinsight/api/src/modules/database/database-connection.service.ts @@ -87,8 +87,9 @@ export class DatabaseConnectionService { this.analytics.sendDatabaseConnectedClientListEvent( clientMetadata.sessionMetadata, - clientMetadata.databaseId, { + databaseId: clientMetadata.databaseId, + ...(client.isInfoCommandDisabled ? { info_command_is_disabled: true } : {}), clients: clients.map((c) => ({ version: version || 'n/a', resp: intVersion < 7 ? undefined : c?.['resp'] || 'n/a', diff --git a/redisinsight/api/src/modules/database/database.analytics.spec.ts b/redisinsight/api/src/modules/database/database.analytics.spec.ts index 22ef4ab53d..47e3972730 100644 --- a/redisinsight/api/src/modules/database/database.analytics.spec.ts +++ b/redisinsight/api/src/modules/database/database.analytics.spec.ts @@ -326,8 +326,8 @@ describe('DatabaseAnalytics', () => { it('should emit event', () => { service.sendDatabaseConnectedClientListEvent( mockSessionMetadata, - mockDatabase.id, { + databaseId: mockDatabase.id, version: mockDatabase.version, resp: '2', }, diff --git a/redisinsight/api/src/modules/database/database.analytics.ts b/redisinsight/api/src/modules/database/database.analytics.ts index 1c35ecc278..dd62d3244e 100644 --- a/redisinsight/api/src/modules/database/database.analytics.ts +++ b/redisinsight/api/src/modules/database/database.analytics.ts @@ -126,17 +126,13 @@ export class DatabaseAnalytics extends TelemetryBaseService { sendDatabaseConnectedClientListEvent( sessionMetadata: SessionMetadata, - databaseId: string, additionalData: object = {}, ): void { try { this.sendEvent( sessionMetadata, TelemetryEvents.DatabaseConnectedClientList, - { - databaseId, - ...additionalData, - }, + additionalData, ); } catch (e) { // continue regardless of error diff --git a/redisinsight/api/src/modules/redis/client/redis.client.ts b/redisinsight/api/src/modules/redis/client/redis.client.ts index 9b36928c77..86a41c86ab 100644 --- a/redisinsight/api/src/modules/redis/client/redis.client.ts +++ b/redisinsight/api/src/modules/redis/client/redis.client.ts @@ -7,7 +7,6 @@ import { convertRedisInfoReplyToObject } from 'src/utils'; import { convertArrayReplyToObject } from '../utils'; import * as semverCompare from 'node-version-compare'; import { RedisDatabaseHelloResponse } from 'src/modules/database/dto/redis-info.dto'; -import ERROR_MESSAGES from 'src/constants/error-messages'; import { plainToClass } from 'class-transformer'; const REDIS_CLIENTS_CONFIG = apiConfig.get('redis_clients'); @@ -52,6 +51,7 @@ export abstract class RedisClient extends EventEmitter2 { public readonly id: string; protected _redisVersion: string | undefined; + protected _isInfoCommandDisabled: boolean | undefined; protected lastTimeUsed: number; @@ -83,6 +83,10 @@ export abstract class RedisClient extends EventEmitter2 { return Date.now() - this.lastTimeUsed > REDIS_CLIENTS_CONFIG.idleThreshold; } + public get isInfoCommandDisabled() { + return this._isInfoCommandDisabled; + } + /** * Checks if client has established connection */ @@ -168,28 +172,31 @@ export abstract class RedisClient extends EventEmitter2 { /** * Get redis database info - * Uses cache by default + * If INFO fails, it will try to get info from HELLO command, which provides limited data + * If HELLO fails, it will return a static object * @param force * @param infoSection - e.g. server, clients, memory, etc. */ public async getInfo(infoSection?: string) { + let infoData: any; // TODO: we should ideally type this + try { - return convertRedisInfoReplyToObject(await this.call( + infoData = convertRedisInfoReplyToObject(await this.call( infoSection ? ['info', infoSection] : ['info'], { replyEncoding: 'utf8' }, ) as string); + this._isInfoCommandDisabled = false; } catch (error) { - if (error.message.includes(ERROR_MESSAGES.NO_INFO_COMMAND_PERMISSION)) { - try { - // Fallback to getting basic information from `hello` command - return await this.getRedisHelloInfo(); - } catch (_error) { - // Ignore: hello is not available pre redis version 6 - } + this._isInfoCommandDisabled = true; + try { + // Fallback to getting basic information from `hello` command + infoData = await this.getRedisHelloInfo(); + } catch (_error) { + // Ignore: hello is not available pre redis version 6 } } - return UNKNOWN_REDIS_INFO; + return infoData ?? UNKNOWN_REDIS_INFO; } private async getRedisHelloInfo() {