Fb/more redis client polish (#95)

* wip 1

* wip 2

* wip 3

* wip 4

* wip 5

* wip 6

* naming consistency between initialize and cds-plugin options

* make tests great again

* lower redis ping interval to 4min

* new harmonized approach

* new harmonized approach 2

* new harmonized approach 3

* clearer option naming

* remove different behavior for local reconnect

* minimal defaulting for cds.requires.redis-featureToggles

* no more reconnect strategy in snaps

* no defaulting for redis-featureToggles

* update jsdocs a little

* ternary for createClient/createCluster

Author: rlindner81

src/feature-toggles.js

@@ -808,7 +808,8 @@ class FeatureToggles {
     configFile: configFilepath,
     configFiles: configFilepaths,
     configAuto,
-    redisClientOptions,
+    customRedisCredentials,
+    customRedisClientOptions,
   } = {}) {
     if (this.__isInitialized) {
       return;
@@ -867,7 +868,7 @@ class FeatureToggles {
       );
     }
 
-    redis.setClientOptions(redisClientOptions);
+    redis.setCustomOptions(customRedisCredentials, customRedisClientOptions);
     const redisIntegrationMode = await redis.getIntegrationMode();
     if (redisIntegrationMode !== REDIS_INTEGRATION_MODE.NO_REDIS) {
       try {
@@ -953,6 +954,8 @@ class FeatureToggles {
    * @property {Config}  [config]
    * @property {string}  [configFile]
    * @property {Config}  [configAuto]
+   * @property {object}  [customRedisCredentials]
+   * @property {object}  [customRedisClientOptions]
    */
   /**
    * Initialize needs to run and finish before other APIs are called. It processes the configuration, sets up

src/plugin.js

@@ -169,13 +169,17 @@ const activate = async () => {
     return;
   }
 
-  const ftsAutoConfig = await _discoverFtsAutoConfig();
+  const configAuto = await _discoverFtsAutoConfig();
+  const { credentials: customRedisCredentials, options: customRedisClientOptions } =
+    cds.requires["redis-featureToggles"] || cds.requires["redis"] || {};
+
   await toggles.initializeFeatures({
     config: envFeatureToggles?.config,
     configFile: envFeatureToggles?.configFile,
     configFiles: envFeatureToggles?.configFiles,
-    configAuto: ftsAutoConfig,
-    redisClientOptions: envFeatureToggles?.clientOptions,
+    configAuto,
+    customRedisCredentials,
+    customRedisClientOptions,
   });
 
   _registerFeatureProvider(envFeatureToggles);

src/redis-adapter.js

@@ -24,7 +24,7 @@ const INTEGRATION_MODE = Object.freeze({
   NO_REDIS: "NO_REDIS",
 });
 const CF_REDIS_SERVICE_LABEL = "redis-cache";
-const REDIS_CLIENT_DEFAULT_PING_INTERVAL = 5 * 60000;
+const REDIS_CLIENT_DEFAULT_PING_INTERVAL = 4 * 60000;
 
 const cfEnv = CfEnv.getInstance();
 const logger = new Logger(COMPONENT_NAME);
@@ -35,14 +35,18 @@ const MODE = Object.freeze({
 });
 
 let __messageHandlers;
-let __clientOptions;
+let __customCredentials;
+let __customClientOptions;
+let __activeOptionsTuple;
 let __canGetClientPromise;
 let __mainClientPromise;
 let __subscriberClientPromise;
 let __integrationModePromise;
 const _reset = () => {
   __messageHandlers = new HandlerCollection();
-  __clientOptions = null;
+  __customCredentials = null;
+  __customClientOptions = null;
+  __activeOptionsTuple = null;
   __canGetClientPromise = null;
   __mainClientPromise = null;
   __subscriberClientPromise = null;
@@ -80,25 +84,20 @@ const _subscribedMessageHandler = async (message, channel) => {
       );
 };
 
-const _localReconnectStrategy = () =>
-  new VError({ name: VERROR_CLUSTER_NAME }, "disabled reconnect, because we are not running on cloud foundry");
-
-/**
- * Lazily create a new redis client. Client creation transparently handles both the Cloud Foundry "redis-cache" service
- * (hyperscaler option) and a local redis-server.
- *
- * @returns {RedisClient|RedisCluster}
- * @private
- */
-const _createClientBase = (clientName) => {
-  try {
-    const localSocketOptions = {
-      host: "localhost",
-      port: 6379,
+const _getRedisOptionsTuple = () => {
+  if (!__activeOptionsTuple) {
+    const defaultClientOptions = {
+      pingInterval: REDIS_CLIENT_DEFAULT_PING_INTERVAL,
+      socket: {
+        host: "localhost",
+        port: 6379,
+      },
     };
-    const credentials = cfEnv.cfServiceCredentialsForLabel(CF_REDIS_SERVICE_LABEL);
+
+    const credentials = __customCredentials || cfEnv.cfServiceCredentialsForLabel(CF_REDIS_SERVICE_LABEL);
     const hasCredentials = Object.keys(credentials).length > 0;
-    const { cluster_mode: isCluster } = credentials;
+
+    const isCluster = !!credentials.cluster_mode;
     const credentialClientOptions = hasCredentials
       ? {
           password: credentials.password,
@@ -112,13 +111,16 @@ const _createClientBase = (clientName) => {
 
     // NOTE: documentation is buried here https://github.com/redis/node-redis/blob/master/docs/client-configuration.md
     const redisClientOptions = {
+      ...defaultClientOptions,
       ...credentialClientOptions,
-      pingInterval: REDIS_CLIENT_DEFAULT_PING_INTERVAL,
-      ...__clientOptions,
+      ...__customClientOptions,
+      // https://nodejs.org/docs/latest-v22.x/api/net.html#socketconnectoptions-connectlistener
+      // https://nodejs.org/docs/latest-v22.x/api/tls.html#tlsconnectoptions-callback
+      // https://nodejs.org/docs/latest-v22.x/api/tls.html#tlscreatesecurecontextoptions
       socket: {
-        ...localSocketOptions,
+        ...defaultClientOptions.socket,
         ...credentialClientOptions?.socket,
-        ...__clientOptions?.socket,
+        ...__customClientOptions?.socket,
       },
     };
 
@@ -132,21 +134,31 @@ const _createClientBase = (clientName) => {
       redisClientOptions.socket.tls = !!redisClientOptions.socket.tls;
     }
 
-    if (
-      redisClientOptions.socket.host === "localhost" &&
-      !Object.prototype.hasOwnProperty.call(redisClientOptions.socket, "reconnectStrategy")
-    ) {
-      redisClientOptions.socket.reconnectStrategy = _localReconnectStrategy;
-    }
+    __activeOptionsTuple = [isCluster, redisClientOptions];
+  }
 
-    if (isCluster) {
-      return redis.createCluster({
-        rootNodes: [redisClientOptions], // NOTE: assume this ignores everything but socket/url
-        // https://github.com/redis/node-redis/issues/1782
-        defaults: redisClientOptions, // NOTE: assume this ignores socket/url
-      });
-    }
-    return redis.createClient(redisClientOptions);
+  return __activeOptionsTuple;
+};
+
+/**
+ * Lazily create a new redis client. Client creation transparently handles
+ * - custom credentials and client options passed in via {@link setCustomOptions},
+ * - Cloud Foundry service with label "redis-cache" (hyperscaler option), and
+ * - local redis-server.
+ *
+ * @returns {RedisClient|RedisCluster}
+ * @private
+ */
+const _createClientBase = (clientName) => {
+  try {
+    const [isCluster, redisClientOptions] = _getRedisOptionsTuple();
+    return isCluster
+      ? redis.createCluster({
+          rootNodes: [redisClientOptions], // NOTE: assume this ignores everything but socket/url
+          // https://github.com/redis/node-redis/issues/1782
+          defaults: redisClientOptions, // NOTE: assume this ignores socket/url
+        })
+      : redis.createClient(redisClientOptions);
   } catch (err) {
     throw new VError(
       { name: VERROR_CLUSTER_NAME, cause: err, info: { clientName } },
@@ -189,8 +201,9 @@ const _closeClientBase = async (client) => {
   }
 };
 
-const setClientOptions = (clientOptions) => {
-  __clientOptions = clientOptions;
+const setCustomOptions = (customCredentials, customClientOptions) => {
+  __customCredentials = customCredentials;
+  __customClientOptions = customClientOptions;
 };
 
 const _canGetClient = async () => {
@@ -213,7 +226,7 @@ const _getIntegrationMode = async () => {
     return INTEGRATION_MODE.NO_REDIS;
   }
   if (cfEnv.isOnCf) {
-    const { cluster_mode: isCluster } = cfEnv.cfServiceCredentialsForLabel(CF_REDIS_SERVICE_LABEL);
+    const [isCluster] = _getRedisOptionsTuple();
     return isCluster ? INTEGRATION_MODE.CF_REDIS_CLUSTER : INTEGRATION_MODE.CF_REDIS;
   }
   return INTEGRATION_MODE.LOCAL_REDIS;
@@ -304,7 +317,7 @@ const sendCommand = async (command) => {
   const mainClient = await getMainClient();
 
   try {
-    const { cluster_mode: isCluster } = cfEnv.cfServiceCredentialsForLabel(CF_REDIS_SERVICE_LABEL);
+    const [isCluster] = _getRedisOptionsTuple();
     if (isCluster) {
       // NOTE: the cluster sendCommand API has a different signature, where it takes two optional args: firstKey and
       //   isReadonly before the command
@@ -594,7 +607,7 @@ const removeAllMessageHandlers = (channel) => __messageHandlers.removeAllHandler
 
 module.exports = {
   REDIS_INTEGRATION_MODE: INTEGRATION_MODE,
-  setClientOptions,
+  setCustomOptions,
   getIntegrationMode,
   getMainClient,
   closeMainClient,
@@ -628,7 +641,6 @@ module.exports = {
     _getSubscriberClient: () => __subscriberClientPromise,
     _setSubscriberClient: (value) => (__subscriberClientPromise = value),
     _subscribedMessageHandler,
-    _localReconnectStrategy,
     _createClientBase,
     _createClientAndConnect,
     _clientExec,

test/__mocks__/redis-adapter.js

@@ -70,7 +70,7 @@ module.exports = {
   watchedHashGetSetObject,
   subscribe: jest.fn(),
   getIntegrationMode: jest.fn(() => REDIS_INTEGRATION_MODE.CF_REDIS),
-  setClientOptions: jest.fn(),
+  setCustomOptions: jest.fn(),
   _reset,
   _setValues,
   _setValue,

test/__snapshots__/redis-adapter.test.js.snap

@@ -5,7 +5,7 @@ exports[`redis-adapter test _createClientBase on CF | AWS-cluster 1`] = `
   {
     "defaults": {
       "password": "mock-password",
-      "pingInterval": 300000,
+      "pingInterval": 240000,
       "socket": {
         "host": "clustercfg.rg-0c94f0c1-db0a-495d-b385-4a44a87fc398.vwvwdz.euc1.cache.amazonaws.com",
         "port": 3991,
@@ -15,7 +15,7 @@ exports[`redis-adapter test _createClientBase on CF | AWS-cluster 1`] = `
     "rootNodes": [
       {
         "password": "mock-password",
-        "pingInterval": 300000,
+        "pingInterval": 240000,
         "socket": {
           "host": "clustercfg.rg-0c94f0c1-db0a-495d-b385-4a44a87fc398.vwvwdz.euc1.cache.amazonaws.com",
           "port": 3991,
@@ -31,7 +31,7 @@ exports[`redis-adapter test _createClientBase on CF | AZURE 1`] = `
 [
   {
     "password": "mock-password",
-    "pingInterval": 300000,
+    "pingInterval": 240000,
     "socket": {
       "host": "redis-389395bc-9765-4d9a-ba6b-7252c136abab.redis.cache.windows.net",
       "port": 6380,
@@ -45,7 +45,7 @@ exports[`redis-adapter test _createClientBase on CF | GCP 1`] = `
 [
   {
     "password": "mock-password",
-    "pingInterval": 300000,
+    "pingInterval": 240000,
     "socket": {
       "host": "10.160.68.75",
       "port": 6378,

test/redis-adapter.test.js

@@ -73,11 +73,10 @@ describe("redis-adapter test", () => {
     expect(redis.createClient.mock.calls[0]).toMatchInlineSnapshot(`
       [
         {
-          "pingInterval": 300000,
+          "pingInterval": 240000,
           "socket": {
             "host": "localhost",
             "port": 6379,
-            "reconnectStrategy": [Function],
           },
         },
       ]