From f13ce83f08aa70d76ed66c8c4d4a25bd06ae1db4 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 29 Nov 2023 22:03:51 -0300 Subject: [PATCH] Fixed manager methods to return a promise in consumer modes while the SDK is not operational --- CHANGES.txt | 3 ++ src/sdkFactory/index.ts | 2 +- src/sdkFactory/types.ts | 10 ++--- .../__tests__/index.asyncCache.spec.ts | 43 +++++++++++++++---- .../__tests__/index.syncCache.spec.ts | 26 ++++++++++- src/sdkManager/index.ts | 17 +++++--- src/trackers/impressionObserver/utils.ts | 2 +- 7 files changed, 77 insertions(+), 26 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index a893bd27..a44fed08 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,6 @@ +1.11.1 (December XX, 2023) + - Bugfixing - Fixed manager methods to return a promise in consumer modes when called while 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. diff --git a/src/sdkFactory/index.ts b/src/sdkFactory/index.ts index 7778c325..cd15e9ef 100644 --- a/src/sdkFactory/index.ts +++ b/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(); diff --git a/src/sdkFactory/types.ts b/src/sdkFactory/types.ts index 3cce9c9f..917d7f18 100644 --- a/src/sdkFactory/types.ts +++ b/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`). diff --git a/src/sdkManager/__tests__/index.asyncCache.spec.ts b/src/sdkManager/__tests__/index.asyncCache.spec.ts index 4081c10b..acff3bee 100644 --- a/src/sdkManager/__tests__/index.asyncCache.spec.ts +++ b/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(); + }); + }); diff --git a/src/sdkManager/__tests__/index.syncCache.spec.ts b/src/sdkManager/__tests__/index.syncCache.spec.ts index 2ce3721b..18e03d21 100644 --- a/src/sdkManager/__tests__/index.syncCache.spec.ts +++ b/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,26 @@ 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. }); + // @TODO tests for not operational + + 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(); + }); + }); diff --git a/src/sdkManager/index.ts b/src/sdkManager/index.ts index 1fec8784..670113be 100644 --- a/src/sdkManager/index.ts +++ b/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( - log: ILogger, + settings: Pick, 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) { return [CONSUMER_MODE, CONSUMER_PARTIAL_MODE].indexOf(settings.mode) === -1 ? true : false; }