Skip to content

Commit

Permalink
Addressed PR comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Guian Gumpac <guian.gumpac@improving.com>
  • Loading branch information
@GumpacG
GumpacG committed Jul 15, 2024
1 parent ad0ca24 commit 0e8e44d
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 48 deletions.
20 changes: 13 additions & 7 deletions node/src/BaseClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2646,23 +2646,29 @@ export class BaseClient {
*
* @param key - The name of the list.
* @param element - The value to search for within the list.
* @param options - The LPOS options.
* @param rank - The rank of the match to return.
* @param count - The number of matches wanted.
* @param maxLength - The maximum number of comparisons to make between the element and the items in the list.
* @returns The index of `element`, or `null` if `element` is not in the list. If the `count` option
* is specified, then the function returns an `array` of indices of matching elements within a list.
* is specified, then the function returns an `array` of indices of matching elements within the list.
*
* @example
* ```typescript
* await client.rpush("myList", ["a", "b", "c", "d", "e", "e"]);
* console.log(await client.lpos("myList", "e", { rank: 2 })); // Output: 5 - the second occurence of "e" is in the fifth element.
* console.log(await client.lpos("myList", "e", { count: 3 })); // Output: [ 4, 5, 6 ] - the elements of the occurences of "e".
* console.log(await client.lpos("myList", "e", { rank: 2 })); // Output: 5 - the second occurrence of "e" is in the fifth element.
* console.log(await client.lpos("myList", "e", { count: 3 })); // Output: [ 4, 5, 6 ] - the elements of the occurrences of "e".
* ```
*/
public lpos(
key: string,
element: string,
options?: LPosOptions,
): Promise<number | number[]> {
return this.createWritePromise(createLPos(key, element, options));
rank?: number,
count?: number,
maxLength?: number,
): Promise<number | number[] | null> {
return this.createWritePromise(
createLPos(key, element, rank, count, maxLength),
);
}

/**
Expand Down
42 changes: 16 additions & 26 deletions node/src/Commands.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1526,41 +1526,31 @@ export function createObjectRefcount(key: string): command_request.Command {
return createCommand(RequestType.ObjectRefCount, [key]);
}

export type LPosOptions = {
rank?: number;
count?: number;
maxLength?: number;
};

function addLPosOptions(options: LPosOptions, args: string[]) {
if (options.rank !== undefined) {
args.push("RANK");
args.push(options.rank.toString());
}

if (options.count !== undefined) {
args.push("COUNT");
args.push(options.count.toString());
}

if (options.maxLength !== undefined) {
args.push("MAXLEN");
args.push(options.maxLength.toString());
}
}

/**
* @internal
*/
export function createLPos(
key: string,
element: string,
options?: LPosOptions,
rank?: number,
count?: number,
maxLength?: number,
): command_request.Command {
const args: string[] = [key, element];

if (options) {
addLPosOptions(options, args);
if (rank !== undefined) {
args.push("RANK");
args.push(rank.toString());
}

if (count !== undefined) {
args.push("COUNT");
args.push(count.toString());
}

if (maxLength !== undefined) {
args.push("MAXLEN");
args.push(maxLength.toString());
}

return createCommand(RequestType.LPos, args);
Expand Down
6 changes: 4 additions & 2 deletions node/src/Transaction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1536,10 +1536,12 @@ export class BaseTransaction<T extends BaseTransaction<T>> {
*
* @param key - The name of the list.
* @param element - The value to search for within the list.
* @param options - The LPOS options.
* @param rank - The rank of the match to return.
* @param count - The number of matches wanted.
* @param maxLength - The maximum number of comparisons to make between the element and the items in the list.
*
* Command Response - The index of `element`, or `null` if `element` is not in the list. If the `count`
* option is specified, then the function returns an `array` of indices of matching elements within a list.
* option is specified, then the function returns an `array` of indices of matching elements within the list.
*/
public lpos(key: string, element: string, options?: LPosOptions): T {
return this.addAndReturn(createLPos(key, element, options));
Expand Down
28 changes: 17 additions & 11 deletions node/tests/SharedTests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3416,14 +3416,18 @@ export function runBaseTests<Context>(config: {
expect(await client.lpos(key, "b", { rank: -2 })).toEqual(2);

// unlimited comparisons
expect(await client.lpos(key, "a", { rank: 1, maxLength:0 })).toEqual(0);
expect(
await client.lpos(key, "a", { rank: 1, maxLength: 0 }),
).toEqual(0);

// limited comparisons
expect(await client.lpos(key, "c", { rank: 1, maxLength:2 })).toBeNull();
expect(
await client.lpos(key, "c", { rank: 1, maxLength: 2 }),
).toBeNull();

// invalid rank value
try {
expect(await client.lpos(key, "a", {rank: 0})).toThrow();
expect(await client.lpos(key, "a", { rank: 0 })).toThrow();
} catch (e) {
expect((e as Error).message).toMatch(
"An error was signalled by the server - ResponseError: RANK can't be zero: use 1 to start from the first match, 2 from the second ... or use negative to start from the end of the list",
Expand All @@ -3432,7 +3436,9 @@ export function runBaseTests<Context>(config: {

// invalid maxlen value
try {
expect(await client.lpos(key, "a", { maxLength: -1 })).toThrow();
expect(
await client.lpos(key, "a", { maxLength: -1 }),
).toThrow();
} catch (e) {
expect((e as Error).message).toMatch(
"An error was signalled by the server - ResponseError: MAXLEN can't be negative",
Expand All @@ -3447,9 +3453,7 @@ export function runBaseTests<Context>(config: {
expect(await client.sadd(wrongDataType, ["a", "b"])).toEqual(2);

try {
expect(
await client.lpos(wrongDataType, "a"),
).toThrow();
expect(await client.lpos(wrongDataType, "a")).toThrow();
} catch (e) {
expect((e as Error).message).toMatch(
"WRONGTYPE: Operation against a key holding the wrong kind of value",
Expand All @@ -3458,7 +3462,9 @@ export function runBaseTests<Context>(config: {

// invalid count value
try {
expect(await client.lpos(key, "a", {count: -1})).toThrow();
expect(
await client.lpos(key, "a", { count: -1 }),
).toThrow();
} catch (e) {
expect((e as Error).message).toMatch(
"An error was signalled by the server - ResponseError: COUNT can't be negative",
Expand All @@ -3472,9 +3478,9 @@ export function runBaseTests<Context>(config: {
expect(await client.lpos(key, "a", { count: 0 })).toEqual([
0, 1, 4,
]);
expect(await client.lpos(key, "a", { rank: 1, count: 0 })).toEqual([
0, 1, 4,
]);
expect(
await client.lpos(key, "a", { rank: 1, count: 0 }),
).toEqual([0, 1, 4]);
expect(
await client.lpos(key, "a", { rank: 2, count: 0 }),
).toEqual([1, 4]);
Expand Down
4 changes: 2 additions & 2 deletions node/tests/TestUtilities.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ export async function transactionTest(
const key12 = "{key}" + uuidv4();
const key13 = "{key}" + uuidv4();
const key14 = "{key}" + uuidv4(); // sorted set
const key15 = "{key}" + uuidv4();
const key15 = "{key}" + uuidv4(); // list
const field = uuidv4();
const value = uuidv4();
const args: ReturnType[] = [];
Expand Down Expand Up @@ -515,6 +515,6 @@ export async function transactionTest(
baseTransaction.lpos(key15, field + "1", { rank: 2 });
args.push(1);
baseTransaction.lpos(key15, field + "1", { rank: 2, count: 0 });
args.push([0, 1]);
args.push([1]);
return args;
}

0 comments on commit 0e8e44d

Please sign in to comment.