fixed set command optional arguments and added tests to set command

valkey-io · May 31, 2024 · 6ce9cbc · 6ce9cbc
1 parent 1415d4d
commit 6ce9cbc
Show file tree

Hide file tree

Showing 3 changed files with 132 additions and 7 deletions.
diff --git a/node/src/Commands.ts b/node/src/Commands.ts
@@ -145,13 +145,13 @@ export function createSet(
         if (options.expiry === "keepExisting") {
             args.push("KEEPTTL");
         } else if (options.expiry?.type === "seconds") {
-            args.push("EX " + options.expiry.count);
+            args.push("EX", String(options.expiry.count));
         } else if (options.expiry?.type === "milliseconds") {
-            args.push("PX " + options.expiry.count);
+            args.push("PX", String(options.expiry.count));
         } else if (options.expiry?.type === "unixSeconds") {
-            args.push("EXAT " + options.expiry.count);
+            args.push("EXAT", String(options.expiry.count));
         } else if (options.expiry?.type === "unixMilliseconds") {
-            args.push("PXAT " + options.expiry.count);
+            args.push("PXAT", String(options.expiry.count));
         }
     }
 

diff --git a/node/tests/RedisClientInternals.test.ts b/node/tests/RedisClientInternals.test.ts
@@ -524,12 +524,13 @@ describe("SocketConnectionInternals", () => {
                     throw new Error("no args");
                 }
 
-                expect(args.length).toEqual(5);
+                expect(args.length).toEqual(6);
                 expect(args[0]).toEqual("foo");
                 expect(args[1]).toEqual("bar");
                 expect(args[2]).toEqual("XX");
                 expect(args[3]).toEqual("GET");
-                expect(args[4]).toEqual("EX 10");
+                expect(args[4]).toEqual("EX");
+                expect(args[5]).toEqual("10");
                 sendResponse(socket, ResponseType.OK, request.callbackIdx);
             });
             const request1 = connection.set("foo", "bar", {
@@ -538,7 +539,7 @@ describe("SocketConnectionInternals", () => {
                 expiry: { type: "seconds", count: 10 },
             });
 
-            await expect(await request1).toMatch("OK");
+            expect(await request1).toMatch("OK");
         });
     });
 

diff --git a/node/tests/SharedTests.ts b/node/tests/SharedTests.ts
@@ -2527,6 +2527,130 @@ export function runBaseTests<Context>(config: {
         },
         config.timeout,
     );
+
+    // Set command tests
+
+    async function classicSet(client: BaseClient, key: string, value: string) {
+        const setRes = await client.set(key, value);
+        expect(setRes).toEqual("OK");
+        const getRes = await client.get(key);
+        expect(getRes).toEqual(value);
+    }
+
+    async function setWithExpiryOptions(
+        client: BaseClient,
+        key: string,
+        value: string,
+    ) {
+        const setRes = await client.set(key, value, {
+            expiry: {
+                type: "milliseconds",
+                count: 500,
+            },
+        });
+        expect(setRes).toEqual("OK");
+        const getRes = await client.get(key);
+        expect(getRes).toEqual(value);
+
+        const setRes2 = await client.set(key, value, {
+            expiry: {
+                type: "seconds",
+                count: 1,
+            },
+        });
+        expect(setRes2).toEqual("OK");
+        const getRes2 = await client.get(key);
+        expect(getRes2).toEqual(value);
+
+        const setRes3 = await client.set(key, value, {
+            expiry: "keepExisting",
+        });
+        expect(setRes3).toEqual("OK");
+        const getRes3 = await client.get(key);
+        expect(getRes3).toEqual(value);
+
+        const setRes4 = await client.set(key, value, {
+            expiry: {
+                type: "unixMilliseconds",
+                count: 2,
+            },
+        });
+        expect(setRes4).toEqual("OK");
+        // wait for the key to expire
+        setTimeout(() => {}, 5);
+        const getRes4 = await client.get(key);
+        // key should have expired
+        expect(getRes4).toEqual(null);
+    }
+
+    async function setWithExistOptions(
+        client: BaseClient,
+        key: string,
+        value: string,
+    ) {
+        const setRes = await client.set(key, value, {
+            conditionalSet: "onlyIfExists",
+        });
+        expect(setRes).toEqual("OK");
+        const getRes = await client.get(key);
+        expect(getRes).toEqual(value);
+
+        const setRes2 = await client.set(key, value + "1", {
+            conditionalSet: "onlyIfDoesNotExist",
+        });
+        // key already exists, so it should not be set
+        expect(setRes2).toEqual(null);
+        // key was already set, so it should not be changed
+        const getRes2 = await client.get(key);
+        expect(getRes2).toEqual(value);
+    }
+
+    async function setWithGetOldOptions(
+        client: BaseClient,
+        key: string,
+        value: string,
+    ) {
+        const setRes = await client.set(key, value, {
+            returnOldValue: true,
+        });
+        expect(setRes).toEqual(value);
+        const getRes = await client.get(key);
+        expect(getRes).toEqual(value);
+        const setRes2 = await client.set(key + "1", value, {
+            returnOldValue: true,
+        });
+        // key did not exist, so it should not return a value
+        expect(setRes2).toEqual(null);
+        const getRes2 = await client.get(key + "1");
+        expect(getRes2).toEqual(value);
+    }
+
+    it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])(
+        "Set commands with options test_%p",
+        async (protocol) => {
+            await runTest(async (client: BaseClient) => {
+                const key = uuidv4();
+                const value = uuidv4();
+                await classicSet(client, key, value);
+                await setWithExpiryOptions(client, key, value);
+                await setWithExistOptions(client, key, value);
+                await setWithGetOldOptions(client, key, value);
+                // try to set with all options, use + 1 to avoid key collision
+                const setRes = await client.set(key + "1", value + "1", {
+                    expiry: {
+                        type: "seconds",
+                        count: 1,
+                    },
+                    conditionalSet: "onlyIfExists",
+                    returnOldValue: true,
+                });
+                expect(setRes).toEqual(value);
+                const getRes = await client.get(key);
+                expect(getRes).toEqual(value + "1");
+            }, protocol);
+        },
+        config.timeout,
+    );
 }
 
 export function runCommonTests<Context>(config: {