diff --git a/CHANGES.txt b/CHANGES.txt index 499e296f..13016b54 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,6 @@ +2.4.1 (XXX XX, 2025) + - Updated synchronization to avoid fetching dereferenced segments in server-side, which were resulting in 404 response errors. + 2.4.0 (May 27, 2025) - Added support for rule-based segments. These segments determine membership at runtime by evaluating their configured rules against the user attributes provided to the SDK. - Added support for feature flag prerequisites. This allows customers to define dependency conditions between flags, which are evaluated before any allowlists or targeting rules. diff --git a/src/storages/AbstractMySegmentsCacheSync.ts b/src/storages/AbstractMySegmentsCacheSync.ts index 7d3dc304..a3cfa630 100644 --- a/src/storages/AbstractMySegmentsCacheSync.ts +++ b/src/storages/AbstractMySegmentsCacheSync.ts @@ -11,6 +11,7 @@ export abstract class AbstractMySegmentsCacheSync implements ISegmentsCacheSync protected abstract addSegment(name: string): boolean protected abstract removeSegment(name: string): boolean protected abstract setChangeNumber(changeNumber?: number): boolean | void + protected abstract getSegments(): string[] /** * For server-side synchronizer: check if `key` is in `name` segment. @@ -27,14 +28,14 @@ export abstract class AbstractMySegmentsCacheSync implements ISegmentsCacheSync // No-op. Not used in client-side. - registerSegments(): boolean { return false; } update() { return false; } /** - * For server-side synchronizer: get the list of segments to fetch changes. - * Also used for the `seC` (segment count) telemetry stat. + * Used for the `seC` (segment count) telemetry stat. */ - abstract getRegisteredSegments(): string[] + getSegmentsCount() { + return this.getSegments().length; + } /** * Only used for the `skC`(segment keys count) telemetry stat: 1 for client-side, and total count of keys in server-side. @@ -68,7 +69,7 @@ export abstract class AbstractMySegmentsCacheSync implements ISegmentsCacheSync } const names = ((segmentsData as IMySegmentsResponse).k || []).map(s => s.n).sort(); - const storedSegmentKeys = this.getRegisteredSegments().sort(); + const storedSegmentKeys = this.getSegments().sort(); // Extreme fast => everything is empty if (!names.length && !storedSegmentKeys.length) return false; diff --git a/src/storages/AbstractSplitsCacheAsync.ts b/src/storages/AbstractSplitsCacheAsync.ts index 420b9202..94c0713b 100644 --- a/src/storages/AbstractSplitsCacheAsync.ts +++ b/src/storages/AbstractSplitsCacheAsync.ts @@ -31,7 +31,7 @@ export abstract class AbstractSplitsCacheAsync implements ISplitsCacheAsync { abstract trafficTypeExists(trafficType: string): Promise abstract clear(): Promise - // @TODO revisit segment-related methods ('usesSegments', 'getRegisteredSegments', 'registerSegments') + // @TODO revisit segment-related methods ('usesSegments') // noop, just keeping the interface. This is used by standalone client-side API only, and so only implemented by InMemory and InLocalStorage. usesSegments(): Promise { return Promise.resolve(true); diff --git a/src/storages/KeyBuilderSS.ts b/src/storages/KeyBuilderSS.ts index cf8d2156..16d00b14 100644 --- a/src/storages/KeyBuilderSS.ts +++ b/src/storages/KeyBuilderSS.ts @@ -29,10 +29,6 @@ export class KeyBuilderSS extends KeyBuilder { this.versionablePrefix = `${metadata.s}/${metadata.n}/${metadata.i}`; } - buildRegisteredSegmentsKey() { - return `${this.prefix}.segments.registered`; - } - buildImpressionsKey() { return `${this.prefix}.impressions`; } diff --git a/src/storages/__tests__/KeyBuilder.spec.ts b/src/storages/__tests__/KeyBuilder.spec.ts index bd21fa66..b473b165 100644 --- a/src/storages/__tests__/KeyBuilder.spec.ts +++ b/src/storages/__tests__/KeyBuilder.spec.ts @@ -36,15 +36,8 @@ test('KEYS / segments keys', () => { const expectedKey = `SPLITIO.segment.${segmentName}`; const expectedTill = `SPLITIO.segment.${segmentName}.till`; - const expectedSegmentRegistered = 'SPLITIO.segments.registered'; - expect(builder.buildSegmentNameKey(segmentName) === expectedKey).toBe(true); expect(builder.buildSegmentTillKey(segmentName) === expectedTill).toBe(true); - expect(builder.buildRegisteredSegmentsKey() === expectedSegmentRegistered).toBe(true); - - // NOT USED - // const expectedReady = 'SPLITIO.segments.ready'; - // expect(builder.buildSegmentsReady() === expectedReady).toBe(true); }); test('KEYS / traffic type keys', () => { diff --git a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts index e3b250b5..af506a6d 100644 --- a/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/MySegmentsCacheInLocal.ts @@ -46,7 +46,7 @@ export class MySegmentsCacheInLocal extends AbstractMySegmentsCacheSync { return localStorage.getItem(this.keys.buildSegmentNameKey(name)) === DEFINED; } - getRegisteredSegments(): string[] { + protected getSegments(): string[] { // Scan current values from localStorage return Object.keys(localStorage).reduce((accum, key) => { let segmentName = this.keys.extractSegmentName(key); diff --git a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts index 37f6ad8e..11b1b72c 100644 --- a/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts +++ b/src/storages/inLocalStorage/RBSegmentsCacheInLocal.ts @@ -105,6 +105,10 @@ export class RBSegmentsCacheInLocal implements IRBSegmentsCacheSync { return item && JSON.parse(item); } + getAll(): IRBSegment[] { + return this.getNames().map(name => this.get(name) as IRBSegment); + } + contains(names: Set): boolean { const namesArray = setToArray(names); const namesInStorage = this.getNames(); diff --git a/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts index bb38fe10..4d5aa3a7 100644 --- a/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/MySegmentsCacheInLocal.spec.ts @@ -17,7 +17,6 @@ test('SEGMENT CACHE / in LocalStorage', () => { expect(cache.getChangeNumber()).toBe(-1); expect(cache.isInSegment('mocked-segment')).toBe(true); - expect(cache.getRegisteredSegments()).toEqual(['mocked-segment', 'mocked-segment-2']); expect(cache.getKeysCount()).toBe(1); }); @@ -29,7 +28,6 @@ test('SEGMENT CACHE / in LocalStorage', () => { }); expect(cache.isInSegment('mocked-segment')).toBe(false); - expect(cache.getRegisteredSegments()).toEqual(['mocked-segment-2']); expect(cache.getKeysCount()).toBe(1); }); diff --git a/src/storages/inMemory/MySegmentsCacheInMemory.ts b/src/storages/inMemory/MySegmentsCacheInMemory.ts index 546a83c3..4785ef78 100644 --- a/src/storages/inMemory/MySegmentsCacheInMemory.ts +++ b/src/storages/inMemory/MySegmentsCacheInMemory.ts @@ -38,7 +38,7 @@ export class MySegmentsCacheInMemory extends AbstractMySegmentsCacheSync { return this.cn || -1; } - getRegisteredSegments() { + protected getSegments() { return Object.keys(this.segmentCache); } diff --git a/src/storages/inMemory/RBSegmentsCacheInMemory.ts b/src/storages/inMemory/RBSegmentsCacheInMemory.ts index 568b0deb..b1855e50 100644 --- a/src/storages/inMemory/RBSegmentsCacheInMemory.ts +++ b/src/storages/inMemory/RBSegmentsCacheInMemory.ts @@ -51,6 +51,10 @@ export class RBSegmentsCacheInMemory implements IRBSegmentsCacheSync { return this.cache[name] || null; } + getAll(): IRBSegment[] { + return this.getNames().map(name => this.get(name) as IRBSegment); + } + contains(names: Set): boolean { const namesArray = setToArray(names); const namesInStorage = this.getNames(); diff --git a/src/storages/inMemory/SegmentsCacheInMemory.ts b/src/storages/inMemory/SegmentsCacheInMemory.ts index d25b89d2..62da0317 100644 --- a/src/storages/inMemory/SegmentsCacheInMemory.ts +++ b/src/storages/inMemory/SegmentsCacheInMemory.ts @@ -36,24 +36,8 @@ export class SegmentsCacheInMemory implements ISegmentsCacheSync { this.segmentChangeNumber = {}; } - private _registerSegment(name: string) { - if (!this.segmentCache[name]) { - this.segmentCache[name] = new Set(); - } - - return true; - } - - registerSegments(names: string[]) { - for (let i = 0; i < names.length; i++) { - this._registerSegment(names[i]); - } - - return true; - } - - getRegisteredSegments() { - return Object.keys(this.segmentCache); + getSegmentsCount() { + return Object.keys(this.segmentCache).length; } getKeysCount() { diff --git a/src/storages/inMemory/TelemetryCacheInMemory.ts b/src/storages/inMemory/TelemetryCacheInMemory.ts index 7e7e3f98..4a6b1c68 100644 --- a/src/storages/inMemory/TelemetryCacheInMemory.ts +++ b/src/storages/inMemory/TelemetryCacheInMemory.ts @@ -49,9 +49,9 @@ export class TelemetryCacheInMemory implements ITelemetryCacheSync { iDe: this.getImpressionStats(DEDUPED), iDr: this.getImpressionStats(DROPPED), spC: this.splits && this.splits.getSplitNames().length, - seC: this.segments && this.segments.getRegisteredSegments().length, + seC: this.segments && this.segments.getSegmentsCount(), skC: this.segments && this.segments.getKeysCount(), - lsC: this.largeSegments && this.largeSegments.getRegisteredSegments().length, + lsC: this.largeSegments && this.largeSegments.getSegmentsCount(), lskC: this.largeSegments && this.largeSegments.getKeysCount(), sL: this.getSessionLength(), eQ: this.getEventStats(QUEUED), diff --git a/src/storages/inMemory/__tests__/MySegmentsCacheInMemory.spec.ts b/src/storages/inMemory/__tests__/MySegmentsCacheInMemory.spec.ts index b936d17c..ecc1cdec 100644 --- a/src/storages/inMemory/__tests__/MySegmentsCacheInMemory.spec.ts +++ b/src/storages/inMemory/__tests__/MySegmentsCacheInMemory.spec.ts @@ -9,16 +9,13 @@ test('MY SEGMENTS CACHE / in memory', () => { expect(cache.getChangeNumber()).toBe(-1); expect(cache.isInSegment('mocked-segment')).toBe(true); - expect(cache.getRegisteredSegments()).toEqual(['mocked-segment', 'mocked-segment-2']); expect(cache.getKeysCount()).toBe(1); expect(cache.resetSegments({ k: [{ n: 'mocked-segment-2' }], cn: 150})).toBe(true); expect(cache.isInSegment('mocked-segment')).toBe(false); - expect(cache.getRegisteredSegments()).toEqual(['mocked-segment-2']); expect(cache.getKeysCount()).toBe(1); cache.clear(); - expect(cache.getRegisteredSegments()).toEqual([]); expect(cache.getChangeNumber()).toBe(-1); }); diff --git a/src/storages/inMemory/__tests__/SegmentsCacheInMemory.spec.ts b/src/storages/inMemory/__tests__/SegmentsCacheInMemory.spec.ts index 5ee2683c..9052de67 100644 --- a/src/storages/inMemory/__tests__/SegmentsCacheInMemory.spec.ts +++ b/src/storages/inMemory/__tests__/SegmentsCacheInMemory.spec.ts @@ -29,16 +29,4 @@ describe('SEGMENTS CACHE IN MEMORY', () => { expect(cache.getKeysCount()).toBe(0); }); - test('registerSegment / getRegisteredSegments', async () => { - const cache = new SegmentsCacheInMemory(); - - await cache.registerSegments(['s1']); - await cache.registerSegments(['s2']); - await cache.registerSegments(['s2', 's3', 's4']); - - const segments = cache.getRegisteredSegments(); - - ['s1', 's2', 's3', 's4'].forEach(s => expect(segments.indexOf(s) !== -1).toBe(true)); - }); - }); diff --git a/src/storages/inRedis/RBSegmentsCacheInRedis.ts b/src/storages/inRedis/RBSegmentsCacheInRedis.ts index dc36f64c..3c39a0e5 100644 --- a/src/storages/inRedis/RBSegmentsCacheInRedis.ts +++ b/src/storages/inRedis/RBSegmentsCacheInRedis.ts @@ -30,6 +30,12 @@ export class RBSegmentsCacheInRedis implements IRBSegmentsCacheAsync { ); } + getAll(): Promise { + return this.getNames().then(names => { + return Promise.all(names.map(name => this.get(name) as Promise)); + }); + } + contains(names: Set): Promise { const namesArray = setToArray(names); return this.getNames().then(namesInStorage => { diff --git a/src/storages/inRedis/SegmentsCacheInRedis.ts b/src/storages/inRedis/SegmentsCacheInRedis.ts index d645495f..62c61f33 100644 --- a/src/storages/inRedis/SegmentsCacheInRedis.ts +++ b/src/storages/inRedis/SegmentsCacheInRedis.ts @@ -51,18 +51,6 @@ export class SegmentsCacheInRedis implements ISegmentsCacheAsync { }); } - registerSegments(segments: string[]) { - if (segments.length) { - return this.redis.sadd(this.keys.buildRegisteredSegmentsKey(), segments).then(() => true); - } else { - return Promise.resolve(true); - } - } - - getRegisteredSegments() { - return this.redis.smembers(this.keys.buildRegisteredSegmentsKey()); - } - // @TODO remove or implement. It is not being used. clear() { return Promise.resolve(); diff --git a/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts b/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts index f31d2c91..d98ba0b4 100644 --- a/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts +++ b/src/storages/inRedis/__tests__/SegmentsCacheInRedis.spec.ts @@ -36,21 +36,4 @@ describe('SEGMENTS CACHE IN REDIS', () => { await connection.disconnect(); }); - test('registerSegment / getRegisteredSegments', async () => { - const connection = new RedisAdapter(loggerMock); - const cache = new SegmentsCacheInRedis(loggerMock, keys, connection); - - await cache.registerSegments(['s1']); - await cache.registerSegments(['s2']); - await cache.registerSegments(['s2', 's3', 's4']); - - const segments = await cache.getRegisteredSegments(); - - ['s1', 's2', 's3', 's4'].forEach(s => expect(segments.indexOf(s) !== -1).toBe(true)); - - // Teardown - await connection.del(await connection.keys(`${prefix}.segment*`)); // @TODO use `cache.clear` method when implemented - await connection.disconnect(); - }); - }); diff --git a/src/storages/pluggable/RBSegmentsCachePluggable.ts b/src/storages/pluggable/RBSegmentsCachePluggable.ts index c1967f6d..fb1a16df 100644 --- a/src/storages/pluggable/RBSegmentsCachePluggable.ts +++ b/src/storages/pluggable/RBSegmentsCachePluggable.ts @@ -29,6 +29,12 @@ export class RBSegmentsCachePluggable implements IRBSegmentsCacheAsync { ); } + getAll(): Promise { + return this.getNames().then(names => { + return Promise.all(names.map(name => this.get(name) as Promise)); + }); + } + contains(names: Set): Promise { const namesArray = setToArray(names); return this.getNames().then(namesInStorage => { diff --git a/src/storages/pluggable/SegmentsCachePluggable.ts b/src/storages/pluggable/SegmentsCachePluggable.ts index 145953d2..d6bf8727 100644 --- a/src/storages/pluggable/SegmentsCachePluggable.ts +++ b/src/storages/pluggable/SegmentsCachePluggable.ts @@ -62,26 +62,6 @@ export class SegmentsCachePluggable implements ISegmentsCacheAsync { }); } - /** - * Add the given segment names to the set of registered segments. - * The returned promise is resolved when the operation success, - * or rejected if it fails (e.g., wrapper operation fails). - */ - registerSegments(segments: string[]) { - if (segments.length) { - return this.wrapper.addItems(this.keys.buildRegisteredSegmentsKey(), segments); - } else { - return Promise.resolve(); - } - } - - /** - * Returns a promise that resolves with the set of registered segments in a list, - * or rejected if it fails (e.g., wrapper operation fails). - */ - getRegisteredSegments() { - return this.wrapper.getItems(this.keys.buildRegisteredSegmentsKey()); - } // @TODO implement if required by DataLoader or Producer mode clear(): Promise { diff --git a/src/storages/pluggable/__tests__/SegmentsCachePluggable.spec.ts b/src/storages/pluggable/__tests__/SegmentsCachePluggable.spec.ts index 459f59eb..984c5f91 100644 --- a/src/storages/pluggable/__tests__/SegmentsCachePluggable.spec.ts +++ b/src/storages/pluggable/__tests__/SegmentsCachePluggable.spec.ts @@ -37,16 +37,4 @@ describe('SEGMENTS CACHE PLUGGABLE', () => { expect(await cache.isInSegment('inexistent-segment', 'a')).toBe(false); }); - test('registerSegment / getRegisteredSegments', async () => { - const cache = new SegmentsCachePluggable(loggerMock, keyBuilder, wrapperMock); - - await cache.registerSegments(['s1']); - await cache.registerSegments(['s2']); - await cache.registerSegments(['s2', 's3', 's4']); - - const segments = await cache.getRegisteredSegments(); - - ['s1', 's2', 's3', 's4'].forEach(s => expect(segments.indexOf(s) !== -1).toBe(true)); - }); - }); diff --git a/src/storages/types.ts b/src/storages/types.ts index 8e93daca..28decc48 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -229,6 +229,7 @@ export interface IRBSegmentsCacheBase { getChangeNumber(): MaybeThenable, clear(): MaybeThenable, contains(names: Set): MaybeThenable, + getAll(): MaybeThenable, } export interface IRBSegmentsCacheSync extends IRBSegmentsCacheBase { @@ -237,6 +238,7 @@ export interface IRBSegmentsCacheSync extends IRBSegmentsCacheBase { getChangeNumber(): number, clear(): void, contains(names: Set): boolean, + getAll(): IRBSegment[], // Used only for smart pausing in client-side standalone. Returns true if the storage contains a RBSegment using segments or large segments matchers usesSegments(): boolean, } @@ -247,14 +249,13 @@ export interface IRBSegmentsCacheAsync extends IRBSegmentsCacheBase { getChangeNumber(): Promise, clear(): Promise, contains(names: Set): Promise, + getAll(): Promise, } /** Segments cache */ export interface ISegmentsCacheBase { isInSegment(name: string, key?: string): MaybeThenable // different signature on Server and Client-Side - registerSegments(names: string[]): MaybeThenable // only for Server-Side - getRegisteredSegments(): MaybeThenable // only for Server-Side getChangeNumber(name: string): MaybeThenable // only for Server-Side update(name: string, addedKeys: string[], removedKeys: string[], changeNumber: number): MaybeThenable // only for Server-Side clear(): MaybeThenable @@ -263,19 +264,17 @@ export interface ISegmentsCacheBase { // Same API for both variants: SegmentsCache and MySegmentsCache (client-side API) export interface ISegmentsCacheSync extends ISegmentsCacheBase { isInSegment(name: string, key?: string): boolean - registerSegments(names: string[]): boolean - getRegisteredSegments(): string[] - getKeysCount(): number // only used for telemetry getChangeNumber(name?: string): number | undefined update(name: string, addedKeys: string[], removedKeys: string[], changeNumber: number): boolean // only for Server-Side resetSegments(segmentsData: MySegmentsData | IMySegmentsResponse): boolean // only for Sync Client-Side clear(): void + // only used for telemetry: + getKeysCount(): number + getSegmentsCount(): number } export interface ISegmentsCacheAsync extends ISegmentsCacheBase { isInSegment(name: string, key: string): Promise - registerSegments(names: string[]): Promise - getRegisteredSegments(): Promise getChangeNumber(name: string): Promise update(name: string, addedKeys: string[], removedKeys: string[], changeNumber: number): Promise clear(): Promise diff --git a/src/sync/polling/syncTasks/segmentsSyncTask.ts b/src/sync/polling/syncTasks/segmentsSyncTask.ts index f5a93711..c7844a39 100644 --- a/src/sync/polling/syncTasks/segmentsSyncTask.ts +++ b/src/sync/polling/syncTasks/segmentsSyncTask.ts @@ -21,7 +21,7 @@ export function segmentsSyncTaskFactory( segmentChangesUpdaterFactory( settings.log, segmentChangesFetcherFactory(fetchSegmentChanges), - storage.segments, + storage, readiness, ), settings.scheduler.segmentsRefreshRate, diff --git a/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts b/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts index b93a7176..d4fa2eff 100644 --- a/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts +++ b/src/sync/polling/updaters/__tests__/splitChangesUpdater.spec.ts @@ -4,7 +4,7 @@ import { splitApiFactory } from '../../../../services/splitApi'; import { SegmentsCacheInMemory } from '../../../../storages/inMemory/SegmentsCacheInMemory'; import { SplitsCacheInMemory } from '../../../../storages/inMemory/SplitsCacheInMemory'; import { splitChangesFetcherFactory } from '../../fetchers/splitChangesFetcher'; -import { splitChangesUpdaterFactory, parseSegments, computeMutation } from '../splitChangesUpdater'; +import { splitChangesUpdaterFactory, parseSegments, computeMutation, getRegisteredSegments } from '../splitChangesUpdater'; import splitChangesMock1 from '../../../../__tests__/mocks/splitchanges.since.-1.json'; import fetchMock from '../../../../__tests__/testUtils/fetchMock'; import { fullSettings, settingsSplitApi } from '../../../../utils/settingsValidation/__tests__/settings.mocks'; @@ -15,6 +15,7 @@ import { splitNotifications } from '../../../streaming/__tests__/dataMocks'; import { RBSegmentsCacheInMemory } from '../../../../storages/inMemory/RBSegmentsCacheInMemory'; import { RB_SEGMENT_UPDATE, SPLIT_UPDATE } from '../../../streaming/constants'; import { IN_RULE_BASED_SEGMENT } from '../../../../utils/constants'; +import { splitWithAccountTTAndUsesSegments } from '../../../../storages/__tests__/testUtils'; const ARCHIVED_FF = 'ARCHIVED'; @@ -101,7 +102,7 @@ const rbsWithExcludedSegment: IRBSegment = { } }; -test('splitChangesUpdater / segments parser', () => { +test('segments parser', () => { let segments = parseSegments(activeSplitWithSegments as ISplit); expect(segments).toEqual(new Set(['A', 'B'])); @@ -112,68 +113,80 @@ test('splitChangesUpdater / segments parser', () => { expect(segments).toEqual(new Set(['D'])); }); -test('splitChangesUpdater / compute splits mutation', () => { +test('compute splits mutation', () => { const splitFiltersValidation = { queryString: null, groupedFilters: { bySet: [], byName: [], byPrefix: [] }, validFilters: [] }; - let segments = new Set(); - let splitsMutation = computeMutation([activeSplitWithSegments, archivedSplit] as ISplit[], segments, splitFiltersValidation); + let splitsMutation = computeMutation([activeSplitWithSegments, archivedSplit] as ISplit[], splitFiltersValidation); expect(splitsMutation.added).toEqual([activeSplitWithSegments]); expect(splitsMutation.removed).toEqual([archivedSplit]); - expect(Array.from(segments)).toEqual(['A', 'B']); // SDK initialization without sets // should process all the notifications - segments = new Set(); - splitsMutation = computeMutation([testFFSetsAB, test2FFSetsX] as ISplit[], segments, splitFiltersValidation); + splitsMutation = computeMutation([testFFSetsAB, test2FFSetsX] as ISplit[], splitFiltersValidation); expect(splitsMutation.added).toEqual([testFFSetsAB, test2FFSetsX]); expect(splitsMutation.removed).toEqual([]); - expect(Array.from(segments)).toEqual([]); }); -test('splitChangesUpdater / compute splits mutation with filters', () => { +test('compute splits mutation with filters', () => { // SDK initialization with sets: [set_a, set_b] let splitFiltersValidation = { queryString: '&sets=set_a,set_b', groupedFilters: { bySet: ['set_a', 'set_b'], byName: ['name_1'], byPrefix: [] }, validFilters: [] }; // fetching new feature flag in sets A & B - let splitsMutation = computeMutation([testFFSetsAB], new Set(), splitFiltersValidation); + let splitsMutation = computeMutation([testFFSetsAB], splitFiltersValidation); // should add it to mutations expect(splitsMutation.added).toEqual([testFFSetsAB]); expect(splitsMutation.removed).toEqual([]); // fetching existing test feature flag removed from set B - splitsMutation = computeMutation([testFFRemoveSetB], new Set(), splitFiltersValidation); + splitsMutation = computeMutation([testFFRemoveSetB], splitFiltersValidation); expect(splitsMutation.added).toEqual([testFFRemoveSetB]); expect(splitsMutation.removed).toEqual([]); // fetching existing test feature flag removed from set B - splitsMutation = computeMutation([testFFRemoveSetA], new Set(), splitFiltersValidation); + splitsMutation = computeMutation([testFFRemoveSetA], splitFiltersValidation); expect(splitsMutation.added).toEqual([]); expect(splitsMutation.removed).toEqual([testFFRemoveSetA]); // fetching existing test feature flag removed from set B - splitsMutation = computeMutation([testFFEmptySet], new Set(), splitFiltersValidation); + splitsMutation = computeMutation([testFFEmptySet], splitFiltersValidation); expect(splitsMutation.added).toEqual([]); expect(splitsMutation.removed).toEqual([testFFEmptySet]); // SDK initialization with names: ['test2'] splitFiltersValidation = { queryString: '&names=test2', groupedFilters: { bySet: [], byName: ['test2'], byPrefix: [] }, validFilters: [] }; - splitsMutation = computeMutation([testFFSetsAB], new Set(), splitFiltersValidation); + splitsMutation = computeMutation([testFFSetsAB], splitFiltersValidation); expect(splitsMutation.added).toEqual([]); expect(splitsMutation.removed).toEqual([testFFSetsAB]); - splitsMutation = computeMutation([test2FFSetsX, testFFEmptySet], new Set(), splitFiltersValidation); + splitsMutation = computeMutation([test2FFSetsX, testFFEmptySet], splitFiltersValidation); expect(splitsMutation.added).toEqual([test2FFSetsX]); expect(splitsMutation.removed).toEqual([testFFEmptySet]); }); +test('get registered segments', async () => { + const storage = { + splits: { + getAll: () => Promise.resolve([splitWithAccountTTAndUsesSegments]) + }, + rbSegments: { + getAll: () => Promise.resolve([rbsWithExcludedSegment]) + } + }; + + // @ts-expect-error + const segments = await getRegisteredSegments(storage); + + expect(segments.sort()).toEqual(['C', 'employees']); +}); + describe('splitChangesUpdater', () => { const splits = new SplitsCacheInMemory(); const updateSplits = jest.spyOn(splits, 'update'); @@ -182,7 +195,6 @@ describe('splitChangesUpdater', () => { const updateRbSegments = jest.spyOn(rbSegments, 'update'); const segments = new SegmentsCacheInMemory(); - const registerSegments = jest.spyOn(segments, 'registerSegments'); const storage = { splits, rbSegments, segments }; @@ -210,7 +222,6 @@ describe('splitChangesUpdater', () => { expect(updateSplits).toBeCalledTimes(1); expect(updateSplits).lastCalledWith(splitChangesMock1.ff.d, [], splitChangesMock1.ff.t); expect(updateRbSegments).toBeCalledTimes(0); // no rbSegments to update - expect(registerSegments).toBeCalledTimes(1); expect(splitsEmitSpy).toBeCalledWith('state::splits-arrived'); expect(result).toBe(true); }); @@ -234,9 +245,6 @@ describe('splitChangesUpdater', () => { expect(updateSplits.mock.calls[index][0].length).toBe(payload.status === ARCHIVED_FF ? 0 : 1); // Remove feature flag if status is ARCHIVED expect(updateSplits.mock.calls[index][1]).toEqual(payload.status === ARCHIVED_FF ? [payload] : []); - // fetch segments after feature flag update - expect(registerSegments).toBeCalledTimes(index + 1); - expect(registerSegments.mock.calls[index][0]).toEqual(payload.status === ARCHIVED_FF ? [] : ['maur-2']); index++; } }); @@ -253,9 +261,6 @@ describe('splitChangesUpdater', () => { expect(updateRbSegments).toBeCalledTimes(1); expect(updateRbSegments).toBeCalledWith([payload], [], changeNumber); - - expect(registerSegments).toBeCalledTimes(1); - expect(registerSegments).toBeCalledWith([]); }); test('flag sets splits-arrived emission', async () => { diff --git a/src/sync/polling/updaters/segmentChangesUpdater.ts b/src/sync/polling/updaters/segmentChangesUpdater.ts index ab951b24..c0576fe5 100644 --- a/src/sync/polling/updaters/segmentChangesUpdater.ts +++ b/src/sync/polling/updaters/segmentChangesUpdater.ts @@ -1,9 +1,10 @@ import { ISegmentChangesFetcher } from '../fetchers/types'; -import { ISegmentsCacheBase } from '../../../storages/types'; +import { IStorageBase } from '../../../storages/types'; import { IReadinessManager } from '../../../readiness/types'; import { SDK_SEGMENTS_ARRIVED } from '../../../readiness/constants'; import { ILogger } from '../../../logger/types'; import { LOG_PREFIX_INSTANTIATION, LOG_PREFIX_SYNC_SEGMENTS } from '../../../logger/constants'; +import { getRegisteredSegments } from './splitChangesUpdater'; type ISegmentChangesUpdater = (fetchOnlyNew?: boolean, segmentName?: string, noCache?: boolean, till?: number) => Promise @@ -21,9 +22,10 @@ type ISegmentChangesUpdater = (fetchOnlyNew?: boolean, segmentName?: string, noC export function segmentChangesUpdaterFactory( log: ILogger, segmentChangesFetcher: ISegmentChangesFetcher, - segments: ISegmentsCacheBase, + storage: Pick, readiness?: IReadinessManager, ): ISegmentChangesUpdater { + const { segments } = storage; let readyOnAlreadyExistentState = true; @@ -60,7 +62,7 @@ export function segmentChangesUpdaterFactory( log.debug(`${LOG_PREFIX_SYNC_SEGMENTS}Started segments update`); // If not a segment name provided, read list of available segments names to be updated. - let segmentsPromise = Promise.resolve(segmentName ? [segmentName] : segments.getRegisteredSegments()); + let segmentsPromise = Promise.resolve(segmentName ? [segmentName] : getRegisteredSegments(storage)); return segmentsPromise.then(segmentNames => { // Async fetchers diff --git a/src/sync/polling/updaters/splitChangesUpdater.ts b/src/sync/polling/updaters/splitChangesUpdater.ts index ea5e5e44..1143b342 100644 --- a/src/sync/polling/updaters/splitChangesUpdater.ts +++ b/src/sync/polling/updaters/splitChangesUpdater.ts @@ -1,4 +1,4 @@ -import { ISegmentsCacheBase, IStorageBase } from '../../../storages/types'; +import { IStorageBase } from '../../../storages/types'; import { ISplitChangesFetcher } from '../fetchers/types'; import { IRBSegment, ISplit, ISplitChangesResponse, ISplitFiltersValidation, MaybeThenable } from '../../../dtos/types'; import { ISplitsEventEmitter } from '../../../readiness/types'; @@ -17,10 +17,9 @@ type SplitChangesUpdater = (noCache?: boolean, till?: number, instantUpdate?: In // Checks that all registered segments have been fetched (changeNumber !== -1 for every segment). // Returns a promise that could be rejected. // @TODO review together with Segments and MySegments storage APIs -function checkAllSegmentsExist(segments: ISegmentsCacheBase): Promise { - let registeredSegments = Promise.resolve(segments.getRegisteredSegments()); - return registeredSegments.then(segmentNames => { - return Promise.all(segmentNames.map(segmentName => segments.getChangeNumber(segmentName))) +function checkAllSegmentsExist(storage: Pick): Promise { + return getRegisteredSegments(storage).then(segmentNames => { + return Promise.all(segmentNames.map(segmentName => storage.segments.getChangeNumber(segmentName))) .then(changeNumbers => changeNumbers.every(changeNumber => changeNumber !== undefined)); }); } @@ -52,6 +51,19 @@ export function parseSegments(ruleEntity: ISplit | IRBSegment, matcherType: type return segments; } +export function getRegisteredSegments(storage: Pick) { + return Promise.all([storage.splits.getAll(), storage.rbSegments.getAll()]).then(([splits, rbSegments]) => { + let segments = (splits as (ISplit | IRBSegment)[]).concat(rbSegments).reduce((segments, ruleEntity) => { + parseSegments(ruleEntity).forEach((segmentName: string) => { + segments.add(segmentName); + }); + return segments; + }, new Set()); + + return setToArray(segments); + }); +} + interface ISplitMutations { added: T[], removed: T[] @@ -83,15 +95,11 @@ function matchFilters(featureFlag: ISplit, filters: ISplitFiltersValidation) { * i.e., an object with added splits, removed splits and used segments. * Exported for testing purposes. */ -export function computeMutation(rules: Array, segments: Set, filters?: ISplitFiltersValidation): ISplitMutations { +export function computeMutation(rules: Array, filters?: ISplitFiltersValidation): ISplitMutations { return rules.reduce((accum, ruleEntity) => { if (ruleEntity.status === 'ACTIVE' && (!filters || matchFilters(ruleEntity as ISplit, filters))) { accum.added.push(ruleEntity); - - parseSegments(ruleEntity).forEach((segmentName: string) => { - segments.add(segmentName); - }); } else { accum.removed.push(ruleEntity); } @@ -124,7 +132,7 @@ export function splitChangesUpdaterFactory( retriesOnFailureBeforeReady: number = 0, isClientSide?: boolean ): SplitChangesUpdater { - const { splits, rbSegments, segments } = storage; + const { splits, rbSegments } = storage; let startingUp = true; @@ -165,29 +173,24 @@ export function splitChangesUpdaterFactory( .then((splitChanges: ISplitChangesResponse) => { startingUp = false; - const usedSegments = new Set(); - let ffUpdate: MaybeThenable = false; if (splitChanges.ff) { - const { added, removed } = computeMutation(splitChanges.ff.d, usedSegments, splitFiltersValidation); + const { added, removed } = computeMutation(splitChanges.ff.d, splitFiltersValidation); log.debug(SYNC_SPLITS_UPDATE, [added.length, removed.length]); ffUpdate = splits.update(added, removed, splitChanges.ff.t); } let rbsUpdate: MaybeThenable = false; if (splitChanges.rbs) { - const { added, removed } = computeMutation(splitChanges.rbs.d, usedSegments); + const { added, removed } = computeMutation(splitChanges.rbs.d); log.debug(SYNC_RBS_UPDATE, [added.length, removed.length]); rbsUpdate = rbSegments.update(added, removed, splitChanges.rbs.t); } - return Promise.all([ffUpdate, rbsUpdate, - // @TODO if at least 1 segment fetch fails due to 404 and other segments are updated in the storage, SDK_UPDATE is not emitted - segments.registerSegments(setToArray(usedSegments)) - ]).then(([ffChanged, rbsChanged]) => { + return Promise.all([ffUpdate, rbsUpdate]).then(([ffChanged, rbsChanged]) => { 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 || ((ffChanged || rbsChanged) && (isClientSide || checkAllSegmentsExist(segments)))) + return Promise.resolve(!splitsEventEmitter.splitsArrived || ((ffChanged || rbsChanged) && (isClientSide || checkAllSegmentsExist(storage)))) .catch(() => false /** noop. just to handle a possible `checkAllSegmentsExist` rejection, before emitting SDK event */) .then(emitSplitsArrivedEvent => { // emit SDK events