From 3f8d48a2fab37c64818cc7d5dd54ad168e132630 Mon Sep 17 00:00:00 2001 From: avifenesh Date: Sun, 26 Jan 2025 15:05:24 +0000 Subject: [PATCH] Update Redis integration to support valkey-cli and valkey-server; adjust package dependencies and improve error handling Signed-off-by: avifenesh --- glide-core/redis-rs/redis/Cargo.toml | 1 + .../redis-rs/redis/tests/support/cluster.rs | 8 +- .../redis-rs/redis/tests/support/mod.rs | 30 +++-- .../commonjs-test/commonjs-test.cjs | 111 +++++++++--------- .../commonjs-test/package.json | 11 +- .../ecmascript-test/ecmascript-test.mjs | 91 +++++++------- .../ecmascript-test/package.json | 9 +- node/package.json | 1 - node/tests/AsyncClient.test.ts | 25 ++-- node/tests/GlideClusterClient.test.ts | 9 +- node/tests/TestUtilities.ts | 71 +++++++++-- 11 files changed, 220 insertions(+), 147 deletions(-) diff --git a/glide-core/redis-rs/redis/Cargo.toml b/glide-core/redis-rs/redis/Cargo.toml index 534b59fc81..ae0b3a94a3 100644 --- a/glide-core/redis-rs/redis/Cargo.toml +++ b/glide-core/redis-rs/redis/Cargo.toml @@ -181,6 +181,7 @@ anyhow = "1" sscanf = "0.4.1" serial_test = "2" versions = "6.3" +which = "7.0.1" [[test]] name = "test_async" diff --git a/glide-core/redis-rs/redis/tests/support/cluster.rs b/glide-core/redis-rs/redis/tests/support/cluster.rs index 991331cfca..50ec2d3441 100644 --- a/glide-core/redis-rs/redis/tests/support/cluster.rs +++ b/glide-core/redis-rs/redis/tests/support/cluster.rs @@ -217,7 +217,13 @@ impl RedisCluster { )); } - let mut cmd = process::Command::new("redis-cli"); + let cli_command = ["valkey-cli", "redis-cli"] + .iter() + .find(|cmd| which::which(cmd).is_ok()) + .map(|&cmd| cmd) + .unwrap_or_else(|| panic!("Neither valkey-cli nor redis-cli exists in the system.")); + + let mut cmd = process::Command::new(cli_command); cmd.stdout(process::Stdio::null()) .arg("--cluster") .arg("create") diff --git a/glide-core/redis-rs/redis/tests/support/mod.rs b/glide-core/redis-rs/redis/tests/support/mod.rs index 72dc7c9a78..cb4ad4d1d1 100644 --- a/glide-core/redis-rs/redis/tests/support/mod.rs +++ b/glide-core/redis-rs/redis/tests/support/mod.rs @@ -225,17 +225,25 @@ impl RedisServer { modules: &[Module], spawner: F, ) -> RedisServer { - let mut redis_cmd = process::Command::new("redis-server"); + // Check wether the server available is redis or valkey + let server_command = ["valkey-server", "redis-server"] + .iter() + .find(|cmd| which::which(cmd).is_ok()) + .map(|&cmd| cmd) + .unwrap_or_else(|| { + panic!("Neither valkey-server nor redis-server exists in the system.") + }); + let mut server_cmd = process::Command::new(server_command); if let Some(config_path) = config_file { - redis_cmd.arg(config_path); + server_cmd.arg(config_path); } // Load Redis Modules for module in modules { match module { Module::Json => { - redis_cmd + server_cmd .arg("--loadmodule") .arg(env::var("REDIS_RS_REDIS_JSON_PATH").expect( "Unable to find path to RedisJSON at REDIS_RS_REDIS_JSON_PATH, is it set?", @@ -244,24 +252,24 @@ impl RedisServer { }; } - redis_cmd + server_cmd .stdout(process::Stdio::null()) .stderr(process::Stdio::null()); let tempdir = tempfile::Builder::new() .prefix("redis") .tempdir() .expect("failed to create tempdir"); - redis_cmd.arg("--logfile").arg(Self::log_file(&tempdir)); + server_cmd.arg("--logfile").arg(Self::log_file(&tempdir)); match addr { redis::ConnectionAddr::Tcp(ref bind, server_port) => { - redis_cmd + server_cmd .arg("--port") .arg(server_port.to_string()) .arg("--bind") .arg(bind); RedisServer { - process: spawner(&mut redis_cmd), + process: spawner(&mut server_cmd), tempdir, addr, tls_paths: None, @@ -273,7 +281,7 @@ impl RedisServer { let auth_client = if mtls_enabled { "yes" } else { "no" }; // prepare redis with TLS - redis_cmd + server_cmd .arg("--tls-port") .arg(port.to_string()) .arg("--port") @@ -300,20 +308,20 @@ impl RedisServer { }; RedisServer { - process: spawner(&mut redis_cmd), + process: spawner(&mut server_cmd), tempdir, addr, tls_paths: Some(tls_paths), } } redis::ConnectionAddr::Unix(ref path) => { - redis_cmd + server_cmd .arg("--port") .arg("0") .arg("--unixsocket") .arg(path); RedisServer { - process: spawner(&mut redis_cmd), + process: spawner(&mut server_cmd), tempdir, addr, tls_paths: None, diff --git a/node/hybrid-node-tests/commonjs-test/commonjs-test.cjs b/node/hybrid-node-tests/commonjs-test/commonjs-test.cjs index bfa8dc9712..d820e3a308 100644 --- a/node/hybrid-node-tests/commonjs-test/commonjs-test.cjs +++ b/node/hybrid-node-tests/commonjs-test/commonjs-test.cjs @@ -1,73 +1,76 @@ /* eslint no-undef: off */ /* eslint @typescript-eslint/no-require-imports: off */ -const { AsyncClient } = require("glide-rs"); -const RedisServer = require("redis-server"); +const { GlideClient } = require("@valkey/valkey-glide"); const FreePort = require("find-free-port"); -const { execFile } = require("child_process"); +const { promisify } = require("node:util"); +const { exec, spawn } = require("node:child_process"); +const execAsync = promisify(exec); const PORT_NUMBER = 4001; -let server; -let port; -function checkCliAvailability(cli) { - return new Promise((resolve) => { - execFile(cli, ["--version"], (error) => { - resolve(!error); - }); - }); +async function checkCommandAvailability(command) { + try { + const { stdout } = await execAsync(`which ${command}`); + console.log(stdout); + return Boolean(stdout.trim()); + } catch (error) { + console.error(error); + return false; + } } -async function flushallOnPort(port) { - const redisCliAvailable = await checkCliAvailability("redis-cli"); - const valkeyCliAvailable = await checkCliAvailability("valkey-cli"); +async function main() { + console.log("Starting main"); + const port = await FreePort(PORT_NUMBER).then(([free_port]) => free_port); + const redisServerAvailable = await checkCommandAvailability("redis-server"); + const valkeyServerAvailable = + await checkCommandAvailability("valkey-server"); - if (!redisCliAvailable && !valkeyCliAvailable) { - throw new Error("Neither redis-cli nor valkey-cli is available"); + if (!redisServerAvailable && !valkeyServerAvailable) { + throw new Error("Neither valkey nor redis are available"); } - const cli = redisCliAvailable ? "valkey-cli" : "redis-cli"; - return new Promise((resolve, reject) => { - execFile(cli, ["-p", port, "FLUSHALL"], (error, _, stderr) => { - if (error) { - console.error(stderr); - reject(error); - } else { + const server = valkeyServerAvailable ? "valkey-server" : "redis-server"; + console.log(server); + + const serverProcess = spawn(server, ["--port", port.toString()], { + stdio: ["ignore", "pipe", "pipe"], + }); + + await new Promise((resolve) => { + serverProcess.stdout.on("data", (data) => { + console.log(`${data}`); + + if (data.toString().includes("Ready to accept connections")) { resolve(); } }); + + serverProcess.stderr.on("data", (data) => { + console.error(`${data}`); + }); }); -} -FreePort(PORT_NUMBER) - .then(([free_port]) => { - port = free_port; - server = new RedisServer(port); - server.open(async (err) => { - if (err) { - console.error("Error opening server:", err); - throw err; - } + const client = await GlideClient.createClient({ + addresses: [{ host: "localhost", port }], + }); + const setResult = await client.set("test", "test"); + console.log(setResult); + let getResult = await client.get("test"); + console.log(getResult); - const client = AsyncClient.CreateConnection( - `redis://localhost:${port}`, - ); - await client.set("test", "test"); - let result = await client.get("test"); + if (getResult !== "test") { + throw new Error("Common Test failed"); + } else { + console.log("Common Test passed"); + } - if (result !== "test") { - throw new Error("Common Test failed"); - } else { - console.log("Common Test passed"); - } + await client.flushall(); + client.close(); + serverProcess.kill(); +} - await flushallOnPort(port).then(() => { - console.log("db flushed"); - }); - await server.close().then(() => { - console.log("server closed"); - }); - }); - }) - .catch((error) => { - console.error("Error occurred while finding a free port:", error); - }); +main().then(() => { + console.log("Done"); + process.exit(0); +}); diff --git a/node/hybrid-node-tests/commonjs-test/package.json b/node/hybrid-node-tests/commonjs-test/package.json index 6e1291a680..fb1d4ab9d6 100644 --- a/node/hybrid-node-tests/commonjs-test/package.json +++ b/node/hybrid-node-tests/commonjs-test/package.json @@ -2,7 +2,7 @@ "name": "common-test", "version": "1.0.0", "description": "", - "main": "Common-test.cjs", + "main": "commonjs-test.cjs", "type": "commonjs", "scripts": { "build": "cd ../../../node && npm run build", @@ -14,13 +14,10 @@ "author": "Amazon Web Services", "license": "Apache-2.0", "dependencies": { - "child_process": "^1.0.2", - "find-free-port": "^2.0.0", - "valkey-glide": "file:../../../node/build-ts/cjs" + "@valkey/valkey-glide": "file:../..", + "find-free-port": "^2.0.0" }, "devDependencies": { - "@types/node": "^22.8", - "prettier": "^3.3", - "typescript": "^5.6" + "prettier": "^3.3" } } diff --git a/node/hybrid-node-tests/ecmascript-test/ecmascript-test.mjs b/node/hybrid-node-tests/ecmascript-test/ecmascript-test.mjs index 328c7e217d..73930a6085 100644 --- a/node/hybrid-node-tests/ecmascript-test/ecmascript-test.mjs +++ b/node/hybrid-node-tests/ecmascript-test/ecmascript-test.mjs @@ -1,66 +1,73 @@ /* eslint no-undef: off */ -import { execFile } from "child_process"; +import { spawn, execFile } from "child_process"; import findFreePorts from "find-free-ports"; -import { AsyncClient } from "glide-rs"; -import RedisServer from "redis-server"; +import { GlideClient } from "@valkey/valkey-glide"; const PORT_NUMBER = 4001; -let server; -let port; -function checkCliAvailability(cli) { +function checkCommandAvailability(command) { return new Promise((resolve) => { - execFile(cli, ["--version"], (error) => { + execFile(command, ["--version"], (error) => { resolve(!error); }); }); } -async function flushallOnPort(port) { - const redisCliAvailable = await checkCliAvailability("redis-cli"); - const valkeyCliAvailable = await checkCliAvailability("valkey-cli"); +async function main() { + console.log("Starting main"); + const port = await findFreePorts(PORT_NUMBER).then( + ([free_port]) => free_port, + ); + const redisServerAvailable = await checkCommandAvailability("redis-server"); + const valkeyServerAvailable = + await checkCommandAvailability("valkey-server"); - if (!redisCliAvailable && !valkeyCliAvailable) { - throw new Error("Neither redis-cli nor valkey-cli is available"); + if (!redisServerAvailable && !valkeyServerAvailable) { + throw new Error("Neither valkey nor redis are available"); } - const cli = redisCliAvailable ? "valkey-cli" : "redis-cli"; - return new Promise((resolve, reject) => { - execFile(cli, ["-p", port, "FLUSHALL"], (error, _, stderr) => { - if (error) { - console.error(stderr); - reject(error); - } else { + const server = valkeyServerAvailable ? "valkey-server" : "redis-server"; + console.log(server); + + const serverProcess = spawn(server, ["--port", port.toString()], { + stdio: ["ignore", "pipe", "pipe"], + }); + + await new Promise((resolve) => { + serverProcess.stdout.on("data", (data) => { + console.log(`${data}`); + + if (data.toString().includes("Ready to accept connections")) { resolve(); } }); - }); -} -port = await findFreePorts(PORT_NUMBER).then(([free_port]) => free_port); -server = await new Promise((resolve, reject) => { - const server = new RedisServer(port); - server.open(async (err) => { - if (err) { - reject(err); - } + serverProcess.stderr.on("data", (data) => { + console.error(`${data}`); + }); + }); - resolve(server); + const client = await GlideClient.createClient({ + addresses: [{ host: "localhost", port }], }); -}); -const client = AsyncClient.CreateConnection("redis://localhost:" + port); -await client.set("test", "test"); -let result = await client.get("test"); + const setResult = await client.set("test", "test"); + console.log(setResult); + let getResult = await client.get("test"); + console.log(getResult); -if (result !== "test") { - throw new Error("Ecma Test failed"); -} else { - console.log("Ecma Test passed"); + if (getResult !== "test") { + throw new Error("Ecma Test failed"); + } else { + console.log("Ecma Test passed"); + } + + await client.flushall(); + client.close(); + serverProcess.kill(); + console.log("Done"); } -await flushallOnPort(port).then(() => { - console.log("db flushed"); -}); -await server.close().then(() => { - console.log("server closed"); +main().catch((err) => { + console.error(err); + process.exit(1); }); diff --git a/node/hybrid-node-tests/ecmascript-test/package.json b/node/hybrid-node-tests/ecmascript-test/package.json index d1d91d5a97..161f273209 100644 --- a/node/hybrid-node-tests/ecmascript-test/package.json +++ b/node/hybrid-node-tests/ecmascript-test/package.json @@ -14,13 +14,10 @@ "prettier:format": "./node_modules/.bin/prettier --write . " }, "dependencies": { - "child_process": "^1.0.2", - "find-free-ports": "^3.1.1", - "valkey-glide": "file:../../../node/build-ts/mjs" + "@valkey/valkey-glide": "file:../..", + "find-free-ports": "^3.1.1" }, "devDependencies": { - "@types/node": "^22.8", - "prettier": "^3.3", - "typescript": "^5.6" + "prettier": "^3.3" } } diff --git a/node/package.json b/node/package.json index 93b701ad30..739c353c3d 100644 --- a/node/package.json +++ b/node/package.json @@ -56,7 +56,6 @@ "jest": "^29.7.0", "jest-html-reporter": "^3.10.2", "protobufjs-cli": "^1.1.3", - "redis-server": "^1.2.2", "replace": "^1.2.2", "semver": "^7.6.3", "ts-jest": "^29.2.5", diff --git a/node/tests/AsyncClient.test.ts b/node/tests/AsyncClient.test.ts index c810b07f91..56cee10300 100644 --- a/node/tests/AsyncClient.test.ts +++ b/node/tests/AsyncClient.test.ts @@ -4,31 +4,24 @@ import { afterAll, afterEach, beforeAll, describe } from "@jest/globals"; import { AsyncClient } from "glide-rs"; -import RedisServer from "redis-server"; import { runCommonTests } from "./SharedTests"; -import { flushallOnPort } from "./TestUtilities"; +import { flushallOnPort, startServer } from "./TestUtilities"; +import { ChildProcess } from "child_process"; /* eslint-disable @typescript-eslint/no-require-imports */ const FreePort = require("find-free-port"); const PORT_NUMBER = 4000; describe("AsyncClient", () => { - let server: RedisServer; + let serverProcess: ChildProcess; let port: number; + beforeAll(async () => { port = await FreePort(PORT_NUMBER).then( ([free_port]: number[]) => free_port, ); - server = await new Promise((resolve, reject) => { - const server = new RedisServer(port); - server.open(async (err: Error | null) => { - if (err) { - reject(err); - } - - resolve(server); - }); - }); + const server = await startServer(port); + serverProcess = server.process; }); afterEach(async () => { @@ -36,12 +29,14 @@ describe("AsyncClient", () => { }); afterAll(async () => { - await server.close(); + if (serverProcess) { + serverProcess.kill(); + } }); runCommonTests({ init: async () => { - const client = await AsyncClient.CreateConnection( + const client = AsyncClient.CreateConnection( "redis://localhost:" + port, ); diff --git a/node/tests/GlideClusterClient.test.ts b/node/tests/GlideClusterClient.test.ts index 0de8f5e15e..ecc013ef74 100644 --- a/node/tests/GlideClusterClient.test.ts +++ b/node/tests/GlideClusterClient.test.ts @@ -334,7 +334,7 @@ describe("GlideClusterClient", () => { .configGet(["timeout", "cluster-node-timeout"]); const result = await client.exec(transaction); const convertedResult = [ - result[0], + result ? result[0] : null, convertGlideRecordToRecord(result[1]), ]; expect(convertedResult).toEqual([ @@ -1802,7 +1802,12 @@ describe("GlideClusterClient", () => { decoder: Decoder.Bytes, route: route, }); - const data = result?.[0] as Buffer; + + if (!result) { + throw new Error("Transaction failed: result is null"); + } + + const data = result[0] as Buffer; // Verify functionRestore transaction = new ClusterTransaction() diff --git a/node/tests/TestUtilities.ts b/node/tests/TestUtilities.ts index 34c62a9ba5..59bf419164 100644 --- a/node/tests/TestUtilities.ts +++ b/node/tests/TestUtilities.ts @@ -5,6 +5,7 @@ import { expect } from "@jest/globals"; import { exec } from "child_process"; import { v4 as uuidv4 } from "uuid"; +import { Socket } from "net"; import { BaseClient, BaseClientConfiguration, @@ -193,14 +194,6 @@ export async function GetAndSetRandomValue(client: Client) { expect(intoString(result)).toEqual(value); } -function checkCliAvailability(cli: string): Promise { - return new Promise((resolve) => { - exec(`${cli} --version`, (error) => { - resolve(!error); - }); - }); -} - export function flushallOnPort(port: number): Promise { return new Promise((resolve, reject) => { checkCliAvailability("valkey-cli") @@ -2128,3 +2121,65 @@ export async function getServerVersion( return version; } + +/** + * Check if a command is available on the system + */ +export function checkCliAvailability(command: string): Promise { + return new Promise((resolve) => { + exec(`${command} --version`, (error) => { + resolve(!error); + }); + }); +} + +/** + * Starts a Redis-compatible server on the specified port + */ +export function startServer( + port: number, +): Promise<{ process: any; command: string }> { + return new Promise((resolve, reject) => { + Promise.all([ + checkCliAvailability("valkey-server"), + checkCliAvailability("redis-server"), + ]).then(([valkeyExists, redisExists]) => { + if (!valkeyExists && !redisExists) { + reject( + new Error( + "Neither valkey-server nor redis-server is available", + ), + ); + return; + } + + const serverCmd = valkeyExists ? "valkey-server" : "redis-server"; + const process = exec( + `${serverCmd} --port ${port}`, + (error, stdout, stderr) => { + if (error) { + console.error(`Server failed to start: ${stderr}`); + reject(error); + } + }, + ); + + // Give the server a moment to start + setTimeout(() => { + const client = new Socket(); + client.connect(port, "localhost", () => { + client.end(); + resolve({ process, command: serverCmd }); + }); + client.on("error", (err) => { + process.kill(); + reject( + new Error( + `Failed to connect to server: ${err.message}`, + ), + ); + }); + }, 1000); + }); + }); +}