Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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.
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();
});

});
24 changes: 22 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,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();
});

});
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;
}
Loading