Skip to content

Commit

Permalink
Merge branch 'impressions_toggle_strategy_refactors' into impressions…
Browse files Browse the repository at this point in the history
…_toggle_manager_and_listener
  • Loading branch information
EmilianoSanchez committed Dec 11, 2024
2 parents 5eb34bf + 334ec45 commit 2b1a189
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 25 deletions.
19 changes: 9 additions & 10 deletions src/sdkClient/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl
const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENT_WITH_CONFIG : TREATMENT);

const wrapUp = (evaluationResult: IEvaluationResult) => {
const queue: SplitIO.ImpressionDTO[] = [];
const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = [];
const treatment = processEvaluation(evaluationResult, featureFlagName, key, attributes, withConfig, methodName, queue);
impressionsTracker.track(queue, attributes);

stopTelemetryTracker(queue[0] && queue[0].label);
stopTelemetryTracker(queue[0] && queue[0][0].label);
return treatment;
};

Expand All @@ -59,14 +59,14 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl
const stopTelemetryTracker = telemetryTracker.trackEval(withConfig ? TREATMENTS_WITH_CONFIG : TREATMENTS);

const wrapUp = (evaluationResults: Record<string, IEvaluationResult>) => {
const queue: SplitIO.ImpressionDTO[] = [];
const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = [];
const treatments: Record<string, SplitIO.Treatment | SplitIO.TreatmentWithConfig> = {};
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].label);
stopTelemetryTracker(queue[0] && queue[0][0].label);
return treatments;
};

Expand All @@ -87,15 +87,15 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl
const stopTelemetryTracker = telemetryTracker.trackEval(method);

const wrapUp = (evaluationResults: Record<string, IEvaluationResult>) => {
const queue: SplitIO.ImpressionDTO[] = [];
const queue: [impression: SplitIO.ImpressionDTO, track?: boolean][] = [];
const treatments: Record<string, SplitIO.Treatment | SplitIO.TreatmentWithConfig> = {};
const evaluations = evaluationResults;
Object.keys(evaluations).forEach(featureFlagName => {
treatments[featureFlagName] = processEvaluation(evaluations[featureFlagName], featureFlagName, key, attributes, withConfig, methodName, queue);
});
impressionsTracker.track(queue, attributes);

stopTelemetryTracker(queue[0] && queue[0].label);
stopTelemetryTracker(queue[0] && queue[0][0].label);
return treatments;
};

Expand Down Expand Up @@ -128,7 +128,7 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl
attributes: SplitIO.Attributes | undefined,
withConfig: boolean,
invokingMethodName: string,
queue: SplitIO.ImpressionDTO[]
queue: [impression: SplitIO.ImpressionDTO, track?: boolean][]
): SplitIO.Treatment | SplitIO.TreatmentWithConfig {
const matchingKey = getMatching(key);
const bucketingKey = getBucketing(key);
Expand All @@ -138,16 +138,15 @@ export function clientFactory(params: ISdkFactoryContext): SplitIO.IClient | Spl

if (validateSplitExistence(log, readinessManager, featureFlagName, label, invokingMethodName)) {
log.info(IMPRESSION_QUEUEING);
queue.push({
queue.push([{
feature: featureFlagName,
keyName: matchingKey,
treatment,
time: Date.now(),
bucketingKey,
label,
changeNumber: changeNumber as number,
track
});
}, track]);
}

if (withConfig) {
Expand Down
16 changes: 8 additions & 8 deletions src/trackers/__tests__/impressionsTracker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,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([[imp1], [imp2], [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.
});
Expand All @@ -98,7 +98,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([[fakeImpression], [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.
Expand Down Expand Up @@ -162,7 +162,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([[impression], [impression2], [impression3]]);

const lastArgs = fakeImpressionsCache.track.mock.calls[fakeImpressionsCache.track.mock.calls.length - 1];

Expand All @@ -187,7 +187,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([[impression], [impression2], [impression3]]);

const lastArgs = fakeImpressionsCache.track.mock.calls[fakeImpressionsCache.track.mock.calls.length - 1];

Expand All @@ -208,19 +208,19 @@ describe('Impressions Tracker', () => {

const tracker = impressionsTrackerFactory(settings, fakeImpressionsCache, fakeNoneStrategy, strategy, fakeWhenInit);

tracker.track([impression]);
tracker.track([[impression]]);
expect(fakeImpressionsCache.track).toBeCalledTimes(1); // impression should be tracked if userConsent is undefined

settings.userConsent = 'UNKNOWN';
tracker.track([impression]);
tracker.track([[impression]]);
expect(fakeImpressionsCache.track).toBeCalledTimes(2); // impression should be tracked if userConsent is unknown

settings.userConsent = 'GRANTED';
tracker.track([impression]);
tracker.track([[impression]]);
expect(fakeImpressionsCache.track).toBeCalledTimes(3); // impression should be tracked if userConsent is granted

settings.userConsent = 'DECLINED';
tracker.track([impression]);
tracker.track([[impression]]);
expect(fakeImpressionsCache.track).toBeCalledTimes(3); // impression should not be tracked if userConsent is declined
});

Expand Down
10 changes: 5 additions & 5 deletions src/trackers/impressionsTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ export function impressionsTrackerFactory(
const { log, impressionListener, runtime: { ip, hostname }, version } = settings;

return {
track(impressions: SplitIO.ImpressionDTO[], attributes?: SplitIO.Attributes) {
track(impressions: [impression: SplitIO.ImpressionDTO, track?: boolean][], attributes?: SplitIO.Attributes) {
if (settings.userConsent === CONSENT_DECLINED) return;

const impressionsToStore = impressions.filter((impression) => {
return impression.track === false ?
const impressionsToStore = impressions.filter(([impression, track]) => {
return track === false ?
noneStrategy.process(impression) :
strategy.process(impression);
});
Expand All @@ -36,7 +36,7 @@ export function impressionsTrackerFactory(
const impressionsToStoreLength = impressionsToStore.length;

if (impressionsToStoreLength) {
const res = impressionsCache.track(impressionsToStore);
const res = impressionsCache.track(impressionsToStore.map((item) => item[0]));

// If we're on an async storage, handle error and log it.
if (thenable(res)) {
Expand All @@ -60,7 +60,7 @@ export function impressionsTrackerFactory(
for (let i = 0; i < impressionsLength; i++) {
const impressionData: SplitIO.ImpressionData = {
// copy of impression, to avoid unexpected behaviour if modified by integrations or impressionListener
impression: objectAssign({}, impressions[i]),
impression: objectAssign({}, impressions[i][0]),
attributes,
ip,
hostname,
Expand Down
2 changes: 1 addition & 1 deletion src/trackers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export interface IImpressionsHandler {
}

export interface IImpressionsTracker {
track(impressions: SplitIO.ImpressionDTO[], attributes?: SplitIO.Attributes): void
track(impressions: [impression: SplitIO.ImpressionDTO, track?: boolean][], attributes?: SplitIO.Attributes): void
}

/** Telemetry tracker */
Expand Down
1 change: 0 additions & 1 deletion types/splitio.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,6 @@ declare namespace SplitIO {
label: string;
changeNumber: number;
pt?: number;
track?: boolean;
}
/**
* Object with information about an impression. It contains the generated impression DTO as well as
Expand Down

0 comments on commit 2b1a189

Please sign in to comment.