fix(client): cache subsequent clients (redis#2963)

nkaradzhov · web-flow · commit def97171a5f1 · 2025-05-14T17:23:22.000+03:00
* fix(client): cache subsequent clients we dont need to recreate a client if its config hasnt changed fixes redis#2954 * handle circular structures * make cache generic
diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts
@@ -17,6 +17,7 @@ import { RedisLegacyClient, RedisLegacyClientType } from './legacy-mode';
 import { RedisPoolOptions, RedisClientPool } from './pool';
 import { RedisVariadicArgument, parseArgs, pushVariadicArguments } from '../commands/generic-transformers';
 import { BasicCommandParser, CommandParser } from './parser';
+import SingleEntryCache from '../single-entry-cache';
 
 export interface RedisClientOptions<
   M extends RedisModules = RedisModules,
@@ -206,23 +207,32 @@ export default class RedisClient<
     }
   }
 
+  static #SingleEntryCache = new SingleEntryCache<any, any>()
+
   static factory<
     M extends RedisModules = {},
     F extends RedisFunctions = {},
     S extends RedisScripts = {},
     RESP extends RespVersions = 2
   >(config?: CommanderConfig<M, F, S, RESP>) {
-    const Client = attachConfig({
-      BaseClass: RedisClient,
-      commands: COMMANDS,
-      createCommand: RedisClient.#createCommand,
-      createModuleCommand: RedisClient.#createModuleCommand,
-      createFunctionCommand: RedisClient.#createFunctionCommand,
-      createScriptCommand: RedisClient.#createScriptCommand,
-      config
-    });
 
-    Client.prototype.Multi = RedisClientMultiCommand.extend(config);
+
+    let Client = RedisClient.#SingleEntryCache.get(config);
+    if (!Client) {
+      Client = attachConfig({
+        BaseClass: RedisClient,
+        commands: COMMANDS,
+        createCommand: RedisClient.#createCommand,
+        createModuleCommand: RedisClient.#createModuleCommand,
+        createFunctionCommand: RedisClient.#createFunctionCommand,
+        createScriptCommand: RedisClient.#createScriptCommand,
+        config
+      });
+
+      Client.prototype.Multi = RedisClientMultiCommand.extend(config);
+
+      RedisClient.#SingleEntryCache.set(config, Client);
+    }
 
     return <TYPE_MAPPING extends TypeMapping = {}>(
       options?: Omit<RedisClientOptions<M, F, S, RESP, TYPE_MAPPING>, keyof Exclude<typeof config, undefined>>
diff --git a/packages/client/lib/client/pool.ts b/packages/client/lib/client/pool.ts
@@ -8,6 +8,7 @@ import { attachConfig, functionArgumentsPrefix, getTransformReply, scriptArgumen
 import { CommandOptions } from './commands-queue';
 import RedisClientMultiCommand, { RedisClientMultiCommandType } from './multi-command';
 import { BasicCommandParser } from './parser';
+import SingleEntryCache from '../single-entry-cache';
 
 export interface RedisPoolOptions {
   /**
@@ -110,6 +111,8 @@ export class RedisClientPool<
     };
   }
 
+  static #SingleEntryCache = new SingleEntryCache<any, any>();
+
   static create<
     M extends RedisModules,
     F extends RedisFunctions,
@@ -120,17 +123,21 @@ export class RedisClientPool<
     clientOptions?: RedisClientOptions<M, F, S, RESP, TYPE_MAPPING>,
     options?: Partial<RedisPoolOptions>
   ) {
-    const Pool = attachConfig({
-      BaseClass: RedisClientPool,
-      commands: COMMANDS,
-      createCommand: RedisClientPool.#createCommand,
-      createModuleCommand: RedisClientPool.#createModuleCommand,
-      createFunctionCommand: RedisClientPool.#createFunctionCommand,
-      createScriptCommand: RedisClientPool.#createScriptCommand,
-      config: clientOptions
-    });
 
-    Pool.prototype.Multi = RedisClientMultiCommand.extend(clientOptions);
+    let Pool = RedisClientPool.#SingleEntryCache.get(clientOptions);
+    if(!Pool) {
+      Pool = attachConfig({
+        BaseClass: RedisClientPool,
+        commands: COMMANDS,
+        createCommand: RedisClientPool.#createCommand,
+        createModuleCommand: RedisClientPool.#createModuleCommand,
+        createFunctionCommand: RedisClientPool.#createFunctionCommand,
+        createScriptCommand: RedisClientPool.#createScriptCommand,
+        config: clientOptions
+      });
+      Pool.prototype.Multi = RedisClientMultiCommand.extend(clientOptions);
+      RedisClientPool.#SingleEntryCache.set(clientOptions, Pool);
+    }
 
     // returning a "proxy" to prevent the namespaces._self to leak between "proxies"
     return Object.create(
diff --git a/packages/client/lib/cluster/index.ts b/packages/client/lib/cluster/index.ts
@@ -12,6 +12,7 @@ import { RedisTcpSocketOptions } from '../client/socket';
 import ASKING from '../commands/ASKING';
 import { BasicCommandParser } from '../client/parser';
 import { parseArgs } from '../commands/generic-transformers';
+import SingleEntryCache from '../single-entry-cache';
 
 interface ClusterCommander<
   M extends RedisModules,
@@ -213,6 +214,8 @@ export default class RedisCluster<
     };
   }
 
+  static #SingleEntryCache = new SingleEntryCache<any, any>();
+
   static factory<
     M extends RedisModules = {},
     F extends RedisFunctions = {},
@@ -221,17 +224,22 @@ export default class RedisCluster<
     TYPE_MAPPING extends TypeMapping = {},
     // POLICIES extends CommandPolicies = {}
   >(config?: ClusterCommander<M, F, S, RESP, TYPE_MAPPING/*, POLICIES*/>) {
-    const Cluster = attachConfig({
-      BaseClass: RedisCluster,
-      commands: COMMANDS,
-      createCommand: RedisCluster.#createCommand,
-      createModuleCommand: RedisCluster.#createModuleCommand,
-      createFunctionCommand: RedisCluster.#createFunctionCommand,
-      createScriptCommand: RedisCluster.#createScriptCommand,
-      config
-    });
-
-    Cluster.prototype.Multi = RedisClusterMultiCommand.extend(config);
+
+    let Cluster = RedisCluster.#SingleEntryCache.get(config);
+    if (!Cluster) {
+      Cluster = attachConfig({
+        BaseClass: RedisCluster,
+        commands: COMMANDS,
+        createCommand: RedisCluster.#createCommand,
+        createModuleCommand: RedisCluster.#createModuleCommand,
+        createFunctionCommand: RedisCluster.#createFunctionCommand,
+        createScriptCommand: RedisCluster.#createScriptCommand,
+        config
+      });
+
+      Cluster.prototype.Multi = RedisClusterMultiCommand.extend(config);
+      RedisCluster.#SingleEntryCache.set(config, Cluster);
+    }
 
     return (options?: Omit<RedisClusterOptions, keyof Exclude<typeof config, undefined>>) => {
       // returning a "proxy" to prevent the namespaces._self to leak between "proxies"
diff --git a/packages/client/lib/single-entry-cache.spec.ts b/packages/client/lib/single-entry-cache.spec.ts
@@ -0,0 +1,85 @@
+import assert from 'node:assert';
+import SingleEntryCache from './single-entry-cache';
+
+describe('SingleEntryCache', () => {
+  let cache: SingleEntryCache;
+  beforeEach(() => {
+    cache = new SingleEntryCache();
+  });
+  
+  it('should return undefined when getting from empty cache', () => {
+    assert.strictEqual(cache.get({ key: 'value' }), undefined);
+  });
+
+  it('should return the cached instance when getting with the same key object', () => {
+    const keyObj = { key: 'value' };
+    const instance = { data: 'test data' };
+    
+    cache.set(keyObj, instance);
+    assert.strictEqual(cache.get(keyObj), instance);
+  });
+
+  it('should return undefined when getting with a different key object', () => {
+    const keyObj1 = { key: 'value1' };
+    const keyObj2 = { key: 'value2' };
+    const instance = { data: 'test data' };
+    
+    cache.set(keyObj1, instance);
+    assert.strictEqual(cache.get(keyObj2), undefined);
+  });
+
+  it('should update the cached instance when setting with the same key object', () => {
+    const keyObj = { key: 'value' };
+    const instance1 = { data: 'test data 1' };
+    const instance2 = { data: 'test data 2' };
+    
+    cache.set(keyObj, instance1);
+    assert.strictEqual(cache.get(keyObj), instance1);
+    
+    cache.set(keyObj, instance2);
+    assert.strictEqual(cache.get(keyObj), instance2);
+  });
+
+  it('should handle undefined key object', () => {
+    const instance = { data: 'test data' };
+    
+    cache.set(undefined, instance);
+    assert.strictEqual(cache.get(undefined), instance);
+  });
+
+  it('should handle complex objects as keys', () => {
+    const keyObj = { 
+      id: 123,
+      nested: { 
+        prop: 'value',
+        array: [1, 2, 3]
+      }
+    };
+    const instance = { data: 'complex test data' };
+    
+    cache.set(keyObj, instance);
+    assert.strictEqual(cache.get(keyObj), instance);
+  });
+
+  it('should consider objects with same properties but different order as different keys', () => {
+    const keyObj1 = { a: 1, b: 2 };
+    const keyObj2 = { b: 2, a: 1 }; // Same properties but different order
+    const instance = { data: 'test data' };
+    
+    cache.set(keyObj1, instance);
+    
+    assert.strictEqual(cache.get(keyObj2), undefined);
+  });
+
+  it('should handle circular structures', () => {
+    const keyObj: any = {};
+    keyObj.self = keyObj;
+
+    const instance = { data: 'test data' };
+    
+    cache.set(keyObj, instance);
+    
+    assert.strictEqual(cache.get(keyObj), instance);
+  });
+
+});
diff --git a/packages/client/lib/single-entry-cache.ts b/packages/client/lib/single-entry-cache.ts
@@ -0,0 +1,37 @@
+export default class SingleEntryCache<K, V> {
+  #cached?: V;
+  #serializedKey?: string;
+
+  /**
+   * Retrieves an instance from the cache based on the provided key object.
+   *
+   * @param keyObj - The key object to look up in the cache.
+   * @returns The cached instance if found, undefined otherwise.
+   *
+   * @remarks
+   * This method uses JSON.stringify for comparison, which may not work correctly
+   * if the properties in the key object are rearranged or reordered.
+   */
+  get(keyObj?: K): V | undefined {
+    return JSON.stringify(keyObj, makeCircularReplacer()) === this.#serializedKey ? this.#cached : undefined;
+  }
+
+  set(keyObj:  K | undefined, obj: V) {
+    this.#cached = obj;
+    this.#serializedKey = JSON.stringify(keyObj, makeCircularReplacer());
+  }
+}
+
+function makeCircularReplacer() {
+  const seen = new WeakSet();
+  return function serialize(_: string, value: any) {
+    if (value && typeof value === 'object') {
+      if (seen.has(value)) {
+        return 'circular';
+      }
+      seen.add(value);
+      return value;
+    }
+    return value;
+  }
+}
