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
1 change: 1 addition & 0 deletions redisinsight/api/src/common/decorators/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './redis-string';
export * from './zset-score';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './zset-score.decorator';
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import {
registerDecorator,
ValidationOptions,
} from 'class-validator';
import { ZSetScoreValidator } from 'src/common/validators';

export function isZSetScore(validationOptions?: ValidationOptions) {
return (object: any, propertyName: string) => {
registerDecorator({
name: 'isZSetScore',
target: object.constructor,
propertyName,
options: validationOptions,
validator: ZSetScoreValidator,
});
};
}
1 change: 1 addition & 0 deletions redisinsight/api/src/common/validators/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './redis-string.validator';
export * from './zset-score.validator';
17 changes: 17 additions & 0 deletions redisinsight/api/src/common/validators/zset-score.validator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { isNumber } from 'lodash';
import {
ValidationArguments,
ValidatorConstraint,
ValidatorConstraintInterface,
} from 'class-validator';

@ValidatorConstraint({ name: 'RedisStringValidator', async: true })
export class ZSetScoreValidator implements ValidatorConstraintInterface {
async validate(value: any) {
return value === 'inf' || value === '-inf' || isNumber(value);
}

defaultMessage(args: ValidationArguments) {
return `${args.property || 'field'} must be a string or a number`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

must be a string? "hello world" will be not a valid value here...

}
}
19 changes: 16 additions & 3 deletions redisinsight/api/src/modules/browser/__mocks__/z-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,21 @@ import { SortOrder } from 'src/constants';

export const mockZSetMemberDto: ZSetMemberDto = {
name: Buffer.from('member1'),
score: 0,
score: '-inf',
};
export const mockZSetMemberDto2: ZSetMemberDto = {
name: Buffer.from('member2'),
score: 0,
};
export const mockZSetMemberDto3: ZSetMemberDto = {
name: Buffer.from('member3'),
score: 2,
};

export const mockZSetMemberDto4: ZSetMemberDto = {
name: Buffer.from('member4'),
score: 'inf',
};
export const mockGetMembersDto: GetZSetMembersDto = {
keyName: Buffer.from('zSet'),
offset: 0,
Expand All @@ -28,7 +37,7 @@ export const mockSearchMembersDto: SearchZSetMembersDto = {
};
export const mockAddMembersDto: AddMembersToZSetDto = {
keyName: mockGetMembersDto.keyName,
members: [mockZSetMemberDto, mockZSetMemberDto2],
members: [mockZSetMemberDto, mockZSetMemberDto2, mockZSetMemberDto3, mockZSetMemberDto4],
};
export const mockUpdateMemberDto: UpdateMemberInZSetDto = {
keyName: mockGetMembersDto.keyName,
Expand All @@ -39,10 +48,14 @@ export const mockMembersForZAddCommand = [
mockZSetMemberDto.name,
mockZSetMemberDto2.score,
mockZSetMemberDto2.name,
mockZSetMemberDto3.score,
mockZSetMemberDto3.name,
mockZSetMemberDto4.score,
mockZSetMemberDto4.name,
];
export const mockDeleteMembersDto: DeleteMembersFromZSetDto = {
keyName: mockAddMembersDto.keyName,
members: [mockZSetMemberDto.name, mockZSetMemberDto2.name],
members: [mockZSetMemberDto.name, mockZSetMemberDto2.name, mockZSetMemberDto3.name, mockZSetMemberDto4.name],
};
export const getZSetMembersInAscResponse = {
keyName: mockGetMembersDto.keyName,
Expand Down
11 changes: 5 additions & 6 deletions redisinsight/api/src/modules/browser/dto/z-set.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
IsNotEmptyObject,
IsNumber, IsString,
Min,
ValidateNested
ValidateNested,
} from 'class-validator';
import { Type } from 'class-transformer';
import { SortOrder } from 'src/constants';
Expand All @@ -18,7 +18,7 @@ import {
DeleteMembersFromSetResponse,
} from 'src/modules/browser/dto/set.dto';
import { RedisString } from 'src/common/constants';
import { IsRedisString, RedisStringType } from 'src/common/decorators';
import { IsRedisString, RedisStringType, isZSetScore } from 'src/common/decorators';
import {
KeyDto, KeyResponse, KeyWithExpireDto, ScanDataTypeDto,
} from './keys.dto';
Expand Down Expand Up @@ -77,13 +77,12 @@ export class ZSetMemberDto {

@ApiProperty({
description: 'Member score value.',
type: Number,
type: Number || String,
default: 1,
})
@IsDefined()
@IsNumber({ maxDecimalPlaces: 15 })
@Type(() => Number)
score: number;
@isZSetScore()
score: number | 'inf' | '-inf';
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is better to create a type (like type ZSetScore = ...) and use it everywhere

}

export class AddMembersToZSetDto extends KeyDto {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ describe('ZSetBusinessService', () => {
BrowserToolZSetCommands.ZRange,
expect.anything(),
)
.mockResolvedValue(['member1', '0', 'member2', '2']);
.mockResolvedValue(['member1', '-inf', 'member2', '0', 'member3', '2', 'member4', 'inf']);

const result = await service.getMembers(
mockClientOptions,
Expand All @@ -199,7 +199,7 @@ describe('ZSetBusinessService', () => {
BrowserToolZSetCommands.ZRevRange,
expect.anything(),
)
.mockResolvedValue(['member2', '2', 'member1', '0']);
.mockResolvedValue(['member4', 'inf', 'member3', '2', 'member2', '0', 'member1', '-inf']);

const result = await service.getMembers(mockClientOptions, {
...mockGetMembersDto,
Expand Down Expand Up @@ -485,7 +485,7 @@ describe('ZSetBusinessService', () => {
BrowserToolZSetCommands.ZScan,
expect.anything(),
)
.mockResolvedValue([0, ['member1', '0', 'member2', '2']]);
.mockResolvedValue([0, ['member1', '-inf', 'member2', '0', 'member3', '2', 'member4', 'inf']]);

const result = await service.searchMembers(
mockClientOptions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
Logger,
NotFoundException,
} from '@nestjs/common';
import { isNull } from 'lodash';
import { isNull, isNaN } from 'lodash';
import * as isGlob from 'is-glob';
import config from 'src/utils/config';
import { catchAclError, catchTransactionError, unescapeGlob } from 'src/utils';
Expand Down Expand Up @@ -277,8 +277,10 @@ export class ZSetBusinessService {
BrowserToolZSetCommands.ZScore,
[keyName, member],
);
const formattedScore = isNaN(parseFloat(score)) ? String(score) : parseFloat(score);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we move this to some function to not duplicate logic everywhere?


if (!isNull(score)) {
result.members.push(plainToClass(ZSetMemberDto, { name: member, score }));
result.members.push(plainToClass(ZSetMemberDto, { name: member, score: formattedScore }));
}
} else {
const scanResult = await this.scanZSet(clientOptions, dto);
Expand Down Expand Up @@ -375,7 +377,6 @@ export class ZSetBusinessService {
nextCursor: null,
members: [],
};

while (result.nextCursor !== 0 && result.members.length < count) {
const scanResult = await this.browserTool.execCommand(
clientOptions,
Expand Down Expand Up @@ -406,11 +407,13 @@ export class ZSetBusinessService {
const result: ZSetMemberDto[] = [];
while (reply.length) {
const member = reply.splice(0, 2);
const score = isNaN(parseFloat(member[1])) ? String(member[1]) : parseFloat(member[1]);
result.push(plainToClass(ZSetMemberDto, {
Copy link
Collaborator

Choose a reason for hiding this comment

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

we are using "plainToClass" here. Maybe it is better to create own transformer and isNaN(parseFloat(member[1])) ? String(member[1]) : parseFloat(member[1]); will be done automatically inside?
P.S. See comment above

name: member[0],
score: parseFloat(member[1]),
score,
}));
}

return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ const dataSchema = Joi.object({
member: Joi.object().keys({
name: Joi.string().required(),
// todo: allow(true) - is incorrect but will be transformed to number by BE. Investigate/fix it
score: Joi.number().required().allow(true),
score: Joi.number().required().allow('inf', '-inf').label('.score'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

it is better to create own type here the same way as for redisString so it will look like Joi.zSetScore().required() or something similar

}).messages({
'number.base': '{#lavel} must be a number',
'number.base': '{#lavel} must be a string or a number',
}),
}).strict();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import {
requirements,
generateInvalidDataTestCases,
validateInvalidDataTestCase,
getMainCheckFn, JoiRedisString
getMainCheckFn,
JoiRedisString,
} from '../deps';
const { server, request, constants, rte } = deps;

Expand Down Expand Up @@ -38,7 +39,7 @@ const responseSchema = Joi.object().keys({
total: Joi.number().integer().required(),
members: Joi.array().items(Joi.object().keys({
name: JoiRedisString.required(),
score: Joi.number().required(),
score: Joi.number().required().allow('inf', '-inf'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

the same

})).required(),
}).required();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const responseSchema = Joi.object().keys({
nextCursor: Joi.number().integer().required(),
members: Joi.array().items(Joi.object().keys({
name: JoiRedisString.required(),
score: Joi.number().required(),
score: Joi.number().required().allow('inf', '-inf'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

the same

})).required(),
}).required();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const dataSchema = Joi.object({
// todo: allow(true) - is incorrect but will be transformed to number by BE. Investigate/fix it
score: Joi.number().required().allow(true).label('.score'),
})).messages({
'number.base': '{#lavel} must be a number',
'number.base': '{#lavel} must be a string or a number',
'array.sparse': 'members must be either object or array',
'array.base': 'property {#label} must be either object or array',
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ const dataSchema = Joi.object({
keyName: Joi.string().allow('').required(),
members: Joi.array().items(Joi.object().keys({
name: Joi.string().required().label('.name'),
// todo: allow(true) - is incorrect but will be transformed to number by BE. Investigate/fix it
score: Joi.number().required().allow(true).label('.score'),
score: Joi.number().required().allow('inf', '-inf').label('.score'),
})).messages({
'number.base': '{#lavel} must be a number',
'number.base': '{#lavel} must be a string or a number',
'array.sparse': 'members must be either object or array',
'array.base': 'property {#label} must be either object or array',
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ jest.mock('uiSrc/slices/browser/zset', () => {
setZsetInitialState: jest.fn,
zsetDataSelector: jest.fn().mockReturnValue({
...defaultState,
total: 3,
total: 4,
key: 'z',
keyName: 'z',
members: [
{ name: { type: 'Buffer', data: [49] }, score: 1 },
{ name: { type: 'Buffer', data: [50] }, score: 2 },
{ name: { type: 'Buffer', data: [51] }, score: 3 },
{ name: { type: 'Buffer', data: [52] }, score: 'inf' },
],
}),
updateZsetScoreStateSelector: jest.fn().mockReturnValue(defaultState.updateScore),
Expand Down Expand Up @@ -52,6 +53,16 @@ describe('ZSetDetails', () => {
expect(screen.getByTestId(/zset-edit-button-1/)).toBeInTheDocument()
})

it('should render disabled edit button', () => {
render(<ZSetDetails {...instance(mockedProps)} />)
expect(screen.getByTestId(/zset-edit-button-4/)).toBeDisabled()
})

it('should render enabled edit button', () => {
render(<ZSetDetails {...instance(mockedProps)} />)
expect(screen.getByTestId(/zset-edit-button-3/)).not.toBeDisabled()
})

it('should render editor after click edit button and able to change value', () => {
render(<ZSetDetails {...instance(mockedProps)} />)
fireEvent.click(screen.getAllByTestId(/zset-edit-button/)[0])
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useCallback, useEffect, useRef, useState } from 'react'
import { useDispatch, useSelector } from 'react-redux'
import { toNumber } from 'lodash'
import { toNumber, isNumber } from 'lodash'
import cx from 'classnames'
import { EuiButtonIcon, EuiProgress, EuiText, EuiToolTip } from '@elastic/eui'
import { CellMeasurerCache } from 'react-virtualized'
Expand Down Expand Up @@ -39,8 +39,8 @@ import InlineItemEditor from 'uiSrc/components/inline-item-editor/InlineItemEdit
import { IColumnSearchState, ITableColumn } from 'uiSrc/components/virtual-table/interfaces'
import { StopPropagation } from 'uiSrc/components/virtual-table'
import { getColumnWidth } from 'uiSrc/components/virtual-grid'
import { AddMembersToZSetDto, SearchZSetMembersResponse } from 'apiSrc/modules/browser/dto'
import { stringToBuffer } from 'uiSrc/utils/formatters/bufferFormatters'
import { AddMembersToZSetDto, SearchZSetMembersResponse } from 'apiSrc/modules/browser/dto'
import PopoverDelete from '../popover-delete/PopoverDelete'

import styles from './styles.module.scss'
Expand Down Expand Up @@ -326,20 +326,22 @@ const ZSetDetails = (props: Props) => {
minWidth: 100,
maxWidth: 100,
absoluteWidth: 100,
render: function Actions(_act: any, { name: nameItem }: IZsetMember) {
render: function Actions(_act: any, { name: nameItem, score }: IZsetMember) {
const name = bufferToString(nameItem, viewFormat)
return (
<StopPropagation>
<div className="value-table-actions">
<EuiButtonIcon
iconType="pencil"
aria-label="Edit field"
className="editFieldBtn"
color="primary"
disabled={updateLoading}
onClick={() => handleEditMember(nameItem, true)}
data-testid={`zset-edit-button-${name}`}
/>
<EuiToolTip content={!isNumber(score) ? 'Use CLI or Workbench to edit the score' : null}>
<EuiButtonIcon
iconType="pencil"
aria-label="Edit field"
className="editFieldBtn"
color="primary"
disabled={updateLoading || !isNumber(score)}
onClick={() => handleEditMember(nameItem, true)}
data-testid={`zset-edit-button-${name}`}
/>
</EuiToolTip>
<PopoverDelete
header={createDeleteFieldHeader(nameItem)}
text={createDeleteFieldMessage(key ?? '')}
Expand Down