Skip to content

Commit

Permalink
Polishing
Browse files Browse the repository at this point in the history
  • Loading branch information
EmilianoSanchez committed Dec 15, 2023
1 parent f0349f2 commit 2ec9665
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 46 deletions.
20 changes: 19 additions & 1 deletion src/__tests__/actions.browser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { sdkBrowserConfig } from './utils/sdkConfigs';
import {
SPLIT_READY, SPLIT_READY_WITH_EVALUATIONS, SPLIT_READY_FROM_CACHE, SPLIT_READY_FROM_CACHE_WITH_EVALUATIONS,
SPLIT_UPDATE, SPLIT_UPDATE_WITH_EVALUATIONS, SPLIT_TIMEDOUT, SPLIT_DESTROY, ADD_TREATMENTS,
ERROR_GETT_NO_INITSPLITSDK, ERROR_DESTROY_NO_INITSPLITSDK, getControlTreatmentsWithConfig,
ERROR_GETT_NO_INITSPLITSDK, ERROR_DESTROY_NO_INITSPLITSDK, getControlTreatmentsWithConfig, ERROR_GETT_NO_PARAM_OBJECT,
} from '../constants';

/** Test targets */
Expand Down Expand Up @@ -160,6 +160,24 @@ describe('getTreatments', () => {
expect(store.getActions().length).toBe(0);
});

it('logs error and dispatches a no-op async action if the provided param is invalid', async () => {
// Init SDK and set ready
const store = mockStore(STATE_INITIAL);
const actionResult = store.dispatch<any>(initSplitSdk({ config: sdkBrowserConfig }));
(splitSdk.factory as any).client().__emitter__.emit(Event.SDK_READY);
await actionResult;

const consoleLogSpy = jest.spyOn(console, 'log');
store.clearActions();

// @ts-expect-error testing invalid input
store.dispatch<any>(getTreatments());

expect(store.getActions().length).toBe(0);
expect(consoleLogSpy).toBeCalledWith(ERROR_GETT_NO_PARAM_OBJECT);
consoleLogSpy.mockRestore();
});

it('dispatches an ADD_TREATMENTS action if Split SDK is ready', (done) => {

// Init SDK and set ready
Expand Down
34 changes: 14 additions & 20 deletions src/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,32 +49,26 @@ describe('validateGetTreatmentsParams', () => {
it('should ignore flagSets if both splitNames and flagSets are provided', () => {
const consoleSpy = jest.spyOn(console, 'log');

expect(validateGetTreatmentsParams({ splitNames: ['some split', null], flagSets: ['flag set', null] })).toStrictEqual({ splitNames: ['some split'], flagSets: undefined });
expect(validateGetTreatmentsParams({ splitNames: true, flagSets: ['flag set', null] })).toStrictEqual({ splitNames: [], flagSets: undefined });

expect(consoleSpy.mock.calls).toEqual([
[WARN_FEATUREFLAGS_AND_FLAGSETS],
['[ERROR] you passed a null or undefined feature flag name, feature flag name must be a non-empty string.'],
[WARN_FEATUREFLAGS_AND_FLAGSETS],
['[ERROR] feature flag names must be a non-empty array.']
]);
expect(validateGetTreatmentsParams({ splitNames: ['some split'], flagSets: ['flag set', null] })).toStrictEqual({ splitNames: ['some split'], flagSets: undefined });

expect(consoleSpy.mock.calls).toEqual([[WARN_FEATUREFLAGS_AND_FLAGSETS]]);
consoleSpy.mockRestore();
});

it('should return a valid object if splitNames and flagSets values are invalid', () => {
it('should return false if splitNames and flagSets values are invalid', () => {
// Invalid values for splitNames and flagSets are converted to empty arrays
expect(validateGetTreatmentsParams({ splitNames: {} })).toStrictEqual({ splitNames: [], flagSets: undefined });
expect(validateGetTreatmentsParams({ flagSets: {} })).toStrictEqual({ splitNames: undefined, flagSets: [] });
expect(validateGetTreatmentsParams({ splitNames: true })).toStrictEqual({ splitNames: [], flagSets: undefined });
expect(validateGetTreatmentsParams({ flagSets: true })).toStrictEqual({ splitNames: undefined, flagSets: [] });
expect(validateGetTreatmentsParams({ splitNames: null, flagSets: null })).toStrictEqual({ splitNames: undefined, flagSets: [] });
expect(validateGetTreatmentsParams({})).toStrictEqual({ splitNames: undefined, flagSets: [] });
expect(validateGetTreatmentsParams({ splitNames: {} })).toStrictEqual(false);
expect(validateGetTreatmentsParams({ flagSets: {} })).toStrictEqual(false);
expect(validateGetTreatmentsParams({ splitNames: true })).toStrictEqual(false);
expect(validateGetTreatmentsParams({ flagSets: true })).toStrictEqual(false);
expect(validateGetTreatmentsParams({ splitNames: null, flagSets: null })).toStrictEqual(false);
expect(validateGetTreatmentsParams({})).toStrictEqual(false);
});

it('should return a valid object if the provided param is not an object', () => { // @ts-expect-error testing invalid input
expect(validateGetTreatmentsParams()).toStrictEqual({ splitNames: undefined, flagSets: [] });
expect(validateGetTreatmentsParams([])).toStrictEqual({ splitNames: undefined, flagSets: [] });
expect(validateGetTreatmentsParams('invalid')).toStrictEqual({ splitNames: undefined, flagSets: [] });
it('should return false if the provided param is not an object', () => { // @ts-expect-error testing invalid input
expect(validateGetTreatmentsParams()).toStrictEqual(false);
expect(validateGetTreatmentsParams([])).toStrictEqual(false);
expect(validateGetTreatmentsParams('invalid')).toStrictEqual(false);
});

});
4 changes: 3 additions & 1 deletion src/asyncActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ export function getTreatments(params: IGetTreatmentsParams): Action | (() => voi
return () => { };
}

params = validateGetTreatmentsParams(params);
params = validateGetTreatmentsParams(params) as IGetTreatmentsParams;
if (!params) return () => { };

const splitNames = params.splitNames as string[];

if (!splitSdk.isDetached) { // Split SDK running in Browser
Expand Down
8 changes: 5 additions & 3 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,16 @@ export const SPLIT_DESTROY = 'SPLIT_DESTROY';
export const ADD_TREATMENTS = 'ADD_TREATMENTS';

// Warning and error messages
export const ERROR_GETT_NO_INITSPLITSDK = '[ERROR] To use "getTreatments" the SDK must be first initialized with a "initSplitSdk" action';
export const ERROR_GETT_NO_INITSPLITSDK = '[ERROR] To use "getTreatments" the SDK must be first initialized with an "initSplitSdk" action';

export const ERROR_DESTROY_NO_INITSPLITSDK = '[ERROR] To use "destroySplitSdk" the SDK must be first initialized with a "initSplitSdk" action';
export const ERROR_DESTROY_NO_INITSPLITSDK = '[ERROR] To use "destroySplitSdk" the SDK must be first initialized with an "initSplitSdk" action';

export const ERROR_TRACK_NO_INITSPLITSDK = '[ERROR] To use "track" the SDK must be first initialized with an "initSplitSdk" action';

export const ERROR_MANAGER_NO_INITSPLITSDK = '[ERROR] To use the manager, the SDK must be first initialized with an "initSplitSdk" action';

export const ERROR_SELECTOR_NO_SPLITSTATE = '[ERROR] When using selectors, "splitState" value must be a proper splitio piece of state';

export const WARN_FEATUREFLAGS_AND_FLAGSETS = '[WARN] Both splitNames and flagSets properties were provided. flagSets will be ignored.';
export const ERROR_GETT_NO_PARAM_OBJECT = '[ERROR] "getTreatments" must be called with a param object containing either splitNames or flagSets properties';

export const WARN_FEATUREFLAGS_AND_FLAGSETS = '[WARN] Both splitNames and flagSets properties were provided. flagSets will be ignored';
43 changes: 26 additions & 17 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { WARN_FEATUREFLAGS_AND_FLAGSETS } from './constants';
import { ERROR_GETT_NO_PARAM_OBJECT, WARN_FEATUREFLAGS_AND_FLAGSETS } from './constants';
import { IGetTreatmentsParams } from './types';

/**
Expand Down Expand Up @@ -37,26 +37,35 @@ export function getStatus(client: SplitIO.IClient): IClientStatus {
* Validates and sanitizes the parameters passed to the "getTreatments" action creator.
* The returned object is a copy of the passed one, with the "splitNames" and "flagSets" properties converted to an array of strings.
*/
export function validateGetTreatmentsParams(params: unknown) {
if (!isObject(params)) {
console.log('[ERROR] "getTreatments" must be called with a param object.');
params = {};
export function validateGetTreatmentsParams(params: any): IGetTreatmentsParams | false {
if (!isObject(params) || (!params.splitNames && !params.flagSets)) {
console.log(ERROR_GETT_NO_PARAM_OBJECT);
return false;
}

const { splitNames, flagSets } = params as IGetTreatmentsParams;
if (splitNames && flagSets) console.log(WARN_FEATUREFLAGS_AND_FLAGSETS);
let { splitNames, flagSets } = params;

if (splitNames) {
// Feature flag names are sanitized because they are passed to the getControlTreatmentsWithConfig function.
splitNames = validateFeatureFlags(typeof splitNames === 'string' ? [splitNames] : splitNames);
if (!splitNames) return false;

// Ignore flagSets if splitNames are provided
if (flagSets) {
console.log(WARN_FEATUREFLAGS_AND_FLAGSETS);
flagSets = undefined;
}
} else {
// Flag set names are not sanitized, because they are not used by Redux SDK directly. We just make sure it is an array.
flagSets = typeof flagSets === 'string' ? [flagSets] : flagSets;
if (!Array.isArray(flagSets)) return false;
}

return {
...params as IGetTreatmentsParams,
splitNames: splitNames ?
// Feature flag names are sanitized because they are passed to the getControlTreatmentsWithConfig function.
validateFeatureFlags(typeof splitNames === 'string' ? [splitNames] : splitNames) || [] :
undefined,
flagSets: splitNames ?
undefined :
// Flag set names are not sanitized, because they are not used by Redux SDK directly. We just make sure it is an array.
typeof flagSets === 'string' ? [flagSets] : Array.isArray(flagSets) ? flagSets : [],
} as IGetTreatmentsParams;
...params,
splitNames,
flagSets,
};
}

// The following input validation utils are based on the ones in the React SDK. They might be replaced by utils from the JS SDK in the future.
Expand Down
7 changes: 4 additions & 3 deletions types/constants.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ export declare const SPLIT_UPDATE_WITH_EVALUATIONS = "SPLIT_UPDATE_WITH_EVALUATI
export declare const SPLIT_TIMEDOUT = "SPLIT_TIMEDOUT";
export declare const SPLIT_DESTROY = "SPLIT_DESTROY";
export declare const ADD_TREATMENTS = "ADD_TREATMENTS";
export declare const ERROR_GETT_NO_INITSPLITSDK = "[ERROR] To use \"getTreatments\" the SDK must be first initialized with a \"initSplitSdk\" action";
export declare const ERROR_DESTROY_NO_INITSPLITSDK = "[ERROR] To use \"destroySplitSdk\" the SDK must be first initialized with a \"initSplitSdk\" action";
export declare const ERROR_GETT_NO_INITSPLITSDK = "[ERROR] To use \"getTreatments\" the SDK must be first initialized with an \"initSplitSdk\" action";
export declare const ERROR_DESTROY_NO_INITSPLITSDK = "[ERROR] To use \"destroySplitSdk\" the SDK must be first initialized with an \"initSplitSdk\" action";
export declare const ERROR_TRACK_NO_INITSPLITSDK = "[ERROR] To use \"track\" the SDK must be first initialized with an \"initSplitSdk\" action";
export declare const ERROR_MANAGER_NO_INITSPLITSDK = "[ERROR] To use the manager, the SDK must be first initialized with an \"initSplitSdk\" action";
export declare const ERROR_SELECTOR_NO_SPLITSTATE = "[ERROR] When using selectors, \"splitState\" value must be a proper splitio piece of state";
export declare const WARN_FEATUREFLAGS_AND_FLAGSETS = "[WARN] Both splitNames and flagSets properties were provided. flagSets will be ignored.";
export declare const ERROR_GETT_NO_PARAM_OBJECT = "[ERROR] \"getTreatments\" must be called with a param object containing either splitNames or flagSets properties";
export declare const WARN_FEATUREFLAGS_AND_FLAGSETS = "[WARN] Both splitNames and flagSets properties were provided. flagSets will be ignored";
2 changes: 1 addition & 1 deletion types/utils.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,4 @@ export declare function getStatus(client: SplitIO.IClient): IClientStatus;
* Validates and sanitizes the parameters passed to the "getTreatments" action creator.
* The returned object is a copy of the passed one, with the "splitNames" and "flagSets" properties converted to an array of strings.
*/
export declare function validateGetTreatmentsParams(params: unknown): IGetTreatmentsParams;
export declare function validateGetTreatmentsParams(params: any): IGetTreatmentsParams | false;

0 comments on commit 2ec9665

Please sign in to comment.