From ac5659a8647c4c26610a9dbcfe4d3916d6e0fccb Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 11 Dec 2024 15:16:06 -0300 Subject: [PATCH 1/2] Polishing --- src/storages/inMemory/InMemoryStorage.ts | 4 ++-- src/storages/inMemory/InMemoryStorageCS.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/storages/inMemory/InMemoryStorage.ts b/src/storages/inMemory/InMemoryStorage.ts index b8833370..7ec099d1 100644 --- a/src/storages/inMemory/InMemoryStorage.ts +++ b/src/storages/inMemory/InMemoryStorage.ts @@ -37,8 +37,8 @@ export function InMemoryStorageFactory(params: IStorageFactoryParams): IStorageS const noopTrack = () => true; storage.impressions.track = noopTrack; storage.events.track = noopTrack; - if (storage.impressionCounts) storage.impressionCounts.track = noopTrack; - if (storage.uniqueKeys) storage.uniqueKeys.track = noopTrack; + storage.impressionCounts.track = noopTrack; + storage.uniqueKeys.track = noopTrack; } return storage; diff --git a/src/storages/inMemory/InMemoryStorageCS.ts b/src/storages/inMemory/InMemoryStorageCS.ts index 09d62ac8..bfaec159 100644 --- a/src/storages/inMemory/InMemoryStorageCS.ts +++ b/src/storages/inMemory/InMemoryStorageCS.ts @@ -55,8 +55,8 @@ export function InMemoryStorageCSFactory(params: IStorageFactoryParams): IStorag const noopTrack = () => true; storage.impressions.track = noopTrack; storage.events.track = noopTrack; - if (storage.impressionCounts) storage.impressionCounts.track = noopTrack; - if (storage.uniqueKeys) storage.uniqueKeys.track = noopTrack; + storage.impressionCounts.track = noopTrack; + storage.uniqueKeys.track = noopTrack; } return storage; From 522debdf3f97d2e669631eb0c7d128f32a8ad930 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 11 Dec 2024 15:26:31 -0300 Subject: [PATCH 2/2] New type ImpressionDecorated --- src/sdkClient/client.ts | 36 ++++++++++--------- .../__tests__/impressionsTracker.spec.ts | 16 ++++----- src/trackers/impressionsTracker.ts | 6 ++-- src/trackers/types.ts | 13 ++++++- 4 files changed, 43 insertions(+), 28 deletions(-) diff --git a/src/sdkClient/client.ts b/src/sdkClient/client.ts index 5216d2d2..7a280122 100644 --- a/src/sdkClient/client.ts +++ b/src/sdkClient/client.ts @@ -11,6 +11,7 @@ import { IMPRESSION, IMPRESSION_QUEUEING } from '../logger/constants'; import { ISdkFactoryContext } from '../sdkFactory/types'; import { isConsumerMode } from '../utils/settingsValidation/mode'; import { Method } from '../sync/submitters/types'; +import { ImpressionDecorated } from '../trackers/types'; const treatmentNotReady = { treatment: CONTROL, label: SDK_NOT_READY }; @@ -34,11 +35,11 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENT_WITH_CONFIG : TREATMENT); const wrapUp = (evaluationResult: IEvaluationResult) => { - const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = []; + const queue: ImpressionDecorated[] = []; const treatment = processEvaluation(evaluationResult, featureFlagName, key, attributes, withConfig, methodName, queue); impressionsTracker.track(queue, attributes); - stopTelemetryTracker(queue[0] && queue[0][0].label); + stopTelemetryTracker(queue[0] && queue[0].imp.label); return treatment; }; @@ -59,14 +60,14 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENTS_WITH_CONFIG : TREATMENTS); const wrapUp = (evaluationResults: Record) => { - const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = []; + const queue: ImpressionDecorated[] = []; const treatments: Record = {}; Object.keys(evaluationResults).forEach(featureFlagName => { treatments[featureFlagName] = processEvaluation(evaluationResults[featureFlagName], featureFlagName, key, attributes, withConfig, methodName, queue); }); impressionsTracker.track(queue, attributes); - stopTelemetryTracker(queue[0] && queue[0][0].label); + stopTelemetryTracker(queue[0] && queue[0].imp.label); return treatments; }; @@ -87,7 +88,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl const stopTelemetryTracker = telemetryTracker.trackEval(method); const wrapUp = (evaluationResults: Record) => { - const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = []; + const queue: ImpressionDecorated[] = []; const treatments: Record = {}; const evaluations = evaluationResults; Object.keys(evaluations).forEach(featureFlagName => { @@ -95,7 +96,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl }); impressionsTracker.track(queue, attributes); - stopTelemetryTracker(queue[0] && queue[0][0].label); + stopTelemetryTracker(queue[0] && queue[0].imp.label); return treatments; }; @@ -128,7 +129,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl attributes: SplitIO.Attributes | undefined, withConfig: boolean, invokingMethodName: string, - queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] + queue: ImpressionDecorated[] ): SplitIO.Treatment | SplitIO.TreatmentWithConfig { const matchingKey = getMatching(key); const bucketingKey = getBucketing(key); @@ -138,15 +139,18 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl if (validateSplitExistence(log, readinessManager, featureFlagName, label, invokingMethodName)) { log.info(IMPRESSION_QUEUEING); - queue.push([{ - feature: featureFlagName, - keyName: matchingKey, - treatment, - time: Date.now(), - bucketingKey, - label, - changeNumber: changeNumber as number, - }, track]); + queue.push({ + imp: { + feature: featureFlagName, + keyName: matchingKey, + treatment, + time: Date.now(), + bucketingKey, + label, + changeNumber: changeNumber as number, + }, + track + }); } if (withConfig) { diff --git a/src/trackers/__tests__/impressionsTracker.spec.ts b/src/trackers/__tests__/impressionsTracker.spec.ts index 8a0cfd51..4d54bf68 100644 --- a/src/trackers/__tests__/impressionsTracker.spec.ts +++ b/src/trackers/__tests__/impressionsTracker.spec.ts @@ -70,7 +70,7 @@ describe('Impressions Tracker', () => { expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker - tracker.track([[imp1], [imp2], [imp3]]); + tracker.track([{ imp: imp1 }, { imp: imp2 }, { imp: imp3 }]); expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([imp1, imp2, imp3]); // Should call the storage track method once we invoke .track() method, passing queued params in a sequence. }); @@ -93,7 +93,7 @@ describe('Impressions Tracker', () => { expect(fakeIntegrationsManager.handleImpression).not.toBeCalled(); // The integrations manager handleImpression method should not be invoked if we haven't tracked impressions. // We signal that we actually want to track the queued impressions. - tracker.track([[fakeImpression], [fakeImpression2]], fakeAttributes); + tracker.track([{ imp: fakeImpression }, { imp: fakeImpression2 }], fakeAttributes); expect(fakeImpressionsCache.track.mock.calls[0][0]).toEqual([fakeImpression, fakeImpression2]); // Even with a listener, impression should be sent to the cache expect(fakeListener.logImpression).not.toBeCalled(); // The listener should not be executed synchronously. @@ -157,7 +157,7 @@ describe('Impressions Tracker', () => { expect(fakeImpressionsCache.track).not.toBeCalled(); // storage method should not be called until impressions are tracked. trackers.forEach(tracker => { - tracker.track([[impression], [impression2], [impression3]]); + tracker.track([{ imp: impression }, { imp: impression2 }, { imp: impression3 }]); const lastArgs = fakeImpressionsCache.track.mock.calls[fakeImpressionsCache.track.mock.calls.length - 1]; @@ -182,7 +182,7 @@ describe('Impressions Tracker', () => { expect(fakeImpressionsCache.track).not.toBeCalled(); // cache method should not be called by just creating a tracker - tracker.track([[impression], [impression2], [impression3]]); + tracker.track([{ imp: impression }, { imp: impression2 }, { imp: impression3 }]); const lastArgs = fakeImpressionsCache.track.mock.calls[fakeImpressionsCache.track.mock.calls.length - 1]; @@ -203,19 +203,19 @@ describe('Impressions Tracker', () => { const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache, strategy, fakeWhenInit); - tracker.track([[impression]]); + tracker.track([{ imp: impression }]); expect(fakeImpressionsCache.track).toBeCalledTimes(1); // impression should be tracked if userConsent is undefined settings.userConsent = 'UNKNOWN'; - tracker.track([[impression]]); + tracker.track([{ imp: impression }]); expect(fakeImpressionsCache.track).toBeCalledTimes(2); // impression should be tracked if userConsent is unknown settings.userConsent = 'GRANTED'; - tracker.track([[impression]]); + tracker.track([{ imp: impression }]); expect(fakeImpressionsCache.track).toBeCalledTimes(3); // impression should be tracked if userConsent is granted settings.userConsent = 'DECLINED'; - tracker.track([[impression]]); + tracker.track([{ imp: impression }]); expect(fakeImpressionsCache.track).toBeCalledTimes(3); // impression should not be tracked if userConsent is declined }); diff --git a/src/trackers/impressionsTracker.ts b/src/trackers/impressionsTracker.ts index 54a5ed0b..1d3980cf 100644 --- a/src/trackers/impressionsTracker.ts +++ b/src/trackers/impressionsTracker.ts @@ -1,7 +1,7 @@ import { objectAssign } from '../utils/lang/objectAssign'; import { thenable } from '../utils/promise/thenable'; import { IImpressionsCacheBase, ITelemetryCacheSync, ITelemetryCacheAsync } from '../storages/types'; -import { IImpressionsHandler, IImpressionsTracker, IStrategy } from './types'; +import { IImpressionsHandler, IImpressionsTracker, ImpressionDecorated, IStrategy } from './types'; import { ISettings } from '../types'; import { IMPRESSIONS_TRACKER_SUCCESS, ERROR_IMPRESSIONS_TRACKER, ERROR_IMPRESSIONS_LISTENER } from '../logger/constants'; import { CONSENT_DECLINED, DEDUPED, QUEUED } from '../utils/constants'; @@ -28,10 +28,10 @@ export function impressionsTrackerFactory( const { log, impressionListener, runtime: { ip, hostname }, version } = settings; return { - track(imps: [impression: SplitIO.ImpressionDTO, track?: boolean][], attributes?: SplitIO.Attributes) { + track(imps: ImpressionDecorated[], attributes?: SplitIO.Attributes) { if (settings.userConsent === CONSENT_DECLINED) return; - const impressions = imps.map((item) => item[0]); + const impressions = imps.map((item) => item.imp); const impressionsCount = impressions.length; const { impressionsToStore, impressionsToListener, deduped } = strategy.process(impressions); diff --git a/src/trackers/types.ts b/src/trackers/types.ts index 71c5534a..a53c1486 100644 --- a/src/trackers/types.ts +++ b/src/trackers/types.ts @@ -17,8 +17,19 @@ export interface IImpressionsHandler { handleImpression(impressionData: SplitIO.ImpressionData): any } +export type ImpressionDecorated = { + /** + * Impression DTO + */ + imp: SplitIO.ImpressionDTO, + /** + * Whether the impression should be tracked or not + */ + track?: boolean +}; + export interface IImpressionsTracker { - track(impressions: [impression: SplitIO.ImpressionDTO, track?: boolean][], attributes?: SplitIO.Attributes): void + track(impressions: ImpressionDecorated[], attributes?: SplitIO.Attributes): void } /** Telemetry tracker */