Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Node binding: keep strings as binary data #1563

Merged
merged 5 commits into from
Jun 16, 2024
Merged

Node binding: keep strings as binary data #1563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
599f6e8
Node binding: string values returning from Rust core, are preserved a…
eifrah-aws Jun 13, 2024
f40ac24
Fixed linter issues
eifrah-aws Jun 13, 2024
1ca27f4
More linter fixes
eifrah-aws Jun 13, 2024
698583f
When converting Set into Array, sort the items
eifrah-aws Jun 13, 2024
a7208c1
stlye fixes
eifrah-aws Jun 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions node/rust-client/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,7 @@ fn redis_value_to_js(val: Value, js_env: Env) -> Result<JsUnknown> {
.map(|val| val.into_unknown()),
Value::Okay => js_env.create_string("OK").map(|val| val.into_unknown()),
Value::Int(num) => js_env.create_int64(num).map(|val| val.into_unknown()),
Value::BulkString(data) => {
let str = to_js_result(std::str::from_utf8(data.as_ref()))?;
js_env.create_string(str).map(|val| val.into_unknown())
}
Value::BulkString(data) => Ok(js_env.create_buffer_with_data(data)?.into_unknown()),
Value::Array(array) => {
let mut js_array_view = js_env.create_array_with_length(array.len())?;
for (index, item) in array.into_iter().enumerate() {
Expand Down Expand Up @@ -224,7 +221,9 @@ fn redis_value_to_js(val: Value, js_env: Env) -> Result<JsUnknown> {
}
}

#[napi(ts_return_type = "null | string | number | {} | Boolean | BigInt | Set<any> | any[]")]
#[napi(
ts_return_type = "null | string | Uint8Array | number | {} | Boolean | BigInt | Set<any> | any[]"
)]
pub fn value_from_split_pointer(js_env: Env, high_bits: u32, low_bits: u32) -> Result<JsUnknown> {
let mut bytes = [0_u8; 8];
(&mut bytes[..4])
Expand Down
4 changes: 2 additions & 2 deletions node/src/BaseClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,8 +504,8 @@ export class BaseClient {
* ```
*/
public set(
key: string,
value: string,
key: string | Uint8Array,
value: string | Uint8Array,
options?: SetOptions,
): Promise<"OK" | string | null> {
return this.createWritePromise(createSet(key, value, options));
Expand Down
20 changes: 13 additions & 7 deletions node/src/Commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { redis_request } from "./ProtobufMessage";

import RequestType = redis_request.RequestType;

function isLargeCommand(args: string[]) {
function isLargeCommand(args: BulkString[]) {
let lenSum = 0;

for (const arg of args) {
Expand All @@ -23,14 +23,20 @@ function isLargeCommand(args: string[]) {
return false;
}

type BulkString = string | Uint8Array;

/**
* Convert a string array into Uint8Array[]
*/
function toBuffersArray(args: string[]) {
function toBuffersArray(args: BulkString[]) {
const argsBytes: Uint8Array[] = [];

for (const str of args) {
argsBytes.push(Buffer.from(str));
for (const arg of args) {
if (typeof arg == "string") {
argsBytes.push(Buffer.from(arg));
} else {
argsBytes.push(arg);
}
}

return argsBytes;
Expand All @@ -56,7 +62,7 @@ export function parseInfoResponse(response: string): Record<string, string> {

function createCommand(
requestType: redis_request.RequestType,
args: string[],
args: BulkString[],
): redis_request.Command {
const singleCommand = redis_request.Command.create({
requestType,
Expand Down Expand Up @@ -137,8 +143,8 @@ export type SetOptions = {
* @internal
*/
export function createSet(
key: string,
value: string,
key: BulkString,
value: BulkString,
options?: SetOptions,
): redis_request.Command {
const args = [key, value];
Expand Down
24 changes: 15 additions & 9 deletions node/tests/RedisClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import {
parseCommandLineArgs,
parseEndpoints,
transactionTest,
intoString,
checkSimple,
} from "./TestUtilities";

/* eslint-disable @typescript-eslint/no-var-requires */
Expand Down Expand Up @@ -108,9 +110,13 @@ describe("RedisClient", () => {
getClientConfigurationOption(cluster.getAddresses(), protocol),
);
const result = await client.info();
expect(result).toEqual(expect.stringContaining("# Server"));
expect(result).toEqual(expect.stringContaining("# Replication"));
expect(result).toEqual(
expect(intoString(result)).toEqual(
expect.stringContaining("# Server"),
);
expect(intoString(result)).toEqual(
expect.stringContaining("# Replication"),
);
expect(intoString(result)).toEqual(
expect.not.stringContaining("# Latencystats"),
);
},
Expand All @@ -123,20 +129,20 @@ describe("RedisClient", () => {
getClientConfigurationOption(cluster.getAddresses(), protocol),
);
let selectResult = await client.select(0);
expect(selectResult).toEqual("OK");
checkSimple(selectResult).toEqual("OK");

const key = uuidv4();
const value = uuidv4();
const result = await client.set(key, value);
expect(result).toEqual("OK");
checkSimple(result).toEqual("OK");

selectResult = await client.select(1);
expect(selectResult).toEqual("OK");
checkSimple(selectResult).toEqual("OK");
expect(await client.get(key)).toEqual(null);

selectResult = await client.select(0);
expect(selectResult).toEqual("OK");
expect(await client.get(key)).toEqual(value);
checkSimple(selectResult).toEqual("OK");
checkSimple(await client.get(key)).toEqual(value);
},
);

Expand All @@ -151,7 +157,7 @@ describe("RedisClient", () => {
transaction.select(0);
const result = await client.exec(transaction);
expectedRes.push("OK");
expect(result).toEqual(expectedRes);
expect(intoString(result)).toEqual(intoString(expectedRes));
},
);

Expand Down
85 changes: 47 additions & 38 deletions node/tests/RedisClusterClient.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import {
parseCommandLineArgs,
parseEndpoints,
transactionTest,
intoString,
intoArray,
} from "./TestUtilities";
type Context = {
client: RedisClusterClient;
Expand Down Expand Up @@ -92,24 +94,20 @@ describe("RedisClusterClient", () => {
const info_server = getFirstResult(
await client.info([InfoOptions.Server]),
);
expect(info_server).toEqual(expect.stringContaining("# Server"));

const result = (await client.info([
InfoOptions.Replication,
])) as Record<string, string>;
const clusterNodes = await client.customCommand([
"CLUSTER",
"NODES",
]);
expect(
(clusterNodes as string)?.split("master").length - 1,
).toEqual(Object.keys(result).length);
Object.values(result).every((item) => {
expect(item).toEqual(expect.stringContaining("# Replication"));
expect(item).toEqual(
expect.not.stringContaining("# Errorstats"),
);
});
expect(intoString(info_server)).toEqual(
expect.stringContaining("# Server"),
);

const infoReplicationValues = Object.values(
await client.info([InfoOptions.Replication]),
);

const replicationInfo = intoArray(infoReplicationValues);

for (const item of replicationInfo) {
expect(item).toContain("role:master");
expect(item).toContain("# Replication");
}
},
TIMEOUT,
);
Expand All @@ -124,9 +122,12 @@ describe("RedisClusterClient", () => {
[InfoOptions.Server],
"randomNode",
);
expect(typeof result).toEqual("string");
expect(result).toEqual(expect.stringContaining("# Server"));
expect(result).toEqual(expect.not.stringContaining("# Errorstats"));
expect(intoString(result)).toEqual(
expect.stringContaining("# Server"),
);
expect(intoString(result)).toEqual(
expect.not.stringContaining("# Errorstats"),
);
},
TIMEOUT,
);
Expand All @@ -148,10 +149,12 @@ describe("RedisClusterClient", () => {
getClientConfigurationOption(cluster.getAddresses(), protocol),
);
const result = cleanResult(
(await client.customCommand(
["cluster", "nodes"],
"randomNode",
)) as string,
intoString(
await client.customCommand(
["cluster", "nodes"],
"randomNode",
),
),
);

// check that routing without explicit port works
Expand All @@ -162,10 +165,12 @@ describe("RedisClusterClient", () => {
}

const secondResult = cleanResult(
(await client.customCommand(["cluster", "nodes"], {
type: "routeByAddress",
host,
})) as string,
intoString(
await client.customCommand(["cluster", "nodes"], {
type: "routeByAddress",
host,
}),
),
);

expect(result).toEqual(secondResult);
Expand All @@ -174,11 +179,13 @@ describe("RedisClusterClient", () => {

// check that routing with explicit port works
const thirdResult = cleanResult(
(await client.customCommand(["cluster", "nodes"], {
type: "routeByAddress",
host: host2,
port: Number(port),
})) as string,
intoString(
await client.customCommand(["cluster", "nodes"], {
type: "routeByAddress",
host: host2,
port: Number(port),
}),
),
);

expect(result).toEqual(thirdResult);
Expand Down Expand Up @@ -212,7 +219,9 @@ describe("RedisClusterClient", () => {
transaction.configSet({ timeout: "1000" });
transaction.configGet(["timeout"]);
const result = await client.exec(transaction);
expect(result).toEqual(["OK", { timeout: "1000" }]);
expect(intoString(result)).toEqual(
intoString(["OK", { timeout: "1000" }]),
);
},
TIMEOUT,
);
Expand All @@ -226,7 +235,7 @@ describe("RedisClusterClient", () => {
const transaction = new ClusterTransaction();
const expectedRes = await transactionTest(transaction);
const result = await client.exec(transaction);
expect(result).toEqual(expectedRes);
expect(intoString(result)).toEqual(intoString(expectedRes));
},
TIMEOUT,
);
Expand Down Expand Up @@ -267,8 +276,8 @@ describe("RedisClusterClient", () => {
const echoDict = await client.echo(message, "allNodes");

expect(typeof echoDict).toBe("object");
expect(Object.values(echoDict)).toEqual(
expect.arrayContaining([message]),
expect(intoArray(echoDict)).toEqual(
expect.arrayContaining(intoArray([message])),
);
},
TIMEOUT,
Expand Down
Loading
Loading