Merge pull request #276 from splitio/manager_async_fix

Update manager methods to return a promise in consumer modes while the SDK is not operational

Author: EmilianoSanchez

CHANGES.txt

@@ -1,3 +1,6 @@
+1.11.1 (December XX, 2023)
+  - Bugfixing - Fixed manager methods in consumer modes to return results in a promise when the SDK is not operational (not ready or destroyed).
+
 1.11.0 (November 3, 2023)
   - Added support for Flag Sets on the SDK, which enables grouping feature flags and interacting with the group rather than individually (more details in our documentation):
     - Added new variations of the get treatment methods to support evaluating flags in given flag set/s.

src/sdkFactory/index.ts

@@ -83,7 +83,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO.
 
   // SDK client and manager
   const clientMethod = sdkClientMethodFactory(ctx);
-  const managerInstance = sdkManagerFactory(log, storage.splits, sdkReadinessManager);
+  const managerInstance = sdkManagerFactory(settings, storage.splits, sdkReadinessManager);
 
   syncManager && syncManager.start();
   signalListener && signalListener.start();

src/sdkFactory/types.ts

@@ -1,9 +1,9 @@
 import { IIntegrationManager, IIntegrationFactoryParams } from '../integrations/types';
 import { ISignalListener } from '../listeners/types';
-import { ILogger } from '../logger/types';
 import { IReadinessManager, ISdkReadinessManager } from '../readiness/types';
+import type { sdkManagerFactory } from '../sdkManager';
 import { IFetch, ISplitApi, IEventSourceConstructor } from '../services/types';
-import { IStorageAsync, IStorageSync, ISplitsCacheSync, ISplitsCacheAsync, IStorageFactoryParams } from '../storages/types';
+import { IStorageAsync, IStorageSync, IStorageFactoryParams } from '../storages/types';
 import { ISyncManager } from '../sync/types';
 import { IImpressionObserver } from '../trackers/impressionObserver/types';
 import { IImpressionsTracker, IEventTracker, ITelemetryTracker, IFilterAdapter, IUniqueKeysTracker } from '../trackers/types';
@@ -87,11 +87,7 @@ export interface ISdkFactoryParams {
   syncManagerFactory?: (params: ISdkFactoryContextSync) => ISyncManager,
 
   // Sdk manager factory
-  sdkManagerFactory: (
-    log: ILogger,
-    splits: ISplitsCacheSync | ISplitsCacheAsync,
-    sdkReadinessManager: ISdkReadinessManager
-  ) => SplitIO.IManager | SplitIO.IAsyncManager,
+  sdkManagerFactory: typeof sdkManagerFactory,
 
   // Sdk client method factory (ISDK::client method).
   // It Allows to distinguish SDK clients with the client-side API (`ICsSDK`) or server-side API (`ISDK` or `IAsyncSDK`).

src/sdkManager/__tests__/index.asyncCache.spec.ts

@@ -9,6 +9,7 @@ import { KeyBuilderSS } from '../../storages/KeyBuilderSS';
 import { ISdkReadinessManager } from '../../readiness/types';
 import { loggerMock } from '../../logger/__tests__/sdkLogger.mock';
 import { metadata } from '../../storages/__tests__/KeyBuilder.spec';
+import { SplitIO } from '../../types';
 
 // @ts-expect-error
 const sdkReadinessManagerMock = {
@@ -21,16 +22,16 @@ const sdkReadinessManagerMock = {
 
 const keys = new KeyBuilderSS('prefix', metadata);
 
-describe('MANAGER API', () => {
+describe('Manager with async cache', () => {
 
   afterEach(() => { loggerMock.mockClear(); });
 
-  test('Async cache (In Redis)', async () => {
+  test('returns the expected data from the cache', async () => {
 
     /** Setup: create manager */
     const connection = new Redis({});
     const cache = new SplitsCacheInRedis(loggerMock, keys, connection);
-    const manager = sdkManagerFactory(loggerMock, cache, sdkReadinessManagerMock);
+    const manager = sdkManagerFactory({ mode: 'consumer', log: loggerMock }, cache, sdkReadinessManagerMock);
     await cache.clear();
     await cache.addSplit(splitObject.name, splitObject as any);
 
@@ -43,7 +44,6 @@ describe('MANAGER API', () => {
     const split = await manager.split(splitObject.name);
     expect(split).toEqual(splitView);
 
-
     /** List all the split names */
 
     const names = await manager.names();
@@ -66,18 +66,16 @@ describe('MANAGER API', () => {
     expect(await manager.splits()).toEqual([]); // If the factory/client is destroyed, `manager.splits()` will return empty array either way since the storage is not valid.
     expect(await manager.names()).toEqual([]); // If the factory/client is destroyed, `manager.names()` will return empty array either way since the storage is not valid.
 
-
-
     /** Teardown */
     await cache.removeSplit(splitObject.name);
     await connection.quit();
   });
 
-  test('Async cache with error', async () => {
+  test('handles storage errors', async () => {
     // passing an empty object as wrapper, to make method calls of splits cache fail returning a rejected promise.
     // @ts-expect-error
     const cache = new SplitsCachePluggable(loggerMock, keys, wrapperAdapter(loggerMock, {}));
-    const manager = sdkManagerFactory(loggerMock, cache, sdkReadinessManagerMock);
+    const manager = sdkManagerFactory({ mode: 'consumer_partial', log: loggerMock }, cache, sdkReadinessManagerMock);
 
     expect(await manager.split('some_spplit')).toEqual(null);
     expect(await manager.splits()).toEqual([]);
@@ -86,4 +84,33 @@ describe('MANAGER API', () => {
     expect(loggerMock.error).toBeCalledTimes(3); // 3 error logs, one for each attempt to call a wrapper method
   });
 
+  test('returns empty results when not operational', async () => {
+    // SDK is flagged as destroyed
+    const sdkReadinessManagerMock = {
+      readinessManager: {
+        isReady: () => true,
+        isReadyFromCache: () => true,
+        isDestroyed: () => true
+      },
+      sdkStatus: {}
+    };
+    // @ts-expect-error
+    const manager = sdkManagerFactory({ mode: 'consumer_partial', log: loggerMock }, {}, sdkReadinessManagerMock) as SplitIO.IAsyncManager;
+
+    function validateManager() {
+      expect(manager.split('some_spplit')).resolves.toBe(null);
+      expect(manager.splits()).resolves.toEqual([]);
+      expect(manager.names()).resolves.toEqual([]);
+    }
+
+    validateManager();
+
+    // SDK is not ready
+    sdkReadinessManagerMock.readinessManager.isReady = () => false;
+    sdkReadinessManagerMock.readinessManager.isReadyFromCache = () => false;
+    sdkReadinessManagerMock.readinessManager.isDestroyed = () => false;
+
+    validateManager();
+  });
+
 });

src/sdkManager/__tests__/index.syncCache.spec.ts

@@ -14,11 +14,11 @@ const sdkReadinessManagerMock = {
   sdkStatus: jest.fn()
 } as ISdkReadinessManager;
 
-describe('MANAGER API / Sync cache (In Memory)', () => {
+describe('Manager with sync cache (In Memory)', () => {
 
   /** Setup: create manager */
   const cache = new SplitsCacheInMemory();
-  const manager = sdkManagerFactory(loggerMock, cache, sdkReadinessManagerMock);
+  const manager = sdkManagerFactory({ mode: 'standalone', log: loggerMock }, cache, sdkReadinessManagerMock);
   cache.addSplit(splitObject.name, splitObject as any);
 
   test('List all splits', () => {
@@ -57,4 +57,24 @@ describe('MANAGER API / Sync cache (In Memory)', () => {
     expect(manager.names()).toEqual([]); // If the factory/client is destroyed, `manager.names()` will return empty array either way since the storage is not valid.
   });
 
+  test('returns empty results when not operational', async () => {
+    // SDK is flagged as destroyed
+    sdkReadinessManagerMock.readinessManager.isDestroyed = () => true;
+
+    function validateManager() {
+      expect(manager.split('some_spplit')).toBe(null);
+      expect(manager.splits()).toEqual([]);
+      expect(manager.names()).toEqual([]);
+    }
+
+    validateManager();
+
+    // SDK is not ready
+    sdkReadinessManagerMock.readinessManager.isReady = () => false;
+    sdkReadinessManagerMock.readinessManager.isReadyFromCache = () => false;
+    sdkReadinessManagerMock.readinessManager.isDestroyed = () => false;
+
+    validateManager();
+  });
+
 });

src/sdkManager/index.ts

@@ -5,8 +5,8 @@ import { validateSplit, validateSplitExistance, validateIfNotDestroyed, validate
 import { ISplitsCacheAsync, ISplitsCacheSync } from '../storages/types';
 import { ISdkReadinessManager } from '../readiness/types';
 import { ISplit } from '../dtos/types';
-import { SplitIO } from '../types';
-import { ILogger } from '../logger/types';
+import { ISettings, SplitIO } from '../types';
+import { isStorageSync } from '../trackers/impressionObserver/utils';
 
 const SPLIT_FN_LABEL = 'split';
 const SPLITS_FN_LABEL = 'splits';
@@ -49,11 +49,14 @@ function objectsToViews(splitObjects: ISplit[]) {
 }
 
 export function sdkManagerFactory<TSplitCache extends ISplitsCacheSync | ISplitsCacheAsync>(
-  log: ILogger,
+  settings: Pick<ISettings, 'log' | 'mode'>,
   splits: TSplitCache,
-  { readinessManager, sdkStatus }: ISdkReadinessManager
+  { readinessManager, sdkStatus }: ISdkReadinessManager,
 ): TSplitCache extends ISplitsCacheAsync ? SplitIO.IAsyncManager : SplitIO.IManager {
 
+  const log = settings.log;
+  const isSync = isStorageSync(settings);
+
   return objectAssign(
     // Proto-linkage of the readiness Event Emitter
     Object.create(sdkStatus),
@@ -64,7 +67,7 @@ export function sdkManagerFactory<TSplitCache extends ISplitsCacheSync | ISplits
       split(featureFlagName: string) {
         const splitName = validateSplit(log, featureFlagName, SPLIT_FN_LABEL);
         if (!validateIfNotDestroyed(log, readinessManager, SPLIT_FN_LABEL) || !validateIfOperational(log, readinessManager, SPLIT_FN_LABEL) || !splitName) {
-          return null;
+          return isSync ? null : Promise.resolve(null);
         }
 
         const split = splits.getSplit(splitName);
@@ -85,7 +88,7 @@ export function sdkManagerFactory<TSplitCache extends ISplitsCacheSync | ISplits
        */
       splits() {
         if (!validateIfNotDestroyed(log, readinessManager, SPLITS_FN_LABEL) || !validateIfOperational(log, readinessManager, SPLITS_FN_LABEL)) {
-          return [];
+          return isSync ? [] : Promise.resolve([]);
         }
         const currentSplits = splits.getAll();
 
@@ -98,7 +101,7 @@ export function sdkManagerFactory<TSplitCache extends ISplitsCacheSync | ISplits
        */
       names() {
         if (!validateIfNotDestroyed(log, readinessManager, NAMES_FN_LABEL) || !validateIfOperational(log, readinessManager, NAMES_FN_LABEL)) {
-          return [];
+          return isSync ? [] : Promise.resolve([]);
         }
         const splitNames = splits.getSplitNames();
 

src/trackers/impressionObserver/utils.ts

@@ -4,6 +4,6 @@ import { ISettings } from '../../types';
 /**
  * Storage is async if mode is consumer or partial consumer
  */
-export function isStorageSync(settings: ISettings) {
+export function isStorageSync(settings: Pick<ISettings, 'mode'>) {
   return [CONSUMER_MODE, CONSUMER_PARTIAL_MODE].indexOf(settings.mode) === -1 ? true : false;
 }