Skip to content

Commit

Permalink
Fixed manager methods to return a promise in consumer modes while the…
Browse files Browse the repository at this point in the history
… SDK is not operational
  • Loading branch information
EmilianoSanchez committed Nov 30, 2023
1 parent 50cd611 commit f13ce83
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 26 deletions.
3 changes: 3 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/sdkFactory/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
10 changes: 3 additions & 7 deletions src/sdkFactory/types.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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`).
Expand Down
43 changes: 35 additions & 8 deletions src/sdkManager/__tests__/index.asyncCache.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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);

Expand All @@ -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();
Expand All @@ -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([]);
Expand All @@ -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();
});

});
26 changes: 24 additions & 2 deletions src/sdkManager/__tests__/index.syncCache.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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();
});

});
17 changes: 10 additions & 7 deletions src/sdkManager/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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),
Expand All @@ -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);
Expand All @@ -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();

Expand All @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion src/trackers/impressionObserver/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit f13ce83

Please sign in to comment.