From 0e8e44d79d984b016ef033b804dc4186a371dbd6 Mon Sep 17 00:00:00 2001 From: Guian Gumpac Date: Mon, 15 Jul 2024 09:36:12 -0700 Subject: [PATCH] Addressed PR comments Signed-off-by: Guian Gumpac --- node/src/BaseClient.ts | 20 +++++++++++------- node/src/Commands.ts | 42 ++++++++++++++----------------------- node/src/Transaction.ts | 6 ++++-- node/tests/SharedTests.ts | 28 +++++++++++++++---------- node/tests/TestUtilities.ts | 4 ++-- 5 files changed, 52 insertions(+), 48 deletions(-) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index a2bc14cefa..98a18901c6 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -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 { - return this.createWritePromise(createLPos(key, element, options)); + rank?: number, + count?: number, + maxLength?: number, + ): Promise { + return this.createWritePromise( + createLPos(key, element, rank, count, maxLength), + ); } /** diff --git a/node/src/Commands.ts b/node/src/Commands.ts index 0d8a5f4f7f..8857d8a113 100644 --- a/node/src/Commands.ts +++ b/node/src/Commands.ts @@ -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); diff --git a/node/src/Transaction.ts b/node/src/Transaction.ts index 2643a4fdb8..2a627ccd51 100644 --- a/node/src/Transaction.ts +++ b/node/src/Transaction.ts @@ -1536,10 +1536,12 @@ export class BaseTransaction> { * * @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)); diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index a40607a745..29d0e47d00 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -3416,14 +3416,18 @@ export function runBaseTests(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", @@ -3432,7 +3436,9 @@ export function runBaseTests(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", @@ -3447,9 +3453,7 @@ export function runBaseTests(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", @@ -3458,7 +3462,9 @@ export function runBaseTests(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", @@ -3472,9 +3478,9 @@ export function runBaseTests(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]); diff --git a/node/tests/TestUtilities.ts b/node/tests/TestUtilities.ts index e11bbfc15a..a57da0a612 100644 --- a/node/tests/TestUtilities.ts +++ b/node/tests/TestUtilities.ts @@ -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[] = []; @@ -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; }