Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions redisinsight/api/src/common/constants/general.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
export enum TransformGroup {
Secure = 'security',
}

export const UNKNOWN_REDIS_INFO = {
server: {
redis_version: 'unknown',
redis_mode: 'standalone',
},
};
1 change: 1 addition & 0 deletions redisinsight/api/src/constants/error-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's avoid getting various info sections avross the app.

Copy link
Collaborator Author

@KrumTy KrumTy Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why though? Preserving the behavior is one thing, but I figured querying just specific sections should be faster. E.g. if we have an interval, querying the data and only carrying about one section it would make sense to only query that section. I might be wrong, let me know.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this slightly impact performance. Just don't like idea of merging objects in getInfo funciton.
If we want to stay with this logic we should enhance getInfo command to reply only particular section as it works now (most probably without storing it)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored; I've removed the stored info object


const strategy = this.getClusterInfoStrategy(get(info, 'server.redis_version'));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -63,10 +63,7 @@ export abstract class AbstractInfoStrategy implements IClusterInfo {
* @private
*/
private async getClusterNodeInfo(nodeClient: RedisClient, node): Promise<ClusterNodeDetails> {
const info = convertRedisInfoReplyToObject(await nodeClient.sendCommand(
['info'],
{ replyEncoding: 'utf8' },
) as string);
const info = await nodeClient.getInfo();

return {
...node,
Expand Down
45 changes: 45 additions & 0 deletions redisinsight/api/src/modules/database/dto/redis-info.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
}
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -298,9 +299,8 @@ describe('DatabaseInfoProvider', () => {

describe('determineDatabaseServer', () => {
it('get modules by using MODULE LIST command', async () => {
when(standaloneClient.call)
.calledWith(['info', 'server'], expect.anything())
.mockResolvedValue(mockRedisServerInfoResponse);
when(standaloneClient.getInfo)
.mockResolvedValue(convertRedisInfoReplyToObject(mockRedisServerInfoResponse));

const result = await service.determineDatabaseServer(standaloneClient);

Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -379,11 +375,40 @@ describe('DatabaseInfoProvider', () => {
nodes: [mockRedisGeneralInfo, mockRedisGeneralInfo],
});
});
it('should throw an error if no permission to run \'info\' command', async () => {
when(standaloneClient.sendCommand)
.calledWith(['info'], { replyEncoding: 'utf8' })
it('should get info from hello command when info command is not available', async () => {
when(standaloneClient.getInfo)
.mockResolvedValue({
replication: {
role: mockRedisGeneralInfo.role,
},
server: {
redis_mode: mockRedisServerInfoDto.redis_mode,
redis_version: mockRedisGeneralInfo.version,
server_name: 'redis',
},
});

const result = await service.getRedisGeneralInfo(standaloneClient);

expect(result).toEqual({
...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 () => {
when(standaloneClient.getInfo)
.mockRejectedValue({
message: 'NOPERM this user has no permissions to run the \'info\' command',
message: 'NOPERM this user has no permissions to run the \'hello\' command',
});

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {
calculateRedisHitRatio,
catchAclError,
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';
Expand Down Expand Up @@ -76,10 +75,7 @@ export class DatabaseInfoProvider {
*/
public async determineDatabaseServer(client: RedisClient): Promise<string> {
try {
const reply = convertRedisInfoReplyToObject(await client.call(
['info', 'server'],
{ replyEncoding: 'utf8' },
) as string);
const reply = await client.getInfo();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like here

return reply['server']?.redis_version;
} catch (e) {
// continue regardless of error
Expand Down Expand Up @@ -135,10 +131,7 @@ export class DatabaseInfoProvider {
client: RedisClient,
): Promise<RedisDatabaseInfoResponse> {
try {
const info = convertRedisInfoReplyToObject(await client.sendCommand(
['info'],
{ replyEncoding: 'utf8' },
) as string);
const info = await client.getInfo();
const serverInfo = info['server'];
const memoryInfo = info['memory'];
const keyspaceInfo = info['keyspace'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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({
Expand All @@ -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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
};
Expand Down
Loading
Loading