Skip to content

Commit

Permalink
DX-997: Make cursor field in scan commands string (#1115)
Browse files Browse the repository at this point in the history
* fix: make return type of cursor commands string

scan commands can return a '0' or a long number as a string. If we deserialize, we get 0 from '0' and a truncated number from the long number string. Second one is non-ideal.

In this case, we want to return the cursor as a string. This means excluding the cursor from the deserialization, hence the new deserializeScanResponse method.

* fix: reorder deserialize and commandOptions

this way, overwriting deserialize will be possible.
  • Loading branch information
CahidArda authored Jun 12, 2024
1 parent 98652ea commit cac6492
Show file tree
Hide file tree
Showing 10 changed files with 86 additions and 50 deletions.
2 changes: 1 addition & 1 deletion pkg/commands/geo_search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ export class GeoSearchCommand<
...(opts?.withHash ? ["WITHHASH"] : []),
],
{
...commandOptions,
deserialize: transform,
...commandOptions,
},
);
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/commands/hscan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe("without options", () => {
const res = await new HScanCommand([key, 0]).exec(client);

expect(res.length).toBe(2);
expect(typeof res[0]).toBe("number");
expect(typeof res[0]).toBe("string");
expect(res![1].length > 0).toBe(true);
});
});
Expand All @@ -23,10 +23,10 @@ describe("with match", () => {
test("returns cursor and members", async () => {
const key = newKey();
await new HSetCommand([key, { field: "value" }]).exec(client);
const res = await new HScanCommand([key, 0, { match: "field" }]).exec(client);
const res = await new HScanCommand([key, "0", { match: "field" }]).exec(client);

expect(res.length).toBe(2);
expect(typeof res[0]).toBe("number");
expect(typeof res[0]).toBe("string");
expect(res![1].length > 0).toBe(true);
});
});
Expand All @@ -35,10 +35,10 @@ describe("with count", () => {
test("returns cursor and members", async () => {
const key = newKey();
await new HSetCommand([key, { field: "value" }]).exec(client);
const res = await new HScanCommand([key, 0, { count: 1 }]).exec(client);
const res = await new HScanCommand([key, "0", { count: 1 }]).exec(client);

expect(res.length).toBe(2);
expect(typeof res[0]).toBe("number");
expect(typeof res[0]).toBe("string");
expect(res![1].length > 0).toBe(true);
});
});
16 changes: 10 additions & 6 deletions pkg/commands/hscan.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,29 @@
import { deserializeScanResponse } from "../util";
import { Command, CommandOptions } from "./command";
import { ScanCommandOptions } from "./scan";

/**
* @see https://redis.io/commands/hscan
*/
export class HScanCommand extends Command<
[number, (string | number)[]],
[number, (string | number)[]]
[string, (string | number)[]],
[string, (string | number)[]]
> {
constructor(
[key, cursor, cmdOpts]: [key: string, cursor: number, cmdOpts?: ScanCommandOptions],
opts?: CommandOptions<[number, (string | number)[]], [number, (string | number)[]]>,
[key, cursor, cmdOpts]: [key: string, cursor: string | number, cmdOpts?: ScanCommandOptions],
opts?: CommandOptions<[string, (string | number)[]], [string, (string | number)[]]>,
) {
const command = ["hscan", key, cursor];
const command: (number | string)[] = ["hscan", key, cursor];
if (cmdOpts?.match) {
command.push("match", cmdOpts.match);
}
if (typeof cmdOpts?.count === "number") {
command.push("count", cmdOpts.count);
}

super(command, opts);
super(command, {
deserialize: deserializeScanResponse,
...opts,
});
}
}
16 changes: 8 additions & 8 deletions pkg/commands/scan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ test("without options", () => {
const key = newKey();
const value = randomID();
await new SetCommand([key, value]).exec(client);
let cursor = 0;
let cursor = "0";
const found: string[] = [];
do {
const res = await new ScanCommand([cursor]).exec(client);
cursor = res[0];
found.push(...res[1]);
} while (cursor !== 0);
} while (cursor !== "0");
expect(found.includes(key)).toBeTrue();
});
});
Expand All @@ -32,14 +32,14 @@ test("with match", () => {
const value = randomID();
await new SetCommand([key, value]).exec(client);

let cursor = 0;
let cursor = "0";
const found: string[] = [];
do {
const res = await new ScanCommand([cursor, { match: key }]).exec(client);
expect(typeof res[0]).toEqual("number");
cursor = res[0];
found.push(...res[1]);
} while (cursor !== 0);
} while (cursor !== "0");

expect(found).toEqual([key]);
});
Expand All @@ -51,13 +51,13 @@ test("with count", () => {
const value = randomID();
await new SetCommand([key, value]).exec(client);

let cursor = 0;
let cursor = "0";
const found: string[] = [];
do {
const res = await new ScanCommand([cursor, { count: 1 }]).exec(client);
cursor = res[0];
found.push(...res[1]);
} while (cursor !== 0);
} while (cursor !== "0");

expect(found.includes(key)).toEqual(true);
});
Expand All @@ -74,13 +74,13 @@ test("with type", () => {
// Add a non-string type
await new ZAddCommand([key2, { score: 1, member: "abc" }]).exec(client);

let cursor = 0;
let cursor = "0";
const found: string[] = [];
do {
const res = await new ScanCommand([cursor, { type: "string" }]).exec(client);
cursor = res[0];
found.push(...res[1]);
} while (cursor !== 0);
} while (cursor !== "0");

expect(found.length).toEqual(1);
for (const key of found) {
Expand Down
17 changes: 12 additions & 5 deletions pkg/commands/scan.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { deserializeScanResponse } from "../util";
import { Command, CommandOptions } from "./command";

export type ScanCommandOptions = {
Expand All @@ -8,12 +9,12 @@ export type ScanCommandOptions = {
/**
* @see https://redis.io/commands/scan
*/
export class ScanCommand extends Command<[number, string[]], [number, string[]]> {
export class ScanCommand extends Command<[string, string[]], [string, string[]]> {
constructor(
[cursor, opts]: [cursor: number, opts?: ScanCommandOptions],
cmdOpts?: CommandOptions<[number, string[]], [number, string[]]>,
[cursor, opts]: [cursor: string | number, opts?: ScanCommandOptions],
cmdOpts?: CommandOptions<[string, string[]], [string, string[]]>,
) {
const command = ["scan", cursor];
const command: (number | string)[] = ["scan", cursor];
if (opts?.match) {
command.push("match", opts.match);
}
Expand All @@ -23,6 +24,12 @@ export class ScanCommand extends Command<[number, string[]], [number, string[]]>
if (opts?.type && opts.type.length > 0) {
command.push("type", opts.type);
}
super(command, cmdOpts);
super(
command,
{
deserialize: deserializeScanResponse,
...cmdOpts,
}
);
}
}
18 changes: 9 additions & 9 deletions pkg/commands/sscan.test.ts
Original file line number Diff line number Diff line change
@@ -1,48 +1,48 @@
import { keygen, newHttpClient, randomID } from "../test-utils";

import { afterAll, expect, test } from "bun:test";
import { afterAll, describe, expect, test } from "bun:test";
import { SAddCommand } from "./sadd";
import { SScanCommand } from "./sscan";
const client = newHttpClient();

const { newKey, cleanup } = keygen();

afterAll(cleanup);
test("without options", () => {
describe("without options", () => {
test("returns cursor and members", async () => {
const key = newKey();
const member = randomID();
await new SAddCommand([key, member]).exec(client);
const res = await new SScanCommand([key, 0]).exec(client);

expect(res.length).toBe(2);
expect(typeof res[0]).toBe("number");
expect(typeof res[0]).toBe("string");
expect(res![1].length > 0).toBe(true);
});
});

test("with match", () => {
describe("with match", () => {
test("returns cursor and members", async () => {
const key = newKey();
const member = randomID();
await new SAddCommand([key, member]).exec(client);
const res = await new SScanCommand([key, 0, { match: member }]).exec(client);
const res = await new SScanCommand([key, "0", { match: member }]).exec(client);

expect(res.length).toBe(2);
expect(typeof res[0]).toBe("number");
expect(typeof res[0]).toBe("string");
expect(res![1].length > 0).toBe(true);
});
});

test("with count", () => {
describe("with count", () => {
test("returns cursor and members", async () => {
const key = newKey();
const member = randomID();
await new SAddCommand([key, member]).exec(client);
const res = await new SScanCommand([key, 0, { count: 1 }]).exec(client);
const res = await new SScanCommand([key, "0", { count: 1 }]).exec(client);

expect(res.length).toBe(2);
expect(typeof res[0]).toBe("number");
expect(typeof res[0]).toBe("string");
expect(res![1].length > 0).toBe(true);
});
});
19 changes: 13 additions & 6 deletions pkg/commands/sscan.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,32 @@
import { deserializeScanResponse } from "../util";
import { Command, CommandOptions } from "./command";
import { ScanCommandOptions } from "./scan";

/**
* @see https://redis.io/commands/sscan
*/
export class SScanCommand extends Command<
[number, (string | number)[]],
[number, (string | number)[]]
[string, (string | number)[]],
[string, (string | number)[]]
> {
constructor(
[key, cursor, opts]: [key: string, cursor: number, opts?: ScanCommandOptions],
cmdOpts?: CommandOptions<[number, (string | number)[]], [number, (string | number)[]]>,
[key, cursor, opts]: [key: string, cursor: string | number, opts?: ScanCommandOptions],
cmdOpts?: CommandOptions<[string, (string | number)[]], [string, (string | number)[]]>,
) {
const command = ["sscan", key, cursor];
const command: (number | string)[] = ["sscan", key, cursor];
if (opts?.match) {
command.push("match", opts.match);
}
if (typeof opts?.count === "number") {
command.push("count", opts.count);
}

super(command, cmdOpts);
super(
command,
{
deserialize: deserializeScanResponse,
...cmdOpts,
}
);
}
}
8 changes: 4 additions & 4 deletions pkg/commands/zscan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe("without options", () => {
const res = await new ZScanCommand([key, 0]).exec(client);

expect(res.length).toBe(2);
expect(typeof res[0]).toBe("number");
expect(typeof res[0]).toBe("string");
expect(res![1].length > 0).toBe(true);
});
});
Expand All @@ -26,10 +26,10 @@ describe("with match", () => {
const key = newKey();
const value = randomID();
await new ZAddCommand([key, { score: 0, member: value }]).exec(client);
const res = await new ZScanCommand([key, 0, { match: value }]).exec(client);
const res = await new ZScanCommand([key, "0", { match: value }]).exec(client);

expect(res.length).toBe(2);
expect(typeof res[0]).toBe("number");
expect(typeof res[0]).toBe("string");
expect(res![1].length > 0).toBe(true);
});
});
Expand All @@ -39,7 +39,7 @@ test("with count", () => {
const key = newKey();
const value = randomID();
await new ZAddCommand([key, { score: 0, member: value }]).exec(client);
const res = await new ZScanCommand([key, 0, { count: 1 }]).exec(client);
const res = await new ZScanCommand([key, "0", { count: 1 }]).exec(client);

expect(res.length).toBe(2);
expect(typeof res[0]).toBe("number");
Expand Down
19 changes: 13 additions & 6 deletions pkg/commands/zscan.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,32 @@
import { deserializeScanResponse } from "../util";
import { Command, CommandOptions } from "./command";
import { ScanCommandOptions } from "./scan";

/**
* @see https://redis.io/commands/zscan
*/
export class ZScanCommand extends Command<
[number, (string | number)[]],
[number, (string | number)[]]
[string, (string | number)[]],
[string, (string | number)[]]
> {
constructor(
[key, cursor, opts]: [key: string, cursor: number, opts?: ScanCommandOptions],
cmdOpts?: CommandOptions<[number, (string | number)[]], [number, (string | number)[]]>,
[key, cursor, opts]: [key: string, cursor: string | number, opts?: ScanCommandOptions],
cmdOpts?: CommandOptions<[string, (string | number)[]], [string, (string | number)[]]>,
) {
const command = ["zscan", key, cursor];
const command: (number | string)[] = ["zscan", key, cursor];
if (opts?.match) {
command.push("match", opts.match);
}
if (typeof opts?.count === "number") {
command.push("count", opts.count);
}

super(command, cmdOpts);
super(
command,
{
deserialize: deserializeScanResponse,
...cmdOpts,
}
);
}
}
11 changes: 11 additions & 0 deletions pkg/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,14 @@ export function parseResponse<TResult>(result: unknown): TResult {
return result as TResult;
}
}

/**
* Deserializes a scan result, excluding the cursor
* which can be string "0" or a big number string.
* Either way, we want it to stay as a string.
*
* @param result
*/
export function deserializeScanResponse<TResult>(result: [string, ...any]): TResult {
return [result[0], ...parseResponse<any[]>(result.slice(1))] as TResult;
}

0 comments on commit cac6492

Please sign in to comment.