From 04ac92243ea28b99d931deafc228b17b80a6bc5f Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Fri, 12 Jul 2024 16:35:00 -0300 Subject: [PATCH] Refactor usesSegments method for usesMatcher --- src/storages/AbstractSplitsCacheAsync.ts | 4 +-- src/storages/AbstractSplitsCacheSync.ts | 9 +++---- src/storages/KeyBuilder.ts | 3 +++ .../inLocalStorage/SplitsCacheInLocal.ts | 25 +++++++++++++++---- .../__tests__/SplitsCacheInLocal.spec.ts | 15 +++++------ src/storages/inMemory/SplitsCacheInMemory.ts | 25 +++++++++++-------- src/storages/types.ts | 6 ++--- src/sync/__tests__/syncManagerOnline.spec.ts | 2 +- src/sync/polling/pollingManagerCS.ts | 9 ++++--- .../polling/updaters/mySegmentsUpdater.ts | 3 ++- src/sync/syncManagerOnline.ts | 6 ++--- src/utils/constants/index.ts | 1 + 12 files changed, 67 insertions(+), 41 deletions(-) diff --git a/src/storages/AbstractSplitsCacheAsync.ts b/src/storages/AbstractSplitsCacheAsync.ts index 9e4e136c..4abb6e34 100644 --- a/src/storages/AbstractSplitsCacheAsync.ts +++ b/src/storages/AbstractSplitsCacheAsync.ts @@ -22,9 +22,9 @@ 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 ('usesMatcher', 'getRegisteredSegments', 'registerSegments') // noop, just keeping the interface. This is used by standalone client-side API only, and so only implemented by InMemory and InLocalStorage. - usesSegments(): Promise { + usesMatcher(): Promise { return Promise.resolve(true); } diff --git a/src/storages/AbstractSplitsCacheSync.ts b/src/storages/AbstractSplitsCacheSync.ts index d516837e..1d33eb93 100644 --- a/src/storages/AbstractSplitsCacheSync.ts +++ b/src/storages/AbstractSplitsCacheSync.ts @@ -2,7 +2,6 @@ import { ISplitsCacheSync } from './types'; import { ISplit } from '../dtos/types'; import { objectAssign } from '../utils/lang/objectAssign'; import { ISet } from '../utils/lang/sets'; -import { IN_SEGMENT } from '../utils/constants'; /** * This class provides a skeletal implementation of the ISplitsCacheSync interface @@ -44,7 +43,7 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync { abstract trafficTypeExists(trafficType: string): boolean - abstract usesSegments(): boolean + abstract usesMatcher(matcherType: string): boolean abstract clear(): void @@ -86,15 +85,15 @@ export abstract class AbstractSplitsCacheSync implements ISplitsCacheSync { /** * Given a parsed split, it returns a boolean flagging if its conditions use segments matchers (rules & whitelists). - * This util is intended to simplify the implementation of `splitsCache::usesSegments` method + * This util is intended to simplify the implementation of `splitsCache::usesMatcher` method */ -export function usesSegments(split: ISplit) { +export function usesMatcher(split: ISplit, matcherType: string) { const conditions = split.conditions || []; for (let i = 0; i < conditions.length; i++) { const matchers = conditions[i].matcherGroup.matchers; for (let j = 0; j < matchers.length; j++) { - if (matchers[j].matcherType === IN_SEGMENT) return true; + if (matchers[j].matcherType === matcherType) return true; } } diff --git a/src/storages/KeyBuilder.ts b/src/storages/KeyBuilder.ts index e70b251b..60ab7af8 100644 --- a/src/storages/KeyBuilder.ts +++ b/src/storages/KeyBuilder.ts @@ -51,6 +51,9 @@ export class KeyBuilder { buildSplitsWithSegmentCountKey() { return `${this.prefix}.splits.usingSegments`; } + buildSplitsWithLargeSegmentCountKey() { + return `${this.prefix}.splits.usingLargeSegments`; + } buildSegmentNameKey(segmentName: string) { return `${this.prefix}.segment.${segmentName}`; diff --git a/src/storages/inLocalStorage/SplitsCacheInLocal.ts b/src/storages/inLocalStorage/SplitsCacheInLocal.ts index ccd4859f..30f47f67 100644 --- a/src/storages/inLocalStorage/SplitsCacheInLocal.ts +++ b/src/storages/inLocalStorage/SplitsCacheInLocal.ts @@ -1,5 +1,5 @@ import { ISplit } from '../../dtos/types'; -import { AbstractSplitsCacheSync, usesSegments } from '../AbstractSplitsCacheSync'; +import { AbstractSplitsCacheSync, usesMatcher } from '../AbstractSplitsCacheSync'; import { isFiniteNumber, toNumber, isNaNNumber } from '../../utils/lang'; import { KeyBuilderCS } from '../KeyBuilderCS'; import { ILogger } from '../../logger/types'; @@ -7,6 +7,7 @@ import { LOG_PREFIX } from './constants'; import { ISet, _Set, setToArray } from '../../utils/lang/sets'; import { ISettings } from '../../types'; import { getStorageHash } from '../KeyBuilder'; +import { IN_LARGE_SEGMENT, IN_SEGMENT } from '../../utils/constants'; /** * ISplitsCacheSync implementation that stores split definitions in browser LocalStorage. @@ -50,10 +51,15 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { const ttKey = this.keys.buildTrafficTypeKey(split.trafficTypeName); this._decrementCount(ttKey); - if (usesSegments(split)) { + if (usesMatcher(split, IN_SEGMENT)) { const segmentsCountKey = this.keys.buildSplitsWithSegmentCountKey(); this._decrementCount(segmentsCountKey); } + + if (usesMatcher(split, IN_LARGE_SEGMENT)) { + const segmentsCountKey = this.keys.buildSplitsWithLargeSegmentCountKey(); + this._decrementCount(segmentsCountKey); + } } } catch (e) { this.log.error(LOG_PREFIX + e); @@ -67,11 +73,17 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { // @ts-expect-error localStorage.setItem(ttKey, toNumber(localStorage.getItem(ttKey)) + 1); - if (usesSegments(split)) { + if (usesMatcher(split, IN_SEGMENT)) { const segmentsCountKey = this.keys.buildSplitsWithSegmentCountKey(); // @ts-expect-error localStorage.setItem(segmentsCountKey, toNumber(localStorage.getItem(segmentsCountKey)) + 1); } + + if (usesMatcher(split, IN_LARGE_SEGMENT)) { + const segmentsCountKey = this.keys.buildSplitsWithLargeSegmentCountKey(); + // @ts-expect-error + localStorage.setItem(segmentsCountKey, toNumber(localStorage.getItem(segmentsCountKey)) + 1); + } } } catch (e) { this.log.error(LOG_PREFIX + e); @@ -203,11 +215,14 @@ export class SplitsCacheInLocal extends AbstractSplitsCacheSync { return isFiniteNumber(ttCount) && ttCount > 0; } - usesSegments() { + usesMatcher(matcherType: string) { // If cache hasn't been synchronized with the cloud, assume we need them. if (!this.hasSync) return true; - const storedCount = localStorage.getItem(this.keys.buildSplitsWithSegmentCountKey()); + const storedCount = localStorage.getItem(matcherType === IN_SEGMENT ? + this.keys.buildSplitsWithSegmentCountKey() : + this.keys.buildSplitsWithLargeSegmentCountKey() + ); const splitsWithSegmentsCount = storedCount === null ? 0 : toNumber(storedCount); if (isFiniteNumber(splitsWithSegmentsCount)) { diff --git a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts index 732ca8b7..bf857888 100644 --- a/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts +++ b/src/storages/inLocalStorage/__tests__/SplitsCacheInLocal.spec.ts @@ -4,6 +4,7 @@ import { splitWithUserTT, splitWithAccountTT, splitWithAccountTTAndUsesSegments, import { ISplit } from '../../../dtos/types'; import { _Set } from '../../../utils/lang/sets'; import { fullSettings } from '../../../utils/settingsValidation/__tests__/settings.mocks'; +import { IN_SEGMENT } from '../../../utils/constants'; test('SPLIT CACHE / LocalStorage', () => { @@ -141,26 +142,26 @@ test('SPLIT CACHE / LocalStorage / killLocally', () => { }); -test('SPLIT CACHE / LocalStorage / usesSegments', () => { +test('SPLIT CACHE / LocalStorage / usesMatcher', () => { const cache = new SplitsCacheInLocal(fullSettings, new KeyBuilderCS('SPLITIO', 'user')); - expect(cache.usesSegments()).toBe(true); // true initially, until data is synchronized + expect(cache.usesMatcher(IN_SEGMENT)).toBe(true); // true initially, until data is synchronized cache.setChangeNumber(1); // to indicate that data has been synced. cache.addSplits([['split1', splitWithUserTT], ['split2', splitWithAccountTT],]); - expect(cache.usesSegments()).toBe(false); // 0 splits using segments + expect(cache.usesMatcher(IN_SEGMENT)).toBe(false); // 0 splits using segments cache.addSplit('split3', splitWithAccountTTAndUsesSegments); - expect(cache.usesSegments()).toBe(true); // 1 split using segments + expect(cache.usesMatcher(IN_SEGMENT)).toBe(true); // 1 split using segments cache.addSplit('split4', splitWithAccountTTAndUsesSegments); - expect(cache.usesSegments()).toBe(true); // 2 splits using segments + expect(cache.usesMatcher(IN_SEGMENT)).toBe(true); // 2 splits using segments cache.removeSplit('split3'); - expect(cache.usesSegments()).toBe(true); // 1 split using segments + expect(cache.usesMatcher(IN_SEGMENT)).toBe(true); // 1 split using segments cache.removeSplit('split4'); - expect(cache.usesSegments()).toBe(false); // 0 splits using segments + expect(cache.usesMatcher(IN_SEGMENT)).toBe(false); // 0 splits using segments }); test('SPLIT CACHE / LocalStorage / flag set cache tests', () => { diff --git a/src/storages/inMemory/SplitsCacheInMemory.ts b/src/storages/inMemory/SplitsCacheInMemory.ts index cf570eea..b8f8c06b 100644 --- a/src/storages/inMemory/SplitsCacheInMemory.ts +++ b/src/storages/inMemory/SplitsCacheInMemory.ts @@ -1,7 +1,8 @@ import { ISplit, ISplitFiltersValidation } from '../../dtos/types'; -import { AbstractSplitsCacheSync, usesSegments } from '../AbstractSplitsCacheSync'; +import { AbstractSplitsCacheSync, usesMatcher } from '../AbstractSplitsCacheSync'; import { isFiniteNumber } from '../../utils/lang'; import { ISet, _Set } from '../../utils/lang/sets'; +import { IN_LARGE_SEGMENT, IN_SEGMENT } from '../../utils/constants'; /** * Default ISplitsCacheSync implementation that stores split definitions in memory. @@ -13,7 +14,8 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync { private splitsCache: Record = {}; private ttCache: Record = {}; private changeNumber: number = -1; - private splitsWithSegmentsCount: number = 0; + private segmentsCount: number = 0; + private largeSegmentsCount: number = 0; private flagSetsCache: Record> = {}; constructor(splitFiltersValidation?: ISplitFiltersValidation) { @@ -25,7 +27,8 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync { this.splitsCache = {}; this.ttCache = {}; this.changeNumber = -1; - this.splitsWithSegmentsCount = 0; + this.segmentsCount = 0; + this.largeSegmentsCount = 0; } addSplit(name: string, split: ISplit): boolean { @@ -38,9 +41,9 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync { this.removeFromFlagSets(previousSplit.name, previousSplit.sets); - if (usesSegments(previousSplit)) { // Substract from segments count for the previous version of this Split. - this.splitsWithSegmentsCount--; - } + // Substract from segments count for the previous version of this Split. + if (usesMatcher(previousSplit, IN_SEGMENT)) this.segmentsCount--; + if (usesMatcher(previousSplit, IN_LARGE_SEGMENT)) this.largeSegmentsCount--; } if (split) { @@ -52,7 +55,8 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync { this.addToFlagSets(split); // Add to segments count for the new version of the Split - if (usesSegments(split)) this.splitsWithSegmentsCount++; + if (usesMatcher(split, IN_SEGMENT)) this.segmentsCount++; + if (usesMatcher(split, IN_LARGE_SEGMENT)) this.largeSegmentsCount++; return true; } else { @@ -72,7 +76,8 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync { this.removeFromFlagSets(split.name, split.sets); // Update the segments count. - if (usesSegments(split)) this.splitsWithSegmentsCount--; + if (usesMatcher(split, IN_SEGMENT)) this.segmentsCount--; + if (usesMatcher(split, IN_LARGE_SEGMENT)) this.largeSegmentsCount--; return true; } else { @@ -101,8 +106,8 @@ export class SplitsCacheInMemory extends AbstractSplitsCacheSync { return isFiniteNumber(this.ttCache[trafficType]) && this.ttCache[trafficType] > 0; } - usesSegments(): boolean { - return this.getChangeNumber() === -1 || this.splitsWithSegmentsCount > 0; + usesMatcher(matcherType: string): boolean { + return this.getChangeNumber() === -1 || (matcherType === IN_SEGMENT ? this.segmentsCount > 0 : this.largeSegmentsCount > 0); } getNamesByFlagSets(flagSets: string[]): ISet[] { diff --git a/src/storages/types.ts b/src/storages/types.ts index 61602e91..10e83604 100644 --- a/src/storages/types.ts +++ b/src/storages/types.ts @@ -205,7 +205,7 @@ export interface ISplitsCacheBase { // should never reject or throw an exception. Instead return true by default, asssuming the TT might exist. trafficTypeExists(trafficType: string): MaybeThenable, // only for Client-Side - usesSegments(): MaybeThenable, + usesMatcher(matcherType: string): MaybeThenable, clear(): MaybeThenable, // should never reject or throw an exception. Instead return false by default, to avoid emitting SDK_READY_FROM_CACHE. checkCache(): MaybeThenable, @@ -223,7 +223,7 @@ export interface ISplitsCacheSync extends ISplitsCacheBase { getAll(): ISplit[], getSplitNames(): string[], trafficTypeExists(trafficType: string): boolean, - usesSegments(): boolean, + usesMatcher(matcherType: string): boolean, clear(): void, checkCache(): boolean, killLocally(name: string, defaultTreatment: string, changeNumber: number): boolean, @@ -240,7 +240,7 @@ export interface ISplitsCacheAsync extends ISplitsCacheBase { getAll(): Promise, getSplitNames(): Promise, trafficTypeExists(trafficType: string): Promise, - usesSegments(): Promise, + usesMatcher(matcherType: string): Promise, clear(): Promise, checkCache(): Promise, killLocally(name: string, defaultTreatment: string, changeNumber: number): Promise, diff --git a/src/sync/__tests__/syncManagerOnline.spec.ts b/src/sync/__tests__/syncManagerOnline.spec.ts index 44b9c8b3..164c93eb 100644 --- a/src/sync/__tests__/syncManagerOnline.spec.ts +++ b/src/sync/__tests__/syncManagerOnline.spec.ts @@ -12,7 +12,7 @@ jest.mock('../submitters/submitterManager', () => { // Mocked storageManager const storageManagerMock = { splits: { - usesSegments: () => false + usesMatcher: () => false } }; diff --git a/src/sync/polling/pollingManagerCS.ts b/src/sync/polling/pollingManagerCS.ts index ac3253f2..1a17490c 100644 --- a/src/sync/polling/pollingManagerCS.ts +++ b/src/sync/polling/pollingManagerCS.ts @@ -8,6 +8,7 @@ import { getMatching } from '../../utils/key'; import { SDK_SPLITS_ARRIVED, SDK_SEGMENTS_ARRIVED } from '../../readiness/constants'; import { POLLING_SMART_PAUSING, POLLING_START, POLLING_STOP } from '../../logger/constants'; import { ISdkFactoryContextSync } from '../../sdkFactory/types'; +import { IN_SEGMENT } from '../../utils/constants'; /** * Expose start / stop mechanism for polling data from services. @@ -43,7 +44,7 @@ export function pollingManagerCSFactory( // smart pausing readiness.splits.on(SDK_SPLITS_ARRIVED, () => { if (!splitsSyncTask.isRunning()) return; // noop if not doing polling - const splitsHaveSegments = storage.splits.usesSegments(); + const splitsHaveSegments = storage.splits.usesMatcher(IN_SEGMENT); if (splitsHaveSegments !== mySegmentsSyncTask.isRunning()) { log.info(POLLING_SMART_PAUSING, [splitsHaveSegments ? 'ON' : 'OFF']); if (splitsHaveSegments) { @@ -59,9 +60,9 @@ export function pollingManagerCSFactory( // smart ready function smartReady() { - if (!readiness.isReady() && !storage.splits.usesSegments()) readiness.segments.emit(SDK_SEGMENTS_ARRIVED); + if (!readiness.isReady() && !storage.splits.usesMatcher(IN_SEGMENT)) readiness.segments.emit(SDK_SEGMENTS_ARRIVED); } - if (!storage.splits.usesSegments()) setTimeout(smartReady, 0); + if (!storage.splits.usesMatcher(IN_SEGMENT)) setTimeout(smartReady, 0); else readiness.splits.once(SDK_SPLITS_ARRIVED, smartReady); mySegmentsSyncTasks[matchingKey] = mySegmentsSyncTask; @@ -77,7 +78,7 @@ export function pollingManagerCSFactory( log.info(POLLING_START); splitsSyncTask.start(); - if (storage.splits.usesSegments()) startMySegmentsSyncTasks(); + if (storage.splits.usesMatcher(IN_SEGMENT)) startMySegmentsSyncTasks(); }, // Stop periodic fetching (polling) diff --git a/src/sync/polling/updaters/mySegmentsUpdater.ts b/src/sync/polling/updaters/mySegmentsUpdater.ts index 421e0c3f..ab62b1fc 100644 --- a/src/sync/polling/updaters/mySegmentsUpdater.ts +++ b/src/sync/polling/updaters/mySegmentsUpdater.ts @@ -6,6 +6,7 @@ import { SDK_SEGMENTS_ARRIVED } from '../../../readiness/constants'; import { ILogger } from '../../../logger/types'; import { SYNC_MYSEGMENTS_FETCH_RETRY } from '../../../logger/constants'; import { MySegmentsData } from '../types'; +import { IN_SEGMENT } from '../../../utils/constants'; type IMySegmentsUpdater = (segmentList?: string[], noCache?: boolean) => Promise @@ -55,7 +56,7 @@ export function mySegmentsUpdaterFactory( } // Notify update if required - if (splitsCache.usesSegments() && (shouldNotifyUpdate || readyOnAlreadyExistentState)) { + if (splitsCache.usesMatcher(IN_SEGMENT) && (shouldNotifyUpdate || readyOnAlreadyExistentState)) { readyOnAlreadyExistentState = false; segmentsEventEmitter.emit(SDK_SEGMENTS_ARRIVED); } diff --git a/src/sync/syncManagerOnline.ts b/src/sync/syncManagerOnline.ts index b6407630..d399e699 100644 --- a/src/sync/syncManagerOnline.ts +++ b/src/sync/syncManagerOnline.ts @@ -7,7 +7,7 @@ import { IPollingManager, IPollingManagerCS } from './polling/types'; import { PUSH_SUBSYSTEM_UP, PUSH_SUBSYSTEM_DOWN } from './streaming/constants'; import { SYNC_START_POLLING, SYNC_CONTINUE_POLLING, SYNC_STOP_POLLING } from '../logger/constants'; import { isConsentGranted } from '../consent'; -import { POLLING, STREAMING, SYNC_MODE_UPDATE } from '../utils/constants'; +import { IN_SEGMENT, POLLING, STREAMING, SYNC_MODE_UPDATE } from '../utils/constants'; import { ISdkFactoryContextSync } from '../sdkFactory/types'; /** @@ -150,7 +150,7 @@ export function syncManagerOnlineFactory( if (pushManager) { if (pollingManager!.isRunning()) { // if doing polling, we must start the periodic fetch of data - if (storage.splits.usesSegments()) mySegmentsSyncTask.start(); + if (storage.splits.usesMatcher(IN_SEGMENT)) mySegmentsSyncTask.start(); } else { // if not polling, we must execute the sync task for the initial fetch // of segments since `syncAll` was already executed when starting the main client @@ -158,7 +158,7 @@ export function syncManagerOnlineFactory( } pushManager.add(matchingKey, mySegmentsSyncTask); } else { - if (storage.splits.usesSegments()) mySegmentsSyncTask.start(); + if (storage.splits.usesMatcher(IN_SEGMENT)) mySegmentsSyncTask.start(); } } else { if (!readinessManager.isReady()) mySegmentsSyncTask.execute(); diff --git a/src/utils/constants/index.ts b/src/utils/constants/index.ts index 77bda4e1..51decc18 100644 --- a/src/utils/constants/index.ts +++ b/src/utils/constants/index.ts @@ -109,3 +109,4 @@ export const FLAG_SPEC_VERSION = '1.1'; // Matcher types export const IN_SEGMENT = 'IN_SEGMENT'; +export const IN_LARGE_SEGMENT = 'IN_LARGE_SEGMENT';