Skip to content

Commit

Permalink
Node: Allow opt in to RESP2 (#730)
Browse files Browse the repository at this point in the history
* Add more type conversions and commands.

* Python: Use RESP3 by default, allow RESP2 opt-in.

* JS: Allow to opt-in to RESP2 usage.

---------

Co-authored-by: nihohit <[email protected]>
  • Loading branch information
shachlanAmazon and nihohit authored Jan 1, 2024
1 parent cfdf3d2 commit 8276a3d
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 25 deletions.
6 changes: 5 additions & 1 deletion node/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
export { BaseClientConfiguration, ReturnType } from "./src/BaseClient";
export {
BaseClientConfiguration,
ProtocolVersion,
ReturnType,
} from "./src/BaseClient";
export {
ExpireOptions,
InfoOptions,
Expand Down
9 changes: 8 additions & 1 deletion node/src/BaseClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ export type ReturnTypeAttribute = {
value: ReturnType;
attributes: ReturnTypeMap;
};
export type ProtocolVersion = connection_request.ProtocolVersion;
export const ProtocolVersion = connection_request.ProtocolVersion;
export type ReturnType =
| "OK"
| string
Expand Down Expand Up @@ -139,6 +141,11 @@ export type BaseClientConfiguration = {
* If not set, `Primary` will be used.
*/
readFrom?: ReadFrom;
/**
* Choose the Redis protocol to be used with the server.
* If not set, `RESP3` will be used.
*/
server_protocol?: ProtocolVersion;
};

function getRequestErrorClass(
Expand Down Expand Up @@ -882,7 +889,7 @@ export class BaseClient {
}
: undefined;
return {
protocol: connection_request.ProtocolVersion.RESP3,
protocol: options.server_protocol,
addresses: options.addresses,
tlsMode: options.useTLS
? connection_request.TlsMode.SecureTls
Expand Down
38 changes: 24 additions & 14 deletions node/tests/RedisClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ import {
import { BufferReader, BufferWriter } from "protobufjs";
import RedisServer from "redis-server";
import { v4 as uuidv4 } from "uuid";
import { BaseClientConfiguration, RedisClient, Transaction } from "../build-ts";
import {
BaseClientConfiguration,
ProtocolVersion,
RedisClient,
Transaction,
} from "../build-ts";
import { redis_request } from "../src/ProtobufMessage";
import { runBaseTests } from "./SharedTests";
import { flushallOnPort, transactionTest } from "./TestUtilities";
Expand Down Expand Up @@ -133,23 +138,26 @@ describe("RedisClient", () => {
client.close();
});

it("can send transactions", async () => {
const client = await RedisClient.createClient(getOptions(port));
const transaction = new Transaction();
const expectedRes = transactionTest(transaction);
transaction.select(0);
const result = await client.exec(transaction);
expectedRes.push("OK");
expect(result).toEqual(expectedRes);
client.close();
});
it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])(
`can send transactions_%p`,
async () => {
const client = await RedisClient.createClient(getOptions(port));
const transaction = new Transaction();
const expectedRes = transactionTest(transaction);
transaction.select(0);
const result = await client.exec(transaction);
expectedRes.push("OK");
expect(result).toEqual(expectedRes);
client.close();
}
);

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"]);
const result1 = await client1.customCommand(["WATCH", "key"]);
expect(result1).toEqual("OK");

const result2 = await client2.set("key", "foo");
Expand All @@ -163,8 +171,10 @@ describe("RedisClient", () => {
});

runBaseTests<Context>({
init: async () => {
const client = await RedisClient.createClient(getOptions(port));
init: async (protocol?) => {
const options = getOptions(port);
options.server_protocol = protocol;
const client = await RedisClient.createClient(options);

return { client, context: { client } };
},
Expand Down
13 changes: 7 additions & 6 deletions node/tests/RedisClusterClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
BaseClientConfiguration,
ClusterTransaction,
InfoOptions,
ProtocolVersion,
RedisClusterClient,
} from "../";
import { runBaseTests } from "./SharedTests";
Expand Down Expand Up @@ -124,11 +125,11 @@ describe("RedisClusterClient", () => {
};

runBaseTests<Context>({
init: async () => {
init: async (protocol) => {
const options = getOptions(cluster.ports());
options.server_protocol = protocol;
testsFailed += 1;
const client = await RedisClusterClient.createClient(
getOptions(cluster.ports())
);
const client = await RedisClusterClient.createClient(options);
return {
context: {
client,
Expand Down Expand Up @@ -208,8 +209,8 @@ describe("RedisClusterClient", () => {
TIMEOUT
);

it(
"can send transactions",
it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])(
`can send transactions_%p`,
async () => {
const client = await RedisClusterClient.createClient(
getOptions(cluster.ports())
Expand Down
36 changes: 33 additions & 3 deletions node/tests/SharedTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { v4 as uuidv4 } from "uuid";
import {
ExpireOptions,
InfoOptions,
ProtocolVersion,
RedisClient,
RedisClusterClient,
parseInfoResponse,
Expand Down Expand Up @@ -31,7 +32,7 @@ async function getVersion(): Promise<[number, number, number]> {
export type BaseClient = RedisClient | RedisClusterClient;

export function runBaseTests<Context>(config: {
init: () => Promise<{
init: (protocol?: ProtocolVersion) => Promise<{
context: Context;
client: BaseClient;
}>;
Expand All @@ -40,8 +41,11 @@ export function runBaseTests<Context>(config: {
}) {
runCommonTests(config);

const runTest = async (test: (client: BaseClient) => Promise<void>) => {
const { context, client } = await config.init();
const runTest = async (
test: (client: BaseClient) => Promise<void>,
protocol?: ProtocolVersion
) => {
const { context, client } = await config.init(protocol);
let testSucceeded = false;
try {
await test(client);
Expand Down Expand Up @@ -69,6 +73,32 @@ export function runBaseTests<Context>(config: {
config.timeout
);

it(
"Check protocol version is RESP3",
async () => {
await runTest(async (client: BaseClient) => {
const result = (await client.customCommand(["HELLO"])) as {
proto: number;
};
expect(result?.proto).toEqual(3);
});
},
config.timeout
);

it(
"Check possible to opt-in to RESP2",
async () => {
await runTest(async (client: BaseClient) => {
const result = (await client.customCommand(["HELLO"])) as {
proto: number;
};
expect(result?.proto).toEqual(2);
}, ProtocolVersion.RESP2);
},
config.timeout
);

it(
"set with return of old value works",
async () => {
Expand Down

0 comments on commit 8276a3d

Please sign in to comment.