From 8c95b46545b4349ea5ac8b909debbfed87f172bc Mon Sep 17 00:00:00 2001 From: ort-bot Date: Mon, 20 May 2024 00:20:00 +0000 Subject: [PATCH 1/4] Add SMOVE command for node --- node/src/BaseClient.ts | 25 ++++++++++++++ node/src/Commands.ts | 12 +++++++ node/src/Transaction.ts | 15 +++++++++ node/tests/SharedTests.ts | 66 +++++++++++++++++++++++++++++++++++++ node/tests/TestUtilities.ts | 2 ++ 5 files changed, 120 insertions(+) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index be0ee6e9da..bfef8cc4f7 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -65,6 +65,7 @@ import { createSCard, createSIsMember, createSMembers, + createSMove, createSPop, createSRem, createSet, @@ -1224,6 +1225,30 @@ export class BaseClient { ); } + /** Moves `member` from the set at `source` to the set at `destination`, removing it from the source set. + * Creates a new destination set if needed. The operation is atomic. + * See https://valkey.io/commands/smove for more details. + * + * @param source - The key of the set to remove the element from. + * @param destination - The key of the set to add the element to. + * @param member - The set element to move. + * @returns True on success, or False if the `source` set does not exist or the element is not a member of the source set. + * + * @example + * ```typescript + * const result == await client.smove("set1", "set2", "member1"); + * console.log(result); // Output: True # "member1" was moved from "set1" to "set2". + */ + public smove( + source: string, + destination: string, + member: string, + ): Promise { + return this.createWritePromise( + createSMove(source, destination, member), + ); + } + /** Returns the set cardinality (number of elements) of the set stored at `key`. * See https://redis.io/commands/scard/ for details. * diff --git a/node/src/Commands.ts b/node/src/Commands.ts index 65fec321f8..f1613f26cf 100644 --- a/node/src/Commands.ts +++ b/node/src/Commands.ts @@ -549,6 +549,18 @@ export function createSMembers(key: string): redis_request.Command { return createCommand(RequestType.SMembers, [key]); } +/** + * + * @internal + */ +export function createSMove( + source: string, + destination: string, + member: string, +): redis_request.Command { + return createCommand(RequestType.SMove, [source, destination, member]); +} + /** * @internal */ diff --git a/node/src/Transaction.ts b/node/src/Transaction.ts index a457ba5e41..8775a234f3 100644 --- a/node/src/Transaction.ts +++ b/node/src/Transaction.ts @@ -68,6 +68,7 @@ import { createSCard, createSIsMember, createSMembers, + createSMove, createSPop, createSRem, createSelect, @@ -678,6 +679,20 @@ export class BaseTransaction> { return this.addAndReturn(createSMembers(key), true); } + /** Moves `member` from the set at `source` to the set at `destination`, removing it from the source set. + * Creates a new destination set if needed. The operation is atomic. + * See https://valkey.io/commands/smove for more details. + * + * @param source - The key of the set to remove the element from. + * @param destination - The key of the set to add the element to. + * @param member - The set element to move. + * + * Command Response - True on success, or False if the `source` set does not exist or the element is not a member of the source set. + */ + public smove(source: string, destination: string, member: string): T { + return this.addAndReturn(createSMove(source, destination, member)); + } + /** Returns the set cardinality (number of elements) of the set stored at `key`. * See https://redis.io/commands/scard/ for details. * diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index f706370bda..df6324dc1f 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -1046,6 +1046,72 @@ export function runBaseTests(config: { config.timeout, ); + it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])( + `smove test_%p`, + async (protocol) => { + await runTest(async (client: BaseClient) => { + const key1 = "{key}" + uuidv4(); + const key2 = "{key}" + uuidv4(); + const key3 = "{key}" + uuidv4(); + const string_key = "{key}" + uuidv4(); + const non_existing_key = "{key}" + uuidv4(); + + expect(await client.sadd(key1, ["1", "2", "3"])).toEqual(3); + expect(await client.sadd(key2, ["2", "3"])).toEqual(2); + + // move an element + expect(await client.smove(key1, key2, "1")); + expect(await client.smembers(key1)).toEqual( + new Set(["2", "3"]), + ); + expect(await client.smembers(key2)).toEqual( + new Set(["1", "2", "3"]), + ); + + // moved element already exists in the destination set + expect(await client.smove(key2, key1, "2")); + expect(await client.smembers(key1)).toEqual( + new Set(["2", "3"]), + ); + expect(await client.smembers(key2)).toEqual( + new Set(["1", "3"]), + ); + + // attempt to move from a non-existing key + expect(await client.smove(non_existing_key, key1, "4")).toEqual( + false, + ); + expect(await client.smembers(key1)).toEqual( + new Set(["2", "3"]), + ); + + // move to a new set + expect(await client.smove(key1, key3, "2")); + expect(await client.smembers(key1)).toEqual(new Set(["3"])); + expect(await client.smembers(key3)).toEqual(new Set(["2"])); + + // attempt to move a missing element + expect(await client.smove(key1, key3, "42")).toEqual(false); + expect(await client.smembers(key1)).toEqual(new Set(["3"])); + expect(await client.smembers(key3)).toEqual(new Set(["2"])); + + // move missing element to missing key + expect( + await client.smove(key1, non_existing_key, "42"), + ).toEqual(false); + expect(await client.smembers(key1)).toEqual(new Set(["3"])); + expect(await client.type(non_existing_key)).toEqual("none"); + + // key exists, but it is not a set + expect(await client.set(string_key, "value")).toEqual("OK"); + await expect( + client.smove(string_key, key1, "_"), + ).rejects.toThrow(); + }, protocol); + }, + config.timeout, + ); + it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])( `srem, scard and smembers with non existing key_%p`, async (protocol) => { diff --git a/node/tests/TestUtilities.ts b/node/tests/TestUtilities.ts index 7c6a0ef45f..82e746a4c6 100644 --- a/node/tests/TestUtilities.ts +++ b/node/tests/TestUtilities.ts @@ -300,6 +300,8 @@ export async function transactionTest( args.push("bar"); baseTransaction.spopCount(key7, 2); args.push(new Set()); + baseTransaction.smove(key7, key7, "non_existing_member"); + args.push(false); baseTransaction.scard(key7); args.push(0); baseTransaction.zadd(key8, { From 1891175456822788884bde230c9915cfff65f2d0 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Tue, 28 May 2024 12:22:49 +0000 Subject: [PATCH 2/4] formatting --- node/src/BaseClient.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index bfef8cc4f7..6e8cbed217 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -1238,6 +1238,7 @@ export class BaseClient { * ```typescript * const result == await client.smove("set1", "set2", "member1"); * console.log(result); // Output: True # "member1" was moved from "set1" to "set2". + * ``` */ public smove( source: string, From 64d2273f093a6f27d0674f741f6ea7303b02563b Mon Sep 17 00:00:00 2001 From: adarovadya Date: Tue, 28 May 2024 19:49:44 +0300 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Shoham Elias <116083498+shohamazon@users.noreply.github.com> --- node/src/BaseClient.ts | 6 ++++-- node/src/Transaction.ts | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index 6e8cbed217..a7aa2e7d1d 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -1229,6 +1229,8 @@ export class BaseClient { * Creates a new destination set if needed. The operation is atomic. * See https://valkey.io/commands/smove for more details. * + * Note: When in cluster mode, `source` and `destination` must map to the same hash slot. + * * @param source - The key of the set to remove the element from. * @param destination - The key of the set to add the element to. * @param member - The set element to move. @@ -1236,8 +1238,8 @@ export class BaseClient { * * @example * ```typescript - * const result == await client.smove("set1", "set2", "member1"); - * console.log(result); // Output: True # "member1" was moved from "set1" to "set2". + * const result = await client.smove("set1", "set2", "member1"); + * console.log(result); // Output: True - "member1" was moved from "set1" to "set2". * ``` */ public smove( diff --git a/node/src/Transaction.ts b/node/src/Transaction.ts index 8775a234f3..2511e05d39 100644 --- a/node/src/Transaction.ts +++ b/node/src/Transaction.ts @@ -683,6 +683,8 @@ export class BaseTransaction> { * Creates a new destination set if needed. The operation is atomic. * See https://valkey.io/commands/smove for more details. * + * Note: When in cluster mode, `source` and `destination` must map to the same hash slot. + * * @param source - The key of the set to remove the element from. * @param destination - The key of the set to add the element to. * @param member - The set element to move. From 6c0ace1de4877eed55d5ac6867707ba061ca6bb4 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Wed, 29 May 2024 08:17:13 +0000 Subject: [PATCH 4/4] fix comments --- node/src/BaseClient.ts | 6 +++--- node/src/Transaction.ts | 4 +--- node/tests/RedisClusterClient.test.ts | 1 + 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index a7aa2e7d1d..648c1ca9dc 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -1229,17 +1229,17 @@ export class BaseClient { * Creates a new destination set if needed. The operation is atomic. * See https://valkey.io/commands/smove for more details. * - * Note: When in cluster mode, `source` and `destination` must map to the same hash slot. + * @remarks When in cluster mode, `source` and `destination` must map to the same hash slot. * * @param source - The key of the set to remove the element from. * @param destination - The key of the set to add the element to. * @param member - The set element to move. - * @returns True on success, or False if the `source` set does not exist or the element is not a member of the source set. + * @returns `true` on success, or `false` if the `source` set does not exist or the element is not a member of the source set. * * @example * ```typescript * const result = await client.smove("set1", "set2", "member1"); - * console.log(result); // Output: True - "member1" was moved from "set1" to "set2". + * console.log(result); // Output: true - "member1" was moved from "set1" to "set2". * ``` */ public smove( diff --git a/node/src/Transaction.ts b/node/src/Transaction.ts index 2511e05d39..a4ac9389cb 100644 --- a/node/src/Transaction.ts +++ b/node/src/Transaction.ts @@ -683,13 +683,11 @@ export class BaseTransaction> { * Creates a new destination set if needed. The operation is atomic. * See https://valkey.io/commands/smove for more details. * - * Note: When in cluster mode, `source` and `destination` must map to the same hash slot. - * * @param source - The key of the set to remove the element from. * @param destination - The key of the set to add the element to. * @param member - The set element to move. * - * Command Response - True on success, or False if the `source` set does not exist or the element is not a member of the source set. + * Command Response - `true` on success, or `false` if the `source` set does not exist or the element is not a member of the source set. */ public smove(source: string, destination: string, member: string): T { return this.addAndReturn(createSMove(source, destination, member)); diff --git a/node/tests/RedisClusterClient.test.ts b/node/tests/RedisClusterClient.test.ts index 49eefee96c..6114aec653 100644 --- a/node/tests/RedisClusterClient.test.ts +++ b/node/tests/RedisClusterClient.test.ts @@ -286,6 +286,7 @@ describe("RedisClusterClient", () => { client.blpop(["abc", "zxy", "lkn"], 0.1), client.rename("abc", "zxy"), client.brpop(["abc", "zxy", "lkn"], 0.1), + client.smove("abc", "zxy", "value"), // TODO all rest multi-key commands except ones tested below ];