[Pluggable storage] polishing

.github/workflows/ci-cd.yml

@@ -24,7 +24,7 @@ jobs:
       - name: Set up nodejs
         uses: actions/setup-node@v2
         with:
-          node-version: 'lts/*'
+          node-version: '14'
           cache: 'npm'
 
       - name: npm ci

package.json

@@ -33,7 +33,7 @@
   },
   "dependencies": {
     "@babel/runtime": "^7.13.10",
-    "@splitsoftware/splitio-commons": "1.0.0",
+    "@splitsoftware/splitio-commons": "1.0.1-rc.2",
     "@types/google.analytics": "0.0.40",
     "events": "3.1.0",
     "ioredis": "^4.28.0",
@@ -103,15 +103,15 @@
     "test-browser-errors": "cross-env NODE_ENV=test karma start karma/errors.ci.karma.conf.js",
     "test-browser-gaintegration": "cross-env NODE_ENV=test karma start karma/gaintegration.ci.karma.conf.js",
     "test-browser-push": "cross-env NODE_ENV=test karma start karma/push.ci.karma.conf.js",
-    "test-node": "npm run test-node-unit && npm run test-node-online && npm run test-node-offline && npm run test-node-destroy && npm run test-node-errors && npm run test-node-push && npm run test-node-redis $$ npm run test-node-custom",
+    "test-node": "npm run test-node-unit && npm run test-node-online && npm run test-node-offline && npm run test-node-destroy && npm run test-node-errors && npm run test-node-push && npm run test-node-consumer-redis $$ npm run test-node-consumer-pluggable",
     "test-node-unit": "cross-env NODE_ENV=test tape -r @babel/register \"src/*/**/__tests__/**/!(browser).spec.js\" | tap-min",
     "test-node-online": "cross-env NODE_ENV=test tape -r @babel/register src/__tests__/node.spec.js | tap-min",
     "test-node-destroy": "cross-env NODE_ENV=test tape -r @babel/register src/__tests__/destroy/node.spec.js | tap-min",
     "test-node-errors": "cross-env NODE_ENV=test tape -r @babel/register src/__tests__/errorCatching/node.spec.js | tap-min",
     "test-node-offline": "cross-env NODE_ENV=test tape -r @babel/register src/__tests__/offline/node.spec.js | tap-min",
     "test-node-push": "cross-env NODE_ENV=test tape -r @babel/register src/__tests__/push/node.spec.js | tap-min",
-    "test-node-redis": "cross-env NODE_ENV=test tape -r @babel/register \"src/__tests__/consumerMode/node_redis.spec.js\" | tap-min",
-    "test-node-custom": "cross-env NODE_ENV=test tape -r @babel/register \"src/__tests__/consumerMode/node_custom.spec.js\" | tap-min",
+    "test-node-consumer-redis": "cross-env NODE_ENV=test tape -r @babel/register \"src/__tests__/consumerMode/node_redis.spec.js\" | tap-min",
+    "test-node-consumer-pluggable": "cross-env NODE_ENV=test tape -r @babel/register \"src/__tests__/consumerMode/node_pluggable.spec.js\" | tap-min",
     "pretest-ts-decls": "npm run build-es && npm run build-cjs && npm link",
     "test-ts-decls": "./scripts/ts-tests.sh",
     "posttest-ts-decls": "npm unlink && npm install",

src/__tests__/browserSuites/ready-promise.spec.js

@@ -418,7 +418,7 @@ export default function readyPromiseAssertions(fetchMock, assert) {
     // We also use the manager to get some of the promises
     const manager = splitio.manager();
 
-    // promise1 is handled inmediately. Thus, the 'reject' callback is expected to be called in 0.15 seconds aprox.
+    // promise1 is handled immediately. Thus, the 'reject' callback is expected to be called in 0.15 seconds aprox.
     setTimeout(() => {
       const promise1 = client.ready();
       const tStart = Date.now();
@@ -434,7 +434,7 @@ export default function readyPromiseAssertions(fetchMock, assert) {
         });
     }, 0);
 
-    // promise2 is handled in 0.15 seconds, when the promise is just rejected. Thus, the 'reject' callback is expected to be called inmediately (0 seconds aprox).
+    // promise2 is handled in 0.15 seconds, when the promise is just rejected. Thus, the 'reject' callback is expected to be called immediately (0 seconds aprox).
     setTimeout(() => {
       const promise2 = manager.ready();
       const tStart = Date.now();
@@ -446,11 +446,11 @@ export default function readyPromiseAssertions(fetchMock, assert) {
           t.pass('### SDK TIMED OUT - time out is triggered before retry attempt finishes');
           assertGetTreatmentControlNotReady(t, client);
           const tDelta = Date.now() - tStart;
-          assert.ok(tDelta < 20, 'The "reject" callback is expected to be called inmediately (0 seconds aprox).');
+          assert.ok(tDelta < 20, 'The "reject" callback is expected to be called immediately (0 seconds aprox).');
         });
     }, fromSecondsToMillis(0.15));
 
-    // promise3 is handled in 0.2 seconds, when the promise is just resolved. Thus, the 'resolve' callback is expected to be called inmediately (0 seconds aprox).
+    // promise3 is handled in 0.2 seconds, when the promise is just resolved. Thus, the 'resolve' callback is expected to be called immediately (0 seconds aprox).
     setTimeout(() => {
       const promise3 = manager.ready();
       const tStart = Date.now();
@@ -459,7 +459,7 @@ export default function readyPromiseAssertions(fetchMock, assert) {
           t.pass('### SDK IS READY - retry attempt finishes before the requestTimeoutBeforeReady limit');
           assertGetTreatmentWhenReady(t, client);
           const tDelta = Date.now() - tStart;
-          assert.ok(tDelta < 20, 'The "resolve" callback is expected to be called inmediately (0 seconds aprox).');
+          assert.ok(tDelta < 20, 'The "resolve" callback is expected to be called immediately (0 seconds aprox).');
 
           return Promise.resolve();
         }, () => {

src/__tests__/consumerMode/ioredisWrapper.js

@@ -7,12 +7,21 @@ import { parseRedisOptions } from '../../utils/settings/storage/node';
  * Operations fail until `connect` is resolved when the Redis 'ready' event is emitted.
  *
  * @param {Object} redisOptions redis options with the format expected at `settings.storage.options`
- * @returns {import("@splitsoftware/splitio-commons/types/storages/types").ICustomStorageWrapper} wrapper for IORedis client
+ * @returns {import("@splitsoftware/splitio-commons/types/storages/types").IPluggableStorageWrapper} wrapper for IORedis client
  */
 export function ioredisWrapper(redisOptions) {
 
+  const options = RedisAdapter._defineOptions(parseRedisOptions(redisOptions));
+
   /** @type ioredis.Redis */
-  let redis;
+  const redis = new ioredis(...RedisAdapter._defineLibrarySettings(options));
+
+  let isConnected = false;
+  redis.on('ready', () => { isConnected = true; });
+  // There is no need to listen for redis 'error' event, because in that case ioredis calls will be rejected.
+  // It is done to avoid getting the message `Unhandled error event: Error: connect ECONNREFUSED`.
+  // Also, we cannot reject the connect promise on an error, because the SDK will not get ready if the connection is established after the error.
+  redis.on('error', () => { });
 
   return {
     get(key) {
@@ -31,9 +40,6 @@ export function ioredisWrapper(redisOptions) {
     getKeysByPrefix(prefix) {
       return redis.keys(`${prefix}*`);
     },
-    getByPrefix(prefix) {
-      return this.getKeysByPrefix(prefix).then(keys => redis.mget(...keys));
-    },
     getMany(keys) {
       return redis.mget(...keys);
     },
@@ -65,20 +71,13 @@ export function ioredisWrapper(redisOptions) {
       return redis.smembers(key);
     },
     connect() {
-      const options = RedisAdapter._defineOptions(parseRedisOptions(redisOptions));
-      redis = new ioredis(...RedisAdapter._defineLibrarySettings(options));
-
       return new Promise((res) => {
-        redis.on('ready', res);
-
-        // There is no need to listen for redis 'error' event, because in that case ioredis calls will be rejected.
-        // But it is done to avoid getting the ioredis message `Unhandled error event: Error: connect ECONNREFUSED`.
-        // If we reject the promise, the SDK client will not get ready if the redis connection is established after an error.
-        redis.on('error', () => { });
+        if (isConnected) res();
+        else redis.on('ready', res);
       });
     },
-    close() {
-      return Promise.resolve(redis && redis.disconnect()); // close the connection
+    disconnect() {
+      return Promise.resolve(redis && redis.disconnect());
     }
   };
 }