diff --git a/src/EventProcessor.js b/src/EventProcessor.js index 8b6489e..e6a0fac 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -1,5 +1,5 @@ const EventSender = require('./EventSender'); -const EventSummarizer = require('./EventSummarizer'); +const MultiEventSummarizer = require('./MultiEventSummarizer'); const ContextFilter = require('./ContextFilter'); const errors = require('./errors'); const messages = require('./messages'); @@ -17,8 +17,8 @@ function EventProcessor( const processor = {}; const eventSender = sender || EventSender(platform, environmentId, options); const mainEventsUrl = utils.appendUrlPath(options.eventsUrl, '/events/bulk/' + environmentId); - const summarizer = EventSummarizer(); const contextFilter = ContextFilter(options); + const summarizer = MultiEventSummarizer(contextFilter); const samplingInterval = options.samplingInterval; const eventCapacity = options.eventCapacity; const flushInterval = options.flushInterval; @@ -117,17 +117,19 @@ function EventProcessor( } }; - processor.flush = function() { + processor.flush = async function() { if (disabled) { return Promise.resolve(); } const eventsToSend = queue; - const summary = summarizer.getSummary(); - summarizer.clearSummary(); - if (summary) { - summary.kind = 'summary'; - eventsToSend.push(summary); - } + const summaries = summarizer.getSummaries(); + + summaries.forEach(summary => { + if (Object.keys(summary.features).length) { + eventsToSend.push(summary); + } + }); + if (diagnosticsAccumulator) { // For diagnostic events, we record how many events were in the queue at the last flush (since "how // many events happened to be in the queue at the moment we decided to send a diagnostic event" would diff --git a/src/EventSummarizer.js b/src/EventSummarizer.js index 4c1033c..7d15e14 100644 --- a/src/EventSummarizer.js +++ b/src/EventSummarizer.js @@ -89,6 +89,7 @@ function EventSummarizer() { startDate, endDate, features: flagsOut, + kind: 'summary', }; }; diff --git a/src/MultiEventSummarizer.js b/src/MultiEventSummarizer.js new file mode 100644 index 0000000..f2818a9 --- /dev/null +++ b/src/MultiEventSummarizer.js @@ -0,0 +1,60 @@ +const canonicalize = require('./canonicalize'); +const EventSummarizer = require('./EventSummarizer'); + +/** + * Construct a multi-event summarizer. This summarizer produces a summary event for each unique context. + * @param {{filter: (context: any) => any}} contextFilter + */ +function MultiEventSummarizer(contextFilter) { + let summarizers = {}; + let contexts = {}; + + /** + * Summarize the given event. + * @param {{ + * kind: string, + * context?: any, + * }} event + */ + function summarizeEvent(event) { + if (event.kind === 'feature') { + const key = canonicalize(event.context); + if (!key) { + return; + } + + let summarizer = summarizers[key]; + if (!summarizer) { + summarizers[key] = EventSummarizer(); + summarizer = summarizers[key]; + contexts[key] = event.context; + } + + summarizer.summarizeEvent(event); + } + } + + /** + * Get the summaries of the events that have been summarized. + * @returns {any[]} + */ + function getSummaries() { + const summarizersToFlush = summarizers; + const contextsForSummaries = contexts; + + summarizers = {}; + contexts = {}; + return Object.entries(summarizersToFlush).map(([key, summarizer]) => { + const summary = summarizer.getSummary(); + summary.context = contextFilter.filter(contextsForSummaries[key]); + return summary; + }); + } + + return { + summarizeEvent, + getSummaries, + }; +} + +module.exports = MultiEventSummarizer; diff --git a/src/__tests__/EventProcessor-test.js b/src/__tests__/EventProcessor-test.js index 5eb994d..f1082e9 100644 --- a/src/__tests__/EventProcessor-test.js +++ b/src/__tests__/EventProcessor-test.js @@ -485,6 +485,73 @@ describe.each([ }); }); + it('generates separate summary events for different contexts', async () => { + await withProcessorAndSender(defaultConfig, async (ep, mockEventSender) => { + const context1 = { key: 'user1', kind: 'user' }; + const context2 = { key: 'user2', kind: 'user' }; + + // Create feature events for two different contexts + const event1 = { + kind: 'feature', + creationDate: 1000, + context: context1, + key: 'flag1', + version: 11, + variation: 1, + value: 'value1', + default: 'default1', + trackEvents: false, + }; + + const event2 = { + kind: 'feature', + creationDate: 1000, + context: context2, + key: 'flag2', + version: 22, + variation: 2, + value: 'value2', + default: 'default2', + trackEvents: false, + }; + + ep.enqueue(event1); + ep.enqueue(event2); + await ep.flush(); + + expect(mockEventSender.calls.length()).toEqual(1); + const output = (await mockEventSender.calls.take()).events; + + // Should have two summary events, one for each context + expect(output.length).toEqual(2); + + // Find the summary event for each context + const summary1 = output.find(e => e.context.key === 'user1'); + const summary2 = output.find(e => e.context.key === 'user2'); + + // Verify each summary event has the correct context and flag data + expect(summary1).toBeDefined(); + expect(summary1.context).toEqual(context1); + expect(summary1.features).toEqual({ + flag1: { + contextKinds: ['user'], + default: 'default1', + counters: [{ version: 11, variation: 1, value: 'value1', count: 1 }], + }, + }); + + expect(summary2).toBeDefined(); + expect(summary2.context).toEqual(context2); + expect(summary2.features).toEqual({ + flag2: { + contextKinds: ['user'], + default: 'default2', + counters: [{ version: 22, variation: 2, value: 'value2', count: 1 }], + }, + }); + }); + }); + describe('interaction with diagnostic events', () => { it('sets eventsInLastBatch on flush', async () => { const e0 = { kind: 'custom', creationDate: 1000, context: eventContext, key: 'key0' }; @@ -525,5 +592,50 @@ describe.each([ expect(diagnosticAccumulator.getProps().droppedEvents).toEqual(2); }); }); + + it('filters context in summary events', async () => { + const event = { + kind: 'feature', + creationDate: 1000, + context: eventContext, + key: 'flagkey', + version: 11, + variation: 1, + value: 'value', + default: 'default', + trackEvents: true, + }; + + // Configure with allAttributesPrivate set to true + const config = { ...defaultConfig, allAttributesPrivate: true }; + + const sender = MockEventSender(); + const ep = EventProcessor(platform, config, envId, null, null, sender); + try { + ep.enqueue(event); + await ep.flush(); + + expect(sender.calls.length()).toEqual(1); + const output = (await sender.calls.take()).events; + expect(output.length).toEqual(2); + + // Verify the feature event has filtered context + checkFeatureEvent(output[0], event, false, filteredContext); + + // Verify the summary event has filtered context + const summaryEvent = output[1]; + checkSummaryEvent(summaryEvent); + expect(summaryEvent.context).toEqual(filteredContext); + expect(summaryEvent.features).toEqual({ + flagkey: { + contextKinds: ['user'], + default: 'default', + counters: [{ version: 11, variation: 1, value: 'value', count: 1 }], + }, + }); + } finally { + ep.stop(); + } + }); }); }); diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index 55773e6..e3f74fb 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -672,7 +672,9 @@ describe('LDClient', () => { await client.waitForInitialization(5); }); - expect(eventsServer.requests.length()).toEqual(1); + // Flushing is an async operation, so we cannot ensure that the requests are made by + // the time we reach this point. If we await the nextRequest(), then it will catch + // whatever was flushed. const req = await eventsServer.nextRequest(); const data = JSON.parse(req.body); expect(data.length).toEqual(1); diff --git a/src/__tests__/MultiEventSummarizer-test.js b/src/__tests__/MultiEventSummarizer-test.js new file mode 100644 index 0000000..e52e840 --- /dev/null +++ b/src/__tests__/MultiEventSummarizer-test.js @@ -0,0 +1,148 @@ +const MultiEventSummarizer = require('../MultiEventSummarizer'); +const ContextFilter = require('../ContextFilter'); + +function makeEvent(key, version, variation, value, defaultVal, context) { + return { + kind: 'feature', + creationDate: 1000, + key: key, + version: version, + context: context, + variation: variation, + value: value, + default: defaultVal, + }; +} + +describe('given a multi-event summarizer and context filter', () => { + let summarizer; + let contextFilter; + + beforeEach(() => { + contextFilter = ContextFilter(false, []); + summarizer = MultiEventSummarizer(contextFilter); + }); + + it('creates new summarizer for new context hash', async () => { + const context = { kind: 'user', key: 'user1' }; + const event = { kind: 'feature', context }; + + summarizer.summarizeEvent(event); + + const summaries = await summarizer.getSummaries(); + expect(summaries).toHaveLength(1); + }); + + it('uses existing summarizer for same context hash', async () => { + const context = { kind: 'user', key: 'user1' }; + const event1 = { kind: 'feature', context, value: 'value1' }; + const event2 = { kind: 'feature', context, value: 'value2' }; + + summarizer.summarizeEvent(event1); + summarizer.summarizeEvent(event2); + + const summaries = await summarizer.getSummaries(); + expect(summaries).toHaveLength(1); + }); + + it('ignores non-feature events', async () => { + const context = { kind: 'user', key: 'user1' }; + const event = { kind: 'identify', context }; + + summarizer.summarizeEvent(event); + + const summaries = await summarizer.getSummaries(); + expect(summaries).toHaveLength(0); + }); + + it('handles multiple different contexts', async () => { + const context1 = { kind: 'user', key: 'user1' }; + const context2 = { kind: 'user', key: 'user2' }; + const event1 = { kind: 'feature', context: context1 }; + const event2 = { kind: 'feature', context: context2 }; + + summarizer.summarizeEvent(event1); + summarizer.summarizeEvent(event2); + + const summaries = await summarizer.getSummaries(); + expect(summaries).toHaveLength(2); + }); + + it('automatically clears summaries when summarized', async () => { + const context = { kind: 'user', key: 'user1' }; + const event = { kind: 'feature', context }; + + summarizer.summarizeEvent(event); + + const summariesA = await summarizer.getSummaries(); + const summariesB = await summarizer.getSummaries(); + expect(summariesA).toHaveLength(1); + expect(summariesB).toHaveLength(0); + }); + + it('increments counters for feature events across multiple contexts', async () => { + const context1 = { kind: 'user', key: 'user1' }; + const context2 = { kind: 'user', key: 'user2' }; + + // Events for context1 (using values 100-199) + const event1 = makeEvent('key1', 11, 1, 100, 111, context1); + const event2 = makeEvent('key1', 11, 2, 150, 111, context1); + const event3 = makeEvent('key2', 22, 1, 199, 222, context1); + + // Events for context2 (using values 200-299) + const event4 = makeEvent('key1', 11, 1, 200, 211, context2); + const event5 = makeEvent('key1', 11, 2, 250, 211, context2); + const event6 = makeEvent('key2', 22, 1, 299, 222, context2); + + summarizer.summarizeEvent(event1); + summarizer.summarizeEvent(event2); + summarizer.summarizeEvent(event3); + summarizer.summarizeEvent(event4); + summarizer.summarizeEvent(event5); + summarizer.summarizeEvent(event6); + + const summaries = await summarizer.getSummaries(); + expect(summaries).toHaveLength(2); + + // Sort summaries by context key to make assertions consistent + summaries.sort((a, b) => a.context.key.localeCompare(b.context.key)); + + // Verify first context's summary (user1, values 100-199) + const summary1 = summaries[0]; + summary1.features.key1.counters.sort((a, b) => a.value - b.value); + expect(summary1.features).toEqual({ + key1: { + contextKinds: ['user'], + default: 111, + counters: [ + { value: 100, variation: 1, version: 11, count: 1 }, + { value: 150, variation: 2, version: 11, count: 1 }, + ], + }, + key2: { + contextKinds: ['user'], + default: 222, + counters: [{ value: 199, variation: 1, version: 22, count: 1 }], + }, + }); + + // Verify second context's summary (user2, values 200-299) + const summary2 = summaries[1]; + summary2.features.key1.counters.sort((a, b) => a.value - b.value); + expect(summary2.features).toEqual({ + key1: { + contextKinds: ['user'], + default: 211, + counters: [ + { value: 200, variation: 1, version: 11, count: 1 }, + { value: 250, variation: 2, version: 11, count: 1 }, + ], + }, + key2: { + contextKinds: ['user'], + default: 222, + counters: [{ value: 299, variation: 1, version: 22, count: 1 }], + }, + }); + }); +}); diff --git a/src/__tests__/canonicalize-test.js b/src/__tests__/canonicalize-test.js new file mode 100644 index 0000000..1b340f7 --- /dev/null +++ b/src/__tests__/canonicalize-test.js @@ -0,0 +1,86 @@ +const fs = require('fs'); +const path = require('path'); + +const canonicalize = require('../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 = []; + const expected = '[]'; + const result = canonicalize(input); + expect(result).toEqual(expected); +}); + +it('handles arrays of null/undefined', () => { + const input = [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 = {}; + const b = { a }; + a.b = b; + + expect(() => canonicalize(a)).toThrow('Cycle detected'); +}); diff --git a/src/__tests__/context-test.js b/src/__tests__/context-test.js index ca24c4a..6c7f726 100644 --- a/src/__tests__/context-test.js +++ b/src/__tests__/context-test.js @@ -147,7 +147,7 @@ describe('getContextKeys', () => { expect(keys).toEqual({ user: 'test-user-key' }); }); - it.only('ignores empty string and null keys from multi context', () => { + it('ignores empty string and null keys from multi context', () => { const context = { kind: 'multi', user: { diff --git a/src/__tests__/testdata/LICENSE.txt b/src/__tests__/testdata/LICENSE.txt new file mode 100644 index 0000000..7c18a96 --- /dev/null +++ b/src/__tests__/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/src/__tests__/testdata/VENDOR.txt b/src/__tests__/testdata/VENDOR.txt new file mode 100644 index 0000000..c6f4119 --- /dev/null +++ b/src/__tests__/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/src/__tests__/testdata/input/arrays.json b/src/__tests__/testdata/input/arrays.json new file mode 100644 index 0000000..20e6226 --- /dev/null +++ b/src/__tests__/testdata/input/arrays.json @@ -0,0 +1,8 @@ +[ + 56, + { + "d": true, + "10": null, + "1": [ ] + } +] diff --git a/src/__tests__/testdata/input/french.json b/src/__tests__/testdata/input/french.json new file mode 100644 index 0000000..4ff6d3d --- /dev/null +++ b/src/__tests__/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/src/__tests__/testdata/input/structures.json b/src/__tests__/testdata/input/structures.json new file mode 100644 index 0000000..eb71efb --- /dev/null +++ b/src/__tests__/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/src/__tests__/testdata/input/unicode.json b/src/__tests__/testdata/input/unicode.json new file mode 100644 index 0000000..4b5bc76 --- /dev/null +++ b/src/__tests__/testdata/input/unicode.json @@ -0,0 +1,3 @@ +{ + "Unnormalized Unicode":"A\u030a" +} diff --git a/src/__tests__/testdata/input/values.json b/src/__tests__/testdata/input/values.json new file mode 100644 index 0000000..f7712c2 --- /dev/null +++ b/src/__tests__/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/src/__tests__/testdata/input/weird.json b/src/__tests__/testdata/input/weird.json new file mode 100644 index 0000000..53fabe6 --- /dev/null +++ b/src/__tests__/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/src/__tests__/testdata/output/arrays.json b/src/__tests__/testdata/output/arrays.json new file mode 100644 index 0000000..5efb93d --- /dev/null +++ b/src/__tests__/testdata/output/arrays.json @@ -0,0 +1 @@ +[56,{"1":[],"10":null,"d":true}] \ No newline at end of file diff --git a/src/__tests__/testdata/output/french.json b/src/__tests__/testdata/output/french.json new file mode 100644 index 0000000..2e15cd1 --- /dev/null +++ b/src/__tests__/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/src/__tests__/testdata/output/structures.json b/src/__tests__/testdata/output/structures.json new file mode 100644 index 0000000..dc21e24 --- /dev/null +++ b/src/__tests__/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/src/__tests__/testdata/output/unicode.json b/src/__tests__/testdata/output/unicode.json new file mode 100644 index 0000000..ee60fd1 --- /dev/null +++ b/src/__tests__/testdata/output/unicode.json @@ -0,0 +1 @@ +{"Unnormalized Unicode":"Å"} \ No newline at end of file diff --git a/src/__tests__/testdata/output/values.json b/src/__tests__/testdata/output/values.json new file mode 100644 index 0000000..29b720b --- /dev/null +++ b/src/__tests__/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/src/__tests__/testdata/output/weird.json b/src/__tests__/testdata/output/weird.json new file mode 100644 index 0000000..62c83a3 --- /dev/null +++ b/src/__tests__/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/src/canonicalize.js b/src/canonicalize.js new file mode 100644 index 0000000..b987837 --- /dev/null +++ b/src/canonicalize.js @@ -0,0 +1,41 @@ +/** + * 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 {any} object The object to serialize. + * @param {any[]?} visited The list of objects that have already been visited to avoid cycles. + * @returns {string} The canonicalized JSON string. + */ +function canonicalize(object, visited = []) { + // 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(',')}}`; +} + +module.exports = canonicalize; diff --git a/src/context.js b/src/context.js index 643cbd1..40ed20c 100644 --- a/src/context.js +++ b/src/context.js @@ -1,10 +1,10 @@ +const { commonBasicLogger } = require('./loggers'); + /** * Validate a context kind. * @param {string} kind * @returns true if the kind is valid. */ -const { commonBasicLogger } = require('./loggers'); - function validKind(kind) { return typeof kind === 'string' && kind !== 'kind' && kind.match(/^(\w|\.|-)+$/); } @@ -44,7 +44,7 @@ function checkContext(context, allowLegacyKey) { /** * For a given context get a list of context kinds. * @param {Object} context - * @returns A list of kinds in the context. + * @returns {string[]} A list of kinds in the context. */ function getContextKinds(context) { if (context) {