From 246283846aeb6a0e9a9a458b2625887d4c5cca13 Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Wed, 1 Nov 2023 17:11:10 -0300 Subject: [PATCH 1/3] [SDKS-7657] fix SDK_UPDATE issue --- .../__tests__/splitChangesUpdater.spec.ts | 40 ++++++++++++++++++- .../polling/updaters/splitChangesUpdater.ts | 15 ++++++- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts b/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts index 3729eded..54dec32b 100644 --- a/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts +++ b/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts @@ -168,9 +168,9 @@ describe('splitChangesUpdater', () => { const readinessManager = readinessManagerFactory(EventEmitter); const splitsEmitSpy = jest.spyOn(readinessManager.splits, 'emit'); - const splitFiltersValidation = { queryString: null, groupedFilters: { bySet: [], byName: [], byPrefix: [] }, validFilters: [] }; + let splitFiltersValidation = { queryString: null, groupedFilters: { bySet: [], byName: [], byPrefix: [] }, validFilters: [] }; - const splitChangesUpdater = splitChangesUpdaterFactory(loggerMock, splitChangesFetcher, splitsCache, segmentsCache, splitFiltersValidation, readinessManager.splits, 1000, 1); + let splitChangesUpdater = splitChangesUpdaterFactory(loggerMock, splitChangesFetcher, splitsCache, segmentsCache, splitFiltersValidation, readinessManager.splits, 1000, 1); afterEach(() => { jest.clearAllMocks(); @@ -213,4 +213,40 @@ describe('splitChangesUpdater', () => { index++; } }); + + test('flag sets splits-arrived emition', async () => { + const payload = splitNotifications[3].decoded as Pick; + const setMocks = [ + { sets: [], shouldEmit: false }, /* should not emit if flag does not have any set */ + { sets: ['set_a'], shouldEmit: true }, /* should emit if flag is in configured sets */ + { sets: ['set_b'], shouldEmit: true }, /* should emit if flag was just removed from configured sets */ + { sets: ['set_b'], shouldEmit: false }, /* should NOT emit if flag is nor was just removed from configured sets */ + { sets: ['set_c'], shouldEmit: false }, /* should NOT emit if flag is nor was just removed from configured sets */ + { sets: ['set_a'], shouldEmit: true }, /* should emit if flag is back in configured sets */ + ]; + + splitChangesUpdater = splitChangesUpdaterFactory(loggerMock, splitChangesFetcher, new SplitsCacheInMemory(), segmentsCache, splitFiltersValidation, readinessManager.splits, 1000, 1, true); + + let index = 0; + let calls = 0; + // emit always if not configured sets + for (const setMock of setMocks) { + await expect(splitChangesUpdater(undefined, undefined, { payload: {...payload, sets: setMock.sets, status: 'ACTIVE'}, changeNumber: index })).resolves.toBe(true); + expect(splitsEmitSpy.mock.calls[index][0]).toBe('state::splits-arrived'); + index++; + } + + // @ts-ignore + splitFiltersValidation = { queryString: null, groupedFilters: { bySet: ['set_a'], byName: [], byPrefix: [] }, validFilters: [] }; + splitChangesUpdater = splitChangesUpdaterFactory(loggerMock, splitChangesFetcher, new SplitsCacheInMemory(), segmentsCache, splitFiltersValidation, readinessManager.splits, 1000, 1, true); + splitsEmitSpy.mockReset(); + index = 0; + for (const setMock of setMocks) { + await expect(splitChangesUpdater(undefined, undefined, { payload: {...payload, sets: setMock.sets, status: 'ACTIVE'}, changeNumber: index })).resolves.toBe(true); + if (setMock.shouldEmit) calls++; + expect(splitsEmitSpy.mock.calls.length).toBe(calls); + index++; + } + + }); }); diff --git a/src/sync/polling/updaters/splitChangesUpdater.ts b/src/sync/polling/updaters/splitChangesUpdater.ts index 3a7e616a..5f6575b3 100644 --- a/src/sync/polling/updaters/splitChangesUpdater.ts +++ b/src/sync/polling/updaters/splitChangesUpdater.ts @@ -127,6 +127,16 @@ export function splitChangesUpdaterFactory( return promise; } + /** Returns true if at least one split was updated */ + function update(flagsChange: [boolean | void, void | boolean[], void | boolean[], boolean | void] | [any, any, any]) { + const [, added, removed, ] = flagsChange; + // There is at least one added or modified feature flag + if (added && added.some((update: boolean) => update)) return true; + // There is at least one removed feature flag + if (removed && removed.some((update: boolean) => update)) return true; + return false; + } + /** * SplitChanges updater returns a promise that resolves with a `false` boolean value if it fails to fetch splits or synchronize them with the storage. * Returned promise will not be rejected. @@ -163,8 +173,9 @@ export function splitChangesUpdaterFactory( splits.addSplits(mutation.added), splits.removeSplits(mutation.removed), segments.registerSegments(mutation.segments) - ]).then(() => { - + ]).then((flagsChange) => { + const triggerSdkUpdate = update(flagsChange); + if (!triggerSdkUpdate) return true; if (splitsEventEmitter) { // To emit SDK_SPLITS_ARRIVED for server-side SDK, we must check that all registered segments have been fetched return Promise.resolve(!splitsEventEmitter.splitsArrived || (since !== splitChanges.till && (isClientSide || checkAllSegmentsExist(segments)))) From 2e7446bb3b58bb8c700f973ffd81838b13a78ee1 Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Wed, 1 Nov 2023 17:42:20 -0300 Subject: [PATCH 2/3] update changes file --- CHANGES.txt | 9 +++++++++ package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 0bb2cde5..92f937b5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,12 @@ +1.11.0 (November XX, 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. + - getTreatmentsByFlagSet and getTreatmentsByFlagSets + - getTreatmentsWithConfigByFlagSets and getTreatmentsWithConfigByFlagSets + - Added a new optional Split Filter configuration option. This allows the SDK and Split services to only synchronize the flags in the specified flag sets, avoiding unused or unwanted flags from being synced on the SDK instance, bringing all the benefits from a reduced payload. + - Note: Only applicable when the SDK is in charge of the rollout data synchronization. When not applicable, the SDK will log a warning on init. + - Added `sets` property to the `SplitView` object returned by the `split` and `splits` methods of the SDK manager to expose flag sets on flag views. + 1.10.0 (October 20, 2023) - Added `defaultTreatment` property to the `SplitView` object returned by the `split` and `splits` methods of the SDK manager (Related to issue https://github.com/splitio/javascript-commons/issues/225). - Updated log warning message to include the feature flag name when `getTreatment` method is called and the SDK client is not ready. diff --git a/package-lock.json b/package-lock.json index 076a3177..31eea521 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.10.1-rc.0", + "version": "1.10.1-rc.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-commons", - "version": "1.10.1-rc.0", + "version": "1.10.1-rc.1", "license": "Apache-2.0", "dependencies": { "tslib": "^2.3.1" diff --git a/package.json b/package.json index 347331d3..5560e64e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.10.1-rc.0", + "version": "1.10.1-rc.1", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", From 065c6e0f6c3ef9011c3605e17952ab2ced0c61ef Mon Sep 17 00:00:00 2001 From: Emmanuel Zamora Date: Thu, 2 Nov 2023 14:57:04 -0300 Subject: [PATCH 3/3] approach improvement --- package-lock.json | 4 ++-- package.json | 2 +- src/sync/polling/updaters/splitChangesUpdater.ts | 6 ++---- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/package-lock.json b/package-lock.json index 31eea521..db3d8b54 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.10.1-rc.1", + "version": "1.10.1-rc.3", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@splitsoftware/splitio-commons", - "version": "1.10.1-rc.1", + "version": "1.10.1-rc.3", "license": "Apache-2.0", "dependencies": { "tslib": "^2.3.1" diff --git a/package.json b/package.json index 5560e64e..2bd2eed9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.10.1-rc.1", + "version": "1.10.1-rc.3", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/sync/polling/updaters/splitChangesUpdater.ts b/src/sync/polling/updaters/splitChangesUpdater.ts index 5f6575b3..583da077 100644 --- a/src/sync/polling/updaters/splitChangesUpdater.ts +++ b/src/sync/polling/updaters/splitChangesUpdater.ts @@ -128,7 +128,7 @@ export function splitChangesUpdaterFactory( } /** Returns true if at least one split was updated */ - function update(flagsChange: [boolean | void, void | boolean[], void | boolean[], boolean | void] | [any, any, any]) { + function isThereUpdate(flagsChange: [boolean | void, void | boolean[], void | boolean[], boolean | void] | [any, any, any]) { const [, added, removed, ] = flagsChange; // There is at least one added or modified feature flag if (added && added.some((update: boolean) => update)) return true; @@ -174,11 +174,9 @@ export function splitChangesUpdaterFactory( splits.removeSplits(mutation.removed), segments.registerSegments(mutation.segments) ]).then((flagsChange) => { - const triggerSdkUpdate = update(flagsChange); - if (!triggerSdkUpdate) return true; if (splitsEventEmitter) { // To emit SDK_SPLITS_ARRIVED for server-side SDK, we must check that all registered segments have been fetched - return Promise.resolve(!splitsEventEmitter.splitsArrived || (since !== splitChanges.till && (isClientSide || checkAllSegmentsExist(segments)))) + return Promise.resolve(!splitsEventEmitter.splitsArrived || (since !== splitChanges.till && isThereUpdate(flagsChange) && (isClientSide || checkAllSegmentsExist(segments)))) .catch(() => false /** noop. just to handle a possible `checkAllSegmentsExist` rejection, before emitting SDK event */) .then(emitSplitsArrivedEvent => { // emit SDK events