From 7979d986f66d6b41403c5b0efb99af0811905916 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 11 Apr 2025 15:16:32 -0700 Subject: [PATCH 01/26] feat: Add support for multiple context event summaries. --- packages/sdk/browser/src/BrowserClient.ts | 4 +- .../browser/src/platform/BrowserRequests.ts | 1 + .../shared/common/__tests__/Context.test.ts | 193 ++++++++++++++++++ .../internal/events/EventSummarizer.test.ts | 20 +- .../shared/common/__tests__/setupCrypto.ts | 21 +- .../shared/common/src/AttributeReference.ts | 4 + packages/shared/common/src/Context.ts | 66 ++++++ .../src/api/subsystem/LDEventSummarizer.ts | 54 +++++ .../src/internal/events/EventProcessor.ts | 29 ++- .../src/internal/events/EventSummarizer.ts | 61 +++--- .../src/internal/events/LDEventSummarizer.ts | 0 .../events/MultiEventSummarizer.test.ts | 131 ++++++++++++ .../internal/events/MultiEventSummarizer.ts | 42 ++++ .../shared/sdk-client/src/LDClientImpl.ts | 2 + .../src/events/createEventProcessor.ts | 1 + 15 files changed, 567 insertions(+), 62 deletions(-) create mode 100644 packages/shared/common/src/api/subsystem/LDEventSummarizer.ts create mode 100644 packages/shared/common/src/internal/events/LDEventSummarizer.ts create mode 100644 packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts create mode 100644 packages/shared/common/src/internal/events/MultiEventSummarizer.ts diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index 0ef3490e99..6593cd18eb 100644 --- a/packages/sdk/browser/src/BrowserClient.ts +++ b/packages/sdk/browser/src/BrowserClient.ts @@ -117,12 +117,14 @@ export class BrowserClient extends LDClientImpl implements LDClient { const platform = overridePlatform ?? new BrowserPlatform(logger); const validatedBrowserOptions = validateOptions(options, logger); + console.log('validatedBrowserOptions', validatedBrowserOptions); const { eventUrlTransformer } = validatedBrowserOptions; + console.log('filtered options', filterToBaseOptions({ ...options, logger })); super( clientSideId, autoEnvAttributes, platform, - filterToBaseOptions({ ...options, logger }), + filterToBaseOptions({ ...validatedBrowserOptions, logger }), ( flagManager: FlagManager, configuration: Configuration, diff --git a/packages/sdk/browser/src/platform/BrowserRequests.ts b/packages/sdk/browser/src/platform/BrowserRequests.ts index 5e73467843..cbb455c252 100644 --- a/packages/sdk/browser/src/platform/BrowserRequests.ts +++ b/packages/sdk/browser/src/platform/BrowserRequests.ts @@ -11,6 +11,7 @@ import DefaultBrowserEventSource from './DefaultBrowserEventSource'; export default class BrowserRequests implements Requests { fetch(url: string, options?: Options): Promise { + delete options?.compressBodyIfPossible; // @ts-ignore return fetch(url, options); } diff --git a/packages/shared/common/__tests__/Context.test.ts b/packages/shared/common/__tests__/Context.test.ts index 2bb450e47b..41b863f20e 100644 --- a/packages/shared/common/__tests__/Context.test.ts +++ b/packages/shared/common/__tests__/Context.test.ts @@ -1,5 +1,6 @@ import AttributeReference from '../src/AttributeReference'; import Context from '../src/Context'; +import { setupCrypto } from './setupCrypto'; // A sample of invalid characters. const invalidSampleChars = [ @@ -325,3 +326,195 @@ describe('given a multi context', () => { expect(Context.toLDContext(input)).toEqual(expected); }); }); + +describe('given mock crypto', () => { + const crypto = setupCrypto(); + + it('two equal contexts hash the same', async () => { + const a = Context.fromLDContext({ + kind: 'multi', + org: { + key: 'testKey', + name: 'testName', + cat: 'calico', + dog: 'lab', + anonymous: true, + _meta: { + privateAttributes: ['/a/b/c', 'cat', 'custom/dog'], + }, + }, + customer: { + key: 'testKey', + name: 'testName', + bird: 'party parrot', + chicken: 'hen', + }, + }); + + const b = Context.fromLDContext({ + kind: 'multi', + org: { + key: 'testKey', + name: 'testName', + cat: 'calico', + dog: 'lab', + anonymous: true, + _meta: { + privateAttributes: ['/a/b/c', 'cat', 'custom/dog'], + }, + }, + customer: { + key: 'testKey', + name: 'testName', + bird: 'party parrot', + chicken: 'hen', + }, + }); + expect(await a.hash(crypto)).toEqual(await b.hash(crypto)); + }); + + it('legacy and non-legacy equivalent contexts hash the same', async () => { + const legacy = Context.fromLDContext({ + key: 'testKey', + name: 'testName', + custom: { cat: 'calico', dog: 'lab' }, + anonymous: true, + privateAttributeNames: ['cat', 'dog'], + }); + + const nonLegacy = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + name: 'testName', + cat: 'calico', + dog: 'lab', + anonymous: true, + _meta: { + privateAttributes: ['cat', 'dog'], + }, + }); + + expect(await legacy.hash(crypto)).toEqual(await nonLegacy.hash(crypto)); + }); + + it('single context and multi-context with one kind hash the same', async () => { + const single = Context.fromLDContext({ + kind: 'org', + key: 'testKey', + name: 'testName', + value: 'testValue', + }); + + const multi = Context.fromLDContext({ + kind: 'multi', + org: { + key: 'testKey', + name: 'testName', + value: 'testValue', + }, + }); + + expect(await single.hash(crypto)).toEqual(await multi.hash(crypto)); + }); + + it('handles shared references without getting stuck', async () => { + const sharedObject = { value: 'shared' }; + const context = Context.fromLDContext({ + kind: 'multi', + org: { + key: 'testKey', + shared: sharedObject, + }, + user: { + key: 'testKey', + shared: sharedObject, + }, + }); + + const hash = await context.hash(crypto); + expect(hash).toBeDefined(); + }); + + it('returns undefined for contexts with cycles', async () => { + const cyclicObject: any = { value: 'cyclic' }; + cyclicObject.self = cyclicObject; + + const context = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + cyclic: cyclicObject, + }); + + expect(await context.hash(crypto)).toBeUndefined(); + }); + + it('handles nested objects correctly', async () => { + const context = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + nested: { + level1: { + level2: { + value: 'deep' + } + } + } + }); + + const hash = await context.hash(crypto); + expect(hash).toBeDefined(); + }); + + it('handles arrays correctly', async () => { + const context = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + array: [1, 2, 3], + nestedArray: [[1, 2], [3, 4]] + }); + + const hash = await context.hash(crypto); + expect(hash).toBeDefined(); + }); + + it('handles primitive values correctly', async () => { + const context = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + string: 'test', + number: 42, + boolean: true, + nullValue: null, + undefinedValue: undefined + }); + + const hash = await context.hash(crypto); + expect(hash).toBeDefined(); + }); + + it('includes private attributes in hash calculation', async () => { + const baseContext = { + kind: 'user', + key: 'testKey', + name: 'testName', + nested: { + value: 'testValue' + } + }; + + const contextWithPrivate = Context.fromLDContext({ + ...baseContext, + _meta: { + privateAttributes: ['name', 'nested/value'] + } + }); + + const contextWithoutPrivate = Context.fromLDContext(baseContext); + + const hashWithPrivate = await contextWithPrivate.hash(crypto); + const hashWithoutPrivate = await contextWithoutPrivate.hash(crypto); + + // The hashes should be different because private attributes are included in the hash + expect(hashWithPrivate).not.toEqual(hashWithoutPrivate); + }); +}); diff --git a/packages/shared/common/__tests__/internal/events/EventSummarizer.test.ts b/packages/shared/common/__tests__/internal/events/EventSummarizer.test.ts index 1fd713a9f7..f8eee7251f 100644 --- a/packages/shared/common/__tests__/internal/events/EventSummarizer.test.ts +++ b/packages/shared/common/__tests__/internal/events/EventSummarizer.test.ts @@ -12,16 +12,16 @@ describe('given an event summarizer', () => { }); it('does nothing for an identify event.', () => { - const beforeSummary = summarizer.getSummary(); + const beforeSummary = summarizer.getSummaries()[0]; summarizer.summarizeEvent(new InputIdentifyEvent(context)); - const afterSummary = summarizer.getSummary(); + const afterSummary = summarizer.getSummaries()[0]; expect(beforeSummary).toEqual(afterSummary); }); it('does nothing for a custom event.', () => { - const beforeSummary = summarizer.getSummary(); + const beforeSummary = summarizer.getSummaries()[0]; summarizer.summarizeEvent(new InputCustomEvent(context, 'custom', 'potato', 17)); - const afterSummary = summarizer.getSummary(); + const afterSummary = summarizer.getSummaries()[0]; expect(beforeSummary).toEqual(afterSummary); }); @@ -33,9 +33,9 @@ describe('given an event summarizer', () => { context, excludeFromSummaries: true, }; - const beforeSummary = summarizer.getSummary(); + const beforeSummary = summarizer.getSummaries()[0]; summarizer.summarizeEvent(event as any); - const afterSummary = summarizer.getSummary(); + const afterSummary = summarizer.getSummaries()[0]; expect(beforeSummary).toEqual(afterSummary); }); @@ -62,7 +62,7 @@ describe('given an event summarizer', () => { summarizer.summarizeEvent(event1 as any); summarizer.summarizeEvent(event2 as any); summarizer.summarizeEvent(event3 as any); - const data = summarizer.getSummary(); + const data = summarizer.getSummaries()[0]; expect(data.startDate).toEqual(1000); expect(data.endDate).toEqual(2000); @@ -135,7 +135,7 @@ describe('given an event summarizer', () => { summarizer.summarizeEvent(event4 as any); summarizer.summarizeEvent(event5 as any); summarizer.summarizeEvent(event6 as any); - const summary = summarizer.getSummary(); + const summary = summarizer.getSummaries()[0]; summary.features.key1.counters.sort((a, b) => a.value - b.value); const expectedFeatures = { @@ -223,7 +223,7 @@ describe('given an event summarizer', () => { summarizer.summarizeEvent(event1 as any); summarizer.summarizeEvent(event2 as any); summarizer.summarizeEvent(event3 as any); - const data = summarizer.getSummary(); + const data = summarizer.getSummaries()[0]; data.features.key1.counters.sort((a, b) => a.value - b.value); const expectedFeatures = { @@ -282,7 +282,7 @@ describe('given an event summarizer', () => { summarizer.summarizeEvent(event1 as any); summarizer.summarizeEvent(event2 as any); summarizer.summarizeEvent(event3 as any); - const data = summarizer.getSummary(); + const data = summarizer.getSummaries()[0]; const expectedFeatures = { key1: { diff --git a/packages/shared/common/__tests__/setupCrypto.ts b/packages/shared/common/__tests__/setupCrypto.ts index 72b2d13efc..e1348fb106 100644 --- a/packages/shared/common/__tests__/setupCrypto.ts +++ b/packages/shared/common/__tests__/setupCrypto.ts @@ -1,14 +1,25 @@ import { Hasher } from '../src/api'; +class MockHasher implements Hasher { + private state: string[] = []; + + update(value: string): Hasher { + this.state.push(value); + return this; + } + + digest(): string { + const result = this.state.join(''); + this.state = []; // Reset state after digest + return result; + } +} + export const setupCrypto = () => { let counter = 0; - const hasher = { - update: jest.fn((): Hasher => hasher), - digest: jest.fn(() => '1234567890123456'), - }; return { - createHash: jest.fn(() => hasher), + createHash: jest.fn(() => new MockHasher()), createHmac: jest.fn(), randomUUID: jest.fn(() => { counter += 1; diff --git a/packages/shared/common/src/AttributeReference.ts b/packages/shared/common/src/AttributeReference.ts index 4973818317..6ac4e4e826 100644 --- a/packages/shared/common/src/AttributeReference.ts +++ b/packages/shared/common/src/AttributeReference.ts @@ -144,4 +144,8 @@ export default class AttributeReference { this._components.every((value, index) => value === other.getComponent(index)) ); } + + public get components() { + return [...this._components]; + } } diff --git a/packages/shared/common/src/Context.ts b/packages/shared/common/src/Context.ts index 6b4a0bca7e..afbdf8bd4d 100644 --- a/packages/shared/common/src/Context.ts +++ b/packages/shared/common/src/Context.ts @@ -1,6 +1,7 @@ /* eslint-disable no-underscore-dangle */ // eslint-disable-next-line max-classes-per-file import type { + Crypto, LDContext, LDContextCommon, LDMultiKindContext, @@ -465,4 +466,69 @@ export default class Context { public get legacy(): boolean { return this._wasLegacy; } + + public async hash(crypto: Crypto): Promise { + if (!this.valid) { + return undefined; + } + + + const hasher = crypto.createHash('sha256'); + + const stack: { + target: any; + visited: any[]; + }[] = []; + + const kinds = this.kinds.sort(); + kinds.forEach(kind => hasher.update(kind)); + + for (const kind of kinds) { + hasher.update(kind); + const context = this._contextForKind(kind)!; + Object.getOwnPropertyNames(context).sort().forEach((key) => { + // Handled using private attributes. + if (key === "_meta") { + return; + } + stack.push({ + target: context[key], + visited: [context], + }); + }); + + const sortedAttributes = this.privateAttributes(kind).map(attr => attr.components.join('/')).sort(); + sortedAttributes.forEach(attr => hasher.update(attr)); + } + + while (stack.length > 0) { + const { target, visited } = stack.pop()!; + if (visited.includes(target)) { + return undefined; + } + visited.push(target); + if (typeof target === 'object' && target !== null && target !== undefined) { + Object.getOwnPropertyNames(target).sort().forEach((key) => { + // Handled using private attributes. + if (key === "_meta") { + return; + } + stack.push({ + target: target[key], + visited: [...visited, target], + }); + }); + } else { + hasher.update(String(target)); + } + } + + if (hasher.digest) { + return hasher.digest('hex'); + } + + // The hasher must have either digest or asyncDigest. + const digest = await hasher.asyncDigest!('hex'); + return digest; + } } diff --git a/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts b/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts new file mode 100644 index 0000000000..bec115bdaf --- /dev/null +++ b/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts @@ -0,0 +1,54 @@ +import InputEvent from '../../internal/events/InputEvent'; + +/** + * @internal + */ +export interface FlagCounter { + value: any; + count: number; + variation?: number; + version?: number; + unknown?: boolean; +} + +/** + * @internal + */ +export interface FlagSummary { + default: any; + counters: FlagCounter[]; + contextKinds?: string[]; +} + +/** + * @internal + */ +export interface SummarizedFlagsEvent { + startDate: number; + endDate: number; + features: Record; + kind: 'summary'; + context?: any; +} + +/** + * Interface for summarizing feature flag evaluation events. + */ +export default interface LDEventSummarizer { + /** + * Processes an event for summarization if it is a feature flag event and not excluded from summaries. + * @param event The event to potentially summarize + */ + summarizeEvent(event: InputEvent): void; + + /** + * Gets the current summary of processed events. + * @returns A summary of all processed feature flag events + */ + getSummaries(): SummarizedFlagsEvent[]; + + /** + * Clears all summarized event data. + */ + clearSummary(): void; +} \ No newline at end of file diff --git a/packages/shared/common/src/internal/events/EventProcessor.ts b/packages/shared/common/src/internal/events/EventProcessor.ts index e6963e5efe..21ec4fadf7 100644 --- a/packages/shared/common/src/internal/events/EventProcessor.ts +++ b/packages/shared/common/src/internal/events/EventProcessor.ts @@ -2,18 +2,20 @@ import { LDEvaluationReason, LDLogger } from '../../api'; import { LDDeliveryStatus, LDEventType } from '../../api/subsystem'; import LDContextDeduplicator from '../../api/subsystem/LDContextDeduplicator'; import LDEventProcessor from '../../api/subsystem/LDEventProcessor'; +import { SummarizedFlagsEvent } from '../../api/subsystem/LDEventSummarizer'; import AttributeReference from '../../AttributeReference'; import ContextFilter from '../../ContextFilter'; import { ClientContext } from '../../options'; import { LDHeaders } from '../../utils'; import { DiagnosticsManager } from '../diagnostics'; import EventSender from './EventSender'; -import EventSummarizer, { SummarizedFlagsEvent } from './EventSummarizer'; +import EventSummarizer from './EventSummarizer'; import { isFeature, isIdentify, isMigration } from './guards'; import InputEvent from './InputEvent'; import InputIdentifyEvent from './InputIdentifyEvent'; import InputMigrationEvent from './InputMigrationEvent'; import LDInvalidSDKKeyError from './LDInvalidSDKKeyError'; +import MultiEventSummarizer from './MultiEventSummarizer'; import shouldSample from './sampling'; type FilteredContext = any; @@ -86,7 +88,7 @@ type DiagnosticEvent = any; interface MigrationOutputEvent extends Omit { // Make the sampling ratio optional so we can omit it when it is one. samplingRatio?: number; - // Context is optional because contextKeys is supported for backwards compatbility and may be provided instead of context. + // Context is optional because contextKeys is supported for backwards compatibility and may be provided instead of context. context?: FilteredContext; } @@ -108,7 +110,7 @@ export interface EventProcessorOptions { export default class EventProcessor implements LDEventProcessor { private _eventSender: EventSender; - private _summarizer = new EventSummarizer(); + private _summarizer; private _queue: OutputEvent[] = []; private _lastKnownPastTime = 0; private _droppedEvents = 0; @@ -133,16 +135,24 @@ export default class EventProcessor implements LDEventProcessor { private readonly _contextDeduplicator?: LDContextDeduplicator, private readonly _diagnosticsManager?: DiagnosticsManager, start: boolean = true, + summariesPerContext: boolean = false, ) { this._capacity = _config.eventsCapacity; this._logger = clientContext.basicConfiguration.logger; this._eventSender = new EventSender(clientContext, baseHeaders); - + this._contextFilter = new ContextFilter( _config.allAttributesPrivate, _config.privateAttributes.map((ref) => new AttributeReference(ref)), ); + if(summariesPerContext) { + this._summarizer = new MultiEventSummarizer(clientContext.platform.crypto, this._contextFilter); + } else { + this._summarizer = new EventSummarizer(); + } + + if (start) { this.start(); } @@ -210,12 +220,15 @@ export default class EventProcessor implements LDEventProcessor { const eventsToFlush = this._queue; this._queue = []; - const summary = this._summarizer.getSummary(); + const summaries = this._summarizer.getSummaries(); this._summarizer.clearSummary(); - if (Object.keys(summary.features).length) { - eventsToFlush.push(summary); - } + summaries.forEach(summary => { + if (Object.keys(summary.features).length) { + eventsToFlush.push(summary); + } + }); + if (!eventsToFlush.length) { return; diff --git a/packages/shared/common/src/internal/events/EventSummarizer.ts b/packages/shared/common/src/internal/events/EventSummarizer.ts index 2e34f2e674..84d67f525b 100644 --- a/packages/shared/common/src/internal/events/EventSummarizer.ts +++ b/packages/shared/common/src/internal/events/EventSummarizer.ts @@ -1,48 +1,20 @@ +import LDEventSummarizer, { SummarizedFlagsEvent, FlagSummary, FlagCounter } from '../../api/subsystem/LDEventSummarizer'; +import Context from '../../Context'; +import ContextFilter from '../../ContextFilter'; import { isFeature } from './guards'; import InputEvalEvent from './InputEvalEvent'; import InputEvent from './InputEvent'; import SummaryCounter from './SummaryCounter'; function counterKey(event: InputEvalEvent) { - return `${event.key}:${ - event.variation !== null && event.variation !== undefined ? event.variation : '' - }:${event.version !== null && event.version !== undefined ? event.version : ''}`; + return `${event.key}:${event.variation !== null && event.variation !== undefined ? event.variation : '' + }:${event.version !== null && event.version !== undefined ? event.version : ''}`; } /** * @internal */ -export interface FlagCounter { - value: any; - count: number; - variation?: number; - version?: number; - unknown?: boolean; -} - -/** - * @internal - */ -export interface FlagSummary { - default: any; - counters: FlagCounter[]; - contextKinds: string[]; -} - -/** - * @internal - */ -export interface SummarizedFlagsEvent { - startDate: number; - endDate: number; - features: Record; - kind: 'summary'; -} - -/** - * @internal - */ -export default class EventSummarizer { +export default class EventSummarizer implements LDEventSummarizer { private _startDate = 0; private _endDate = 0; @@ -51,8 +23,17 @@ export default class EventSummarizer { private _contextKinds: Record> = {}; + private _context?: Context; + + constructor(private readonly _singleContext: boolean = false, private readonly _contextFilter?: ContextFilter) { + } + summarizeEvent(event: InputEvent) { if (isFeature(event) && !event.excludeFromSummaries) { + if(!this._context) { + console.log('setting context', event.context); + this._context = event.context; + } const countKey = counterKey(event); const counter = this._counters[countKey]; let kinds = this._contextKinds[event.key]; @@ -84,7 +65,7 @@ export default class EventSummarizer { } } - getSummary(): SummarizedFlagsEvent { + getSummaries(): SummarizedFlagsEvent[] { const features = Object.values(this._counters).reduce( (acc: Record, counter) => { let flagSummary = acc[counter.key]; @@ -92,7 +73,7 @@ export default class EventSummarizer { flagSummary = { default: counter.default, counters: [], - contextKinds: [...this._contextKinds[counter.key]], + contextKinds: this._singleContext ? undefined : [...this._contextKinds[counter.key]], }; acc[counter.key] = flagSummary; } @@ -116,12 +97,16 @@ export default class EventSummarizer { {}, ); - return { + console.log("HAS CONTEXT", !!this._context); + console.log("SINGLE CONTEXT", this._singleContext); + + return [{ startDate: this._startDate, endDate: this._endDate, features, kind: 'summary', - }; + context: this._context !== undefined && this._singleContext ? this._contextFilter?.filter(this._context) : undefined, + }]; } clearSummary() { diff --git a/packages/shared/common/src/internal/events/LDEventSummarizer.ts b/packages/shared/common/src/internal/events/LDEventSummarizer.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts new file mode 100644 index 0000000000..3a2a1eec6b --- /dev/null +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts @@ -0,0 +1,131 @@ +import MultiEventSummarizer from "./MultiEventSummarizer"; +import InputEvalEvent from "./InputEvalEvent"; +import Context from "../../Context"; +import InputIdentifyEvent from "./InputIdentifyEvent"; +import { setupCrypto } from "../../../__tests__/setupCrypto"; +import ContextFilter from "../../ContextFilter"; + +describe("with mocked crypto and hasher", () => { + let summarizer: MultiEventSummarizer; + + beforeEach(() => { + const crypto = setupCrypto(); + const contextFilter = new ContextFilter(false, []); + summarizer = new MultiEventSummarizer(crypto, contextFilter); + }); + + test("creates new summarizer for new context hash", async () => { + const context = Context.fromLDContext({ kind: "user", key: "user1" }); + const event = new InputEvalEvent( + true, + context, + "flag-key", + "value", + "default", + 1, + 0, + true + ); + + summarizer.summarizeEvent(event); + await new Promise(process.nextTick); // Wait for async operations + + const summaries = summarizer.getSummaries(); + expect(summaries).toHaveLength(1); + }); + + test("uses existing summarizer for same context hash", async () => { + const context = Context.fromLDContext({ kind: "user", key: "user1" }); + const event1 = new InputEvalEvent( + true, + context, + "flag-key", + "value1", + "default", + 1, + 0, + true + ); + const event2 = new InputEvalEvent( + true, + context, + "flag-key", + "value2", + "default", + 1, + 0, + true + ); + + summarizer.summarizeEvent(event1); + summarizer.summarizeEvent(event2); + await new Promise(process.nextTick); + + const summaries = summarizer.getSummaries(); + expect(summaries).toHaveLength(1); + }); + + test("ignores non-feature events", async () => { + const context = Context.fromLDContext({ kind: "user", key: "user1" }); + const event = new InputIdentifyEvent(context); + + summarizer.summarizeEvent(event); + await new Promise(process.nextTick); + + const summaries = summarizer.getSummaries(); + expect(summaries).toHaveLength(0); + }); + + test("handles multiple different contexts", async () => { + const context1 = Context.fromLDContext({ kind: "user", key: "user1" }); + const context2 = Context.fromLDContext({ kind: "user", key: "user2" }); + const event1 = new InputEvalEvent( + true, + context1, + "flag-key", + "value1", + "default", + 1, + 0, + true + ); + const event2 = new InputEvalEvent( + true, + context2, + "flag-key", + "value2", + "default", + 1, + 0, + true + ); + + summarizer.summarizeEvent(event1); + summarizer.summarizeEvent(event2); + await new Promise(process.nextTick); + + const summaries = summarizer.getSummaries(); + expect(summaries).toHaveLength(2); + }); + + test("clears all summarizers", async () => { + const context = Context.fromLDContext({ kind: "user", key: "user1" }); + const event = new InputEvalEvent( + true, + context, + "flag-key", + "value", + "default", + 1, + 0, + true + ); + + summarizer.summarizeEvent(event); + await new Promise(process.nextTick); + summarizer.clearSummary(); + + const summaries = summarizer.getSummaries(); + expect(summaries).toHaveLength(0); + }); +}); \ No newline at end of file diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts new file mode 100644 index 0000000000..a211822a03 --- /dev/null +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts @@ -0,0 +1,42 @@ +import { Crypto } from "../../api"; +import LDEventSummarizer, { SummarizedFlagsEvent } from "../../api/subsystem/LDEventSummarizer"; +import ContextFilter from "../../ContextFilter"; +import EventSummarizer from "./EventSummarizer"; +import { isFeature } from "./guards"; +import InputEvent from "./InputEvent"; + +export default class MultiEventSummarizer implements LDEventSummarizer { + constructor(private readonly _crypto: Crypto, private readonly _contextFilter: ContextFilter) { + } + + private _summarizers: Record = {}; + + summarizeEvent(event: InputEvent) { + // This will execute asynchronously, which means that a flush could happen before the event + // is summarized. When that happens, then the event will just be in the next batch of summaries. + (async () => { + if (isFeature(event)) { + const hash = await event.context.hash(this._crypto); + if (!hash) { + return; + } + // It is important that async operations do not happen between checking that the summarizer + // exists and having it summarize the event. + // If it did, then that event could be lost. + let summarizer = this._summarizers[hash]; + if (!summarizer) { + this._summarizers[hash] = new EventSummarizer(true, this._contextFilter); + summarizer = this._summarizers[hash]; + } + + summarizer.summarizeEvent(event); + } + })(); + } + getSummaries(): SummarizedFlagsEvent[] { + return Object.values(this._summarizers).flatMap(summarizer => summarizer.getSummaries()); + } + clearSummary(): void { + this._summarizers = {}; + } +} \ No newline at end of file diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index df59fb7c44..d62c733176 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -80,7 +80,9 @@ export default class LDClientImpl implements LDClient { throw new Error('Platform must implement Encoding because btoa is required.'); } + console.log('sdk-client options', options); this._config = new ConfigurationImpl(options, internalOptions); + console.log('this._config', this._config); this.logger = this._config.logger; this._baseHeaders = defaultHeaders( diff --git a/packages/shared/sdk-client/src/events/createEventProcessor.ts b/packages/shared/sdk-client/src/events/createEventProcessor.ts index ab4887925c..bc8e2e05a3 100644 --- a/packages/shared/sdk-client/src/events/createEventProcessor.ts +++ b/packages/shared/sdk-client/src/events/createEventProcessor.ts @@ -17,6 +17,7 @@ const createEventProcessor = ( undefined, diagnosticsManager, false, + true, ); } From c7b78afb760438aa65d42305f4c09ae120970317 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 14 Apr 2025 14:30:24 -0700 Subject: [PATCH 02/26] Remove some console.logs. --- packages/sdk/browser/src/BrowserClient.ts | 2 -- .../shared/common/src/internal/events/EventSummarizer.ts | 6 +----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index 6593cd18eb..f668aafb47 100644 --- a/packages/sdk/browser/src/BrowserClient.ts +++ b/packages/sdk/browser/src/BrowserClient.ts @@ -117,9 +117,7 @@ export class BrowserClient extends LDClientImpl implements LDClient { const platform = overridePlatform ?? new BrowserPlatform(logger); const validatedBrowserOptions = validateOptions(options, logger); - console.log('validatedBrowserOptions', validatedBrowserOptions); const { eventUrlTransformer } = validatedBrowserOptions; - console.log('filtered options', filterToBaseOptions({ ...options, logger })); super( clientSideId, autoEnvAttributes, diff --git a/packages/shared/common/src/internal/events/EventSummarizer.ts b/packages/shared/common/src/internal/events/EventSummarizer.ts index 84d67f525b..ad7d2ffc75 100644 --- a/packages/shared/common/src/internal/events/EventSummarizer.ts +++ b/packages/shared/common/src/internal/events/EventSummarizer.ts @@ -31,7 +31,6 @@ export default class EventSummarizer implements LDEventSummarizer { summarizeEvent(event: InputEvent) { if (isFeature(event) && !event.excludeFromSummaries) { if(!this._context) { - console.log('setting context', event.context); this._context = event.context; } const countKey = counterKey(event); @@ -73,7 +72,7 @@ export default class EventSummarizer implements LDEventSummarizer { flagSummary = { default: counter.default, counters: [], - contextKinds: this._singleContext ? undefined : [...this._contextKinds[counter.key]], + contextKinds: [...this._contextKinds[counter.key]], }; acc[counter.key] = flagSummary; } @@ -97,9 +96,6 @@ export default class EventSummarizer implements LDEventSummarizer { {}, ); - console.log("HAS CONTEXT", !!this._context); - console.log("SINGLE CONTEXT", this._singleContext); - return [{ startDate: this._startDate, endDate: this._endDate, From 2bc5b493ec0a49ec3b5bb173617b6978d7f42d66 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 14 Apr 2025 14:34:40 -0700 Subject: [PATCH 03/26] Some cleanup. --- packages/sdk/browser/src/platform/BrowserRequests.ts | 1 - packages/shared/common/src/api/subsystem/LDEventSummarizer.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/sdk/browser/src/platform/BrowserRequests.ts b/packages/sdk/browser/src/platform/BrowserRequests.ts index cbb455c252..5e73467843 100644 --- a/packages/sdk/browser/src/platform/BrowserRequests.ts +++ b/packages/sdk/browser/src/platform/BrowserRequests.ts @@ -11,7 +11,6 @@ import DefaultBrowserEventSource from './DefaultBrowserEventSource'; export default class BrowserRequests implements Requests { fetch(url: string, options?: Options): Promise { - delete options?.compressBodyIfPossible; // @ts-ignore return fetch(url, options); } diff --git a/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts b/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts index bec115bdaf..6b6cde6082 100644 --- a/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts +++ b/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts @@ -51,4 +51,4 @@ export default interface LDEventSummarizer { * Clears all summarized event data. */ clearSummary(): void; -} \ No newline at end of file +} From 0dfd0b0696fb8c177343fdda2ecc7a2d394b44e2 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 14 Apr 2025 14:37:04 -0700 Subject: [PATCH 04/26] Extra blank line for multi summarizer tests. --- .../common/src/internal/events/MultiEventSummarizer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts index 3a2a1eec6b..14b626a11e 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts @@ -128,4 +128,4 @@ describe("with mocked crypto and hasher", () => { const summaries = summarizer.getSummaries(); expect(summaries).toHaveLength(0); }); -}); \ No newline at end of file +}); From 30d03b8c14f511bd67f4be233861f6fa8f5c8ef9 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 14 Apr 2025 14:37:27 -0700 Subject: [PATCH 05/26] More blank lines. --- .../shared/common/src/internal/events/MultiEventSummarizer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts index a211822a03..a028484b61 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts @@ -39,4 +39,4 @@ export default class MultiEventSummarizer implements LDEventSummarizer { clearSummary(): void { this._summarizers = {}; } -} \ No newline at end of file +} From a990ee0a32ab8f87d962d3e1b996c8505257b02e Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 14 Apr 2025 14:38:54 -0700 Subject: [PATCH 06/26] Lint --- .../shared/common/__tests__/Context.test.ts | 23 ++-- packages/shared/common/src/Context.ts | 49 ++++---- .../src/api/subsystem/LDEventSummarizer.ts | 56 ++++----- .../src/internal/events/EventProcessor.ts | 13 ++- .../src/internal/events/EventSummarizer.ts | 38 ++++--- .../events/MultiEventSummarizer.test.ts | 106 +++++------------- .../internal/events/MultiEventSummarizer.ts | 74 ++++++------ .../shared/sdk-client/src/LDClientImpl.ts | 2 - 8 files changed, 164 insertions(+), 197 deletions(-) diff --git a/packages/shared/common/__tests__/Context.test.ts b/packages/shared/common/__tests__/Context.test.ts index 41b863f20e..d9702e9a2a 100644 --- a/packages/shared/common/__tests__/Context.test.ts +++ b/packages/shared/common/__tests__/Context.test.ts @@ -455,10 +455,10 @@ describe('given mock crypto', () => { nested: { level1: { level2: { - value: 'deep' - } - } - } + value: 'deep', + }, + }, + }, }); const hash = await context.hash(crypto); @@ -470,7 +470,10 @@ describe('given mock crypto', () => { kind: 'user', key: 'testKey', array: [1, 2, 3], - nestedArray: [[1, 2], [3, 4]] + nestedArray: [ + [1, 2], + [3, 4], + ], }); const hash = await context.hash(crypto); @@ -485,7 +488,7 @@ describe('given mock crypto', () => { number: 42, boolean: true, nullValue: null, - undefinedValue: undefined + undefinedValue: undefined, }); const hash = await context.hash(crypto); @@ -498,15 +501,15 @@ describe('given mock crypto', () => { key: 'testKey', name: 'testName', nested: { - value: 'testValue' - } + value: 'testValue', + }, }; const contextWithPrivate = Context.fromLDContext({ ...baseContext, _meta: { - privateAttributes: ['name', 'nested/value'] - } + privateAttributes: ['name', 'nested/value'], + }, }); const contextWithoutPrivate = Context.fromLDContext(baseContext); diff --git a/packages/shared/common/src/Context.ts b/packages/shared/common/src/Context.ts index afbdf8bd4d..a854cc6f73 100644 --- a/packages/shared/common/src/Context.ts +++ b/packages/shared/common/src/Context.ts @@ -472,7 +472,6 @@ export default class Context { return undefined; } - const hasher = crypto.createHash('sha256'); const stack: { @@ -481,24 +480,28 @@ export default class Context { }[] = []; const kinds = this.kinds.sort(); - kinds.forEach(kind => hasher.update(kind)); + kinds.forEach((kind) => hasher.update(kind)); for (const kind of kinds) { hasher.update(kind); const context = this._contextForKind(kind)!; - Object.getOwnPropertyNames(context).sort().forEach((key) => { - // Handled using private attributes. - if (key === "_meta") { - return; - } - stack.push({ - target: context[key], - visited: [context], + Object.getOwnPropertyNames(context) + .sort() + .forEach((key) => { + // Handled using private attributes. + if (key === '_meta') { + return; + } + stack.push({ + target: context[key], + visited: [context], + }); }); - }); - const sortedAttributes = this.privateAttributes(kind).map(attr => attr.components.join('/')).sort(); - sortedAttributes.forEach(attr => hasher.update(attr)); + const sortedAttributes = this.privateAttributes(kind) + .map((attr) => attr.components.join('/')) + .sort(); + sortedAttributes.forEach((attr) => hasher.update(attr)); } while (stack.length > 0) { @@ -508,16 +511,18 @@ export default class Context { } visited.push(target); if (typeof target === 'object' && target !== null && target !== undefined) { - Object.getOwnPropertyNames(target).sort().forEach((key) => { - // Handled using private attributes. - if (key === "_meta") { - return; - } - stack.push({ - target: target[key], - visited: [...visited, target], + Object.getOwnPropertyNames(target) + .sort() + .forEach((key) => { + // Handled using private attributes. + if (key === '_meta') { + return; + } + stack.push({ + target: target[key], + visited: [...visited, target], + }); }); - }); } else { hasher.update(String(target)); } diff --git a/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts b/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts index 6b6cde6082..34dde09759 100644 --- a/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts +++ b/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts @@ -4,51 +4,51 @@ import InputEvent from '../../internal/events/InputEvent'; * @internal */ export interface FlagCounter { - value: any; - count: number; - variation?: number; - version?: number; - unknown?: boolean; + value: any; + count: number; + variation?: number; + version?: number; + unknown?: boolean; } /** * @internal */ export interface FlagSummary { - default: any; - counters: FlagCounter[]; - contextKinds?: string[]; + default: any; + counters: FlagCounter[]; + contextKinds?: string[]; } /** * @internal */ export interface SummarizedFlagsEvent { - startDate: number; - endDate: number; - features: Record; - kind: 'summary'; - context?: any; + startDate: number; + endDate: number; + features: Record; + kind: 'summary'; + context?: any; } /** * Interface for summarizing feature flag evaluation events. */ export default interface LDEventSummarizer { - /** - * Processes an event for summarization if it is a feature flag event and not excluded from summaries. - * @param event The event to potentially summarize - */ - summarizeEvent(event: InputEvent): void; + /** + * Processes an event for summarization if it is a feature flag event and not excluded from summaries. + * @param event The event to potentially summarize + */ + summarizeEvent(event: InputEvent): void; - /** - * Gets the current summary of processed events. - * @returns A summary of all processed feature flag events - */ - getSummaries(): SummarizedFlagsEvent[]; + /** + * Gets the current summary of processed events. + * @returns A summary of all processed feature flag events + */ + getSummaries(): SummarizedFlagsEvent[]; - /** - * Clears all summarized event data. - */ - clearSummary(): void; -} + /** + * Clears all summarized event data. + */ + clearSummary(): void; +} diff --git a/packages/shared/common/src/internal/events/EventProcessor.ts b/packages/shared/common/src/internal/events/EventProcessor.ts index 21ec4fadf7..ab07c3ca5d 100644 --- a/packages/shared/common/src/internal/events/EventProcessor.ts +++ b/packages/shared/common/src/internal/events/EventProcessor.ts @@ -140,19 +140,21 @@ export default class EventProcessor implements LDEventProcessor { this._capacity = _config.eventsCapacity; this._logger = clientContext.basicConfiguration.logger; this._eventSender = new EventSender(clientContext, baseHeaders); - + this._contextFilter = new ContextFilter( _config.allAttributesPrivate, _config.privateAttributes.map((ref) => new AttributeReference(ref)), ); - if(summariesPerContext) { - this._summarizer = new MultiEventSummarizer(clientContext.platform.crypto, this._contextFilter); + if (summariesPerContext) { + this._summarizer = new MultiEventSummarizer( + clientContext.platform.crypto, + this._contextFilter, + ); } else { this._summarizer = new EventSummarizer(); } - if (start) { this.start(); } @@ -223,13 +225,12 @@ export default class EventProcessor implements LDEventProcessor { const summaries = this._summarizer.getSummaries(); this._summarizer.clearSummary(); - summaries.forEach(summary => { + summaries.forEach((summary) => { if (Object.keys(summary.features).length) { eventsToFlush.push(summary); } }); - if (!eventsToFlush.length) { return; } diff --git a/packages/shared/common/src/internal/events/EventSummarizer.ts b/packages/shared/common/src/internal/events/EventSummarizer.ts index ad7d2ffc75..3cf299c3be 100644 --- a/packages/shared/common/src/internal/events/EventSummarizer.ts +++ b/packages/shared/common/src/internal/events/EventSummarizer.ts @@ -1,4 +1,8 @@ -import LDEventSummarizer, { SummarizedFlagsEvent, FlagSummary, FlagCounter } from '../../api/subsystem/LDEventSummarizer'; +import LDEventSummarizer, { + FlagCounter, + FlagSummary, + SummarizedFlagsEvent, +} from '../../api/subsystem/LDEventSummarizer'; import Context from '../../Context'; import ContextFilter from '../../ContextFilter'; import { isFeature } from './guards'; @@ -7,8 +11,9 @@ import InputEvent from './InputEvent'; import SummaryCounter from './SummaryCounter'; function counterKey(event: InputEvalEvent) { - return `${event.key}:${event.variation !== null && event.variation !== undefined ? event.variation : '' - }:${event.version !== null && event.version !== undefined ? event.version : ''}`; + return `${event.key}:${ + event.variation !== null && event.variation !== undefined ? event.variation : '' + }:${event.version !== null && event.version !== undefined ? event.version : ''}`; } /** @@ -25,12 +30,14 @@ export default class EventSummarizer implements LDEventSummarizer { private _context?: Context; - constructor(private readonly _singleContext: boolean = false, private readonly _contextFilter?: ContextFilter) { - } + constructor( + private readonly _singleContext: boolean = false, + private readonly _contextFilter?: ContextFilter, + ) {} summarizeEvent(event: InputEvent) { if (isFeature(event) && !event.excludeFromSummaries) { - if(!this._context) { + if (!this._context) { this._context = event.context; } const countKey = counterKey(event); @@ -96,13 +103,18 @@ export default class EventSummarizer implements LDEventSummarizer { {}, ); - return [{ - startDate: this._startDate, - endDate: this._endDate, - features, - kind: 'summary', - context: this._context !== undefined && this._singleContext ? this._contextFilter?.filter(this._context) : undefined, - }]; + return [ + { + startDate: this._startDate, + endDate: this._endDate, + features, + kind: 'summary', + context: + this._context !== undefined && this._singleContext + ? this._contextFilter?.filter(this._context) + : undefined, + }, + ]; } clearSummary() { diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts index 14b626a11e..f92a3521d3 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts @@ -1,11 +1,11 @@ -import MultiEventSummarizer from "./MultiEventSummarizer"; -import InputEvalEvent from "./InputEvalEvent"; -import Context from "../../Context"; -import InputIdentifyEvent from "./InputIdentifyEvent"; -import { setupCrypto } from "../../../__tests__/setupCrypto"; -import ContextFilter from "../../ContextFilter"; - -describe("with mocked crypto and hasher", () => { +import { setupCrypto } from '../../../__tests__/setupCrypto'; +import Context from '../../Context'; +import ContextFilter from '../../ContextFilter'; +import InputEvalEvent from './InputEvalEvent'; +import InputIdentifyEvent from './InputIdentifyEvent'; +import MultiEventSummarizer from './MultiEventSummarizer'; + +describe('with mocked crypto and hasher', () => { let summarizer: MultiEventSummarizer; beforeEach(() => { @@ -14,18 +14,9 @@ describe("with mocked crypto and hasher", () => { summarizer = new MultiEventSummarizer(crypto, contextFilter); }); - test("creates new summarizer for new context hash", async () => { - const context = Context.fromLDContext({ kind: "user", key: "user1" }); - const event = new InputEvalEvent( - true, - context, - "flag-key", - "value", - "default", - 1, - 0, - true - ); + test('creates new summarizer for new context hash', async () => { + const context = Context.fromLDContext({ kind: 'user', key: 'user1' }); + const event = new InputEvalEvent(true, context, 'flag-key', 'value', 'default', 1, 0, true); summarizer.summarizeEvent(event); await new Promise(process.nextTick); // Wait for async operations @@ -34,28 +25,10 @@ describe("with mocked crypto and hasher", () => { expect(summaries).toHaveLength(1); }); - test("uses existing summarizer for same context hash", async () => { - const context = Context.fromLDContext({ kind: "user", key: "user1" }); - const event1 = new InputEvalEvent( - true, - context, - "flag-key", - "value1", - "default", - 1, - 0, - true - ); - const event2 = new InputEvalEvent( - true, - context, - "flag-key", - "value2", - "default", - 1, - 0, - true - ); + test('uses existing summarizer for same context hash', async () => { + const context = Context.fromLDContext({ kind: 'user', key: 'user1' }); + const event1 = new InputEvalEvent(true, context, 'flag-key', 'value1', 'default', 1, 0, true); + const event2 = new InputEvalEvent(true, context, 'flag-key', 'value2', 'default', 1, 0, true); summarizer.summarizeEvent(event1); summarizer.summarizeEvent(event2); @@ -65,8 +38,8 @@ describe("with mocked crypto and hasher", () => { expect(summaries).toHaveLength(1); }); - test("ignores non-feature events", async () => { - const context = Context.fromLDContext({ kind: "user", key: "user1" }); + test('ignores non-feature events', async () => { + const context = Context.fromLDContext({ kind: 'user', key: 'user1' }); const event = new InputIdentifyEvent(context); summarizer.summarizeEvent(event); @@ -76,29 +49,11 @@ describe("with mocked crypto and hasher", () => { expect(summaries).toHaveLength(0); }); - test("handles multiple different contexts", async () => { - const context1 = Context.fromLDContext({ kind: "user", key: "user1" }); - const context2 = Context.fromLDContext({ kind: "user", key: "user2" }); - const event1 = new InputEvalEvent( - true, - context1, - "flag-key", - "value1", - "default", - 1, - 0, - true - ); - const event2 = new InputEvalEvent( - true, - context2, - "flag-key", - "value2", - "default", - 1, - 0, - true - ); + test('handles multiple different contexts', async () => { + const context1 = Context.fromLDContext({ kind: 'user', key: 'user1' }); + const context2 = Context.fromLDContext({ kind: 'user', key: 'user2' }); + const event1 = new InputEvalEvent(true, context1, 'flag-key', 'value1', 'default', 1, 0, true); + const event2 = new InputEvalEvent(true, context2, 'flag-key', 'value2', 'default', 1, 0, true); summarizer.summarizeEvent(event1); summarizer.summarizeEvent(event2); @@ -108,18 +63,9 @@ describe("with mocked crypto and hasher", () => { expect(summaries).toHaveLength(2); }); - test("clears all summarizers", async () => { - const context = Context.fromLDContext({ kind: "user", key: "user1" }); - const event = new InputEvalEvent( - true, - context, - "flag-key", - "value", - "default", - 1, - 0, - true - ); + test('clears all summarizers', async () => { + const context = Context.fromLDContext({ kind: 'user', key: 'user1' }); + const event = new InputEvalEvent(true, context, 'flag-key', 'value', 'default', 1, 0, true); summarizer.summarizeEvent(event); await new Promise(process.nextTick); @@ -128,4 +74,4 @@ describe("with mocked crypto and hasher", () => { const summaries = summarizer.getSummaries(); expect(summaries).toHaveLength(0); }); -}); +}); diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts index a028484b61..0f87d0baac 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts @@ -1,42 +1,44 @@ -import { Crypto } from "../../api"; -import LDEventSummarizer, { SummarizedFlagsEvent } from "../../api/subsystem/LDEventSummarizer"; -import ContextFilter from "../../ContextFilter"; -import EventSummarizer from "./EventSummarizer"; -import { isFeature } from "./guards"; -import InputEvent from "./InputEvent"; +import { Crypto } from '../../api'; +import LDEventSummarizer, { SummarizedFlagsEvent } from '../../api/subsystem/LDEventSummarizer'; +import ContextFilter from '../../ContextFilter'; +import EventSummarizer from './EventSummarizer'; +import { isFeature } from './guards'; +import InputEvent from './InputEvent'; export default class MultiEventSummarizer implements LDEventSummarizer { - constructor(private readonly _crypto: Crypto, private readonly _contextFilter: ContextFilter) { - } + constructor( + private readonly _crypto: Crypto, + private readonly _contextFilter: ContextFilter, + ) {} - private _summarizers: Record = {}; + private _summarizers: Record = {}; - summarizeEvent(event: InputEvent) { - // This will execute asynchronously, which means that a flush could happen before the event - // is summarized. When that happens, then the event will just be in the next batch of summaries. - (async () => { - if (isFeature(event)) { - const hash = await event.context.hash(this._crypto); - if (!hash) { - return; - } - // It is important that async operations do not happen between checking that the summarizer - // exists and having it summarize the event. - // If it did, then that event could be lost. - let summarizer = this._summarizers[hash]; - if (!summarizer) { - this._summarizers[hash] = new EventSummarizer(true, this._contextFilter); - summarizer = this._summarizers[hash]; - } + summarizeEvent(event: InputEvent) { + // This will execute asynchronously, which means that a flush could happen before the event + // is summarized. When that happens, then the event will just be in the next batch of summaries. + (async () => { + if (isFeature(event)) { + const hash = await event.context.hash(this._crypto); + if (!hash) { + return; + } + // It is important that async operations do not happen between checking that the summarizer + // exists and having it summarize the event. + // If it did, then that event could be lost. + let summarizer = this._summarizers[hash]; + if (!summarizer) { + this._summarizers[hash] = new EventSummarizer(true, this._contextFilter); + summarizer = this._summarizers[hash]; + } - summarizer.summarizeEvent(event); - } - })(); - } - getSummaries(): SummarizedFlagsEvent[] { - return Object.values(this._summarizers).flatMap(summarizer => summarizer.getSummaries()); - } - clearSummary(): void { - this._summarizers = {}; - } + summarizer.summarizeEvent(event); + } + })(); + } + getSummaries(): SummarizedFlagsEvent[] { + return Object.values(this._summarizers).flatMap((summarizer) => summarizer.getSummaries()); + } + clearSummary(): void { + this._summarizers = {}; + } } diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index d62c733176..df59fb7c44 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -80,9 +80,7 @@ export default class LDClientImpl implements LDClient { throw new Error('Platform must implement Encoding because btoa is required.'); } - console.log('sdk-client options', options); this._config = new ConfigurationImpl(options, internalOptions); - console.log('this._config', this._config); this.logger = this._config.logger; this._baseHeaders = defaultHeaders( From 99f290d2c3a66e9f6fb2143358d5edde591163d7 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 14 Apr 2025 15:32:55 -0700 Subject: [PATCH 07/26] Tweaks --- packages/sdk/browser/src/BrowserClient.ts | 7 +++-- packages/sdk/browser/src/options.ts | 3 +- .../internal/events/EventSummarizer.test.ts | 20 ++++++------- .../src/api/subsystem/LDEventSummarizer.ts | 8 ++++- .../src/internal/events/EventProcessor.ts | 27 ++++++++++++----- .../src/internal/events/EventSummarizer.ts | 29 +++++++++---------- .../internal/events/MultiEventSummarizer.ts | 19 +++++++----- 7 files changed, 68 insertions(+), 45 deletions(-) diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index f668aafb47..5b34f19ee1 100644 --- a/packages/sdk/browser/src/BrowserClient.ts +++ b/packages/sdk/browser/src/BrowserClient.ts @@ -21,7 +21,7 @@ import { BrowserIdentifyOptions as LDIdentifyOptions } from './BrowserIdentifyOp import { registerStateDetection } from './BrowserStateDetector'; import GoalManager from './goals/GoalManager'; import { Goal, isClick } from './goals/Goals'; -import validateOptions, { BrowserOptions, filterToBaseOptions } from './options'; +import validateOptions, { applyBrowserDefaults, BrowserOptions, filterToBaseOptions } from './options'; import BrowserPlatform from './platform/BrowserPlatform'; /** @@ -116,13 +116,14 @@ export class BrowserClient extends LDClientImpl implements LDClient { const baseUrl = options.baseUri ?? 'https://clientsdk.launchdarkly.com'; const platform = overridePlatform ?? new BrowserPlatform(logger); - const validatedBrowserOptions = validateOptions(options, logger); + const withDefaults = applyBrowserDefaults(options); + const validatedBrowserOptions = validateOptions(withDefaults, logger); const { eventUrlTransformer } = validatedBrowserOptions; super( clientSideId, autoEnvAttributes, platform, - filterToBaseOptions({ ...validatedBrowserOptions, logger }), + filterToBaseOptions({ ...withDefaults, logger }), ( flagManager: FlagManager, configuration: Configuration, diff --git a/packages/sdk/browser/src/options.ts b/packages/sdk/browser/src/options.ts index 89e53a4c82..28d29fa701 100644 --- a/packages/sdk/browser/src/options.ts +++ b/packages/sdk/browser/src/options.ts @@ -84,9 +84,10 @@ export function filterToBaseOptions(opts: BrowserOptions): LDOptionsBase { return baseOptions; } -function applyBrowserDefaults(opts: BrowserOptions) { +export function applyBrowserDefaults(opts: BrowserOptions): BrowserOptions { // eslint-disable-next-line no-param-reassign opts.flushInterval ??= DEFAULT_FLUSH_INTERVAL_SECONDS; + return opts; } export default function validateOptions(opts: BrowserOptions, logger: LDLogger): ValidatedOptions { diff --git a/packages/shared/common/__tests__/internal/events/EventSummarizer.test.ts b/packages/shared/common/__tests__/internal/events/EventSummarizer.test.ts index f8eee7251f..1fd713a9f7 100644 --- a/packages/shared/common/__tests__/internal/events/EventSummarizer.test.ts +++ b/packages/shared/common/__tests__/internal/events/EventSummarizer.test.ts @@ -12,16 +12,16 @@ describe('given an event summarizer', () => { }); it('does nothing for an identify event.', () => { - const beforeSummary = summarizer.getSummaries()[0]; + const beforeSummary = summarizer.getSummary(); summarizer.summarizeEvent(new InputIdentifyEvent(context)); - const afterSummary = summarizer.getSummaries()[0]; + const afterSummary = summarizer.getSummary(); expect(beforeSummary).toEqual(afterSummary); }); it('does nothing for a custom event.', () => { - const beforeSummary = summarizer.getSummaries()[0]; + const beforeSummary = summarizer.getSummary(); summarizer.summarizeEvent(new InputCustomEvent(context, 'custom', 'potato', 17)); - const afterSummary = summarizer.getSummaries()[0]; + const afterSummary = summarizer.getSummary(); expect(beforeSummary).toEqual(afterSummary); }); @@ -33,9 +33,9 @@ describe('given an event summarizer', () => { context, excludeFromSummaries: true, }; - const beforeSummary = summarizer.getSummaries()[0]; + const beforeSummary = summarizer.getSummary(); summarizer.summarizeEvent(event as any); - const afterSummary = summarizer.getSummaries()[0]; + const afterSummary = summarizer.getSummary(); expect(beforeSummary).toEqual(afterSummary); }); @@ -62,7 +62,7 @@ describe('given an event summarizer', () => { summarizer.summarizeEvent(event1 as any); summarizer.summarizeEvent(event2 as any); summarizer.summarizeEvent(event3 as any); - const data = summarizer.getSummaries()[0]; + const data = summarizer.getSummary(); expect(data.startDate).toEqual(1000); expect(data.endDate).toEqual(2000); @@ -135,7 +135,7 @@ describe('given an event summarizer', () => { summarizer.summarizeEvent(event4 as any); summarizer.summarizeEvent(event5 as any); summarizer.summarizeEvent(event6 as any); - const summary = summarizer.getSummaries()[0]; + const summary = summarizer.getSummary(); summary.features.key1.counters.sort((a, b) => a.value - b.value); const expectedFeatures = { @@ -223,7 +223,7 @@ describe('given an event summarizer', () => { summarizer.summarizeEvent(event1 as any); summarizer.summarizeEvent(event2 as any); summarizer.summarizeEvent(event3 as any); - const data = summarizer.getSummaries()[0]; + const data = summarizer.getSummary(); data.features.key1.counters.sort((a, b) => a.value - b.value); const expectedFeatures = { @@ -282,7 +282,7 @@ describe('given an event summarizer', () => { summarizer.summarizeEvent(event1 as any); summarizer.summarizeEvent(event2 as any); summarizer.summarizeEvent(event3 as any); - const data = summarizer.getSummaries()[0]; + const data = summarizer.getSummary(); const expectedFeatures = { key1: { diff --git a/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts b/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts index 34dde09759..4f1fbae7fe 100644 --- a/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts +++ b/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts @@ -31,6 +31,12 @@ export interface SummarizedFlagsEvent { context?: any; } +export interface LDMultiEventSummarizer { + summarizeEvent(event: InputEvent): void; + getSummaries(): Promise; + clearSummary(): void; +} + /** * Interface for summarizing feature flag evaluation events. */ @@ -45,7 +51,7 @@ export default interface LDEventSummarizer { * Gets the current summary of processed events. * @returns A summary of all processed feature flag events */ - getSummaries(): SummarizedFlagsEvent[]; + getSummary(): SummarizedFlagsEvent; /** * Clears all summarized event data. diff --git a/packages/shared/common/src/internal/events/EventProcessor.ts b/packages/shared/common/src/internal/events/EventProcessor.ts index ab07c3ca5d..cfa050271b 100644 --- a/packages/shared/common/src/internal/events/EventProcessor.ts +++ b/packages/shared/common/src/internal/events/EventProcessor.ts @@ -2,7 +2,7 @@ import { LDEvaluationReason, LDLogger } from '../../api'; import { LDDeliveryStatus, LDEventType } from '../../api/subsystem'; import LDContextDeduplicator from '../../api/subsystem/LDContextDeduplicator'; import LDEventProcessor from '../../api/subsystem/LDEventProcessor'; -import { SummarizedFlagsEvent } from '../../api/subsystem/LDEventSummarizer'; +import { LDMultiEventSummarizer, SummarizedFlagsEvent } from '../../api/subsystem/LDEventSummarizer'; import AttributeReference from '../../AttributeReference'; import ContextFilter from '../../ContextFilter'; import { ClientContext } from '../../options'; @@ -108,6 +108,10 @@ export interface EventProcessorOptions { diagnosticRecordingInterval: number; } +function isMultiEventSummarizer(summarizer: LDMultiEventSummarizer | EventSummarizer): summarizer is LDMultiEventSummarizer { + return (summarizer as LDMultiEventSummarizer).getSummaries !== undefined; +} + export default class EventProcessor implements LDEventProcessor { private _eventSender: EventSender; private _summarizer; @@ -215,21 +219,30 @@ export default class EventProcessor implements LDEventProcessor { if (this._shutdown) { throw new LDInvalidSDKKeyError( 'Events cannot be posted because a permanent error has been encountered. ' + - 'This is most likely an invalid SDK key. The specific error information ' + - 'is logged independently.', + 'This is most likely an invalid SDK key. The specific error information ' + + 'is logged independently.', ); } const eventsToFlush = this._queue; this._queue = []; - const summaries = this._summarizer.getSummaries(); - this._summarizer.clearSummary(); - summaries.forEach((summary) => { + if (isMultiEventSummarizer(this._summarizer)) { + const summaries = await this._summarizer.getSummaries(); + + summaries.forEach((summary) => { + if (Object.keys(summary.features).length) { + eventsToFlush.push(summary); + } + }); + } else { + const summary = this._summarizer.getSummary(); if (Object.keys(summary.features).length) { eventsToFlush.push(summary); } - }); + } + + this._summarizer.clearSummary(); if (!eventsToFlush.length) { return; diff --git a/packages/shared/common/src/internal/events/EventSummarizer.ts b/packages/shared/common/src/internal/events/EventSummarizer.ts index 3cf299c3be..2f1f649d37 100644 --- a/packages/shared/common/src/internal/events/EventSummarizer.ts +++ b/packages/shared/common/src/internal/events/EventSummarizer.ts @@ -11,9 +11,8 @@ import InputEvent from './InputEvent'; import SummaryCounter from './SummaryCounter'; function counterKey(event: InputEvalEvent) { - return `${event.key}:${ - event.variation !== null && event.variation !== undefined ? event.variation : '' - }:${event.version !== null && event.version !== undefined ? event.version : ''}`; + return `${event.key}:${event.variation !== null && event.variation !== undefined ? event.variation : '' + }:${event.version !== null && event.version !== undefined ? event.version : ''}`; } /** @@ -33,7 +32,7 @@ export default class EventSummarizer implements LDEventSummarizer { constructor( private readonly _singleContext: boolean = false, private readonly _contextFilter?: ContextFilter, - ) {} + ) { } summarizeEvent(event: InputEvent) { if (isFeature(event) && !event.excludeFromSummaries) { @@ -71,7 +70,7 @@ export default class EventSummarizer implements LDEventSummarizer { } } - getSummaries(): SummarizedFlagsEvent[] { + getSummary(): SummarizedFlagsEvent { const features = Object.values(this._counters).reduce( (acc: Record, counter) => { let flagSummary = acc[counter.key]; @@ -103,18 +102,16 @@ export default class EventSummarizer implements LDEventSummarizer { {}, ); - return [ - { - startDate: this._startDate, + return { + startDate: this._startDate, endDate: this._endDate, - features, - kind: 'summary', - context: - this._context !== undefined && this._singleContext - ? this._contextFilter?.filter(this._context) - : undefined, - }, - ]; + features, + kind: 'summary', + context: + this._context !== undefined && this._singleContext + ? this._contextFilter?.filter(this._context) + : undefined, + }; } clearSummary() { diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts index 0f87d0baac..6cc7b90f92 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts @@ -5,18 +5,18 @@ import EventSummarizer from './EventSummarizer'; import { isFeature } from './guards'; import InputEvent from './InputEvent'; -export default class MultiEventSummarizer implements LDEventSummarizer { +export default class LDMultiEventSummarizer implements LDMultiEventSummarizer { constructor( private readonly _crypto: Crypto, private readonly _contextFilter: ContextFilter, ) {} - - private _summarizers: Record = {}; + private _summarizers: Record = {}; + private _tasks: Promise[] = []; summarizeEvent(event: InputEvent) { // This will execute asynchronously, which means that a flush could happen before the event // is summarized. When that happens, then the event will just be in the next batch of summaries. - (async () => { + this._tasks.push((async () => { if (isFeature(event)) { const hash = await event.context.hash(this._crypto); if (!hash) { @@ -33,11 +33,16 @@ export default class MultiEventSummarizer implements LDEventSummarizer { summarizer.summarizeEvent(event); } - })(); + })()); } - getSummaries(): SummarizedFlagsEvent[] { - return Object.values(this._summarizers).flatMap((summarizer) => summarizer.getSummaries()); + + async getSummaries(): Promise { + await Promise.all(this._tasks); + const tmpTasks = this._tasks; + this._tasks = []; + return Object.values(this._summarizers).map((summarizer) => summarizer.getSummary()); } + clearSummary(): void { this._summarizers = {}; } From 79ea083c44fbd47303648f4ade467f532709cd28 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 14 Apr 2025 15:35:18 -0700 Subject: [PATCH 08/26] Don't mutate. --- packages/sdk/browser/src/options.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/sdk/browser/src/options.ts b/packages/sdk/browser/src/options.ts index 28d29fa701..c0a8870a6e 100644 --- a/packages/sdk/browser/src/options.ts +++ b/packages/sdk/browser/src/options.ts @@ -85,9 +85,9 @@ export function filterToBaseOptions(opts: BrowserOptions): LDOptionsBase { } export function applyBrowserDefaults(opts: BrowserOptions): BrowserOptions { - // eslint-disable-next-line no-param-reassign - opts.flushInterval ??= DEFAULT_FLUSH_INTERVAL_SECONDS; - return opts; + const output = { ...opts }; + output.flushInterval ??= DEFAULT_FLUSH_INTERVAL_SECONDS; + return output; } export default function validateOptions(opts: BrowserOptions, logger: LDLogger): ValidatedOptions { From 83e3651c54efd301e343943c49a74e51d5f98dfe Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 16 Apr 2025 09:50:54 -0700 Subject: [PATCH 09/26] Revert browser-telemetry changes. --- packages/telemetry/browser-telemetry/CHANGELOG.md | 9 +++++++++ packages/telemetry/browser-telemetry/package.json | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/telemetry/browser-telemetry/CHANGELOG.md b/packages/telemetry/browser-telemetry/CHANGELOG.md index 1c2f04429f..dbcb1d243c 100644 --- a/packages/telemetry/browser-telemetry/CHANGELOG.md +++ b/packages/telemetry/browser-telemetry/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## [1.0.5](https://github.com/launchdarkly/js-core/compare/browser-telemetry-v1.0.4...browser-telemetry-v1.0.5) (2025-04-15) + + +### Dependencies + +* The following workspace dependencies were updated + * devDependencies + * @launchdarkly/js-client-sdk bumped from 0.5.1 to 0.5.2 + ## [1.0.4](https://github.com/launchdarkly/js-core/compare/browser-telemetry-v1.0.3...browser-telemetry-v1.0.4) (2025-04-08) diff --git a/packages/telemetry/browser-telemetry/package.json b/packages/telemetry/browser-telemetry/package.json index ae97f4557a..4a7e2b40e1 100644 --- a/packages/telemetry/browser-telemetry/package.json +++ b/packages/telemetry/browser-telemetry/package.json @@ -1,6 +1,6 @@ { "name": "@launchdarkly/browser-telemetry", - "version": "1.0.4", + "version": "1.0.5", "packageManager": "yarn@3.4.1", "type": "module", "main": "./dist/index.cjs", @@ -45,7 +45,7 @@ }, "devDependencies": { "@jest/globals": "^29.7.0", - "@launchdarkly/js-client-sdk": "0.5.1", + "@launchdarkly/js-client-sdk": "0.5.2", "@trivago/prettier-plugin-sort-imports": "^4.1.1", "@types/css-font-loading-module": "^0.0.13", "@types/jest": "^29.5.11", From 5dd86366c24b9761ddef5b4ef7990bade89ddcfb Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 16 Apr 2025 09:52:49 -0700 Subject: [PATCH 10/26] Re-organization. --- .../src/api/subsystem/LDEventSummarizer.ts | 66 ------------------- .../src/internal/events/EventSummarizer.ts | 2 +- .../src/internal/events/LDEventSummarizer.ts | 66 +++++++++++++++++++ .../internal/events/MultiEventSummarizer.ts | 2 +- 4 files changed, 68 insertions(+), 68 deletions(-) delete mode 100644 packages/shared/common/src/api/subsystem/LDEventSummarizer.ts diff --git a/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts b/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts deleted file mode 100644 index 37ecbd91ee..0000000000 --- a/packages/shared/common/src/api/subsystem/LDEventSummarizer.ts +++ /dev/null @@ -1,66 +0,0 @@ -import InputEvent from '../../internal/events/InputEvent'; - -/** - * @internal - */ -export interface FlagCounter { - value: any; - count: number; - variation?: number; - version?: number; - unknown?: boolean; -} - -/** - * @internal - */ -export interface FlagSummary { - default: any; - counters: FlagCounter[]; - contextKinds?: string[]; -} - -/** - * @internal - */ -export interface SummarizedFlagsEvent { - startDate: number; - endDate: number; - features: Record; - kind: 'summary'; - context?: any; -} - -/** - * Interface for summarizing feature flag evaluations bucketed by the context. - */ -export interface LDMultiEventSummarizer { - /** - * Processes an event for summarization if it is a feature flag event and not excluded from summaries. - * @param event The event to potentially summarize - */ - summarizeEvent(event: InputEvent): void; - - /** - * Gets the current summary of processed events. - * @returns A summary of all processed feature flag events - */ - getSummaries(): SummarizedFlagsEvent[]; -} - -/** - * Interface for summarizing feature flag evaluation events. - */ -export default interface LDEventSummarizer { - /** - * Processes an event for summarization if it is a feature flag event and not excluded from summaries. - * @param event The event to potentially summarize - */ - summarizeEvent(event: InputEvent): void; - - /** - * Gets the current summary of processed events. - * @returns A summary of all processed feature flag events - */ - getSummary(): SummarizedFlagsEvent; -} diff --git a/packages/shared/common/src/internal/events/EventSummarizer.ts b/packages/shared/common/src/internal/events/EventSummarizer.ts index c985230fff..964cea4167 100644 --- a/packages/shared/common/src/internal/events/EventSummarizer.ts +++ b/packages/shared/common/src/internal/events/EventSummarizer.ts @@ -2,7 +2,7 @@ import LDEventSummarizer, { FlagCounter, FlagSummary, SummarizedFlagsEvent, -} from '../../api/subsystem/LDEventSummarizer'; +} from './LDEventSummarizer'; import Context from '../../Context'; import ContextFilter from '../../ContextFilter'; import { isFeature } from './guards'; diff --git a/packages/shared/common/src/internal/events/LDEventSummarizer.ts b/packages/shared/common/src/internal/events/LDEventSummarizer.ts index e69de29bb2..37ecbd91ee 100644 --- a/packages/shared/common/src/internal/events/LDEventSummarizer.ts +++ b/packages/shared/common/src/internal/events/LDEventSummarizer.ts @@ -0,0 +1,66 @@ +import InputEvent from '../../internal/events/InputEvent'; + +/** + * @internal + */ +export interface FlagCounter { + value: any; + count: number; + variation?: number; + version?: number; + unknown?: boolean; +} + +/** + * @internal + */ +export interface FlagSummary { + default: any; + counters: FlagCounter[]; + contextKinds?: string[]; +} + +/** + * @internal + */ +export interface SummarizedFlagsEvent { + startDate: number; + endDate: number; + features: Record; + kind: 'summary'; + context?: any; +} + +/** + * Interface for summarizing feature flag evaluations bucketed by the context. + */ +export interface LDMultiEventSummarizer { + /** + * Processes an event for summarization if it is a feature flag event and not excluded from summaries. + * @param event The event to potentially summarize + */ + summarizeEvent(event: InputEvent): void; + + /** + * Gets the current summary of processed events. + * @returns A summary of all processed feature flag events + */ + getSummaries(): SummarizedFlagsEvent[]; +} + +/** + * Interface for summarizing feature flag evaluation events. + */ +export default interface LDEventSummarizer { + /** + * Processes an event for summarization if it is a feature flag event and not excluded from summaries. + * @param event The event to potentially summarize + */ + summarizeEvent(event: InputEvent): void; + + /** + * Gets the current summary of processed events. + * @returns A summary of all processed feature flag events + */ + getSummary(): SummarizedFlagsEvent; +} diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts index 62858907da..5ebe0e718a 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts @@ -1,5 +1,5 @@ import { Crypto } from '../../api'; -import { SummarizedFlagsEvent } from '../../api/subsystem/LDEventSummarizer'; +import { SummarizedFlagsEvent } from './LDEventSummarizer'; import ContextFilter from '../../ContextFilter'; import EventSummarizer from './EventSummarizer'; import { isFeature } from './guards'; From 142299ce75cd41c750b12fc9b8a4ba71cb199e3d Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 16 Apr 2025 09:54:12 -0700 Subject: [PATCH 11/26] Missed import --- packages/shared/common/src/internal/events/EventProcessor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/common/src/internal/events/EventProcessor.ts b/packages/shared/common/src/internal/events/EventProcessor.ts index 21c0e369cc..ce5a4e9c16 100644 --- a/packages/shared/common/src/internal/events/EventProcessor.ts +++ b/packages/shared/common/src/internal/events/EventProcessor.ts @@ -5,7 +5,7 @@ import LDEventProcessor from '../../api/subsystem/LDEventProcessor'; import { LDMultiEventSummarizer, SummarizedFlagsEvent, -} from '../../api/subsystem/LDEventSummarizer'; +} from './LDEventSummarizer'; import AttributeReference from '../../AttributeReference'; import ContextFilter from '../../ContextFilter'; import { ClientContext } from '../../options'; From 0daf2f8bf346e07c4c97a04feb7bd4afc5ffeee1 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 16 Apr 2025 09:56:08 -0700 Subject: [PATCH 12/26] Lint --- .../common/src/internal/events/EventProcessor.ts | 5 +---- .../common/src/internal/events/EventSummarizer.ts | 10 +++++----- .../common/src/internal/events/LDEventSummarizer.ts | 2 +- .../common/src/internal/events/MultiEventSummarizer.ts | 2 +- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/shared/common/src/internal/events/EventProcessor.ts b/packages/shared/common/src/internal/events/EventProcessor.ts index ce5a4e9c16..d6ac10b6f7 100644 --- a/packages/shared/common/src/internal/events/EventProcessor.ts +++ b/packages/shared/common/src/internal/events/EventProcessor.ts @@ -2,10 +2,6 @@ import { LDEvaluationReason, LDLogger } from '../../api'; import { LDDeliveryStatus, LDEventType } from '../../api/subsystem'; import LDContextDeduplicator from '../../api/subsystem/LDContextDeduplicator'; import LDEventProcessor from '../../api/subsystem/LDEventProcessor'; -import { - LDMultiEventSummarizer, - SummarizedFlagsEvent, -} from './LDEventSummarizer'; import AttributeReference from '../../AttributeReference'; import ContextFilter from '../../ContextFilter'; import { ClientContext } from '../../options'; @@ -17,6 +13,7 @@ import { isFeature, isIdentify, isMigration } from './guards'; import InputEvent from './InputEvent'; import InputIdentifyEvent from './InputIdentifyEvent'; import InputMigrationEvent from './InputMigrationEvent'; +import { LDMultiEventSummarizer, SummarizedFlagsEvent } from './LDEventSummarizer'; import LDInvalidSDKKeyError from './LDInvalidSDKKeyError'; import MultiEventSummarizer from './MultiEventSummarizer'; import shouldSample from './sampling'; diff --git a/packages/shared/common/src/internal/events/EventSummarizer.ts b/packages/shared/common/src/internal/events/EventSummarizer.ts index 964cea4167..9bd68a21ea 100644 --- a/packages/shared/common/src/internal/events/EventSummarizer.ts +++ b/packages/shared/common/src/internal/events/EventSummarizer.ts @@ -1,13 +1,13 @@ -import LDEventSummarizer, { - FlagCounter, - FlagSummary, - SummarizedFlagsEvent, -} from './LDEventSummarizer'; import Context from '../../Context'; import ContextFilter from '../../ContextFilter'; import { isFeature } from './guards'; import InputEvalEvent from './InputEvalEvent'; import InputEvent from './InputEvent'; +import LDEventSummarizer, { + FlagCounter, + FlagSummary, + SummarizedFlagsEvent, +} from './LDEventSummarizer'; import SummaryCounter from './SummaryCounter'; function counterKey(event: InputEvalEvent) { diff --git a/packages/shared/common/src/internal/events/LDEventSummarizer.ts b/packages/shared/common/src/internal/events/LDEventSummarizer.ts index 37ecbd91ee..5ca3ff869c 100644 --- a/packages/shared/common/src/internal/events/LDEventSummarizer.ts +++ b/packages/shared/common/src/internal/events/LDEventSummarizer.ts @@ -1,4 +1,4 @@ -import InputEvent from '../../internal/events/InputEvent'; +import InputEvent from './InputEvent'; /** * @internal diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts index 5ebe0e718a..66abcfae96 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts @@ -1,9 +1,9 @@ import { Crypto } from '../../api'; -import { SummarizedFlagsEvent } from './LDEventSummarizer'; import ContextFilter from '../../ContextFilter'; import EventSummarizer from './EventSummarizer'; import { isFeature } from './guards'; import InputEvent from './InputEvent'; +import { SummarizedFlagsEvent } from './LDEventSummarizer'; export default class LDMultiEventSummarizer implements LDMultiEventSummarizer { constructor( From f7fae6f1d00ecbf4026a8cc776c14213c540b551 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 17 Apr 2025 13:58:21 -0700 Subject: [PATCH 13/26] Remove optionality for context kinds. --- packages/shared/common/src/internal/events/LDEventSummarizer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/common/src/internal/events/LDEventSummarizer.ts b/packages/shared/common/src/internal/events/LDEventSummarizer.ts index 5ca3ff869c..53ada4d5b4 100644 --- a/packages/shared/common/src/internal/events/LDEventSummarizer.ts +++ b/packages/shared/common/src/internal/events/LDEventSummarizer.ts @@ -17,7 +17,7 @@ export interface FlagCounter { export interface FlagSummary { default: any; counters: FlagCounter[]; - contextKinds?: string[]; + contextKinds: string[]; } /** From 1922be1d26cd3a8b9c4b4577558562b48313a10a Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 17 Apr 2025 14:49:49 -0700 Subject: [PATCH 14/26] Add contract test capability for per-context summary events. --- .../browser/contract-tests/entity/src/TestHarnessWebSocket.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts b/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts index c1c280d483..20185a02e7 100644 --- a/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts +++ b/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts @@ -42,6 +42,7 @@ export default class TestHarnessWebSocket { 'anonymous-redaction', 'strongly-typed', 'client-prereq-events', + 'client-per-context-summaries' ]; break; From 8c871b604e7fe14d77ce1c3c09cfa5b115539a62 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 17 Apr 2025 15:06:31 -0700 Subject: [PATCH 15/26] Lint --- .../browser/contract-tests/entity/src/TestHarnessWebSocket.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts b/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts index 20185a02e7..2b696ac264 100644 --- a/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts +++ b/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts @@ -9,7 +9,7 @@ export default class TestHarnessWebSocket { private _clientCounter = 0; private _logger: LDLogger = makeLogger('TestHarnessWebSocket'); - constructor(private readonly _url: string) {} + constructor(private readonly _url: string) { } connect() { this._logger.info(`Connecting to web socket.`); From 0dc4419bcb841784d372e8d1b25d351809aaf3c4 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 17 Apr 2025 15:28:04 -0700 Subject: [PATCH 16/26] Lint --- .../browser/contract-tests/entity/src/TestHarnessWebSocket.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts b/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts index 2b696ac264..6fef4c6519 100644 --- a/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts +++ b/packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts @@ -9,7 +9,7 @@ export default class TestHarnessWebSocket { private _clientCounter = 0; private _logger: LDLogger = makeLogger('TestHarnessWebSocket'); - constructor(private readonly _url: string) { } + constructor(private readonly _url: string) {} connect() { this._logger.info(`Connecting to web socket.`); @@ -42,7 +42,7 @@ export default class TestHarnessWebSocket { 'anonymous-redaction', 'strongly-typed', 'client-prereq-events', - 'client-per-context-summaries' + 'client-per-context-summaries', ]; break; From 260692f0c1ec43527bd3f21ad402de5293f8754b Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 22 Apr 2025 15:22:07 -0700 Subject: [PATCH 17/26] Remove double hashing of kind. --- packages/shared/common/src/Context.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/shared/common/src/Context.ts b/packages/shared/common/src/Context.ts index 9212f6aa75..dfab7e9e74 100644 --- a/packages/shared/common/src/Context.ts +++ b/packages/shared/common/src/Context.ts @@ -480,7 +480,6 @@ export default class Context { }[] = []; const kinds = this.kinds.sort(); - kinds.forEach((kind) => hasher.update(kind)); kinds.forEach((kind) => { hasher.update(kind); From c28f996c306ce97eb81ef3532044defc6bd5f245 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 23 Apr 2025 11:32:53 -0700 Subject: [PATCH 18/26] Remove await from sync method. --- packages/shared/common/src/internal/events/EventProcessor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/common/src/internal/events/EventProcessor.ts b/packages/shared/common/src/internal/events/EventProcessor.ts index d6ac10b6f7..f163bc2443 100644 --- a/packages/shared/common/src/internal/events/EventProcessor.ts +++ b/packages/shared/common/src/internal/events/EventProcessor.ts @@ -230,7 +230,7 @@ export default class EventProcessor implements LDEventProcessor { this._queue = []; if (isMultiEventSummarizer(this._summarizer)) { - const summaries = await this._summarizer.getSummaries(); + const summaries = this._summarizer.getSummaries(); summaries.forEach((summary) => { if (Object.keys(summary.features).length) { From 8ba61a49522d45f13a70940a23d9f2c0a4d5baa9 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 23 Apr 2025 14:22:30 -0700 Subject: [PATCH 19/26] Async consistency. --- .../shared/common/__tests__/setupCrypto.ts | 23 ++++++++++++++++ .../src/internal/events/EventProcessor.ts | 13 +++++----- .../src/internal/events/LDEventSummarizer.ts | 2 +- .../events/MultiEventSummarizer.test.ts | 26 +++++++++---------- .../internal/events/MultiEventSummarizer.ts | 26 ++++++++++++++----- 5 files changed, 63 insertions(+), 27 deletions(-) diff --git a/packages/shared/common/__tests__/setupCrypto.ts b/packages/shared/common/__tests__/setupCrypto.ts index a5f7b4b5ec..79be779609 100644 --- a/packages/shared/common/__tests__/setupCrypto.ts +++ b/packages/shared/common/__tests__/setupCrypto.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-classes-per-file */ import { Hasher } from '../src/api'; class MockHasher implements Hasher { @@ -15,6 +16,23 @@ class MockHasher implements Hasher { } } +class MockAsyncHasher implements Hasher { + private _state: string[] = []; + + update(value: string): Hasher { + this._state.push(value); + return this; + } + + async asyncDigest(): Promise { + return new Promise((resolve) => { + setTimeout(() => { + resolve(this._state.join('')); + }, 1); + }); + } +} + export const setupCrypto = () => { let counter = 0; @@ -29,3 +47,8 @@ export const setupCrypto = () => { }), }; }; + +export const setupAsyncCrypto = () => ({ + ...setupCrypto(), + createHash: jest.fn(() => new MockAsyncHasher()), +}); diff --git a/packages/shared/common/src/internal/events/EventProcessor.ts b/packages/shared/common/src/internal/events/EventProcessor.ts index f163bc2443..17a6608d23 100644 --- a/packages/shared/common/src/internal/events/EventProcessor.ts +++ b/packages/shared/common/src/internal/events/EventProcessor.ts @@ -13,7 +13,10 @@ import { isFeature, isIdentify, isMigration } from './guards'; import InputEvent from './InputEvent'; import InputIdentifyEvent from './InputIdentifyEvent'; import InputMigrationEvent from './InputMigrationEvent'; -import { LDMultiEventSummarizer, SummarizedFlagsEvent } from './LDEventSummarizer'; +import LDEventSummarizer, { + LDMultiEventSummarizer, + SummarizedFlagsEvent, +} from './LDEventSummarizer'; import LDInvalidSDKKeyError from './LDInvalidSDKKeyError'; import MultiEventSummarizer from './MultiEventSummarizer'; import shouldSample from './sampling'; @@ -108,15 +111,13 @@ export interface EventProcessorOptions { diagnosticRecordingInterval: number; } -function isMultiEventSummarizer( - summarizer: LDMultiEventSummarizer | EventSummarizer, -): summarizer is LDMultiEventSummarizer { +function isMultiEventSummarizer(summarizer: unknown): summarizer is LDMultiEventSummarizer { return (summarizer as LDMultiEventSummarizer).getSummaries !== undefined; } export default class EventProcessor implements LDEventProcessor { private _eventSender: EventSender; - private _summarizer; + private _summarizer: LDMultiEventSummarizer | LDEventSummarizer; private _queue: OutputEvent[] = []; private _lastKnownPastTime = 0; private _droppedEvents = 0; @@ -230,7 +231,7 @@ export default class EventProcessor implements LDEventProcessor { this._queue = []; if (isMultiEventSummarizer(this._summarizer)) { - const summaries = this._summarizer.getSummaries(); + const summaries = await this._summarizer.getSummaries(); summaries.forEach((summary) => { if (Object.keys(summary.features).length) { diff --git a/packages/shared/common/src/internal/events/LDEventSummarizer.ts b/packages/shared/common/src/internal/events/LDEventSummarizer.ts index 53ada4d5b4..d0d80da059 100644 --- a/packages/shared/common/src/internal/events/LDEventSummarizer.ts +++ b/packages/shared/common/src/internal/events/LDEventSummarizer.ts @@ -45,7 +45,7 @@ export interface LDMultiEventSummarizer { * Gets the current summary of processed events. * @returns A summary of all processed feature flag events */ - getSummaries(): SummarizedFlagsEvent[]; + getSummaries(): Promise; } /** diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts index b412e7ba18..8fa5c9b828 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts @@ -1,15 +1,18 @@ -import { setupCrypto } from '../../../__tests__/setupCrypto'; +import { setupAsyncCrypto, setupCrypto } from '../../../__tests__/setupCrypto'; import Context from '../../Context'; import ContextFilter from '../../ContextFilter'; import InputEvalEvent from './InputEvalEvent'; import InputIdentifyEvent from './InputIdentifyEvent'; import MultiEventSummarizer from './MultiEventSummarizer'; -describe('with mocked crypto and hasher', () => { +// Test with both sync and async crypto implementations +describe.each([ + ['sync', setupCrypto()], + ['async', setupAsyncCrypto()], +])('with mocked crypto and hasher/%s', (_name, crypto) => { let summarizer: MultiEventSummarizer; beforeEach(() => { - const crypto = setupCrypto(); const contextFilter = new ContextFilter(false, []); summarizer = new MultiEventSummarizer(crypto, contextFilter); }); @@ -19,9 +22,8 @@ describe('with mocked crypto and hasher', () => { const event = new InputEvalEvent(true, context, 'flag-key', 'value', 'default', 1, 0, true); summarizer.summarizeEvent(event); - await new Promise(process.nextTick); // Wait for async operations - const summaries = summarizer.getSummaries(); + const summaries = await summarizer.getSummaries(); expect(summaries).toHaveLength(1); }); @@ -32,9 +34,8 @@ describe('with mocked crypto and hasher', () => { summarizer.summarizeEvent(event1); summarizer.summarizeEvent(event2); - await new Promise(process.nextTick); - const summaries = summarizer.getSummaries(); + const summaries = await summarizer.getSummaries(); expect(summaries).toHaveLength(1); }); @@ -43,9 +44,8 @@ describe('with mocked crypto and hasher', () => { const event = new InputIdentifyEvent(context); summarizer.summarizeEvent(event); - await new Promise(process.nextTick); - const summaries = summarizer.getSummaries(); + const summaries = await summarizer.getSummaries(); expect(summaries).toHaveLength(0); }); @@ -57,9 +57,8 @@ describe('with mocked crypto and hasher', () => { summarizer.summarizeEvent(event1); summarizer.summarizeEvent(event2); - await new Promise(process.nextTick); - const summaries = summarizer.getSummaries(); + const summaries = await summarizer.getSummaries(); expect(summaries).toHaveLength(2); }); @@ -68,10 +67,9 @@ describe('with mocked crypto and hasher', () => { const event = new InputEvalEvent(true, context, 'flag-key', 'value', 'default', 1, 0, true); summarizer.summarizeEvent(event); - await new Promise(process.nextTick); - const summariesA = summarizer.getSummaries(); - const summariesB = summarizer.getSummaries(); + const summariesA = await summarizer.getSummaries(); + const summariesB = await summarizer.getSummaries(); expect(summariesA).toHaveLength(1); expect(summariesB).toHaveLength(0); }); diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts index 66abcfae96..8981cf1723 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts @@ -3,19 +3,20 @@ import ContextFilter from '../../ContextFilter'; import EventSummarizer from './EventSummarizer'; import { isFeature } from './guards'; import InputEvent from './InputEvent'; -import { SummarizedFlagsEvent } from './LDEventSummarizer'; +import { LDMultiEventSummarizer, SummarizedFlagsEvent } from './LDEventSummarizer'; -export default class LDMultiEventSummarizer implements LDMultiEventSummarizer { +export default class MultiEventSummarizer implements LDMultiEventSummarizer { constructor( private readonly _crypto: Crypto, private readonly _contextFilter: ContextFilter, ) {} private _summarizers: Record = {}; + private _pendingPromises: Promise[] = []; summarizeEvent(event: InputEvent) { - // This will execute asynchronously, which means that a flush could happen before the event - // is summarized. When that happens, then the event will just be in the next batch of summaries. - (async () => { + // The event is summarized asynchronously, but the promise is created synchronously, this means that all events + // which have been requested to be summarized will be in the next flush. + const promise = (async () => { if (isFeature(event)) { const hash = await event.context.hash(this._crypto); if (!hash) { @@ -33,9 +34,22 @@ export default class LDMultiEventSummarizer implements LDMultiEventSummarizer { summarizer.summarizeEvent(event); } })(); + this._pendingPromises.push(promise); + promise.finally(() => { + const index = this._pendingPromises.indexOf(promise); + if (index !== -1) { + this._pendingPromises.splice(index, 1); + } + }); } - getSummaries(): SummarizedFlagsEvent[] { + async getSummaries(): Promise { + // Wait for any pending summarizations to complete + // Additional tasks queued while waiting will not be waited for. + await Promise.all([...this._pendingPromises]); + + // It is important not to put any async operations between caching the summarizers and clearing them. + // If we did then summerizers added during the async operation would be lost. const summarizersToFlush = this._summarizers; this._summarizers = {}; return Object.values(summarizersToFlush).map((summarizer) => summarizer.getSummary()); From 5a4d62f16b33363af2442073e5c16db952e09910 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 23 Apr 2025 14:35:32 -0700 Subject: [PATCH 20/26] Attempt to pin parse5. --- packages/sdk/browser/package.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/sdk/browser/package.json b/packages/sdk/browser/package.json index 353aee62b0..b64a426c1c 100644 --- a/packages/sdk/browser/package.json +++ b/packages/sdk/browser/package.json @@ -71,12 +71,15 @@ "eslint-plugin-jest": "^27.6.3", "eslint-plugin-prettier": "^5.0.0", "jest": "^29.7.0", - "jest-environment-jsdom": "^29.7.0", + "jest-environment-jsdom": "29.7.0", "prettier": "^3.0.0", "rimraf": "^5.0.5", "ts-jest": "^29.1.1", "tsup": "^8.3.5", "typedoc": "0.25.0", "typescript": "^5.5.3" + }, + "resolutions": { + "parse5": "7.2.1" } } From df6fb054eefdab2a51109a0d026a5b2ce30c05c8 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 23 Apr 2025 14:40:04 -0700 Subject: [PATCH 21/26] Move resolutions to top level. --- package.json | 3 ++- packages/sdk/browser/package.json | 3 --- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index cf21e229fd..204f37e542 100644 --- a/package.json +++ b/package.json @@ -70,6 +70,7 @@ "packageManager": "yarn@3.4.1", "//": "Pin jsonc-parser because v3.3.0 breaks rollup-plugin-esbuild", "resolutions": { - "jsonc-parser": "3.2.0" + "jsonc-parser": "3.2.0", + "parse5": "7.2.1" } } diff --git a/packages/sdk/browser/package.json b/packages/sdk/browser/package.json index b64a426c1c..3e133c28d7 100644 --- a/packages/sdk/browser/package.json +++ b/packages/sdk/browser/package.json @@ -78,8 +78,5 @@ "tsup": "^8.3.5", "typedoc": "0.25.0", "typescript": "^5.5.3" - }, - "resolutions": { - "parse5": "7.2.1" } } From 5d75446093a71fdc40576dc966c51d24899b2540 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 23 Apr 2025 17:15:11 -0700 Subject: [PATCH 22/26] Extended test, fixed missing keys. --- packages/sdk/react-native/example/App.tsx | 3 +- .../sdk/react-native/example/src/welcome.tsx | 2 +- .../shared/common/__tests__/Context.test.ts | 143 ++++++++++++++++++ packages/shared/common/src/Context.ts | 2 + 4 files changed, 148 insertions(+), 2 deletions(-) diff --git a/packages/sdk/react-native/example/App.tsx b/packages/sdk/react-native/example/App.tsx index fad2c67a35..274d171bd6 100644 --- a/packages/sdk/react-native/example/App.tsx +++ b/packages/sdk/react-native/example/App.tsx @@ -8,8 +8,9 @@ import { import Welcome from './src/welcome'; -const featureClient = new ReactNativeLDClient(MOBILE_KEY, AutoEnvAttributes.Enabled, { +const featureClient = new ReactNativeLDClient("mob-8b772ad8-5d5b-435f-982a-900fa5db47e6", AutoEnvAttributes.Enabled, { debug: true, + eventsUri: 'https://eb5d04264133.ngrok.app', applicationInfo: { id: 'ld-rn-test-app', version: '0.0.1', diff --git a/packages/sdk/react-native/example/src/welcome.tsx b/packages/sdk/react-native/example/src/welcome.tsx index f726fe8659..baf70ffba1 100644 --- a/packages/sdk/react-native/example/src/welcome.tsx +++ b/packages/sdk/react-native/example/src/welcome.tsx @@ -24,7 +24,7 @@ export default function Welcome() { return ( - Welcome to LaunchDarkly + Welcome to LaunchDarkly!! {flagKey}: {`${flagValue}`} diff --git a/packages/shared/common/__tests__/Context.test.ts b/packages/shared/common/__tests__/Context.test.ts index d9702e9a2a..12fb4cf802 100644 --- a/packages/shared/common/__tests__/Context.test.ts +++ b/packages/shared/common/__tests__/Context.test.ts @@ -520,4 +520,147 @@ describe('given mock crypto', () => { // The hashes should be different because private attributes are included in the hash expect(hashWithPrivate).not.toEqual(hashWithoutPrivate); }); + + it('uses the keys the keys of attributes in the hash', async () => { + const a = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + a: 'b', + }); + + const b = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + b: 'b', + }); + + const hashA = await a.hash(crypto); + const hashB = await b.hash(crypto); + expect(hashA).not.toBe(hashB); + }); + + it('uses the keys of nested objects inside the hash', async () => { + const a = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + nested: { + level1: { + level2: { + value: 'deep', + }, + }, + }, + }); + + const b = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + nested: { + sub1: { + sub2: { + value: 'deep', + }, + }, + }, + }); + + const hashA = await a.hash(crypto); + const hashB = await b.hash(crypto); + expect(hashA).not.toBe(hashB); + }); + + it('it uses the values of nested array in calculations', async () => { + const a = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + array: [1, 2, 3], + nestedArray: [ + [1, 2], + [3, 4], + ], + }); + + const b = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + array: [1, 2, 3], + nestedArray: [ + [2, 1], + [3, 4], + ], + }); + + const hashA = await a.hash(crypto); + const hashB = await b.hash(crypto); + expect(hashA).not.toBe(hashB); + }); + + it('uses the values of nested objects inside the hash', async () => { + const a = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + nested: { + level1: { + level2: { + value: 'deep', + }, + }, + }, + }); + + const b = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + nested: { + level1: { + level2: { + value: 'deeper', + }, + }, + }, + }); + + const hashA = await a.hash(crypto); + const hashB = await b.hash(crypto); + expect(hashA).not.toBe(hashB); + }); + + it('produces the same value for the given context', async () => { + // This isn't so much a test as it is a detection of change. + // If this test failed, and you didn't expect it, then you probably need to make sure your + // change makes sense. + const complexContext = Context.fromLDContext({ + kind: 'multi', + org: { + key: 'testKey', + name: 'testName', + cat: 'calico', + dog: 'lab', + anonymous: true, + nestedArray: [ + [1, 2], + [3, 4], + ], + _meta: { + privateAttributes: ['/a/b/c', 'cat', 'custom/dog'], + }, + }, + customer: { + key: 'testKey', + name: 'testName', + bird: 'party parrot', + chicken: 'hen', + nested: { + level1: { + level2: { + value: 'deep', + }, + }, + }, + }, + }); + expect(await complexContext.hash(crypto)).toBe( + 'customerbirdchickenkeynamenestedorganonymouscatdogkeynamenestedArraya/b/ccatcustom/dog01length201length24301length221testNametestKeylabcalicotruelevel1level2valuedeeptestNametestKeyhenparty parrot', + ); + }); }); diff --git a/packages/shared/common/src/Context.ts b/packages/shared/common/src/Context.ts index dfab7e9e74..d45f839af8 100644 --- a/packages/shared/common/src/Context.ts +++ b/packages/shared/common/src/Context.ts @@ -491,6 +491,7 @@ export default class Context { if (key === '_meta') { return; } + hasher.update(key); stack.push({ target: context[key], visited: [context], @@ -517,6 +518,7 @@ export default class Context { if (key === '_meta') { return; } + hasher.update(key); stack.push({ target: target[key], visited: [...visited, target], From 6c3f4685b29ab79dd8b35a76efc308f6697619d9 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 24 Apr 2025 09:26:15 -0700 Subject: [PATCH 23/26] Additional tests. --- .../shared/common/__tests__/Context.test.ts | 39 +++++++++++++++++-- packages/shared/common/src/Context.ts | 4 -- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/packages/shared/common/__tests__/Context.test.ts b/packages/shared/common/__tests__/Context.test.ts index 12fb4cf802..ab9928e119 100644 --- a/packages/shared/common/__tests__/Context.test.ts +++ b/packages/shared/common/__tests__/Context.test.ts @@ -330,7 +330,7 @@ describe('given a multi context', () => { describe('given mock crypto', () => { const crypto = setupCrypto(); - it('two equal contexts hash the same', async () => { + it('hashes two equal contexts the same', async () => { const a = Context.fromLDContext({ kind: 'multi', org: { @@ -373,7 +373,7 @@ describe('given mock crypto', () => { expect(await a.hash(crypto)).toEqual(await b.hash(crypto)); }); - it('legacy and non-legacy equivalent contexts hash the same', async () => { + it('hashes legacy and non-legacy equivalent contexts the same', async () => { const legacy = Context.fromLDContext({ key: 'testKey', name: 'testName', @@ -397,7 +397,7 @@ describe('given mock crypto', () => { expect(await legacy.hash(crypto)).toEqual(await nonLegacy.hash(crypto)); }); - it('single context and multi-context with one kind hash the same', async () => { + it('hashes single context and multi-context with one kind the same', async () => { const single = Context.fromLDContext({ kind: 'org', key: 'testKey', @@ -625,6 +625,36 @@ describe('given mock crypto', () => { expect(hashA).not.toBe(hashB); }); + it('hashes _meta in attributes', async () => { + const a = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + nested: { + level1: { + level2: { + _meta: { test: 'a' }, + }, + }, + }, + }); + + const b = Context.fromLDContext({ + kind: 'user', + key: 'testKey', + nested: { + level1: { + level2: { + _meta: { test: 'b' }, + }, + }, + }, + }); + + const hashA = await a.hash(crypto); + const hashB = await b.hash(crypto); + expect(hashA).not.toBe(hashB); + }); + it('produces the same value for the given context', async () => { // This isn't so much a test as it is a detection of change. // If this test failed, and you didn't expect it, then you probably need to make sure your @@ -654,13 +684,14 @@ describe('given mock crypto', () => { level1: { level2: { value: 'deep', + _meta: { thisShouldBeInTheHash: true }, }, }, }, }, }); expect(await complexContext.hash(crypto)).toBe( - 'customerbirdchickenkeynamenestedorganonymouscatdogkeynamenestedArraya/b/ccatcustom/dog01length201length24301length221testNametestKeylabcalicotruelevel1level2valuedeeptestNametestKeyhenparty parrot', + 'customerbirdchickenkeynamenestedorganonymouscatdogkeynamenestedArraya/b/ccatcustom/dog01length201length24301length221testNametestKeylabcalicotruelevel1level2_metavaluedeepthisShouldBeInTheHashtruetestNametestKeyhenparty parrot', ); }); }); diff --git a/packages/shared/common/src/Context.ts b/packages/shared/common/src/Context.ts index d45f839af8..dccc0ab135 100644 --- a/packages/shared/common/src/Context.ts +++ b/packages/shared/common/src/Context.ts @@ -514,10 +514,6 @@ export default class Context { Object.getOwnPropertyNames(target) .sort() .forEach((key) => { - // Handled using private attributes. - if (key === '_meta') { - return; - } hasher.update(key); stack.push({ target: target[key], From 5f8f99228738ad6f0ba763249937e8c27205983f Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 25 Apr 2025 08:55:43 -0700 Subject: [PATCH 24/26] Collision test. --- .../shared/common/__tests__/Context.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/shared/common/__tests__/Context.test.ts b/packages/shared/common/__tests__/Context.test.ts index ab9928e119..4ab5baf055 100644 --- a/packages/shared/common/__tests__/Context.test.ts +++ b/packages/shared/common/__tests__/Context.test.ts @@ -694,4 +694,22 @@ describe('given mock crypto', () => { 'customerbirdchickenkeynamenestedorganonymouscatdogkeynamenestedArraya/b/ccatcustom/dog01length201length24301length221testNametestKeylabcalicotruelevel1level2_metavaluedeepthisShouldBeInTheHashtruetestNametestKeyhenparty parrot', ); }); + + it('collisiontest', async () => { + const a = Context.fromLDContext({ + kind: 'user', + key: 'bob', + a: 'bcd', + }); + + const b = Context.fromLDContext({ + kind: 'user', + key: 'bob', + a: { b: { c: 'd' } }, + }); + + const hashA = await a.hash(crypto); + const hashB = await b.hash(crypto); + expect(hashA).not.toBe(hashB); + }); }); From 89d2b75a227bd7f3d29c071f37a174d9826a58c2 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 15 May 2025 15:58:31 -0700 Subject: [PATCH 25/26] Use canonicalized JSON. --- .../shared/common/__tests__/Context.test.ts | 46 +--------- .../__tests__/json/canonicalize.test.ts | 86 +++++++++++++++++++ .../__tests__/json/testdata/LICENSE.txt | 13 +++ .../common/__tests__/json/testdata/VENDOR.txt | 2 + .../__tests__/json/testdata/input/arrays.json | 8 ++ .../__tests__/json/testdata/input/french.json | 6 ++ .../json/testdata/input/structures.json | 8 ++ .../json/testdata/input/unicode.json | 3 + .../__tests__/json/testdata/input/values.json | 5 ++ .../__tests__/json/testdata/input/weird.json | 11 +++ .../json/testdata/output/arrays.json | 1 + .../json/testdata/output/french.json | 1 + .../json/testdata/output/structures.json | 1 + .../json/testdata/output/unicode.json | 1 + .../json/testdata/output/values.json | 1 + .../__tests__/json/testdata/output/weird.json | 1 + packages/shared/common/src/Context.ts | 67 +++------------ .../src/internal/events/EventProcessor.ts | 1 + .../events/MultiEventSummarizer.test.ts | 34 ++++++-- .../internal/events/MultiEventSummarizer.ts | 8 +- .../shared/common/src/json/canonicalize.ts | 37 ++++++++ 21 files changed, 234 insertions(+), 107 deletions(-) create mode 100644 packages/shared/common/__tests__/json/canonicalize.test.ts create mode 100644 packages/shared/common/__tests__/json/testdata/LICENSE.txt create mode 100644 packages/shared/common/__tests__/json/testdata/VENDOR.txt create mode 100644 packages/shared/common/__tests__/json/testdata/input/arrays.json create mode 100644 packages/shared/common/__tests__/json/testdata/input/french.json create mode 100644 packages/shared/common/__tests__/json/testdata/input/structures.json create mode 100644 packages/shared/common/__tests__/json/testdata/input/unicode.json create mode 100644 packages/shared/common/__tests__/json/testdata/input/values.json create mode 100644 packages/shared/common/__tests__/json/testdata/input/weird.json create mode 100644 packages/shared/common/__tests__/json/testdata/output/arrays.json create mode 100644 packages/shared/common/__tests__/json/testdata/output/french.json create mode 100644 packages/shared/common/__tests__/json/testdata/output/structures.json create mode 100644 packages/shared/common/__tests__/json/testdata/output/unicode.json create mode 100644 packages/shared/common/__tests__/json/testdata/output/values.json create mode 100644 packages/shared/common/__tests__/json/testdata/output/weird.json create mode 100644 packages/shared/common/src/json/canonicalize.ts diff --git a/packages/shared/common/__tests__/Context.test.ts b/packages/shared/common/__tests__/Context.test.ts index 4ab5baf055..df61e88183 100644 --- a/packages/shared/common/__tests__/Context.test.ts +++ b/packages/shared/common/__tests__/Context.test.ts @@ -373,50 +373,6 @@ describe('given mock crypto', () => { expect(await a.hash(crypto)).toEqual(await b.hash(crypto)); }); - it('hashes legacy and non-legacy equivalent contexts the same', async () => { - const legacy = Context.fromLDContext({ - key: 'testKey', - name: 'testName', - custom: { cat: 'calico', dog: 'lab' }, - anonymous: true, - privateAttributeNames: ['cat', 'dog'], - }); - - const nonLegacy = Context.fromLDContext({ - kind: 'user', - key: 'testKey', - name: 'testName', - cat: 'calico', - dog: 'lab', - anonymous: true, - _meta: { - privateAttributes: ['cat', 'dog'], - }, - }); - - expect(await legacy.hash(crypto)).toEqual(await nonLegacy.hash(crypto)); - }); - - it('hashes single context and multi-context with one kind the same', async () => { - const single = Context.fromLDContext({ - kind: 'org', - key: 'testKey', - name: 'testName', - value: 'testValue', - }); - - const multi = Context.fromLDContext({ - kind: 'multi', - org: { - key: 'testKey', - name: 'testName', - value: 'testValue', - }, - }); - - expect(await single.hash(crypto)).toEqual(await multi.hash(crypto)); - }); - it('handles shared references without getting stuck', async () => { const sharedObject = { value: 'shared' }; const context = Context.fromLDContext({ @@ -691,7 +647,7 @@ describe('given mock crypto', () => { }, }); expect(await complexContext.hash(crypto)).toBe( - 'customerbirdchickenkeynamenestedorganonymouscatdogkeynamenestedArraya/b/ccatcustom/dog01length201length24301length221testNametestKeylabcalicotruelevel1level2_metavaluedeepthisShouldBeInTheHashtruetestNametestKeyhenparty parrot', + '{"_contexts":{"customer":{"bird":"party parrot","chicken":"hen","key":"testKey","name":"testName","nested":{"level1":{"level2":{"_meta":{"thisShouldBeInTheHash":true},"value":"deep"}}}},"org":{"_meta":{"privateAttributes":["/a/b/c","cat","custom/dog"]},"anonymous":true,"cat":"calico","dog":"lab","key":"testKey","name":"testName","nestedArray":[[1,2],[3,4]]}},"_isMulti":true,"_isUser":false,"_privateAttributeReferences":{"customer":[],"org":[{"_components":["a","b","c"],"isValid":true,"redactionName":"/a/b/c"},{"_components":["cat"],"isValid":true,"redactionName":"cat"},{"_components":["custom/dog"],"isValid":true,"redactionName":"custom/dog"}]},"_wasLegacy":false,"kind":"multi","valid":true}', ); }); diff --git a/packages/shared/common/__tests__/json/canonicalize.test.ts b/packages/shared/common/__tests__/json/canonicalize.test.ts new file mode 100644 index 0000000000..20b3226fb9 --- /dev/null +++ b/packages/shared/common/__tests__/json/canonicalize.test.ts @@ -0,0 +1,86 @@ +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +import { canonicalize } from '../../src/json/canonicalize'; + +// Get the test file pairs +const testInputDir = path.join(__dirname, 'testdata', 'input'); +const testOutputDir = path.join(__dirname, 'testdata', 'output'); +const testFiles = fs.readdirSync(testInputDir); + +it.each(testFiles)('should correctly canonicalize %s', (filename) => { + // Load the input and expected output files + const inputPath = path.join(testInputDir, filename); + const outputPath = path.join(testOutputDir, filename); + + const inputData = JSON.parse(fs.readFileSync(inputPath, 'utf8')); + const expectedOutput = fs.readFileSync(outputPath, 'utf8'); + + // Apply the canonicalize function + const result = canonicalize(inputData); + + // Compare results + expect(result).toEqual(expectedOutput); +}); + +it('handles basic arrays', () => { + const input: any[] = []; + const expected = '[]'; + const result = canonicalize(input); + expect(result).toEqual(expected); +}); + +it('handles arrays of null/undefined', () => { + const input: any[] = [null, undefined]; + const expected = '[null,null]'; + const result = canonicalize(input); + expect(result).toEqual(expected); +}); + +it('handles objects with numeric keys', () => { + const input = { + 1: 'one', + 2: 'two', + }; + const expected = '{"1":"one","2":"two"}'; + const result = canonicalize(input); + expect(result).toEqual(expected); +}); + +it('handles objects with undefined values', () => { + const input = { + a: 'b', + c: undefined, + }; + const expected = '{"a":"b"}'; + const result = canonicalize(input); + expect(result).toEqual(expected); +}); + +it('handles an object with a symbol value', () => { + const input = { + a: 'b', + c: Symbol('c'), + }; + const expected = '{"a":"b"}'; + const result = canonicalize(input); + expect(result).toEqual(expected); +}); + +it('handles an object with a symbol key', () => { + const input = { + a: 'b', + [Symbol('c')]: 'd', + }; + const expected = '{"a":"b"}'; + const result = canonicalize(input); + expect(result).toEqual(expected); +}); + +it('should throw an error for objects with cycles', () => { + const a: any = {}; + const b: any = { a }; + a.b = b; + + expect(() => canonicalize(a)).toThrow('Cycle detected'); +}); diff --git a/packages/shared/common/__tests__/json/testdata/LICENSE.txt b/packages/shared/common/__tests__/json/testdata/LICENSE.txt new file mode 100644 index 0000000000..7c18a96714 --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/LICENSE.txt @@ -0,0 +1,13 @@ + Copyright 2018 Anders Rundgren + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. \ No newline at end of file diff --git a/packages/shared/common/__tests__/json/testdata/VENDOR.txt b/packages/shared/common/__tests__/json/testdata/VENDOR.txt new file mode 100644 index 0000000000..c6f4119eea --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/VENDOR.txt @@ -0,0 +1,2 @@ +Test data originally from: +https://github.com/cyberphone/json-canonicalization/tree/master/testdata \ No newline at end of file diff --git a/packages/shared/common/__tests__/json/testdata/input/arrays.json b/packages/shared/common/__tests__/json/testdata/input/arrays.json new file mode 100644 index 0000000000..20e62263c4 --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/input/arrays.json @@ -0,0 +1,8 @@ +[ + 56, + { + "d": true, + "10": null, + "1": [ ] + } +] diff --git a/packages/shared/common/__tests__/json/testdata/input/french.json b/packages/shared/common/__tests__/json/testdata/input/french.json new file mode 100644 index 0000000000..4ff6d3de24 --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/input/french.json @@ -0,0 +1,6 @@ +{ + "peach": "This sorting order", + "péché": "is wrong according to French", + "pêche": "but canonicalization MUST", + "sin": "ignore locale" +} diff --git a/packages/shared/common/__tests__/json/testdata/input/structures.json b/packages/shared/common/__tests__/json/testdata/input/structures.json new file mode 100644 index 0000000000..eb71efb84c --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/input/structures.json @@ -0,0 +1,8 @@ +{ + "1": {"f": {"f": "hi","F": 5} ,"\n": 56.0}, + "10": { }, + "": "empty", + "a": { }, + "111": [ {"e": "yes","E": "no" } ], + "A": { } +} \ No newline at end of file diff --git a/packages/shared/common/__tests__/json/testdata/input/unicode.json b/packages/shared/common/__tests__/json/testdata/input/unicode.json new file mode 100644 index 0000000000..4b5bc76996 --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/input/unicode.json @@ -0,0 +1,3 @@ +{ + "Unnormalized Unicode":"A\u030a" +} diff --git a/packages/shared/common/__tests__/json/testdata/input/values.json b/packages/shared/common/__tests__/json/testdata/input/values.json new file mode 100644 index 0000000000..f7712c2fbb --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/input/values.json @@ -0,0 +1,5 @@ +{ + "numbers": [333333333.33333329, 1E30, 4.50, 2e-3, 0.000000000000000000000000001], + "string": "\u20ac$\u000F\u000aA'\u0042\u0022\u005c\\\"\/", + "literals": [null, true, false] +} \ No newline at end of file diff --git a/packages/shared/common/__tests__/json/testdata/input/weird.json b/packages/shared/common/__tests__/json/testdata/input/weird.json new file mode 100644 index 0000000000..53fabe67bc --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/input/weird.json @@ -0,0 +1,11 @@ +{ + "\u20ac": "Euro Sign", + "\r": "Carriage Return", + "\u000a": "Newline", + "1": "One", + "\u0080": "Control\u007f", + "\ud83d\ude02": "Smiley", + "\u00f6": "Latin Small Letter O With Diaeresis", + "\ufb33": "Hebrew Letter Dalet With Dagesh", + "": "Browser Challenge" +} diff --git a/packages/shared/common/__tests__/json/testdata/output/arrays.json b/packages/shared/common/__tests__/json/testdata/output/arrays.json new file mode 100644 index 0000000000..5efb93db77 --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/output/arrays.json @@ -0,0 +1 @@ +[56,{"1":[],"10":null,"d":true}] \ No newline at end of file diff --git a/packages/shared/common/__tests__/json/testdata/output/french.json b/packages/shared/common/__tests__/json/testdata/output/french.json new file mode 100644 index 0000000000..2e15cd1b22 --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/output/french.json @@ -0,0 +1 @@ +{"peach":"This sorting order","péché":"is wrong according to French","pêche":"but canonicalization MUST","sin":"ignore locale"} \ No newline at end of file diff --git a/packages/shared/common/__tests__/json/testdata/output/structures.json b/packages/shared/common/__tests__/json/testdata/output/structures.json new file mode 100644 index 0000000000..dc21e2424e --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/output/structures.json @@ -0,0 +1 @@ +{"":"empty","1":{"\n":56,"f":{"F":5,"f":"hi"}},"10":{},"111":[{"E":"no","e":"yes"}],"A":{},"a":{}} \ No newline at end of file diff --git a/packages/shared/common/__tests__/json/testdata/output/unicode.json b/packages/shared/common/__tests__/json/testdata/output/unicode.json new file mode 100644 index 0000000000..ee60fd121b --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/output/unicode.json @@ -0,0 +1 @@ +{"Unnormalized Unicode":"Å"} \ No newline at end of file diff --git a/packages/shared/common/__tests__/json/testdata/output/values.json b/packages/shared/common/__tests__/json/testdata/output/values.json new file mode 100644 index 0000000000..29b720b6e7 --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/output/values.json @@ -0,0 +1 @@ +{"literals":[null,true,false],"numbers":[333333333.3333333,1e+30,4.5,0.002,1e-27],"string":"€$\u000f\nA'B\"\\\\\"/"} \ No newline at end of file diff --git a/packages/shared/common/__tests__/json/testdata/output/weird.json b/packages/shared/common/__tests__/json/testdata/output/weird.json new file mode 100644 index 0000000000..62c83a333e --- /dev/null +++ b/packages/shared/common/__tests__/json/testdata/output/weird.json @@ -0,0 +1 @@ +{"\n":"Newline","\r":"Carriage Return","1":"One","":"Browser Challenge","€":"Control","ö":"Latin Small Letter O With Diaeresis","€":"Euro Sign","😂":"Smiley","דּ":"Hebrew Letter Dalet With Dagesh"} \ No newline at end of file diff --git a/packages/shared/common/src/Context.ts b/packages/shared/common/src/Context.ts index dccc0ab135..8b013429a0 100644 --- a/packages/shared/common/src/Context.ts +++ b/packages/shared/common/src/Context.ts @@ -10,6 +10,7 @@ import type { } from './api'; import AttributeReference from './AttributeReference'; import { isLegacyUser, isMultiKind, isSingleKind } from './internal/context'; +import { canonicalize } from './json/canonicalize'; import { TypeValidators } from './validators'; // The general strategy for the context is to transform the passed in context @@ -472,65 +473,21 @@ export default class Context { return undefined; } - const hasher = crypto.createHash('sha256'); + try { + const canonicalized = canonicalize(this); - const stack: { - target: any; - visited: any[]; - }[] = []; + const hasher = crypto.createHash('sha256'); + hasher.update(canonicalized); - const kinds = this.kinds.sort(); - - kinds.forEach((kind) => { - hasher.update(kind); - const context = this._contextForKind(kind)!; - Object.getOwnPropertyNames(context) - .sort() - .forEach((key) => { - // Handled using private attributes. - if (key === '_meta') { - return; - } - hasher.update(key); - stack.push({ - target: context[key], - visited: [context], - }); - }); - - const sortedAttributes = this.privateAttributes(kind) - .map((attr) => attr.components.join('/')) - .sort(); - sortedAttributes.forEach((attr) => hasher.update(attr)); - }); - - while (stack.length > 0) { - const { target, visited } = stack.pop()!; - if (visited.includes(target)) { - return undefined; + if (hasher.digest) { + return hasher.digest('hex'); } - visited.push(target); - if (typeof target === 'object' && target !== null && target !== undefined) { - Object.getOwnPropertyNames(target) - .sort() - .forEach((key) => { - hasher.update(key); - stack.push({ - target: target[key], - visited: [...visited, target], - }); - }); - } else { - hasher.update(String(target)); - } - } - if (hasher.digest) { - return hasher.digest('hex'); + // The hasher must have either digest or asyncDigest. + const digest = await hasher.asyncDigest!('hex'); + return digest; + } catch { + return undefined; } - - // The hasher must have either digest or asyncDigest. - const digest = await hasher.asyncDigest!('hex'); - return digest; } } diff --git a/packages/shared/common/src/internal/events/EventProcessor.ts b/packages/shared/common/src/internal/events/EventProcessor.ts index 17a6608d23..808e085b15 100644 --- a/packages/shared/common/src/internal/events/EventProcessor.ts +++ b/packages/shared/common/src/internal/events/EventProcessor.ts @@ -157,6 +157,7 @@ export default class EventProcessor implements LDEventProcessor { this._summarizer = new MultiEventSummarizer( clientContext.platform.crypto, this._contextFilter, + this._logger, ); } else { this._summarizer = new EventSummarizer(); diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts index 8fa5c9b828..4454ff1611 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts @@ -1,4 +1,5 @@ import { setupAsyncCrypto, setupCrypto } from '../../../__tests__/setupCrypto'; +import { LDLogger } from '../../api'; import Context from '../../Context'; import ContextFilter from '../../ContextFilter'; import InputEvalEvent from './InputEvalEvent'; @@ -10,14 +11,21 @@ describe.each([ ['sync', setupCrypto()], ['async', setupAsyncCrypto()], ])('with mocked crypto and hasher/%s', (_name, crypto) => { + let logger: LDLogger; let summarizer: MultiEventSummarizer; beforeEach(() => { const contextFilter = new ContextFilter(false, []); - summarizer = new MultiEventSummarizer(crypto, contextFilter); + logger = { + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + }; + summarizer = new MultiEventSummarizer(crypto, contextFilter, logger); }); - test('creates new summarizer for new context hash', async () => { + it('creates new summarizer for new context hash', async () => { const context = Context.fromLDContext({ kind: 'user', key: 'user1' }); const event = new InputEvalEvent(true, context, 'flag-key', 'value', 'default', 1, 0, true); @@ -27,7 +35,7 @@ describe.each([ expect(summaries).toHaveLength(1); }); - test('uses existing summarizer for same context hash', async () => { + it('uses existing summarizer for same context hash', async () => { const context = Context.fromLDContext({ kind: 'user', key: 'user1' }); const event1 = new InputEvalEvent(true, context, 'flag-key', 'value1', 'default', 1, 0, true); const event2 = new InputEvalEvent(true, context, 'flag-key', 'value2', 'default', 1, 0, true); @@ -39,7 +47,7 @@ describe.each([ expect(summaries).toHaveLength(1); }); - test('ignores non-feature events', async () => { + it('ignores non-feature events', async () => { const context = Context.fromLDContext({ kind: 'user', key: 'user1' }); const event = new InputIdentifyEvent(context); @@ -49,7 +57,7 @@ describe.each([ expect(summaries).toHaveLength(0); }); - test('handles multiple different contexts', async () => { + it('handles multiple different contexts', async () => { const context1 = Context.fromLDContext({ kind: 'user', key: 'user1' }); const context2 = Context.fromLDContext({ kind: 'user', key: 'user2' }); const event1 = new InputEvalEvent(true, context1, 'flag-key', 'value1', 'default', 1, 0, true); @@ -62,7 +70,7 @@ describe.each([ expect(summaries).toHaveLength(2); }); - test('automatically clears summaries when summarized', async () => { + it('automatically clears summaries when summarized', async () => { const context = Context.fromLDContext({ kind: 'user', key: 'user1' }); const event = new InputEvalEvent(true, context, 'flag-key', 'value', 'default', 1, 0, true); @@ -73,4 +81,18 @@ describe.each([ expect(summariesA).toHaveLength(1); expect(summariesB).toHaveLength(0); }); + + it('logs error when context cannot be hashed', async () => { + const a: any = {}; + const b: any = {a}; + a.b = b; + + const context = Context.fromLDContext({ kind: 'user', key: 'user1', cyclic: a }); + const event = new InputEvalEvent(true, context, 'flag-key', 'value', 'default', 1, 0, true); + + summarizer.summarizeEvent(event); + await summarizer.getSummaries(); + expect(logger.error).toHaveBeenCalledTimes(1); + expect(logger.error).toHaveBeenCalledWith('Unable to hash context, likely the context contains a cycle.'); + }); }); diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts index 8981cf1723..d46d3684d0 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.ts @@ -1,4 +1,4 @@ -import { Crypto } from '../../api'; +import { Crypto, LDLogger } from '../../api'; import ContextFilter from '../../ContextFilter'; import EventSummarizer from './EventSummarizer'; import { isFeature } from './guards'; @@ -9,6 +9,7 @@ export default class MultiEventSummarizer implements LDMultiEventSummarizer { constructor( private readonly _crypto: Crypto, private readonly _contextFilter: ContextFilter, + private readonly _logger?: LDLogger, ) {} private _summarizers: Record = {}; private _pendingPromises: Promise[] = []; @@ -20,6 +21,11 @@ export default class MultiEventSummarizer implements LDMultiEventSummarizer { if (isFeature(event)) { const hash = await event.context.hash(this._crypto); if (!hash) { + if (event.context.valid) { + // The context appeared valid, but it could not be hashed. + // This is likely because of a cycle in the data. + this._logger?.error('Unable to hash context, likely the context contains a cycle.'); + } return; } // It is important that async operations do not happen between checking that the summarizer diff --git a/packages/shared/common/src/json/canonicalize.ts b/packages/shared/common/src/json/canonicalize.ts new file mode 100644 index 0000000000..88211ca1e8 --- /dev/null +++ b/packages/shared/common/src/json/canonicalize.ts @@ -0,0 +1,37 @@ +/** + * Given some object to serialize product a canonicalized JSON string. + * https://www.rfc-editor.org/rfc/rfc8785.html + * + * We do not support custom toJSON methods on objects. Objects should be limited to basic types. + * + * @param object The object to serialize. + */ +export function canonicalize(object: any, visited: any[] = []): string { + // For JavaScript the default JSON serialization will produce canonicalized output for basic types. + if (object === null || typeof object !== 'object') { + return JSON.stringify(object); + } + + if (visited.includes(object)) { + throw new Error('Cycle detected'); + } + + if (Array.isArray(object)) { + const values = object + .map((item) => canonicalize(item, [...visited, object])) + .map((item) => (item === undefined ? 'null' : item)); + return `[${values.join(',')}]`; + } + + const values = Object.keys(object) + .sort() + .map((key) => { + const value = canonicalize(object[key], [...visited, object]); + if (value !== undefined) { + return `${JSON.stringify(key)}:${value}`; + } + return undefined; + }) + .filter((item) => item !== undefined); + return `{${values.join(',')}}`; +} From 233a73133cebe852b0941628ebcde7dc698ec225 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 15 May 2025 16:08:04 -0700 Subject: [PATCH 26/26] Lint and package size increase. --- .github/workflows/browser.yml | 2 +- .../src/internal/events/MultiEventSummarizer.test.ts | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/browser.yml b/.github/workflows/browser.yml index 1442c4a567..cead8fc216 100644 --- a/.github/workflows/browser.yml +++ b/.github/workflows/browser.yml @@ -41,4 +41,4 @@ jobs: target_file: 'packages/sdk/browser/dist/index.js' package_name: '@launchdarkly/js-client-sdk' pr_number: ${{ github.event.number }} - size_limit: 21000 + size_limit: 25000 diff --git a/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts index a2c826c99d..9219242b0f 100644 --- a/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts +++ b/packages/shared/common/src/internal/events/MultiEventSummarizer.test.ts @@ -35,7 +35,7 @@ describe.each([ expect(summaries).toHaveLength(1); }); -it('uses existing summarizer for same context hash', async () => { + it('uses existing summarizer for same context hash', async () => { const context = Context.fromLDContext({ kind: 'user', key: 'user1' }); const event1 = new InputEvalEvent(true, context, 'flag-key', 'value1', 'default', 1, 0, true); const event2 = new InputEvalEvent(true, context, 'flag-key', 'value2', 'default', 1, 0, true); @@ -84,7 +84,7 @@ it('uses existing summarizer for same context hash', async () => { it('logs error when context cannot be hashed', async () => { const a: any = {}; - const b: any = {a}; + const b: any = { a }; a.b = b; const context = Context.fromLDContext({ kind: 'user', key: 'user1', cyclic: a }); @@ -93,6 +93,8 @@ it('uses existing summarizer for same context hash', async () => { summarizer.summarizeEvent(event); await summarizer.getSummaries(); expect(logger.error).toHaveBeenCalledTimes(1); - expect(logger.error).toHaveBeenCalledWith('Unable to hash context, likely the context contains a cycle.'); + expect(logger.error).toHaveBeenCalledWith( + 'Unable to hash context, likely the context contains a cycle.', + ); }); });