Skip to content

Commit

Permalink
Fix exec's return type, to show watch failures.
Browse files Browse the repository at this point in the history
This change ensures that the type system informs the user of the
possibility of WATCH failures in transaction execution.
  • Loading branch information
nihohit committed Dec 14, 2023
1 parent 2f26caf commit 041d141
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 6 deletions.
11 changes: 10 additions & 1 deletion node/src/RedisClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,16 @@ export class RedisClient extends BaseClient {
);
}

public exec(transaction: Transaction): Promise<ReturnType[]> {
/** 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<ReturnType[] | null> {
return this.createWritePromise(transaction.commands);
}

Expand Down
3 changes: 2 additions & 1 deletion node/src/RedisClusterClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ReturnType[]> {
): Promise<ReturnType[] | null> {
return this.createWritePromise(
transaction.commands,
toProtobufRoute(route)
Expand Down
18 changes: 18 additions & 0 deletions node/tests/RedisClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Context>({
init: async () => {
const client = await RedisClient.createClient(getOptions(port));
Expand Down
26 changes: 26 additions & 0 deletions node/tests/RedisClusterClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
});
5 changes: 3 additions & 2 deletions python/python/pybushka/async_commands/cluster_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions python/python/pybushka/async_commands/standalone_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,18 @@ 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.
Args:
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)
Expand Down
45 changes: 45 additions & 0 deletions python/python/tests/test_transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand Down

0 comments on commit 041d141

Please sign in to comment.