Skip to content

Commit

Permalink
Refactoring.
Browse files Browse the repository at this point in the history
  • Loading branch information
kinyoklion committed Oct 18, 2024
1 parent 2b68e5e commit 6387ebb
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 24 deletions.
16 changes: 16 additions & 0 deletions src/__tests__/LDClient-events-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,22 @@ describe('LDClient events', () => {
});
});

it('does not send duplicate events for prerequisites with all flags.', async () => {
const initData = makeBootstrap({
foo: { value: 'a', variation: 1, version: 2 },
bar: { value: 'b', variation: 1, version: 3, prerequisites: ['foo'] },
});
await withClientAndEventProcessor(user, { bootstrap: initData }, async (client, ep) => {
await client.waitForInitialization(5);
client.allFlags();

expect(ep.events.length).toEqual(3);
expectIdentifyEvent(ep.events[0], user);
expectFeatureEvent({ e: ep.events[1], key: 'foo', user, value: 'a', variation: 1, version: 2, defaultVal: null });
expectFeatureEvent({ e: ep.events[2], key: 'bar', user, value: 'b', variation: 1, version: 3, defaultVal: null });
});
});

it('does not send feature events for allFlags() if sendEventsOnlyForVariation is set', async () => {
const initData = makeBootstrap({
foo: { value: 'a', variation: 1, version: 2 },
Expand Down
59 changes: 35 additions & 24 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {
const diagnosticsAccumulator = diagnosticsEnabled ? diagnostics.DiagnosticsAccumulator(new Date().getTime()) : null;
const diagnosticsManager = diagnosticsEnabled
? diagnostics.DiagnosticsManager(
platform,
persistentStorage,
diagnosticsAccumulator,
eventSender,
environment,
options,
diagnosticId
)
platform,
persistentStorage,
diagnosticsAccumulator,
eventSender,
environment,
options,
diagnosticId
)
: null;

const stream = Stream(platform, options, environment, diagnosticsAccumulator);
Expand Down Expand Up @@ -299,14 +299,14 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {
}

function variation(key, defaultValue) {
return variationDetailInternal(key, defaultValue, true, false, true).value;
return variationDetailInternal(key, defaultValue, true, false, false, true).value;
}

function variationDetail(key, defaultValue) {
return variationDetailInternal(key, defaultValue, true, true, true);
return variationDetailInternal(key, defaultValue, true, true, false, true);
}

function variationDetailInternal(key, defaultValue, sendEvent, includeReasonInEvent, notifyInspectionUsed) {
function variationDetailInternal(key, defaultValue, sendEvent, includeReasonInEvent, isAllFlags, notifyInspection) {
let detail;
let flag;

Expand All @@ -321,14 +321,18 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {
}

if (sendEvent) {
flag?.prerequisites?.forEach(key => {
variationDetailInternal(key, undefined, sendEvent, false, false);
});
// For an all-flags evaluation, with events enabled, each flag will get an event, so we do not
// need to duplicate the prerequisites.
if (!isAllFlags) {
flag?.prerequisites?.forEach(key => {
variationDetailInternal(key, undefined, sendEvent, false, false, false);
});
}
sendFlagEvent(key, detail, defaultValue, includeReasonInEvent);
}

// For the all flags case `onFlags` will be called instead.
if (notifyInspectionUsed) {
if (!isAllFlags && notifyInspection) {
notifyInspectionFlagUsed(key, detail);
}

Expand All @@ -355,7 +359,14 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {

for (const key in flags) {
if (utils.objectHasOwnProperty(flags, key) && !flags[key].deleted) {
results[key] = variationDetailInternal(key, null, !options.sendEventsOnlyForVariation, false, false).value;
results[key] = variationDetailInternal(
key,
null,
!options.sendEventsOnlyForVariation,
false,
true,
false
).value;
}
}

Expand Down Expand Up @@ -419,7 +430,7 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {
}
};
stream.connect(ident.getContext(), hash, {
ping: function() {
ping: function () {
logger.debug(messages.debugStreamPing());
const contextAtTimeOfPingEvent = ident.getContext();
requestor
Expand All @@ -435,7 +446,7 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {
emitter.maybeReportError(new errors.LDFlagFetchError(messages.errorFetchingFlags(err)));
});
},
put: function(e) {
put: function (e) {
const data = tryParseData(e.data);
if (!data) {
return;
Expand All @@ -445,7 +456,7 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {
// Don't wait for this Promise to be resolved; note that replaceAllFlags is guaranteed
// never to have an unhandled rejection
},
patch: function(e) {
patch: function (e) {
const data = tryParseData(e.data);
if (!data) {
return;
Expand All @@ -472,7 +483,7 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {
logger.debug(messages.debugStreamPatchIgnored(data.key));
}
},
delete: function(e) {
delete: function (e) {
const data = tryParseData(e.data);
if (!data) {
return;
Expand Down Expand Up @@ -529,7 +540,7 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {

notifyInspectionFlagsChanged();

return handleFlagChanges(changes).catch(() => {}); // swallow any exceptions from this Promise
return handleFlagChanges(changes).catch(() => { }); // swallow any exceptions from this Promise
}

// Returns a Promise which will be resolved when we have dispatched all change events and updated
Expand Down Expand Up @@ -785,8 +796,8 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {
if (timeout > highTimeoutThreshold) {
logger.warn(
'The waitForInitialization function was called with a timeout greater than ' +
`${highTimeoutThreshold} seconds. We recommend a timeout of ` +
`${highTimeoutThreshold} seconds or less.`
`${highTimeoutThreshold} seconds. We recommend a timeout of ` +
`${highTimeoutThreshold} seconds or less.`
);
}

Expand All @@ -810,7 +821,7 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {
}
logger.warn(
'The waitForInitialization function was called without a timeout specified.' +
' In a future version a default timeout will be applied.'
' In a future version a default timeout will be applied.'
);
return initializationStateTracker.getInitializationPromise();
}
Expand Down

0 comments on commit 6387ebb

Please sign in to comment.