From 041d14133da76885ab0a509312d0fd0c28a3f4a8 Mon Sep 17 00:00:00 2001 From: nihohit Date: Thu, 14 Dec 2023 13:22:46 +0000 Subject: [PATCH] Fix `exec`'s return type, to show watch failures. This change ensures that the type system informs the user of the possibility of WATCH failures in transaction execution. --- node/src/RedisClient.ts | 11 ++++- node/src/RedisClusterClient.ts | 3 +- node/tests/RedisClient.test.ts | 18 ++++++++ node/tests/RedisClusterClient.test.ts | 26 +++++++++++ .../async_commands/cluster_commands.py | 5 ++- .../async_commands/standalone_commands.py | 5 ++- python/python/tests/test_transaction.py | 45 +++++++++++++++++++ 7 files changed, 107 insertions(+), 6 deletions(-) diff --git a/node/src/RedisClient.ts b/node/src/RedisClient.ts index e1e9dfe878..5494b4ec09 100644 --- a/node/src/RedisClient.ts +++ b/node/src/RedisClient.ts @@ -81,7 +81,16 @@ export class RedisClient extends BaseClient { ); } - public exec(transaction: Transaction): Promise { + /** Execute a transaction by processing the queued commands. + * See https://redis.io/topics/Transactions/ for details on Redis Transactions. + * + * @param transaction - A Transaction object containing a list of commands to be executed. + * @returns A list of results corresponding to the execution of each command in the transaction. + * If a command returns a value, it will be included in the list. If a command doesn't return a value, + * the list entry will be null. + * If the transaction failed due to a WATCH command, `exec` will return `null`. + */ + public exec(transaction: Transaction): Promise { return this.createWritePromise(transaction.commands); } diff --git a/node/src/RedisClusterClient.ts b/node/src/RedisClusterClient.ts index 2cb83b0b26..e396baaa76 100644 --- a/node/src/RedisClusterClient.ts +++ b/node/src/RedisClusterClient.ts @@ -185,11 +185,12 @@ export class RedisClusterClient extends BaseClient { * @returns A list of results corresponding to the execution of each command in the transaction. * If a command returns a value, it will be included in the list. If a command doesn't return a value, * the list entry will be null. + * If the transaction failed due to a WATCH command, `exec` will return `null`. */ public exec( transaction: ClusterTransaction | BaseTransaction, route?: SingleNodeRoute - ): Promise { + ): Promise { return this.createWritePromise( transaction.commands, toProtobufRoute(route) diff --git a/node/tests/RedisClient.test.ts b/node/tests/RedisClient.test.ts index 762c42ca81..259b5f1e78 100644 --- a/node/tests/RedisClient.test.ts +++ b/node/tests/RedisClient.test.ts @@ -144,6 +144,24 @@ describe("RedisClient", () => { client.dispose(); }); + it("can return null on WATCH transaction failures", async () => { + const client1 = await RedisClient.createClient(getOptions(port)); + const client2 = await RedisClient.createClient(getOptions(port)); + const transaction = new Transaction(); + transaction.get("key"); + const result1 = await client1.customCommand("WATCH", ["key"]); + expect(result1).toEqual("OK"); + + const result2 = await client2.set("key", "foo"); + expect(result2).toEqual("OK"); + + const result3 = await client1.exec(transaction); + expect(result3).toBeNull(); + + client1.dispose(); + client2.dispose(); + }); + runBaseTests({ init: async () => { const client = await RedisClient.createClient(getOptions(port)); diff --git a/node/tests/RedisClusterClient.test.ts b/node/tests/RedisClusterClient.test.ts index bd3c066a87..54be3a33f2 100644 --- a/node/tests/RedisClusterClient.test.ts +++ b/node/tests/RedisClusterClient.test.ts @@ -221,4 +221,30 @@ describe("RedisClusterClient", () => { }, TIMEOUT ); + + it( + "can return null on WATCH transaction failures", + async () => { + const client1 = await RedisClusterClient.createClient( + getOptions(cluster.ports()) + ); + const client2 = await RedisClusterClient.createClient( + getOptions(cluster.ports()) + ); + const transaction = new ClusterTransaction(); + transaction.get("key"); + const result1 = await client1.customCommand("WATCH", ["key"]); + expect(result1).toEqual("OK"); + + const result2 = await client2.set("key", "foo"); + expect(result2).toEqual("OK"); + + const result3 = await client1.exec(transaction); + expect(result3).toBeNull(); + + client1.dispose(); + client2.dispose(); + }, + TIMEOUT + ); }); diff --git a/python/python/pybushka/async_commands/cluster_commands.py b/python/python/pybushka/async_commands/cluster_commands.py index a5ca20d9ea..3d7649ca6c 100644 --- a/python/python/pybushka/async_commands/cluster_commands.py +++ b/python/python/pybushka/async_commands/cluster_commands.py @@ -60,7 +60,7 @@ async def exec( self, transaction: BaseTransaction | ClusterTransaction, route: Optional[TSingleNodeRoute] = None, - ) -> List[TResult]: + ) -> Optional[List[TResult]]: """Execute a transaction by processing the queued commands. See https://redis.io/topics/Transactions/ for details on Redis Transactions. @@ -71,9 +71,10 @@ async def exec( If `route` is provided, the client will route the command to the nodes defined by `route`. Returns: - List[TResult]: A list of results corresponding to the execution of each command + Optional[List[TResult]]: A list of results corresponding to the execution of each command in the transaction. If a command returns a value, it will be included in the list. If a command doesn't return a value, the list entry will be None. + If the transaction failed due to a WATCH command, `exec` will return `None`. """ commands = transaction.commands[:] return await self.execute_transaction(commands, route) diff --git a/python/python/pybushka/async_commands/standalone_commands.py b/python/python/pybushka/async_commands/standalone_commands.py index d0bf67fed6..2d1e331f83 100644 --- a/python/python/pybushka/async_commands/standalone_commands.py +++ b/python/python/pybushka/async_commands/standalone_commands.py @@ -44,7 +44,7 @@ async def info( async def exec( self, transaction: BaseTransaction | Transaction, - ) -> List[TResult]: + ) -> Optional[List[TResult]]: """Execute a transaction by processing the queued commands. See https://redis.io/topics/Transactions/ for details on Redis Transactions. @@ -52,9 +52,10 @@ async def exec( transaction (Transaction): A Transaction object containing a list of commands to be executed. Returns: - List[TResult]: A list of results corresponding to the execution of each command + Optional[List[TResult]]: A list of results corresponding to the execution of each command in the transaction. If a command returns a value, it will be included in the list. If a command doesn't return a value, the list entry will be None. + If the transaction failed due to a WATCH command, `exec` will return `None`. """ commands = transaction.commands[:] return await self.execute_transaction(commands) diff --git a/python/python/tests/test_transaction.py b/python/python/tests/test_transaction.py index 75123af952..de345555b5 100644 --- a/python/python/tests/test_transaction.py +++ b/python/python/tests/test_transaction.py @@ -10,6 +10,7 @@ ) from pybushka.constants import OK, TResult from pybushka.redis_client import RedisClient, RedisClusterClient, TRedisClient +from tests.conftest import create_client from tests.test_async_client import get_random_string @@ -207,6 +208,28 @@ async def test_cluster_transaction(self, redis_client: RedisClusterClient): assert "# Memory" in result[0] assert result[1:] == expected + @pytest.mark.parametrize("cluster_mode", [True]) + async def test_cluster_can_return_null_on_watch_transaction_failures( + self, redis_client: RedisClusterClient, request + ): + client2 = await create_client( + request, + True, + ) + keyslot = get_random_string(3) + transaction = ClusterTransaction() + transaction.get(keyslot) + result = await redis_client.custom_command(["WATCH", keyslot]) + assert result == "ok" + + result = await client2.set(keyslot, "foo") + assert result == "ok" + + result = await redis_client.exec(transaction) + assert result is None + + client2.close() + @pytest.mark.parametrize("cluster_mode", [False]) async def test_standalone_transaction(self, redis_client: RedisClient): keyslot = get_random_string(3) @@ -226,6 +249,28 @@ async def test_standalone_transaction(self, redis_client: RedisClient): assert result[1:6] == [OK, OK, value, OK, None] assert result[6:] == expected + @pytest.mark.parametrize("cluster_mode", [False]) + async def test_standalone_can_return_null_on_watch_transaction_failures( + self, redis_client: RedisClient, request + ): + client2 = await create_client( + request, + False, + ) + keyslot = get_random_string(3) + transaction = ClusterTransaction() + transaction.get(keyslot) + result = await redis_client.custom_command(["WATCH", keyslot]) + assert result == "ok" + + result = await client2.set(keyslot, "foo") + assert result == "ok" + + result = await redis_client.exec(transaction) + assert result is None + + client2.close() + def test_transaction_clear(self): transaction = Transaction() transaction.info()