Skip to content

Commit d3cdb3f

Browse files
committed
address comments
1 parent fd4df8d commit d3cdb3f

4 files changed

Lines changed: 19 additions & 15 deletions

File tree

redisinsight/api/src/modules/browser/array/array.service.spec.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -400,15 +400,11 @@ describe('ArrayService', () => {
400400
).rejects.toThrow(NotFoundException);
401401
});
402402

403-
it('should reject when range exceeds the 1M cap', async () => {
404-
await expect(
405-
service.scan(mockBrowserClientMetadata, {
406-
...mockGetArrayScanDto,
407-
start: '0',
408-
end: String(ARRAY_RANGE_MAX_ELEMENTS),
409-
}),
410-
).rejects.toThrow(BadRequestException);
411-
});
403+
// No span cap on scan: ARSCAN skips empty slots server-side and the
404+
// sparse-array use case routinely spans far more indexes than it
405+
// returns. The DTO caps `limit` at ARRAY_RANGE_MAX_ELEMENTS to keep
406+
// result-set size bounded — exercised via DTO validation tests, not
407+
// here (the service trusts the DTO).
412408

413409
it('should forward reversed ranges (start > end) to Redis as-is', async () => {
414410
when(mockStandaloneRedisClient.sendCommand)

redisinsight/api/src/modules/browser/array/array.service.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,10 @@ export class ArrayService {
169169
this.logger.debug('Scanning array range.', clientMetadata);
170170
const { keyName, start, end, limit } = dto;
171171

172-
this.assertValidRange(start, end);
173-
172+
// No |end - start| span cap here (unlike ARGETRANGE): ARSCAN skips
173+
// empty slots server-side and the sparse-array use case routinely
174+
// spans far more indexes than it returns. LIMIT (capped on the DTO)
175+
// is the natural backpressure on result-set size.
174176
const client =
175177
await this.databaseClientFactory.getOrCreateClient(clientMetadata);
176178
await checkIfKeyNotExists(keyName, client);
Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
// Hard cap on the |end - start| + 1 span for ARGETRANGE and ARSCAN, mirrored
2-
// from the server-side ARGETRANGE limit so callers get a clear 400.
1+
// Hard cap on result-set size shared by ARGETRANGE (applied as |end - start|
2+
// + 1 span, since the reply is dense), ARMGET (applied as @ArrayMaxSize on
3+
// the indexes list), and ARSCAN (applied as @Max on the optional LIMIT).
4+
// Mirrored from the server-side ARGETRANGE limit so callers get a clear 400.
35
// https://redis.io/docs/latest/develop/data-types/arrays/#limits
46
export const ARRAY_RANGE_MAX_ELEMENTS = 1_000_000;

redisinsight/api/src/modules/browser/array/dto/get.array-scan.dto.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { ApiPropertyOptional, ApiProperty } from '@nestjs/swagger';
22
import { KeyDto } from 'src/modules/browser/keys/dto';
33
import { IsArrayIndex } from 'src/common/decorators';
4-
import { IsInt, IsOptional, Min } from 'class-validator';
4+
import { IsInt, IsOptional, Max, Min } from 'class-validator';
55
import { Type } from 'class-transformer';
6+
import { ARRAY_RANGE_MAX_ELEMENTS } from 'src/modules/browser/array/constants';
67

78
export class GetArrayScanDto extends KeyDto {
89
@ApiProperty({
@@ -26,14 +27,17 @@ export class GetArrayScanDto extends KeyDto {
2627

2728
@ApiPropertyOptional({
2829
description:
29-
'Maximum number of populated elements to return. ' +
30+
'Maximum number of populated elements to return (1..' +
31+
`${ARRAY_RANGE_MAX_ELEMENTS.toLocaleString('en-US')}). ` +
3032
'Maps to the ARSCAN LIMIT option.',
3133
type: Number,
3234
minimum: 1,
35+
maximum: ARRAY_RANGE_MAX_ELEMENTS,
3336
})
3437
@IsOptional()
3538
@IsInt()
3639
@Min(1)
40+
@Max(ARRAY_RANGE_MAX_ELEMENTS)
3741
@Type(() => Number)
3842
limit?: number;
3943
}

0 commit comments

Comments
 (0)