Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions redisinsight/api/src/constants/error-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,18 @@
CLI_UNTERMINATED_QUOTES: () => 'Invalid argument(s): Unterminated quotes.',
CLI_INVALID_QUOTES_CLOSING: () =>
'Invalid argument(s): Closing quote must be followed by a space or nothing at all.',
CLUSTER_NODE_NOT_FOUND: (node: string) =>

Check warning on line 105 in redisinsight/api/src/constants/error-messages.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🕹️ Function is not covered

Warning! Not covered function
`Node ${node} not exist in OSS Cluster.`,

Check warning on line 106 in redisinsight/api/src/constants/error-messages.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🧾 Statement is not covered

Warning! Not covered statement
REDIS_MODULE_IS_REQUIRED: (module: string) =>
`Required ${module} module is not loaded.`,
UNABLE_TO_LIST_VECTOR_SET_ELEMENTS: (commands: string[]) =>
`Unable to list vector set elements. The following commands failed: ${commands.join(', ')}.`,
APP_SETTINGS_NOT_FOUND: () => 'Could not find application settings.',

Check warning on line 111 in redisinsight/api/src/constants/error-messages.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🧾 Statement is not covered

Warning! Not covered statement

Check warning on line 111 in redisinsight/api/src/constants/error-messages.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🕹️ Function is not covered

Warning! Not covered function
SERVER_INFO_NOT_FOUND: () => 'Could not find server info.',
INCREASE_MINIMUM_LIMIT: (count?: number) =>
count
? `Set MAXSEARCHRESULTS to at least ${numberWithSpaces(count)}.`
: 'Increase MAXSEARCHRESULTS value to search more.',

Check warning on line 116 in redisinsight/api/src/constants/error-messages.ts

View workflow job for this annotation

GitHub Actions / Coverage annotations (🧪 jest-coverage-report-action)

🌿 Branch is not covered

Warning! Not covered branch
INVALID_WINDOW_ID: 'Invalid window id.',
UNDEFINED_WINDOW_ID: 'Undefined window id.',
LIBRARY_NOT_EXIST: 'This library does not exist.',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
BadRequestException,
ForbiddenException,
InternalServerErrorException,
NotFoundException,
} from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
Expand Down Expand Up @@ -39,6 +40,7 @@ import { BrowserToolKeysCommands } from 'src/modules/browser/constants/browser-t
import { VectorSetService } from 'src/modules/browser/vector-set/vector-set.service';
import { DatabaseClientFactory } from 'src/modules/database/providers/database.client.factory';
import { RedisFeature } from 'src/modules/redis/client';
import ERROR_MESSAGES from 'src/constants/error-messages';

describe('VectorSetService', () => {
const client = mockStandaloneRedisClient;
Expand Down Expand Up @@ -326,8 +328,6 @@ describe('VectorSetService', () => {
]);

beforeEach(() => {
client.isFeatureSupported = jest.fn().mockResolvedValue(true);

when(client.sendCommand)
.calledWith([BrowserToolKeysCommands.Exists, mockDto.keyName])
.mockResolvedValue(true);
Expand Down Expand Up @@ -357,9 +357,6 @@ describe('VectorSetService', () => {
mockDto,
);

expect(client.isFeatureSupported).toHaveBeenCalledWith(
RedisFeature.VRangeCommand,
);
expect(client.sendCommand).toHaveBeenCalledWith([
BrowserToolVectorSetCommands.VRange,
mockDto.keyName,
Expand Down Expand Up @@ -465,8 +462,16 @@ describe('VectorSetService', () => {
).rejects.toThrow(ForbiddenException);
});

it('should fallback to VRANDMEMBER when VRANGE is not supported', async () => {
client.isFeatureSupported = jest.fn().mockResolvedValue(false);
it('should fallback to VRANDMEMBER when VRANGE is an unknown command', async () => {
when(client.sendCommand)
.calledWith([
BrowserToolVectorSetCommands.VRange,
mockDto.keyName,
mockDto.start,
mockDto.end,
mockDto.count,
])
.mockRejectedValue(new Error("ERR unknown command 'VRANGE'"));

when(client.sendCommand)
.calledWith([
Expand All @@ -481,9 +486,6 @@ describe('VectorSetService', () => {
mockDto,
);

expect(client.isFeatureSupported).toHaveBeenCalledWith(
RedisFeature.VRangeCommand,
);
expect(client.sendCommand).toHaveBeenCalledWith([
BrowserToolVectorSetCommands.VRandMember,
mockDto.keyName,
Expand All @@ -494,6 +496,62 @@ describe('VectorSetService', () => {
expect(result.isPaginationSupported).toBe(false);
});

it('should not fallback and should surface the error when VRANGE fails with a non-command error', async () => {
when(client.sendCommand)
.calledWith([
BrowserToolVectorSetCommands.VRange,
mockDto.keyName,
mockDto.start,
mockDto.end,
mockDto.count,
])
.mockRejectedValue({
...mockRedisNoPermError,
command: 'VRANGE',
});

await expect(
service.getElements(mockBrowserClientMetadata, mockDto),
).rejects.toThrow(ForbiddenException);

expect(client.sendCommand).not.toHaveBeenCalledWith([
BrowserToolVectorSetCommands.VRandMember,
mockDto.keyName,
mockDto.count,
]);
});

it('should throw a server error when both VRANGE and VRANDMEMBER are unknown commands', async () => {
when(client.sendCommand)
.calledWith([
BrowserToolVectorSetCommands.VRange,
mockDto.keyName,
mockDto.start,
mockDto.end,
mockDto.count,
])
.mockRejectedValue(new Error("ERR unknown command 'VRANGE'"));

when(client.sendCommand)
.calledWith([
BrowserToolVectorSetCommands.VRandMember,
mockDto.keyName,
mockDto.count,
])
.mockRejectedValue(new Error("ERR unknown command 'VRANDMEMBER'"));

await expect(
service.getElements(mockBrowserClientMetadata, mockDto),
).rejects.toThrow(
new InternalServerErrorException(
ERROR_MESSAGES.UNABLE_TO_LIST_VECTOR_SET_ELEMENTS([
BrowserToolVectorSetCommands.VRange,
BrowserToolVectorSetCommands.VRandMember,
]),
),
);
});

it('should keep attributes undefined when VGETATTR fails for an element', async () => {
client.sendPipeline = jest
.fn()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { BadRequestException, Injectable, Logger } from '@nestjs/common';
import {
BadRequestException,
Injectable,
InternalServerErrorException,
Logger,
} from '@nestjs/common';
import { catchAclError, catchMultiTransactionError } from 'src/utils';
import { RedisErrorCodes } from 'src/constants';
import ERROR_MESSAGES from 'src/constants/error-messages';
import { ReplyError } from 'src/models';
import {
BrowserToolKeysCommands,
Expand Down Expand Up @@ -135,7 +141,7 @@ export class VectorSetService {
'Getting elements of the VectorSet data type stored at key.',
clientMetadata,
);
const { keyName, start, end, count } = dto;
const { keyName } = dto;
const client: RedisClient =
await this.databaseClientFactory.getOrCreateClient(clientMetadata);

Expand All @@ -146,32 +152,8 @@ export class VectorSetService {
keyName,
])) as number;

const isVRangeSupported = await client.isFeatureSupported(
RedisFeature.VRangeCommand,
);

let elementNames: string[];
let nextCursor: string | undefined;

if (isVRangeSupported) {
elementNames = (await client.sendCommand([
BrowserToolVectorSetCommands.VRange,
keyName,
start || '-',
end || '+',
count,
])) as string[];

if (elementNames.length === count && elementNames.length > 0) {
nextCursor = `(${elementNames[elementNames.length - 1]}`;
}
} else {
elementNames = (await client.sendCommand([
BrowserToolVectorSetCommands.VRandMember,
keyName,
count,
])) as string[];
}
const { elementNames, nextCursor, isPaginationSupported } =
await this.listElementNames(client, dto);

const elements: { name: string | Buffer; attributes?: string }[] =
elementNames.map((name) => ({ name }));
Expand All @@ -188,7 +170,7 @@ export class VectorSetService {
keyName,
total,
nextCursor,
isPaginationSupported: isVRangeSupported,
isPaginationSupported,
elements,
});
} catch (error) {
Expand Down Expand Up @@ -455,6 +437,91 @@ export class VectorSetService {
}
}

/**
* Resolve the element names for `getElements`, preferring the ordered,
* paginated VRANGE and falling back to the unordered VRANDMEMBER when VRANGE
* is unavailable. When neither command can list the elements, throws so the
* caller surfaces a server error.
*/
private async listElementNames(
client: RedisClient,
dto: GetVectorSetElementsDto,
): Promise<{
elementNames: string[];
nextCursor?: string;
isPaginationSupported: boolean;
}> {
const { keyName, start, end, count } = dto;

const vRangeCommand: RedisClientCommand = [
BrowserToolVectorSetCommands.VRange,
keyName,
start || '-',
end || '+',
count,
];
const vRangeResult = await this.trySendCommand(client, vRangeCommand);
if (vRangeResult !== null) {
const nextCursor =
vRangeResult.length === count && vRangeResult.length > 0
? `(${vRangeResult[vRangeResult.length - 1]}`
: undefined;

return {
elementNames: vRangeResult,
nextCursor,
isPaginationSupported: true,
};
}

const vRandMemberCommand: RedisClientCommand = [
BrowserToolVectorSetCommands.VRandMember,
keyName,
count,
];
const vRandMemberResult = await this.trySendCommand(
client,
vRandMemberCommand,
);
if (vRandMemberResult !== null) {
return { elementNames: vRandMemberResult, isPaginationSupported: false };
}

throw new InternalServerErrorException(
ERROR_MESSAGES.UNABLE_TO_LIST_VECTOR_SET_ELEMENTS([
`${BrowserToolVectorSetCommands.VRange}`,
`${BrowserToolVectorSetCommands.VRandMember}`,
]),
);
}

/**
* Send a command and return its reply, or `null` when the command itself is
* unavailable (unknown/unsupported on the connected Redis version or proxy)
* so callers can fall back. `null` is distinct from an empty reply, which is a
* valid result. Any other error (permission, bad arguments, transient) is
* rethrown so it surfaces to the caller instead of triggering a silent
* fallback.
*/
private async trySendCommand(
client: RedisClient,
command: RedisClientCommand,
): Promise<string[] | null> {
try {
return (await client.sendCommand(command)) as string[];
} catch (error) {
if (this.isUnsupportedCommandError(error)) {
return null;
}
throw error;
}
}

private isUnsupportedCommandError(error: unknown): boolean {
const message = (error as ReplyError)?.message ?? '';
return message.includes(RedisErrorCodes.UnknownCommand);
}

/**
* Translate a thrown Redis error into the appropriate HTTP exception:
* `WRONGTYPE` becomes a 400 (the client targeted a non-vector-set key), and
Expand Down
4 changes: 0 additions & 4 deletions redisinsight/api/src/modules/redis/client/redis.client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export type RedisClientCommandReply =
export enum RedisFeature {
HashFieldsExpiration = 'HashFieldsExpiration',
UnlinkCommand = 'UnlinkCommand',
VRangeCommand = 'VRangeCommand',
VsimWithAttribs = 'VsimWithAttribs',
ArrayCommands = 'ArrayCommands',
}
Expand Down Expand Up @@ -185,9 +184,6 @@ export abstract class RedisClient extends EventEmitter2 {
case RedisFeature.UnlinkCommand:
// UNLINK command was introduced in Redis 4.0.0
return this.isRedisVersionAtLeast('4.0.0');
case RedisFeature.VRangeCommand:
// VRANGE command was introduced in Redis 8.4
return this.isRedisVersionAtLeast('8.4');
case RedisFeature.VsimWithAttribs:
// VSIM WITHATTRIBS option is broken on 8.0.0–8.0.2 and was fixed in 8.0.3
return this.isRedisVersionAtLeast('8.0.3');
Expand Down
Loading