From 0ed4c6b1eee933c6857eb6cc592d5b00c5fdeec4 Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Thu, 25 Jul 2024 11:12:44 -0700 Subject: [PATCH 01/14] current work Signed-off-by: Chloe Yip --- node/src/BaseClient.ts | 41 +++++++++++++++++++++++++++++++++++++++++ node/src/Commands.ts | 19 +++++++++++++++++++ node/src/Transaction.ts | 21 +++++++++++++++++++++ 3 files changed, 81 insertions(+) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index ef25d8be0e..e9f2fa5647 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -1589,6 +1589,47 @@ export class BaseClient { ); } + /** + Blocks the connection until it pops atomically and removes the left/right-most element to the + list stored at `source` depending on `where_from`, and pushes the element at the first/last element + of the list stored at `destination` depending on `where_to`. + `BLMOVE` is the blocking variant of `LMOVE`. + + Note: + 1. When in cluster mode, both `source` and `destination` must map to the same hash slot. + 2. `BLMOVE` is a client blocking command, see https://github.com/aws/glide-for-redis/wiki/General-Concepts#blocking-commands for more details and best practices. + See https://valkey.io/commands/blmove/ for details. + Args: + source (str): The key to the source list. + destination (str): The key to the destination list. + where_from (ListDirection): The direction to remove the element from (`ListDirection.LEFT` or `ListDirection.RIGHT`). + where_to (ListDirection): The direction to add the element to (`ListDirection.LEFT` or `ListDirection.RIGHT`). + timeout (float): The number of seconds to wait for a blocking operation to complete. A value of `0` will block indefinitely. + Returns: + Optional[str]: The popped element, or None if `source` does not exist or if the operation timed-out. + Examples: + >>> await client.lpush("testKey1", ["two", "one"]) + >>> await client.lpush("testKey2", ["four", "three"]) + >>> await client.blmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT, 0.1) + "one" + >>> await client.lrange("testKey1", 0, -1) + ["two"] + >>> updated_array2 = await client.lrange("testKey2", 0, -1) + ["one", "three", "four"] + Since: Redis version 6.2.0. + */ + public async blmove( + source: string, + destination: string, + whereFrom: ListDirection, + whereTo: ListDirection, + timeout: number, + ): Promise { + return this.createWritePromise( + createBLMove(source, destination, whereFrom, whereTo, timeout), + ); + } + /** * Sets the list element at `index` to `element`. * The index is zero-based, so `0` means the first element, `1` the second element and so on. diff --git a/node/src/Commands.ts b/node/src/Commands.ts index 4c1c6254c3..bf5a00bdf9 100644 --- a/node/src/Commands.ts +++ b/node/src/Commands.ts @@ -615,6 +615,25 @@ export function createLMove( ]); } +/** + * @internal + */ +export function createBLMove( + source: string, + destination: string, + whereFrom: ListDirection, + whereTo: ListDirection, + timeout: number +): command_request.Command { + return createCommand(RequestType.LMove, [ + source, + destination, + whereFrom, + whereTo, + number + ]); +} + /** * @internal */ diff --git a/node/src/Transaction.ts b/node/src/Transaction.ts index b945655cbb..d60c8ba86a 100644 --- a/node/src/Transaction.ts +++ b/node/src/Transaction.ts @@ -82,6 +82,7 @@ import { createLInsert, createLLen, createLMove, + createBLMove createLPop, createLPos, createLPush, @@ -793,6 +794,26 @@ export class BaseTransaction> { ); } + /** + * + * Blocks the connection until it pops atomically and removes the left/right-most element to the + * list stored at `source` depending on `whereFrom`, and pushes the element at the first/last element + * of the list stored at `destination` depending on `whereTo`. + * `BLMOVE` is the blocking variant of `LMOVE`. + */ + public blmove( + source: string, + destination: string, + whereFrom: ListDirection, + whereTo: ListDirection, + timeout: number, + + ): T { + return this.addAndReturn( + createLMove(source, destination, whereFrom, whereTo, timeout), + ); + } + /** * Sets the list element at `index` to `element`. * The index is zero-based, so `0` means the first element, `1` the second element and so on. From db6c1d9888e8a459fbf94fc9dc2080de3256f904 Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Fri, 26 Jul 2024 11:55:23 -0700 Subject: [PATCH 02/14] fox timeout error Signed-off-by: Chloe Yip --- node/src/BaseClient.ts | 62 +++++++++-------- node/src/Commands.ts | 6 +- node/src/Transaction.ts | 21 +++++- node/tests/SharedTests.ts | 138 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 194 insertions(+), 33 deletions(-) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index e9f2fa5647..d8d85a1c9a 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -72,6 +72,7 @@ import { createLInsert, createLLen, createLMove, + createBLMove, createLPop, createLPos, createLPush, @@ -1590,33 +1591,40 @@ export class BaseClient { } /** - Blocks the connection until it pops atomically and removes the left/right-most element to the - list stored at `source` depending on `where_from`, and pushes the element at the first/last element - of the list stored at `destination` depending on `where_to`. - `BLMOVE` is the blocking variant of `LMOVE`. - - Note: - 1. When in cluster mode, both `source` and `destination` must map to the same hash slot. - 2. `BLMOVE` is a client blocking command, see https://github.com/aws/glide-for-redis/wiki/General-Concepts#blocking-commands for more details and best practices. - See https://valkey.io/commands/blmove/ for details. - Args: - source (str): The key to the source list. - destination (str): The key to the destination list. - where_from (ListDirection): The direction to remove the element from (`ListDirection.LEFT` or `ListDirection.RIGHT`). - where_to (ListDirection): The direction to add the element to (`ListDirection.LEFT` or `ListDirection.RIGHT`). - timeout (float): The number of seconds to wait for a blocking operation to complete. A value of `0` will block indefinitely. - Returns: - Optional[str]: The popped element, or None if `source` does not exist or if the operation timed-out. - Examples: - >>> await client.lpush("testKey1", ["two", "one"]) - >>> await client.lpush("testKey2", ["four", "three"]) - >>> await client.blmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT, 0.1) - "one" - >>> await client.lrange("testKey1", 0, -1) - ["two"] - >>> updated_array2 = await client.lrange("testKey2", 0, -1) - ["one", "three", "four"] - Since: Redis version 6.2.0. + * Blocks the connection until it pops atomically and removes the left/right-most element to the + * list stored at `source` depending on `whereFrom`, and pushes the element at the first/last element + * of the list stored at `destination` depending on `whereTo`. + * `BLMOVE` is the blocking variant of `LMOVE`. + * + * Note: + * 1. When in cluster mode, both `source` and `destination` must map to the same hash slot. + * 2. `BLMOVE` is a client blocking command, see https://github.com/aws/glide-for-redis/wiki/General-Concepts#blocking-commands for more details and best practices. + * + * See https://valkey.io/commands/blmove/ for details. + * + * @param source - The key to the source list. + * @param destination - The key to the destination list. + * @param whereFrom - The {@link ListDirection} to remove the element from. + * @param whereTo - The {@link ListDirection} to add the element to. + * @param timeout - The number of seconds to wait for a blocking operation to complete. A value of `0` will block indefinitely. + * @returns The popped element, or `null` if `source` does not exist or if the operation timed-out. + * + * + * Since: Valkey version 6.2.0. + * + * @example + * ```typescript + * await client.lpush("testKey1", ["two", "one"]); + * await client.lpush("testKey2", ["four", "three"]); + * const result = await client.blmove("testKey1", "testKey2", ListDirection.LEFT, ListDirection.LEFT, 0.1); + * console.log(result); // Output: "one" + * + * const result2 = await client.lrange("testKey1", 0, -1); + * console.log(result2); // Output: "two" + * + * const updated_array2 = await client.lrange("testKey2", 0, -1); + * console.log(updated_array2); // Output: "one", "three", "four"] + * ``` */ public async blmove( source: string, diff --git a/node/src/Commands.ts b/node/src/Commands.ts index bf5a00bdf9..1c42e043fc 100644 --- a/node/src/Commands.ts +++ b/node/src/Commands.ts @@ -623,14 +623,14 @@ export function createBLMove( destination: string, whereFrom: ListDirection, whereTo: ListDirection, - timeout: number + timeout: number, ): command_request.Command { - return createCommand(RequestType.LMove, [ + return createCommand(RequestType.BLMove, [ source, destination, whereFrom, whereTo, - number + timeout.toString(), ]); } diff --git a/node/src/Transaction.ts b/node/src/Transaction.ts index d60c8ba86a..62a6a55f56 100644 --- a/node/src/Transaction.ts +++ b/node/src/Transaction.ts @@ -82,7 +82,7 @@ import { createLInsert, createLLen, createLMove, - createBLMove + createBLMove, createLPop, createLPos, createLPush, @@ -800,6 +800,22 @@ export class BaseTransaction> { * list stored at `source` depending on `whereFrom`, and pushes the element at the first/last element * of the list stored at `destination` depending on `whereTo`. * `BLMOVE` is the blocking variant of `LMOVE`. + * + * Note: + * 1. When in cluster mode, both `source` and `destination` must map to the same hash slot. + * 2. `BLMOVE` is a client blocking command, see https://github.com/aws/glide-for-redis/wiki/General-Concepts#blocking-commands for more details and best practices. + * + * See https://valkey.io/commands/blmove/ for details. + * + * @param source - The key to the source list. + * @param destination - The key to the destination list. + * @param whereFrom - The {@link ListDirection} to remove the element from. + * @param whereTo - The {@link ListDirection} to add the element to. + * @param timeout - The number of seconds to wait for a blocking operation to complete. A value of `0` will block indefinitely. + * + * Command Response - The popped element, or `null` if `source` does not exist or if the operation timed-out. + * + * Since: Valkey version 6.2.0. */ public blmove( source: string, @@ -807,10 +823,9 @@ export class BaseTransaction> { whereFrom: ListDirection, whereTo: ListDirection, timeout: number, - ): T { return this.addAndReturn( - createLMove(source, destination, whereFrom, whereTo, timeout), + createBLMove(source, destination, whereFrom, whereTo, timeout), ); } diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index 81fb10e214..8d9abbe823 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -1334,6 +1334,144 @@ export function runBaseTests(config: { config.timeout, ); + it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])( + `blmove list_%p`, + async (protocol) => { + await runTest(async (client: BaseClient, cluster) => { + if (cluster.checkIfServerVersionLessThan("6.2.0")) { + return; + } + + const key1 = "{key}-1" + uuidv4(); + const key2 = "{key}-2" + uuidv4(); + const lpushArgs1 = ["2", "1"]; + const lpushArgs2 = ["4", "3"]; + + // Initialize the tests + expect(await client.lpush(key1, lpushArgs1)).toEqual(2); + expect(await client.lpush(key2, lpushArgs2)).toEqual(2); + + // Move from LEFT to LEFT with blocking + checkSimple( + await client.blmove( + key1, + key2, + ListDirection.LEFT, + ListDirection.LEFT, + 0.1, + ), + ).toEqual("1"); + + // Move from LEFT to RIGHT with blocking + checkSimple( + await client.blmove( + key1, + key2, + ListDirection.LEFT, + ListDirection.RIGHT, + 0.1, + ), + ).toEqual("2"); + + checkSimple(await client.lrange(key2, 0, -1)).toEqual([ + "1", + "3", + "4", + "2", + ]); + checkSimple(await client.lrange(key1, 0, -1)).toEqual([]); + + // Move from RIGHT to LEFT non-existing destination with blocking + checkSimple( + await client.blmove( + key2, + key1, + ListDirection.RIGHT, + ListDirection.LEFT, + 0.1, + ), + ).toEqual("2"); + + checkSimple(await client.lrange(key2, 0, -1)).toEqual([ + "1", + "3", + "4", + ]); + checkSimple(await client.lrange(key1, 0, -1)).toEqual(["2"]); + + // Move from RIGHT to RIGHT with blocking + checkSimple( + await client.blmove( + key2, + key1, + ListDirection.RIGHT, + ListDirection.RIGHT, + 0.1, + ), + ).toEqual("4"); + + checkSimple(await client.lrange(key2, 0, -1)).toEqual([ + "1", + "3", + ]); + checkSimple(await client.lrange(key1, 0, -1)).toEqual([ + "2", + "4", + ]); + + // Non-existing source key with blocking + expect( + await client.blmove( + "{key}-non_existing_key" + uuidv4(), + key1, + ListDirection.LEFT, + ListDirection.LEFT, + 0.1, + ), + ).toEqual(null); + + // Non-list source key with blocking + const key3 = "{key}-3" + uuidv4(); + checkSimple(await client.set(key3, "value")).toEqual("OK"); + await expect( + client.blmove( + key3, + key1, + ListDirection.LEFT, + ListDirection.LEFT, + 0.1, + ), + ).rejects.toThrow(RequestError); + + // Non-list destination key + await expect( + client.blmove( + key1, + key3, + ListDirection.LEFT, + ListDirection.LEFT, + 0.1, + ), + ).rejects.toThrow(RequestError); + + // BLMOVE is called against a non-existing key with no timeout, but we wrap the call in an asyncio timeout to + // avoid having the test block forever + if (client instanceof GlideClient) { + expect( + await client.blmove( + "{SameSlot}non_existing_key", + key2, + ListDirection.LEFT, + ListDirection.RIGHT, + 0, + ), + ).rejects.toThrow(); + } + }, protocol); + }, + config.timeout, + ); + it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])( `lset test_%p`, async (protocol) => { From 04ece0135c89a56eb3745bfb5d86fd9ede40d6fc Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Fri, 26 Jul 2024 14:17:23 -0700 Subject: [PATCH 03/14] implement transacton Signed-off-by: Chloe Yip --- node/tests/SharedTests.ts | 48 +++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index 8d9abbe823..3fcf700d95 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -29,6 +29,7 @@ import { Script, UpdateByScore, parseInfoResponse, + TimeoutError, } from "../"; import { RedisCluster } from "../../utils/TestUtils"; import { SingleNodeRoute } from "../build-ts/src/GlideClusterClient"; @@ -1456,17 +1457,44 @@ export function runBaseTests(config: { // BLMOVE is called against a non-existing key with no timeout, but we wrap the call in an asyncio timeout to // avoid having the test block forever - if (client instanceof GlideClient) { - expect( - await client.blmove( - "{SameSlot}non_existing_key", - key2, - ListDirection.LEFT, - ListDirection.RIGHT, - 0, - ), - ).rejects.toThrow(); + + // async setTimeout(() => { + // if (client instanceof GlideClient) { + // expect( + // await client.blmove( + // "{SameSlot}non_existing_key", + // key2, + // ListDirection.LEFT, + // ListDirection.RIGHT, + // 0, + // ), + // ).rejects.toThrow(TimeoutError); + // } + // }, 10000); + + // Helper function to wrap setTimeout in a promise + function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); } + + // Async function using delay + async function blmove_timeout_test() { + await delay(10000); // Wait for 10 seconds + + if (client instanceof GlideClusterClient) { + expect( + await client.blmove( + "{SameSlot}non_existing_key", + key2, + ListDirection.LEFT, + ListDirection.RIGHT, + 0, + ), + ).rejects.toThrow(TimeoutError); + } + } + + blmove_timeout_test(); }, protocol); }, config.timeout, From 6885e03e2928b670e055c62c6632bba44b039f31 Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Fri, 26 Jul 2024 14:49:29 -0700 Subject: [PATCH 04/14] run linter Signed-off-by: Chloe Yip --- node/tests/SharedTests.ts | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index 3fcf700d95..ba7ea777b9 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -1459,20 +1459,19 @@ export function runBaseTests(config: { // avoid having the test block forever // async setTimeout(() => { - // if (client instanceof GlideClient) { - // expect( - // await client.blmove( - // "{SameSlot}non_existing_key", - // key2, - // ListDirection.LEFT, - // ListDirection.RIGHT, - // 0, - // ), - // ).rejects.toThrow(TimeoutError); - // } + // if (client instanceof GlideClient) { + // expect( + // await client.blmove( + // "{SameSlot}non_existing_key", + // key2, + // ListDirection.LEFT, + // ListDirection.RIGHT, + // 0, + // ), + // ).rejects.toThrow(TimeoutError); + // } // }, 10000); - // Helper function to wrap setTimeout in a promise function delay(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } From fc22dfeb135babf275f645375ddeb6be201caae1 Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Fri, 26 Jul 2024 14:53:55 -0700 Subject: [PATCH 05/14] add changelog Signed-off-by: Chloe Yip --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10e2e3cf99..dc96030d75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ #### Changes +* Node: Added BLMOVE command ([#2027](https://github.com/valkey-io/valkey-glide/pull/2027)) * Node: Added LMOVE command ([#2002](https://github.com/valkey-io/valkey-glide/pull/2002)) * Node: Added GEOPOS command ([#1991](https://github.com/valkey-io/valkey-glide/pull/1991)) * Node: Added BITCOUNT command ([#1982](https://github.com/valkey-io/valkey-glide/pull/1982)) From cafb3ebfdcc803b5b3c577623758dfd9992085fd Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Fri, 26 Jul 2024 15:28:11 -0700 Subject: [PATCH 06/14] fixed shared test Signed-off-by: Chloe Yip --- node/tests/SharedTests.ts | 43 +++++++++------------------------------ 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index ba7ea777b9..b8dfdbe7d0 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -1457,40 +1457,17 @@ export function runBaseTests(config: { // BLMOVE is called against a non-existing key with no timeout, but we wrap the call in an asyncio timeout to // avoid having the test block forever - - // async setTimeout(() => { - // if (client instanceof GlideClient) { - // expect( - // await client.blmove( - // "{SameSlot}non_existing_key", - // key2, - // ListDirection.LEFT, - // ListDirection.RIGHT, - // 0, - // ), - // ).rejects.toThrow(TimeoutError); - // } - // }, 10000); - - function delay(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)); - } - - // Async function using delay async function blmove_timeout_test() { - await delay(10000); // Wait for 10 seconds - - if (client instanceof GlideClusterClient) { - expect( - await client.blmove( - "{SameSlot}non_existing_key", - key2, - ListDirection.LEFT, - ListDirection.RIGHT, - 0, - ), - ).rejects.toThrow(TimeoutError); - } + await wait(50000); + expect( + await client.blmove( + "{SameSlot}non_existing_key", + key2, + ListDirection.LEFT, + ListDirection.RIGHT, + 0, + ), + ).rejects.toThrow(TimeoutError); } blmove_timeout_test(); From 7bad778dc4cf07ad811d2ed7cb1712226ff4f040 Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Fri, 26 Jul 2024 16:53:02 -0700 Subject: [PATCH 07/14] implement blmove Signed-off-by: Chloe Yip --- node/tests/SharedTests.ts | 12 ++++++++---- node/tests/TestUtilities.ts | 19 ++++++++++++++----- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index b8dfdbe7d0..5ccca21e7e 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -1459,18 +1459,22 @@ export function runBaseTests(config: { // avoid having the test block forever async function blmove_timeout_test() { await wait(50000); - expect( - await client.blmove( + await expect( + client.blmove( "{SameSlot}non_existing_key", key2, ListDirection.LEFT, ListDirection.RIGHT, 0, ), - ).rejects.toThrow(TimeoutError); + ).rejects.toThrow(ClosingError); } - blmove_timeout_test(); + try { + blmove_timeout_test(); + } catch (ClosingError) { + console.log("Closing error with timeout occurred."); + } }, protocol); }, config.timeout, diff --git a/node/tests/TestUtilities.ts b/node/tests/TestUtilities.ts index 17ce37c6f6..1f8f81a12f 100644 --- a/node/tests/TestUtilities.ts +++ b/node/tests/TestUtilities.ts @@ -487,13 +487,22 @@ export async function transactionTest( field + "3", ]); - baseTransaction.lpopCount(key5, 2); - responseData.push(["lpopCount(key5, 2)", [field + "2"]]); - } else { - baseTransaction.lpopCount(key5, 2); - responseData.push(["lpopCount(key5, 2)", [field + "3", field + "2"]]); + baseTransaction.blmove( + key20, + key5, + ListDirection.LEFT, + ListDirection.LEFT, + 3, + ); + responseData.push([ + "blmove(key20, key5, ListDirection.LEFT, ListDirection.LEFT, 3)", + field + "3", + ]); } + baseTransaction.lpopCount(key5, 2); + responseData.push(["lpopCount(key5, 2)", [field + "3", field + "2"]]); + baseTransaction.linsert( key5, InsertPosition.Before, From c1ed2ccaff67962db695b7802ccedb525f53ebc7 Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Fri, 26 Jul 2024 16:56:14 -0700 Subject: [PATCH 08/14] removed timeout error Signed-off-by: Chloe Yip --- node/tests/SharedTests.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index 5ccca21e7e..828988ef7e 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -29,7 +29,6 @@ import { Script, UpdateByScore, parseInfoResponse, - TimeoutError, } from "../"; import { RedisCluster } from "../../utils/TestUtils"; import { SingleNodeRoute } from "../build-ts/src/GlideClusterClient"; From 4a9e9dfb08de71b52e19b8dd9c24373216924c32 Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Fri, 26 Jul 2024 17:16:31 -0700 Subject: [PATCH 09/14] remove extra line Signed-off-by: Chloe Yip --- node/src/BaseClient.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index d8d85a1c9a..9d463c0282 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -1609,7 +1609,6 @@ export class BaseClient { * @param timeout - The number of seconds to wait for a blocking operation to complete. A value of `0` will block indefinitely. * @returns The popped element, or `null` if `source` does not exist or if the operation timed-out. * - * * Since: Valkey version 6.2.0. * * @example From 8926856f472752e913118d59cbacfbd54085f2c7 Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Mon, 29 Jul 2024 11:33:23 -0700 Subject: [PATCH 10/14] address comments and fixed tests Signed-off-by: Chloe Yip --- node/src/BaseClient.ts | 6 +++--- node/src/Transaction.ts | 4 ++-- node/tests/RedisClusterClient.test.ts | 8 ++++++++ node/tests/SharedTests.ts | 23 +++-------------------- 4 files changed, 16 insertions(+), 25 deletions(-) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index 9d463c0282..e2cda7b055 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -1594,9 +1594,9 @@ export class BaseClient { * Blocks the connection until it pops atomically and removes the left/right-most element to the * list stored at `source` depending on `whereFrom`, and pushes the element at the first/last element * of the list stored at `destination` depending on `whereTo`. - * `BLMOVE` is the blocking variant of `LMOVE`. + * `BLMOVE` is the blocking variant of {@link lmove}. * - * Note: + * @remarks * 1. When in cluster mode, both `source` and `destination` must map to the same hash slot. * 2. `BLMOVE` is a client blocking command, see https://github.com/aws/glide-for-redis/wiki/General-Concepts#blocking-commands for more details and best practices. * @@ -1622,7 +1622,7 @@ export class BaseClient { * console.log(result2); // Output: "two" * * const updated_array2 = await client.lrange("testKey2", 0, -1); - * console.log(updated_array2); // Output: "one", "three", "four"] + * console.log(updated_array2); // Output: ["one", "three", "four"] * ``` */ public async blmove( diff --git a/node/src/Transaction.ts b/node/src/Transaction.ts index 62a6a55f56..1f1ae10f37 100644 --- a/node/src/Transaction.ts +++ b/node/src/Transaction.ts @@ -799,9 +799,9 @@ export class BaseTransaction> { * Blocks the connection until it pops atomically and removes the left/right-most element to the * list stored at `source` depending on `whereFrom`, and pushes the element at the first/last element * of the list stored at `destination` depending on `whereTo`. - * `BLMOVE` is the blocking variant of `LMOVE`. + * `BLMOVE` is the blocking variant of {@link lmove}. * - * Note: + * @remarks * 1. When in cluster mode, both `source` and `destination` must map to the same hash slot. * 2. `BLMOVE` is a client blocking command, see https://github.com/aws/glide-for-redis/wiki/General-Concepts#blocking-commands for more details and best practices. * diff --git a/node/tests/RedisClusterClient.test.ts b/node/tests/RedisClusterClient.test.ts index b60f064b69..4f63e58da2 100644 --- a/node/tests/RedisClusterClient.test.ts +++ b/node/tests/RedisClusterClient.test.ts @@ -17,6 +17,7 @@ import { ClusterTransaction, GlideClusterClient, InfoOptions, + ListDirection, ProtocolVersion, Routes, ScoreFilter, @@ -322,6 +323,13 @@ describe("GlideClusterClient", () => { if (gte(cluster.getVersion(), "6.2.0")) { promises.push( + client.blmove( + "abc", + "def", + ListDirection.LEFT, + ListDirection.LEFT, + 0.2, + ), client.zdiff(["abc", "zxy", "lkn"]), client.zdiffWithScores(["abc", "zxy", "lkn"]), client.zdiffstore("abc", ["zxy", "lkn"]), diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index 828988ef7e..1efd5f6bb7 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -29,6 +29,7 @@ import { Script, UpdateByScore, parseInfoResponse, + TimeoutError, } from "../"; import { RedisCluster } from "../../utils/TestUtils"; import { SingleNodeRoute } from "../build-ts/src/GlideClusterClient"; @@ -1454,26 +1455,8 @@ export function runBaseTests(config: { ), ).rejects.toThrow(RequestError); - // BLMOVE is called against a non-existing key with no timeout, but we wrap the call in an asyncio timeout to - // avoid having the test block forever - async function blmove_timeout_test() { - await wait(50000); - await expect( - client.blmove( - "{SameSlot}non_existing_key", - key2, - ListDirection.LEFT, - ListDirection.RIGHT, - 0, - ), - ).rejects.toThrow(ClosingError); - } - - try { - blmove_timeout_test(); - } catch (ClosingError) { - console.log("Closing error with timeout occurred."); - } + // TODO: add test case with 0 timeout (no timeout) should never time out, + // but we wrap the test with timeout to avoid test failing or stuck forever }, protocol); }, config.timeout, From bbd62d561971dede232255fcda55896b1612aec2 Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Mon, 29 Jul 2024 11:43:19 -0700 Subject: [PATCH 11/14] changed since Signed-off-by: Chloe Yip --- node/src/BaseClient.ts | 4 ++-- node/src/Transaction.ts | 2 +- node/tests/SharedTests.ts | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index 48fd9ce62f..f7e2d38720 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -1643,8 +1643,8 @@ export class BaseClient { * @param timeout - The number of seconds to wait for a blocking operation to complete. A value of `0` will block indefinitely. * @returns The popped element, or `null` if `source` does not exist or if the operation timed-out. * - * Since: Valkey version 6.2.0. - * + * since Valkey version 6.2.0. + * * @example * ```typescript * await client.lpush("testKey1", ["two", "one"]); diff --git a/node/src/Transaction.ts b/node/src/Transaction.ts index eeb396eee2..b6a5eca3c2 100644 --- a/node/src/Transaction.ts +++ b/node/src/Transaction.ts @@ -841,7 +841,7 @@ export class BaseTransaction> { * * Command Response - The popped element, or `null` if `source` does not exist or if the operation timed-out. * - * Since: Valkey version 6.2.0. + * since Valkey version 6.2.0. */ public blmove( source: string, diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts index 268d261d09..ee4a2453eb 100644 --- a/node/tests/SharedTests.ts +++ b/node/tests/SharedTests.ts @@ -30,7 +30,6 @@ import { SortOrder, UpdateByScore, parseInfoResponse, - TimeoutError, } from "../"; import { RedisCluster } from "../../utils/TestUtils"; import { SingleNodeRoute } from "../build-ts/src/GlideClusterClient"; From 9adbfcbf132e9f0f06d3f76a33c5b79d59b1ffe2 Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Mon, 29 Jul 2024 11:44:44 -0700 Subject: [PATCH 12/14] ran linter Signed-off-by: Chloe Yip --- node/src/BaseClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index f7e2d38720..dc17a5eec1 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -1644,7 +1644,7 @@ export class BaseClient { * @returns The popped element, or `null` if `source` does not exist or if the operation timed-out. * * since Valkey version 6.2.0. - * + * * @example * ```typescript * await client.lpush("testKey1", ["two", "one"]); From 4f874356b56f6de1e92a3d7f3f510543391965ca Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Tue, 30 Jul 2024 11:59:09 -0700 Subject: [PATCH 13/14] add blocking test Signed-off-by: Chloe Yip --- node/tests/RedisClient.test.ts | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/node/tests/RedisClient.test.ts b/node/tests/RedisClient.test.ts index fb4b3e4bd8..e56abfd09b 100644 --- a/node/tests/RedisClient.test.ts +++ b/node/tests/RedisClient.test.ts @@ -30,6 +30,7 @@ import { transactionTest, validateTransactionResponse, } from "./TestUtilities"; +import { ListDirection } from ".."; /* eslint-disable @typescript-eslint/no-var-requires */ @@ -126,6 +127,38 @@ describe("GlideClient", () => { }, ); + it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])( + "blocking timeout tests_%p", + async (protocol) => { + client = await GlideClient.createClient( + getClientConfigurationOption( + cluster.getAddresses(), + protocol, + 300, + ), + ); + + const blmovePromise = client.blmove( + "source", + "destination", + ListDirection.LEFT, + ListDirection.LEFT, + 0.1, + ); + const timeoutPromise = new Promise((resolve) => { + setTimeout(resolve, 500); + }); + + try { + await Promise.race([blmovePromise, timeoutPromise]); + } finally { + Promise.resolve(blmovePromise); + client.close(); + } + }, + 5000, + ); + it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])( "select dbsize flushdb test %p", async (protocol) => { From d24f0ad385b808edc42a2a27ad3bbebeb60c8b91 Mon Sep 17 00:00:00 2001 From: Chloe Yip Date: Tue, 30 Jul 2024 13:13:30 -0700 Subject: [PATCH 14/14] change name of blocking test Signed-off-by: Chloe Yip --- node/tests/RedisClient.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/tests/RedisClient.test.ts b/node/tests/RedisClient.test.ts index e56abfd09b..4105876521 100644 --- a/node/tests/RedisClient.test.ts +++ b/node/tests/RedisClient.test.ts @@ -128,7 +128,7 @@ describe("GlideClient", () => { ); it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])( - "blocking timeout tests_%p", + "check that blocking commands returns never timeout_%p", async (protocol) => { client = await GlideClient.createClient( getClientConfigurationOption(