From 6387ebb7eed765eb375ca024c485ce61b24e1b9f Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 18 Oct 2024 09:01:02 -0700 Subject: [PATCH] Refactoring. --- src/__tests__/LDClient-events-test.js | 16 ++++++++ src/index.js | 59 ++++++++++++++++----------- 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/src/__tests__/LDClient-events-test.js b/src/__tests__/LDClient-events-test.js index 9ce8f31..e882baf 100644 --- a/src/__tests__/LDClient-events-test.js +++ b/src/__tests__/LDClient-events-test.js @@ -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 }, diff --git a/src/index.js b/src/index.js index dc10e07..d77a37d 100644 --- a/src/index.js +++ b/src/index.js @@ -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); @@ -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; @@ -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); } @@ -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; } } @@ -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 @@ -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; @@ -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; @@ -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; @@ -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 @@ -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.` ); } @@ -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(); }