From 353410b7250bfe7076d46287d50c3e8d7017f357 Mon Sep 17 00:00:00 2001 From: Prateek Kumar Date: Wed, 20 Nov 2024 18:30:29 +0530 Subject: [PATCH 01/29] Node: Check exports Signed-off-by: Prateek Kumar --- node/tests/ExportedSymbols.test.ts | 36 ++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 node/tests/ExportedSymbols.test.ts diff --git a/node/tests/ExportedSymbols.test.ts b/node/tests/ExportedSymbols.test.ts new file mode 100644 index 0000000000..34136e9f93 --- /dev/null +++ b/node/tests/ExportedSymbols.test.ts @@ -0,0 +1,36 @@ +import { it } from "@jest/globals"; + +var exportedSymbols = require('../index'); +var exportedSymbolsRust = require('../rust-client/index') + + +describe("Exported Symbols test", () => { + var excludedSymbols: string[] = []; + beforeAll(() => { + /** + * Add Excluded symbols + * Example: + * excludedSymbols.push('convertGlideRecord'); + */ + + /** + * Add Excluded symbols for rust client + * Example: + * excludedSymbols.push('convertGlideRecord'); + */ + + }); + it("check excluded symbols are not exported", () => { + // Check exported symbols for valkey glide package + let exportedSymbolsList = Object.keys(exportedSymbols); + const filteredExportedList = exportedSymbolsList.filter(symbol => excludedSymbols.includes(symbol)); + console.log("Following symbols are exported but are in the exluded list, please remove: " + filteredExportedList); + expect(filteredExportedList.length).toBe(0); + + // Check exported symbols from rust client package. + let exportedRustSymbolsList = Object.keys(exportedSymbolsRust); + const filteredExportedListForRust = exportedRustSymbolsList.filter(symbol => excludedSymbols.includes(symbol)); + console.log("Following symbols are exported but are in the exluded list, please remove: " + filteredExportedListForRust); + expect(filteredExportedListForRust.length).toBe(0); + }); +}); From 20390666b7038767714d075c3b20a77f816770af Mon Sep 17 00:00:00 2001 From: Prateek Kumar Date: Thu, 21 Nov 2024 23:12:27 +0530 Subject: [PATCH 02/29] Node: Get all .ts file names --- node/tests/ExportedSymbols.test.ts | 55 ++++++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/node/tests/ExportedSymbols.test.ts b/node/tests/ExportedSymbols.test.ts index 34136e9f93..cc4b10cde7 100644 --- a/node/tests/ExportedSymbols.test.ts +++ b/node/tests/ExportedSymbols.test.ts @@ -1,8 +1,10 @@ import { it } from "@jest/globals"; +const fs = require('fs/promises'); var exportedSymbols = require('../index'); var exportedSymbolsRust = require('../rust-client/index') - +let i = 0; +let filesWithNodeCode: string[] = []; describe("Exported Symbols test", () => { var excludedSymbols: string[] = []; @@ -20,17 +22,56 @@ describe("Exported Symbols test", () => { */ }); - it("check excluded symbols are not exported", () => { + it("check excluded symbols are not exported", async () => { // Check exported symbols for valkey glide package let exportedSymbolsList = Object.keys(exportedSymbols); const filteredExportedList = exportedSymbolsList.filter(symbol => excludedSymbols.includes(symbol)); console.log("Following symbols are exported but are in the exluded list, please remove: " + filteredExportedList); expect(filteredExportedList.length).toBe(0); - // Check exported symbols from rust client package. - let exportedRustSymbolsList = Object.keys(exportedSymbolsRust); - const filteredExportedListForRust = exportedRustSymbolsList.filter(symbol => excludedSymbols.includes(symbol)); - console.log("Following symbols are exported but are in the exluded list, please remove: " + filteredExportedListForRust); - expect(filteredExportedListForRust.length).toBe(0); + // // Check exported symbols from rust client package. + // let exportedRustSymbolsList = Object.keys(exportedSymbolsRust); + // const filteredExportedListForRust = exportedRustSymbolsList.filter(symbol => excludedSymbols.includes(symbol)); + // console.log("Following symbols are exported but are in the exluded list, please remove: " + filteredExportedListForRust); + // expect(filteredExportedListForRust.length).toBe(0); + + const testFolder = './'; + await getFiles(testFolder); + console.log('Total files found =' + i); + console.log(filesWithNodeCode); }); }); + +const skipFolders = ['build-ts', 'commonjs-test', 'glide-logs', 'hybrid-node-tests', 'node_modules', 'npm', '.cargo', 'target', 'tests']; +async function getFiles(folderName: string) { + const files = await fs.readdir(folderName, { withFileTypes: true }); + for (let file of files) { + if (file.isDirectory()) { + if (skipFolders.includes(file.name)) { + continue; + } + await getFiles(folderName + file.name + '/'); + } else { + if (file.name.endsWith('.js') || + file.name.endsWith('.d.ts') || + file.name.endsWith('.json') || + file.name.endsWith('.rs') || + file.name.endsWith('.html') || + file.name.endsWith('.node') || + file.name.endsWith('.lock') || + file.name.endsWith('.toml') || + file.name.endsWith('.yml') || + file.name.endsWith('.rdb') || + file.name.endsWith('.md') || + file.name.localeCompare('.gitignore') == 0 || + file.name.localeCompare('.prettierignore') == 0 || + file.name.localeCompare('THIRD_PARTY_LICENSES_NODE') == 0 || + file.name.localeCompare('index.ts') == 0) { + continue; + } + i++; + filesWithNodeCode.push(folderName + file.name); + } + } + +} From 07b96dfdc13c43130d20596ca3963d8140c528d4 Mon Sep 17 00:00:00 2001 From: Prateek Kumar Date: Thu, 21 Nov 2024 23:26:38 +0530 Subject: [PATCH 03/29] Node: code cleanup --- node/tests/ExportedSymbols.test.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/node/tests/ExportedSymbols.test.ts b/node/tests/ExportedSymbols.test.ts index cc4b10cde7..ae6790b560 100644 --- a/node/tests/ExportedSymbols.test.ts +++ b/node/tests/ExportedSymbols.test.ts @@ -2,9 +2,9 @@ import { it } from "@jest/globals"; const fs = require('fs/promises'); var exportedSymbols = require('../index'); -var exportedSymbolsRust = require('../rust-client/index') let i = 0; let filesWithNodeCode: string[] = []; +let getActualSymbolsList: string[] = []; describe("Exported Symbols test", () => { var excludedSymbols: string[] = []; @@ -15,12 +15,6 @@ describe("Exported Symbols test", () => { * excludedSymbols.push('convertGlideRecord'); */ - /** - * Add Excluded symbols for rust client - * Example: - * excludedSymbols.push('convertGlideRecord'); - */ - }); it("check excluded symbols are not exported", async () => { // Check exported symbols for valkey glide package @@ -29,12 +23,6 @@ describe("Exported Symbols test", () => { console.log("Following symbols are exported but are in the exluded list, please remove: " + filteredExportedList); expect(filteredExportedList.length).toBe(0); - // // Check exported symbols from rust client package. - // let exportedRustSymbolsList = Object.keys(exportedSymbolsRust); - // const filteredExportedListForRust = exportedRustSymbolsList.filter(symbol => excludedSymbols.includes(symbol)); - // console.log("Following symbols are exported but are in the exluded list, please remove: " + filteredExportedListForRust); - // expect(filteredExportedListForRust.length).toBe(0); - const testFolder = './'; await getFiles(testFolder); console.log('Total files found =' + i); @@ -73,5 +61,4 @@ async function getFiles(folderName: string) { filesWithNodeCode.push(folderName + file.name); } } - } From 6c744111bec0fafca1e9f454c9391d4e07a0c2a0 Mon Sep 17 00:00:00 2001 From: Prateek Kumar Date: Fri, 22 Nov 2024 00:00:10 +0530 Subject: [PATCH 04/29] Node: code cleanup Signed-off-by: Prateek Kumar --- node/tests/ExportedSymbols.test.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/node/tests/ExportedSymbols.test.ts b/node/tests/ExportedSymbols.test.ts index ae6790b560..215a89c9ec 100644 --- a/node/tests/ExportedSymbols.test.ts +++ b/node/tests/ExportedSymbols.test.ts @@ -7,26 +7,31 @@ let filesWithNodeCode: string[] = []; let getActualSymbolsList: string[] = []; describe("Exported Symbols test", () => { - var excludedSymbols: string[] = []; + var excludedSymbolList: string[] = []; beforeAll(() => { /** * Add Excluded symbols * Example: * excludedSymbols.push('convertGlideRecord'); */ - }); it("check excluded symbols are not exported", async () => { // Check exported symbols for valkey glide package - let exportedSymbolsList = Object.keys(exportedSymbols); - const filteredExportedList = exportedSymbolsList.filter(symbol => excludedSymbols.includes(symbol)); - console.log("Following symbols are exported but are in the exluded list, please remove: " + filteredExportedList); + let exportedSymbolsList = Object.keys(exportedSymbols); // exportedList + const filteredExportedList = exportedSymbolsList.filter(symbol => excludedSymbolList.includes(symbol)); + console.log("Following symbols are exported but are in the exlcuded list, please remove: " + filteredExportedList); expect(filteredExportedList.length).toBe(0); const testFolder = './'; await getFiles(testFolder); console.log('Total files found =' + i); console.log(filesWithNodeCode); + + let actualSymbolList: string[] = []; //Actual list + + //1. Test if actualSymbolList - exportedSymbolsList = excludedSymbolList + //2. If actualSymbolList - exportedSymbolsList != excludedSymbolList, + // throw an error that either some symbol not exported or there is some error in the error list. }); }); From 7bda107b70c0ba1114156c85ee86ad48e7a93406 Mon Sep 17 00:00:00 2001 From: Prateek Kumar Date: Mon, 25 Nov 2024 11:54:06 -0800 Subject: [PATCH 05/29] Node: find missing symbols Signed-off-by: Prateek Kumar --- node/tests/ExportedSymbols.test.ts | 79 +++++++++++++++++++++++++----- 1 file changed, 68 insertions(+), 11 deletions(-) diff --git a/node/tests/ExportedSymbols.test.ts b/node/tests/ExportedSymbols.test.ts index 215a89c9ec..0dfed333d9 100644 --- a/node/tests/ExportedSymbols.test.ts +++ b/node/tests/ExportedSymbols.test.ts @@ -1,10 +1,13 @@ import { it } from "@jest/globals"; +import * as ts from "typescript"; const fs = require('fs/promises'); -var exportedSymbols = require('../index'); +const f = require('fs'); + +var exportedSymbols = require('../npm/glide/index'); let i = 0; let filesWithNodeCode: string[] = []; -let getActualSymbolsList: string[] = []; +let getActualSymbolsList: any[] = []; describe("Exported Symbols test", () => { var excludedSymbolList: string[] = []; @@ -18,20 +21,30 @@ describe("Exported Symbols test", () => { it("check excluded symbols are not exported", async () => { // Check exported symbols for valkey glide package let exportedSymbolsList = Object.keys(exportedSymbols); // exportedList - const filteredExportedList = exportedSymbolsList.filter(symbol => excludedSymbolList.includes(symbol)); - console.log("Following symbols are exported but are in the exlcuded list, please remove: " + filteredExportedList); - expect(filteredExportedList.length).toBe(0); + // const filteredExportedList = exportedSymbolsList.filter(symbol => excludedSymbolList.includes(symbol)); + // console.log("Following symbols are exported but are in the exlcuded list, please remove: " + filteredExportedList); + // expect(filteredExportedList.length).toBe(0); const testFolder = './'; await getFiles(testFolder); - console.log('Total files found =' + i); - console.log(filesWithNodeCode); - - let actualSymbolList: string[] = []; //Actual list - + console.log('Total files found =' + i); // 10 + console.log(filesWithNodeCode); //[] + for (let file of filesWithNodeCode) { + const sourceCode = await f.readFileSync(file, 'utf8'); + const sourceFile = await ts.createSourceFile(file, sourceCode, ts.ScriptTarget.Latest, true); + visit(sourceFile, 0); + } + console.log("actual exports in the source code ="); + //console.log(getActualSymbolsList); + console.log("Total exported symbols in index=" + exportedSymbolsList.length); + console.log("Total symbols in the source code=" + getActualSymbolsList.length); + // console.log(exportedSymbolsList); + let l = getActualSymbolsList.filter(actualSymbol => !exportedSymbolsList.includes(actualSymbol)); + console.log("Total missed symbols=" + l.length); + console.log(l); //1. Test if actualSymbolList - exportedSymbolsList = excludedSymbolList //2. If actualSymbolList - exportedSymbolsList != excludedSymbolList, - // throw an error that either some symbol not exported or there is some error in the error list. + // throw an error that either some symbol not exported or there is some error in the excluded symbol list. }); }); @@ -67,3 +80,47 @@ async function getFiles(folderName: string) { } } } + +function visit(node: ts.Node, depth = 0) { + if (depth == 2) { + let name: string | undefined = ""; + if (ts.isFunctionDeclaration(node)) { + name = node.name?.text; + } else if (ts.isVariableStatement(node)) { + name = ""; + } else if (ts.isInterfaceDeclaration(node)) { + name = node.name?.text; + } else if (ts.isClassDeclaration(node)) { + name = node.name?.text; + } else if (ts.isTypeAliasDeclaration(node)) { + name = node.name?.text; + } else if (ts.isEnumDeclaration(node)) { + name = node.name?.text; + } else if (ts.isModuleDeclaration(node)) { + name = node.name?.text; + } else if (ts.SyntaxKind[node.kind] == "FirstStatement") { + name = node.name?.text; + } + let f = 0; + for (let c of node.getChildren()) { + if (c.getText().trim() == "export") { + f = 1; + } + } + if (f == 1) { + if (name == '') { + console.log('depth=' + depth + ", ts.SyntaxKind===" + ts.SyntaxKind[node.kind] + ", name=" + name); + console.log(node.getText()); + } + getActualSymbolsList.push(name); + + } else { + // console.log('depth=' + depth + ", ts.SyntaxKind===" + ts.SyntaxKind[node.kind] + ", name=" + name + " ---not exported"); + } + } + if (depth <= 1) { + for (let c of node.getChildren()) { + visit(c, depth + 1); + } + } +} From befe8129babb7223ff353f62d12bf499eb5a8476 Mon Sep 17 00:00:00 2001 From: adarovadya Date: Wed, 20 Nov 2024 16:23:01 +0200 Subject: [PATCH 06/29] Revert "Python: add AZ Affinity ReadFrom strategy Support" (#2719) * Revert "Python: add AZ Affinity ReadFrom strategy Support (#2676)" This reverts commit 4d33f95c344bb07f4b6d4b6bf37ce5fab55be00f. * removed unessesrry import * add Unicode-3.0 license --------- Signed-off-by: Adar Ovadia Co-authored-by: Adar Ovadia --- CHANGELOG.md | 1 - deny.toml | 3 +- python/python/glide/config.py | 18 -- python/python/glide/glide_client.py | 1 - python/python/tests/conftest.py | 31 --- python/python/tests/test_config.py | 15 -- .../python/tests/test_read_from_strategy.py | 228 ------------------ 7 files changed, 2 insertions(+), 295 deletions(-) delete mode 100644 python/python/tests/test_read_from_strategy.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 80f7c054d4..228aa7bdf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,4 @@ #### Changes -* Python: AZ Affinity - Python Wrapper Support ([#2676](https://github.com/valkey-io/valkey-glide/pull/2676)) * Python: Client API for retrieving internal statistics ([#2707](https://github.com/valkey-io/valkey-glide/pull/2707)) * Node, Python: Adding support for replacing connection configured password ([#2651](https://github.com/valkey-io/valkey-glide/pull/2651))([#2659](https://github.com/valkey-io/valkey-glide/pull/2659)) * Node: Add FT._ALIASLIST command([#2652](https://github.com/valkey-io/valkey-glide/pull/2652)) diff --git a/deny.toml b/deny.toml index 0a43bfa193..526ce9bd1e 100644 --- a/deny.toml +++ b/deny.toml @@ -57,7 +57,8 @@ allow = [ "Unicode-DFS-2016", "ISC", "OpenSSL", - "MPL-2.0" + "MPL-2.0", + "Unicode-3.0" ] # The confidence threshold for detecting a license from license text. # The higher the value, the more closely the license text must be to the diff --git a/python/python/glide/config.py b/python/python/glide/config.py index b33c037cbf..db85202876 100644 --- a/python/python/glide/config.py +++ b/python/python/glide/config.py @@ -41,11 +41,6 @@ class ReadFrom(Enum): Spread the requests between all replicas in a round robin manner. If no replica is available, route the requests to the primary. """ - AZ_AFFINITY = ProtobufReadFrom.AZAffinity - """ - Spread the read requests between replicas in the same client's AZ (Aviliablity zone) in a round robin manner, - falling back to other replicas or the primary if needed - """ class ProtocolVersion(Enum): @@ -140,7 +135,6 @@ def __init__( client_name: Optional[str] = None, protocol: ProtocolVersion = ProtocolVersion.RESP3, inflight_requests_limit: Optional[int] = None, - client_az: Optional[str] = None, ): """ Represents the configuration settings for a Glide client. @@ -178,12 +172,6 @@ def __init__( self.client_name = client_name self.protocol = protocol self.inflight_requests_limit = inflight_requests_limit - self.client_az = client_az - - if read_from == ReadFrom.AZ_AFFINITY and not client_az: - raise ValueError( - "client_az mus t be set when read_from is set to AZ_AFFINITY" - ) def _create_a_protobuf_conn_request( self, cluster_mode: bool = False @@ -216,8 +204,6 @@ def _create_a_protobuf_conn_request( request.protocol = self.protocol.value if self.inflight_requests_limit: request.inflight_requests_limit = self.inflight_requests_limit - if self.client_az: - request.client_az = self.client_az return request @@ -307,7 +293,6 @@ def __init__( protocol: ProtocolVersion = ProtocolVersion.RESP3, pubsub_subscriptions: Optional[PubSubSubscriptions] = None, inflight_requests_limit: Optional[int] = None, - client_az: Optional[str] = None, ): super().__init__( addresses=addresses, @@ -318,7 +303,6 @@ def __init__( client_name=client_name, protocol=protocol, inflight_requests_limit=inflight_requests_limit, - client_az=client_az, ) self.reconnect_strategy = reconnect_strategy self.database_id = database_id @@ -458,7 +442,6 @@ def __init__( ] = PeriodicChecksStatus.ENABLED_DEFAULT_CONFIGS, pubsub_subscriptions: Optional[PubSubSubscriptions] = None, inflight_requests_limit: Optional[int] = None, - client_az: Optional[str] = None, ): super().__init__( addresses=addresses, @@ -469,7 +452,6 @@ def __init__( client_name=client_name, protocol=protocol, inflight_requests_limit=inflight_requests_limit, - client_az=client_az, ) self.periodic_checks = periodic_checks self.pubsub_subscriptions = pubsub_subscriptions diff --git a/python/python/glide/glide_client.py b/python/python/glide/glide_client.py index ea648c49ba..6178b997a7 100644 --- a/python/python/glide/glide_client.py +++ b/python/python/glide/glide_client.py @@ -26,7 +26,6 @@ from glide.protobuf.response_pb2 import RequestErrorType, Response from glide.protobuf_codec import PartialMessageException, ProtobufCodec from glide.routes import Route, set_protobuf_route -from pyrsistent import optional from .glide import ( DEFAULT_TIMEOUT_IN_MILLISECONDS, diff --git a/python/python/tests/conftest.py b/python/python/tests/conftest.py index 85bc58c4b1..437fbd8fbb 100644 --- a/python/python/tests/conftest.py +++ b/python/python/tests/conftest.py @@ -9,7 +9,6 @@ GlideClusterClientConfiguration, NodeAddress, ProtocolVersion, - ReadFrom, ServerCredentials, ) from glide.exceptions import ClosingError, RequestError @@ -133,7 +132,6 @@ def create_clusters(tls, load_module, cluster_endpoints, standalone_endpoints): cluster_mode=True, load_module=load_module, addresses=cluster_endpoints, - replica_count=1, ) pytest.standalone_cluster = ValkeyCluster( tls=tls, @@ -250,8 +248,6 @@ async def create_client( GlideClientConfiguration.PubSubSubscriptions ] = None, inflight_requests_limit: Optional[int] = None, - read_from: ReadFrom = ReadFrom.PRIMARY, - client_az: Optional[str] = None, ) -> Union[GlideClient, GlideClusterClient]: # Create async socket client use_tls = request.config.getoption("--tls") @@ -269,8 +265,6 @@ async def create_client( request_timeout=timeout, pubsub_subscriptions=cluster_mode_pubsub, inflight_requests_limit=inflight_requests_limit, - read_from=read_from, - client_az=client_az, ) return await GlideClusterClient.create(cluster_config) else: @@ -287,8 +281,6 @@ async def create_client( request_timeout=timeout, pubsub_subscriptions=standalone_mode_pubsub, inflight_requests_limit=inflight_requests_limit, - read_from=read_from, - client_az=client_az, ) return await GlideClient.create(config) @@ -389,26 +381,3 @@ async def test_meow_meow(...): reason=f"This feature added in version {min_version}", allow_module_level=True, ) - - -@pytest.fixture(scope="module") -def multiple_replicas_cluster(request): - """ - Fixture to create a special cluster with 4 replicas for specific tests. - """ - tls = request.config.getoption("--tls") - load_module = request.config.getoption("--load-module") - cluster_endpoints = request.config.getoption("--cluster-endpoints") - - if not cluster_endpoints: - multiple_replica_cluster = ValkeyCluster( - tls=tls, - cluster_mode=True, - load_module=load_module, - addresses=cluster_endpoints, - replica_count=4, - ) - yield multiple_replica_cluster - multiple_replica_cluster.__del__() - else: - yield None diff --git a/python/python/tests/test_config.py b/python/python/tests/test_config.py index 3b22adb09c..93c280245f 100644 --- a/python/python/tests/test_config.py +++ b/python/python/tests/test_config.py @@ -52,18 +52,3 @@ def test_periodic_checks_interval_to_protobuf(): config.periodic_checks = PeriodicChecksManualInterval(30) request = config._create_a_protobuf_conn_request(cluster_mode=True) assert request.periodic_checks_manual_interval.duration_in_sec == 30 - - -def test_convert_config_with_azaffinity_to_protobuf(): - az = "us-east-1a" - config = BaseClientConfiguration( - [NodeAddress("127.0.0.1")], - use_tls=True, - read_from=ReadFrom.AZ_AFFINITY, - client_az=az, - ) - request = config._create_a_protobuf_conn_request() - assert isinstance(request, ConnectionRequest) - assert request.tls_mode is TlsMode.SecureTls - assert request.read_from == ProtobufReadFrom.AZAffinity - assert request.client_az == az diff --git a/python/python/tests/test_read_from_strategy.py b/python/python/tests/test_read_from_strategy.py deleted file mode 100644 index fc15481a07..0000000000 --- a/python/python/tests/test_read_from_strategy.py +++ /dev/null @@ -1,228 +0,0 @@ -# Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 - -import re - -import pytest -from glide.async_commands.core import InfoSection -from glide.config import ProtocolVersion, ReadFrom -from glide.constants import OK -from glide.glide_client import GlideClusterClient -from glide.routes import AllNodes, SlotIdRoute, SlotType -from tests.conftest import create_client -from tests.utils.utils import get_first_result - - -@pytest.mark.asyncio -@pytest.mark.usefixtures("multiple_replicas_cluster") -class TestAZAffinity: - async def _get_num_replicas(self, client: GlideClusterClient) -> int: - info_replicas = get_first_result( - await client.info([InfoSection.REPLICATION]) - ).decode() - match = re.search(r"connected_slaves:(\d+)", info_replicas) - if match: - return int(match.group(1)) - else: - raise ValueError( - "Could not find the number of replicas in the INFO REPLICATION response" - ) - - @pytest.mark.skip_if_version_below("8.0.0") - @pytest.mark.parametrize("cluster_mode", [True]) - @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) - async def test_routing_by_slot_to_replica_with_az_affinity_strategy_to_all_replicas( - self, - request, - cluster_mode: bool, - protocol: ProtocolVersion, - multiple_replicas_cluster, - ): - """Test that the client with AZ affinity strategy routes in a round-robin manner to all replicas within the specified AZ""" - - az = "us-east-1a" - client_for_config_set = await create_client( - request, - cluster_mode, - addresses=multiple_replicas_cluster.nodes_addr, - protocol=protocol, - timeout=2000, - ) - await client_for_config_set.config_resetstat() == OK - await client_for_config_set.custom_command( - ["CONFIG", "SET", "availability-zone", az], AllNodes() - ) - await client_for_config_set.close() - - client_for_testing_az = await create_client( - request, - cluster_mode, - addresses=multiple_replicas_cluster.nodes_addr, - protocol=protocol, - read_from=ReadFrom.AZ_AFFINITY, - timeout=2000, - client_az=az, - ) - azs = await client_for_testing_az.custom_command( - ["CONFIG", "GET", "availability-zone"], AllNodes() - ) - - # Check that all replicas have the availability zone set to the az - assert all( - ( - node[1].decode() == az - if isinstance(node, list) - else node[b"availability-zone"].decode() == az - ) - for node in azs.values() - ) - - n_replicas = await self._get_num_replicas(client_for_testing_az) - GET_CALLS = 3 * n_replicas - get_cmdstat = f"cmdstat_get:calls={GET_CALLS // n_replicas}" - - for _ in range(GET_CALLS): - await client_for_testing_az.get("foo") - - info_result = await client_for_testing_az.info( - [InfoSection.COMMAND_STATS, InfoSection.SERVER], AllNodes() - ) - - # Check that all replicas have the same number of GET calls - matching_entries_count = sum( - 1 - for value in info_result.values() - if get_cmdstat in value.decode() and az in value.decode() - ) - assert matching_entries_count == n_replicas - - await client_for_testing_az.close() - - @pytest.mark.skip_if_version_below("8.0.0") - @pytest.mark.parametrize("cluster_mode", [True]) - @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) - async def test_routing_with_az_affinity_strategy_to_1_replica( - self, - request, - cluster_mode: bool, - protocol: ProtocolVersion, - multiple_replicas_cluster, - ): - """Test that the client with az affinity strategy will only route to the 1 replica with the same az""" - az = "us-east-1a" - GET_CALLS = 3 - get_cmdstat = f"cmdstat_get:calls={GET_CALLS}" - - client_for_config_set = await create_client( - request, - cluster_mode, - addresses=multiple_replicas_cluster.nodes_addr, - protocol=protocol, - timeout=2000, - ) - - # Reset the availability zone for all nodes - await client_for_config_set.custom_command( - ["CONFIG", "SET", "availability-zone", ""], - route=AllNodes(), - ) - await client_for_config_set.config_resetstat() == OK - - # 12182 is the slot of "foo" - await client_for_config_set.custom_command( - ["CONFIG", "SET", "availability-zone", az], - route=SlotIdRoute(SlotType.REPLICA, 12182), - ) - - await client_for_config_set.close() - - client_for_testing_az = await create_client( - request, - cluster_mode, - addresses=multiple_replicas_cluster.nodes_addr, - protocol=protocol, - read_from=ReadFrom.AZ_AFFINITY, - timeout=2000, - client_az=az, - ) - - for _ in range(GET_CALLS): - await client_for_testing_az.get("foo") - - info_result = await client_for_testing_az.info( - [InfoSection.SERVER, InfoSection.COMMAND_STATS], AllNodes() - ) - - # Check that only the replica with az has all the GET calls - matching_entries_count = sum( - 1 - for value in info_result.values() - if get_cmdstat in value.decode() and az in value.decode() - ) - assert matching_entries_count == 1 - - # Check that the other replicas have no availability zone set - changed_az_count = sum( - 1 - for node in info_result.values() - if f"availability_zone:{az}" in node.decode() - ) - assert changed_az_count == 1 - - await client_for_testing_az.close() - - @pytest.mark.skip_if_version_below("8.0.0") - @pytest.mark.parametrize("cluster_mode", [True]) - @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) - async def test_az_affinity_non_existing_az( - self, - request, - cluster_mode: bool, - protocol: ProtocolVersion, - multiple_replicas_cluster, - ): - GET_CALLS = 4 - - client_for_testing_az = await create_client( - request, - cluster_mode, - addresses=multiple_replicas_cluster.nodes_addr, - protocol=protocol, - read_from=ReadFrom.AZ_AFFINITY, - timeout=2000, - client_az="non-existing-az", - ) - await client_for_testing_az.config_resetstat() == OK - - for _ in range(GET_CALLS): - await client_for_testing_az.get("foo") - - n_replicas = await self._get_num_replicas(client_for_testing_az) - # We expect the calls to be distributed evenly among the replicas - get_cmdstat = f"cmdstat_get:calls={GET_CALLS // n_replicas}" - - info_result = await client_for_testing_az.info( - [InfoSection.COMMAND_STATS, InfoSection.SERVER], AllNodes() - ) - - matching_entries_count = sum( - 1 for value in info_result.values() if get_cmdstat in value.decode() - ) - assert matching_entries_count == GET_CALLS - - await client_for_testing_az.close() - - @pytest.mark.skip_if_version_below("8.0.0") - @pytest.mark.parametrize("cluster_mode", [True]) - @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) - async def test_az_affinity_requires_client_az( - self, request, cluster_mode: bool, protocol: ProtocolVersion - ): - """Test that setting read_from to AZ_AFFINITY without client_az raises an error.""" - with pytest.raises(ValueError): - await create_client( - request, - cluster_mode=cluster_mode, - protocol=protocol, - read_from=ReadFrom.AZ_AFFINITY, - timeout=2000, - ) From 9111740bef873bcf096ddb90a7c59059ffe3a7e3 Mon Sep 17 00:00:00 2001 From: Shoham Elias <116083498+shohamazon@users.noreply.github.com> Date: Wed, 20 Nov 2024 17:49:51 +0200 Subject: [PATCH 07/29] Python: rename glide json (#2702) * Python: rename glide json Signed-off-by: Shoham Elias --------- Signed-off-by: Shoham Elias --- python/python/glide/__init__.py | 6 +- .../server_modules/{json.py => glide_json.py} | 405 +++++++++--------- .../tests/tests_server_modules/test_ft.py | 2 +- .../tests/tests_server_modules/test_json.py | 4 +- 4 files changed, 214 insertions(+), 203 deletions(-) rename python/python/glide/async_commands/server_modules/{json.py => glide_json.py} (80%) diff --git a/python/python/glide/__init__.py b/python/python/glide/__init__.py index 6f77cac760..c08814ee7a 100644 --- a/python/python/glide/__init__.py +++ b/python/python/glide/__init__.py @@ -32,7 +32,7 @@ InsertPosition, UpdateOptions, ) -from glide.async_commands.server_modules import ft, json +from glide.async_commands.server_modules import ft, glide_json from glide.async_commands.server_modules.ft_options.ft_aggregate_options import ( FtAggregateApply, FtAggregateClause, @@ -67,7 +67,7 @@ FtSearchOptions, ReturnField, ) -from glide.async_commands.server_modules.json import ( +from glide.async_commands.server_modules.glide_json import ( JsonArrIndexOptions, JsonArrPopOptions, JsonGetOptions, @@ -264,7 +264,7 @@ # PubSub "PubSubMsg", # Json - "json", + "glide_json", "JsonGetOptions", "JsonArrIndexOptions", "JsonArrPopOptions", diff --git a/python/python/glide/async_commands/server_modules/json.py b/python/python/glide/async_commands/server_modules/glide_json.py similarity index 80% rename from python/python/glide/async_commands/server_modules/json.py rename to python/python/glide/async_commands/server_modules/glide_json.py index d4c3f8797b..ba9fe26e57 100644 --- a/python/python/glide/async_commands/server_modules/json.py +++ b/python/python/glide/async_commands/server_modules/glide_json.py @@ -3,16 +3,16 @@ Examples: - >>> from glide import json - >>> import json as jsonpy + >>> from glide import glide_json + >>> import json >>> value = {'a': 1.0, 'b': 2} - >>> json_str = jsonpy.dumps(value) # Convert Python dictionary to JSON string using json.dumps() + >>> json_str = json.dumps(value) # Convert Python dictionary to JSON string using json.dumps() >>> await json.set(client, "doc", "$", json_str) 'OK' # Indicates successful setting of the value at path '$' in the key stored at `doc`. - >>> json_get = await json.get(client, "doc", "$") # Returns the value at path '$' in the JSON document stored at `doc` as JSON string. + >>> json_get = await glide_json.get(client, "doc", "$") # Returns the value at path '$' in the JSON document stored at `doc` as JSON string. >>> print(json_get) b"[{\"a\":1.0,\"b\":2}]" - >>> jsonpy.loads(str(json_get)) + >>> json.loads(str(json_get)) [{"a": 1.0, "b" :2}] # JSON object retrieved from the key `doc` using json.loads() """ from typing import List, Optional, Union, cast @@ -100,7 +100,7 @@ def __init__(self, path: TEncodable, index: Optional[int] = None): def to_args(self) -> List[TEncodable]: """ - Get the options as a list of arguments for the JSON.ARRPOP command. + Get the options as a list of arguments for the `JSON.ARRPOP` command. Returns: List[TEncodable]: A list containing the path and, if specified, the index. @@ -135,11 +135,11 @@ async def set( If `value` isn't set because of `set_condition`, returns None. Examples: - >>> from glide import json - >>> import json as jsonpy + >>> from glide import glide_json + >>> import json >>> value = {'a': 1.0, 'b': 2} - >>> json_str = jsonpy.dumps(value) - >>> await json.set(client, "doc", "$", json_str) + >>> json_str = json.dumps(value) + >>> await glide_json.set(client, "doc", "$", json_str) 'OK' # Indicates successful setting of the value at path '$' in the key stored at `doc`. """ args = ["JSON.SET", key, path, value] @@ -181,16 +181,16 @@ async def get( For more information about the returned type, see `TJsonResponse`. Examples: - >>> from glide import json, JsonGetOptions - >>> import as jsonpy - >>> json_str = await json.get(client, "doc", "$") - >>> jsonpy.loads(str(json_str)) # Parse JSON string to Python data + >>> from glide import glide_json, JsonGetOptions + >>> import json + >>> json_str = await glide_json.get(client, "doc", "$") + >>> json.loads(str(json_str)) # Parse JSON string to Python data [{"a": 1.0, "b" :2}] # JSON object retrieved from the key `doc` using json.loads() - >>> await json.get(client, "doc", "$") + >>> await glide_json.get(client, "doc", "$") b"[{\"a\":1.0,\"b\":2}]" # Returns the value at path '$' in the JSON document stored at `doc`. - >>> await json.get(client, "doc", ["$.a", "$.b"], JsonGetOptions(indent=" ", newline="\n", space=" ")) + >>> await glide_json.get(client, "doc", ["$.a", "$.b"], JsonGetOptions(indent=" ", newline="\n", space=" ")) b"{\n \"$.a\": [\n 1.0\n ],\n \"$.b\": [\n 2\n ]\n}" # Returns the values at paths '$.a' and '$.b' in the JSON document stored at `doc`, with specified formatting options. - >>> await json.get(client, "doc", "$.non_existing_path") + >>> await glide_json.get(client, "doc", "$.non_existing_path") b"[]" # Returns an empty array since the path '$.non_existing_path' does not exist in the JSON document stored at `doc`. """ args = ["JSON.GET", key] @@ -204,53 +204,6 @@ async def get( return cast(TJsonResponse[Optional[bytes]], await client.custom_command(args)) -async def mget( - client: TGlideClient, - keys: List[TEncodable], - path: TEncodable, -) -> List[Optional[bytes]]: - """ - Retrieves the JSON values at the specified `path` stored at multiple `keys`. - - Note: - In cluster mode, if keys in `keys` map to different hash slots, the command - will be split across these slots and executed separately for each. This means the command - is atomic only at the slot level. If one or more slot-specific requests fail, the entire - call will return the first encountered error, even though some requests may have succeeded - while others did not. If this behavior impacts your application logic, consider splitting - the request into sub-requests per slot to ensure atomicity. - - Args: - client (TGlideClient): The client to execute the command. - keys (List[TEncodable]): A list of keys for the JSON documents. - path (TEncodable): The path within the JSON documents. - - Returns: - List[Optional[bytes]]: - For JSONPath (`path` starts with `$`): - Returns a list of byte representations of the values found at the given path for each key. - If `path` does not exist within the key, the entry will be an empty array. - For legacy path (`path` doesn't starts with `$`): - Returns a list of byte representations of the values found at the given path for each key. - If `path` does not exist within the key, the entry will be None. - If a key doesn't exist, the corresponding list element will be None. - - - Examples: - >>> from glide import json as glideJson - >>> import json - >>> json_strs = await glideJson.mget(client, ["doc1", "doc2"], "$") - >>> [json.loads(js) for js in json_strs] # Parse JSON strings to Python data - [[{"a": 1.0, "b": 2}], [{"a": 2.0, "b": {"a": 3.0, "b" : 4.0}}]] # JSON objects retrieved from keys `doc1` and `doc2` - >>> await glideJson.mget(client, ["doc1", "doc2"], "$.a") - [b"[1.0]", b"[2.0]"] # Returns values at path '$.a' for the JSON documents stored at `doc1` and `doc2`. - >>> await glideJson.mget(client, ["doc1"], "$.non_existing_path") - [None] # Returns an empty array since the path '$.non_existing_path' does not exist in the JSON document stored at `doc1`. - """ - args = ["JSON.MGET"] + keys + [path] - return cast(TJsonResponse[Optional[bytes]], await client.custom_command(args)) - - async def arrappend( client: TGlideClient, key: TEncodable, @@ -281,15 +234,15 @@ async def arrappend( For more information about the returned type, see `TJsonResponse`. Examples: - >>> from glide import json as valkeyJson + >>> from glide import glide_json >>> import json - >>> await valkeyJson.set(client, "doc", "$", '{"a": 1, "b": ["one", "two"]}') + >>> await glide_json.set(client, "doc", "$", '{"a": 1, "b": ["one", "two"]}') 'OK' # Indicates successful setting of the value at path '$' in the key stored at `doc`. - >>> await valkeyJson.arrappend(client, "doc", ["three"], "$.b") + >>> await glide_json.arrappend(client, "doc", ["three"], "$.b") [3] # Returns the new length of the array at path '$.b' after appending the value. - >>> await valkeyJson.arrappend(client, "doc", ["four"], ".b") + >>> await glide_json.arrappend(client, "doc", ["four"], ".b") 4 # Returns the new length of the array at path '.b' after appending the value. - >>> json.loads(await valkeyJson.get(client, "doc", ".")) + >>> json.loads(await glide_json.get(client, "doc", ".")) {"a": 1, "b": ["one", "two", "three", "four"]} # Returns the updated JSON document """ args = ["JSON.ARRAPPEND", key, path] + values @@ -324,7 +277,7 @@ async def arrindex( Defaults to the full array if not provided. See `JsonArrIndexOptions`. Returns: - Optional[Union[int, List[int]]]: + Optional[TJsonResponse[int]]: For JSONPath (`path` starts with `$`): Returns an array of integers for every possible path, indicating of the first occurrence of `value` within the array, or None for JSON values matching the path that are not an array. @@ -336,28 +289,29 @@ async def arrindex( If multiple paths match, the index of the value from the first matching array is returned. If the JSON value at the `path` is not an array or if `path` does not exist, an error is raised. If `key` does not exist, an error is raised. + For more information about the returned type, see `TJsonResponse`. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '[[], ["a"], ["a", "b"], ["a", "b", "c"]]') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '[[], ["a"], ["a", "b"], ["a", "b", "c"]]') 'OK' - >>> await json.arrindex(client, "doc", "$[*]", '"b"') + >>> await glide_json.arrindex(client, "doc", "$[*]", '"b"') [-1, -1, 1, 1] - >>> await json.set(client, "doc", ".", '{"children": ["John", "Jack", "Tom", "Bob", "Mike"]}') + >>> await glide_json.set(client, "doc", ".", '{"children": ["John", "Jack", "Tom", "Bob", "Mike"]}') 'OK' - >>> await json.arrindex(client, "doc", ".children", '"Tom"') + >>> await glide_json.arrindex(client, "doc", ".children", '"Tom"') 2 - >>> await json.set(client, "doc", "$", '{"fruits": ["apple", "banana", "cherry", "banana", "grape"]}') + >>> await glide_json.set(client, "doc", "$", '{"fruits": ["apple", "banana", "cherry", "banana", "grape"]}') 'OK' - >>> await json.arrindex(client, "doc", "$.fruits", '"banana"', JsonArrIndexOptions(start=2, end=4)) + >>> await glide_json.arrindex(client, "doc", "$.fruits", '"banana"', JsonArrIndexOptions(start=2, end=4)) 3 - >>> await json.set(client, "k", ".", '[1, 2, "a", 4, "a", 6, 7, "b"]') + >>> await glide_json.set(client, "k", ".", '[1, 2, "a", 4, "a", 6, 7, "b"]') 'OK' - >>> await json.arrindex(client, "k", ".", '"b"', JsonArrIndexOptions(start=4, end=0)) + >>> await glide_json.arrindex(client, "k", ".", '"b"', JsonArrIndexOptions(start=4, end=0)) 7 # "b" found at index 7 within the specified range, treating end=0 as the entire array's end. - >>> await json.arrindex(client, "k", ".", '"b"', JsonArrIndexOptions(start=4, end=-1)) + >>> await glide_json.arrindex(client, "k", ".", '"b"', JsonArrIndexOptions(start=4, end=-1)) 7 # "b" found at index 7, with end=-1 covering the full array to its last element. - >>> await json.arrindex(client, "k", ".", '"b"', JsonArrIndexOptions(start=4, end=7)) + >>> await glide_json.arrindex(client, "k", ".", '"b"', JsonArrIndexOptions(start=4, end=7)) -1 # "b" not found within the range from index 4 to exclusive end at index 7. """ args = ["JSON.ARRINDEX", key, path, value] @@ -398,21 +352,22 @@ async def arrinsert( If `path` doesn't exist or the value at `path` is not an array, an error is raised. If the index is out of bounds, an error is raised. If `key` doesn't exist, an error is raised. + For more information about the returned type, see `TJsonResponse`. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '[[], ["a"], ["a", "b"]]') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '[[], ["a"], ["a", "b"]]') 'OK' - >>> await json.arrinsert(client, "doc", "$[*]", 0, ['"c"', '{"key": "value"}', "true", "null", '["bar"]']) + >>> await glide_json.arrinsert(client, "doc", "$[*]", 0, ['"c"', '{"key": "value"}', "true", "null", '["bar"]']) [5, 6, 7] # New lengths of arrays after insertion - >>> await json.get(client, "doc") + >>> await glide_json.get(client, "doc") b'[["c",{"key":"value"},true,null,["bar"]],["c",{"key":"value"},true,null,["bar"],"a"],["c",{"key":"value"},true,null,["bar"],"a","b"]]' - >>> await json.set(client, "doc", "$", '[[], ["a"], ["a", "b"]]') + >>> await glide_json.set(client, "doc", "$", '[[], ["a"], ["a", "b"]]') 'OK' - >>> await json.arrinsert(client, "doc", ".", 0, ['"c"']) + >>> await glide_json.arrinsert(client, "doc", ".", 0, ['"c"']) 4 # New length of the root array after insertion - >>> await json.get(client, "doc") + >>> await glide_json.get(client, "doc") b'[\"c\",[],[\"a\"],[\"a\",\"b\"]]' """ args = ["JSON.ARRINSERT", key, path, str(index)] + values @@ -443,25 +398,26 @@ async def arrlen( If multiple paths match, the length of the first array match is returned. If the JSON value at `path` is not a array or if `path` doesn't exist, an error is raised. If `key` doesn't exist, None is returned. + For more information about the returned type, see `TJsonResponse`. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '{"a": [1, 2, 3], "b": {"a": [1, 2], "c": {"a": 42}}}') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '{"a": [1, 2, 3], "b": {"a": [1, 2], "c": {"a": 42}}}') 'OK' # JSON is successfully set for doc - >>> await json.arrlen(client, "doc", "$") + >>> await glide_json.arrlen(client, "doc", "$") [None] # No array at the root path. - >>> await json.arrlen(client, "doc", "$.a") + >>> await glide_json.arrlen(client, "doc", "$.a") [3] # Retrieves the length of the array at path $.a. - >>> await json.arrlen(client, "doc", "$..a") + >>> await glide_json.arrlen(client, "doc", "$..a") [3, 2, None] # Retrieves lengths of arrays found at all levels of the path `$..a`. - >>> await json.arrlen(client, "doc", "..a") + >>> await glide_json.arrlen(client, "doc", "..a") 3 # Legacy path retrieves the first array match at path `..a`. - >>> await json.arrlen(client, "non_existing_key", "$.a") + >>> await glide_json.arrlen(client, "non_existing_key", "$.a") None # Returns None because the key does not exist. - >>> await json.set(client, "doc", "$", '[1, 2, 3, 4]') + >>> await glide_json.set(client, "doc", "$", '[1, 2, 3, 4]') 'OK' # JSON is successfully set for doc - >>> await json.arrlen(client, "doc") + >>> await glide_json.arrlen(client, "doc") 4 # Retrieves lengths of array in root. """ args = ["JSON.ARRLEN", key] @@ -500,29 +456,30 @@ async def arrpop( If multiple paths match, the value from the first matching array that is not empty is returned. If the JSON value at `options.path` is not a array or if `options.path` doesn't exist, an error is raised. If `key` doesn't exist, an error is raised. + For more information about the returned type, see `TJsonResponse`. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '{"a": [1, 2, true], "b": {"a": [3, 4, ["value", 3, false], 5], "c": {"a": 42}}}') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '{"a": [1, 2, true], "b": {"a": [3, 4, ["value", 3, false], 5], "c": {"a": 42}}}') b'OK' - >>> await json.arrpop(client, "doc", JsonArrPopOptions(path="$.a", index=1)) + >>> await glide_json.arrpop(client, "doc", JsonArrPopOptions(path="$.a", index=1)) [b'2'] # Pop second element from array at path $.a - >>> await json.arrpop(client, "doc", JsonArrPopOptions(path="$..a")) + >>> await glide_json.arrpop(client, "doc", JsonArrPopOptions(path="$..a")) [b'true', b'5', None] # Pop last elements from all arrays matching path `$..a` #### Using a legacy path (..) to pop the first matching array - >>> await json.arrpop(client, "doc", JsonArrPopOptions(path="..a")) + >>> await glide_json.arrpop(client, "doc", JsonArrPopOptions(path="..a")) b"1" # First match popped (from array at path ..a) #### Even though only one value is returned from `..a`, subsequent arrays are also affected - >>> await json.get(client, "doc", "$..a") + >>> await glide_json.get(client, "doc", "$..a") b"[[], [3, 4], 42]" # Remaining elements after pop show the changes - >>> await json.set(client, "doc", "$", '[[], ["a"], ["a", "b", "c"]]') + >>> await glide_json.set(client, "doc", "$", '[[], ["a"], ["a", "b", "c"]]') b'OK' # JSON is successfully set - >>> await json.arrpop(client, "doc", JsonArrPopOptions(path=".", index=-1)) + >>> await glide_json.arrpop(client, "doc", JsonArrPopOptions(path=".", index=-1)) b'["a","b","c"]' # Pop last elements at path `.` - >>> await json.arrpop(client, "doc") + >>> await glide_json.arrpop(client, "doc") b'["a"]' # Pop last elements at path `.` """ args = ["JSON.ARRPOP", key] @@ -567,21 +524,22 @@ async def arrtrim( If multiple paths match, the length of the first trimmed array match is returned. If `path` doesn't exist, or the value at `path` is not an array, an error is raised. If `key` doesn't exist, an error is raised. + For more information about the returned type, see `TJsonResponse`. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '[[], ["a"], ["a", "b"], ["a", "b", "c"]]') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '[[], ["a"], ["a", "b"], ["a", "b", "c"]]') 'OK' - >>> await json.arrtrim(client, "doc", "$[*]", 0, 1) + >>> await glide_json.arrtrim(client, "doc", "$[*]", 0, 1) [0, 1, 2, 2] - >>> await json.get(client, "doc") + >>> await glide_json.get(client, "doc") b'[[],[\"a\"],[\"a\",\"b\"],[\"a\",\"b\"]]' - >>> await json.set(client, "doc", "$", '{"children": ["John", "Jack", "Tom", "Bob", "Mike"]}') + >>> await glide_json.set(client, "doc", "$", '{"children": ["John", "Jack", "Tom", "Bob", "Mike"]}') 'OK' - >>> await json.arrtrim(client, "doc", ".children", 0, 1) + >>> await glide_json.arrtrim(client, "doc", ".children", 0, 1) 2 - >>> await json.get(client, "doc", ".children") + >>> await glide_json.get(client, "doc", ".children") b'["John","Jack"]' """ return cast( @@ -611,28 +569,28 @@ async def clear( If `key doesn't exist, an error is raised. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '{"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14, "nullVal": null}') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '{"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14, "nullVal": null}') 'OK' # JSON document is successfully set. - >>> await json.clear(client, "doc", "$.*") + >>> await glide_json.clear(client, "doc", "$.*") 6 # 6 values are cleared (arrays/objects/strings/numbers/booleans), but `null` remains as is. - >>> await json.get(client, "doc", "$") + >>> await glide_json.get(client, "doc", "$") b'[{"obj":{},"arr":[],"str":"","bool":false,"int":0,"float":0.0,"nullVal":null}]' - >>> await json.clear(client, "doc", "$.*") + >>> await glide_json.clear(client, "doc", "$.*") 0 # No further clearing needed since the containers are already empty and the values are defaults. - >>> await json.set(client, "doc", "$", '{"a": 1, "b": {"a": [5, 6, 7], "b": {"a": true}}, "c": {"a": "value", "b": {"a": 3.5}}, "d": {"a": {"foo": "foo"}}, "nullVal": null}') + >>> await glide_json.set(client, "doc", "$", '{"a": 1, "b": {"a": [5, 6, 7], "b": {"a": true}}, "c": {"a": "value", "b": {"a": 3.5}}, "d": {"a": {"foo": "foo"}}, "nullVal": null}') 'OK' - >>> await json.clear(client, "doc", "b.a[1:3]") + >>> await glide_json.clear(client, "doc", "b.a[1:3]") 2 # 2 elements (`6` and `7`) are cleared. - >>> await json.clear(client, "doc", "b.a[1:3]") + >>> await glide_json.clear(client, "doc", "b.a[1:3]") 0 # No elements cleared since specified slice has already been cleared. - >>> await json.get(client, "doc", "$..a") + >>> await glide_json.get(client, "doc", "$..a") b'[1,[5,0,0],true,"value",3.5,{"foo":"foo"}]' - >>> await json.clear(client, "doc", "$..a") + >>> await glide_json.clear(client, "doc", "$..a") 6 # All numeric, boolean, and string values across paths are cleared. - >>> await json.get(client, "doc", "$..a") + >>> await glide_json.get(client, "doc", "$..a") b'[0,[],false,"",0.0,{}]' """ args = ["JSON.CLEAR", key] @@ -673,21 +631,22 @@ async def debug_fields( If `path` doesn't exist, an error is raised. If `path` is not provided, it reports the total number of fields in the entire JSON document. If `key` doesn't exist, None is returned. + For more information about the returned type, see `TJsonUniversalResponse`. Examples: - >>> from glide import json - >>> await json.set(client, "k1", "$", '[1, 2.3, "foo", true, null, {}, [], {"a":1, "b":2}, [1,2,3]]') + >>> from glide import glide_json + >>> await glide_json.set(client, "k1", "$", '[1, 2.3, "foo", true, null, {}, [], {"a":1, "b":2}, [1,2,3]]') 'OK' - >>> await json.debug_fields(client, "k1", "$[*]") + >>> await glide_json.debug_fields(client, "k1", "$[*]") [1, 1, 1, 1, 1, 0, 0, 2, 3] - >>> await json.debug_fields(client, "k1", ".") + >>> await glide_json.debug_fields(client, "k1", ".") 14 # 9 top-level fields + 5 nested address fields - >>> await json.set(client, "k1", "$", '{"firstName":"John","lastName":"Smith","age":27,"weight":135.25,"isAlive":true,"address":{"street":"21 2nd Street","city":"New York","state":"NY","zipcode":"10021-3100"},"phoneNumbers":[{"type":"home","number":"212 555-1234"},{"type":"office","number":"646 555-4567"}],"children":[],"spouse":null}') + >>> await glide_json.set(client, "k1", "$", '{"firstName":"John","lastName":"Smith","age":27,"weight":135.25,"isAlive":true,"address":{"street":"21 2nd Street","city":"New York","state":"NY","zipcode":"10021-3100"},"phoneNumbers":[{"type":"home","number":"212 555-1234"},{"type":"office","number":"646 555-4567"}],"children":[],"spouse":null}') 'OK' - >>> await json.debug_fields(client, "k1") + >>> await glide_json.debug_fields(client, "k1") 19 - >>> await json.debug_fields(client, "k1", ".address") + >>> await glide_json.debug_fields(client, "k1", ".address") 4 """ args = ["JSON.DEBUG", "FIELDS", key] @@ -723,19 +682,20 @@ async def debug_memory( If `path` doesn't exist, an error is raised. If `path` is not provided, it reports the total memory usage in bytes in the entire JSON document. If `key` doesn't exist, None is returned. + For more information about the returned type, see `TJsonUniversalResponse`. Examples: - >>> from glide import json - >>> await json.set(client, "k1", "$", '[1, 2.3, "foo", true, null, {}, [], {"a":1, "b":2}, [1,2,3]]') + >>> from glide import glide_json + >>> await glide_json.set(client, "k1", "$", '[1, 2.3, "foo", true, null, {}, [], {"a":1, "b":2}, [1,2,3]]') 'OK' - >>> await json.debug_memory(client, "k1", "$[*]") + >>> await glide_json.debug_memory(client, "k1", "$[*]") [16, 16, 19, 16, 16, 16, 16, 66, 64] - >>> await json.set(client, "k1", "$", '{"firstName":"John","lastName":"Smith","age":27,"weight":135.25,"isAlive":true,"address":{"street":"21 2nd Street","city":"New York","state":"NY","zipcode":"10021-3100"},"phoneNumbers":[{"type":"home","number":"212 555-1234"},{"type":"office","number":"646 555-4567"}],"children":[],"spouse":null}') + >>> await glide_json.set(client, "k1", "$", '{"firstName":"John","lastName":"Smith","age":27,"weight":135.25,"isAlive":true,"address":{"street":"21 2nd Street","city":"New York","state":"NY","zipcode":"10021-3100"},"phoneNumbers":[{"type":"home","number":"212 555-1234"},{"type":"office","number":"646 555-4567"}],"children":[],"spouse":null}') 'OK' - >>> await json.debug_memory(client, "k1") + >>> await glide_json.debug_memory(client, "k1") 472 - >>> await json.debug_memory(client, "k1", ".phoneNumbers") + >>> await glide_json.debug_memory(client, "k1", ".phoneNumbers") 164 """ args = ["JSON.DEBUG", "MEMORY", key] @@ -766,14 +726,14 @@ async def delete( If `key` or `path` doesn't exist, returns 0. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '{"a": 1, "nested": {"a": 2, "b": 3}}') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '{"a": 1, "nested": {"a": 2, "b": 3}}') 'OK' # Indicates successful setting of the value at path '$' in the key stored at `doc`. - >>> await json.delete(client, "doc", "$..a") + >>> await glide_json.delete(client, "doc", "$..a") 2 # Indicates successful deletion of the specific values in the key stored at `doc`. - >>> await json.get(client, "doc", "$") + >>> await glide_json.get(client, "doc", "$") "[{\"nested\":{\"b\":3}}]" # Returns the value at path '$' in the JSON document stored at `doc`. - >>> await json.delete(client, "doc") + >>> await glide_json.delete(client, "doc") 1 # Deletes the entire JSON document stored at `doc`. """ @@ -801,14 +761,14 @@ async def forget( If `key` or `path` doesn't exist, returns 0. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '{"a": 1, "nested": {"a": 2, "b": 3}}') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '{"a": 1, "nested": {"a": 2, "b": 3}}') 'OK' # Indicates successful setting of the value at path '$' in the key stored at `doc`. - >>> await json.forget(client, "doc", "$..a") + >>> await glide_json.forget(client, "doc", "$..a") 2 # Indicates successful deletion of the specific values in the key stored at `doc`. - >>> await json.get(client, "doc", "$") + >>> await glide_json.get(client, "doc", "$") "[{\"nested\":{\"b\":3}}]" # Returns the value at path '$' in the JSON document stored at `doc`. - >>> await json.forget(client, "doc") + >>> await glide_json.forget(client, "doc") 1 # Deletes the entire JSON document stored at `doc`. """ @@ -818,6 +778,53 @@ async def forget( ) +async def mget( + client: TGlideClient, + keys: List[TEncodable], + path: TEncodable, +) -> List[Optional[bytes]]: + """ + Retrieves the JSON values at the specified `path` stored at multiple `keys`. + + Note: + In cluster mode, if keys in `keys` map to different hash slots, the command + will be split across these slots and executed separately for each. This means the command + is atomic only at the slot level. If one or more slot-specific requests fail, the entire + call will return the first encountered error, even though some requests may have succeeded + while others did not. If this behavior impacts your application logic, consider splitting + the request into sub-requests per slot to ensure atomicity. + + Args: + client (TGlideClient): The client to execute the command. + keys (List[TEncodable]): A list of keys for the JSON documents. + path (TEncodable): The path within the JSON documents. + + Returns: + List[Optional[bytes]]: + For JSONPath (`path` starts with `$`): + Returns a list of byte representations of the values found at the given path for each key. + If `path` does not exist within the key, the entry will be an empty array. + For legacy path (`path` doesn't starts with `$`): + Returns a list of byte representations of the values found at the given path for each key. + If `path` does not exist within the key, the entry will be None. + If a key doesn't exist, the corresponding list element will be None. + + + Examples: + >>> from glide import glide_json + >>> import json + >>> json_strs = await glide_json.mget(client, ["doc1", "doc2"], "$") + >>> [json.loads(js) for js in json_strs] # Parse JSON strings to Python data + [[{"a": 1.0, "b": 2}], [{"a": 2.0, "b": {"a": 3.0, "b" : 4.0}}]] # JSON objects retrieved from keys `doc1` and `doc2` + >>> await glide_json.mget(client, ["doc1", "doc2"], "$.a") + [b"[1.0]", b"[2.0]"] # Returns values at path '$.a' for the JSON documents stored at `doc1` and `doc2`. + >>> await glide_json.mget(client, ["doc1"], "$.non_existing_path") + [None] # Returns an empty array since the path '$.non_existing_path' does not exist in the JSON document stored at `doc1`. + """ + args = ["JSON.MGET"] + keys + [path] + return cast(TJsonResponse[Optional[bytes]], await client.custom_command(args)) + + async def numincrby( client: TGlideClient, key: TEncodable, @@ -847,12 +854,12 @@ async def numincrby( If the result is out of the range of 64-bit IEEE double, an error is raised. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '{"a": [], "b": [1], "c": [1, 2], "d": [1, 2, 3]}') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '{"a": [], "b": [1], "c": [1, 2], "d": [1, 2, 3]}') 'OK' - >>> await json.numincrby(client, "doc", "$.d[*]", 10) + >>> await glide_json.numincrby(client, "doc", "$.d[*]", 10) b'[11,12,13]' # Increment each element in `d` array by 10. - >>> await json.numincrby(client, "doc", ".c[1]", 10) + >>> await glide_json.numincrby(client, "doc", ".c[1]", 10) b'12' # Increment the second element in the `c` array by 10. """ args = ["JSON.NUMINCRBY", key, path, str(number)] @@ -889,12 +896,12 @@ async def nummultby( If the result is out of the range of 64-bit IEEE double, an error is raised. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '{"a": [], "b": [1], "c": [1, 2], "d": [1, 2, 3]}') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '{"a": [], "b": [1], "c": [1, 2], "d": [1, 2, 3]}') 'OK' - >>> await json.nummultby(client, "doc", "$.d[*]", 2) + >>> await glide_json.nummultby(client, "doc", "$.d[*]", 2) b'[2,4,6]' # Multiplies each element in the `d` array by 2. - >>> await json.nummultby(client, "doc", ".c[1]", 2) + >>> await glide_json.nummultby(client, "doc", ".c[1]", 2) b'4' # Multiplies the second element in the `c` array by 2. """ args = ["JSON.NUMMULTBY", key, path, str(number)] @@ -926,23 +933,24 @@ async def objlen( If multiple paths match, the length of the first object match is returned. If the JSON value at `path` is not an object or if `path` doesn't exist, an error is raised. If `key` doesn't exist, None is returned. + For more information about the returned type, see `TJsonResponse`. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '{"a": 1.0, "b": {"a": {"x": 1, "y": 2}, "b": 2.5, "c": true}}') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '{"a": 1.0, "b": {"a": {"x": 1, "y": 2}, "b": 2.5, "c": true}}') b'OK' # Indicates successful setting of the value at the root path '$' in the key `doc`. - >>> await json.objlen(client, "doc", "$") + >>> await glide_json.objlen(client, "doc", "$") [2] # Returns the number of key-value pairs at the root object, which has 2 keys: 'a' and 'b'. - >>> await json.objlen(client, "doc", ".") + >>> await glide_json.objlen(client, "doc", ".") 2 # Returns the number of key-value pairs for the object matching the path '.', which has 2 keys: 'a' and 'b'. - >>> await json.objlen(client, "doc", "$.b") + >>> await glide_json.objlen(client, "doc", "$.b") [3] # Returns the length of the object at path '$.b', which has 3 keys: 'a', 'b', and 'c'. - >>> await json.objlen(client, "doc", ".b") + >>> await glide_json.objlen(client, "doc", ".b") 3 # Returns the length of the nested object at path '.b', which has 3 keys. - >>> await json.objlen(client, "doc", "$..a") + >>> await glide_json.objlen(client, "doc", "$..a") [None, 2] - >>> await json.objlen(client, "doc") + >>> await glide_json.objlen(client, "doc") 2 # Returns the number of key-value pairs for the object matching the path '.', which has 2 keys: 'a' and 'b'. """ args = ["JSON.OBJLEN", key] @@ -980,18 +988,19 @@ async def objkeys( If a value matching the path is not an object, an error is raised. If `path` doesn't exist, None is returned. If `key` doesn't exist, None is returned. + For more information about the returned type, see `TJsonUniversalResponse`. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '{"a": 1.0, "b": {"a": {"x": 1, "y": 2}, "b": 2.5, "c": true}}') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '{"a": 1.0, "b": {"a": {"x": 1, "y": 2}, "b": 2.5, "c": true}}') b'OK' # Indicates successful setting of the value at the root path '$' in the key `doc`. - >>> await json.objkeys(client, "doc", "$") + >>> await glide_json.objkeys(client, "doc", "$") [[b"a", b"b"]] # Returns a list of arrays containing the key names for objects matching the path '$'. - >>> await json.objkeys(client, "doc", ".") + >>> await glide_json.objkeys(client, "doc", ".") [b"a", b"b"] # Returns key names for the object matching the path '.' as it is the only match. - >>> await json.objkeys(client, "doc", "$.b") + >>> await glide_json.objkeys(client, "doc", "$.b") [[b"a", b"b", b"c"]] # Returns key names as a nested list for objects matching the JSONPath '$.b'. - >>> await json.objkeys(client, "doc", ".b") + >>> await glide_json.objkeys(client, "doc", ".b") [b"a", b"b", b"c"] # Returns key names for the nested object at path '.b'. """ args = ["JSON.OBJKEYS", key] @@ -1036,14 +1045,15 @@ async def resp( If multiple paths match, the value of the first JSON value match is returned. If `path` doesn't exist, an error is raised. If `key` doesn't exist, an None is returned. + For more information about the returned type, see `TJsonUniversalResponse`. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '{"a": [1, 2, 3], "b": {"a": [1, 2], "c": {"a": 42}}}') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '{"a": [1, 2, 3], "b": {"a": [1, 2], "c": {"a": 42}}}') 'OK' - >>> await json.resp(client, "doc", "$..a") + >>> await glide_json.resp(client, "doc", "$..a") [[b"[", 1, 2, 3],[b"[", 1, 2],42] - >>> await json.resp(client, "doc", "..a") + >>> await glide_json.resp(client, "doc", "..a") [b"[", 1, 2, 3] """ args = ["JSON.RESP", key] @@ -1087,15 +1097,15 @@ async def strappend( For more information about the returned type, see `TJsonResponse`. Examples: - >>> from glide import json - >>> import json as jsonpy - >>> await json.set(client, "doc", "$", jsonpy.dumps({"a":"foo", "nested": {"a": "hello"}, "nested2": {"a": 31}})) + >>> from glide import glide_json + >>> import json + >>> await glide_json.set(client, "doc", "$", json.dumps({"a":"foo", "nested": {"a": "hello"}, "nested2": {"a": 31}})) 'OK' - >>> await json.strappend(client, "doc", jsonpy.dumps("baz"), "$..a") + >>> await glide_json.strappend(client, "doc", json.dumps("baz"), "$..a") [6, 8, None] # The new length of the string values at path '$..a' in the key stored at `doc` after the append operation. - >>> await json.strappend(client, "doc", '"foo"', "nested.a") + >>> await glide_json.strappend(client, "doc", '"foo"', "nested.a") 11 # The length of the string value after appending "foo" to the string at path 'nested.array' in the key stored at `doc`. - >>> jsonpy.loads(await json.get(client, jsonpy.dumps("doc"), "$")) + >>> json.loads(await glide_json.get(client, json.dumps("doc"), "$")) [{"a":"foobaz", "nested": {"a": "hellobazfoo"}, "nested2": {"a": 31}}] # The updated JSON value in the key stored at `doc`. """ @@ -1133,17 +1143,17 @@ async def strlen( For more information about the returned type, see `TJsonResponse`. Examples: - >>> from glide import json - >>> import jsonpy - >>> await json.set(client, "doc", "$", jsonpy.dumps({"a":"foo", "nested": {"a": "hello"}, "nested2": {"a": 31}})) + >>> from glide import glide_json + >>> import json + >>> await glide_json.set(client, "doc", "$", json.dumps({"a":"foo", "nested": {"a": "hello"}, "nested2": {"a": 31}})) 'OK' - >>> await json.strlen(client, "doc", "$..a") + >>> await glide_json.strlen(client, "doc", "$..a") [3, 5, None] # The length of the string values at path '$..a' in the key stored at `doc`. - >>> await json.strlen(client, "doc", "nested.a") + >>> await glide_json.strlen(client, "doc", "nested.a") 5 # The length of the JSON value at path 'nested.a' in the key stored at `doc`. - >>> await json.strlen(client, "doc", "$") + >>> await glide_json.strlen(client, "doc", "$") [None] # Returns an array with None since the value at root path does in the JSON document stored at `doc` is not a string. - >>> await json.strlen(client, "non_existing_key", ".") + >>> await glide_json.strlen(client, "non_existing_key", ".") None # `key` doesn't exist. """ @@ -1181,15 +1191,15 @@ async def toggle( For more information about the returned type, see `TJsonResponse`. Examples: - >>> from glide import json - >>> import json as jsonpy - >>> await json.set(client, "doc", "$", jsonpy.dumps({"bool": True, "nested": {"bool": False, "nested": {"bool": 10}}})) + >>> from glide import glide_json + >>> import json + >>> await glide_json.set(client, "doc", "$", json.dumps({"bool": True, "nested": {"bool": False, "nested": {"bool": 10}}})) 'OK' - >>> await json.toggle(client, "doc", "$.bool") + >>> await glide_json.toggle(client, "doc", "$.bool") [False, True, None] # Indicates successful toggling of the Boolean values at path '$.bool' in the key stored at `doc`. - >>> await json.toggle(client, "doc", "bool") + >>> await glide_json.toggle(client, "doc", "bool") True # Indicates successful toggling of the Boolean value at path 'bool' in the key stored at `doc`. - >>> jsonpy.loads(await json.get(client, "doc", "$")) + >>> json.loads(await glide_json.get(client, "doc", "$")) [{"bool": True, "nested": {"bool": True, "nested": {"bool": 10}}}] # The updated JSON value in the key stored at `doc`. """ @@ -1222,16 +1232,17 @@ async def type( If multiple paths match, the type of the first JSON value match is returned. If `path` doesn't exist, None will be returned. If `key` doesn't exist, None is returned. + For more information about the returned type, see `TJsonUniversalResponse`. Examples: - >>> from glide import json - >>> await json.set(client, "doc", "$", '{"a": 1, "nested": {"a": 2, "b": 3}}') + >>> from glide import glide_json + >>> await glide_json.set(client, "doc", "$", '{"a": 1, "nested": {"a": 2, "b": 3}}') 'OK' - >>> await json.type(client, "doc", "$.nested") + >>> await glide_json.type(client, "doc", "$.nested") [b'object'] # Indicates the type of the value at path '$.nested' in the key stored at `doc`. - >>> await json.type(client, "doc", "$.nested.a") + >>> await glide_json.type(client, "doc", "$.nested.a") [b'integer'] # Indicates the type of the value at path '$.nested.a' in the key stored at `doc`. - >>> await json.type(client, "doc", "$[*]") + >>> await glide_json.type(client, "doc", "$[*]") [b'integer', b'object'] # Array of types in all top level elements. """ args = ["JSON.TYPE", key] diff --git a/python/python/tests/tests_server_modules/test_ft.py b/python/python/tests/tests_server_modules/test_ft.py index ebacb209f0..ee2b9416ee 100644 --- a/python/python/tests/tests_server_modules/test_ft.py +++ b/python/python/tests/tests_server_modules/test_ft.py @@ -8,7 +8,7 @@ import pytest from glide.async_commands.command_args import OrderBy from glide.async_commands.server_modules import ft -from glide.async_commands.server_modules import json as GlideJson +from glide.async_commands.server_modules import glide_json as GlideJson from glide.async_commands.server_modules.ft_options.ft_aggregate_options import ( FtAggregateApply, FtAggregateGroupBy, diff --git a/python/python/tests/tests_server_modules/test_json.py b/python/python/tests/tests_server_modules/test_json.py index eba62faa17..85657914de 100644 --- a/python/python/tests/tests_server_modules/test_json.py +++ b/python/python/tests/tests_server_modules/test_json.py @@ -7,8 +7,8 @@ import pytest from glide.async_commands.core import ConditionalChange, InfoSection -from glide.async_commands.server_modules import json -from glide.async_commands.server_modules.json import ( +from glide.async_commands.server_modules import glide_json as json +from glide.async_commands.server_modules.glide_json import ( JsonArrIndexOptions, JsonArrPopOptions, JsonGetOptions, From 9c26a5bdfc10d858abb9df84a765a3dbd4e94b61 Mon Sep 17 00:00:00 2001 From: Muhammad Awawdi Date: Wed, 20 Nov 2024 20:00:32 +0200 Subject: [PATCH 08/29] Node: add AZ Affinity ReadFrom strategy Support (#2686) * Added AZAffinity strategy to Node.js --------- Signed-off-by: Muhammad Awawdi --- node/src/BaseClient.ts | 14 + node/tests/GlideClient.test.ts | 24 +- node/tests/GlideClusterClient.test.ts | 460 +++++++++++++++++++++++++- 3 files changed, 482 insertions(+), 16 deletions(-) diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index 6ba8f975cf..7c2e3feb57 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -574,6 +574,18 @@ export interface BaseClientConfiguration { * used. */ inflightRequestsLimit?: number; + /** + * Availability Zone of the client. + * If ReadFrom strategy is AZAffinity, this setting ensures that readonly commands are directed to replicas within the specified AZ if exits. + * + * @example + * ```typescript + * // Example configuration for setting client availability zone and read strategy + * configuration.clientAz = 'us-east-1a'; // Sets the client's availability zone + * configuration.readFrom = 'AZAffinity'; // Directs read operations to nodes within the same AZ + * ``` + */ + clientAz?: string; } /** @@ -719,6 +731,7 @@ export class BaseClient { private readonly pubsubFutures: [PromiseFunction, ErrorFunction][] = []; private pendingPushNotification: response.Response[] = []; private readonly inflightRequestsLimit: number; + private readonly clientAz: string | undefined; private config: BaseClientConfiguration | undefined; protected configurePubsub( @@ -7578,6 +7591,7 @@ export class BaseClient { readFrom, authenticationInfo, inflightRequestsLimit: options.inflightRequestsLimit, + clientAz: options.clientAz ?? null, }; } diff --git a/node/tests/GlideClient.test.ts b/node/tests/GlideClient.test.ts index 49f056f2b0..0c77bde519 100644 --- a/node/tests/GlideClient.test.ts +++ b/node/tests/GlideClient.test.ts @@ -51,7 +51,9 @@ const TIMEOUT = 50000; describe("GlideClient", () => { let testsFailed = 0; let cluster: ValkeyCluster; + let azCluster: ValkeyCluster; let client: GlideClient; + let azClient: GlideClient; beforeAll(async () => { const standaloneAddresses = global.STAND_ALONE_ENDPOINT; cluster = standaloneAddresses @@ -61,17 +63,28 @@ describe("GlideClient", () => { getServerVersion, ) : await ValkeyCluster.createCluster(false, 1, 1, getServerVersion); + + azCluster = standaloneAddresses + ? await ValkeyCluster.initFromExistingCluster( + false, + parseEndpoints(standaloneAddresses), + getServerVersion, + ) + : await ValkeyCluster.createCluster(false, 1, 1, getServerVersion); }, 20000); afterEach(async () => { await flushAndCloseClient(false, cluster.getAddresses(), client); + await flushAndCloseClient(false, azCluster.getAddresses(), azClient); }); afterAll(async () => { if (testsFailed === 0) { await cluster.close(); + await azCluster.close(); } else { await cluster.close(true); + await azCluster.close(); } }, TIMEOUT); @@ -1500,7 +1513,6 @@ describe("GlideClient", () => { } }, ); - runBaseTests({ init: async (protocol, configOverrides) => { const config = getClientConfigurationOption( @@ -1508,10 +1520,18 @@ describe("GlideClient", () => { protocol, configOverrides, ); + client = await GlideClient.createClient(config); + + const configNew = getClientConfigurationOption( + azCluster.getAddresses(), + protocol, + configOverrides, + ); testsFailed += 1; + azClient = await GlideClient.createClient(configNew); client = await GlideClient.createClient(config); - return { client, cluster }; + return { client, cluster, azClient, azCluster }; }, close: (testSucceeded: boolean) => { if (testSucceeded) { diff --git a/node/tests/GlideClusterClient.test.ts b/node/tests/GlideClusterClient.test.ts index 0768b61088..79868c36a8 100644 --- a/node/tests/GlideClusterClient.test.ts +++ b/node/tests/GlideClusterClient.test.ts @@ -27,6 +27,7 @@ import { InfoOptions, ListDirection, ProtocolVersion, + ReadFrom, RequestError, Routes, ScoreFilter, @@ -61,45 +62,80 @@ const TIMEOUT = 50000; describe("GlideClusterClient", () => { let testsFailed = 0; let cluster: ValkeyCluster; + let azCluster: ValkeyCluster; let client: GlideClusterClient; + let azClient: GlideClusterClient; beforeAll(async () => { const clusterAddresses = global.CLUSTER_ENDPOINTS; - // Connect to cluster or create a new one based on the parsed addresses - cluster = clusterAddresses - ? await ValkeyCluster.initFromExistingCluster( - true, - parseEndpoints(clusterAddresses), - getServerVersion, - ) - : // setting replicaCount to 1 to facilitate tests routed to replicas - await ValkeyCluster.createCluster(true, 3, 1, getServerVersion); - }, 40000); + + if (clusterAddresses) { + // Initialize current cluster from existing addresses + cluster = await ValkeyCluster.initFromExistingCluster( + true, + parseEndpoints(clusterAddresses), + getServerVersion, + ); + + // Initialize cluster from existing addresses for AzAffinity test + azCluster = await ValkeyCluster.initFromExistingCluster( + true, + parseEndpoints(clusterAddresses), + getServerVersion, + ); + } else { + cluster = await ValkeyCluster.createCluster( + true, + 3, + 1, + getServerVersion, + ); + + azCluster = await ValkeyCluster.createCluster( + true, + 3, + 4, + getServerVersion, + ); + } + }, 120000); afterEach(async () => { await flushAndCloseClient(true, cluster.getAddresses(), client); + await flushAndCloseClient(true, azCluster.getAddresses(), azClient); }); afterAll(async () => { if (testsFailed === 0) { - await cluster.close(); + if (cluster) await cluster.close(); + if (azCluster) await azCluster.close(); } else { - await cluster.close(true); + if (cluster) await cluster.close(true); + if (azCluster) await azCluster.close(true); } }); runBaseTests({ init: async (protocol, configOverrides) => { - const config = getClientConfigurationOption( + const configCurrent = getClientConfigurationOption( cluster.getAddresses(), protocol, configOverrides, ); + client = await GlideClusterClient.createClient(configCurrent); + + const configNew = getClientConfigurationOption( + azCluster.getAddresses(), + protocol, + configOverrides, + ); + azClient = await GlideClusterClient.createClient(configNew); testsFailed += 1; - client = await GlideClusterClient.createClient(config); return { client, + azClient, cluster, + azCluster, }; }, close: (testSucceeded: boolean) => { @@ -1966,4 +2002,400 @@ describe("GlideClusterClient", () => { } }, ); + describe("GlideClusterClient - AZAffinity Read Strategy Test", () => { + async function getNumberOfReplicas( + azClient: GlideClusterClient, + ): Promise { + const replicationInfo = await azClient.customCommand([ + "INFO", + "REPLICATION", + ]); + + if (Array.isArray(replicationInfo)) { + // Handle array response from cluster (CME Mode) + let totalReplicas = 0; + + for (const node of replicationInfo) { + const nodeInfo = node as { + key: string; + value: string | string[] | null; + }; + + if (typeof nodeInfo.value === "string") { + const lines = nodeInfo.value.split(/\r?\n/); + const connectedReplicasLine = lines.find( + (line) => + line.startsWith("connected_slaves:") || + line.startsWith("connected_replicas:"), + ); + + if (connectedReplicasLine) { + const parts = connectedReplicasLine.split(":"); + const numReplicas = parseInt(parts[1], 10); + + if (!isNaN(numReplicas)) { + // Sum up replicas from each primary node + totalReplicas += numReplicas; + } + } + } + } + + if (totalReplicas > 0) { + return totalReplicas; + } + + throw new Error( + "Could not find replica information in any node's response", + ); + } + + throw new Error( + "Unexpected response format from INFO REPLICATION command", + ); + } + + it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])( + "should route GET commands to all replicas with the same AZ using protocol %p", + async (protocol) => { + const az = "us-east-1a"; + const GET_CALLS_PER_REPLICA = 3; + + let client_for_config_set; + let client_for_testing_az; + + try { + // Stage 1: Configure nodes + client_for_config_set = + await GlideClusterClient.createClient( + getClientConfigurationOption( + azCluster.getAddresses(), + protocol, + ), + ); + + // Skip test if version is below 8.0.0 + if (cluster.checkIfServerVersionLessThan("8.0.0")) { + console.log( + "Skipping test: requires Valkey 8.0.0 or higher", + ); + return; + } + + await client_for_config_set.customCommand([ + "CONFIG", + "RESETSTAT", + ]); + await client_for_config_set.customCommand( + ["CONFIG", "SET", "availability-zone", az], + { route: "allNodes" }, + ); + + // Retrieve the number of replicas dynamically + const n_replicas = await getNumberOfReplicas( + client_for_config_set, + ); + + if (n_replicas === 0) { + throw new Error( + "No replicas found in the cluster. Test requires at least one replica.", + ); + } + + const GET_CALLS = GET_CALLS_PER_REPLICA * n_replicas; + const get_cmdstat = `calls=${GET_CALLS_PER_REPLICA}`; + + // Stage 2: Create AZ affinity client and verify configuration + client_for_testing_az = + await GlideClusterClient.createClient( + getClientConfigurationOption( + azCluster.getAddresses(), + protocol, + { + readFrom: "AZAffinity" as ReadFrom, + clientAz: az, + }, + ), + ); + + const azs = await client_for_testing_az.customCommand( + ["CONFIG", "GET", "availability-zone"], + { route: "allNodes" }, + ); + + if (Array.isArray(azs)) { + const allAZsMatch = azs.every((node) => { + const nodeResponse = node as { + key: string; + value: string | number; + }; + + if (protocol === ProtocolVersion.RESP2) { + // RESP2: Direct array format ["availability-zone", "us-east-1a"] + return ( + Array.isArray(nodeResponse.value) && + nodeResponse.value[1] === az + ); + } else { + // RESP3: Nested object format [{ key: "availability-zone", value: "us-east-1a" }] + return ( + Array.isArray(nodeResponse.value) && + nodeResponse.value[0]?.key === + "availability-zone" && + nodeResponse.value[0]?.value === az + ); + } + }); + expect(allAZsMatch).toBe(true); + } else { + throw new Error( + "Unexpected response format from CONFIG GET command", + ); + } + + // Stage 3: Set test data and perform GET operations + await client_for_testing_az.set("foo", "testvalue"); + + for (let i = 0; i < GET_CALLS; i++) { + await client_for_testing_az.get("foo"); + } + + // Stage 4: Verify GET commands were routed correctly + const info_result = + await client_for_testing_az.customCommand( + ["INFO", "ALL"], // Get both replication and commandstats info + { route: "allNodes" }, + ); + + if (Array.isArray(info_result)) { + const matching_entries_count = info_result.filter( + (node) => { + const nodeInfo = node as { + key: string; + value: string | string[] | null; + }; + const infoStr = + nodeInfo.value?.toString() || ""; + + // Check if this is a replica node AND it has the expected number of GET calls + const isReplicaNode = + infoStr.includes("role:slave") || + infoStr.includes("role:replica"); + + return ( + isReplicaNode && + infoStr.includes(get_cmdstat) + ); + }, + ).length; + + expect(matching_entries_count).toBe(n_replicas); // Should expect 12 as the cluster was created with 3 primary and 4 replicas, totalling 12 replica nodes + } else { + throw new Error( + "Unexpected response format from INFO command", + ); + } + } finally { + // Cleanup + await client_for_config_set?.close(); + await client_for_testing_az?.close(); + } + }, + ); + }); + describe("GlideClusterClient - AZAffinity Routing to 1 replica", () => { + it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])( + "should route commands to single replica with AZ using protocol %p", + async (protocol) => { + const az = "us-east-1a"; + const GET_CALLS = 3; + const get_cmdstat = `calls=${GET_CALLS}`; + let client_for_config_set; + let client_for_testing_az; + + try { + // Stage 1: Configure nodes + client_for_config_set = + await GlideClusterClient.createClient( + getClientConfigurationOption( + azCluster.getAddresses(), + protocol, + ), + ); + + // Skip test if version is below 8.0.0 + if (cluster.checkIfServerVersionLessThan("8.0.0")) { + console.log( + "Skipping test: requires Valkey 8.0.0 or higher", + ); + return; + } + + await client_for_config_set.customCommand( + ["CONFIG", "SET", "availability-zone", ""], + { route: "allNodes" }, + ); + + await client_for_config_set.customCommand([ + "CONFIG", + "RESETSTAT", + ]); + + await client_for_config_set.customCommand( + ["CONFIG", "SET", "availability-zone", az], + { route: { type: "replicaSlotId", id: 12182 } }, + ); + + // Stage 2: Create AZ affinity client and verify configuration + client_for_testing_az = + await GlideClusterClient.createClient( + getClientConfigurationOption( + azCluster.getAddresses(), + protocol, + { + readFrom: "AZAffinity", + clientAz: az, + }, + ), + ); + await client_for_testing_az.set("foo", "testvalue"); + + for (let i = 0; i < GET_CALLS; i++) { + await client_for_testing_az.get("foo"); + } + + // Stage 4: Verify GET commands were routed correctly + const info_result = + await client_for_testing_az.customCommand( + ["INFO", "ALL"], + { route: "allNodes" }, + ); + + // Process the info_result to check that only one replica has the GET calls + if (Array.isArray(info_result)) { + // Count the number of nodes where both get_cmdstat and az are present + const matching_entries_count = info_result.filter( + (node) => { + const nodeInfo = node as { + key: string; + value: string | string[] | null; + }; + const infoStr = + nodeInfo.value?.toString() || ""; + return ( + infoStr.includes(get_cmdstat) && + infoStr.includes(`availability_zone:${az}`) + ); + }, + ).length; + + expect(matching_entries_count).toBe(1); + + // Check that only one node has the availability zone set to az + const changed_az_count = info_result.filter((node) => { + const nodeInfo = node as { + key: string; + value: string | string[] | null; + }; + const infoStr = nodeInfo.value?.toString() || ""; + return infoStr.includes(`availability_zone:${az}`); + }).length; + + expect(changed_az_count).toBe(1); + } else { + throw new Error( + "Unexpected response format from INFO command", + ); + } + } finally { + await client_for_config_set?.close(); + await client_for_testing_az?.close(); + } + }, + ); + }); + describe("GlideClusterClient - AZAffinity with Non-existing AZ", () => { + it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])( + "should route commands to a replica when AZ does not exist using protocol %p", + async (protocol) => { + const GET_CALLS = 4; + const replica_calls = 1; + const get_cmdstat = `cmdstat_get:calls=${replica_calls}`; + let client_for_testing_az; + + try { + // Skip test if server version is below 8.0.0 + if (azCluster.checkIfServerVersionLessThan("8.0.0")) { + console.log( + "Skipping test: requires Valkey 8.0.0 or higher", + ); + return; + } + + // Create a client configured for AZAffinity with a non-existing AZ + client_for_testing_az = + await GlideClusterClient.createClient( + getClientConfigurationOption( + azCluster.getAddresses(), + protocol, + { + readFrom: "AZAffinity", + clientAz: "non-existing-az", + requestTimeout: 2000, + }, + ), + ); + + // Reset command stats on all nodes + await client_for_testing_az.customCommand( + ["CONFIG", "RESETSTAT"], + { route: "allNodes" }, + ); + + // Issue GET commands + for (let i = 0; i < GET_CALLS; i++) { + await client_for_testing_az.get("foo"); + } + + // Fetch command stats from all nodes + const info_result = + await client_for_testing_az.customCommand( + ["INFO", "COMMANDSTATS"], + { route: "allNodes" }, + ); + + // Inline matching logic + let matchingEntriesCount = 0; + + if ( + typeof info_result === "object" && + info_result !== null + ) { + const nodeResponses = Object.values(info_result); + + for (const response of nodeResponses) { + if ( + response && + typeof response === "object" && + "value" in response && + response.value.includes(get_cmdstat) + ) { + matchingEntriesCount++; + } + } + } else { + throw new Error( + "Unexpected response format from INFO command", + ); + } + + // Validate that only one replica handled the GET calls + expect(matchingEntriesCount).toBe(4); + } finally { + // Cleanup: Close the client after test execution + await client_for_testing_az?.close(); + } + }, + ); + }); }); From d15769c0171728fbba1a03ced197f2693ceb674a Mon Sep 17 00:00:00 2001 From: adarovadya Date: Wed, 20 Nov 2024 22:49:00 +0200 Subject: [PATCH 09/29] Python: add AZAffinity ReadFrom strategy support (#2723) * Python: add AZAffinity ReadFrom strategy --------- Signed-off-by: BoazBD Signed-off-by: Adar Ovadia Co-authored-by: BoazBD Co-authored-by: Adar Ovadia --- CHANGELOG.md | 1 + python/python/glide/config.py | 18 ++ python/python/tests/conftest.py | 31 +++ python/python/tests/test_config.py | 15 ++ .../python/tests/test_read_from_strategy.py | 226 ++++++++++++++++++ python/python/tests/utils/cluster.py | 2 +- 6 files changed, 292 insertions(+), 1 deletion(-) create mode 100644 python/python/tests/test_read_from_strategy.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 228aa7bdf2..29190e8f2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ #### Changes * Python: Client API for retrieving internal statistics ([#2707](https://github.com/valkey-io/valkey-glide/pull/2707)) +* Python: AZ Affinity - Python Wrapper Support ([#2676](https://github.com/valkey-io/valkey-glide/pull/2676)) * Node, Python: Adding support for replacing connection configured password ([#2651](https://github.com/valkey-io/valkey-glide/pull/2651))([#2659](https://github.com/valkey-io/valkey-glide/pull/2659)) * Node: Add FT._ALIASLIST command([#2652](https://github.com/valkey-io/valkey-glide/pull/2652)) * Python: Python: `FT._ALIASLIST` command added([#2638](https://github.com/valkey-io/valkey-glide/pull/2638)) diff --git a/python/python/glide/config.py b/python/python/glide/config.py index db85202876..b33c037cbf 100644 --- a/python/python/glide/config.py +++ b/python/python/glide/config.py @@ -41,6 +41,11 @@ class ReadFrom(Enum): Spread the requests between all replicas in a round robin manner. If no replica is available, route the requests to the primary. """ + AZ_AFFINITY = ProtobufReadFrom.AZAffinity + """ + Spread the read requests between replicas in the same client's AZ (Aviliablity zone) in a round robin manner, + falling back to other replicas or the primary if needed + """ class ProtocolVersion(Enum): @@ -135,6 +140,7 @@ def __init__( client_name: Optional[str] = None, protocol: ProtocolVersion = ProtocolVersion.RESP3, inflight_requests_limit: Optional[int] = None, + client_az: Optional[str] = None, ): """ Represents the configuration settings for a Glide client. @@ -172,6 +178,12 @@ def __init__( self.client_name = client_name self.protocol = protocol self.inflight_requests_limit = inflight_requests_limit + self.client_az = client_az + + if read_from == ReadFrom.AZ_AFFINITY and not client_az: + raise ValueError( + "client_az mus t be set when read_from is set to AZ_AFFINITY" + ) def _create_a_protobuf_conn_request( self, cluster_mode: bool = False @@ -204,6 +216,8 @@ def _create_a_protobuf_conn_request( request.protocol = self.protocol.value if self.inflight_requests_limit: request.inflight_requests_limit = self.inflight_requests_limit + if self.client_az: + request.client_az = self.client_az return request @@ -293,6 +307,7 @@ def __init__( protocol: ProtocolVersion = ProtocolVersion.RESP3, pubsub_subscriptions: Optional[PubSubSubscriptions] = None, inflight_requests_limit: Optional[int] = None, + client_az: Optional[str] = None, ): super().__init__( addresses=addresses, @@ -303,6 +318,7 @@ def __init__( client_name=client_name, protocol=protocol, inflight_requests_limit=inflight_requests_limit, + client_az=client_az, ) self.reconnect_strategy = reconnect_strategy self.database_id = database_id @@ -442,6 +458,7 @@ def __init__( ] = PeriodicChecksStatus.ENABLED_DEFAULT_CONFIGS, pubsub_subscriptions: Optional[PubSubSubscriptions] = None, inflight_requests_limit: Optional[int] = None, + client_az: Optional[str] = None, ): super().__init__( addresses=addresses, @@ -452,6 +469,7 @@ def __init__( client_name=client_name, protocol=protocol, inflight_requests_limit=inflight_requests_limit, + client_az=client_az, ) self.periodic_checks = periodic_checks self.pubsub_subscriptions = pubsub_subscriptions diff --git a/python/python/tests/conftest.py b/python/python/tests/conftest.py index 437fbd8fbb..f733361107 100644 --- a/python/python/tests/conftest.py +++ b/python/python/tests/conftest.py @@ -9,6 +9,7 @@ GlideClusterClientConfiguration, NodeAddress, ProtocolVersion, + ReadFrom, ServerCredentials, ) from glide.exceptions import ClosingError, RequestError @@ -132,6 +133,7 @@ def create_clusters(tls, load_module, cluster_endpoints, standalone_endpoints): cluster_mode=True, load_module=load_module, addresses=cluster_endpoints, + replica_count=2, ) pytest.standalone_cluster = ValkeyCluster( tls=tls, @@ -248,6 +250,8 @@ async def create_client( GlideClientConfiguration.PubSubSubscriptions ] = None, inflight_requests_limit: Optional[int] = None, + read_from: ReadFrom = ReadFrom.PRIMARY, + client_az: Optional[str] = None, ) -> Union[GlideClient, GlideClusterClient]: # Create async socket client use_tls = request.config.getoption("--tls") @@ -265,6 +269,8 @@ async def create_client( request_timeout=timeout, pubsub_subscriptions=cluster_mode_pubsub, inflight_requests_limit=inflight_requests_limit, + read_from=read_from, + client_az=client_az, ) return await GlideClusterClient.create(cluster_config) else: @@ -281,6 +287,8 @@ async def create_client( request_timeout=timeout, pubsub_subscriptions=standalone_mode_pubsub, inflight_requests_limit=inflight_requests_limit, + read_from=read_from, + client_az=client_az, ) return await GlideClient.create(config) @@ -381,3 +389,26 @@ async def test_meow_meow(...): reason=f"This feature added in version {min_version}", allow_module_level=True, ) + + +# @pytest.fixture(scope="module") +# def multiple_replicas_cluster(request): +# """ +# Fixture to create a special cluster with 4 replicas for specific tests. +# """ +# tls = request.config.getoption("--tls") +# load_module = request.config.getoption("--load-module") +# cluster_endpoints = request.config.getoption("--cluster-endpoints") + +# if not cluster_endpoints: +# multiple_replica_cluster = ValkeyCluster( +# tls=tls, +# cluster_mode=True, +# load_module=load_module, +# addresses=cluster_endpoints, +# replica_count=4, +# ) +# yield multiple_replica_cluster +# multiple_replica_cluster.__del__() +# else: +# yield None diff --git a/python/python/tests/test_config.py b/python/python/tests/test_config.py index 93c280245f..3b22adb09c 100644 --- a/python/python/tests/test_config.py +++ b/python/python/tests/test_config.py @@ -52,3 +52,18 @@ def test_periodic_checks_interval_to_protobuf(): config.periodic_checks = PeriodicChecksManualInterval(30) request = config._create_a_protobuf_conn_request(cluster_mode=True) assert request.periodic_checks_manual_interval.duration_in_sec == 30 + + +def test_convert_config_with_azaffinity_to_protobuf(): + az = "us-east-1a" + config = BaseClientConfiguration( + [NodeAddress("127.0.0.1")], + use_tls=True, + read_from=ReadFrom.AZ_AFFINITY, + client_az=az, + ) + request = config._create_a_protobuf_conn_request() + assert isinstance(request, ConnectionRequest) + assert request.tls_mode is TlsMode.SecureTls + assert request.read_from == ProtobufReadFrom.AZAffinity + assert request.client_az == az diff --git a/python/python/tests/test_read_from_strategy.py b/python/python/tests/test_read_from_strategy.py new file mode 100644 index 0000000000..429234ec82 --- /dev/null +++ b/python/python/tests/test_read_from_strategy.py @@ -0,0 +1,226 @@ +# Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 + +import re + +import pytest +from glide.async_commands.core import InfoSection +from glide.config import ProtocolVersion, ReadFrom +from glide.constants import OK +from glide.glide_client import GlideClusterClient +from glide.routes import AllNodes, SlotIdRoute, SlotType +from tests.conftest import create_client +from tests.utils.utils import get_first_result + + +@pytest.mark.asyncio +# @pytest.mark.usefixtures("multiple_replicas_cluster") +class TestAZAffinity: + async def _get_num_replicas(self, client: GlideClusterClient) -> int: + info_replicas = get_first_result( + await client.info([InfoSection.REPLICATION]) + ).decode() + match = re.search(r"connected_slaves:(\d+)", info_replicas) + if match: + return int(match.group(1)) + else: + raise ValueError( + "Could not find the number of replicas in the INFO REPLICATION response" + ) + + @pytest.mark.skip_if_version_below("8.0.0") + @pytest.mark.parametrize("cluster_mode", [True]) + @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) + async def test_routing_with_az_affinity_strategy_to_1_replica( + self, + request, + cluster_mode: bool, + protocol: ProtocolVersion, + # multiple_replicas_cluster, + ): + """Test that the client with az affinity strategy will only route to the 1 replica with the same az""" + az = "us-east-1a" + GET_CALLS = 3 + get_cmdstat = f"cmdstat_get:calls={GET_CALLS}" + + client_for_config_set = await create_client( + request, + cluster_mode, + # addresses=multiple_replicas_cluster.nodes_addr, + protocol=protocol, + timeout=2000, + ) + + # Reset the availability zone for all nodes + await client_for_config_set.custom_command( + ["CONFIG", "SET", "availability-zone", ""], + route=AllNodes(), + ) + assert await client_for_config_set.config_resetstat() == OK + + # 12182 is the slot of "foo" + await client_for_config_set.custom_command( + ["CONFIG", "SET", "availability-zone", az], + route=SlotIdRoute(SlotType.REPLICA, 12182), + ) + + client_for_testing_az = await create_client( + request, + cluster_mode, + # addresses=multiple_replicas_cluster.nodes_addr, + protocol=protocol, + read_from=ReadFrom.AZ_AFFINITY, + timeout=2000, + client_az=az, + ) + + for _ in range(GET_CALLS): + await client_for_testing_az.get("foo") + + info_result = await client_for_testing_az.info( + [InfoSection.SERVER, InfoSection.COMMAND_STATS], AllNodes() + ) + + # Check that only the replica with az has all the GET calls + matching_entries_count = sum( + 1 + for value in info_result.values() + if get_cmdstat in value.decode() and az in value.decode() + ) + assert matching_entries_count == 1 + + # Check that the other replicas have no availability zone set + changed_az_count = sum( + 1 + for node in info_result.values() + if f"availability_zone:{az}" in node.decode() + ) + assert changed_az_count == 1 + await client_for_testing_az.close() + await client_for_config_set.close() + + @pytest.mark.skip_if_version_below("8.0.0") + @pytest.mark.parametrize("cluster_mode", [True]) + @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) + async def test_routing_by_slot_to_replica_with_az_affinity_strategy_to_all_replicas( + self, + request, + cluster_mode: bool, + protocol: ProtocolVersion, + # multiple_replicas_cluster, + ): + """Test that the client with AZ affinity strategy routes in a round-robin manner to all replicas within the specified AZ""" + + az = "us-east-1a" + client_for_config_set = await create_client( + request, + cluster_mode, + # addresses=multiple_replicas_cluster.nodes_addr, + protocol=protocol, + timeout=2000, + ) + assert await client_for_config_set.config_resetstat() == OK + await client_for_config_set.custom_command( + ["CONFIG", "SET", "availability-zone", az], AllNodes() + ) + + client_for_testing_az = await create_client( + request, + cluster_mode, + # addresses=multiple_replicas_cluster.nodes_addr, + protocol=protocol, + read_from=ReadFrom.AZ_AFFINITY, + timeout=2000, + client_az=az, + ) + azs = await client_for_testing_az.custom_command( + ["CONFIG", "GET", "availability-zone"], AllNodes() + ) + + # Check that all replicas have the availability zone set to the az + assert all( + ( + node[1].decode() == az + if isinstance(node, list) + else node[b"availability-zone"].decode() == az + ) + for node in azs.values() + ) + + n_replicas = await self._get_num_replicas(client_for_testing_az) + GET_CALLS = 4 * n_replicas + get_cmdstat = f"cmdstat_get:calls={GET_CALLS // n_replicas}" + + for _ in range(GET_CALLS): + await client_for_testing_az.get("foo") + + info_result = await client_for_testing_az.info( + [InfoSection.COMMAND_STATS, InfoSection.SERVER], AllNodes() + ) + + # Check that all replicas have the same number of GET calls + matching_entries_count = sum( + 1 + for value in info_result.values() + if get_cmdstat in value.decode() and az in value.decode() + ) + assert matching_entries_count == n_replicas + + await client_for_config_set.close() + await client_for_testing_az.close() + + @pytest.mark.skip_if_version_below("8.0.0") + @pytest.mark.parametrize("cluster_mode", [True]) + @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) + async def test_az_affinity_non_existing_az( + self, + request, + cluster_mode: bool, + protocol: ProtocolVersion, + # multiple_replicas_cluster, + ): + GET_CALLS = 4 + + client_for_testing_az = await create_client( + request, + cluster_mode, + # addresses=multiple_replicas_cluster.nodes_addr, + protocol=protocol, + read_from=ReadFrom.AZ_AFFINITY, + timeout=2000, + client_az="non-existing-az", + ) + assert await client_for_testing_az.config_resetstat() == OK + + for _ in range(GET_CALLS): + await client_for_testing_az.get("foo") + + n_replicas = await self._get_num_replicas(client_for_testing_az) + # We expect the calls to be distributed evenly among the replicas + get_cmdstat = f"cmdstat_get:calls={GET_CALLS // n_replicas}" + + info_result = await client_for_testing_az.info( + [InfoSection.COMMAND_STATS, InfoSection.SERVER], AllNodes() + ) + + matching_entries_count = sum( + 1 for value in info_result.values() if get_cmdstat in value.decode() + ) + assert matching_entries_count == n_replicas + + await client_for_testing_az.close() + + @pytest.mark.skip_if_version_below("8.0.0") + @pytest.mark.parametrize("cluster_mode", [True]) + @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) + async def test_az_affinity_requires_client_az( + self, request, cluster_mode: bool, protocol: ProtocolVersion + ): + """Test that setting read_from to AZ_AFFINITY without client_az raises an error.""" + with pytest.raises(ValueError): + await create_client( + request, + cluster_mode=cluster_mode, + protocol=protocol, + read_from=ReadFrom.AZ_AFFINITY, + timeout=2000, + ) diff --git a/python/python/tests/utils/cluster.py b/python/python/tests/utils/cluster.py index e0bfb231ae..fa17742e7b 100644 --- a/python/python/tests/utils/cluster.py +++ b/python/python/tests/utils/cluster.py @@ -45,7 +45,7 @@ def __init__( stderr=subprocess.PIPE, text=True, ) - output, err = p.communicate(timeout=40) + output, err = p.communicate(timeout=80) if p.returncode != 0: raise Exception(f"Failed to create a cluster. Executed: {p}:\n{err}") self.parse_cluster_script_start_output(output) From 07e55c69be0ad5383956c83987bc516f244dabfc Mon Sep 17 00:00:00 2001 From: tjzhang-BQ <111323543+tjzhang-BQ@users.noreply.github.com> Date: Wed, 20 Nov 2024 15:00:33 -0800 Subject: [PATCH 10/29] Java: AZ Awareness (#2678) * Java: AZ awareness Signed-off-by: TJ Zhang --- .../BaseClientConfiguration.java | 6 + .../api/models/configuration/ReadFrom.java | 7 +- .../glide/managers/ConnectionManager.java | 20 +- .../glide/managers/ConnectionManagerTest.java | 55 ++++++ java/integTest/build.gradle | 22 +++ .../src/test/java/glide/ConnectionTests.java | 176 ++++++++++++++++++ .../test/java/glide/TestConfiguration.java | 2 + .../src/test/java/glide/TestUtilities.java | 12 ++ 8 files changed, 295 insertions(+), 5 deletions(-) diff --git a/java/client/src/main/java/glide/api/models/configuration/BaseClientConfiguration.java b/java/client/src/main/java/glide/api/models/configuration/BaseClientConfiguration.java index e0a4ed5500..7d9d5d5b68 100644 --- a/java/client/src/main/java/glide/api/models/configuration/BaseClientConfiguration.java +++ b/java/client/src/main/java/glide/api/models/configuration/BaseClientConfiguration.java @@ -74,4 +74,10 @@ public abstract class BaseClientConfiguration { * used. */ private final Integer inflightRequestsLimit; + + /** + * Availability Zone of the client. If ReadFrom strategy is AZAffinity, this setting ensures that + * readonly commands are directed to replicas within the specified AZ if exits. + */ + private final String clientAZ; } diff --git a/java/client/src/main/java/glide/api/models/configuration/ReadFrom.java b/java/client/src/main/java/glide/api/models/configuration/ReadFrom.java index 2d80ae7b60..29a212d8c7 100644 --- a/java/client/src/main/java/glide/api/models/configuration/ReadFrom.java +++ b/java/client/src/main/java/glide/api/models/configuration/ReadFrom.java @@ -9,5 +9,10 @@ public enum ReadFrom { * Spread the requests between all replicas in a round-robin manner. If no replica is available, * route the requests to the primary. */ - PREFER_REPLICA + PREFER_REPLICA, + /** + * Spread the read requests between replicas in the same client's AZ (Aviliablity zone) in a + * round-robin manner, falling back to other replicas or the primary if needed. + */ + AZ_AFFINITY, } diff --git a/java/client/src/main/java/glide/managers/ConnectionManager.java b/java/client/src/main/java/glide/managers/ConnectionManager.java index a5a8b9c5c3..99b383a9ed 100644 --- a/java/client/src/main/java/glide/managers/ConnectionManager.java +++ b/java/client/src/main/java/glide/managers/ConnectionManager.java @@ -14,6 +14,7 @@ import glide.api.models.configuration.NodeAddress; import glide.api.models.configuration.ReadFrom; import glide.api.models.exceptions.ClosingException; +import glide.api.models.exceptions.ConfigurationError; import glide.connectors.handlers.ChannelHandler; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Future; @@ -122,6 +123,14 @@ private ConnectionRequest.Builder setupConnectionRequestBuilderBaseConfiguration connectionRequestBuilder.setInflightRequestsLimit(configuration.getInflightRequestsLimit()); } + if (configuration.getReadFrom() == ReadFrom.AZ_AFFINITY) { + if (configuration.getClientAZ() == null) { + throw new ConfigurationError( + "`clientAZ` must be set when read_from is set to `AZ_AFFINITY`"); + } + connectionRequestBuilder.setClientAz(configuration.getClientAZ()); + } + return connectionRequestBuilder; } @@ -200,11 +209,14 @@ private ConnectionRequest.Builder setupConnectionRequestBuilderGlideClusterClien * @return Protobuf defined ReadFrom enum */ private ConnectionRequestOuterClass.ReadFrom mapReadFromEnum(ReadFrom readFrom) { - if (readFrom == ReadFrom.PREFER_REPLICA) { - return ConnectionRequestOuterClass.ReadFrom.PreferReplica; + switch (readFrom) { + case PREFER_REPLICA: + return ConnectionRequestOuterClass.ReadFrom.PreferReplica; + case AZ_AFFINITY: + return ConnectionRequestOuterClass.ReadFrom.AZAffinity; + default: + return ConnectionRequestOuterClass.ReadFrom.Primary; } - - return ConnectionRequestOuterClass.ReadFrom.Primary; } /** Check a response received from Glide. */ diff --git a/java/client/src/test/java/glide/managers/ConnectionManagerTest.java b/java/client/src/test/java/glide/managers/ConnectionManagerTest.java index 7a8f1a0d44..80e9783c88 100644 --- a/java/client/src/test/java/glide/managers/ConnectionManagerTest.java +++ b/java/client/src/test/java/glide/managers/ConnectionManagerTest.java @@ -32,6 +32,7 @@ import glide.api.models.configuration.ServerCredentials; import glide.api.models.configuration.StandaloneSubscriptionConfiguration; import glide.api.models.exceptions.ClosingException; +import glide.api.models.exceptions.ConfigurationError; import glide.connectors.handlers.ChannelHandler; import io.netty.channel.ChannelFuture; import java.util.Map; @@ -268,4 +269,58 @@ public void connection_on_resp_pointer_throws_ClosingException() { assertEquals("Unexpected data in response", executionException.getCause().getMessage()); verify(channel).close(); } + + @SneakyThrows + @Test + public void test_convert_config_with_azaffinity_to_protobuf() { + // setup + String az = "us-east-1a"; + GlideClientConfiguration config = + GlideClientConfiguration.builder() + .address(NodeAddress.builder().host(DEFAULT_HOST).port(DEFAULT_PORT).build()) + .useTLS(true) + .readFrom(ReadFrom.AZ_AFFINITY) + .clientAZ(az) + .build(); + + ConnectionRequest request = + ConnectionRequest.newBuilder() + .addAddresses( + ConnectionRequestOuterClass.NodeAddress.newBuilder() + .setHost(DEFAULT_HOST) + .setPort(DEFAULT_PORT) + .build()) + .setTlsMode(TlsMode.SecureTls) + .setReadFrom(ConnectionRequestOuterClass.ReadFrom.AZAffinity) + .setClientAz(az) + .build(); + + CompletableFuture completedFuture = new CompletableFuture<>(); + Response response = Response.newBuilder().setConstantResponse(ConstantResponse.OK).build(); + completedFuture.complete(response); + + // execute + when(channel.connect(eq(request))).thenReturn(completedFuture); + CompletableFuture result = connectionManager.connectToValkey(config); + + // verify + assertNull(result.get()); + verify(channel).connect(eq(request)); + } + + @SneakyThrows + @Test + public void test_az_affinity_without_client_az_throws_ConfigurationError() { + // setup + String az = "us-east-1a"; + GlideClientConfiguration config = + GlideClientConfiguration.builder() + .address(NodeAddress.builder().host(DEFAULT_HOST).port(DEFAULT_PORT).build()) + .useTLS(true) + .readFrom(ReadFrom.AZ_AFFINITY) + .build(); + + // verify + assertThrows(ConfigurationError.class, () -> connectionManager.connectToValkey(config)); + } } diff --git a/java/integTest/build.gradle b/java/integTest/build.gradle index 93cea92401..918d59ea23 100644 --- a/java/integTest/build.gradle +++ b/java/integTest/build.gradle @@ -33,6 +33,7 @@ dependencies { def standaloneHosts = '' def clusterHosts = '' +def azClusterHosts = '' ext { extractAddressesFromClusterManagerOutput = { String output -> @@ -84,6 +85,25 @@ tasks.register('startCluster') { } } +tasks.register('startClusterForAz') { + doLast { + if (System.getProperty("cluster-endpoints") == null) { + new ByteArrayOutputStream().withStream { os -> + exec { + workingDir "${project.rootDir}/../utils" + def args = ['python3', 'cluster_manager.py', 'start', '--cluster-mode', '-r', '4'] + if (System.getProperty("tls") == 'true') args.add(2, '--tls') + commandLine args + standardOutput = os + } + azClusterHosts = extractAddressesFromClusterManagerOutput(os.toString()) + } + } else { + azClusterHosts = System.getProperty("cluster-endpoints") + } + } +} + tasks.register('startStandalone') { doLast { if (System.getProperty("standalone-endpoints") == null) { @@ -107,6 +127,7 @@ test.dependsOn 'stopAllBeforeTests' stopAllBeforeTests.finalizedBy 'clearDirs' clearDirs.finalizedBy 'startStandalone' clearDirs.finalizedBy 'startCluster' +clearDirs.finalizedBy 'startClusterForAz' test.finalizedBy 'stopAllAfterTests' test.dependsOn ':client:buildRustRelease' @@ -114,6 +135,7 @@ tasks.withType(Test) { doFirst { systemProperty 'test.server.standalone', standaloneHosts systemProperty 'test.server.cluster', clusterHosts + systemProperty 'test.server.azcluster', azClusterHosts systemProperty 'test.server.tls', System.getProperty("tls") } diff --git a/java/integTest/src/test/java/glide/ConnectionTests.java b/java/integTest/src/test/java/glide/ConnectionTests.java index de17f54e1c..2aec2e4e6b 100644 --- a/java/integTest/src/test/java/glide/ConnectionTests.java +++ b/java/integTest/src/test/java/glide/ConnectionTests.java @@ -1,11 +1,25 @@ /** Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 */ package glide; +import static glide.TestConfiguration.SERVER_VERSION; +import static glide.TestUtilities.azClusterClientConfig; import static glide.TestUtilities.commonClientConfig; import static glide.TestUtilities.commonClusterClientConfig; +import static glide.api.BaseClient.OK; +import static glide.api.models.configuration.RequestRoutingConfiguration.SimpleMultiNodeRoute.ALL_NODES; +import static glide.api.models.configuration.RequestRoutingConfiguration.SlotType.PRIMARY; +import static glide.api.models.configuration.RequestRoutingConfiguration.SlotType.REPLICA; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import glide.api.GlideClient; import glide.api.GlideClusterClient; +import glide.api.models.ClusterValue; +import glide.api.models.commands.InfoOptions; +import glide.api.models.configuration.ReadFrom; +import glide.api.models.configuration.RequestRoutingConfiguration; +import java.util.Map; +import java.util.stream.Stream; import lombok.SneakyThrows; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; @@ -26,4 +40,166 @@ public void cluster_client() { var clusterClient = GlideClusterClient.createClient(commonClusterClientConfig().build()).get(); clusterClient.close(); } + + @SneakyThrows + public GlideClusterClient createAzTestClient(String az) { + return GlideClusterClient.createClient( + azClusterClientConfig() + .readFrom(ReadFrom.AZ_AFFINITY) + .clientAZ(az) + .requestTimeout(2000) + .build()) + .get(); + } + + /** + * Test that the client with AZ affinity strategy routes in a round-robin manner to all replicas + * within the specified AZ. + */ + @SneakyThrows + @Test + public void test_routing_by_slot_to_replica_with_az_affinity_strategy_to_all_replicas() { + assumeTrue(SERVER_VERSION.isGreaterThanOrEqualTo("8.0.0"), "Skip for versions below 8"); + + String az = "us-east-1a"; + + // Create client for setting the configs + GlideClusterClient configSetClient = + GlideClusterClient.createClient(azClusterClientConfig().requestTimeout(2000).build()).get(); + assertEquals(configSetClient.configResetStat().get(), OK); + + // Get Replica Count for current cluster + var clusterInfo = + configSetClient + .customCommand( + new String[] {"INFO", "REPLICATION"}, + new RequestRoutingConfiguration.SlotKeyRoute("key", PRIMARY)) + .get(); + long nReplicas = + Long.parseLong( + Stream.of(((String) clusterInfo.getSingleValue()).split("\\R")) + .map(line -> line.split(":", 2)) + .filter(parts -> parts.length == 2 && parts[0].trim().equals("connected_slaves")) + .map(parts -> parts[1].trim()) + .findFirst() + .get()); + long nGetCalls = 3 * nReplicas; + String getCmdstat = String.format("cmdstat_get:calls=%d", 3); + + // Setting AZ for all Nodes + configSetClient.configSet(Map.of("availability-zone", az), ALL_NODES).get(); + configSetClient.close(); + + // Creating Client with AZ configuration for testing + GlideClusterClient azTestClient = createAzTestClient(az); + ClusterValue> azGetResult = + azTestClient.configGet(new String[] {"availability-zone"}, ALL_NODES).get(); + Map> azData = azGetResult.getMultiValue(); + + // Check that all replicas have the availability zone set to the az + for (var entry : azData.entrySet()) { + assertEquals(az, entry.getValue().get("availability-zone")); + } + + // execute GET commands + for (int i = 0; i < nGetCalls; i++) { + azTestClient.get("foo").get(); + } + + ClusterValue infoResult = + azTestClient.info(new InfoOptions.Section[] {InfoOptions.Section.ALL}, ALL_NODES).get(); + Map infoData = infoResult.getMultiValue(); + + // Check that all replicas have the same number of GET calls + long matchingEntries = + infoData.values().stream() + .filter(value -> value.contains(getCmdstat) && value.contains(az)) + .count(); + assertEquals(nReplicas, matchingEntries); + azTestClient.close(); + } + + /** + * Test that the client with az affinity strategy will only route to the 1 replica with the same + * az. + */ + @SneakyThrows + @Test + public void test_routing_with_az_affinity_strategy_to_1_replica() { + assumeTrue(SERVER_VERSION.isGreaterThanOrEqualTo("8.0.0"), "Skip for versions below 8"); + + String az = "us-east-1a"; + int nGetCalls = 3; + String getCmdstat = String.format("cmdstat_get:calls=%d", nGetCalls); + + GlideClusterClient configSetClient = + GlideClusterClient.createClient(azClusterClientConfig().requestTimeout(2000).build()).get(); + + // reset availability zone for all nodes + configSetClient.configSet(Map.of("availability-zone", ""), ALL_NODES).get(); + assertEquals(configSetClient.configResetStat().get(), OK); + + Long fooSlotKey = + (Long) + configSetClient + .customCommand(new String[] {"CLUSTER", "KEYSLOT", "foo"}) + .get() + .getSingleValue(); + int convertedKey = Integer.parseInt(fooSlotKey.toString()); + configSetClient + .configSet( + Map.of("availability-zone", az), + new RequestRoutingConfiguration.SlotIdRoute(convertedKey, REPLICA)) + .get(); + configSetClient.close(); + + GlideClusterClient azTestClient = createAzTestClient(az); + + // execute GET commands + for (int i = 0; i < nGetCalls; i++) { + azTestClient.get("foo").get(); + } + + ClusterValue infoResult = + azTestClient.info(new InfoOptions.Section[] {InfoOptions.Section.ALL}, ALL_NODES).get(); + Map infoData = infoResult.getMultiValue(); + + // Check that all replicas have the same number of GET calls + long matchingEntries = + infoData.values().stream() + .filter(value -> value.contains(getCmdstat) && value.contains(az)) + .count(); + assertEquals(1, matchingEntries); + azTestClient.close(); + } + + @SneakyThrows + @Test + public void test_az_affinity_non_existing_az() { + assumeTrue(SERVER_VERSION.isGreaterThanOrEqualTo("8.0.0"), "Skip for versions below 8"); + + int nGetCalls = 4; + int nReplicaCalls = 1; + String getCmdstat = String.format("cmdstat_get:calls=%d", nReplicaCalls); + + GlideClusterClient azTestClient = createAzTestClient("non-existing-az"); + assertEquals(azTestClient.configResetStat(ALL_NODES).get(), OK); + + // execute GET commands + for (int i = 0; i < nGetCalls; i++) { + azTestClient.get("foo").get(); + } + + ClusterValue infoResult = + azTestClient + .info(new InfoOptions.Section[] {InfoOptions.Section.COMMANDSTATS}, ALL_NODES) + .get(); + Map infoData = infoResult.getMultiValue(); + + // We expect the calls to be distributed evenly among the replicas + long matchingEntries = + infoData.values().stream().filter(value -> value.contains(getCmdstat)).count(); + assertEquals(4, matchingEntries); + azTestClient.close(); + } } diff --git a/java/integTest/src/test/java/glide/TestConfiguration.java b/java/integTest/src/test/java/glide/TestConfiguration.java index 864e384e1d..812c06c301 100644 --- a/java/integTest/src/test/java/glide/TestConfiguration.java +++ b/java/integTest/src/test/java/glide/TestConfiguration.java @@ -16,6 +16,8 @@ public final class TestConfiguration { System.getProperty("test.server.standalone", "").split(","); public static final String[] CLUSTER_HOSTS = System.getProperty("test.server.cluster", "").split(","); + public static final String[] AZ_CLUSTER_HOSTS = + System.getProperty("test.server.azcluster", "").split(","); public static final Semver SERVER_VERSION; public static final boolean TLS = Boolean.parseBoolean(System.getProperty("test.server.tls", "")); diff --git a/java/integTest/src/test/java/glide/TestUtilities.java b/java/integTest/src/test/java/glide/TestUtilities.java index 2535fcaa8e..6c1efc75da 100644 --- a/java/integTest/src/test/java/glide/TestUtilities.java +++ b/java/integTest/src/test/java/glide/TestUtilities.java @@ -1,6 +1,7 @@ /** Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 */ package glide; +import static glide.TestConfiguration.AZ_CLUSTER_HOSTS; import static glide.TestConfiguration.CLUSTER_HOSTS; import static glide.TestConfiguration.STANDALONE_HOSTS; import static glide.TestConfiguration.TLS; @@ -111,6 +112,17 @@ public static Map parseInfoResponseToMap(String serverInfo) { return builder.useTLS(TLS); } + public static GlideClusterClientConfiguration.GlideClusterClientConfigurationBuilder + azClusterClientConfig() { + var builder = GlideClusterClientConfiguration.builder(); + for (var host : AZ_CLUSTER_HOSTS) { + var parts = host.split(":"); + builder.address( + NodeAddress.builder().host(parts[0]).port(Integer.parseInt(parts[1])).build()); + } + return builder.useTLS(TLS); + } + /** * Deep traverse and compare two objects, including comparing content of all nested collections * recursively. Floating point numbers comparison performed with 1e-6 delta. From e19e500c7164872db7ce00571bd8b936e0143858 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 20 Nov 2024 17:22:24 -0800 Subject: [PATCH 11/29] Java: Add password update API (#2677) * Add password update API Signed-off-by: Yury-Fridlyand Signed-off-by: Andrew Carbonetto Co-authored-by: Andrew Carbonetto Co-authored-by: prateek-kumar-improving --- CHANGELOG.md | 2 +- .../src/main/java/glide/api/BaseClient.java | 60 +++++++++ .../java/glide/managers/CommandManager.java | 21 +++ .../test/java/glide/SharedClientTests.java | 14 +- .../glide/cluster/ClusterClientTests.java | 118 ++++++++++++++++ .../standalone/StandaloneClientTests.java | 127 +++++++++++++++++- 6 files changed, 333 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29190e8f2d..92d19ac7b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ #### Changes * Python: Client API for retrieving internal statistics ([#2707](https://github.com/valkey-io/valkey-glide/pull/2707)) +* Node, Python, Java: Adding support for replacing connection configured password ([#2651](https://github.com/valkey-io/valkey-glide/pull/2651), [#2659](https://github.com/valkey-io/valkey-glide/pull/2659), [#2677](https://github.com/valkey-io/valkey-glide/pull/2677)) * Python: AZ Affinity - Python Wrapper Support ([#2676](https://github.com/valkey-io/valkey-glide/pull/2676)) -* Node, Python: Adding support for replacing connection configured password ([#2651](https://github.com/valkey-io/valkey-glide/pull/2651))([#2659](https://github.com/valkey-io/valkey-glide/pull/2659)) * Node: Add FT._ALIASLIST command([#2652](https://github.com/valkey-io/valkey-glide/pull/2652)) * Python: Python: `FT._ALIASLIST` command added([#2638](https://github.com/valkey-io/valkey-glide/pull/2638)) * Node: alias commands added: FT.ALIASADD, FT.ALIADDEL, FT.ALIASUPDATE([#2596](https://github.com/valkey-io/valkey-glide/pull/2596)) diff --git a/java/client/src/main/java/glide/api/BaseClient.java b/java/client/src/main/java/glide/api/BaseClient.java index 1a7c6b811a..fd8015cc2e 100644 --- a/java/client/src/main/java/glide/api/BaseClient.java +++ b/java/client/src/main/java/glide/api/BaseClient.java @@ -777,6 +777,66 @@ protected Map handleLcsIdxResponse(Map response) return response; } + /** + * Update the current connection with a new password. + * + *

This method is useful in scenarios where the server password has changed or when utilizing + * short-lived passwords for enhanced security. It allows the client to update its password to + * reconnect upon disconnection without the need to recreate the client instance. This ensures + * that the internal reconnection mechanism can handle reconnection seamlessly, preventing the + * loss of in-flight commands. + * + * @param immediateAuth A boolean flag. If true, the client will + * authenticate immediately with the new password against all connections, Using AUTH + * command.
+ * If password supplied is an empty string, the client will not perform auth and a warning + * will be returned.
+ * The default is `false`. + * @apiNote This method updates the client's internal password configuration and does not perform + * password rotation on the server side. + * @param password A new password to set. + * @return "OK". + * @example + *

{@code
+     * String response = client.resetConnectionPassword("new_password", RE_AUTHENTICATE).get();
+     * assert response.equals("OK");
+     * }
+ */ + public CompletableFuture updateConnectionPassword( + @NonNull String password, boolean immediateAuth) { + return commandManager.submitPasswordUpdate( + Optional.of(password), immediateAuth, this::handleStringResponse); + } + + /** + * Update the current connection by removing the password. + * + *

This method is useful in scenarios where the server password has changed or when utilizing + * short-lived passwords for enhanced security. It allows the client to update its password to + * reconnect upon disconnection without the need to recreate the client instance. This ensures + * that the internal reconnection mechanism can handle reconnection seamlessly, preventing the + * loss of in-flight commands. + * + * @apiNote This method updates the client's internal password configuration and does not perform + * password rotation on the server side. + * @param immediateAuth A boolean flag. If true, the client will + * authenticate immediately with the new password against all connections, Using AUTH + * command.
+ * If password supplied is an empty string, the client will not perform auth and a warning + * will be returned.
+ * The default is `false`. + * @return "OK". + * @example + *

{@code
+     * String response = client.resetConnectionPassword(true).get();
+     * assert response.equals("OK");
+     * }
+ */ + public CompletableFuture updateConnectionPassword(boolean immediateAuth) { + return commandManager.submitPasswordUpdate( + Optional.empty(), immediateAuth, this::handleStringResponse); + } + @Override public CompletableFuture del(@NonNull String[] keys) { return commandManager.submitNewCommand(Del, keys, this::handleLongResponse); diff --git a/java/client/src/main/java/glide/managers/CommandManager.java b/java/client/src/main/java/glide/managers/CommandManager.java index 639e258d81..d069c6bd72 100644 --- a/java/client/src/main/java/glide/managers/CommandManager.java +++ b/java/client/src/main/java/glide/managers/CommandManager.java @@ -12,6 +12,7 @@ import command_request.CommandRequestOuterClass.ScriptInvocationPointers; import command_request.CommandRequestOuterClass.SimpleRoutes; import command_request.CommandRequestOuterClass.SlotTypes; +import command_request.CommandRequestOuterClass.UpdateConnectionPassword; import glide.api.models.ClusterTransaction; import glide.api.models.GlideString; import glide.api.models.Script; @@ -218,6 +219,26 @@ public CompletableFuture submitClusterScan( return submitCommandToChannel(command, responseHandler); } + /** + * Submit a password update request to GLIDE core. + * + * @param password A new password to set or empty value to remove the password. + * @param immediateAuth immediately perform auth. + * @param responseHandler A response handler. + * @return A request promise. + * @param Type of the response. + */ + public CompletableFuture submitPasswordUpdate( + Optional password, + boolean immediateAuth, + GlideExceptionCheckedFunction responseHandler) { + var builder = UpdateConnectionPassword.newBuilder().setImmediateAuth(immediateAuth); + password.ifPresent(builder::setPassword); + + var command = CommandRequest.newBuilder().setUpdateConnectionPassword(builder.build()); + return submitCommandToChannel(command, responseHandler); + } + /** * Take a command request and send to channel. * diff --git a/java/integTest/src/test/java/glide/SharedClientTests.java b/java/integTest/src/test/java/glide/SharedClientTests.java index f509c6d041..26a4144e96 100644 --- a/java/integTest/src/test/java/glide/SharedClientTests.java +++ b/java/integTest/src/test/java/glide/SharedClientTests.java @@ -48,13 +48,15 @@ public static void init() { GlideClusterClient.createClient(commonClusterClientConfig().requestTimeout(10000).build()) .get(); clients = List.of(Arguments.of(standaloneClient), Arguments.of(clusterClient)); - assertTrue(!clusterClient.getStatistics().isEmpty()); - assertEquals( - clusterClient.getStatistics().size(), 2); // we expect 2 items in the statistics map + } - assertTrue(!standaloneClient.getStatistics().isEmpty()); - assertEquals( - standaloneClient.getStatistics().size(), 2); // we expect 2 items in the statistics map + @SneakyThrows + @ParameterizedTest(autoCloseArguments = false) + @MethodSource("getClients") + public void validate_statistics(BaseClient client) { + assertFalse(client.getStatistics().isEmpty()); + // we expect 2 items in the statistics map + assertEquals(2, client.getStatistics().size()); } @AfterAll diff --git a/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java b/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java index 288b31f5da..b777a0c9fe 100644 --- a/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java +++ b/java/integTest/src/test/java/glide/cluster/ClusterClientTests.java @@ -6,6 +6,8 @@ import static glide.TestUtilities.getRandomString; import static glide.api.BaseClient.OK; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -14,8 +16,11 @@ import glide.api.models.configuration.ServerCredentials; import glide.api.models.exceptions.ClosingException; import glide.api.models.exceptions.RequestException; +import java.util.Map; +import java.util.UUID; import java.util.concurrent.ExecutionException; import lombok.SneakyThrows; +import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; @@ -158,4 +163,117 @@ public void closed_client_throws_ExecutionException_with_ClosingException_as_cau assertThrows(ExecutionException.class, () -> client.set("foo", "bar").get()); assertTrue(executionException.getCause() instanceof ClosingException); } + + @SneakyThrows + @Test + public void test_update_connection_password() { + GlideClusterClient adminClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get(); + String pwd = UUID.randomUUID().toString(); + + try (GlideClusterClient testClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); + + // Update password without re-authentication + assertEquals(OK, testClient.updateConnectionPassword(pwd, false).get()); + + // Verify client still works with old auth + assertNotNull(testClient.info().get()); + + // Update server password + // Kill all other clients to force reconnection + assertEquals("OK", adminClient.configSet(Map.of("requirepass", pwd)).get()); + adminClient.customCommand(new String[] {"CLIENT", "KILL", "TYPE", "NORMAL"}).get(); + + // Verify client auto-reconnects with new password + assertNotNull(testClient.info().get()); + } finally { + adminClient.configSet(Map.of("requirepass", "")).get(); + adminClient.close(); + } + } + + @SneakyThrows + @Test + public void test_update_connection_password_auth_non_valid_pass() { + // Test Client fails on call to updateConnectionPassword with invalid parameters + try (GlideClusterClient testClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + var emptyPasswordException = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword("", true).get()); + assertInstanceOf(RequestException.class, emptyPasswordException.getCause()); + + var noPasswordException = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(true).get()); + assertInstanceOf(RequestException.class, noPasswordException.getCause()); + } + } + + @SneakyThrows + @Test + public void test_update_connection_password_no_server_auth() { + var pwd = UUID.randomUUID().toString(); + + try (GlideClusterClient testClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); + + // Test that immediate re-authentication fails when no server password is set. + var exception = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + } + } + + @SneakyThrows + @Test + public void test_update_connection_password_long() { + var pwd = RandomStringUtils.randomAlphabetic(1000); + + try (GlideClusterClient testClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); + + // Test replacing connection password with a long password string. + assertEquals(OK, testClient.updateConnectionPassword(pwd, false).get()); + } + } + + @Timeout(50) + @SneakyThrows + @Test + public void test_replace_password_immediateAuth_wrong_password() { + var pwd = UUID.randomUUID().toString(); + var notThePwd = UUID.randomUUID().toString(); + + GlideClusterClient adminClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get(); + try (GlideClusterClient testClient = + GlideClusterClient.createClient(commonClusterClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); + + // set the password to something else + adminClient.configSet(Map.of("requirepass", notThePwd)).get(); + + // Test that re-authentication fails when using wrong password. + var exception = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + + // But using something else password returns OK + assertEquals(OK, testClient.updateConnectionPassword(notThePwd, true).get()); + } finally { + adminClient.configSet(Map.of("requirepass", "")).get(); + adminClient.close(); + } + } } diff --git a/java/integTest/src/test/java/glide/standalone/StandaloneClientTests.java b/java/integTest/src/test/java/glide/standalone/StandaloneClientTests.java index e61b97ed4e..02551a6b9c 100644 --- a/java/integTest/src/test/java/glide/standalone/StandaloneClientTests.java +++ b/java/integTest/src/test/java/glide/standalone/StandaloneClientTests.java @@ -6,6 +6,8 @@ import static glide.TestUtilities.getRandomString; import static glide.api.BaseClient.OK; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -14,10 +16,15 @@ import glide.api.models.configuration.ServerCredentials; import glide.api.models.exceptions.ClosingException; import glide.api.models.exceptions.RequestException; +import java.util.Map; +import java.util.UUID; import java.util.concurrent.ExecutionException; import lombok.SneakyThrows; +import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; @Timeout(10) // seconds public class StandaloneClientTests { @@ -45,7 +52,7 @@ public void can_connect_with_auth_require_pass() { GlideClient client = GlideClient.createClient(commonClientConfig().build()).get(); String password = "TEST_AUTH"; - client.customCommand(new String[] {"CONFIG", "SET", "requirepass", password}).get(); + client.configSet(Map.of("requirepass", password)).get(); // Creation of a new client without a password should fail ExecutionException exception = @@ -69,7 +76,7 @@ public void can_connect_with_auth_require_pass() { assertEquals(value, auth_client.get(key).get()); // Reset password - client.customCommand(new String[] {"CONFIG", "SET", "requirepass", ""}).get(); + client.configSet(Map.of("requirepass", "")).get(); auth_client.close(); client.close(); @@ -159,4 +166,120 @@ public void closed_client_throws_ExecutionException_with_ClosingException_as_cau assertThrows(ExecutionException.class, () -> client.set("key", "value").get()); assertTrue(executionException.getCause() instanceof ClosingException); } + + @SneakyThrows + @Test + public void update_connection_password_auth_non_valid_pass() { + // Test Client fails on call to updateConnectionPassword with invalid parameters + try (var testClient = GlideClient.createClient(commonClientConfig().build()).get()) { + var emptyPasswordException = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword("", true).get()); + assertInstanceOf(RequestException.class, emptyPasswordException.getCause()); + + var noPasswordException = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(true).get()); + assertInstanceOf(RequestException.class, noPasswordException.getCause()); + } + } + + @SneakyThrows + @Test + public void update_connection_password_no_server_auth() { + var pwd = UUID.randomUUID().toString(); + + try (var testClient = GlideClient.createClient(commonClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); + + // Test that immediate re-authentication fails when no server password is set. + var exception = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + } + } + + @SneakyThrows + @Test + public void update_connection_password_long() { + var pwd = RandomStringUtils.randomAlphabetic(1000); + + try (var testClient = GlideClient.createClient(commonClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); + + // Test replacing connection password with a long password string. + assertEquals(OK, testClient.updateConnectionPassword(pwd, false).get()); + } + } + + @Timeout(50) + @SneakyThrows + @Test + public void replace_password_immediateAuth_wrong_password() { + var pwd = UUID.randomUUID().toString(); + var notThePwd = UUID.randomUUID().toString(); + + GlideClient adminClient = GlideClient.createClient(commonClientConfig().build()).get(); + try (var testClient = GlideClient.createClient(commonClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); + + // set the password to something else + adminClient.configSet(Map.of("requirepass", notThePwd)).get(); + + // Test that re-authentication fails when using wrong password. + var exception = + assertThrows( + ExecutionException.class, () -> testClient.updateConnectionPassword(pwd, true).get()); + assertInstanceOf(RequestException.class, exception.getCause()); + + // But using something else password returns OK + assertEquals(OK, testClient.updateConnectionPassword(notThePwd, true).get()); + } finally { + adminClient.configSet(Map.of("requirepass", "")).get(); + adminClient.close(); + } + } + + @SneakyThrows + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void update_connection_password_connection_lost_before_password_update( + boolean immediateAuth) { + GlideClient adminClient = GlideClient.createClient(commonClientConfig().build()).get(); + var pwd = UUID.randomUUID().toString(); + + try (var testClient = GlideClient.createClient(commonClientConfig().build()).get()) { + // validate that we can use the client + assertNotNull(testClient.info().get()); + + // set the password and forcefully drop connection for the testClient + assertEquals("OK", adminClient.configSet(Map.of("requirepass", pwd)).get()); + adminClient.customCommand(new String[] {"CLIENT", "KILL", "TYPE", "NORMAL"}).get(); + + /* + * Some explanation for the curious mind: + * Our library is abstracting a connection or connections, with a lot of mechanism around it, making it behave like what we call a "client". + * When using standalone mode, the client is a single connection, so on disconnection the first thing it planned to do is to reconnect. + * + * There's no reason to get other commands and to take care of them since to serve commands we need to be connected. + * + * Hence, the client will try to reconnect and will not listen try to take care of new tasks, but will let them wait in line, + * so the update connection password will not be able to reach the connection and will return an error. + * For future versions, standalone will be considered as a different animal then it is now, since standalone is not necessarily one node. + * It can be replicated and have a lot of nodes, and to be what we like to call "one shard cluster". + * So, in the future, we will have many existing connection and request can be managed also when one connection is locked, + */ + var exception = + assertThrows( + ExecutionException.class, + () -> testClient.updateConnectionPassword(pwd, immediateAuth).get()); + } finally { + adminClient.configSet(Map.of("requirepass", "")).get(); + adminClient.close(); + } + } } From 6e94ff32b40fcd14562faaac968f298be7cc0c45 Mon Sep 17 00:00:00 2001 From: adarovadya Date: Thu, 21 Nov 2024 08:31:25 +0200 Subject: [PATCH 12/29] Python: nit fix (#2726) nit: removed comments Signed-off-by: Adar Ovadia Co-authored-by: Adar Ovadia --- python/python/tests/conftest.py | 23 ------------------- .../python/tests/test_read_from_strategy.py | 5 ---- 2 files changed, 28 deletions(-) diff --git a/python/python/tests/conftest.py b/python/python/tests/conftest.py index f733361107..15ff15cf4e 100644 --- a/python/python/tests/conftest.py +++ b/python/python/tests/conftest.py @@ -389,26 +389,3 @@ async def test_meow_meow(...): reason=f"This feature added in version {min_version}", allow_module_level=True, ) - - -# @pytest.fixture(scope="module") -# def multiple_replicas_cluster(request): -# """ -# Fixture to create a special cluster with 4 replicas for specific tests. -# """ -# tls = request.config.getoption("--tls") -# load_module = request.config.getoption("--load-module") -# cluster_endpoints = request.config.getoption("--cluster-endpoints") - -# if not cluster_endpoints: -# multiple_replica_cluster = ValkeyCluster( -# tls=tls, -# cluster_mode=True, -# load_module=load_module, -# addresses=cluster_endpoints, -# replica_count=4, -# ) -# yield multiple_replica_cluster -# multiple_replica_cluster.__del__() -# else: -# yield None diff --git a/python/python/tests/test_read_from_strategy.py b/python/python/tests/test_read_from_strategy.py index 429234ec82..03f3f8e9ae 100644 --- a/python/python/tests/test_read_from_strategy.py +++ b/python/python/tests/test_read_from_strategy.py @@ -35,7 +35,6 @@ async def test_routing_with_az_affinity_strategy_to_1_replica( request, cluster_mode: bool, protocol: ProtocolVersion, - # multiple_replicas_cluster, ): """Test that the client with az affinity strategy will only route to the 1 replica with the same az""" az = "us-east-1a" @@ -66,7 +65,6 @@ async def test_routing_with_az_affinity_strategy_to_1_replica( client_for_testing_az = await create_client( request, cluster_mode, - # addresses=multiple_replicas_cluster.nodes_addr, protocol=protocol, read_from=ReadFrom.AZ_AFFINITY, timeout=2000, @@ -106,7 +104,6 @@ async def test_routing_by_slot_to_replica_with_az_affinity_strategy_to_all_repli request, cluster_mode: bool, protocol: ProtocolVersion, - # multiple_replicas_cluster, ): """Test that the client with AZ affinity strategy routes in a round-robin manner to all replicas within the specified AZ""" @@ -126,7 +123,6 @@ async def test_routing_by_slot_to_replica_with_az_affinity_strategy_to_all_repli client_for_testing_az = await create_client( request, cluster_mode, - # addresses=multiple_replicas_cluster.nodes_addr, protocol=protocol, read_from=ReadFrom.AZ_AFFINITY, timeout=2000, @@ -176,7 +172,6 @@ async def test_az_affinity_non_existing_az( request, cluster_mode: bool, protocol: ProtocolVersion, - # multiple_replicas_cluster, ): GET_CALLS = 4 From cb2525bc59a6e08ee7ecf0fccc231f571d688621 Mon Sep 17 00:00:00 2001 From: eifrah-aws Date: Thu, 21 Nov 2024 12:18:31 +0200 Subject: [PATCH 13/29] =?UTF-8?q?Allow=20user=20to=20set=20different=20log?= =?UTF-8?q?ging=20directory=20by=20passing=20environment=20=E2=80=A6=20(#2?= =?UTF-8?q?705)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 2 ++ logger_core/src/lib.rs | 67 ++++++++++++++++++++++++++++++++++++++--- python/requirements.txt | 1 + 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92d19ac7b4..9f76b4ebb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -495,6 +495,7 @@ * Node: Added LINDEX command ([#999](https://github.com/valkey-io/valkey-glide/pull/999)) * Python, Node: Added ZPOPMAX command ([#996](https://github.com/valkey-io/valkey-glide/pull/996), [#1009](https://github.com/valkey-io/valkey-glide/pull/1009)) * Python: Added DBSIZE command ([#1040](https://github.com/valkey-io/valkey-glide/pull/1040)) +* Core: Log directory can now be modified by setting the environment variable `GLIDE_LOG_DIR` ([#2704](https://github.com/valkey-io/valkey-glide/issues/2704)) #### Features @@ -514,3 +515,4 @@ Preview release of **GLIDE for Redis** a Polyglot Redis client. See the [README](README.md) for additional information. + diff --git a/logger_core/src/lib.rs b/logger_core/src/lib.rs index fd2acd0640..a31e4fdbc2 100644 --- a/logger_core/src/lib.rs +++ b/logger_core/src/lib.rs @@ -55,6 +55,7 @@ pub static INITIATE_ONCE: InitiateOnce = InitiateOnce { }; const FILE_DIRECTORY: &str = "glide-logs"; +const ENV_GLIDE_LOG_DIR: &str = "GLIDE_LOG_DIR"; /// Wraps [RollingFileAppender] to defer initialization until logging is required, /// allowing [init] to disable file logging on read-only filesystems. @@ -117,6 +118,21 @@ impl Level { } } +/// Attempt to read a directory path from an environment variable. If the environment variable `envname` exists +/// and contains a valid path - this function will create and return that path. In any case of failure, +/// this method returns `None` (e.g. the environment variable exists but contains an empty path etc) +pub fn create_directory_from_env(envname: &str) -> Option { + let Ok(dirpath) = std::env::var(envname) else { + return None; + }; + + if dirpath.trim().is_empty() || std::fs::create_dir_all(&dirpath).is_err() { + return None; + } + + Some(dirpath) +} + // Initialize the global logger to error level on the first call only // In any of the calls to the function, including the first - resetting the existence loggers to the new setting // provided by using the global reloadable handle @@ -131,17 +147,21 @@ pub fn init(minimal_level: Option, file_name: Option<&str>) -> Level { let (stdout_layer, stdout_reload) = reload::Layer::new(stdout_fmt); + // Check if the environment variable GLIDE_LOG is set + let logs_dir = + create_directory_from_env(ENV_GLIDE_LOG_DIR).unwrap_or(FILE_DIRECTORY.to_string()); let file_appender = LazyRollingFileAppender::new( Rotation::HOURLY, - FILE_DIRECTORY, + logs_dir, file_name.unwrap_or("output.log"), ); + let file_fmt = tracing_subscriber::fmt::layer() .with_writer(file_appender) .with_filter(LevelFilter::OFF); let (file_layer, file_reload) = reload::Layer::new(file_fmt); - // Enable logging only from allowed crates + // If user has set the environment variable "RUST_LOG" with a valid log verbosity, use it let log_level = if let Ok(level) = std::env::var("RUST_LOG") { let trace_level = tracing::Level::from_str(&level).unwrap_or(tracing::Level::TRACE); LevelFilter::from(trace_level) @@ -149,6 +169,7 @@ pub fn init(minimal_level: Option, file_name: Option<&str>) -> Level { LevelFilter::TRACE }; + // Enable logging only from allowed crates let targets_filter = filter::Targets::new() .with_target("glide", log_level) .with_target("redis", log_level) @@ -184,8 +205,10 @@ pub fn init(minimal_level: Option, file_name: Option<&str>) -> Level { }); } Some(file) => { - let file_appender = - LazyRollingFileAppender::new(Rotation::HOURLY, FILE_DIRECTORY, file); + // Check if the environment variable GLIDE_LOG is set + let logs_dir = + create_directory_from_env(ENV_GLIDE_LOG_DIR).unwrap_or(FILE_DIRECTORY.to_string()); + let file_appender = LazyRollingFileAppender::new(Rotation::HOURLY, logs_dir, file); let _ = reloads .file_reload .write() @@ -247,3 +270,39 @@ pub fn log, Identifier: AsRef>( Level::Off => (), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_directory_from_env() { + let dir_path = format!("{}/glide-logs", std::env::temp_dir().display()); + // Case 1: try to create an already existing folder + // make sure we are starting fresh + let _ = std::fs::remove_dir_all(&dir_path); + // Create the directory + assert!(std::fs::create_dir_all(&dir_path).is_ok()); + + std::env::set_var(ENV_GLIDE_LOG_DIR, &dir_path); + assert!(create_directory_from_env(ENV_GLIDE_LOG_DIR).is_some()); + assert!(std::fs::metadata(&dir_path).is_ok()); + + // Case 2: try to create a new folder (i.e. the folder does not already exist) + let _ = std::fs::remove_dir_all(&dir_path); + + // Create the directory + assert!(std::fs::create_dir_all(&dir_path).is_ok()); + assert!(std::fs::metadata(&dir_path).is_ok()); + + std::env::set_var(ENV_GLIDE_LOG_DIR, &dir_path); + assert!(create_directory_from_env(ENV_GLIDE_LOG_DIR).is_some()); + + // make sure we are starting fresh + let _ = std::fs::remove_dir_all(&dir_path); + + // Case 3: empty variable is not acceptable + std::env::set_var(ENV_GLIDE_LOG_DIR, ""); + assert!(create_directory_from_env(ENV_GLIDE_LOG_DIR).is_none()); + } +} diff --git a/python/requirements.txt b/python/requirements.txt index 93c90b2cac..cffc0870cb 100644 --- a/python/requirements.txt +++ b/python/requirements.txt @@ -5,3 +5,4 @@ pytest pytest-asyncio typing_extensions==4.8.0;python_version<"3.11" pytest-html +pyrsistent From 417369af98cf9db33739218dd0fba6fcc761a9a5 Mon Sep 17 00:00:00 2001 From: Muhammad Awawdi Date: Thu, 21 Nov 2024 13:15:06 +0200 Subject: [PATCH 14/29] Node - Client API for retrieving internal statistics (#2727) --------- Signed-off-by: Muhammad Awawdi --- CHANGELOG.md | 1 + node/rust-client/src/lib.rs | 12 ++++++++++ node/src/BaseClient.ts | 9 +++++++ node/tests/GlideClusterClient.test.ts | 34 +++++++++++++++++++++++++++ 4 files changed, 56 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f76b4ebb3..1729c2b2dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ #### Changes +* Node: Client API for retrieving internal statistics ([#2727](https://github.com/valkey-io/valkey-glide/pull/2727)) * Python: Client API for retrieving internal statistics ([#2707](https://github.com/valkey-io/valkey-glide/pull/2707)) * Node, Python, Java: Adding support for replacing connection configured password ([#2651](https://github.com/valkey-io/valkey-glide/pull/2651), [#2659](https://github.com/valkey-io/valkey-glide/pull/2659), [#2677](https://github.com/valkey-io/valkey-glide/pull/2677)) * Python: AZ Affinity - Python Wrapper Support ([#2676](https://github.com/valkey-io/valkey-glide/pull/2676)) diff --git a/node/rust-client/src/lib.rs b/node/rust-client/src/lib.rs index 82e546d295..b15b18f521 100644 --- a/node/rust-client/src/lib.rs +++ b/node/rust-client/src/lib.rs @@ -1,3 +1,4 @@ +use glide_core::Telemetry; use redis::GlideConnectionOptions; /** * Copyright Valkey GLIDE Project Contributors - SPDX Identifier: Apache-2.0 @@ -484,3 +485,14 @@ impl Drop for ClusterScanCursor { glide_core::cluster_scan_container::remove_scan_state_cursor(self.cursor.clone()); } } + +#[napi] +pub fn get_statistics(env: Env) -> Result { + let total_connections = Telemetry::total_connections().to_string(); + let total_clients = Telemetry::total_clients().to_string(); + let mut stats: JsObject = env.create_object()?; + stats.set_named_property("total_connections", total_connections)?; + stats.set_named_property("total_clients", total_clients)?; + + Ok(stats) +} diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index 7c2e3feb57..d3ee6a91dc 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -7,6 +7,7 @@ import { DEFAULT_TIMEOUT_IN_MILLISECONDS, Script, StartSocketConnection, + getStatistics, valueFromSplitPointer, } from "glide-rs"; import * as net from "net"; @@ -7743,4 +7744,12 @@ export class BaseClient { return response; } + /** + * Return a statistics + * + * @return Return an object that contains the statistics collected internally by GLIDE core + */ + public getStatistics(): object { + return getStatistics(); + } } diff --git a/node/tests/GlideClusterClient.test.ts b/node/tests/GlideClusterClient.test.ts index 79868c36a8..f74e137cac 100644 --- a/node/tests/GlideClusterClient.test.ts +++ b/node/tests/GlideClusterClient.test.ts @@ -2398,4 +2398,38 @@ describe("GlideClusterClient", () => { }, ); }); + describe("GlideClusterClient - Get Statistics", () => { + it.each([ProtocolVersion.RESP2, ProtocolVersion.RESP3])( + "should return valid statistics using protocol %p", + async (protocol) => { + let glideClientForTesting; + + try { + // Create a GlideClusterClient instance for testing + glideClientForTesting = + await GlideClusterClient.createClient( + getClientConfigurationOption( + cluster.getAddresses(), + protocol, + { + requestTimeout: 2000, + }, + ), + ); + + // Fetch statistics using get_statistics method + const stats = await glideClientForTesting.getStatistics(); + + // Assertions to check if stats object has correct structure + expect(typeof stats).toBe("object"); + expect(stats).toHaveProperty("total_connections"); + expect(stats).toHaveProperty("total_clients"); + expect(Object.keys(stats)).toHaveLength(2); + } finally { + // Ensure the client is properly closed + await glideClientForTesting?.close(); + } + }, + ); + }); }); From 62686e13706a5ed4cec3f1eb7c10de09e3ccdbd5 Mon Sep 17 00:00:00 2001 From: Muhammad Awawdi Date: Thu, 21 Nov 2024 14:49:10 +0200 Subject: [PATCH 15/29] Decrease Warnings in the CI (#2703) ============================================== Signed-off-by: Muhammad Awawdi --- python/python/tests/test_async_client.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/python/python/tests/test_async_client.py b/python/python/tests/test_async_client.py index da812c07a3..b32aa6936d 100644 --- a/python/python/tests/test_async_client.py +++ b/python/python/tests/test_async_client.py @@ -137,6 +137,7 @@ async def test_send_and_receive_large_values(self, request, cluster_mode, protoc assert len(value) == length await glide_client.set(key, value) assert await glide_client.get(key) == value.encode() + await glide_client.close() @pytest.mark.parametrize("cluster_mode", [True, False]) @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) @@ -202,6 +203,8 @@ async def test_can_connect_with_auth_requirepass( key = get_random_string(10) assert await auth_client.set(key, key) == OK assert await auth_client.get(key) == key.encode() + await auth_client.close() + finally: # Reset the password auth_client = await create_client( @@ -211,6 +214,7 @@ async def test_can_connect_with_auth_requirepass( addresses=glide_client.config.addresses, ) await auth_client.custom_command(["CONFIG", "SET", "requirepass", ""]) + await auth_client.close() @pytest.mark.parametrize("cluster_mode", [True, False]) @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) @@ -254,6 +258,7 @@ async def test_can_connect_with_auth_acl( # This client isn't authorized to perform SET await testuser_client.set("foo", "bar") assert "NOPERM" in str(e) + await testuser_client.close() finally: # Delete this user await glide_client.custom_command(["ACL", "DELUSER", username]) @@ -265,6 +270,7 @@ async def test_select_standalone_database_id(self, request, cluster_mode): ) client_info = await glide_client.custom_command(["CLIENT", "INFO"]) assert b"db=4" in client_info + await glide_client.close() @pytest.mark.parametrize("cluster_mode", [True, False]) @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) @@ -277,6 +283,7 @@ async def test_client_name(self, request, cluster_mode, protocol): ) client_info = await glide_client.custom_command(["CLIENT", "INFO"]) assert b"name=TEST_CLIENT_NAME" in client_info + await glide_client.close() @pytest.mark.parametrize("cluster_mode", [True, False]) @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) @@ -8482,6 +8489,7 @@ async def wait_and_function_kill(): with pytest.raises(RequestError) as e: assert await glide_client.function_kill() assert "NotBusy" in str(e) + await test_client.close() @pytest.mark.parametrize("cluster_mode", [False, True]) @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) @@ -8532,6 +8540,7 @@ async def wait_and_function_kill(): endless_fcall_route_call(), wait_and_function_kill(), ) + await test_client.close() @pytest.mark.parametrize("cluster_mode", [True]) @pytest.mark.parametrize("protocol", [ProtocolVersion.RESP2, ProtocolVersion.RESP3]) From cb9ce08b2fa557db67b60622069405122c6057d7 Mon Sep 17 00:00:00 2001 From: tjzhang-BQ <111323543+tjzhang-BQ@users.noreply.github.com> Date: Thu, 21 Nov 2024 15:41:51 -0800 Subject: [PATCH 16/29] Java: adding changelog for java az support change (#2734) * Java: adding changelog for java az support change Signed-off-by: TJ Zhang --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1729c2b2dd..a99efefb2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ * Node: Client API for retrieving internal statistics ([#2727](https://github.com/valkey-io/valkey-glide/pull/2727)) * Python: Client API for retrieving internal statistics ([#2707](https://github.com/valkey-io/valkey-glide/pull/2707)) * Node, Python, Java: Adding support for replacing connection configured password ([#2651](https://github.com/valkey-io/valkey-glide/pull/2651), [#2659](https://github.com/valkey-io/valkey-glide/pull/2659), [#2677](https://github.com/valkey-io/valkey-glide/pull/2677)) -* Python: AZ Affinity - Python Wrapper Support ([#2676](https://github.com/valkey-io/valkey-glide/pull/2676)) +* Node, Python, Java: AZ Affinity - Python Wrapper Support ([#2686](https://github.com/valkey-io/valkey-glide/pull/2686), [#2676](https://github.com/valkey-io/valkey-glide/pull/2676), [#2678](https://github.com/valkey-io/valkey-glide/pull/2678)) * Node: Add FT._ALIASLIST command([#2652](https://github.com/valkey-io/valkey-glide/pull/2652)) * Python: Python: `FT._ALIASLIST` command added([#2638](https://github.com/valkey-io/valkey-glide/pull/2638)) * Node: alias commands added: FT.ALIASADD, FT.ALIADDEL, FT.ALIASUPDATE([#2596](https://github.com/valkey-io/valkey-glide/pull/2596)) From 7d84045b834e8380c180dbe9299937ed709468b2 Mon Sep 17 00:00:00 2001 From: Avi Fenesh <55848801+avifenesh@users.noreply.github.com> Date: Mon, 25 Nov 2024 15:14:43 +0200 Subject: [PATCH 17/29] Node rc fixes + unpublish on failure mechanism (#2737) * Update Node.js Docker image and enhance npm publish workflow with unpublish feature Signed-off-by: avifenesh * unpublishing a package owned by more than one cant be unpublised, hence the usage is deprecating Signed-off-by: avifenesh --------- Signed-off-by: avifenesh --- .github/json_matrices/build-matrix.json | 4 +- .github/workflows/npm-cd.yml | 76 ++++++++++++++++++++----- node/package.json | 4 +- 3 files changed, 67 insertions(+), 17 deletions(-) diff --git a/.github/json_matrices/build-matrix.json b/.github/json_matrices/build-matrix.json index 5e10c38a37..0d0e7c10bb 100644 --- a/.github/json_matrices/build-matrix.json +++ b/.github/json_matrices/build-matrix.json @@ -33,7 +33,7 @@ "ARCH": "arm64", "TARGET": "aarch64-unknown-linux-musl", "RUNNER": ["self-hosted", "Linux", "ARM64"], - "IMAGE": "node:lts-alpine3.19", + "IMAGE": "node:lts-alpine", "CONTAINER_OPTIONS": "--user root --privileged --rm", "PACKAGE_MANAGERS": ["npm"], "languages": ["node"] @@ -44,7 +44,7 @@ "ARCH": "x64", "TARGET": "x86_64-unknown-linux-musl", "RUNNER": "ubuntu-latest", - "IMAGE": "node:lts-alpine3.19", + "IMAGE": "node:lts-alpine", "CONTAINER_OPTIONS": "--user root --privileged", "PACKAGE_MANAGERS": ["npm"], "languages": ["node"] diff --git a/.github/workflows/npm-cd.yml b/.github/workflows/npm-cd.yml index 8ff2936fbc..24497e83cc 100644 --- a/.github/workflows/npm-cd.yml +++ b/.github/workflows/npm-cd.yml @@ -181,18 +181,22 @@ jobs: working-directory: ./node run: | npm pkg fix - set +e - # 2>&1 1>&3- redirects stderr to stdout and then redirects the original stdout to another file descriptor, - # effectively separating stderr and stdout. The 3>&1 at the end redirects the original stdout back to the console. - # https://github.com/npm/npm/issues/118#issuecomment-325440 - ignoring notice messages since currentlly they are directed to stderr - { npm_publish_err=$(npm publish --tag ${{ env.NPM_TAG }} --access public 2>&1 1>&3- | grep -Ev "notice|ExperimentalWarning") ;} 3>&1 - if [[ "$npm_publish_err" == *"You cannot publish over the previously published versions"* ]] - then - echo "Skipping publishing, package already published" - elif [[ ! -z "$npm_publish_err" ]] - then - echo "Failed to publish with error: ${npm_publish_err}" - exit 1 + set +e # Disable immediate exit on non-zero exit codes + + # Redirect stderr to stdout, filter out notices and warnings + { npm_publish_err=$(npm publish --tag "${NPM_TAG}" --access public --loglevel=error 2>&1 1>&3- | grep -Ev "notice|ExperimentalWarning|WARN") ;} 3>&1 + publish_exit_code=$? + + # Re-enable immediate exit + set -e + + if [[ $publish_exit_code -eq 0 ]]; then + echo "Package published successfully." + elif echo "$npm_publish_err" | grep -q "You cannot publish over the previously published versions"; then + echo "Skipping publishing, package already published." + elif [[ ! -z "$npm_publish_err" ]]; then + echo "Failed to publish with error: $npm_publish_err" + exit 1 fi env: NODE_AUTH_TOKEN: ${{ secrets.NPM_AUTH_TOKEN }} @@ -375,10 +379,56 @@ jobs: npm install --no-save @valkey/valkey-glide@${{ env.NPM_TAG }} npm run test + - name: Deprecating packages on failure + if: ${{ failure() }} + shell: bash + env: + GH_EVENT_NAME: ${{ github.event_name }} + GH_EVENT_INPUT_VERSION: ${{ github.event.inputs.version }} + GH_REF: ${{ github.ref }} + NODE_AUTH_TOKEN: ${{ secrets.NPM_AUTH_TOKEN }} + PLATFORM_MATRIX: ${{ needs.load-platform-matrix.outputs.PLATFORM_MATRIX }} + run: | + # Detect OS and install jq + if [[ "${OSTYPE}" == "darwin"* ]]; then + brew install jq || true + elif command -v apk > /dev/null; then + apk add --no-cache jq + else + sudo apt-get update && sudo apt-get install -y jq + fi + + # Set RELEASE_VERSION correctly using environment variables + if [[ "${GH_EVENT_NAME}" == "workflow_dispatch" ]]; then + RELEASE_VERSION="${GH_EVENT_INPUT_VERSION}" + else + RELEASE_VERSION="${GH_REF#refs/tags/v}" + fi + + # Validate RELEASE_VERSION + if [[ ! "${RELEASE_VERSION}" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-rc[0-9]+)?$ ]]; then + echo "Invalid release version format: ${RELEASE_VERSION}" + exit 1 + fi + + echo "Release version for Deprecating: ${RELEASE_VERSION}" + + # Deprecating base package + npm deprecate "@valkey/valkey-glide@${RELEASE_VERSION}" "This version has been deprecated" --force || true + + # Process platform matrix + echo "${PLATFORM_MATRIX}" > platform_matrix.json + + while read -r pkg; do + package_name="@valkey/valkey-glide-${pkg}" + echo "Deprecating ${package_name}@${RELEASE_VERSION}" + npm deprecate "${package_name}@${RELEASE_VERSION}" "This version has been deprecated" --force || true + done < <(jq -r '.[] | "\(.NAMED_OS)\(.TARGET | test("musl") | if . then "-musl" else "" end)-\(.ARCH)"' platform_matrix.json) + # Reset the repository to make sure we get the clean checkout of the action later in other actions. # It is not required since in other actions we are cleaning before the action, but it is a good practice to do it here as well. - name: Reset repository - if: ${{ contains(matrix.build.RUNNER, 'self-hosted') }} + if: ${{ always() }} && ${{ contains(matrix.build.RUNNER, 'self-hosted') }} shell: bash run: | git reset --hard diff --git a/node/package.json b/node/package.json index cda3849de7..62f177898b 100644 --- a/node/package.json +++ b/node/package.json @@ -60,8 +60,8 @@ "semver": "^7.6.3", "ts-jest": "^29.2.5", "ts-node": "^10.9.2", - "typescript": "^5.5.4", - "uuid": "^11.0" + "typescript": "^5.6.3", + "uuid": "^11.0.3" }, "author": "Valkey GLIDE Maintainers", "license": "Apache-2.0", From 6fa46ebf505fafb77e7cc439537b7127bbda3eee Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Mon, 25 Nov 2024 11:25:05 -0800 Subject: [PATCH 18/29] Fix node scripts and docs. (#2744) Signed-off-by: Yury-Fridlyand --- node/DEVELOPER.md | 23 ++++++++++++++++------- node/package.json | 5 +++-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/node/DEVELOPER.md b/node/DEVELOPER.md index 15c8b53a14..8878fdd91d 100644 --- a/node/DEVELOPER.md +++ b/node/DEVELOPER.md @@ -126,28 +126,37 @@ To run tests, use the following command: npm test ``` -To execute a specific test, include the [`testNamePattern`](https://jestjs.io/docs/cli#--testnamepatternregex) option. For example: +Simplified test suite skips few time consuming tests and runs faster: ```bash -npm run test -- --testNamePattern="transaction" +npm test-minimum +``` + +To execute a specific test, use the [`testNamePattern`](https://jestjs.io/docs/cli#--testnamepatternregex) option with `test-dbg` script. For example: + +```bash +npm run test-dbg -- --testNamePattern="transaction" ``` IT suite starts the server for testing - standalone and cluster installation using `cluster_manager` script. To run the integration tests with existing servers, run the following command: ```bash -npm run test -- --cluster-endpoints=localhost:7000 --standalone-endpoints=localhost:6379 +npm run test-dbg -- --cluster-endpoints=localhost:7000 --standalone-endpoints=localhost:6379 # If those endpoints use TLS, add `--tls=true` (applies to both endpoints) -npm run test -- --cluster-endpoints=localhost:7000 --standalone-endpoints=localhost:6379 --tls=true +npm run test-dbg -- --cluster-endpoints=localhost:7000 --standalone-endpoints=localhost:6379 --tls=true ``` -By default, the server_modules tests do not run using `npm run test`. After pointing to a server with JSON and VSS modules setup, -run the following command: +Parameters `cluster-endpoints`, `standalone-endpoints` and `tls` could be used with all test suites. + +By default, the server modules tests do not run using `npm run test`. This test suite also does not start the server. +In order to run these tests, use: ```bash -npm run test-modules +npm run test-modules -- --cluster-endpoints=
: ``` +Note: these tests don't run with standalone server as of now. ### REPL (interactive shell) diff --git a/node/package.json b/node/package.json index 62f177898b..b581123576 100644 --- a/node/package.json +++ b/node/package.json @@ -33,8 +33,9 @@ "clean": "rm -rf build-ts rust-client/target docs glide-logs rust-client/glide-rs.*.node rust-client/index.* src/ProtobufMessage.*", "fix-protobuf-file": "replace 'this\\.encode\\(message, writer\\)\\.ldelim' 'this.encode(message, writer && writer.len ? writer.fork() : writer).ldelim' src/ProtobufMessage.js", "test": "npm run build-test-utils && jest --verbose --testPathIgnorePatterns='ServerModules'", - "test-minimum": "npm run build-test-utils && jest --verbose --runInBand --testNamePattern='^(.(?!(GlideJson|GlideFt|pubsub|kill)))*$'", - "test-modules": "npm run build-test-utils && jest --verbose --runInBand --testNamePattern='(GlideJson|GlideFt)'", + "test-dbg": "npm run build-test-utils && jest --runInBand", + "test-minimum": "npm run build-test-utils && jest --verbose --testNamePattern='^(.(?!(GlideJson|GlideFt|pubsub|kill)))*$'", + "test-modules": "npm run build-test-utils && jest --verbose --testNamePattern='(GlideJson|GlideFt)'", "build-test-utils": "cd ../utils && npm i && npm run build", "lint:fix": "npm run install-linting && npx eslint -c ../eslint.config.mjs --fix && npm run prettier:format", "lint": "npm run install-linting && npx eslint -c ../eslint.config.mjs && npm run prettier:check:ci", From 7123a682b0a6d588b3abfb35bfa5573dd97b1821 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Mon, 25 Nov 2024 11:32:42 -0800 Subject: [PATCH 19/29] Java: Fix readme (#2751) * Fix readme Signed-off-by: Yury-Fridlyand --- java/README.md | 17 ----------------- java/client/build.gradle | 1 - java/integTest/build.gradle | 2 +- 3 files changed, 1 insertion(+), 19 deletions(-) diff --git a/java/README.md b/java/README.md index 1252475c32..4264d4c838 100644 --- a/java/README.md +++ b/java/README.md @@ -57,7 +57,6 @@ Additionally, consider installing the Gradle plugin, [OS Detector](https://githu There are 4 types of classifiers for Valkey GLIDE which are ``` osx-aarch_64 -osx-x86_64 linux-aarch_64 linux-x86_64 ``` @@ -71,11 +70,6 @@ dependencies { implementation group: 'io.valkey', name: 'valkey-glide', version: '1.+', classifier: 'osx-aarch_64' } -// osx-x86_64 -dependencies { - implementation group: 'io.valkey', name: 'valkey-glide', version: '1.+', classifier: 'osx-x86_64' -} - // linux-aarch_64 dependencies { implementation group: 'io.valkey', name: 'valkey-glide', version: '1.+', classifier: 'linux-aarch_64' @@ -107,14 +101,6 @@ Maven: [1.0.0,2.0.0) - - - io.valkey - valkey-glide - osx-x86_64 - [1.0.0,2.0.0) - - io.valkey @@ -138,9 +124,6 @@ SBT: // osx-aarch_64 libraryDependencies += "io.valkey" % "valkey-glide" % "1.+" classifier "osx-aarch_64" -// osx-x86_64 -libraryDependencies += "io.valkey" % "valkey-glide" % "1.+" classifier "osx-x86_64" - // linux-aarch_64 libraryDependencies += "io.valkey" % "valkey-glide" % "1.+" classifier "linux-aarch_64" diff --git a/java/client/build.gradle b/java/client/build.gradle index 364b09ca1e..cc2446251d 100644 --- a/java/client/build.gradle +++ b/java/client/build.gradle @@ -22,7 +22,6 @@ dependencies { // At the moment, Windows is not supported implementation group: 'io.netty', name: 'netty-transport-native-epoll', version: '4.1.100.Final', classifier: 'linux-x86_64' implementation group: 'io.netty', name: 'netty-transport-native-epoll', version: '4.1.100.Final', classifier: 'linux-aarch_64' - implementation group: 'io.netty', name: 'netty-transport-native-kqueue', version: '4.1.100.Final', classifier: 'osx-x86_64' implementation group: 'io.netty', name: 'netty-transport-native-kqueue', version: '4.1.100.Final', classifier: 'osx-aarch_64' // junit diff --git a/java/integTest/build.gradle b/java/integTest/build.gradle index 918d59ea23..53b690aa49 100644 --- a/java/integTest/build.gradle +++ b/java/integTest/build.gradle @@ -16,7 +16,7 @@ dependencies { // https://github.com/netty/netty/wiki/Native-transports // At the moment, Windows is not supported implementation group: 'io.netty', name: 'netty-transport-native-epoll', version: '4.1.100.Final', classifier: 'linux-x86_64' - implementation group: 'io.netty', name: 'netty-transport-native-kqueue', version: '4.1.100.Final', classifier: 'osx-x86_64' + implementation group: 'io.netty', name: 'netty-transport-native-epoll', version: '4.1.100.Final', classifier: 'linux-aarch_64' implementation group: 'io.netty', name: 'netty-transport-native-kqueue', version: '4.1.100.Final', classifier: 'osx-aarch_64' // junit From ebd33c5fa572db9b8e1c9a6c1b63bf09f831e240 Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Mon, 25 Nov 2024 11:43:53 -0800 Subject: [PATCH 20/29] fix changelog (#2752) Signed-off-by: Yury-Fridlyand --- CHANGELOG.md | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a99efefb2a..e52f9b2161 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,26 +3,26 @@ * Python: Client API for retrieving internal statistics ([#2707](https://github.com/valkey-io/valkey-glide/pull/2707)) * Node, Python, Java: Adding support for replacing connection configured password ([#2651](https://github.com/valkey-io/valkey-glide/pull/2651), [#2659](https://github.com/valkey-io/valkey-glide/pull/2659), [#2677](https://github.com/valkey-io/valkey-glide/pull/2677)) * Node, Python, Java: AZ Affinity - Python Wrapper Support ([#2686](https://github.com/valkey-io/valkey-glide/pull/2686), [#2676](https://github.com/valkey-io/valkey-glide/pull/2676), [#2678](https://github.com/valkey-io/valkey-glide/pull/2678)) -* Node: Add FT._ALIASLIST command([#2652](https://github.com/valkey-io/valkey-glide/pull/2652)) -* Python: Python: `FT._ALIASLIST` command added([#2638](https://github.com/valkey-io/valkey-glide/pull/2638)) -* Node: alias commands added: FT.ALIASADD, FT.ALIADDEL, FT.ALIASUPDATE([#2596](https://github.com/valkey-io/valkey-glide/pull/2596)) +* Node: Add `FT._ALIASLIST` command ([#2652](https://github.com/valkey-io/valkey-glide/pull/2652)) +* Python: Add `FT._ALIASLIST` command ([#2638](https://github.com/valkey-io/valkey-glide/pull/2638)) +* Node: Add `FT.ALIASADD`, `FT.ALIADDEL`, `FT.ALIASUPDATE` ([#2596](https://github.com/valkey-io/valkey-glide/pull/2596)) * Python code cleanup ([#2573](https://github.com/valkey-io/valkey-glide/pull/2573)) -* Python: Python: FT.PROFILE command added ([#2543](https://github.com/valkey-io/valkey-glide/pull/2543)) -* Python: Python: FT.AGGREGATE command added([#2530](https://github.com/valkey-io/valkey-glide/pull/2530)) -* Python: Add JSON.OBJLEN command ([#2495](https://github.com/valkey-io/valkey-glide/pull/2495)) -* Python: FT.EXPLAIN and FT.EXPLAINCLI commands added([#2508](https://github.com/valkey-io/valkey-glide/pull/2508)) -* Python: Python FT.INFO command added([#2429](https://github.com/valkey-io/valkey-glide/pull/2494)) -* Python: Add FT.SEARCH command([#2470](https://github.com/valkey-io/valkey-glide/pull/2470)) -* Python: Add commands FT.ALIASADD, FT.ALIASDEL, FT.ALIASUPDATE([#2471](https://github.com/valkey-io/valkey-glide/pull/2471)) -* Python: Python FT.DROPINDEX command ([#2437](https://github.com/valkey-io/valkey-glide/pull/2437)) -* Python: Python: Added FT.CREATE command([#2413](https://github.com/valkey-io/valkey-glide/pull/2413)) -* Python: Add JSON.MGET command ([#2507](https://github.com/valkey-io/valkey-glide/pull/2507)) -* Python: Add JSON.ARRLEN command ([#2403](https://github.com/valkey-io/valkey-glide/pull/2403)) -* Python: Add JSON.CLEAR command ([#2418](https://github.com/valkey-io/valkey-glide/pull/2418)) -* Python: Add JSON.TYPE command ([#2409](https://github.com/valkey-io/valkey-glide/pull/2409)) -* Python: Add JSON.NUMINCRBY command ([#2448](https://github.com/valkey-io/valkey-glide/pull/2448)) -* Python: Add JSON.NUMMULTBY command ([#2458](https://github.com/valkey-io/valkey-glide/pull/2458)) -* Python: Add JSON.ARRINDEX command ([#2528](https://github.com/valkey-io/valkey-glide/pull/2528)) +* Python: Add `FT.PROFILE` command ([#2543](https://github.com/valkey-io/valkey-glide/pull/2543)) +* Python: Add `FT.AGGREGATE` command ([#2530](https://github.com/valkey-io/valkey-glide/pull/2530)) +* Python: Add `JSON.OBJLEN` command ([#2495](https://github.com/valkey-io/valkey-glide/pull/2495)) +* Python: Add `FT.EXPLAIN` and `FT.EXPLAINCLI` commands ([#2508](https://github.com/valkey-io/valkey-glide/pull/2508)) +* Python: Add `FT.INFO` command ([#2429](https://github.com/valkey-io/valkey-glide/pull/2494)) +* Python: Add `FT.SEARCH` command ([#2470](https://github.com/valkey-io/valkey-glide/pull/2470)) +* Python: Add commands `FT.ALIASADD`, `FT.ALIASDEL`, `FT.ALIASUPDATE` ([#2471](https://github.com/valkey-io/valkey-glide/pull/2471)) +* Python: Add `FT.DROPINDEX` command ([#2437](https://github.com/valkey-io/valkey-glide/pull/2437)) +* Python: Add `FT.CREATE` command ([#2413](https://github.com/valkey-io/valkey-glide/pull/2413)) +* Python: Add `JSON.MGET` command ([#2507](https://github.com/valkey-io/valkey-glide/pull/2507)) +* Python: Add `JSON.ARRLEN` command ([#2403](https://github.com/valkey-io/valkey-glide/pull/2403)) +* Python: Add `JSON.CLEAR` command ([#2418](https://github.com/valkey-io/valkey-glide/pull/2418)) +* Python: Add `JSON.TYPE` command ([#2409](https://github.com/valkey-io/valkey-glide/pull/2409)) +* Python: Add `JSON.NUMINCRBY` command ([#2448](https://github.com/valkey-io/valkey-glide/pull/2448)) +* Python: Add `JSON.NUMMULTBY` command ([#2458](https://github.com/valkey-io/valkey-glide/pull/2458)) +* Python: Add `JSON.ARRINDEX` command ([#2528](https://github.com/valkey-io/valkey-glide/pull/2528)) * Python: Add `FT._LIST` command ([#2571](https://github.com/valkey-io/valkey-glide/pull/2571)) * Python: Add `JSON.DEBUG_MEMORY` and `JSON.DEBUG_FIELDS` commands ([#2481](https://github.com/valkey-io/valkey-glide/pull/2481)) * Java: Added `FT.CREATE` ([#2414](https://github.com/valkey-io/valkey-glide/pull/2414)) @@ -72,7 +72,7 @@ * Python: Add `JSON.STRAPPEND`, `JSON.STRLEN` commands ([#2372](https://github.com/valkey-io/valkey-glide/pull/2372)) * Node: Added `JSON.ARRINDEX` ([#2559](https://github.com/valkey-io/valkey-glide/pull/2559)) * Node: Added `JSON.OBJLEN` and `JSON.OBJKEYS` ([#2563](https://github.com/valkey-io/valkey-glide/pull/2563)) -* Python: Add `JSON.STRAPPEND` , `JSON.STRLEN` commands ([#2372](https://github.com/valkey-io/valkey-glide/pull/2372)) +* Python: Add `JSON.STRAPPEND`, `JSON.STRLEN` commands ([#2372](https://github.com/valkey-io/valkey-glide/pull/2372)) * Python: Add `JSON.OBJKEYS` command ([#2395](https://github.com/valkey-io/valkey-glide/pull/2395)) * Python: Add `JSON.ARRINSERT` command ([#2464](https://github.com/valkey-io/valkey-glide/pull/2464)) * Python: Add `JSON.ARRTRIM` command ([#2457](https://github.com/valkey-io/valkey-glide/pull/2457)) @@ -89,7 +89,7 @@ * Core: Fix list of readonly commands ([#2634](https://github.com/valkey-io/valkey-glide/pull/2634), [#2649](https://github.com/valkey-io/valkey-glide/pull/2649)) * Core: Improve retry logic and update unmaintained dependencies for Rust lint CI ([#2673](https://github.com/valkey-io/valkey-glide/pull/2643)) * Core: Release the read lock while creating connections in `refresh_connections` ([#2630](https://github.com/valkey-io/valkey-glide/issues/2630)) -* Core: SlotMap refactor - Added NodesMap, Update the slot map upon MOVED errors ([#2682](https://github.com/valkey-io/valkey-glide/issues/2682)) +* Core: SlotMap refactor - Added NodesMap, Update the slot map upon MOVED errors ([#2682](https://github.com/valkey-io/valkey-glide/issues/2682)) #### Breaking Changes From bcae8750cf267ce4aaa2f575a4ea06b0cbcc8b0b Mon Sep 17 00:00:00 2001 From: Prateek Kumar Date: Mon, 25 Nov 2024 17:28:56 -0800 Subject: [PATCH 21/29] Node: check exports Signed-off-by: Prateek Kumar --- node/package.json | 3 ++- node/tests/ExportedSymbols.test.ts | 11 +++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/node/package.json b/node/package.json index b581123576..b1f2b8d2bf 100644 --- a/node/package.json +++ b/node/package.json @@ -14,7 +14,8 @@ "glide-rs": "file:rust-client", "long": "^5.2.3", "npmignore": "^0.3.1", - "protobufjs": "^7.4.0" + "protobufjs": "^7.4.0", + "@valkey/valkey-glide": "1.1.0" }, "bundleDependencies": [ "glide-rs" diff --git a/node/tests/ExportedSymbols.test.ts b/node/tests/ExportedSymbols.test.ts index 0dfed333d9..8125359595 100644 --- a/node/tests/ExportedSymbols.test.ts +++ b/node/tests/ExportedSymbols.test.ts @@ -1,10 +1,12 @@ -import { it } from "@jest/globals"; +import { beforeAll, it } from "@jest/globals"; +import { describe } from "node:test"; import * as ts from "typescript"; const fs = require('fs/promises'); const f = require('fs'); -var exportedSymbols = require('../npm/glide/index'); +var exportedSymbols = require('@valkey/valkey-glide'); +// var exportedSymbols = []; let i = 0; let filesWithNodeCode: string[] = []; let getActualSymbolsList: any[] = []; @@ -105,6 +107,11 @@ function visit(node: ts.Node, depth = 0) { for (let c of node.getChildren()) { if (c.getText().trim() == "export") { f = 1; + break; + } else if (c.getText().trim().includes("@internal")) { + console.log('depth=' + depth + ", ts.SyntaxKind===" + ts.SyntaxKind[node.kind] + ", name=" + name + ", c.text" + c.getText() + ", @internal"); + f = 0; + break; } } if (f == 1) { From 0e7d1699e31942232cee69b3a1b24102e5d7d06f Mon Sep 17 00:00:00 2001 From: Prateek Kumar Date: Mon, 25 Nov 2024 17:48:24 -0800 Subject: [PATCH 22/29] Node package.json updated Signed-off-by: Prateek Kumar --- node/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/package.json b/node/package.json index b1f2b8d2bf..983a765f88 100644 --- a/node/package.json +++ b/node/package.json @@ -15,7 +15,7 @@ "long": "^5.2.3", "npmignore": "^0.3.1", "protobufjs": "^7.4.0", - "@valkey/valkey-glide": "1.1.0" + "@valkey/valkey-glide": "1.2.0-rc16" }, "bundleDependencies": [ "glide-rs" From cbf93a8f72bf77dac5669b9146822b47068fc437 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 26 Nov 2024 14:19:35 -0800 Subject: [PATCH 23/29] Node: add API test for glide release package Signed-off-by: Andrew Carbonetto --- node/npm/glide/index.ts | 19 +- node/npm/glide/jest.config.js | 32 ++++ node/npm/glide/package.json.tmpl | 76 ++++++++ node/npm/glide/tests/ExportedSymbols.test.ts | 191 +++++++++++++++++++ node/npm/glide/tests/tsconfig.json | 7 + node/package.json | 5 +- node/tests/ExportedSymbols.test.ts | 133 ------------- 7 files changed, 318 insertions(+), 145 deletions(-) create mode 100644 node/npm/glide/jest.config.js create mode 100644 node/npm/glide/package.json.tmpl create mode 100644 node/npm/glide/tests/ExportedSymbols.test.ts create mode 100644 node/npm/glide/tests/tsconfig.json delete mode 100644 node/tests/ExportedSymbols.test.ts diff --git a/node/npm/glide/index.ts b/node/npm/glide/index.ts index 43b80b463d..ed309cbf06 100644 --- a/node/npm/glide/index.ts +++ b/node/npm/glide/index.ts @@ -11,7 +11,8 @@ let globalObject = global as unknown; /* eslint-disable @typescript-eslint/no-require-imports */ function loadNativeBinding() { - let nativeBinding = null; + let nativeBinding = []; + const scope = process.env.scope || "@scope"; switch (platform) { case "linux": @@ -19,13 +20,13 @@ function loadNativeBinding() { case "x64": switch (familySync()) { case GLIBC: - nativeBinding = require("@scope/valkey-glide-linux-x64"); + nativeBinding = require(`${scope}valkey-glide-linux-x64`); break; case MUSL: - nativeBinding = require("@scope/valkey-glide-linux-musl-x64"); + nativeBinding = require(`${scope}valkey-glide-linux-musl-x64`); break; default: - nativeBinding = require("@scope/valkey-glide-linux-x64"); + nativeBinding = require(`${scope}valkey-glide-linux-x64`); break; } @@ -33,13 +34,13 @@ function loadNativeBinding() { case "arm64": switch (familySync()) { case GLIBC: - nativeBinding = require("@scope/valkey-glide-linux-arm64"); + nativeBinding = require(`${scope}valkey-glide-linux-arm64`); break; case MUSL: - nativeBinding = require("@scope/valkey-glide-linux-musl-arm64"); + nativeBinding = require(`${scope}valkey-glide-linux-musl-arm64`); break; default: - nativeBinding = require("@scope/valkey-glide-linux-arm64"); + nativeBinding = require(`${scope}valkey-glide-linux-arm64`); break; } @@ -54,10 +55,10 @@ function loadNativeBinding() { case "darwin": switch (arch) { case "x64": - nativeBinding = require("@scope/valkey-glide-darwin-x64"); + nativeBinding = require(`${scope}valkey-glide-darwin-x64`); break; case "arm64": - nativeBinding = require("@scope/valkey-glide-darwin-arm64"); + nativeBinding = require(`${scope}valkey-glide-darwin-arm64`); break; default: throw new Error( diff --git a/node/npm/glide/jest.config.js b/node/npm/glide/jest.config.js new file mode 100644 index 0000000000..0a09bcecaa --- /dev/null +++ b/node/npm/glide/jest.config.js @@ -0,0 +1,32 @@ +/* eslint no-undef: off */ +module.exports = { + preset: "ts-jest", + transform: { + "^.+\\.(t|j)s$": ["ts-jest", { isolatedModules: true }], + }, + testEnvironment: "node", + testRegex: "/tests/.*\\.(test|spec)?\\.(ts|tsx)$", + moduleFileExtensions: [ + "ts", + "tsx", + "js", + "jsx", + "json", + "node", + "cjs", + "mjs", + ], + testTimeout: 600000, + reporters: [ + "default", + [ + "./node_modules/jest-html-reporter", + { + includeFailureMsg: true, + includeSuiteFailure: true, + executionTimeWarningThreshold: 60, + sort: "status", + }, + ], + ] +}; diff --git a/node/npm/glide/package.json.tmpl b/node/npm/glide/package.json.tmpl new file mode 100644 index 0000000000..1705958c5c --- /dev/null +++ b/node/npm/glide/package.json.tmpl @@ -0,0 +1,76 @@ +{ + "name": "${scope}${pkg_name}", + "types": "build-ts/index.d.ts", + "version": "${package_version}", + "description": "General Language Independent Driver for the Enterprise (GLIDE) for Valkey", + "main": "build-ts/index.js", + "module": "build-ts/index.js", + "type": "commonjs", + "scripts": { + "lint": "eslint .", + "lint:fix": "eslint . --fix", + "clean": "rm -rf build-ts/", + "copy-declaration-files": "cp ../../build-ts/*.d.ts build-ts/ && cp ../../build-ts/src/*.d.ts build-ts/src/ && cp ../../build-ts/src/server-modules/*.d.ts build-ts/src/server-modules/", + "build": "tsc && mkdir -p build-ts/src && npm run copy-declaration-files", + "build:test": "npm run build:package && npm i && npm run build", + "build:package": "pkg_name=valkey-glide && package_version=99.99.0 && scope=@valkey/ && envsubst < package.json.tmpl > \"package.json\"", + "test": "jest --verbose" + }, + "files": [ + "/build-ts" + ], + "repository": { + "type": "git", + "url": "git+https://github.com/valkey-io/valkey-glide.git" + }, + "keywords": [ + "valkey", + "valkeyClient", + "client", + "valkey-glide" + ], + "author": "Valkey GLIDE Maintainers", + "license": "Apache-2.0", + "bugs": { + "url": "https://github.com/valkey-io/valkey-glide/issues" + }, + "homepage": "https://github.com/valkey-io/valkey-glide#readme", + "devDependencies": { + "@jest/globals": "^29.7.0", + "@types/jest": "^29.5.14", + "ts-jest": "^29.2.5", + "jest": "^29.7.0", + "jest-html-reporter": "^3.10.2", + "@types/node": "^18.11.18", + "@typescript-eslint/eslint-plugin": "^5.48.0", + "@typescript-eslint/parser": "^5.48.0", + "eslint": "^8.31.0", + "typescript": "^4.9.4", + "@valkey/valkey-glide-impl": "../.." + }, + "optionalDependencies": { + "${scope}valkey-glide-darwin-arm64": "${package_version}", + "${scope}valkey-glide-darwin-x64": "${package_version}", + "${scope}valkey-glide-linux-arm64": "${package_version}", + "${scope}valkey-glide-linux-x64": "${package_version}", + "${scope}valkey-glide-linux-musl-arm64": "${package_version}", + "${scope}valkey-glide-linux-musl-x64": "${package_version}" + }, + "eslintConfig": { + "extends": [ + "eslint:recommended", + "plugin:@typescript-eslint/recommended" + ], + "parser": "@typescript-eslint/parser", + "plugins": [ + "@typescript-eslint" + ], + "ignorePatterns": [ + "build-ts/*" + ], + "root": true + }, + "dependencies": { + "detect-libc": "^2.0.3" + } +} diff --git a/node/npm/glide/tests/ExportedSymbols.test.ts b/node/npm/glide/tests/ExportedSymbols.test.ts new file mode 100644 index 0000000000..1ff034d88d --- /dev/null +++ b/node/npm/glide/tests/ExportedSymbols.test.ts @@ -0,0 +1,191 @@ +import { beforeAll, it } from "@jest/globals"; +import * as f from 'fs/promises'; +import { describe } from "node:test"; +import * as ts from "typescript"; +import * as glideApi from "../"; + +describe("Exported Symbols test", () => { + + beforeAll(() => { + /** + * Add Excluded symbols + * Example: + * excludedSymbols.push('convertGlideRecord'); + */ + }); + + it("check excluded symbols are not exported", async () => { + + // Check exported symbols for valkey glide package + const exportedSymbolsList = Object.keys(glideApi).sort(); // exportedList + console.log(exportedSymbolsList); + console.log(exportedSymbolsList.length); + + const testFolder = './build-ts/'; + const filesWithNodeCode = await getFiles(testFolder); + console.log(filesWithNodeCode); + + const actualExported = []; + + for (const file of filesWithNodeCode) { + const sourceCode = await f.readFile(file, 'utf8'); + const sourceFile = await ts.createSourceFile(file, sourceCode, ts.ScriptTarget.Latest, true); + actualExported.push(...visitRoot(sourceFile)); + } + + actualExported.sort(); + console.log(actualExported); + console.log(actualExported.length); + + expect(exportedSymbolsList).toEqual(actualExported); + }); +}); + + +// function getFiles(folderName: fs.PathLike): string[] { +// const files = fs.readdirSync(folderName).filter(file => file.endsWith('.ts') && !file.endsWith('.d.ts')); +// return files; +// } + +async function getFiles(folderName: string): Promise { + // console.log(folderName); + + const files = await f.readdir(folderName, { withFileTypes: true }); + + // const skipFolders = [ + // 'build-ts', + // 'commonjs-test', + // 'glide-logs', + // 'hybrid-node-tests', + // 'node_modules', + // 'npm', + // '.cargo', + // 'target', + // 'tests' + // ]; + + const filesWithNodeCode = []; + + for (const file of files) { + if (file.isDirectory()) { + // if (skipFolders.includes(file.name)) { + // continue; + // } + + filesWithNodeCode.push(...(await getFiles(folderName + file.name + '/'))); + } else { + // console.log("not a dir: " + file.name); + + // if (file.name.endsWith('.js') || + // file.name.endsWith('.d.ts') || + // file.name.endsWith('.json') || + // file.name.endsWith('.rs') || + // file.name.endsWith('.html') || + // file.name.endsWith('.node') || + // file.name.endsWith('.lock') || + // file.name.endsWith('.toml') || + // file.name.endsWith('.yml') || + // file.name.endsWith('.rdb') || + // file.name.endsWith('.md') || + // file.name.localeCompare('.gitignore') == 0 || + // file.name.localeCompare('.prettierignore') == 0 || + // file.name.localeCompare('THIRD_PARTY_LICENSES_NODE') == 0 || + // file.name.localeCompare('index.ts') == 0) { + // continue; + // } + + if (!file.name.endsWith('.d.ts')) { + continue; + } + + // i++; + filesWithNodeCode.push(folderName + file.name); + } + } + + return filesWithNodeCode; +} + +function visitRoot(root: ts.Node) { + const children: ts.Node[] = root.getChildren(); + + const resultList: string[] = []; + + for (const node of children) { + const nodeList: string[] = node.getChildren().map(c => visit(c)).filter(c => c !== undefined); + + if (nodeList.length > 0) { + resultList.push(...nodeList); + } + + console.log(resultList); + } + + return resultList; +} + +function visit(node: ts.Node) { + let name: string | undefined = ""; + + switch (node.kind) { + case ts.SyntaxKind.FirstStatement: + case ts.SyntaxKind.ExportDeclaration: + case ts.SyntaxKind.ExportAssignment: + case ts.SyntaxKind.ImportDeclaration: + return; + } + + // list of kind we like: + // InterfaceDeclaration + // FunctionDeclaration + + if (ts.isFunctionDeclaration(node)) { + name = node.name?.text; + } else if (ts.isVariableStatement(node)) { + name = ""; + } else if (ts.isInterfaceDeclaration(node)) { + name = node.name?.text; + } else if (ts.isClassDeclaration(node)) { + name = node.name?.text; + } else if (ts.isTypeAliasDeclaration(node)) { + name = node.name?.text; + } else if (ts.isEnumDeclaration(node)) { + name = node.name?.text; + } else if (ts.isModuleDeclaration(node)) { + name = node.name?.text; + } + + const debug: string[] = []; + const children = node.getChildren(); + const isInternal = children.find(c => (ts.SyntaxKind[c.kind] == "JSDocComment"))?.getText().includes('@internal'); + const isExported = children.find(c => (ts.SyntaxKind[c.kind] == "SyntaxList"))?.getChildren().find(c => (ts.SyntaxKind[c.kind] == "ExportKeyword")); + + if (isExported && !isInternal) { + // debug.push('depth=' + depth + ", ts.SyntaxKind===" + ts.SyntaxKind[node.kind] + ", name=" + name); + // if (name) { + // debug.push(`name=${name} kind=${ts.SyntaxKind[node.kind]}`); + // } else { + // debug.push(`name=unnamed kind=${ts.SyntaxKind[node.kind]}`); + // } + + // console.log(debug); + return name; + } + + if (isExported && isInternal) { + // marked correctly... no-op + } + + if (!isExported && isInternal) { + // no-op + } + + if (!isExported && !isInternal) { + // these are okay for now... + // debug.push(`PRIVATE??? name=unnamed kind=${ts.SyntaxKind[node.kind]} text=${node.getText()}`); + } + + // if (debug.length > 0) { + // console.log(debug); + // } +} diff --git a/node/npm/glide/tests/tsconfig.json b/node/npm/glide/tests/tsconfig.json new file mode 100644 index 0000000000..2a8aaebf40 --- /dev/null +++ b/node/npm/glide/tests/tsconfig.json @@ -0,0 +1,7 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "rootDir": "../" + }, + "include": ["./*.test.ts"] +} diff --git a/node/package.json b/node/package.json index 983a765f88..81d3133bc9 100644 --- a/node/package.json +++ b/node/package.json @@ -1,5 +1,5 @@ { - "name": "@valkey/valkey-glide", + "name": "@valkey/valkey-glide-impl", "description": "General Language Independent Driver for the Enterprise (GLIDE) for Valkey", "main": "build-ts/index.js", "module": "build-ts/index.js", @@ -14,8 +14,7 @@ "glide-rs": "file:rust-client", "long": "^5.2.3", "npmignore": "^0.3.1", - "protobufjs": "^7.4.0", - "@valkey/valkey-glide": "1.2.0-rc16" + "protobufjs": "^7.4.0" }, "bundleDependencies": [ "glide-rs" diff --git a/node/tests/ExportedSymbols.test.ts b/node/tests/ExportedSymbols.test.ts deleted file mode 100644 index 8125359595..0000000000 --- a/node/tests/ExportedSymbols.test.ts +++ /dev/null @@ -1,133 +0,0 @@ -import { beforeAll, it } from "@jest/globals"; -import { describe } from "node:test"; -import * as ts from "typescript"; - -const fs = require('fs/promises'); -const f = require('fs'); - -var exportedSymbols = require('@valkey/valkey-glide'); -// var exportedSymbols = []; -let i = 0; -let filesWithNodeCode: string[] = []; -let getActualSymbolsList: any[] = []; - -describe("Exported Symbols test", () => { - var excludedSymbolList: string[] = []; - beforeAll(() => { - /** - * Add Excluded symbols - * Example: - * excludedSymbols.push('convertGlideRecord'); - */ - }); - it("check excluded symbols are not exported", async () => { - // Check exported symbols for valkey glide package - let exportedSymbolsList = Object.keys(exportedSymbols); // exportedList - // const filteredExportedList = exportedSymbolsList.filter(symbol => excludedSymbolList.includes(symbol)); - // console.log("Following symbols are exported but are in the exlcuded list, please remove: " + filteredExportedList); - // expect(filteredExportedList.length).toBe(0); - - const testFolder = './'; - await getFiles(testFolder); - console.log('Total files found =' + i); // 10 - console.log(filesWithNodeCode); //[] - for (let file of filesWithNodeCode) { - const sourceCode = await f.readFileSync(file, 'utf8'); - const sourceFile = await ts.createSourceFile(file, sourceCode, ts.ScriptTarget.Latest, true); - visit(sourceFile, 0); - } - console.log("actual exports in the source code ="); - //console.log(getActualSymbolsList); - console.log("Total exported symbols in index=" + exportedSymbolsList.length); - console.log("Total symbols in the source code=" + getActualSymbolsList.length); - // console.log(exportedSymbolsList); - let l = getActualSymbolsList.filter(actualSymbol => !exportedSymbolsList.includes(actualSymbol)); - console.log("Total missed symbols=" + l.length); - console.log(l); - //1. Test if actualSymbolList - exportedSymbolsList = excludedSymbolList - //2. If actualSymbolList - exportedSymbolsList != excludedSymbolList, - // throw an error that either some symbol not exported or there is some error in the excluded symbol list. - }); -}); - -const skipFolders = ['build-ts', 'commonjs-test', 'glide-logs', 'hybrid-node-tests', 'node_modules', 'npm', '.cargo', 'target', 'tests']; -async function getFiles(folderName: string) { - const files = await fs.readdir(folderName, { withFileTypes: true }); - for (let file of files) { - if (file.isDirectory()) { - if (skipFolders.includes(file.name)) { - continue; - } - await getFiles(folderName + file.name + '/'); - } else { - if (file.name.endsWith('.js') || - file.name.endsWith('.d.ts') || - file.name.endsWith('.json') || - file.name.endsWith('.rs') || - file.name.endsWith('.html') || - file.name.endsWith('.node') || - file.name.endsWith('.lock') || - file.name.endsWith('.toml') || - file.name.endsWith('.yml') || - file.name.endsWith('.rdb') || - file.name.endsWith('.md') || - file.name.localeCompare('.gitignore') == 0 || - file.name.localeCompare('.prettierignore') == 0 || - file.name.localeCompare('THIRD_PARTY_LICENSES_NODE') == 0 || - file.name.localeCompare('index.ts') == 0) { - continue; - } - i++; - filesWithNodeCode.push(folderName + file.name); - } - } -} - -function visit(node: ts.Node, depth = 0) { - if (depth == 2) { - let name: string | undefined = ""; - if (ts.isFunctionDeclaration(node)) { - name = node.name?.text; - } else if (ts.isVariableStatement(node)) { - name = ""; - } else if (ts.isInterfaceDeclaration(node)) { - name = node.name?.text; - } else if (ts.isClassDeclaration(node)) { - name = node.name?.text; - } else if (ts.isTypeAliasDeclaration(node)) { - name = node.name?.text; - } else if (ts.isEnumDeclaration(node)) { - name = node.name?.text; - } else if (ts.isModuleDeclaration(node)) { - name = node.name?.text; - } else if (ts.SyntaxKind[node.kind] == "FirstStatement") { - name = node.name?.text; - } - let f = 0; - for (let c of node.getChildren()) { - if (c.getText().trim() == "export") { - f = 1; - break; - } else if (c.getText().trim().includes("@internal")) { - console.log('depth=' + depth + ", ts.SyntaxKind===" + ts.SyntaxKind[node.kind] + ", name=" + name + ", c.text" + c.getText() + ", @internal"); - f = 0; - break; - } - } - if (f == 1) { - if (name == '') { - console.log('depth=' + depth + ", ts.SyntaxKind===" + ts.SyntaxKind[node.kind] + ", name=" + name); - console.log(node.getText()); - } - getActualSymbolsList.push(name); - - } else { - // console.log('depth=' + depth + ", ts.SyntaxKind===" + ts.SyntaxKind[node.kind] + ", name=" + name + " ---not exported"); - } - } - if (depth <= 1) { - for (let c of node.getChildren()) { - visit(c, depth + 1); - } - } -} From 385bbaefd740c7d874f6866717c3a0d50a56b3f4 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 26 Nov 2024 14:40:29 -0800 Subject: [PATCH 24/29] Fix build script Signed-off-by: Andrew Carbonetto --- node/npm/glide/package.json.tmpl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/node/npm/glide/package.json.tmpl b/node/npm/glide/package.json.tmpl index 1705958c5c..2c0ca3cedb 100644 --- a/node/npm/glide/package.json.tmpl +++ b/node/npm/glide/package.json.tmpl @@ -12,8 +12,7 @@ "clean": "rm -rf build-ts/", "copy-declaration-files": "cp ../../build-ts/*.d.ts build-ts/ && cp ../../build-ts/src/*.d.ts build-ts/src/ && cp ../../build-ts/src/server-modules/*.d.ts build-ts/src/server-modules/", "build": "tsc && mkdir -p build-ts/src && npm run copy-declaration-files", - "build:test": "npm run build:package && npm i && npm run build", - "build:package": "pkg_name=valkey-glide && package_version=99.99.0 && scope=@valkey/ && envsubst < package.json.tmpl > \"package.json\"", + "build:test": "npm i && npm run build", "test": "jest --verbose" }, "files": [ From a48d8b8af687382237d177f80c6608cb1e0d3540 Mon Sep 17 00:00:00 2001 From: Prateek Kumar Date: Tue, 26 Nov 2024 17:27:46 -0800 Subject: [PATCH 25/29] Node: Update loadNativeBinding() in index.ts Signed-off-by: Prateek Kumar --- node/npm/glide/index.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/node/npm/glide/index.ts b/node/npm/glide/index.ts index ed309cbf06..3d5a5ab83b 100644 --- a/node/npm/glide/index.ts +++ b/node/npm/glide/index.ts @@ -11,9 +11,14 @@ let globalObject = global as unknown; /* eslint-disable @typescript-eslint/no-require-imports */ function loadNativeBinding() { - let nativeBinding = []; const scope = process.env.scope || "@scope"; + if (process.env.JEST_WORKER_ID !== undefined) { + return require(`${scope}valkey-glide-impl`); + } + + let nativeBinding = [] + switch (platform) { case "linux": switch (arch) { From 0232cc88d89398edcc7f34b5ee55be044e3bbc87 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 26 Nov 2024 17:43:52 -0800 Subject: [PATCH 26/29] Additional cleanup for Node package builder Signed-off-by: Andrew Carbonetto --- node/DEVELOPER.md | 18 +++ node/npm/glide/index.ts | 127 +++++++++---------- node/npm/glide/tests/ExportedSymbols.test.ts | 60 ++++----- node/rust-client/src/lib.rs | 9 ++ node/src/BaseClient.ts | 4 + node/src/Commands.ts | 3 +- node/src/Transaction.ts | 2 +- node/src/server-modules/GlideFtOptions.ts | 2 +- 8 files changed, 121 insertions(+), 104 deletions(-) diff --git a/node/DEVELOPER.md b/node/DEVELOPER.md index 8878fdd91d..733eaa34b8 100644 --- a/node/DEVELOPER.md +++ b/node/DEVELOPER.md @@ -107,6 +107,24 @@ Before starting this step, make sure you've installed all software requirments. 5. Integrating the built GLIDE package into your project: Add the package to your project using the folder path with the command `npm install /node`. +6. Testing the GLIDE npm package: + The `node/npm/glide` folder contains a package wrapper that re-exports all the native bindings. To build and test this package, follow these steps: + ```bash + cd node; + npm run build; # build the @valkey/valkey-glide-impl package + cd npm/glide; # navigate to the npm package directory + + export scope=@valkey/ + export pkg_name=valkey-glide + export package_version=99.99.0 # set the version as desired + export native_binding=impl # to run against the local build at ../.. + envsubst < package.json.tmpl > "package.json" # only needs to be run when the env variables change + + npm i + npm run build:test + npm test + ``` + - For a fast build, execute `npm run build`. This will perform a full, unoptimized build, which is suitable for developing tests. Keep in mind that performance is significantly affected in an unoptimized build, so it's required to build with the `build:release` or `build:benchmark` option when measuring performance. - If your modifications are limited to the TypeScript code, run `npm run build-external` to build the external package without rebuilding the internal package. - If your modifications are limited to the Rust code, execute `npm run build-internal` to build the internal package and generate TypeScript code. diff --git a/node/npm/glide/index.ts b/node/npm/glide/index.ts index 3d5a5ab83b..277c6b0412 100644 --- a/node/npm/glide/index.ts +++ b/node/npm/glide/index.ts @@ -13,71 +13,68 @@ let globalObject = global as unknown; function loadNativeBinding() { const scope = process.env.scope || "@scope"; - if (process.env.JEST_WORKER_ID !== undefined) { - return require(`${scope}valkey-glide-impl`); - } - - let nativeBinding = [] + let nativeBinding = []; + let nativeStr = process.env.native_binding; - switch (platform) { - case "linux": - switch (arch) { - case "x64": - switch (familySync()) { - case GLIBC: - nativeBinding = require(`${scope}valkey-glide-linux-x64`); - break; - case MUSL: - nativeBinding = require(`${scope}valkey-glide-linux-musl-x64`); - break; - default: - nativeBinding = require(`${scope}valkey-glide-linux-x64`); - break; - } + if (nativeStr == undefined) { + switch (platform) { + case "linux": + switch (arch) { + case "x64": + switch (familySync()) { + case MUSL: + nativeStr = "linux-musl-x64"; + break; + case GLIBC: + default: + nativeStr = "linux-x64"; + break; + } - break; - case "arm64": - switch (familySync()) { - case GLIBC: - nativeBinding = require(`${scope}valkey-glide-linux-arm64`); - break; - case MUSL: - nativeBinding = require(`${scope}valkey-glide-linux-musl-arm64`); - break; - default: - nativeBinding = require(`${scope}valkey-glide-linux-arm64`); - break; - } + break; + case "arm64": + switch (familySync()) { + case MUSL: + nativeStr = "linux-musl-arm64"; + break; + case GLIBC: + default: + nativeStr = "linux-arm64"; + break; + } - break; - default: - throw new Error( - `Unsupported OS: ${platform}, architecture: ${arch}`, - ); - } + break; + default: + throw new Error( + `Unsupported OS: ${platform}, architecture: ${arch}`, + ); + } - break; - case "darwin": - switch (arch) { - case "x64": - nativeBinding = require(`${scope}valkey-glide-darwin-x64`); - break; - case "arm64": - nativeBinding = require(`${scope}valkey-glide-darwin-arm64`); - break; - default: - throw new Error( - `Unsupported OS: ${platform}, architecture: ${arch}`, - ); - } + break; + case "darwin": + switch (arch) { + case "x64": + nativeStr = "darwin-x64"; + break; + case "arm64": + nativeStr = "darwin-arm64"; + break; + default: + throw new Error( + `Unsupported OS: ${platform}, architecture: ${arch}`, + ); + } - break; - default: - throw new Error( - `Unsupported OS: ${platform}, architecture: ${arch}`, - ); + break; + default: + throw new Error( + `Unsupported OS: ${platform}, architecture: ${arch}`, + ); + } } + nativeBinding = require(`${scope}valkey-glide-${nativeStr}`); + if (!nativeBinding) { throw new Error(`Failed to load native binding`); } @@ -90,6 +87,7 @@ function initialize() { const { AggregationType, BaseScanOptions, + ScanOptions, ZScanOptions, HScanOptions, BitEncoding, @@ -125,6 +123,7 @@ function initialize() { GlideClientConfiguration, GlideJson, GlideFt, + Field, TextField, TagField, NumericField, @@ -144,7 +143,6 @@ function initialize() { FtAggregateApply, FtAggregateReturnType, FtSearchReturnType, - FtProfileOtions, GlideRecord, GlideString, JsonGetOptions, @@ -181,16 +179,13 @@ function initialize() { InfBoundary, KeyWeight, Boundary, - UpdateOptions, ProtocolVersion, RangeByIndex, RangeByScore, RangeByLex, ReadFrom, ServerCredentials, - SortClusterOptions, SortOptions, - SortedSetRange, StreamGroupOptions, StreamTrimOptions, StreamAddOptions, @@ -221,7 +216,6 @@ function initialize() { createLeakedDouble, createLeakedMap, createLeakedString, - parseInfoResponse, Script, ObjectType, ClusterScanCursor, @@ -239,6 +233,7 @@ function initialize() { module.exports = { AggregationType, BaseScanOptions, + ScanOptions, HScanOptions, ZScanOptions, BitEncoding, @@ -259,6 +254,7 @@ function initialize() { DecoderOption, GeoAddOptions, GlideFt, + Field, TextField, TagField, NumericField, @@ -278,7 +274,6 @@ function initialize() { FtAggregateApply, FtAggregateReturnType, FtSearchReturnType, - FtProfileOtions, GlideRecord, GlideJson, GlideString, @@ -332,16 +327,13 @@ function initialize() { InfBoundary, KeyWeight, Boundary, - UpdateOptions, ProtocolVersion, RangeByIndex, RangeByScore, RangeByLex, ReadFrom, ServerCredentials, - SortClusterOptions, SortOptions, - SortedSetRange, StreamGroupOptions, StreamTrimOptions, StreamAddOptions, @@ -370,7 +362,6 @@ function initialize() { createLeakedDouble, createLeakedMap, createLeakedString, - parseInfoResponse, Script, ObjectType, ClusterScanCursor, diff --git a/node/npm/glide/tests/ExportedSymbols.test.ts b/node/npm/glide/tests/ExportedSymbols.test.ts index 1ff034d88d..34a243debd 100644 --- a/node/npm/glide/tests/ExportedSymbols.test.ts +++ b/node/npm/glide/tests/ExportedSymbols.test.ts @@ -1,4 +1,4 @@ -import { beforeAll, it } from "@jest/globals"; +import { it } from "@jest/globals"; import * as f from 'fs/promises'; import { describe } from "node:test"; import * as ts from "typescript"; @@ -6,38 +6,32 @@ import * as glideApi from "../"; describe("Exported Symbols test", () => { - beforeAll(() => { - /** - * Add Excluded symbols - * Example: - * excludedSymbols.push('convertGlideRecord'); - */ - }); - it("check excluded symbols are not exported", async () => { // Check exported symbols for valkey glide package const exportedSymbolsList = Object.keys(glideApi).sort(); // exportedList - console.log(exportedSymbolsList); - console.log(exportedSymbolsList.length); + // console.log(exportedSymbolsList); + // console.log(exportedSymbolsList.length); - const testFolder = './build-ts/'; - const filesWithNodeCode = await getFiles(testFolder); + const implBuildFolder = './build-ts/'; + // const rustClientBuildFolder = './node_modules/@valkey/valkey-glide-impl/rust-client/'; + // const filesWithNodeCode = [...await getFiles(implBuildFolder), ...await getFiles(rustClientBuildFolder)]; + const filesWithNodeCode = await getFiles(implBuildFolder); console.log(filesWithNodeCode); - const actualExported = []; + const internallyExported = []; for (const file of filesWithNodeCode) { const sourceCode = await f.readFile(file, 'utf8'); const sourceFile = await ts.createSourceFile(file, sourceCode, ts.ScriptTarget.Latest, true); - actualExported.push(...visitRoot(sourceFile)); + internallyExported.push(...visitRoot(sourceFile)); } - actualExported.sort(); - console.log(actualExported); - console.log(actualExported.length); + internallyExported.sort(); + // console.log(internallyExported); + // console.log(internallyExported.length); - expect(exportedSymbolsList).toEqual(actualExported); + expect(exportedSymbolsList).toEqual(internallyExported); }); }); @@ -52,25 +46,25 @@ async function getFiles(folderName: string): Promise { const files = await f.readdir(folderName, { withFileTypes: true }); - // const skipFolders = [ - // 'build-ts', - // 'commonjs-test', - // 'glide-logs', - // 'hybrid-node-tests', - // 'node_modules', - // 'npm', - // '.cargo', - // 'target', - // 'tests' - // ]; + const skipFolders = [ + // 'build-ts', + 'commonjs-test', + 'glide-logs', + 'hybrid-node-tests', + 'node_modules', + 'npm', + '.cargo', + 'target', + 'tests' + ]; const filesWithNodeCode = []; for (const file of files) { if (file.isDirectory()) { - // if (skipFolders.includes(file.name)) { - // continue; - // } + if (skipFolders.includes(file.name)) { + continue; + } filesWithNodeCode.push(...(await getFiles(folderName + file.name + '/'))); } else { diff --git a/node/rust-client/src/lib.rs b/node/rust-client/src/lib.rs index b15b18f521..a43e0235ce 100644 --- a/node/rust-client/src/lib.rs +++ b/node/rust-client/src/lib.rs @@ -307,6 +307,7 @@ fn split_pointer(pointer: *mut T) -> [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test /// This function is for tests that require a value allocated on the heap. /// Should NOT be used in production. #[cfg(feature = "testing_utilities")] @@ -317,6 +318,9 @@ pub fn create_leaked_string(message: String) -> [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test +/// This function is for tests that require a value allocated on the heap. +/// Should NOT be used in production. pub fn create_leaked_string_vec(message: Vec) -> [u32; 2] { // Convert the string vec -> Bytes vector let bytes_vec: Vec = message.iter().map(|v| Bytes::from(v.to_vec())).collect(); @@ -325,6 +329,7 @@ pub fn create_leaked_string_vec(message: Vec) -> [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test /// This function is for tests that require a value allocated on the heap. /// Should NOT be used in production. #[cfg(feature = "testing_utilities")] @@ -338,6 +343,7 @@ pub fn create_leaked_map(map: HashMap) -> [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test /// This function is for tests that require a value allocated on the heap. /// Should NOT be used in production. #[cfg(feature = "testing_utilities")] @@ -349,6 +355,7 @@ pub fn create_leaked_array(array: Vec) -> [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test /// This function is for tests that require a value allocated on the heap. /// Should NOT be used in production. #[cfg(feature = "testing_utilities")] @@ -364,6 +371,7 @@ pub fn create_leaked_attribute(message: String, attribute: HashMap [u32; 2] { } #[napi(ts_return_type = "[number, number]")] +/// @internal @test /// This function is for tests that require a value allocated on the heap. /// Should NOT be used in production. #[cfg(feature = "testing_utilities")] diff --git a/node/src/BaseClient.ts b/node/src/BaseClient.ts index 71b1ffd89c..97282fab85 100644 --- a/node/src/BaseClient.ts +++ b/node/src/BaseClient.ts @@ -777,6 +777,10 @@ export interface PubSubMsg { */ export type WritePromiseOptions = RouteOption & DecoderOption; +/** + * @internal + * Base client interface for GLIDE +*/ export class BaseClient { private socket: net.Socket; protected readonly promiseCallbackFunctions: [ diff --git a/node/src/Commands.ts b/node/src/Commands.ts index 8411cf212d..e466527285 100644 --- a/node/src/Commands.ts +++ b/node/src/Commands.ts @@ -54,7 +54,7 @@ function toBuffersArray(args: GlideString[]) { } /** - * @internal + * @internal @test */ export function parseInfoResponse(response: string): Record { const lines = response.split("\n"); @@ -418,6 +418,7 @@ export function createHGet( } /** + * @internal * This function converts an input from {@link HashDataType} or `Record` types to `HashDataType`. * * @param fieldsAndValues - field names and their values. diff --git a/node/src/Transaction.ts b/node/src/Transaction.ts index 485d7f06d0..0851ae8141 100644 --- a/node/src/Transaction.ts +++ b/node/src/Transaction.ts @@ -288,7 +288,7 @@ import { command_request } from "./ProtobufMessage"; * console.log(result); // Output: ['OK', 'value'] * ``` */ -export class BaseTransaction> { +class BaseTransaction> { /** * @internal */ diff --git a/node/src/server-modules/GlideFtOptions.ts b/node/src/server-modules/GlideFtOptions.ts index 6d9e8f4528..e5c16e0e7d 100644 --- a/node/src/server-modules/GlideFtOptions.ts +++ b/node/src/server-modules/GlideFtOptions.ts @@ -58,7 +58,7 @@ export type VectorField = BaseField & { /** * Base class for defining vector field attributes to be used after the vector algorithm name. */ -export interface VectorFieldAttributes { +interface VectorFieldAttributes { /** Number of dimensions in the vector. Equivalent to `DIM` in the module API. */ dimensions: number; /** From 10cf60de66ed62dc19aa5076ff9dac9ce3ddfb3d Mon Sep 17 00:00:00 2001 From: Prateek Kumar Date: Tue, 26 Nov 2024 18:13:55 -0800 Subject: [PATCH 27/29] Node: branch rebased Signed-off-by: Prateek Kumar --- node/npm/glide/package.json.tmpl | 2 +- node/npm/glide/tests/ExportedSymbols.test.ts | 41 +++++++++++--------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/node/npm/glide/package.json.tmpl b/node/npm/glide/package.json.tmpl index 2c0ca3cedb..8fccdc9510 100644 --- a/node/npm/glide/package.json.tmpl +++ b/node/npm/glide/package.json.tmpl @@ -11,7 +11,7 @@ "lint:fix": "eslint . --fix", "clean": "rm -rf build-ts/", "copy-declaration-files": "cp ../../build-ts/*.d.ts build-ts/ && cp ../../build-ts/src/*.d.ts build-ts/src/ && cp ../../build-ts/src/server-modules/*.d.ts build-ts/src/server-modules/", - "build": "tsc && mkdir -p build-ts/src && npm run copy-declaration-files", + "build": "tsc && mkdir -p build-ts/src && mkdir -p build-ts/src/server-modules && npm run copy-declaration-files", "build:test": "npm i && npm run build", "test": "jest --verbose" }, diff --git a/node/npm/glide/tests/ExportedSymbols.test.ts b/node/npm/glide/tests/ExportedSymbols.test.ts index 34a243debd..3a98940b02 100644 --- a/node/npm/glide/tests/ExportedSymbols.test.ts +++ b/node/npm/glide/tests/ExportedSymbols.test.ts @@ -10,8 +10,6 @@ describe("Exported Symbols test", () => { // Check exported symbols for valkey glide package const exportedSymbolsList = Object.keys(glideApi).sort(); // exportedList - // console.log(exportedSymbolsList); - // console.log(exportedSymbolsList.length); const implBuildFolder = './build-ts/'; // const rustClientBuildFolder = './node_modules/@valkey/valkey-glide-impl/rust-client/'; @@ -20,7 +18,7 @@ describe("Exported Symbols test", () => { console.log(filesWithNodeCode); const internallyExported = []; - + for (const file of filesWithNodeCode) { const sourceCode = await f.readFile(file, 'utf8'); const sourceFile = await ts.createSourceFile(file, sourceCode, ts.ScriptTarget.Latest, true); @@ -28,9 +26,14 @@ describe("Exported Symbols test", () => { } internallyExported.sort(); - // console.log(internallyExported); - // console.log(internallyExported.length); + let missingSymbols = internallyExported.filter((e: string) => !exportedSymbolsList.includes(e)); + let doesNotExistExports = exportedSymbolsList.filter((e: string | any) => !internallyExported.includes(e)); + + console.log("Missing exports="); + console.log(missingSymbols); + console.log("Exports does not exist="); + console.log(doesNotExistExports); expect(exportedSymbolsList).toEqual(internallyExported); }); }); @@ -48,19 +51,19 @@ async function getFiles(folderName: string): Promise { const skipFolders = [ // 'build-ts', - 'commonjs-test', - 'glide-logs', - 'hybrid-node-tests', - 'node_modules', - 'npm', - '.cargo', - 'target', + 'commonjs-test', + 'glide-logs', + 'hybrid-node-tests', + 'node_modules', + 'npm', + '.cargo', + 'target', 'tests' ]; const filesWithNodeCode = []; - for (const file of files) { + for (const file of files) { if (file.isDirectory()) { if (skipFolders.includes(file.name)) { continue; @@ -96,7 +99,7 @@ async function getFiles(folderName: string): Promise { filesWithNodeCode.push(folderName + file.name); } } - + return filesWithNodeCode; } @@ -114,13 +117,13 @@ function visitRoot(root: ts.Node) { console.log(resultList); } - + return resultList; } function visit(node: ts.Node) { let name: string | undefined = ""; - + switch (node.kind) { case ts.SyntaxKind.FirstStatement: case ts.SyntaxKind.ExportDeclaration: @@ -165,11 +168,11 @@ function visit(node: ts.Node) { // console.log(debug); return name; } - + if (isExported && isInternal) { // marked correctly... no-op } - + if (!isExported && isInternal) { // no-op } @@ -178,7 +181,7 @@ function visit(node: ts.Node) { // these are okay for now... // debug.push(`PRIVATE??? name=unnamed kind=${ts.SyntaxKind[node.kind]} text=${node.getText()}`); } - + // if (debug.length > 0) { // console.log(debug); // } From d88e21a5682e4c350c467885a30e54f92ac87a0b Mon Sep 17 00:00:00 2001 From: Prateek Kumar Date: Tue, 26 Nov 2024 18:15:50 -0800 Subject: [PATCH 28/29] Node: update list type Signed-off-by: Prateek Kumar --- node/npm/glide/tests/ExportedSymbols.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/npm/glide/tests/ExportedSymbols.test.ts b/node/npm/glide/tests/ExportedSymbols.test.ts index 3a98940b02..985ecec814 100644 --- a/node/npm/glide/tests/ExportedSymbols.test.ts +++ b/node/npm/glide/tests/ExportedSymbols.test.ts @@ -17,7 +17,7 @@ describe("Exported Symbols test", () => { const filesWithNodeCode = await getFiles(implBuildFolder); console.log(filesWithNodeCode); - const internallyExported = []; + const internallyExported: any = []; for (const file of filesWithNodeCode) { const sourceCode = await f.readFile(file, 'utf8'); From 7cc2b1a896dea7ec52eba32956fa8a399391903a Mon Sep 17 00:00:00 2001 From: Prateek Kumar Date: Tue, 10 Dec 2024 13:54:00 -0800 Subject: [PATCH 29/29] Resolve merge conflicts Signed-off-by: Prateek Kumar --- node/npm/glide/package.json | 16 ++++-- node/npm/glide/package.json.tmpl | 2 +- node/npm/glide/tests/ExportedSymbols.test.ts | 57 +++----------------- 3 files changed, 21 insertions(+), 54 deletions(-) diff --git a/node/npm/glide/package.json b/node/npm/glide/package.json index 78ec8d0821..80e9d87036 100644 --- a/node/npm/glide/package.json +++ b/node/npm/glide/package.json @@ -1,7 +1,7 @@ { - "name": "${scope}${pkg_name}", + "name": "@valkey/valkey-glide", "types": "build-ts/index.d.ts", - "version": "${package_version}", + "version": "99.99.0", "description": "General Language Independent Driver for the Enterprise (GLIDE) for Valkey", "main": "build-ts/index.js", "module": "build-ts/index.js", @@ -11,7 +11,9 @@ "lint:fix": "eslint . --fix", "clean": "rm -rf build-ts/", "copy-declaration-files": "cp ../../build-ts/*.d.ts build-ts/ && cp ../../build-ts/src/*.d.ts build-ts/src/ && cp ../../build-ts/src/server-modules/*.d.ts build-ts/src/server-modules/", - "build": "tsc && mkdir -p build-ts/src && mkdir -p build-ts/src/server-modules && npm run copy-declaration-files" + "build": "tsc && mkdir -p build-ts/src && mkdir -p build-ts/src/server-modules && npm run copy-declaration-files", + "build:test": "npm i && npm run build", + "test": "jest --verbose" }, "files": [ "/build-ts" @@ -33,11 +35,17 @@ }, "homepage": "https://github.com/valkey-io/valkey-glide#readme", "devDependencies": { + "@jest/globals": "^29.7.0", + "@types/jest": "^29.5.14", + "ts-jest": "^29.2.5", + "jest": "^29.7.0", + "jest-html-reporter": "^3.10.2", "@types/node": "^18.11.18", "@typescript-eslint/eslint-plugin": "^5.48.0", "@typescript-eslint/parser": "^5.48.0", "eslint": "^8.31.0", - "typescript": "^4.9.4" + "typescript": "^4.9.4", + "@valkey/valkey-glide-darwin-x64": "../.." }, "optionalDependencies": { "${scope}valkey-glide-darwin-arm64": "${package_version}", diff --git a/node/npm/glide/package.json.tmpl b/node/npm/glide/package.json.tmpl index 8fccdc9510..bc5ae26eb7 100644 --- a/node/npm/glide/package.json.tmpl +++ b/node/npm/glide/package.json.tmpl @@ -45,7 +45,7 @@ "@typescript-eslint/parser": "^5.48.0", "eslint": "^8.31.0", "typescript": "^4.9.4", - "@valkey/valkey-glide-impl": "../.." + "${scope}valkey-glide-${nativeStr}": "../.." }, "optionalDependencies": { "${scope}valkey-glide-darwin-arm64": "${package_version}", diff --git a/node/npm/glide/tests/ExportedSymbols.test.ts b/node/npm/glide/tests/ExportedSymbols.test.ts index 985ecec814..eaa6549a83 100644 --- a/node/npm/glide/tests/ExportedSymbols.test.ts +++ b/node/npm/glide/tests/ExportedSymbols.test.ts @@ -45,12 +45,9 @@ describe("Exported Symbols test", () => { // } async function getFiles(folderName: string): Promise { - // console.log(folderName); - const files = await f.readdir(folderName, { withFileTypes: true }); const skipFolders = [ - // 'build-ts', 'commonjs-test', 'glide-logs', 'hybrid-node-tests', @@ -71,31 +68,9 @@ async function getFiles(folderName: string): Promise { filesWithNodeCode.push(...(await getFiles(folderName + file.name + '/'))); } else { - // console.log("not a dir: " + file.name); - - // if (file.name.endsWith('.js') || - // file.name.endsWith('.d.ts') || - // file.name.endsWith('.json') || - // file.name.endsWith('.rs') || - // file.name.endsWith('.html') || - // file.name.endsWith('.node') || - // file.name.endsWith('.lock') || - // file.name.endsWith('.toml') || - // file.name.endsWith('.yml') || - // file.name.endsWith('.rdb') || - // file.name.endsWith('.md') || - // file.name.localeCompare('.gitignore') == 0 || - // file.name.localeCompare('.prettierignore') == 0 || - // file.name.localeCompare('THIRD_PARTY_LICENSES_NODE') == 0 || - // file.name.localeCompare('index.ts') == 0) { - // continue; - // } - if (!file.name.endsWith('.d.ts')) { continue; } - - // i++; filesWithNodeCode.push(folderName + file.name); } } @@ -104,18 +79,17 @@ async function getFiles(folderName: string): Promise { } function visitRoot(root: ts.Node) { + // (Root Level)->(Level 1) const children: ts.Node[] = root.getChildren(); const resultList: string[] = []; - + // (Root Level) -> (Level 1) -> Level 2. This is the level in the AST where all the exported symbols in a file are present. for (const node of children) { const nodeList: string[] = node.getChildren().map(c => visit(c)).filter(c => c !== undefined); if (nodeList.length > 0) { resultList.push(...nodeList); } - - console.log(resultList); } return resultList; @@ -124,6 +98,7 @@ function visitRoot(root: ts.Node) { function visit(node: ts.Node) { let name: string | undefined = ""; + // List of exported symbols we want to ignore. switch (node.kind) { case ts.SyntaxKind.FirstStatement: case ts.SyntaxKind.ExportDeclaration: @@ -132,10 +107,7 @@ function visit(node: ts.Node) { return; } - // list of kind we like: - // InterfaceDeclaration - // FunctionDeclaration - + // list exported symbols we want to check for, like, InterfaceDeclaration, FunctionDeclaration, etc. if (ts.isFunctionDeclaration(node)) { name = node.name?.text; } else if (ts.isVariableStatement(node)) { @@ -152,25 +124,17 @@ function visit(node: ts.Node) { name = node.name?.text; } - const debug: string[] = []; const children = node.getChildren(); const isInternal = children.find(c => (ts.SyntaxKind[c.kind] == "JSDocComment"))?.getText().includes('@internal'); const isExported = children.find(c => (ts.SyntaxKind[c.kind] == "SyntaxList"))?.getChildren().find(c => (ts.SyntaxKind[c.kind] == "ExportKeyword")); if (isExported && !isInternal) { - // debug.push('depth=' + depth + ", ts.SyntaxKind===" + ts.SyntaxKind[node.kind] + ", name=" + name); - // if (name) { - // debug.push(`name=${name} kind=${ts.SyntaxKind[node.kind]}`); - // } else { - // debug.push(`name=unnamed kind=${ts.SyntaxKind[node.kind]}`); - // } - - // console.log(debug); + // Not internal symbol exported for external use. return name; } - if (isExported && isInternal) { - // marked correctly... no-op + if ((isExported && isInternal)) { + // marked correctly... no-op. Exported for internal use in the code. } if (!isExported && isInternal) { @@ -178,11 +142,6 @@ function visit(node: ts.Node) { } if (!isExported && !isInternal) { - // these are okay for now... - // debug.push(`PRIVATE??? name=unnamed kind=${ts.SyntaxKind[node.kind]} text=${node.getText()}`); + // no-op } - - // if (debug.length > 0) { - // console.log(debug); - // } }