From a1da884c88a34bc0ab630455f50558f04e1d0c3e Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 14 Oct 2024 22:59:07 -0300 Subject: [PATCH] Bugfix for issue 121 --- CHANGES.txt | 3 + src/__tests__/asyncActions.browser.test.ts | 15 +++-- src/__tests__/reducer.test.ts | 16 ++++-- src/actions.ts | 6 +- src/asyncActions.ts | 6 +- src/reducer.ts | 66 +++++++++++----------- src/utils.ts | 2 +- 7 files changed, 65 insertions(+), 49 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 441f867..08c1acc 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,6 @@ +1.14.1 (October 15, 2024) + - Bugfixing - Fixed error in `splitReducer` when handling actions with a `null` payload, preventing crashes caused by accessing undefined payload properties (Related to https://github.com/splitio/redux-client/issues/121). + 1.14.0 (September 13, 2024) - Added `status` property to Split reducer's slice of state to track the SDK events of non-default clients (Related to https://github.com/splitio/redux-client/issues/113). - Added `lastUpdate` and `isTimedout` properties to the object returned by the `getStatus` helper and `selectTreatmentAndStatus` and `selectTreatmentWithConfigAndStatus` selectors, to expose the last event timestamp and the timedout status of the SDK clients (Related to https://github.com/splitio/redux-client/issues/113). diff --git a/src/__tests__/asyncActions.browser.test.ts b/src/__tests__/asyncActions.browser.test.ts index d6ee032..4d23117 100644 --- a/src/__tests__/asyncActions.browser.test.ts +++ b/src/__tests__/asyncActions.browser.test.ts @@ -285,7 +285,8 @@ describe('getTreatments', () => { payload: { key: sdkBrowserConfig.core.key, treatments: expect.any(Object), - timestamp: expect.any(Number) + timestamp: expect.any(Number), + nonDefaultKey: false } }); @@ -364,7 +365,8 @@ describe('getTreatments', () => { payload: { key: sdkBrowserConfig.core.key, treatments: expect.any(Object), - timestamp: expect.any(Number) + timestamp: expect.any(Number), + nonDefaultKey: false } }); @@ -438,7 +440,8 @@ describe('getTreatments', () => { payload: { key: sdkBrowserConfig.core.key, treatments: expect.any(Object), - timestamp: expect.any(Number) + timestamp: expect.any(Number), + nonDefaultKey: false } }); @@ -459,7 +462,8 @@ describe('getTreatments', () => { payload: { key: sdkBrowserConfig.core.key, treatments: expect.any(Object), - timestamp: expect.any(Number) + timestamp: expect.any(Number), + nonDefaultKey: false } }); @@ -478,7 +482,8 @@ describe('getTreatments', () => { payload: { key: sdkBrowserConfig.core.key, treatments: expect.any(Object), - timestamp: expect.any(Number) + timestamp: expect.any(Number), + nonDefaultKey: false } }); diff --git a/src/__tests__/reducer.test.ts b/src/__tests__/reducer.test.ts index 07fb1b6..10ef362 100644 --- a/src/__tests__/reducer.test.ts +++ b/src/__tests__/reducer.test.ts @@ -190,7 +190,7 @@ describe('Split reducer', () => { const initialTreatments = initialState.treatments; // default key - const action = actionCreator(key, treatments, 1000); + const action = actionCreator(key, treatments, 1000, false); expect(splitReducer(initialState, action)).toEqual({ ...initialState, isReady, @@ -231,7 +231,7 @@ describe('Split reducer', () => { const newTreatments: SplitIO.TreatmentsWithConfig = { test_split: { ...previousTreatment }, }; - const action = actionCreator(key, newTreatments, 1000); + const action = actionCreator(key, newTreatments, 1000, false); const reduxState = splitReducer(stateWithTreatments, action); // control assertion - treatment object was not replaced in the state @@ -262,7 +262,7 @@ describe('Split reducer', () => { config: previousTreatment.config, }, }; - const action = actionCreator(key, newTreatments, 1000); + const action = actionCreator(key, newTreatments, 1000, false); const reduxState = splitReducer(stateWithTreatments, action); // control assertion - treatment object was replaced in the state @@ -293,7 +293,7 @@ describe('Split reducer', () => { }, }; // const action = addTreatments(key, newTreatments); - const action = actionCreator(key, newTreatments, 1000); + const action = actionCreator(key, newTreatments, 1000, false); const reduxState = splitReducer(stateWithTreatments, action); // control assertion - treatment object was replaced in the state @@ -314,4 +314,12 @@ describe('Split reducer', () => { }); }); + it('should ignore other actions', () => { + expect(splitReducer(initialState, { type: 'OTHER_ACTION' })).toBe(initialState); + expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: null })).toBe(initialState); + expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: undefined })).toBe(initialState); + expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: {} })).toBe(initialState); + expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: true })).toBe(initialState); + }); + }); diff --git a/src/actions.ts b/src/actions.ts index 0c47857..1998057 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -18,7 +18,7 @@ export function splitReady(timestamp: number, key?: SplitIO.SplitKey) { }; } -export function splitReadyWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey?: boolean) { +export function splitReadyWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey: boolean) { return { type: SPLIT_READY_WITH_EVALUATIONS, payload: { @@ -40,7 +40,7 @@ export function splitReadyFromCache(timestamp: number, key?: SplitIO.SplitKey) { }; } -export function splitReadyFromCacheWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey?: boolean) { +export function splitReadyFromCacheWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey: boolean) { return { type: SPLIT_READY_FROM_CACHE_WITH_EVALUATIONS, payload: { @@ -62,7 +62,7 @@ export function splitUpdate(timestamp: number, key?: SplitIO.SplitKey) { }; } -export function splitUpdateWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey?: boolean) { +export function splitUpdateWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey: boolean) { return { type: SPLIT_UPDATE_WITH_EVALUATIONS, payload: { diff --git a/src/asyncActions.ts b/src/asyncActions.ts index d2cff5e..6092f03 100644 --- a/src/asyncActions.ts +++ b/src/asyncActions.ts @@ -236,7 +236,7 @@ export function getClient(splitSdk: ISplitSdk, key?: SplitIO.SplitKey): IClientN if (client.evalOnReady.length) { const treatments = __getTreatments(client, client.evalOnReady); - splitSdk.dispatch(splitReadyWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key && true)); + splitSdk.dispatch(splitReadyWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key ? true : false)); } else { splitSdk.dispatch(splitReady(lastUpdate, key)); } @@ -255,7 +255,7 @@ export function getClient(splitSdk: ISplitSdk, key?: SplitIO.SplitKey): IClientN if (client.evalOnReadyFromCache.length) { const treatments = __getTreatments(client, client.evalOnReadyFromCache); - splitSdk.dispatch(splitReadyFromCacheWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key && true)); + splitSdk.dispatch(splitReadyFromCacheWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key ? true : false)); } else { splitSdk.dispatch(splitReadyFromCache(lastUpdate, key)); } @@ -270,7 +270,7 @@ export function getClient(splitSdk: ISplitSdk, key?: SplitIO.SplitKey): IClientN if (evalOnUpdate.length) { const treatments = __getTreatments(client, evalOnUpdate); - splitSdk.dispatch(splitUpdateWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key && true)); + splitSdk.dispatch(splitUpdateWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key ? true : false)); } else { splitSdk.dispatch(splitUpdate(lastUpdate, key)); } diff --git a/src/reducer.ts b/src/reducer.ts index b220288..b35dab9 100644 --- a/src/reducer.ts +++ b/src/reducer.ts @@ -22,65 +22,66 @@ const initialState: ISplitState = { treatments: {}, }; -function setStatus(state: ISplitState, patch: Partial, key?: string) { - return key ? { +function setStatus(state: ISplitState, patch: Partial, action: ISplitAction) { + const { timestamp, key, nonDefaultKey } = action.payload; + + return nonDefaultKey || (nonDefaultKey === undefined && key) ? { ...state, status: { ...state.status, [key]: state.status && state.status[key] ? { ...state.status[key], ...patch, + lastUpdate: timestamp, } : { ...initialStatus, ...patch, + lastUpdate: timestamp, } }, } : { ...state, ...patch, + lastUpdate: timestamp, }; } -function setReady(state: ISplitState, timestamp: number, key?: string) { +function setReady(state: ISplitState, action: ISplitAction) { return setStatus(state, { isReady: true, isTimedout: false, - lastUpdate: timestamp, - }, key); + }, action); } -function setReadyFromCache(state: ISplitState, timestamp: number, key?: string) { +function setReadyFromCache(state: ISplitState, action: ISplitAction) { return setStatus(state, { isReadyFromCache: true, - lastUpdate: timestamp, - }, key); + }, action); } -function setTimedout(state: ISplitState, timestamp: number, key?: string) { +function setTimedout(state: ISplitState, action: ISplitAction) { return setStatus(state, { isTimedout: true, hasTimedout: true, - lastUpdate: timestamp, - }, key); + }, action); } -function setUpdated(state: ISplitState, timestamp: number, key?: string) { - return setStatus(state, { - lastUpdate: timestamp, - }, key); +function setUpdated(state: ISplitState, action: ISplitAction) { + return setStatus(state, {}, action); } -function setDestroyed(state: ISplitState, timestamp: number, key?: string) { +function setDestroyed(state: ISplitState, action: ISplitAction) { return setStatus(state, { isDestroyed: true, - lastUpdate: timestamp, - }, key); + }, action); } /** * Copy the given `treatments` for the given `key` to a `result` Split's slice of state. Returns the `result` object. */ -function assignTreatments(result: ISplitState, key: string, treatments: SplitIO.TreatmentsWithConfig): ISplitState { +function assignTreatments(result: ISplitState, action: ISplitAction): ISplitState { + const { key, treatments } = action.payload; + result.treatments = { ...result.treatments }; Object.entries(treatments).forEach(([featureFlagName, treatment]) => { if (result.treatments[featureFlagName]) { @@ -106,42 +107,41 @@ export const splitReducer: Reducer = function ( state = initialState, action, ) { - const { type, payload: { timestamp, key, treatments, nonDefaultKey } = {} } = action as ISplitAction; - switch (type) { + switch (action.type) { case SPLIT_READY: - return setReady(state, timestamp, key); + return setReady(state, action as ISplitAction); case SPLIT_READY_FROM_CACHE: - return setReadyFromCache(state, timestamp, key); + return setReadyFromCache(state, action as ISplitAction); case SPLIT_TIMEDOUT: - return setTimedout(state, timestamp, key); + return setTimedout(state, action as ISplitAction); case SPLIT_UPDATE: - return setUpdated(state, timestamp, key); + return setUpdated(state, action as ISplitAction); case SPLIT_DESTROY: - return setDestroyed(state, timestamp, key); + return setDestroyed(state, action as ISplitAction); case ADD_TREATMENTS: { const result = { ...state }; - return assignTreatments(result, key, treatments); + return assignTreatments(result, action as ISplitAction); } case SPLIT_READY_WITH_EVALUATIONS: { - const result = setReady(state, timestamp, nonDefaultKey && key); - return assignTreatments(result, key, treatments); + const result = setReady(state, action as ISplitAction); + return assignTreatments(result, action as ISplitAction); } case SPLIT_READY_FROM_CACHE_WITH_EVALUATIONS: { - const result = setReadyFromCache(state, timestamp, nonDefaultKey && key); - return assignTreatments(result, key, treatments); + const result = setReadyFromCache(state, action as ISplitAction); + return assignTreatments(result, action as ISplitAction); } case SPLIT_UPDATE_WITH_EVALUATIONS: { - const result = setUpdated(state, timestamp, nonDefaultKey && key); - return assignTreatments(result, key, treatments); + const result = setUpdated(state, action as ISplitAction); + return assignTreatments(result, action as ISplitAction); } default: diff --git a/src/utils.ts b/src/utils.ts index 3af94d3..ca53411 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -6,7 +6,7 @@ import { IGetTreatmentsParams } from './types'; * Validates if a given value is a plain object */ export function isObject(obj: unknown) { - return obj && typeof obj === 'object' && obj.constructor === Object; + return obj !== null && typeof obj === 'object' && (obj.constructor === Object || (obj.constructor != null && obj.constructor.name === 'Object')); } /**