From b5f21bc316664c48521a4fbb134583b59b4f2867 Mon Sep 17 00:00:00 2001 From: Alexis Georges Date: Mon, 26 Mar 2018 08:35:37 -0400 Subject: [PATCH 01/59] fix promise/callback utility: avoid rejecting and returning the promise if the caller is using the callback interface --- src/__tests__/utils-test.js | 32 +++++++++++--------------------- src/utils.js | 9 ++++++--- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/__tests__/utils-test.js b/src/__tests__/utils-test.js index f5e6f174..7ef1a741 100644 --- a/src/__tests__/utils-test.js +++ b/src/__tests__/utils-test.js @@ -21,31 +21,21 @@ describe('utils', function() { }); it('should call the callback with a value if the promise resolves', function(done) { - const callback = sinon.spy(); - const promise = wrapPromiseCallback(Promise.resolve('woohoo'), callback); - - promise.then(function(result) { - expect(result).to.equal('woohoo'); - // callback run on next tick to maintain asynchronous expections - setTimeout(function() { - expect(callback.calledWith(null, 'woohoo')).to.be.true; - done(); - }, 0); + const promise = wrapPromiseCallback(Promise.resolve('woohoo'), function(error, value) { + expect(promise).to.be.undefined; + expect(error).to.be.null; + expect(value).to.equal('woohoo'); + done(); }); }); it('should call the callback with an error if the promise rejects', function(done) { - const error = new Error('something went wrong'); - const callback = sinon.spy(); - const promise = wrapPromiseCallback(Promise.reject(error), callback); - - promise.catch(function(v) { - expect(v).to.equal(error); - // callback run on next tick to maintain asynchronous expections - setTimeout(function() { - expect(callback.calledWith(error, null)).to.be.true; - done(); - }, 0); + const actualError = new Error('something went wrong'); + const promise = wrapPromiseCallback(Promise.reject(actualError), function(error, value) { + expect(promise).to.be.undefined; + expect(error).to.equal(actualError); + expect(value).to.be.null; + done(); }); }); }); diff --git a/src/utils.js b/src/utils.js index 9be2c3f4..163dab49 100644 --- a/src/utils.js +++ b/src/utils.js @@ -72,10 +72,10 @@ function merge(target, varArgs) { // .length of function is 2 * * @param {Promise} promise * @param {Function} callback - * @returns Promise + * @returns Promise | undefined */ function wrapPromiseCallback(promise, callback) { - return promise.then( + var ret = promise.then( function(value) { if (callback) { setTimeout(function() { callback(null, value); }, 0); @@ -85,10 +85,13 @@ function wrapPromiseCallback(promise, callback) { function(error) { if (callback) { setTimeout(function() { callback(error, null); }, 0); + } else { + return Promise.reject(error); } - return Promise.reject(error); } ); + + return !callback ? ret : undefined; } module.exports = { From c3dcdbbeee4f898082a7ea51572d33fa23837298 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 11 Apr 2018 13:14:28 -0700 Subject: [PATCH 02/59] change EventSerializer to UserFilter --- src/EventProcessor.js | 10 ++- src/EventSerializer.js | 85 ------------------- src/UserFilter.js | 55 ++++++++++++ src/__tests__/EventProcessor-test.js | 8 +- src/__tests__/EventSerializer-test.js | 117 -------------------------- src/__tests__/UserFilter-test.js | 97 +++++++++++++++++++++ src/index.js | 3 +- 7 files changed, 164 insertions(+), 211 deletions(-) delete mode 100644 src/EventSerializer.js create mode 100644 src/UserFilter.js delete mode 100644 src/__tests__/EventSerializer-test.js create mode 100644 src/__tests__/UserFilter-test.js diff --git a/src/EventProcessor.js b/src/EventProcessor.js index 137678fb..d1ade545 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -1,3 +1,4 @@ +import UserFilter from './UserFilter'; import * as utils from './utils'; const MAX_URL_LENGTH = 2000; @@ -38,18 +39,23 @@ function sendEvents(eventsUrl, events, sync) { } } -export default function EventProcessor(eventsUrl, eventSerializer) { +export default function EventProcessor(options, eventsUrl) { const processor = {}; + const userFilter = UserFilter(options); let queue = []; let initialFlush = true; + function serializeEvents(events) { + return events.map(e => (e.user ? Object.assign({}, e, { user: userFilter.filterUser(e.user) }) : e)); + } + processor.enqueue = function(event) { queue.push(event); }; processor.flush = function(user, sync) { const finalSync = sync === undefined ? false : sync; - const serializedQueue = eventSerializer.serializeEvents(queue); + const serializedQueue = serializeEvents(queue); if (!user) { if (initialFlush) { diff --git a/src/EventSerializer.js b/src/EventSerializer.js deleted file mode 100644 index c4d48c48..00000000 --- a/src/EventSerializer.js +++ /dev/null @@ -1,85 +0,0 @@ -/** - * The EventSerializer object transforms the internal representation of events into objects suitable to be sent - * as JSON to the server. This includes hiding any private user attributes. - * - * @param {Object} the LaunchDarkly client configuration object - **/ -export default function EventSerializer(config) { - const serializer = {}; - const allAttributesPrivate = config.all_attributes_private; - const privateAttributeNames = config.private_attribute_names || []; - const ignoreAttrs = { key: true, custom: true, anonymous: true }; - const allowedTopLevelAttrs = { - key: true, - secondary: true, - ip: true, - country: true, - email: true, - firstName: true, - lastName: true, - avatar: true, - name: true, - anonymous: true, - custom: true, - }; - - function serializeEvent(event) { - return Object.keys(event) - .map(key => [key, key === 'user' ? filterUser(event[key]) : event[key]]) - .reduce( - (acc, p) => - Object.assign({}, acc, { - [p[0]]: p[1], - }), - {} - ); - } - - function filterUser(user) { - const userPrivateAttrs = user.privateAttributeNames || []; - - const isPrivateAttr = name => - !ignoreAttrs[name] && - (allAttributesPrivate || userPrivateAttrs.indexOf(name) !== -1 || privateAttributeNames.indexOf(name) !== -1); - - const filterAttrs = (props, isAttributeAllowed) => - Object.keys(props).reduce( - (acc, name) => { - const nextAcc = [...acc]; - - if (isAttributeAllowed(name)) { - if (isPrivateAttr(name)) { - // add to hidden list - nextAcc[1][name] = true; - } else { - nextAcc[0][name] = props[name]; - } - } - return acc; - }, - [{}, {}] - ); - - const result = filterAttrs(user, key => allowedTopLevelAttrs[key]); - const filteredProps = result[0]; - const removedAttrs = result[1]; - - if (user.custom) { - const customResult = filterAttrs(user.custom, () => true); - filteredProps.custom = customResult[0]; - Object.assign(removedAttrs, customResult[1]); - } - - const removedAttrNames = Object.keys(removedAttrs); - - if (removedAttrNames.length) { - removedAttrNames.sort(); - filteredProps.privateAttrs = removedAttrNames; - } - return filteredProps; - } - - serializer.serializeEvents = events => events.map(serializeEvent); - - return serializer; -} diff --git a/src/UserFilter.js b/src/UserFilter.js new file mode 100644 index 00000000..c235339a --- /dev/null +++ b/src/UserFilter.js @@ -0,0 +1,55 @@ +/** + * The UserFilter object transforms user objects into objects suitable to be sent as JSON to + * the server, hiding any private user attributes. + * + * @param {Object} the LaunchDarkly client configuration object + **/ +export default function UserFilter(config) { + const filter = {}; + const allAttributesPrivate = config.all_attributes_private; + const privateAttributeNames = config.private_attribute_names || []; + const ignoreAttrs = { key: true, custom: true, anonymous: true }; + const allowedTopLevelAttrs = { + key: true, secondary: true, ip: true, country: true, email: true, + firstName: true, lastName: true, avatar: true, name: true, anonymous: true, custom: true + }; + + filter.filterUser = function (user) { + let allPrivateAttrs = {}; + let userPrivateAttrs = user.privateAttributeNames || []; + + const isPrivateAttr = function (name) { + return !ignoreAttrs[name] && ( + allAttributesPrivate || userPrivateAttrs.indexOf(name) !== -1 || + privateAttributeNames.indexOf(name) !== -1); + } + const filterAttrs = function (props, isAttributeAllowed) { + return Object.keys(props).reduce(function (acc, name) { + if (isAttributeAllowed(name)) { + if (isPrivateAttr(name)) { + // add to hidden list + acc[1][name] = true; + } else { + acc[0][name] = props[name]; + } + } + return acc; + }, [{}, {}]); + } + let result = filterAttrs(user, function (key) { return allowedTopLevelAttrs[key]; }); + let filteredProps = result[0]; + let removedAttrs = result[1]; + if (user.custom) { + var customResult = filterAttrs(user.custom, function (key) { return true; }); + filteredProps.custom = customResult[0]; + Object.assign(removedAttrs, customResult[1]); + } + var removedAttrNames = Object.keys(removedAttrs); + if (removedAttrNames.length) { + removedAttrNames.sort(); + filteredProps.privateAttrs = removedAttrNames; + } + return filteredProps; + } + return filter; +} \ No newline at end of file diff --git a/src/__tests__/EventProcessor-test.js b/src/__tests__/EventProcessor-test.js index 9440cbc7..beab229e 100644 --- a/src/__tests__/EventProcessor-test.js +++ b/src/__tests__/EventProcessor-test.js @@ -1,6 +1,5 @@ import sinon from 'sinon'; -import EventSerializer from '../EventSerializer'; import EventProcessor from '../EventProcessor'; describe('EventProcessor', () => { @@ -8,7 +7,6 @@ describe('EventProcessor', () => { let xhr; let requests = []; let warnSpy; - const serializer = EventSerializer({}); beforeEach(() => { sandbox = sinon.sandbox.create(); @@ -28,14 +26,14 @@ describe('EventProcessor', () => { it('should warn about missing user on initial flush', () => { const warnSpy = sandbox.spy(console, 'warn'); - const processor = EventProcessor('/fake-url', serializer); + const processor = EventProcessor({}, '/fake-url'); processor.flush(null); warnSpy.restore(); expect(warnSpy.called).toEqual(true); }); it('should flush asynchronously', () => { - const processor = EventProcessor('/fake-url', serializer); + const processor = EventProcessor({}, '/fake-url'); const user = { key: 'foo' }; const event = { kind: 'identify', key: user.key }; @@ -51,7 +49,7 @@ describe('EventProcessor', () => { }); it('should flush synchronously', () => { - const processor = EventProcessor('/fake-url', serializer); + const processor = EventProcessor({}, '/fake-url'); const user = { key: 'foo' }; const event = { kind: 'identify', key: user.key }; diff --git a/src/__tests__/EventSerializer-test.js b/src/__tests__/EventSerializer-test.js deleted file mode 100644 index f662b4c2..00000000 --- a/src/__tests__/EventSerializer-test.js +++ /dev/null @@ -1,117 +0,0 @@ -import EventSerializer from '../EventSerializer.js'; - -describe('EventSerializer', () => { - // users to serialize - const user = { - key: 'abc', - firstName: 'Sue', - custom: { bizzle: 'def', dizzle: 'ghi' }, - }; - - const userSpecifyingOwnPrivateAttr = { - key: 'abc', - firstName: 'Sue', - custom: { bizzle: 'def', dizzle: 'ghi' }, - privateAttributeNames: ['dizzle', 'unused'], - }; - - const userWithUnknownTopLevelAttrs = { - key: 'abc', - firstName: 'Sue', - species: 'human', - hatSize: 6, - custom: { bizzle: 'def', dizzle: 'ghi' }, - }; - - const anonUser = { - key: 'abc', - anonymous: true, - custom: { bizzle: 'def', dizzle: 'ghi' }, - }; - - // expected results from serializing user - const userWithAllAttrsHidden = { - key: 'abc', - custom: {}, - privateAttrs: ['bizzle', 'dizzle', 'firstName'], - }; - - const userWithSomeAttrsHidden = { - key: 'abc', - custom: { - dizzle: 'ghi', - }, - privateAttrs: ['bizzle', 'firstName'], - }; - - const userWithOwnSpecifiedAttrHidden = { - key: 'abc', - firstName: 'Sue', - custom: { - bizzle: 'def', - }, - privateAttrs: ['dizzle'], - }; - - const anonUserWithAllAttrsHidden = { - key: 'abc', - anonymous: true, - custom: {}, - privateAttrs: ['bizzle', 'dizzle'], - }; - - function makeEvent(user) { - return { - creationDate: 1000000, - key: 'xyz', - kind: 'thing', - user: user, - }; - } - - it('includes all user attributes by default', () => { - const es = EventSerializer({}); - const event = makeEvent(user); - expect(es.serializeEvents([event])).toEqual([event]); - }); - - it('hides all except key if all_attrs_private is true', () => { - // eslint-disable-next-line camelcase - const es = EventSerializer({ all_attributes_private: true }); - const event = makeEvent(user); - expect(es.serializeEvents([event])).toEqual([makeEvent(userWithAllAttrsHidden)]); - }); - - it('hides some attributes if private_attr_names is set', () => { - // eslint-disable-next-line camelcase - const es = EventSerializer({ private_attribute_names: ['firstName', 'bizzle'] }); - const event = makeEvent(user); - expect(es.serializeEvents([event])).toEqual([makeEvent(userWithSomeAttrsHidden)]); - }); - - it('hides attributes specified in per-user privateAttrs', () => { - const es = EventSerializer({}); - const event = makeEvent(userSpecifyingOwnPrivateAttr); - expect(es.serializeEvents([event])).toEqual([makeEvent(userWithOwnSpecifiedAttrHidden)]); - }); - - it('looks at both per-user privateAttrs and global config', () => { - // eslint-disable-next-line camelcase - const es = EventSerializer({ private_attribute_names: ['firstName', 'bizzle'] }); - const event = makeEvent(userSpecifyingOwnPrivateAttr); - expect(es.serializeEvents([event])).toEqual([makeEvent(userWithAllAttrsHidden)]); - }); - - it('strips unknown top-level attributes', () => { - const es = EventSerializer({}); - const event = makeEvent(userWithUnknownTopLevelAttrs); - expect(es.serializeEvents([event])).toEqual([makeEvent(user)]); - }); - - it('leaves the "anonymous" attribute as is', () => { - // eslint-disable-next-line camelcase - const es = EventSerializer({ all_attributes_private: true }); - const event = makeEvent(anonUser); - expect(es.serializeEvents([event])).toEqual([makeEvent(anonUserWithAllAttrsHidden)]); - }); -}); diff --git a/src/__tests__/UserFilter-test.js b/src/__tests__/UserFilter-test.js new file mode 100644 index 00000000..7545c58c --- /dev/null +++ b/src/__tests__/UserFilter-test.js @@ -0,0 +1,97 @@ +import UserFilter from '../UserFilter'; + +describe('UserFilter', () => { + // users to serialize + const user = { + 'key': 'abc', + 'firstName': 'Sue', + 'custom': { 'bizzle': 'def', 'dizzle': 'ghi' } + }; + + const userSpecifyingOwnPrivateAttr = { + 'key': 'abc', + 'firstName': 'Sue', + 'custom': { 'bizzle': 'def', 'dizzle': 'ghi' }, + 'privateAttributeNames': [ 'dizzle', 'unused' ] + }; + + var userWithUnknownTopLevelAttrs = { + 'key': 'abc', + 'firstName': 'Sue', + 'species': 'human', + 'hatSize': 6, + 'custom': { 'bizzle': 'def', 'dizzle': 'ghi' } + }; + + const anonUser = { + 'key': 'abc', + 'anonymous': true, + 'custom': { 'bizzle': 'def', 'dizzle': 'ghi' } + }; + + // expected results from serializing user + const userWithAllAttrsHidden = { + 'key': 'abc', + 'custom': { }, + 'privateAttrs': [ 'bizzle', 'dizzle', 'firstName' ] + }; + + const userWithSomeAttrsHidden = { + 'key': 'abc', + 'custom': { + 'dizzle': 'ghi' + }, + 'privateAttrs': [ 'bizzle', 'firstName' ] + }; + + const userWithOwnSpecifiedAttrHidden = { + 'key': 'abc', + 'firstName': 'Sue', + 'custom': { + 'bizzle': 'def' + }, + 'privateAttrs': [ 'dizzle' ] + }; + + const anonUserWithAllAttrsHidden = { + 'key': 'abc', + 'anonymous': true, + 'custom': { }, + 'privateAttrs': [ 'bizzle', 'dizzle' ] + }; + + it('includes all user attributes by default', () => { + const uf = UserFilter({}); + expect(uf.filterUser(user)).toEqual(user); + }); + + it('hides all except key if all_attrs_private is true', () => { + const uf = UserFilter({ all_attributes_private: true}); + expect(uf.filterUser(user)).toEqual(userWithAllAttrsHidden); + }); + + it('hides some attributes if private_attr_names is set', () => { + const uf = UserFilter({ private_attribute_names: [ 'firstName', 'bizzle' ]}); + expect(uf.filterUser(user)).toEqual(userWithSomeAttrsHidden); + }); + + it('hides attributes specified in per-user privateAttrs', () => { + const uf = UserFilter({}); + expect(uf.filterUser(userSpecifyingOwnPrivateAttr)).toEqual(userWithOwnSpecifiedAttrHidden); + }); + + it('looks at both per-user privateAttrs and global config', () => { + const uf = UserFilter({ private_attribute_names: [ 'firstName', 'bizzle' ]}); + expect(uf.filterUser(userSpecifyingOwnPrivateAttr)).toEqual(userWithAllAttrsHidden); + }); + + it('strips unknown top-level attributes', () => { + const uf = UserFilter({}); + expect(uf.filterUser(userWithUnknownTopLevelAttrs)).toEqual(user); + }); + + it('leaves the "anonymous" attribute as is', () => { + const uf = UserFilter({ all_attributes_private: true}); + expect(uf.filterUser(anonUser)).toEqual(anonUserWithAllAttrsHidden); + }); +}); diff --git a/src/index.js b/src/index.js index 27f64690..a4a38f17 100644 --- a/src/index.js +++ b/src/index.js @@ -1,6 +1,5 @@ import EventProcessor from './EventProcessor'; import EventEmitter from './EventEmitter'; -import EventSerializer from './EventSerializer'; import GoalTracker from './GoalTracker'; import Stream from './Stream'; import Requestor from './Requestor'; @@ -24,7 +23,7 @@ function initialize(env, user, options = {}) { const environment = env; const emitter = EventEmitter(); const stream = Stream(streamUrl, environment, hash, options.useReport); - const events = EventProcessor(eventsUrl + '/a/' + environment + '.gif', EventSerializer(options)); + const events = EventProcessor(options, eventsUrl + '/a/' + environment + '.gif'); const requestor = Requestor(baseUrl, environment, options.useReport); const seenRequests = {}; let samplingInterval = parseInt(options.samplingInterval, 10) || 0; From 108d3b6c93a5fb4c6dec95168337b2b8257f22a5 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 11 Apr 2018 13:42:51 -0700 Subject: [PATCH 03/59] summary events (2): add summary counters --- src/EventProcessor.js | 11 ++++ src/EventSummarizer.js | 71 ++++++++++++++++++++++++++ src/__tests__/EventSummarizer-test.js | 73 +++++++++++++++++++++++++++ 3 files changed, 155 insertions(+) create mode 100644 src/EventSummarizer.js create mode 100644 src/__tests__/EventSummarizer-test.js diff --git a/src/EventProcessor.js b/src/EventProcessor.js index d1ade545..66472f28 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -1,3 +1,4 @@ +import EventSummarizer from './EventSummarizer'; import UserFilter from './UserFilter'; import * as utils from './utils'; @@ -41,6 +42,7 @@ function sendEvents(eventsUrl, events, sync) { export default function EventProcessor(options, eventsUrl) { const processor = {}; + const summarizer = EventSummarizer(); const userFilter = UserFilter(options); let queue = []; let initialFlush = true; @@ -50,12 +52,16 @@ export default function EventProcessor(options, eventsUrl) { } processor.enqueue = function(event) { + // Add event to the summary counters if appropriate + summarizer.summarizeEvent(event); queue.push(event); }; processor.flush = function(user, sync) { const finalSync = sync === undefined ? false : sync; const serializedQueue = serializeEvents(queue); + const summary = summarizer.getSummary(); + summarizer.clearSummary(); if (!user) { if (initialFlush) { @@ -70,6 +76,11 @@ export default function EventProcessor(options, eventsUrl) { initialFlush = false; + if (summary) { + summary.kind = 'summary'; + serializedQueue.push(summary); + } + if (serializedQueue.length === 0) { return Promise.resolve(); } diff --git a/src/EventSummarizer.js b/src/EventSummarizer.js new file mode 100644 index 00000000..7e341ea3 --- /dev/null +++ b/src/EventSummarizer.js @@ -0,0 +1,71 @@ +export default function EventSummarizer() { + const es = {}; + + let startDate = 0, + endDate = 0, + counters = {}; + + es.summarizeEvent = function(event) { + if (event.kind === 'feature') { + const counterKey = event.key + ':' + (event.variation || '') + (event.version || ''); + const counterVal = counters[counterKey]; + if (counterVal) { + counterVal.count = counterVal.count + 1; + } else { + counters[counterKey] = { + count: 1, + key: event.key, + version: event.version, + value: event.value, + default: event.default, + }; + } + if (startDate === 0 || event.creationDate < startDate) { + startDate = event.creationDate; + } + if (event.creationDate > endDate) { + endDate = event.creationDate; + } + } + }; + + es.getSummary = function() { + const flagsOut = {}; + let empty = true; + for (let i in counters) { + const c = counters[i]; + let flag = flagsOut[c.key]; + if (!flag) { + flag = { + default: c.default, + counters: [], + }; + flagsOut[c.key] = flag; + } + const counterOut = { + value: c.value, + count: c.count, + }; + if (c.version) { + counterOut.version = c.version; + } else { + counterOut.unknown = true; + } + flag.counters.push(counterOut); + empty = false; + } + return empty ? null : { + startDate: startDate, + endDate: endDate, + features: flagsOut, + }; + }; + + es.clearSummary = function() { + startDate = 0; + endDate = 0; + counters = {}; + }; + + return es; +} diff --git a/src/__tests__/EventSummarizer-test.js b/src/__tests__/EventSummarizer-test.js new file mode 100644 index 00000000..c12e0bd9 --- /dev/null +++ b/src/__tests__/EventSummarizer-test.js @@ -0,0 +1,73 @@ +import EventSummarizer from '../EventSummarizer'; + +describe('EventSummarizer', () => { + const user = { key: 'key1' }; + + it('does nothing for identify event', () => { + const es = EventSummarizer(); + const snapshot = es.getSummary(); + es.summarizeEvent({ kind: 'identify', creationDate: 1000, user: user }); + expect(es.getSummary()).toEqual(snapshot); + }); + + it('does nothing for custom event', () => { + const es = EventSummarizer(); + const snapshot = es.getSummary(); + es.summarizeEvent({ kind: 'custom', creationDate: 1000, key: 'eventkey', user: user }); + expect(es.getSummary()).toEqual(snapshot); + }); + + it('sets start and end dates for feature events', () => { + const es = EventSummarizer(); + const event1 = { kind: 'feature', creationDate: 2000, key: 'key', user: user }; + const event2 = { kind: 'feature', creationDate: 1000, key: 'key', user: user }; + const event3 = { kind: 'feature', creationDate: 1500, key: 'key', user: user }; + es.summarizeEvent(event1); + es.summarizeEvent(event2); + es.summarizeEvent(event3); + const data = es.getSummary(); + + expect(data.startDate).toEqual(1000); + expect(data.endDate).toEqual(2000); + }); + + it('increments counters for feature events', () => { + const es = EventSummarizer(); + const event1 = { kind: 'feature', creationDate: 1000, key: 'key1', version: 11, user: user, + variation: 1, value: 100, default: 111 }; + const event2 = { kind: 'feature', creationDate: 1000, key: 'key1', version: 11, user: user, + variation: 2, value: 200, default: 111 }; + const event3 = { kind: 'feature', creationDate: 1000, key: 'key2', version: 22, user: user, + variation: 1, value: 999, default: 222 }; + const event4 = { kind: 'feature', creationDate: 1000, key: 'key1', version: 11, user: user, + variation: 1, value: 100, default: 111 }; + const event5 = { kind: 'feature', creationDate: 1000, key: 'badkey', user: user, + value: 333, default: 333 }; + es.summarizeEvent(event1); + es.summarizeEvent(event2); + es.summarizeEvent(event3); + es.summarizeEvent(event4); + es.summarizeEvent(event5); + const data = es.getSummary(); + + data.features.key1.counters.sort((a, b) => a.value - b.value); + const expectedFeatures = { + key1: { + default: 111, + counters: [ + { value: 100, version: 11, count: 2 }, + { value: 200, version: 11, count: 1 }, + ], + }, + key2: { + default: 222, + counters: [ { value: 999, version: 22, count: 1 }] + }, + badkey: { + default: 333, + counters: [ { value: 333, unknown: true, count: 1 }] + } + }; + expect(data.features).toEqual(expectedFeatures); + }); +}); From 28e694c96fa19452acfb71ee6cf233eb459c44f6 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 11 Apr 2018 13:57:52 -0700 Subject: [PATCH 04/59] summary events (3): add user deduplication --- package-lock.json | 11 +++++++++-- package.json | 3 ++- src/EventProcessor.js | 45 ++++++++++++++++++++++++++++++++++++------- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index e3e4271f..73d1fb66 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4449,8 +4449,7 @@ "inherits": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.3.tgz", - "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=", - "dev": true + "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=" }, "inquirer": { "version": "3.3.0", @@ -6388,6 +6387,14 @@ "js-tokens": "3.0.2" } }, + "lru": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/lru/-/lru-3.1.0.tgz", + "integrity": "sha1-6n+4VG2DczOWoTCR12z+tMBoN9U=", + "requires": { + "inherits": "2.0.3" + } + }, "lru-cache": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-4.1.1.tgz", diff --git a/package.json b/package.json index 6999941e..1991acb7 100755 --- a/package.json +++ b/package.json @@ -72,7 +72,8 @@ }, "dependencies": { "Base64": "1.0.1", - "escape-string-regexp": "1.0.5" + "escape-string-regexp": "1.0.5", + "lru": "3.1.0" }, "repository": { "type": "git", diff --git a/src/EventProcessor.js b/src/EventProcessor.js index 66472f28..5e752f09 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -1,3 +1,4 @@ +import LRU from 'lru'; import EventSummarizer from './EventSummarizer'; import UserFilter from './UserFilter'; import * as utils from './utils'; @@ -43,23 +44,53 @@ function sendEvents(eventsUrl, events, sync) { export default function EventProcessor(options, eventsUrl) { const processor = {}; const summarizer = EventSummarizer(); + const userKeysCache = LRU(options.user_keys_capacity || 1000); const userFilter = UserFilter(options); + const inlineUsers = !!options.inline_users_in_events; let queue = []; let initialFlush = true; - function serializeEvents(events) { - return events.map(e => (e.user ? Object.assign({}, e, { user: userFilter.filterUser(e.user) }) : e)); + function makeOutputEvent(e) { + if (!e.user) { + return e; + } + if (inlineUsers) { + return Object.assign({}, e, { user: userFilter.filterUser(e.user) }); + } else { + const ret = Object.assign({}, e, { userKey: e.user.key }); + delete ret['user']; + return ret; + } } processor.enqueue = function(event) { // Add event to the summary counters if appropriate summarizer.summarizeEvent(event); - queue.push(event); + + // For each user we haven't seen before, we add an index event - unless this is already + // an identify event for that user. + let addIndexEvent = false; + if (!inlineUsers) { + if (event.user && !userKeysCache.get(event.user.key)) { + userKeysCache.set(event.user.key, true); + if (event.kind !== 'identify') { + addIndexEvent = true; + } + } + } + + if (addIndexEvent) { + queue.push({ + kind: 'index', + creationDate: event.creationDate, + user: userFilter.filter_user(event.user), + }); + } + queue.push(makeOutputEvent(event)); }; processor.flush = function(user, sync) { const finalSync = sync === undefined ? false : sync; - const serializedQueue = serializeEvents(queue); const summary = summarizer.getSummary(); summarizer.clearSummary(); @@ -78,14 +109,14 @@ export default function EventProcessor(options, eventsUrl) { if (summary) { summary.kind = 'summary'; - serializedQueue.push(summary); + queue.push(summary); } - if (serializedQueue.length === 0) { + if (queue.length === 0) { return Promise.resolve(); } - const chunks = utils.chunkUserEventsForUrl(MAX_URL_LENGTH - eventsUrl.length, serializedQueue); + const chunks = utils.chunkUserEventsForUrl(MAX_URL_LENGTH - eventsUrl.length, queue); const results = []; for (let i = 0; i < chunks.length; i++) { From 2ce74c22e3b76cde2d8c4b41b4b52a133f25aab1 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 11 Apr 2018 14:20:53 -0700 Subject: [PATCH 05/59] identify events always have an inline user --- src/EventProcessor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EventProcessor.js b/src/EventProcessor.js index 5e752f09..7469325d 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -54,7 +54,7 @@ export default function EventProcessor(options, eventsUrl) { if (!e.user) { return e; } - if (inlineUsers) { + if (inlineUsers || e.kind === 'identify') { // identify events always have an inline user return Object.assign({}, e, { user: userFilter.filterUser(e.user) }); } else { const ret = Object.assign({}, e, { userKey: e.user.key }); From a09f98fb6ff729e0ef60cc0f63b751b9f86948db Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 11 Apr 2018 15:32:29 -0700 Subject: [PATCH 06/59] summary events (4): break HTTP logic out of event processor, add unit tests for both --- src/EventProcessor.js | 62 +----- src/EventSender.js | 55 ++++++ src/__tests__/EventProcessor-test.js | 285 +++++++++++++++++++++++++-- src/__tests__/EventSender-test.js | 95 +++++++++ src/utils.js | 4 +- 5 files changed, 427 insertions(+), 74 deletions(-) create mode 100644 src/EventSender.js create mode 100644 src/__tests__/EventSender-test.js diff --git a/src/EventProcessor.js b/src/EventProcessor.js index 7469325d..2d9dd1e7 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -1,48 +1,11 @@ import LRU from 'lru'; +import EventSender from './EventSender'; import EventSummarizer from './EventSummarizer'; import UserFilter from './UserFilter'; -import * as utils from './utils'; -const MAX_URL_LENGTH = 2000; -const hasCors = 'withCredentials' in new XMLHttpRequest(); - -function sendEvents(eventsUrl, events, sync) { - const src = eventsUrl + '?d=' + utils.base64URLEncode(JSON.stringify(events)); - - const send = onDone => { - // Detect browser support for CORS - if (hasCors) { - /* supports cross-domain requests */ - const xhr = new XMLHttpRequest(); - xhr.open('GET', src, !sync); - - if (!sync) { - xhr.addEventListener('load', onDone); - } - - xhr.send(); - } else { - const img = new Image(); - - if (!sync) { - img.addEventListener('load', onDone); - } - - img.src = src; - } - }; - - if (sync) { - send(); - } else { - return new Promise(resolve => { - send(resolve); - }); - } -} - -export default function EventProcessor(options, eventsUrl) { +export default function EventProcessor(options, eventsUrl, sender) { const processor = {}; + const eventSender = sender || EventSender(eventsUrl); const summarizer = EventSummarizer(); const userKeysCache = LRU(options.user_keys_capacity || 1000); const userFilter = UserFilter(options); @@ -83,14 +46,14 @@ export default function EventProcessor(options, eventsUrl) { queue.push({ kind: 'index', creationDate: event.creationDate, - user: userFilter.filter_user(event.user), + user: userFilter.filterUser(event.user), }); } queue.push(makeOutputEvent(event)); }; processor.flush = function(user, sync) { - const finalSync = sync === undefined ? false : sync; + const eventsToSend = queue; const summary = summarizer.getSummary(); summarizer.clearSummary(); @@ -109,23 +72,14 @@ export default function EventProcessor(options, eventsUrl) { if (summary) { summary.kind = 'summary'; - queue.push(summary); + eventsToSend.push(summary); } - if (queue.length === 0) { + if (eventsToSend.length === 0) { return Promise.resolve(); } - - const chunks = utils.chunkUserEventsForUrl(MAX_URL_LENGTH - eventsUrl.length, queue); - - const results = []; - for (let i = 0; i < chunks.length; i++) { - results.push(sendEvents(eventsUrl, chunks[i], finalSync)); - } - queue = []; - - return sync ? Promise.resolve() : Promise.all(results); + return eventSender.sendEvents(eventsToSend, sync); }; return processor; diff --git a/src/EventSender.js b/src/EventSender.js new file mode 100644 index 00000000..2f06db42 --- /dev/null +++ b/src/EventSender.js @@ -0,0 +1,55 @@ +import * as utils from './utils'; + +const MAX_URL_LENGTH = 2000; +const hasCors = 'withCredentials' in new XMLHttpRequest(); + +export default function EventSender(eventsUrl) { + const sender = {}; + + function sendChunk(events, sync) { + const src = eventsUrl + '?d=' + utils.base64URLEncode(JSON.stringify(events)); + + const send = onDone => { + // Detect browser support for CORS + if (hasCors) { + /* supports cross-domain requests */ + const xhr = new XMLHttpRequest(); + xhr.open('GET', src, !sync); + + if (!sync) { + xhr.addEventListener('load', onDone); + } + + xhr.send(); + } else { + const img = new Image(); + + if (!sync) { + img.addEventListener('load', onDone); + } + + img.src = src; + } + }; + + if (sync) { + send(); + } else { + return new Promise(resolve => { + send(resolve); + }); + } + } + + sender.sendEvents = function(events, sync) { + const finalSync = sync === undefined ? false : sync; + const chunks = utils.chunkUserEventsForUrl(MAX_URL_LENGTH - eventsUrl.length, events); + const results = []; + for (let i = 0; i < chunks.length; i++) { + results.push(sendChunk(chunks[i], finalSync)); + } + return sync ? Promise.resolve() : Promise.all(results); + }; + + return sender; +} diff --git a/src/__tests__/EventProcessor-test.js b/src/__tests__/EventProcessor-test.js index beab229e..f1a1be5c 100644 --- a/src/__tests__/EventProcessor-test.js +++ b/src/__tests__/EventProcessor-test.js @@ -4,37 +4,78 @@ import EventProcessor from '../EventProcessor'; describe('EventProcessor', () => { let sandbox; - let xhr; - let requests = []; let warnSpy; + const mockEventSender = {}; + const user = { key: 'userKey', name: 'Red' }; + const filteredUser = { key: 'userKey', privateAttrs: [ 'name' ] }; + const eventsUrl = '/fake-url'; + + mockEventSender.sendEvents = function(events, sync) { + mockEventSender.calls.push({ + events: events, + sync: !!sync, + }); + return Promise.resolve(); + }; beforeEach(() => { sandbox = sinon.sandbox.create(); - requests = []; - xhr = sinon.useFakeXMLHttpRequest(); - xhr.onCreate = function(xhr) { - requests.push(xhr); - }; warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + mockEventSender.calls = []; }); afterEach(() => { sandbox.restore(); - xhr.restore(); warnSpy.mockRestore(); }); + + function checkIndexEvent(e, source, user) { + expect(e.kind).toEqual('index'); + expect(e.creationDate).toEqual(source.creationDate); + expect(e.user).toEqual(user); + } + + function checkFeatureEvent(e, source, debug, inlineUser) { + expect(e.kind).toEqual(debug ? 'debug' : 'feature'); + expect(e.creationDate).toEqual(source.creationDate); + expect(e.key).toEqual(source.key); + expect(e.version).toEqual(source.version); + expect(e.value).toEqual(source.value); + expect(e.default).toEqual(source.default); + if (inlineUser) { + expect(e.user).toEqual(inlineUser); + } else { + expect(e.userKey).toEqual(source.user.key); + } + } + + function checkCustomEvent(e, source, inlineUser) { + expect(e.kind).toEqual('custom'); + expect(e.creationDate).toEqual(source.creationDate); + expect(e.key).toEqual(source.key); + expect(e.data).toEqual(source.data); + if (inlineUser) { + expect(e.user).toEqual(inlineUser); + } else { + expect(e.userKey).toEqual(source.user.key); + } + } + + function checkSummaryEvent(e) { + expect(e.kind).toEqual('summary'); + } + it('should warn about missing user on initial flush', () => { const warnSpy = sandbox.spy(console, 'warn'); - const processor = EventProcessor({}, '/fake-url'); + const processor = EventProcessor({}, eventsUrl, mockEventSender); processor.flush(null); warnSpy.restore(); expect(warnSpy.called).toEqual(true); }); it('should flush asynchronously', () => { - const processor = EventProcessor({}, '/fake-url'); - const user = { key: 'foo' }; + const processor = EventProcessor({}, eventsUrl, mockEventSender); const event = { kind: 'identify', key: user.key }; processor.enqueue(event); @@ -42,14 +83,13 @@ describe('EventProcessor', () => { processor.enqueue(event); processor.enqueue(event); processor.flush(user); - requests[0].respond(); - expect(requests.length).toEqual(1); - expect(requests[0].async).toEqual(true); + expect(mockEventSender.calls.length).toEqual(1); + expect(mockEventSender.calls[0].sync).toEqual(false); }); it('should flush synchronously', () => { - const processor = EventProcessor({}, '/fake-url'); + const processor = EventProcessor({}, eventsUrl, mockEventSender); const user = { key: 'foo' }; const event = { kind: 'identify', key: user.key }; @@ -58,9 +98,218 @@ describe('EventProcessor', () => { processor.enqueue(event); processor.enqueue(event); processor.flush(user, true); - requests[0].respond(); - expect(requests.length).toEqual(1); - expect(requests[0].async).toEqual(false); + expect(mockEventSender.calls.length).toEqual(1); + expect(mockEventSender.calls[0].sync).toEqual(true); + }); + + it('should enqueue identify event', done => { + const ep = EventProcessor({}, eventsUrl, mockEventSender); + const event = { kind: 'identify', creationDate: 1000, key: user.key, user: user }; + ep.enqueue(event); + ep.flush(user, false).then(() => { + expect(mockEventSender.calls.length).toEqual(1); + expect(mockEventSender.calls[0].events).toEqual([event]); + done(); + }); + }); + + it('filters user in identify event', done => { + const config = { all_attributes_private: true }; + const ep = EventProcessor(config, eventsUrl, mockEventSender); + const event = { kind: 'identify', creationDate: 1000, key: user.key, user: user }; + ep.enqueue(event); + ep.flush(user, false).then(() => { + expect(mockEventSender.calls.length).toEqual(1); + expect(mockEventSender.calls[0].events).toEqual([{ + kind: 'identify', + creationDate: event.creationDate, + key: user.key, + user: filteredUser, + }]); + done(); + }); + }); + + it('queues individual feature event with index event', done => { + const ep = EventProcessor({}, eventsUrl, mockEventSender); + const event = { + kind: 'feature', + creationDate: 1000, + key: 'flagkey', + user: user, + }; + ep.enqueue(event); + ep.flush(user, false).then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(3); + checkIndexEvent(output[0], event, user); + checkFeatureEvent(output[1], event, false); + checkSummaryEvent(output[2]); + done(); + }); + }); + + it('filters user in index event', done => { + const config = { all_attributes_private: true }; + const ep = EventProcessor(config, eventsUrl, mockEventSender); + const event = { + kind: 'feature', + creationDate: 1000, + key: 'flagkey', + user: user, + }; + ep.enqueue(event); + ep.flush(user, false).then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(3); + checkIndexEvent(output[0], event, filteredUser); + checkFeatureEvent(output[1], event, false); + checkSummaryEvent(output[2]); + done(); + }); + }); + + it('can include inline user in feature event', done => { + const config = { inline_users_in_events: true }; + const ep = EventProcessor(config, eventsUrl, mockEventSender); + const event = { + kind: 'feature', + creationDate: 1000, + key: 'flagkey', + user: user, + }; + ep.enqueue(event); + ep.flush(user, false).then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(2); + checkFeatureEvent(output[0], event, false, user); + checkSummaryEvent(output[1]); + done(); + }); + }); + + it('filters user in feature event', done => { + const config = { all_attributes_private: true, inline_users_in_events: true }; + const ep = EventProcessor(config, eventsUrl, mockEventSender); + const event = { + kind: 'feature', + creationDate: 1000, + key: 'flagkey', + user: user, + }; + ep.enqueue(event); + ep.flush(user, false).then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(2); + checkFeatureEvent(output[0], event, false, filteredUser); + checkSummaryEvent(output[1]); + done(); + }); + }); + + it('generates only one index event from two feature events for same user', done => { + const ep = EventProcessor({}, eventsUrl, mockEventSender); + const e1 = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey1', + version: 11, value: 'value' }; + const e2 = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey2', + version: 11, value: 'value' }; + ep.enqueue(e1); + ep.enqueue(e2); + ep.flush(user, false).then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(4); + checkIndexEvent(output[0], e1, user); + checkFeatureEvent(output[1], e1, false); + checkFeatureEvent(output[2], e2, false); + checkSummaryEvent(output[3]); + done(); + }); + }); + + it('summarizes events', done => { + const ep = EventProcessor({}, eventsUrl, mockEventSender); + const e1 = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey1', + version: 11, variation: 1, value: 'value1', default: 'default1', trackEvents: false }; + const e2 = { kind: 'feature', creationDate: 2000, user: user, key: 'flagkey2', + version: 22, variation: 1, value: 'value2', default: 'default2', trackEvents: false }; + ep.enqueue(e1); + ep.enqueue(e2); + ep.flush(user, false).then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(4); + const se = output[3]; + checkSummaryEvent(se); + expect(se.startDate).toEqual(1000); + expect(se.endDate).toEqual(2000); + expect(se.features).toEqual({ + flagkey1: { + default: 'default1', + counters: [ { version: 11, value: 'value1', count: 1 } ] + }, + flagkey2: { + default: 'default2', + counters: [ { version: 22, value: 'value2', count: 1 } ] + } + }); + done(); + }); + }); + + it('queues custom event with user', done => { + const ep = EventProcessor({}, eventsUrl, mockEventSender); + const e = { kind: 'custom', creationDate: 1000, user: user, key: 'eventkey', + data: { thing: 'stuff' } }; + ep.enqueue(e); + ep.flush(user, false).then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(2); + checkIndexEvent(output[0], e, user); + checkCustomEvent(output[1], e); + done(); + }); + }); + + it('can include inline user in custom event', done => { + const config = { inline_users_in_events: true }; + const ep = EventProcessor(config, eventsUrl, mockEventSender); + const e = { kind: 'custom', creationDate: 1000, user: user, key: 'eventkey', + data: { thing: 'stuff' } }; + ep.enqueue(e); + ep.flush(user, false).then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(1); + checkCustomEvent(output[0], e, user); + done(); + }); + }); + + it('filters user in custom event', done => { + const config = { all_attributes_private: true, inline_users_in_events: true }; + const ep = EventProcessor(config, eventsUrl, mockEventSender); + const e = { kind: 'custom', creationDate: 1000, user: user, key: 'eventkey', + data: { thing: 'stuff' } }; + ep.enqueue(e); + ep.flush(user, false).then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(1); + checkCustomEvent(output[0], e, filteredUser); + done(); + }); + }); + + it('sends nothing if there are no events to flush', () => { + const processor = EventProcessor({}, '/fake-url', mockEventSender); + processor.flush(user, false); + expect(mockEventSender.calls.length).toEqual(0); }); }); diff --git a/src/__tests__/EventSender-test.js b/src/__tests__/EventSender-test.js new file mode 100644 index 00000000..d93d682f --- /dev/null +++ b/src/__tests__/EventSender-test.js @@ -0,0 +1,95 @@ +import Base64 from 'Base64'; +import sinon from 'sinon'; + +import EventSender from '../EventSender'; + +describe('EventSender', () => { + let sandbox; + let xhr; + let requests = []; + let warnSpy; + const eventsUrl = '/fake-url'; + + beforeEach(() => { + sandbox = sinon.sandbox.create(); + requests = []; + xhr = sinon.useFakeXMLHttpRequest(); + xhr.onCreate = function(xhr) { + requests.push(xhr); + }; + warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + sandbox.restore(); + xhr.restore(); + warnSpy.mockRestore(); + }); + + function base64URLDecode(str) { + let s = str; + while (s.length % 4 !== 0) { + s = s + '='; + } + s = s.replace(/_/g, '/').replace(/-/g, '+'); + return decodeURIComponent(escape(Base64.atob(s))); + } + + function decodeOutputFromUrl(url) { + const prefix = eventsUrl + '?d='; + if (!url.startsWith(prefix)) { + throw 'URL "' + url + '" did not have expected prefix "' + prefix + '"'; + } + return JSON.parse(base64URLDecode(url.substring(prefix.length))); + } + + it('should send asynchronously', () => { + const sender = EventSender(eventsUrl); + const event = { kind: 'identify', key: 'userKey' }; + sender.sendEvents([event], false); + requests[0].respond(); + expect(requests.length).toEqual(1); + expect(requests[0].async).toEqual(true); + }); + + it('should send synchronously', () => { + const sender = EventSender(eventsUrl); + const event = { kind: 'identify', key: 'userKey' }; + sender.sendEvents([event], true); + requests[0].respond(); + expect(requests.length).toEqual(1); + expect(requests[0].async).toEqual(false); + }); + + it('should encode events in a single chunk if they fit', done => { + const sender = EventSender(eventsUrl); + const event1 = { kind: 'identify', key: 'userKey1' }; + const event2 = { kind: 'identify', key: 'userKey2' }; + const events = [event1, event2]; + sender.sendEvents(events, true).then(() => { + expect(requests.length).toEqual(1); + expect(decodeOutputFromUrl(requests[0].url)).toEqual(events); + done(); + }); + requests[0].respond(); + }); + + it('should send events in multiple chunks if necessary', done => { + const sender = EventSender(eventsUrl); + const events = []; + for (let i = 0; i < 80; i++) { + events.push({ kind: 'identify', key: 'thisIsALongUserKey' + i }); + } + sender.sendEvents(events, true).then(() => { + expect(requests.length).toEqual(3); + const output0 = decodeOutputFromUrl(requests[0].url); + const output1 = decodeOutputFromUrl(requests[1].url); + const output2 = decodeOutputFromUrl(requests[2].url); + expect(output0).toEqual(events.slice(0, 31)); + expect(output1).toEqual(events.slice(31, 62)); + expect(output2).toEqual(events.slice(62, 80)); + done(); + }); + requests[0].respond(); + }); +}); diff --git a/src/utils.js b/src/utils.js index 2b9f1024..4d85fdb8 100644 --- a/src/utils.js +++ b/src/utils.js @@ -105,7 +105,7 @@ export function chunkUserEventsForUrl(maxLength, events) { chunk = []; while (remainingSpace > 0) { - const event = allEvents.pop(); + const event = allEvents.shift(); if (!event) { break; } @@ -114,7 +114,7 @@ export function chunkUserEventsForUrl(maxLength, events) { // to try in the next round, unless this event alone is larger // than the limit, in which case, screw it, and try it anyway. if (remainingSpace < 0 && chunk.length > 0) { - allEvents.push(event); + allEvents.unshift(event); } else { chunk.push(event); } From f6d8beaec352dded08cf170109b8a4ff09ae4967 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 11 Apr 2018 15:37:38 -0700 Subject: [PATCH 07/59] typo --- src/EventProcessor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EventProcessor.js b/src/EventProcessor.js index 7469325d..f8d68afd 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -83,7 +83,7 @@ export default function EventProcessor(options, eventsUrl) { queue.push({ kind: 'index', creationDate: event.creationDate, - user: userFilter.filter_user(event.user), + user: userFilter.filterUser(event.user), }); } queue.push(makeOutputEvent(event)); From e424398293fbd94b1924c80e4ee2176eae6696aa Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 11 Apr 2018 17:02:39 -0700 Subject: [PATCH 08/59] summary events (5): implement debug events + misc cleanup, tests --- src/EventProcessor.js | 155 +++++++++++++++---- src/EventSender.js | 16 +- src/Identity.js | 2 +- src/UserFilter.js | 29 ++-- src/__tests__/EventProcessor-test.js | 220 +++++++++++++++++++++------ src/__tests__/LDClient-test.js | 11 ++ src/index.js | 70 +++++---- 7 files changed, 379 insertions(+), 124 deletions(-) diff --git a/src/EventProcessor.js b/src/EventProcessor.js index 2d9dd1e7..c4076a21 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -2,38 +2,109 @@ import LRU from 'lru'; import EventSender from './EventSender'; import EventSummarizer from './EventSummarizer'; import UserFilter from './UserFilter'; +import * as errors from './errors'; +import * as utils from './utils'; -export default function EventProcessor(options, eventsUrl, sender) { +export default function EventProcessor(options, eventsUrl, emitter, sender) { const processor = {}; const eventSender = sender || EventSender(eventsUrl); const summarizer = EventSummarizer(); - const userKeysCache = LRU(options.user_keys_capacity || 1000); + const userKeysCache = LRU(options.userKeysCapacity || 1000); const userFilter = UserFilter(options); - const inlineUsers = !!options.inline_users_in_events; + const inlineUsers = !!options.inlineUsersInEvents; let queue = []; - let initialFlush = true; + let flushInterval; + let userKeysFlushInterval; + let samplingInterval; + let lastKnownPastTime = 0; + let disabled = false; + let flushTimer; + let usersFlushTimer; - function makeOutputEvent(e) { - if (!e.user) { - return e; + function reportArgumentError(message) { + utils.onNextTick(() => { + emitter.maybeReportError(new errors.LDInvalidArgumentError(message)); + }); + } + + if (options.samplingInterval !== undefined && (isNaN(options.samplingInterval) || options.samplingInterval < 0)) { + samplingInterval = 0; + reportArgumentError('Invalid sampling interval configured. Sampling interval must be an integer >= 0.'); + } else { + samplingInterval = options.samplingInterval || 0; + } + + if (options.flushInterval !== undefined && (isNan(options.flushInterval) || options.flushInterval < 2000)) { + flushInterval = 2000; + reportArgumentError('Invalid flush interval configured. Must be an integer >= 2000 (milliseconds).'); + } else { + flushInterval = options.flushInterval || 2000; + } + + if (options.userKeysFlushInterval !== undefined && (isNan(options.userKeysFlushInterval) || options.userKeysFlushInterval < 2000)) { + userKeysFlushInterval = 300000; + reportArgumentError('Invalid user keys flush interval configured. Must be an integer >= 2000 (milliseconds).'); + } else { + userKeysFlushInterval = options.userKeysFlushInterval || 300000; + } + + function shouldSampleEvent() { + return samplingInterval === 0 || Math.floor(Math.random() * samplingInterval) === 0; + } + + function shouldDebugEvent(e) { + if (e.debugEventsUntilDate) { + // The "last known past time" comes from the last HTTP response we got from the server. + // In case the client's time is set wrong, at least we know that any expiration date + // earlier than that point is definitely in the past. If there's any discrepancy, we + // want to err on the side of cutting off event debugging sooner. + return e.debugEventsUntilDate > lastKnownPastTime && e.debugEventsUntilDate > new Date().getTime(); } + return false; + } + + // Transform an event from its internal format to the format we use when sending a payload. + function makeOutputEvent(e) { + const ret = Object.assign({}, e); if (inlineUsers || e.kind === 'identify') { // identify events always have an inline user - return Object.assign({}, e, { user: userFilter.filterUser(e.user) }); + ret.user = userFilter.filterUser(e.user); } else { - const ret = Object.assign({}, e, { userKey: e.user.key }); + ret.userKey = e.user.key; delete ret['user']; - return ret; } + if (e.kind === 'feature') { + delete ret['trackEvents']; + delete ret['debugEventsUntilDate']; + delete ret['variation']; + } + return ret; } processor.enqueue = function(event) { + if (disabled) { + return; + } + let addFullEvent = false; + let addDebugEvent = false; + let addIndexEvent = false; + // Add event to the summary counters if appropriate summarizer.summarizeEvent(event); + // Decide whether to add the event to the payload. Feature events may be added twice, once for + // the event (if tracked) and once for debugging. + if (event.kind === 'feature') { + if (shouldSampleEvent()) { + addFullEvent = !!event.trackEvents; + addDebugEvent = shouldDebugEvent(event); + } + } else { + addFullEvent = shouldSampleEvent(); + } + // For each user we haven't seen before, we add an index event - unless this is already // an identify event for that user. - let addIndexEvent = false; - if (!inlineUsers) { + if (!addFullEvent || !inlineUsers) { if (event.user && !userKeysCache.get(event.user.key)) { userKeysCache.set(event.user.key, true); if (event.kind !== 'identify') { @@ -49,37 +120,61 @@ export default function EventProcessor(options, eventsUrl, sender) { user: userFilter.filterUser(event.user), }); } - queue.push(makeOutputEvent(event)); + if (addFullEvent) { + queue.push(makeOutputEvent(event)); + } + if (addDebugEvent) { + const debugEvent = Object.assign({}, event, { kind: 'debug' }); + delete debugEvent['trackEvents']; + delete debugEvent['debugEventsUntilDate']; + delete debugEvent['variation']; + queue.push(debugEvent); + } }; - processor.flush = function(user, sync) { + processor.flush = function(sync) { + if (disabled) { + return Promise.resolve(); + } const eventsToSend = queue; const summary = summarizer.getSummary(); summarizer.clearSummary(); - - if (!user) { - if (initialFlush) { - if (console && console.warn) { - console.warn( - 'Be sure to call `identify` in the LaunchDarkly client: http://docs.launchdarkly.com/docs/running-an-ab-test#include-the-client-side-snippet' - ); - } - } - return Promise.resolve(); - } - - initialFlush = false; - if (summary) { summary.kind = 'summary'; eventsToSend.push(summary); } - if (eventsToSend.length === 0) { return Promise.resolve(); } queue = []; - return eventSender.sendEvents(eventsToSend, sync); + return eventSender.sendEvents(eventsToSend, sync).then(responseInfo => { + if (responseInfo) { + if (responseInfo.serverTime) { + lastKnownPastTime = responseInfo.serverTime; + } + if (responseInfo.status === 401) { + disabled = true; + } + } + }); + }; + + processor.start = function() { + const flushTick = () => { + processor.flush(); + flushTimer = setTimeout(flushTick, flushInterval); + }; + flushTimer = setTimeout(flushTick, flushInterval); + const usersFlushTick = () => { + userKeysCache.clear(); + usersFlushTimer = setTimeout(usersFlushTick, userKeysFlushInterval); + }; + usersFlushTimer = setTimeout(usersFlushTick, userKeysFlushInterval); + }; + + processor.stop = function() { + clearTimeout(flushTimer); + clearTimeout(usersFlushTimer); }; return processor; diff --git a/src/EventSender.js b/src/EventSender.js index 2f06db42..10c9d62d 100644 --- a/src/EventSender.js +++ b/src/EventSender.js @@ -6,6 +6,18 @@ const hasCors = 'withCredentials' in new XMLHttpRequest(); export default function EventSender(eventsUrl) { const sender = {}; + function getResponseInfo(xhr) { + const ret = { status: xhr.status }; + const dateStr = xhr.getResponseHeader('Date'); + if (dateStr) { + const date = Date.parse(dateStr); + if (date) { + ret.serverTime = date.getTime(); + } + } + return ret; + } + function sendChunk(events, sync) { const src = eventsUrl + '?d=' + utils.base64URLEncode(JSON.stringify(events)); @@ -17,7 +29,9 @@ export default function EventSender(eventsUrl) { xhr.open('GET', src, !sync); if (!sync) { - xhr.addEventListener('load', onDone); + xhr.addEventListener('load', () => { + onDone(getResponseInfo(xhr)); + }); } xhr.send(); diff --git a/src/Identity.js b/src/Identity.js index 36754f89..37c6a7a4 100644 --- a/src/Identity.js +++ b/src/Identity.js @@ -18,7 +18,7 @@ export default function Identity(initialUser, onChange) { }; ident.getUser = function() { - return utils.clone(user); + return user ? utils.clone(user) : null; }; if (initialUser) { diff --git a/src/UserFilter.js b/src/UserFilter.js index c235339a..c5af0ada 100644 --- a/src/UserFilter.js +++ b/src/UserFilter.js @@ -14,17 +14,19 @@ export default function UserFilter(config) { firstName: true, lastName: true, avatar: true, name: true, anonymous: true, custom: true }; - filter.filterUser = function (user) { - let allPrivateAttrs = {}; - let userPrivateAttrs = user.privateAttributeNames || []; + filter.filterUser = function(user) { + if (!user) { + return null; + } + const userPrivateAttrs = user.privateAttributeNames || []; - const isPrivateAttr = function (name) { + const isPrivateAttr = function(name) { return !ignoreAttrs[name] && ( allAttributesPrivate || userPrivateAttrs.indexOf(name) !== -1 || privateAttributeNames.indexOf(name) !== -1); } - const filterAttrs = function (props, isAttributeAllowed) { - return Object.keys(props).reduce(function (acc, name) { + const filterAttrs = function(props, isAttributeAllowed) { + return Object.keys(props).reduce((acc, name) => { if (isAttributeAllowed(name)) { if (isPrivateAttr(name)) { // add to hidden list @@ -35,21 +37,22 @@ export default function UserFilter(config) { } return acc; }, [{}, {}]); - } - let result = filterAttrs(user, function (key) { return allowedTopLevelAttrs[key]; }); - let filteredProps = result[0]; - let removedAttrs = result[1]; + }; + const result = filterAttrs(user, key => allowedTopLevelAttrs[key]); + const filteredProps = result[0]; + const removedAttrs = result[1]; if (user.custom) { - var customResult = filterAttrs(user.custom, function (key) { return true; }); + const customResult = filterAttrs(user.custom, key => true); filteredProps.custom = customResult[0]; Object.assign(removedAttrs, customResult[1]); } - var removedAttrNames = Object.keys(removedAttrs); + const removedAttrNames = Object.keys(removedAttrs); if (removedAttrNames.length) { removedAttrNames.sort(); filteredProps.privateAttrs = removedAttrNames; } return filteredProps; - } + }; + return filter; } \ No newline at end of file diff --git a/src/__tests__/EventProcessor-test.js b/src/__tests__/EventProcessor-test.js index f1a1be5c..a589bf1c 100644 --- a/src/__tests__/EventProcessor-test.js +++ b/src/__tests__/EventProcessor-test.js @@ -15,13 +15,14 @@ describe('EventProcessor', () => { events: events, sync: !!sync, }); - return Promise.resolve(); + return Promise.resolve({ serverTime: mockEventSender.serverTime, status: mockEventSender.status || 200 }); }; beforeEach(() => { sandbox = sinon.sandbox.create(); warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); mockEventSender.calls = []; + mockEventSender.serverTime = null; }); afterEach(() => { @@ -29,7 +30,6 @@ describe('EventProcessor', () => { warnSpy.mockRestore(); }); - function checkIndexEvent(e, source, user) { expect(e.kind).toEqual('index'); expect(e.creationDate).toEqual(source.creationDate); @@ -66,30 +66,22 @@ describe('EventProcessor', () => { expect(e.kind).toEqual('summary'); } - it('should warn about missing user on initial flush', () => { - const warnSpy = sandbox.spy(console, 'warn'); - const processor = EventProcessor({}, eventsUrl, mockEventSender); - processor.flush(null); - warnSpy.restore(); - expect(warnSpy.called).toEqual(true); - }); - it('should flush asynchronously', () => { - const processor = EventProcessor({}, eventsUrl, mockEventSender); + const processor = EventProcessor({}, eventsUrl, null, mockEventSender); const event = { kind: 'identify', key: user.key }; processor.enqueue(event); processor.enqueue(event); processor.enqueue(event); processor.enqueue(event); - processor.flush(user); + processor.flush(); expect(mockEventSender.calls.length).toEqual(1); expect(mockEventSender.calls[0].sync).toEqual(false); }); it('should flush synchronously', () => { - const processor = EventProcessor({}, eventsUrl, mockEventSender); + const processor = EventProcessor({}, eventsUrl, null, mockEventSender); const user = { key: 'foo' }; const event = { kind: 'identify', key: user.key }; @@ -97,17 +89,17 @@ describe('EventProcessor', () => { processor.enqueue(event); processor.enqueue(event); processor.enqueue(event); - processor.flush(user, true); + processor.flush(true); expect(mockEventSender.calls.length).toEqual(1); expect(mockEventSender.calls[0].sync).toEqual(true); }); it('should enqueue identify event', done => { - const ep = EventProcessor({}, eventsUrl, mockEventSender); + const ep = EventProcessor({}, eventsUrl, null, mockEventSender); const event = { kind: 'identify', creationDate: 1000, key: user.key, user: user }; ep.enqueue(event); - ep.flush(user, false).then(() => { + ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(1); expect(mockEventSender.calls[0].events).toEqual([event]); done(); @@ -116,10 +108,10 @@ describe('EventProcessor', () => { it('filters user in identify event', done => { const config = { all_attributes_private: true }; - const ep = EventProcessor(config, eventsUrl, mockEventSender); + const ep = EventProcessor(config, eventsUrl, null, mockEventSender); const event = { kind: 'identify', creationDate: 1000, key: user.key, user: user }; ep.enqueue(event); - ep.flush(user, false).then(() => { + ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(1); expect(mockEventSender.calls[0].events).toEqual([{ kind: 'identify', @@ -132,15 +124,16 @@ describe('EventProcessor', () => { }); it('queues individual feature event with index event', done => { - const ep = EventProcessor({}, eventsUrl, mockEventSender); + const ep = EventProcessor({}, eventsUrl, null, mockEventSender); const event = { kind: 'feature', creationDate: 1000, key: 'flagkey', user: user, + trackEvents: true, }; ep.enqueue(event); - ep.flush(user, false).then(() => { + ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(1); const output = mockEventSender.calls[0].events; expect(output.length).toEqual(3); @@ -153,15 +146,16 @@ describe('EventProcessor', () => { it('filters user in index event', done => { const config = { all_attributes_private: true }; - const ep = EventProcessor(config, eventsUrl, mockEventSender); + const ep = EventProcessor(config, eventsUrl, null, mockEventSender); const event = { kind: 'feature', creationDate: 1000, key: 'flagkey', user: user, + trackEvents: true, }; ep.enqueue(event); - ep.flush(user, false).then(() => { + ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(1); const output = mockEventSender.calls[0].events; expect(output.length).toEqual(3); @@ -173,16 +167,17 @@ describe('EventProcessor', () => { }); it('can include inline user in feature event', done => { - const config = { inline_users_in_events: true }; - const ep = EventProcessor(config, eventsUrl, mockEventSender); + const config = { inlineUsersInEvents: true }; + const ep = EventProcessor(config, eventsUrl, null, mockEventSender); const event = { kind: 'feature', creationDate: 1000, key: 'flagkey', user: user, + trackEvents: true, }; ep.enqueue(event); - ep.flush(user, false).then(() => { + ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(1); const output = mockEventSender.calls[0].events; expect(output.length).toEqual(2); @@ -193,16 +188,17 @@ describe('EventProcessor', () => { }); it('filters user in feature event', done => { - const config = { all_attributes_private: true, inline_users_in_events: true }; - const ep = EventProcessor(config, eventsUrl, mockEventSender); + const config = { all_attributes_private: true, inlineUsersInEvents: true }; + const ep = EventProcessor(config, eventsUrl, null, mockEventSender); const event = { kind: 'feature', creationDate: 1000, key: 'flagkey', user: user, + trackEvents: true, }; ep.enqueue(event); - ep.flush(user, false).then(() => { + ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(1); const output = mockEventSender.calls[0].events; expect(output.length).toEqual(2); @@ -212,15 +208,124 @@ describe('EventProcessor', () => { }); }); + it('still generates index event if inline_users is true but feature event is not tracked', done => { + const config = { inlineUsersInEvents: true }; + const ep = EventProcessor(config, eventsUrl, null, mockEventSender); + const e = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey', + version: 11, variation: 1, value: 'value', trackEvents: false }; + ep.enqueue(e); + ep.flush().then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(2); + checkIndexEvent(output[0], e, user); + checkSummaryEvent(output[1]); + done(); + }); + }); + + it('sets event kind to debug if event is temporarily in debug mode', done => { + const ep = EventProcessor({}, eventsUrl, null, mockEventSender); + const futureTime = new Date().getTime() + 1000000; + const e = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey', + version: 11, variation: 1, value: 'value', trackEvents: false, debugEventsUntilDate: futureTime }; + ep.enqueue(e); + ep.flush().then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(3); + checkIndexEvent(output[0], e, user); + checkFeatureEvent(output[1], e, true, user); + checkSummaryEvent(output[2]); + done(); + }); + }); + + it('can both track and debug an event', done => { + const ep = EventProcessor({}, eventsUrl, null, mockEventSender); + const futureTime = new Date().getTime() + 1000000; + const e = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey', + version: 11, variation: 1, value: 'value', trackEvents: true, debugEventsUntilDate: futureTime }; + ep.enqueue(e); + ep.flush().then(() => { + expect(mockEventSender.calls.length).toEqual(1); + const output = mockEventSender.calls[0].events; + expect(output.length).toEqual(4); + checkIndexEvent(output[0], e, user); + checkFeatureEvent(output[1], e, false); + checkFeatureEvent(output[2], e, true, user); + checkSummaryEvent(output[3]); + done(); + }); + }); + + it('expires debug mode based on client time if client time is later than server time', done => { + const ep = EventProcessor({}, eventsUrl, null, mockEventSender); + + // Pick a server time that is somewhat behind the client time + const serverTime = new Date().getTime() - 20000; + mockEventSender.serverTime = serverTime; + + // Send and flush an event we don't care about, just to set the last server time + ep.enqueue({ kind: 'identify', user: { key: 'otherUser' } }); + ep.flush().then(() => { + // Now send an event with debug mode on, with a "debug until" time that is further in + // the future than the server time, but in the past compared to the client. + const debugUntil = serverTime + 1000; + const e = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey', + version: 11, variation: 1, value: 'value', trackEvents: false, debugEventsUntilDate: debugUntil }; + ep.enqueue(e); + + // Should get a summary event only, not a full feature event + ep.flush().then(() => { + expect(mockEventSender.calls.length).toEqual(2); + const output = mockEventSender.calls[1].events; + expect(output.length).toEqual(2); + checkIndexEvent(output[0], e, user); + checkSummaryEvent(output[1]); + done(); + }); + }); + }); + + it('expires debug mode based on server time if server time is later than client time', done => { + const ep = EventProcessor({}, eventsUrl, null, mockEventSender); + + // Pick a server time that is somewhat ahead of the client time + const serverTime = new Date().getTime() + 20000; + mockEventSender.serverTime = serverTime; + + // Send and flush an event we don't care about, just to set the last server time + ep.enqueue({ kind: 'identify', user: { key: 'otherUser' } }); + ep.flush().then(() => { + // Now send an event with debug mode on, with a "debug until" time that is further in + // the future than the client time, but in the past compared to the server. + const debugUntil = serverTime - 1000; + const e = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey', + version: 11, variation: 1, value: 'value', trackEvents: false, debugEventsUntilDate: debugUntil }; + ep.enqueue(e); + + // Should get a summary event only, not a full feature event + ep.flush().then(() => { + expect(mockEventSender.calls.length).toEqual(2); + const output = mockEventSender.calls[1].events; + expect(output.length).toEqual(2); + checkIndexEvent(output[0], e, user); + checkSummaryEvent(output[1]); + done(); + }); + }); + }); + it('generates only one index event from two feature events for same user', done => { - const ep = EventProcessor({}, eventsUrl, mockEventSender); + const ep = EventProcessor({}, eventsUrl, null, mockEventSender); const e1 = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey1', - version: 11, value: 'value' }; + version: 11, value: 'value', trackEvents: true }; const e2 = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey2', - version: 11, value: 'value' }; + version: 11, value: 'value', trackEvents: true }; ep.enqueue(e1); ep.enqueue(e2); - ep.flush(user, false).then(() => { + ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(1); const output = mockEventSender.calls[0].events; expect(output.length).toEqual(4); @@ -232,19 +337,19 @@ describe('EventProcessor', () => { }); }); - it('summarizes events', done => { - const ep = EventProcessor({}, eventsUrl, mockEventSender); + it('summarizes nontracked events', done => { + const ep = EventProcessor({}, eventsUrl, null, mockEventSender); const e1 = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey1', version: 11, variation: 1, value: 'value1', default: 'default1', trackEvents: false }; const e2 = { kind: 'feature', creationDate: 2000, user: user, key: 'flagkey2', version: 22, variation: 1, value: 'value2', default: 'default2', trackEvents: false }; ep.enqueue(e1); ep.enqueue(e2); - ep.flush(user, false).then(() => { + ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(1); const output = mockEventSender.calls[0].events; - expect(output.length).toEqual(4); - const se = output[3]; + expect(output.length).toEqual(2); + const se = output[1]; checkSummaryEvent(se); expect(se.startDate).toEqual(1000); expect(se.endDate).toEqual(2000); @@ -263,11 +368,11 @@ describe('EventProcessor', () => { }); it('queues custom event with user', done => { - const ep = EventProcessor({}, eventsUrl, mockEventSender); + const ep = EventProcessor({}, eventsUrl, null, mockEventSender); const e = { kind: 'custom', creationDate: 1000, user: user, key: 'eventkey', data: { thing: 'stuff' } }; ep.enqueue(e); - ep.flush(user, false).then(() => { + ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(1); const output = mockEventSender.calls[0].events; expect(output.length).toEqual(2); @@ -278,12 +383,12 @@ describe('EventProcessor', () => { }); it('can include inline user in custom event', done => { - const config = { inline_users_in_events: true }; - const ep = EventProcessor(config, eventsUrl, mockEventSender); + const config = { inlineUsersInEvents: true }; + const ep = EventProcessor(config, eventsUrl, null, mockEventSender); const e = { kind: 'custom', creationDate: 1000, user: user, key: 'eventkey', data: { thing: 'stuff' } }; ep.enqueue(e); - ep.flush(user, false).then(() => { + ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(1); const output = mockEventSender.calls[0].events; expect(output.length).toEqual(1); @@ -293,12 +398,12 @@ describe('EventProcessor', () => { }); it('filters user in custom event', done => { - const config = { all_attributes_private: true, inline_users_in_events: true }; - const ep = EventProcessor(config, eventsUrl, mockEventSender); + const config = { all_attributes_private: true, inlineUsersInEvents: true }; + const ep = EventProcessor(config, eventsUrl, null, mockEventSender); const e = { kind: 'custom', creationDate: 1000, user: user, key: 'eventkey', data: { thing: 'stuff' } }; ep.enqueue(e); - ep.flush(user, false).then(() => { + ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(1); const output = mockEventSender.calls[0].events; expect(output.length).toEqual(1); @@ -307,9 +412,26 @@ describe('EventProcessor', () => { }); }); - it('sends nothing if there are no events to flush', () => { - const processor = EventProcessor({}, '/fake-url', mockEventSender); - processor.flush(user, false); - expect(mockEventSender.calls.length).toEqual(0); + it('sends nothing if there are no events to flush', done => { + const ep = EventProcessor({}, '/fake-url', null, mockEventSender); + ep.flush().then(() => { + expect(mockEventSender.calls.length).toEqual(0); + done(); + }); + }); + + it('stops sending events after a 401 error', done => { + const ep = EventProcessor({}, '/fake-url', null, mockEventSender); + const e = { kind: 'identify', creationDate: 1000, user: user }; + ep.enqueue(e); + mockEventSender.status = 401; + ep.flush().then(() => { + expect(mockEventSender.calls.length).toEqual(1); + ep.enqueue(e); + ep.flush().then(() => { + expect(mockEventSender.calls.length).toEqual(1); // still the one from our first flush + done(); + }); + }); }); }); diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index f92f5e81..b3a520b4 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -341,6 +341,17 @@ describe('LDClient', () => { done(); }, 0); }); + + it('should warn about missing user on first event', () => { + const sandbox = sinon.sandbox.create(); + const warnSpy = sandbox.spy(console, 'warn'); + const client = LDClient.initialize(envName, null); + client.track('eventkey', null); + warnSpy.restore(); + sandbox.restore(); + expect(warnSpy.called).toEqual(true); + }); + }); describe('event listening', () => { diff --git a/src/index.js b/src/index.js index a4a38f17..6224fcb8 100644 --- a/src/index.js +++ b/src/index.js @@ -23,15 +23,15 @@ function initialize(env, user, options = {}) { const environment = env; const emitter = EventEmitter(); const stream = Stream(streamUrl, environment, hash, options.useReport); - const events = EventProcessor(options, eventsUrl + '/a/' + environment + '.gif'); + const events = EventProcessor(options, eventsUrl + '/a/' + environment + '.gif', emitter); const requestor = Requestor(baseUrl, environment, options.useReport); const seenRequests = {}; - let samplingInterval = parseInt(options.samplingInterval, 10) || 0; let flags = typeof options.bootstrap === 'object' ? utils.transformValuesToVersionedValues(options.bootstrap) : {}; let goalTracker; let useLocalStorage; let goals; let subscribedToChangeEvents; + let firstEvent = true; function lsKey(env, user) { let key = ''; @@ -42,24 +42,36 @@ function initialize(env, user, options = {}) { } function shouldEnqueueEvent() { - return ( - sendEvents && !doNotTrack() && (samplingInterval === 0 || Math.floor(Math.random() * samplingInterval) === 0) - ); + return sendEvents && !doNotTrack(); } function enqueueEvent(event) { + if (!event.user) { + if (firstEvent) { + if (console && console.warn) { + console.warn( + 'Be sure to call `identify` in the LaunchDarkly client: http://docs.launchdarkly.com/docs/running-an-ab-test#include-the-client-side-snippet' + ); + } + firstEvent = false; + } + return; + } + firstEvent = false; if (shouldEnqueueEvent()) { events.enqueue(event); } } function sendIdentifyEvent(user) { - enqueueEvent({ - kind: 'identify', - key: user.key, - user: user, - creationDate: new Date().getTime(), - }); + if (user) { + enqueueEvent({ + kind: 'identify', + key: user.key, + user: user, + creationDate: new Date().getTime() + }); + } } const ident = Identity(user, sendIdentifyEvent); @@ -77,14 +89,23 @@ function initialize(env, user, options = {}) { seenRequests[cacheKey] = now; - enqueueEvent({ + const event = { kind: 'feature', key: key, user: user, value: value, default: defaultValue, creationDate: now.getTime(), - }); + }; + const flag = flags[key]; + if (flag) { + event.version = flag.version; + event.variation = flag.variation; + event.trackEvents = flag.trackEvents; + event.debugEventsUntilDate = flag.debugEventsUntilDate; + } + + enqueueEvent(event); } function sendGoalEvent(kind, goal) { @@ -128,7 +149,7 @@ function initialize(env, user, options = {}) { function flush(onDone) { return utils.wrapPromiseCallback( - new Promise(resolve => (sendEvents ? resolve(events.flush(ident.getUser())) : resolve()), onDone) + new Promise(resolve => (sendEvents ? resolve(events.flush()) : resolve()), onDone) ); } @@ -340,17 +361,6 @@ function initialize(env, user, options = {}) { } } - if (options.samplingInterval !== undefined && (isNaN(options.samplingInterval) || options.samplingInterval < 0)) { - samplingInterval = 0; - utils.onNextTick(() => { - emitter.maybeReportError( - new errors.LDInvalidArgumentError( - 'Invalid sampling interval configured. Sampling interval must be an integer >= 0.' - ) - ); - }); - } - if (!env) { utils.onNextTick(() => { emitter.maybeReportError(new errors.LDInvalidEnvironmentIdError(messages.environmentNotSpecified())); @@ -472,10 +482,7 @@ function initialize(env, user, options = {}) { function start() { if (sendEvents) { - setTimeout(function tick() { - events.flush(ident.getUser()); - setTimeout(tick, flushInterval); - }, flushInterval); + events.start(); } } @@ -486,7 +493,10 @@ function initialize(env, user, options = {}) { } window.addEventListener('beforeunload', () => { - events.flush(ident.getUser(), true); + if (sendEvents) { + events.stop(); + events.flush(true); + } }); window.addEventListener('message', handleMessage); From 34f3ab9478b1124f1f5dfa519794b39710fa04d0 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 11 Apr 2018 17:09:29 -0700 Subject: [PATCH 09/59] added log warning for 401 error --- src/EventProcessor.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/EventProcessor.js b/src/EventProcessor.js index c4076a21..3f50a63c 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -154,6 +154,11 @@ export default function EventProcessor(options, eventsUrl, emitter, sender) { } if (responseInfo.status === 401) { disabled = true; + utils.onNextTick(() => { + emitter.maybeReportError( + new errors.LDUnexpectedResponseError("Received 401 error, no further events will be posted") + ); + }); } } }); From 3cf5f0e8842a5816cf7babfba4beb1e996e1beaa Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 11 Apr 2018 17:11:14 -0700 Subject: [PATCH 10/59] capture all properties from stream patch event --- src/index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 6224fcb8..b17ac780 100644 --- a/src/index.js +++ b/src/index.js @@ -253,7 +253,9 @@ function initialize(env, user, options = {}) { if (!flags[data.key] || flags[data.key].version < data.version) { const mods = {}; const oldFlag = flags[data.key]; - flags[data.key] = { version: data.version, value: data.value }; + const newFlag = Object.assign({}, data); + delete newFlag['key']; + flags[data.key] = newFlag; if (oldFlag) { mods[data.key] = { previous: oldFlag.value, current: data.value }; } else { From 65fd04c842635eb0eac74784e13681ecc24f4747 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 11 Apr 2018 17:13:15 -0700 Subject: [PATCH 11/59] rm unused --- src/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/index.js b/src/index.js index b17ac780..0a7a442d 100644 --- a/src/index.js +++ b/src/index.js @@ -11,7 +11,6 @@ import * as errors from './errors'; const readyEvent = 'ready'; const changeEvent = 'change'; -const flushInterval = 2000; const locationWatcherInterval = 300; function initialize(env, user, options = {}) { From 9c52063062a9995dc06a354208b508f8b9b21bb6 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 12 Apr 2018 13:07:41 -0700 Subject: [PATCH 12/59] summary events (6): use new schema for eval endpoints and local storage --- src/Requestor.js | 11 ++--- src/__tests__/LDClient-test.js | 33 ++++++++++----- src/__tests__/store-test.js | 13 ++++-- src/index.js | 36 +++++++---------- src/store.js | 73 ++++++++++++++++++++++++---------- src/utils.js | 14 ------- 6 files changed, 101 insertions(+), 79 deletions(-) diff --git a/src/Requestor.js b/src/Requestor.js index b68a92c6..46fcb16c 100644 --- a/src/Requestor.js +++ b/src/Requestor.js @@ -47,21 +47,16 @@ export default function Requestor(baseUrl, environment, useReport) { let cb; if (useReport) { - endpoint = [baseUrl, '/sdk/eval/', environment, '/user', hash ? '?h=' + hash : ''].join(''); + endpoint = [baseUrl, '/sdk/evalx/', environment, '/user', hash ? '?h=' + hash : ''].join(''); body = user; } else { data = utils.base64URLEncode(JSON.stringify(user)); - endpoint = [baseUrl, '/sdk/eval/', environment, '/users/', data, hash ? '?h=' + hash : ''].join(''); + endpoint = [baseUrl, '/sdk/evalx/', environment, '/users/', data, hash ? '?h=' + hash : ''].join(''); } const wrappedCallback = (function(currentCallback) { return function(error, result) { - let finalResult = result; - // if we got flags, convert them to the more verbose format used by the eval stream - if (result) { - finalResult = utils.transformValuesToVersionedValues(result); - } - currentCallback(error, finalResult); + currentCallback(error, result); flagSettingsRequest = null; lastFlagSettingsCallback = null; }; diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index b3a520b4..47435259 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -163,10 +163,13 @@ describe('LDClient', () => { bootstrap: 'localstorage', }); + expect(window.localStorage.getItem(lsKey)).toBeNull(); + client.on('ready', () => { - expect(window.localStorage.getItem(lsKey)).toBeNull(); done(); }); + + requests[0].respond(200, { 'Content-Type': 'application/json' }, '{"enable-foo": {"value": true, "version": 1}}'); }); it('should not clear cached settings if they are valid JSON', done => { @@ -251,16 +254,18 @@ describe('LDClient', () => { hash: 'totallyLegitHash', }); - requests[0].respond(200, { 'Content-Type': 'application/json' }, '{"enable-foo": true}'); - client.on('ready', () => { - expect(JSON.parse(window.localStorage.getItem(lsKeyHash))).toEqual({ 'enable-foo': true }); + expect(JSON.parse(window.localStorage.getItem(lsKeyHash))).toEqual( + { $schema: 1, 'enable-foo': { value: true, version: 1 } } + ); done(); }); + + requests[0].respond(200, { 'Content-Type': 'application/json' }, '{"enable-foo":{"value":true,"version":1}}'); }); it('should clear localStorage when user context is changed', done => { - const json = '{"enable-foo":true}'; + const json = '{"enable-foo":{"value":true,"version":1}}'; const lsKey2 = 'ld:UNKNOWN_ENVIRONMENT_ID:' + btoa('{"key":"user2"}'); const user2 = { key: 'user2' }; @@ -274,7 +279,9 @@ describe('LDClient', () => { client.on('ready', () => { client.identify(user2, null, () => { expect(window.localStorage.getItem(lsKey)).toBeNull(); - expect(JSON.parse(window.localStorage.getItem(lsKey2))).toEqual({ 'enable-foo': true }); + expect(JSON.parse(window.localStorage.getItem(lsKey2))).toEqual( + { $schema: 1, 'enable-foo': { value: true, version: 1 } } + ); done(); }); server.respond(); @@ -402,7 +409,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.ping(); - getLastRequest().respond(200, { 'Content-Type': 'application/json' }, '{"enable-foo": true}'); + getLastRequest().respond(200, { 'Content-Type': 'application/json' }, '{"enable-foo":{"value":true,"version":1}}'); expect(client.variation('enable-foo')).toEqual(true); done(); }); @@ -435,7 +442,9 @@ describe('LDClient', () => { }); expect(client.variation('enable-foo')).toEqual(true); - expect(window.localStorage.getItem(lsKey)).toEqual('{"enable-foo":true}'); + expect(JSON.parse(window.localStorage.getItem(lsKey))).toEqual( + { $schema: 1, 'enable-foo': { value: true, version: 1 } } + ); done(); }); }); @@ -502,7 +511,9 @@ describe('LDClient', () => { }); expect(client.variation('enable-foo')).toEqual(true); - expect(window.localStorage.getItem(lsKey)).toEqual('{"enable-foo":true}'); + expect(JSON.parse(window.localStorage.getItem(lsKey))).toEqual( + { $schema: 1, 'enable-foo': { value: true, version: 1 } } + ); done(); }); }); @@ -639,7 +650,9 @@ describe('LDClient', () => { }); expect(client.variation('enable-foo')).toEqual(undefined); - expect(window.localStorage.getItem(lsKey)).toEqual('{}'); + expect(JSON.parse(window.localStorage.getItem(lsKey))).toEqual( + { $schema: 1, 'enable-foo': { version: 1, deleted: true } } + ); done(); }); }); diff --git a/src/__tests__/store-test.js b/src/__tests__/store-test.js index 0a372dd0..9da5efbf 100644 --- a/src/__tests__/store-test.js +++ b/src/__tests__/store-test.js @@ -1,15 +1,19 @@ -import store from '../store'; +import Identity from '../Identity'; +import Store from '../Store'; import * as messages from '../messages'; -describe('store', () => { +describe('Store', () => { + const ident = Identity(null); + it('should handle localStorage getItem throwing an exception', () => { + const store = Store('env', 'hash', ident); const getItemSpy = jest.spyOn(localStorage, 'getItem').mockImplementation(() => { throw new Error('localstorage getitem error'); }); const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - store.get('foo'); + store.loadFlags(); expect(consoleWarnSpy).toHaveBeenCalledWith(messages.localStorageUnavailable()); consoleWarnSpy.mockRestore(); @@ -17,13 +21,14 @@ describe('store', () => { }); it('should handle localStorage setItem throwing an exception', () => { + const store = Store('env', 'hash', ident); const setItemSpy = jest.spyOn(localStorage, 'setItem').mockImplementation(() => { throw new Error('localstorage getitem error'); }); const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); - store.set('foo', 'bar'); + store.saveFlags({ foo: {} }); expect(consoleWarnSpy).toHaveBeenCalledWith(messages.localStorageUnavailable()); consoleWarnSpy.mockRestore(); diff --git a/src/index.js b/src/index.js index 0a7a442d..44cc5394 100644 --- a/src/index.js +++ b/src/index.js @@ -1,10 +1,10 @@ import EventProcessor from './EventProcessor'; import EventEmitter from './EventEmitter'; import GoalTracker from './GoalTracker'; +import Store from './Store'; import Stream from './Stream'; import Requestor from './Requestor'; import Identity from './Identity'; -import store from './store'; import * as utils from './utils'; import * as messages from './messages'; import * as errors from './errors'; @@ -32,14 +32,6 @@ function initialize(env, user, options = {}) { let subscribedToChangeEvents; let firstEvent = true; - function lsKey(env, user) { - let key = ''; - if (user) { - key = hash || utils.btoa(JSON.stringify(user)); - } - return 'ld:' + env + ':' + key; - } - function shouldEnqueueEvent() { return sendEvents && !doNotTrack(); } @@ -74,7 +66,7 @@ function initialize(env, user, options = {}) { } const ident = Identity(user, sendIdentifyEvent); - let localStorageKey = lsKey(environment, ident.getUser()); + const store = Store(environment, hash, ident); function sendFlagEvent(key, value, defaultValue) { const user = ident.getUser(); @@ -125,6 +117,7 @@ function initialize(env, user, options = {}) { } function identify(user, hash, onDone) { + store.clearFlags(); return utils.wrapPromiseCallback( new Promise((resolve, reject) => { ident.setUser(user); @@ -307,9 +300,7 @@ function initialize(env, user, options = {}) { const keys = Object.keys(changes); if (useLocalStorage) { - store.clear(localStorageKey); - localStorageKey = lsKey(environment, ident.getUser()); - store.set(localStorageKey, JSON.stringify(utils.transformValuesToUnversionedValues(flags))); + store.saveFlags(flags); } if (keys.length > 0) { @@ -389,20 +380,19 @@ function initialize(env, user, options = {}) { ) { useLocalStorage = true; - // check if localStorage data is corrupted, if so clear it - try { - flags = utils.transformValuesToVersionedValues(JSON.parse(store.get(localStorageKey))); - } catch (error) { - store.clear(localStorageKey); - } + flags = store.loadFlags(); if (flags === null) { requestor.fetchFlagSettings(ident.getUser(), hash, (err, settings) => { if (err) { emitter.maybeReportError(new errors.LDFlagFetchError(messages.errorFetchingFlags(err))); } - flags = settings; - settings && store.set(localStorageKey, JSON.stringify(utils.transformValuesToUnversionedValues(flags))); + if (settings) { + flags = settings; + store.saveFlags(flags); + } else { + flags = {}; + } emitter.emit(readyEvent); }); } else { @@ -417,7 +407,9 @@ function initialize(env, user, options = {}) { if (err) { emitter.maybeReportError(new errors.LDFlagFetchError(messages.errorFetchingFlags(err))); } - settings && store.set(localStorageKey, JSON.stringify(utils.transformValuesToUnversionedValues(settings))); + if (settings) { + store.saveFlags(settings); + } }); } } else { diff --git a/src/store.js b/src/store.js index 81617f2c..28321373 100644 --- a/src/store.js +++ b/src/store.js @@ -1,27 +1,58 @@ import * as messages from './messages'; +import * as utils from './utils'; -function get(key) { - try { - return localStorage.getItem(key); - } catch (ex) { - console.warn(messages.localStorageUnavailable()); - } -} +export default function Store(environment, hash, ident) { + const store = {}; -function set(key, item) { - try { - localStorage.setItem(key, item); - } catch (ex) { - console.warn(messages.localStorageUnavailable()); + function getFlagsKey() { + let key = ''; + const user = ident.getUser(); + if (user) { + key = hash || utils.btoa(JSON.stringify(user)); + } + return 'ld:' + environment + ':' + key; } -} -function clear(key) { - localStorage.removeItem(key); -} + store.loadFlags = function() { + const key = getFlagsKey(); + let dataStr, data; + try { + dataStr = localStorage.getItem(key); + } catch (ex) { + console.warn(messages.localStorageUnavailable()); + return null; + } + try { + data = JSON.parse(dataStr); + } catch (ex) { + store.clearFlags(); + return null; + } + if (data) { + const schema = data.$schema; + if (schema === undefined || schema < 1) { + data = utils.transformValuesToVersionedValues(data); + } + } + return data; + }; -export default { - get: get, - set: set, - clear: clear, -}; + store.saveFlags = function(flags) { + const key = getFlagsKey(); + const data = Object.assign({}, flags, { $schema: 1 }); + try { + localStorage.setItem(key, JSON.stringify(data)); + } catch (ex) { + console.warn(messages.localStorageUnavailable()); + } + }; + + store.clearFlags = function() { + const key = getFlagsKey(); + try { + localStorage.removeItem(key); + } catch (ex) {} + }; + + return store; +} diff --git a/src/utils.js b/src/utils.js index 4d85fdb8..34c56b18 100644 --- a/src/utils.js +++ b/src/utils.js @@ -73,20 +73,6 @@ export function transformValuesToVersionedValues(flags) { return ret; } -/** - * Takes a map obtained from the client stream and converts it to the briefer format used in - * bootstrap data or local storagel - */ -export function transformValuesToUnversionedValues(flags) { - const ret = {}; - for (const key in flags) { - if (flags.hasOwnProperty(key)) { - ret[key] = flags[key].value; - } - } - return ret; -} - /** * Returns an array of event groups each of which can be safely URL-encoded * without hitting the safe maximum URL length of certain browsers. From 6908719fa3c6afed639f248f445b849626626ec5 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 12 Apr 2018 13:12:04 -0700 Subject: [PATCH 13/59] rename file --- src/{store.js => Store.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/{store.js => Store.js} (100%) diff --git a/src/store.js b/src/Store.js similarity index 100% rename from src/store.js rename to src/Store.js From 79dfe7b3abce63ec04613d4ff50d9df7d3830621 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 12 Apr 2018 13:14:35 -0700 Subject: [PATCH 14/59] add guard --- src/index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 44cc5394..487059c4 100644 --- a/src/index.js +++ b/src/index.js @@ -117,7 +117,9 @@ function initialize(env, user, options = {}) { } function identify(user, hash, onDone) { - store.clearFlags(); + if (useLocalStorage) { + store.clearFlags(); + } return utils.wrapPromiseCallback( new Promise((resolve, reject) => { ident.setUser(user); From 93e8805da6851dee2894212de4ce0fe4ea819d38 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 12 Apr 2018 14:15:20 -0700 Subject: [PATCH 15/59] camelcase config option --- src/EventProcessor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EventProcessor.js b/src/EventProcessor.js index f8d68afd..bb7c0e2c 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -46,7 +46,7 @@ export default function EventProcessor(options, eventsUrl) { const summarizer = EventSummarizer(); const userKeysCache = LRU(options.user_keys_capacity || 1000); const userFilter = UserFilter(options); - const inlineUsers = !!options.inline_users_in_events; + const inlineUsers = !!options.inlineUsersInEvents; let queue = []; let initialFlush = true; From 43e3ff8584e1daa3a7d767b17576f77bb9f6e6f8 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 12 Apr 2018 14:16:13 -0700 Subject: [PATCH 16/59] camelcase option --- src/__tests__/EventProcessor-test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/__tests__/EventProcessor-test.js b/src/__tests__/EventProcessor-test.js index f1a1be5c..e39038ae 100644 --- a/src/__tests__/EventProcessor-test.js +++ b/src/__tests__/EventProcessor-test.js @@ -173,7 +173,7 @@ describe('EventProcessor', () => { }); it('can include inline user in feature event', done => { - const config = { inline_users_in_events: true }; + const config = { inlineUsersInEvents: true }; const ep = EventProcessor(config, eventsUrl, mockEventSender); const event = { kind: 'feature', @@ -193,7 +193,7 @@ describe('EventProcessor', () => { }); it('filters user in feature event', done => { - const config = { all_attributes_private: true, inline_users_in_events: true }; + const config = { all_attributes_private: true, inlineUsersInEvents: true }; const ep = EventProcessor(config, eventsUrl, mockEventSender); const event = { kind: 'feature', @@ -278,7 +278,7 @@ describe('EventProcessor', () => { }); it('can include inline user in custom event', done => { - const config = { inline_users_in_events: true }; + const config = { inlineUsersInEvents: true }; const ep = EventProcessor(config, eventsUrl, mockEventSender); const e = { kind: 'custom', creationDate: 1000, user: user, key: 'eventkey', data: { thing: 'stuff' } }; @@ -293,7 +293,7 @@ describe('EventProcessor', () => { }); it('filters user in custom event', done => { - const config = { all_attributes_private: true, inline_users_in_events: true }; + const config = { all_attributes_private: true, inlineUsersInEvents: true }; const ep = EventProcessor(config, eventsUrl, mockEventSender); const e = { kind: 'custom', creationDate: 1000, user: user, key: 'eventkey', data: { thing: 'stuff' } }; From fc78d93156a729d8f2b6fb89738ac3ede18ce3b1 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 12 Apr 2018 14:34:38 -0700 Subject: [PATCH 17/59] misc cleanup --- src/UserFilter.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/UserFilter.js b/src/UserFilter.js index c235339a..4653d78a 100644 --- a/src/UserFilter.js +++ b/src/UserFilter.js @@ -14,16 +14,15 @@ export default function UserFilter(config) { firstName: true, lastName: true, avatar: true, name: true, anonymous: true, custom: true }; - filter.filterUser = function (user) { - let allPrivateAttrs = {}; - let userPrivateAttrs = user.privateAttributeNames || []; + filter.filterUser = function(user) { + const userPrivateAttrs = user.privateAttributeNames || []; - const isPrivateAttr = function (name) { + const isPrivateAttr = function(name) { return !ignoreAttrs[name] && ( allAttributesPrivate || userPrivateAttrs.indexOf(name) !== -1 || privateAttributeNames.indexOf(name) !== -1); } - const filterAttrs = function (props, isAttributeAllowed) { + const filterAttrs = function(props, isAttributeAllowed) { return Object.keys(props).reduce(function (acc, name) { if (isAttributeAllowed(name)) { if (isPrivateAttr(name)) { @@ -36,20 +35,20 @@ export default function UserFilter(config) { return acc; }, [{}, {}]); } - let result = filterAttrs(user, function (key) { return allowedTopLevelAttrs[key]; }); - let filteredProps = result[0]; - let removedAttrs = result[1]; + const result = filterAttrs(user, key => allowedTopLevelAttrs[key]); + const filteredProps = result[0]; + const removedAttrs = result[1]; if (user.custom) { - var customResult = filterAttrs(user.custom, function (key) { return true; }); + const customResult = filterAttrs(user.custom, () => true); filteredProps.custom = customResult[0]; Object.assign(removedAttrs, customResult[1]); } - var removedAttrNames = Object.keys(removedAttrs); + const removedAttrNames = Object.keys(removedAttrs); if (removedAttrNames.length) { removedAttrNames.sort(); filteredProps.privateAttrs = removedAttrNames; } return filteredProps; - } + }; return filter; } \ No newline at end of file From 0ab820fcf6b712a7303d250c451098e121afb3cf Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 12 Apr 2018 14:41:22 -0700 Subject: [PATCH 18/59] deprecate snake-cased option names, add camelcased equivalents --- src/UserFilter.js | 19 +++++++++++++++---- src/__tests__/UserFilter-test.js | 32 +++++++++++++++++++++++++++----- src/messages.js | 4 ++++ 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/UserFilter.js b/src/UserFilter.js index 4653d78a..346b131d 100644 --- a/src/UserFilter.js +++ b/src/UserFilter.js @@ -1,3 +1,5 @@ +import * as messages from './messages'; + /** * The UserFilter object transforms user objects into objects suitable to be sent as JSON to * the server, hiding any private user attributes. @@ -6,14 +8,23 @@ **/ export default function UserFilter(config) { const filter = {}; - const allAttributesPrivate = config.all_attributes_private; - const privateAttributeNames = config.private_attribute_names || []; + const allAttributesPrivate = + config.allAttributesPrivate !== undefined ? config.allAttributesPrivate : config.all_attributes_private; + const privateAttributeNames = + (config.privateAttributeNames !== undefined ? config.privateAttributeNames : config.private_attribute_names) || []; const ignoreAttrs = { key: true, custom: true, anonymous: true }; const allowedTopLevelAttrs = { key: true, secondary: true, ip: true, country: true, email: true, firstName: true, lastName: true, avatar: true, name: true, anonymous: true, custom: true }; + if (config.all_attributes_private !== undefined) { + console && console.warn && console.warn(messages.deprecated('all_attributes_private', 'allAttributesPrivate')); + } + if (config.private_attribute_names !== undefined) { + console && console.warn && console.warn(messages.deprecated('private_attribute_names', 'privateAttributeNames')); + } + filter.filterUser = function(user) { const userPrivateAttrs = user.privateAttributeNames || []; @@ -21,7 +32,7 @@ export default function UserFilter(config) { return !ignoreAttrs[name] && ( allAttributesPrivate || userPrivateAttrs.indexOf(name) !== -1 || privateAttributeNames.indexOf(name) !== -1); - } + }; const filterAttrs = function(props, isAttributeAllowed) { return Object.keys(props).reduce(function (acc, name) { if (isAttributeAllowed(name)) { @@ -34,7 +45,7 @@ export default function UserFilter(config) { } return acc; }, [{}, {}]); - } + }; const result = filterAttrs(user, key => allowedTopLevelAttrs[key]); const filteredProps = result[0]; const removedAttrs = result[1]; diff --git a/src/__tests__/UserFilter-test.js b/src/__tests__/UserFilter-test.js index 7545c58c..16a4e179 100644 --- a/src/__tests__/UserFilter-test.js +++ b/src/__tests__/UserFilter-test.js @@ -1,6 +1,8 @@ import UserFilter from '../UserFilter'; describe('UserFilter', () => { + let warnSpy; + // users to serialize const user = { 'key': 'abc', @@ -60,19 +62,39 @@ describe('UserFilter', () => { 'privateAttrs': [ 'bizzle', 'dizzle' ] }; + beforeEach(() => { + warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + warnSpy.mockRestore(); + }); + it('includes all user attributes by default', () => { const uf = UserFilter({}); expect(uf.filterUser(user)).toEqual(user); }); - it('hides all except key if all_attrs_private is true', () => { - const uf = UserFilter({ all_attributes_private: true}); + it('hides all except key if allAttributesPrivate is true', () => { + const uf = UserFilter({ allAttributesPrivate: true }); + expect(uf.filterUser(user)).toEqual(userWithAllAttrsHidden); + }); + + it('allows all_attributes_private as deprecated synonym for allAttributesPrivate', () => { + const uf = UserFilter({ all_attributes_private: true }); expect(uf.filterUser(user)).toEqual(userWithAllAttrsHidden); + expect(warnSpy).toHaveBeenCalled(); + }); + + it('hides some attributes if privateAttributeNames is set', () => { + const uf = UserFilter({ privateAttributeNames: [ 'firstName', 'bizzle' ]}); + expect(uf.filterUser(user)).toEqual(userWithSomeAttrsHidden); }); - it('hides some attributes if private_attr_names is set', () => { + it('allows private_attribute_names as deprecated synonym for privateAttributeNames', () => { const uf = UserFilter({ private_attribute_names: [ 'firstName', 'bizzle' ]}); expect(uf.filterUser(user)).toEqual(userWithSomeAttrsHidden); + expect(warnSpy).toHaveBeenCalled(); }); it('hides attributes specified in per-user privateAttrs', () => { @@ -81,7 +103,7 @@ describe('UserFilter', () => { }); it('looks at both per-user privateAttrs and global config', () => { - const uf = UserFilter({ private_attribute_names: [ 'firstName', 'bizzle' ]}); + const uf = UserFilter({ privateAttributeNames: [ 'firstName', 'bizzle' ]}); expect(uf.filterUser(userSpecifyingOwnPrivateAttr)).toEqual(userWithAllAttrsHidden); }); @@ -91,7 +113,7 @@ describe('UserFilter', () => { }); it('leaves the "anonymous" attribute as is', () => { - const uf = UserFilter({ all_attributes_private: true}); + const uf = UserFilter({ allAttributesPrivate: true }); expect(uf.filterUser(anonUser)).toEqual(anonUserWithAllAttrsHidden); }); }); diff --git a/src/messages.js b/src/messages.js index 984156d8..43cad4a4 100644 --- a/src/messages.js +++ b/src/messages.js @@ -36,3 +36,7 @@ export const userNotSpecified = function() { export const invalidUser = function() { return 'Invalid user specified.' + docLink; }; + +export const deprecated = function(oldName, newName) { + return '[LaunchDarkly] "' + oldName + '" is deprecated, please use "' + newName + '"'; +}; From 176123f34f2dcaee4c86888dac76e06f56a81ba4 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 16 Apr 2018 12:02:06 -0700 Subject: [PATCH 19/59] don't generate index events - so, don't need user keys cache --- package-lock.json | 3 ++- package.json | 3 +-- src/EventProcessor.js | 21 --------------------- 3 files changed, 3 insertions(+), 24 deletions(-) diff --git a/package-lock.json b/package-lock.json index 73d1fb66..26fa20be 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4449,7 +4449,8 @@ "inherits": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/inherits/-/inherits-2.0.3.tgz", - "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=" + "integrity": "sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4=", + "dev": true }, "inquirer": { "version": "3.3.0", diff --git a/package.json b/package.json index 1991acb7..6999941e 100755 --- a/package.json +++ b/package.json @@ -72,8 +72,7 @@ }, "dependencies": { "Base64": "1.0.1", - "escape-string-regexp": "1.0.5", - "lru": "3.1.0" + "escape-string-regexp": "1.0.5" }, "repository": { "type": "git", diff --git a/src/EventProcessor.js b/src/EventProcessor.js index bb7c0e2c..80dadaab 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -1,4 +1,3 @@ -import LRU from 'lru'; import EventSummarizer from './EventSummarizer'; import UserFilter from './UserFilter'; import * as utils from './utils'; @@ -44,7 +43,6 @@ function sendEvents(eventsUrl, events, sync) { export default function EventProcessor(options, eventsUrl) { const processor = {}; const summarizer = EventSummarizer(); - const userKeysCache = LRU(options.user_keys_capacity || 1000); const userFilter = UserFilter(options); const inlineUsers = !!options.inlineUsersInEvents; let queue = []; @@ -67,25 +65,6 @@ export default function EventProcessor(options, eventsUrl) { // Add event to the summary counters if appropriate summarizer.summarizeEvent(event); - // For each user we haven't seen before, we add an index event - unless this is already - // an identify event for that user. - let addIndexEvent = false; - if (!inlineUsers) { - if (event.user && !userKeysCache.get(event.user.key)) { - userKeysCache.set(event.user.key, true); - if (event.kind !== 'identify') { - addIndexEvent = true; - } - } - } - - if (addIndexEvent) { - queue.push({ - kind: 'index', - creationDate: event.creationDate, - user: userFilter.filterUser(event.user), - }); - } queue.push(makeOutputEvent(event)); }; From 22b22dd43ec454d36abc622bcf2925d416663c42 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 16 Apr 2018 12:56:32 -0700 Subject: [PATCH 20/59] options should be last parameter --- src/EventProcessor.js | 2 +- src/__tests__/EventProcessor-test.js | 6 +++--- src/index.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/EventProcessor.js b/src/EventProcessor.js index d1ade545..4066369e 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -39,7 +39,7 @@ function sendEvents(eventsUrl, events, sync) { } } -export default function EventProcessor(options, eventsUrl) { +export default function EventProcessor(eventsUrl, options = {}) { const processor = {}; const userFilter = UserFilter(options); let queue = []; diff --git a/src/__tests__/EventProcessor-test.js b/src/__tests__/EventProcessor-test.js index beab229e..75c033e4 100644 --- a/src/__tests__/EventProcessor-test.js +++ b/src/__tests__/EventProcessor-test.js @@ -26,14 +26,14 @@ describe('EventProcessor', () => { it('should warn about missing user on initial flush', () => { const warnSpy = sandbox.spy(console, 'warn'); - const processor = EventProcessor({}, '/fake-url'); + const processor = EventProcessor('/fake-url'); processor.flush(null); warnSpy.restore(); expect(warnSpy.called).toEqual(true); }); it('should flush asynchronously', () => { - const processor = EventProcessor({}, '/fake-url'); + const processor = EventProcessor('/fake-url'); const user = { key: 'foo' }; const event = { kind: 'identify', key: user.key }; @@ -49,7 +49,7 @@ describe('EventProcessor', () => { }); it('should flush synchronously', () => { - const processor = EventProcessor({}, '/fake-url'); + const processor = EventProcessor('/fake-url'); const user = { key: 'foo' }; const event = { kind: 'identify', key: user.key }; diff --git a/src/index.js b/src/index.js index a4a38f17..fc21db3f 100644 --- a/src/index.js +++ b/src/index.js @@ -23,7 +23,7 @@ function initialize(env, user, options = {}) { const environment = env; const emitter = EventEmitter(); const stream = Stream(streamUrl, environment, hash, options.useReport); - const events = EventProcessor(options, eventsUrl + '/a/' + environment + '.gif'); + const events = EventProcessor(eventsUrl + '/a/' + environment + '.gif', options); const requestor = Requestor(baseUrl, environment, options.useReport); const seenRequests = {}; let samplingInterval = parseInt(options.samplingInterval, 10) || 0; From 9c83d6a09be132cf1fc3ad7e0c24057cd89d1454 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 16 Apr 2018 13:01:17 -0700 Subject: [PATCH 21/59] rm unused --- package-lock.json | 8 -------- 1 file changed, 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index 26fa20be..e3e4271f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6388,14 +6388,6 @@ "js-tokens": "3.0.2" } }, - "lru": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/lru/-/lru-3.1.0.tgz", - "integrity": "sha1-6n+4VG2DczOWoTCR12z+tMBoN9U=", - "requires": { - "inherits": "2.0.3" - } - }, "lru-cache": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/lru-cache/-/lru-cache-4.1.1.tgz", From decf2154ac3fd623a3afba413321b6115b3217a2 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 16 Apr 2018 14:59:12 -0700 Subject: [PATCH 22/59] minor cleanup --- src/EventSummarizer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/EventSummarizer.js b/src/EventSummarizer.js index 7e341ea3..f29ffe27 100644 --- a/src/EventSummarizer.js +++ b/src/EventSummarizer.js @@ -55,8 +55,8 @@ export default function EventSummarizer() { empty = false; } return empty ? null : { - startDate: startDate, - endDate: endDate, + startDate, + endDate, features: flagsOut, }; }; From e1c52676e94dae133fb0e8706c21ba6293d89af3 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 18 Apr 2018 11:27:43 -0700 Subject: [PATCH 23/59] linter fixes --- src/EventProcessor.js | 3 ++- src/EventSummarizer.js | 14 +++++++------ src/UserFilter.js | 46 +++++++++++++++++++++++++++--------------- 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/EventProcessor.js b/src/EventProcessor.js index 771aa62d..63666b9a 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -15,7 +15,8 @@ export default function EventProcessor(eventsUrl, options = {}, sender = null) { if (!e.user) { return e; } - if (inlineUsers || e.kind === 'identify') { // identify events always have an inline user + if (inlineUsers || e.kind === 'identify') { + // identify events always have an inline user return Object.assign({}, e, { user: userFilter.filterUser(e.user) }); } else { const ret = Object.assign({}, e, { userKey: e.user.key }); diff --git a/src/EventSummarizer.js b/src/EventSummarizer.js index f29ffe27..3adba9e5 100644 --- a/src/EventSummarizer.js +++ b/src/EventSummarizer.js @@ -32,7 +32,7 @@ export default function EventSummarizer() { es.getSummary = function() { const flagsOut = {}; let empty = true; - for (let i in counters) { + for (const i in counters) { const c = counters[i]; let flag = flagsOut[c.key]; if (!flag) { @@ -54,11 +54,13 @@ export default function EventSummarizer() { flag.counters.push(counterOut); empty = false; } - return empty ? null : { - startDate, - endDate, - features: flagsOut, - }; + return empty + ? null + : { + startDate, + endDate, + features: flagsOut, + }; }; es.clearSummary = function() { diff --git a/src/UserFilter.js b/src/UserFilter.js index 346b131d..7704b094 100644 --- a/src/UserFilter.js +++ b/src/UserFilter.js @@ -14,8 +14,17 @@ export default function UserFilter(config) { (config.privateAttributeNames !== undefined ? config.privateAttributeNames : config.private_attribute_names) || []; const ignoreAttrs = { key: true, custom: true, anonymous: true }; const allowedTopLevelAttrs = { - key: true, secondary: true, ip: true, country: true, email: true, - firstName: true, lastName: true, avatar: true, name: true, anonymous: true, custom: true + key: true, + secondary: true, + ip: true, + country: true, + email: true, + firstName: true, + lastName: true, + avatar: true, + name: true, + anonymous: true, + custom: true, }; if (config.all_attributes_private !== undefined) { @@ -29,22 +38,27 @@ export default function UserFilter(config) { const userPrivateAttrs = user.privateAttributeNames || []; const isPrivateAttr = function(name) { - return !ignoreAttrs[name] && ( - allAttributesPrivate || userPrivateAttrs.indexOf(name) !== -1 || - privateAttributeNames.indexOf(name) !== -1); + return ( + !ignoreAttrs[name] && + (allAttributesPrivate || userPrivateAttrs.indexOf(name) !== -1 || privateAttributeNames.indexOf(name) !== -1) + ); }; const filterAttrs = function(props, isAttributeAllowed) { - return Object.keys(props).reduce(function (acc, name) { - if (isAttributeAllowed(name)) { - if (isPrivateAttr(name)) { - // add to hidden list - acc[1][name] = true; - } else { - acc[0][name] = props[name]; + return Object.keys(props).reduce( + (acc, name) => { + const ret = acc; + if (isAttributeAllowed(name)) { + if (isPrivateAttr(name)) { + // add to hidden list + ret[1][name] = true; + } else { + ret[0][name] = props[name]; + } } - } - return acc; - }, [{}, {}]); + return ret; + }, + [{}, {}] + ); }; const result = filterAttrs(user, key => allowedTopLevelAttrs[key]); const filteredProps = result[0]; @@ -62,4 +76,4 @@ export default function UserFilter(config) { return filteredProps; }; return filter; -} \ No newline at end of file +} From f55691c4d9f1199b53a0e85988168f1c8e7f2e07 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 18 Apr 2018 11:42:15 -0700 Subject: [PATCH 24/59] more linter fixes --- src/__tests__/EventProcessor-test.js | 70 +++++++++++++++++++-------- src/__tests__/EventSummarizer-test.js | 38 ++++++++------- src/__tests__/UserFilter-test.js | 70 +++++++++++++-------------- src/__tests__/utils-test.js | 1 - 4 files changed, 103 insertions(+), 76 deletions(-) diff --git a/src/__tests__/EventProcessor-test.js b/src/__tests__/EventProcessor-test.js index cf9b9e48..87bd6b6e 100644 --- a/src/__tests__/EventProcessor-test.js +++ b/src/__tests__/EventProcessor-test.js @@ -7,9 +7,9 @@ describe('EventProcessor', () => { let warnSpy; const mockEventSender = {}; const user = { key: 'userKey', name: 'Red' }; - const filteredUser = { key: 'userKey', privateAttrs: [ 'name' ] }; + const filteredUser = { key: 'userKey', privateAttrs: ['name'] }; const eventsUrl = '/fake-url'; - + mockEventSender.sendEvents = function(events, sync) { mockEventSender.calls.push({ events: events, @@ -114,12 +114,14 @@ describe('EventProcessor', () => { ep.enqueue(event); ep.flush(user, false).then(() => { expect(mockEventSender.calls.length).toEqual(1); - expect(mockEventSender.calls[0].events).toEqual([{ - kind: 'identify', - creationDate: event.creationDate, - key: user.key, - user: filteredUser, - }]); + expect(mockEventSender.calls[0].events).toEqual([ + { + kind: 'identify', + creationDate: event.creationDate, + key: user.key, + user: filteredUser, + }, + ]); done(); }); }); @@ -185,10 +187,21 @@ describe('EventProcessor', () => { it('summarizes events', done => { const ep = EventProcessor(eventsUrl, {}, mockEventSender); - const e1 = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey1', - version: 11, variation: 1, value: 'value1', default: 'default1', trackEvents: false }; - const e2 = { kind: 'feature', creationDate: 2000, user: user, key: 'flagkey2', - version: 22, variation: 1, value: 'value2', default: 'default2', trackEvents: false }; + function makeEvent(key, date, version, variation, value, defaultVal) { + return { + kind: 'feature', + creationDate: date, + user: user, + key: key, + version: version, + variation: variation, + value: value, + default: defaultVal, + trackEvents: false, + }; + } + const e1 = makeEvent('flagkey1', 1000, 11, 1, 'value1', 'default1'); + const e2 = makeEvent('flagkey2', 2000, 22, 1, 'value2', 'default2'); ep.enqueue(e1); ep.enqueue(e2); ep.flush(user, false).then(() => { @@ -202,12 +215,12 @@ describe('EventProcessor', () => { expect(se.features).toEqual({ flagkey1: { default: 'default1', - counters: [ { version: 11, value: 'value1', count: 1 } ] + counters: [{ version: 11, value: 'value1', count: 1 }], }, flagkey2: { default: 'default2', - counters: [ { version: 22, value: 'value2', count: 1 } ] - } + counters: [{ version: 22, value: 'value2', count: 1 }], + }, }); done(); }); @@ -215,8 +228,13 @@ describe('EventProcessor', () => { it('queues custom event', done => { const ep = EventProcessor(eventsUrl, {}, mockEventSender); - const e = { kind: 'custom', creationDate: 1000, user: user, key: 'eventkey', - data: { thing: 'stuff' } }; + const e = { + kind: 'custom', + creationDate: 1000, + user: user, + key: 'eventkey', + data: { thing: 'stuff' }, + }; ep.enqueue(e); ep.flush(user, false).then(() => { expect(mockEventSender.calls.length).toEqual(1); @@ -230,8 +248,13 @@ describe('EventProcessor', () => { it('can include inline user in custom event', done => { const config = { inlineUsersInEvents: true }; const ep = EventProcessor(eventsUrl, config, mockEventSender); - const e = { kind: 'custom', creationDate: 1000, user: user, key: 'eventkey', - data: { thing: 'stuff' } }; + const e = { + kind: 'custom', + creationDate: 1000, + user: user, + key: 'eventkey', + data: { thing: 'stuff' }, + }; ep.enqueue(e); ep.flush(user, false).then(() => { expect(mockEventSender.calls.length).toEqual(1); @@ -245,8 +268,13 @@ describe('EventProcessor', () => { it('filters user in custom event', done => { const config = { allAttributesPrivate: true, inlineUsersInEvents: true }; const ep = EventProcessor(eventsUrl, config, mockEventSender); - const e = { kind: 'custom', creationDate: 1000, user: user, key: 'eventkey', - data: { thing: 'stuff' } }; + const e = { + kind: 'custom', + creationDate: 1000, + user: user, + key: 'eventkey', + data: { thing: 'stuff' }, + }; ep.enqueue(e); ep.flush(user, false).then(() => { expect(mockEventSender.calls.length).toEqual(1); diff --git a/src/__tests__/EventSummarizer-test.js b/src/__tests__/EventSummarizer-test.js index c12e0bd9..71814f9e 100644 --- a/src/__tests__/EventSummarizer-test.js +++ b/src/__tests__/EventSummarizer-test.js @@ -33,16 +33,23 @@ describe('EventSummarizer', () => { it('increments counters for feature events', () => { const es = EventSummarizer(); - const event1 = { kind: 'feature', creationDate: 1000, key: 'key1', version: 11, user: user, - variation: 1, value: 100, default: 111 }; - const event2 = { kind: 'feature', creationDate: 1000, key: 'key1', version: 11, user: user, - variation: 2, value: 200, default: 111 }; - const event3 = { kind: 'feature', creationDate: 1000, key: 'key2', version: 22, user: user, - variation: 1, value: 999, default: 222 }; - const event4 = { kind: 'feature', creationDate: 1000, key: 'key1', version: 11, user: user, - variation: 1, value: 100, default: 111 }; - const event5 = { kind: 'feature', creationDate: 1000, key: 'badkey', user: user, - value: 333, default: 333 }; + function makeEvent(key, version, variation, value, defaultVal) { + return { + kind: 'feature', + creationDate: 1000, + key: key, + version: version, + user: user, + variation: variation, + value: value, + default: defaultVal, + }; + } + const event1 = makeEvent('key1', 11, 1, 100, 111); + const event2 = makeEvent('key1', 11, 2, 200, 111); + const event3 = makeEvent('key2', 22, 1, 999, 222); + const event4 = makeEvent('key1', 11, 1, 100, 111); + const event5 = makeEvent('badkey', null, null, 333, 333); es.summarizeEvent(event1); es.summarizeEvent(event2); es.summarizeEvent(event3); @@ -54,19 +61,16 @@ describe('EventSummarizer', () => { const expectedFeatures = { key1: { default: 111, - counters: [ - { value: 100, version: 11, count: 2 }, - { value: 200, version: 11, count: 1 }, - ], + counters: [{ value: 100, version: 11, count: 2 }, { value: 200, version: 11, count: 1 }], }, key2: { default: 222, - counters: [ { value: 999, version: 22, count: 1 }] + counters: [{ value: 999, version: 22, count: 1 }], }, badkey: { default: 333, - counters: [ { value: 333, unknown: true, count: 1 }] - } + counters: [{ value: 333, unknown: true, count: 1 }], + }, }; expect(data.features).toEqual(expectedFeatures); }); diff --git a/src/__tests__/UserFilter-test.js b/src/__tests__/UserFilter-test.js index 16a4e179..e5940199 100644 --- a/src/__tests__/UserFilter-test.js +++ b/src/__tests__/UserFilter-test.js @@ -5,61 +5,57 @@ describe('UserFilter', () => { // users to serialize const user = { - 'key': 'abc', - 'firstName': 'Sue', - 'custom': { 'bizzle': 'def', 'dizzle': 'ghi' } + key: 'abc', + firstName: 'Sue', + custom: { bizzle: 'def', dizzle: 'ghi' }, }; const userSpecifyingOwnPrivateAttr = { - 'key': 'abc', - 'firstName': 'Sue', - 'custom': { 'bizzle': 'def', 'dizzle': 'ghi' }, - 'privateAttributeNames': [ 'dizzle', 'unused' ] + key: 'abc', + firstName: 'Sue', + custom: { bizzle: 'def', dizzle: 'ghi' }, + privateAttributeNames: ['dizzle', 'unused'], }; - var userWithUnknownTopLevelAttrs = { - 'key': 'abc', - 'firstName': 'Sue', - 'species': 'human', - 'hatSize': 6, - 'custom': { 'bizzle': 'def', 'dizzle': 'ghi' } + const userWithUnknownTopLevelAttrs = { + key: 'abc', + firstName: 'Sue', + species: 'human', + hatSize: 6, + custom: { bizzle: 'def', dizzle: 'ghi' }, }; const anonUser = { - 'key': 'abc', - 'anonymous': true, - 'custom': { 'bizzle': 'def', 'dizzle': 'ghi' } + key: 'abc', + anonymous: true, + custom: { bizzle: 'def', dizzle: 'ghi' }, }; // expected results from serializing user const userWithAllAttrsHidden = { - 'key': 'abc', - 'custom': { }, - 'privateAttrs': [ 'bizzle', 'dizzle', 'firstName' ] + key: 'abc', + custom: {}, + privateAttrs: ['bizzle', 'dizzle', 'firstName'], }; const userWithSomeAttrsHidden = { - 'key': 'abc', - 'custom': { - 'dizzle': 'ghi' - }, - 'privateAttrs': [ 'bizzle', 'firstName' ] + key: 'abc', + custom: { dizzle: 'ghi' }, + privateAttrs: ['bizzle', 'firstName'], }; const userWithOwnSpecifiedAttrHidden = { - 'key': 'abc', - 'firstName': 'Sue', - 'custom': { - 'bizzle': 'def' - }, - 'privateAttrs': [ 'dizzle' ] + key: 'abc', + firstName: 'Sue', + custom: { bizzle: 'def' }, + privateAttrs: ['dizzle'], }; const anonUserWithAllAttrsHidden = { - 'key': 'abc', - 'anonymous': true, - 'custom': { }, - 'privateAttrs': [ 'bizzle', 'dizzle' ] + key: 'abc', + anonymous: true, + custom: {}, + privateAttrs: ['bizzle', 'dizzle'], }; beforeEach(() => { @@ -87,12 +83,12 @@ describe('UserFilter', () => { }); it('hides some attributes if privateAttributeNames is set', () => { - const uf = UserFilter({ privateAttributeNames: [ 'firstName', 'bizzle' ]}); + const uf = UserFilter({ privateAttributeNames: ['firstName', 'bizzle'] }); expect(uf.filterUser(user)).toEqual(userWithSomeAttrsHidden); }); it('allows private_attribute_names as deprecated synonym for privateAttributeNames', () => { - const uf = UserFilter({ private_attribute_names: [ 'firstName', 'bizzle' ]}); + const uf = UserFilter({ private_attribute_names: ['firstName', 'bizzle'] }); expect(uf.filterUser(user)).toEqual(userWithSomeAttrsHidden); expect(warnSpy).toHaveBeenCalled(); }); @@ -103,7 +99,7 @@ describe('UserFilter', () => { }); it('looks at both per-user privateAttrs and global config', () => { - const uf = UserFilter({ privateAttributeNames: [ 'firstName', 'bizzle' ]}); + const uf = UserFilter({ privateAttributeNames: ['firstName', 'bizzle'] }); expect(uf.filterUser(userSpecifyingOwnPrivateAttr)).toEqual(userWithAllAttrsHidden); }); diff --git a/src/__tests__/utils-test.js b/src/__tests__/utils-test.js index ef89dacc..7786b020 100644 --- a/src/__tests__/utils-test.js +++ b/src/__tests__/utils-test.js @@ -1,4 +1,3 @@ -import sinon from 'sinon'; import { base64URLEncode, wrapPromiseCallback, chunkUserEventsForUrl } from '../utils'; describe('utils', () => { From 8161a00d0348835437e83f580a064204a47c7e48 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 18 Apr 2018 11:56:36 -0700 Subject: [PATCH 25/59] linter fixes --- src/__tests__/LDClient-test.js | 41 +++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index 417e1ded..1ba967c1 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -255,9 +255,10 @@ describe('LDClient', () => { }); client.on('ready', () => { - expect(JSON.parse(window.localStorage.getItem(lsKeyHash))).toEqual( - { $schema: 1, 'enable-foo': { value: true, version: 1 } } - ); + expect(JSON.parse(window.localStorage.getItem(lsKeyHash))).toEqual({ + $schema: 1, + 'enable-foo': { value: true, version: 1 }, + }); done(); }); @@ -279,9 +280,10 @@ describe('LDClient', () => { client.on('ready', () => { client.identify(user2, null, () => { expect(window.localStorage.getItem(lsKey)).toBeNull(); - expect(JSON.parse(window.localStorage.getItem(lsKey2))).toEqual( - { $schema: 1, 'enable-foo': { value: true, version: 1 } } - ); + expect(JSON.parse(window.localStorage.getItem(lsKey2))).toEqual({ + $schema: 1, + 'enable-foo': { value: true, version: 1 }, + }); done(); }); server.respond(); @@ -408,7 +410,11 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.ping(); - getLastRequest().respond(200, { 'Content-Type': 'application/json' }, '{"enable-foo":{"value":true,"version":1}}'); + getLastRequest().respond( + 200, + { 'Content-Type': 'application/json' }, + '{"enable-foo":{"value":true,"version":1}}' + ); expect(client.variation('enable-foo')).toEqual(true); done(); }); @@ -441,9 +447,10 @@ describe('LDClient', () => { }); expect(client.variation('enable-foo')).toEqual(true); - expect(JSON.parse(window.localStorage.getItem(lsKey))).toEqual( - { $schema: 1, 'enable-foo': { value: true, version: 1 } } - ); + expect(JSON.parse(window.localStorage.getItem(lsKey))).toEqual({ + $schema: 1, + 'enable-foo': { value: true, version: 1 }, + }); done(); }); }); @@ -510,9 +517,10 @@ describe('LDClient', () => { }); expect(client.variation('enable-foo')).toEqual(true); - expect(JSON.parse(window.localStorage.getItem(lsKey))).toEqual( - { $schema: 1, 'enable-foo': { value: true, version: 1 } } - ); + expect(JSON.parse(window.localStorage.getItem(lsKey))).toEqual({ + $schema: 1, + 'enable-foo': { value: true, version: 1 }, + }); done(); }); }); @@ -649,9 +657,10 @@ describe('LDClient', () => { }); expect(client.variation('enable-foo')).toEqual(undefined); - expect(JSON.parse(window.localStorage.getItem(lsKey))).toEqual( - { $schema: 1, 'enable-foo': { version: 1, deleted: true } } - ); + expect(JSON.parse(window.localStorage.getItem(lsKey))).toEqual({ + $schema: 1, + 'enable-foo': { version: 1, deleted: true }, + }); done(); }); }); From ced597326b9ccfb02e4c46f236e3ed811cfa06ae Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 18 Apr 2018 18:23:07 -0700 Subject: [PATCH 26/59] don't create empty request till we're sending something --- src/EventSender.js | 2 +- src/__tests__/EventSender-test.js | 36 +++++++++++++++++++------------ 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/EventSender.js b/src/EventSender.js index 2f06db42..10e9097d 100644 --- a/src/EventSender.js +++ b/src/EventSender.js @@ -1,7 +1,6 @@ import * as utils from './utils'; const MAX_URL_LENGTH = 2000; -const hasCors = 'withCredentials' in new XMLHttpRequest(); export default function EventSender(eventsUrl) { const sender = {}; @@ -10,6 +9,7 @@ export default function EventSender(eventsUrl) { const src = eventsUrl + '?d=' + utils.base64URLEncode(JSON.stringify(events)); const send = onDone => { + const hasCors = 'withCredentials' in new XMLHttpRequest(); // Detect browser support for CORS if (hasCors) { /* supports cross-domain requests */ diff --git a/src/__tests__/EventSender-test.js b/src/__tests__/EventSender-test.js index d93d682f..3d50c306 100644 --- a/src/__tests__/EventSender-test.js +++ b/src/__tests__/EventSender-test.js @@ -26,6 +26,10 @@ describe('EventSender', () => { warnSpy.mockRestore(); }); + function lastRequest() { + return requests[requests.length - 1]; + } + function base64URLDecode(str) { let s = str; while (s.length % 4 !== 0) { @@ -47,18 +51,16 @@ describe('EventSender', () => { const sender = EventSender(eventsUrl); const event = { kind: 'identify', key: 'userKey' }; sender.sendEvents([event], false); - requests[0].respond(); - expect(requests.length).toEqual(1); - expect(requests[0].async).toEqual(true); + lastRequest().respond(); + expect(lastRequest().async).toEqual(true); }); it('should send synchronously', () => { const sender = EventSender(eventsUrl); const event = { kind: 'identify', key: 'userKey' }; sender.sendEvents([event], true); - requests[0].respond(); - expect(requests.length).toEqual(1); - expect(requests[0].async).toEqual(false); + lastRequest().respond(); + expect(lastRequest().async).toEqual(false); }); it('should encode events in a single chunk if they fit', done => { @@ -67,11 +69,10 @@ describe('EventSender', () => { const event2 = { kind: 'identify', key: 'userKey2' }; const events = [event1, event2]; sender.sendEvents(events, true).then(() => { - expect(requests.length).toEqual(1); - expect(decodeOutputFromUrl(requests[0].url)).toEqual(events); + expect(decodeOutputFromUrl(lastRequest().url)).toEqual(events); done(); }); - requests[0].respond(); + lastRequest().respond(); }); it('should send events in multiple chunks if necessary', done => { @@ -81,15 +82,22 @@ describe('EventSender', () => { events.push({ kind: 'identify', key: 'thisIsALongUserKey' + i }); } sender.sendEvents(events, true).then(() => { - expect(requests.length).toEqual(3); - const output0 = decodeOutputFromUrl(requests[0].url); - const output1 = decodeOutputFromUrl(requests[1].url); - const output2 = decodeOutputFromUrl(requests[2].url); + var realRequests = []; + // The array of requests will also contain empty requests from our CORS-checking logic + for (let i = 0; i < requests.length; i++) { + if (requests[i].url) { + realRequests.push(requests[i]); + } + } + expect(realRequests.length).toEqual(3); + const output0 = decodeOutputFromUrl(realRequests[0].url); + const output1 = decodeOutputFromUrl(realRequests[1].url); + const output2 = decodeOutputFromUrl(realRequests[2].url); expect(output0).toEqual(events.slice(0, 31)); expect(output1).toEqual(events.slice(31, 62)); expect(output2).toEqual(events.slice(62, 80)); done(); }); - requests[0].respond(); + lastRequest().respond(); }); }); From fc7016905ce56989c7733582902e431dc35ab46c Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 18 Apr 2018 18:25:03 -0700 Subject: [PATCH 27/59] linter fix --- src/__tests__/EventSender-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/EventSender-test.js b/src/__tests__/EventSender-test.js index 3d50c306..a8940061 100644 --- a/src/__tests__/EventSender-test.js +++ b/src/__tests__/EventSender-test.js @@ -82,7 +82,7 @@ describe('EventSender', () => { events.push({ kind: 'identify', key: 'thisIsALongUserKey' + i }); } sender.sendEvents(events, true).then(() => { - var realRequests = []; + const realRequests = []; // The array of requests will also contain empty requests from our CORS-checking logic for (let i = 0; i < requests.length; i++) { if (requests[i].url) { From 6317376254d7b9cfdf93444db701e8bfd6c68f69 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 23 Apr 2018 10:21:27 -0700 Subject: [PATCH 28/59] rename test file --- src/__tests__/{store-test.js => Store-test.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/__tests__/{store-test.js => Store-test.js} (100%) diff --git a/src/__tests__/store-test.js b/src/__tests__/Store-test.js similarity index 100% rename from src/__tests__/store-test.js rename to src/__tests__/Store-test.js From e07ec5d8a07b7fd0340133112f3225a533fe475b Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 7 May 2018 16:25:38 -0700 Subject: [PATCH 29/59] linter fix --- src/__tests__/EventSender-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/__tests__/EventSender-test.js b/src/__tests__/EventSender-test.js index 4511269c..a8940061 100644 --- a/src/__tests__/EventSender-test.js +++ b/src/__tests__/EventSender-test.js @@ -2,7 +2,6 @@ import Base64 from 'Base64'; import sinon from 'sinon'; import EventSender from '../EventSender'; -import * as utils from '../utils'; describe('EventSender', () => { let sandbox; From b39b48e773b0b603fa731a17a6a9628d01246f82 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Mon, 14 May 2018 19:15:20 -0700 Subject: [PATCH 30/59] don't suppress variation property in events --- src/EventProcessor.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/EventProcessor.js b/src/EventProcessor.js index d0e266f2..f2a738fa 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -65,7 +65,6 @@ export default function EventProcessor(eventsUrl, options = {}, emitter = null, if (e.kind === 'feature') { delete ret['trackEvents']; delete ret['debugEventsUntilDate']; - delete ret['variation']; } return ret; } From 300216dbacfca1be72599aa28e7e079b645ac10b Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 15 May 2018 09:25:41 -0700 Subject: [PATCH 31/59] fix getTime --- src/EventSender.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/EventSender.js b/src/EventSender.js index 57139413..e2129134 100644 --- a/src/EventSender.js +++ b/src/EventSender.js @@ -9,9 +9,9 @@ export default function EventSender(eventsUrl) { const ret = { status: xhr.status }; const dateStr = xhr.getResponseHeader('Date'); if (dateStr) { - const date = Date.parse(dateStr); - if (date) { - ret.serverTime = date.getTime(); + const time = Date.parse(dateStr); + if (time) { + ret.serverTime = time; } } return ret; From 9cb6182b6b6d71ce69b2da3d75cf915fcdcdae5b Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 15 May 2018 14:27:46 -0700 Subject: [PATCH 32/59] temporarily disable event caching and extra flag events, to facilitate integration testing --- src/index.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/index.js b/src/index.js index fee5c77d..36a2788f 100644 --- a/src/index.js +++ b/src/index.js @@ -72,13 +72,14 @@ function initialize(env, user, options = {}) { const user = ident.getUser(); const cacheKey = JSON.stringify(value) + (user && user.key ? user.key : '') + key; const now = new Date(); - const cached = seenRequests[cacheKey]; + // TEMPORARY - turn off caching/throttling logic which interferes with integration + // const cached = seenRequests[cacheKey]; - if (cached && now - cached < 300000 /* five minutes, in ms */) { - return; - } + // if (cached && now - cached < 300000 /* five minutes, in ms */) { + // return; + // } - seenRequests[cacheKey] = now; + // seenRequests[cacheKey] = now; const event = { kind: 'feature', @@ -312,9 +313,11 @@ function initialize(env, user, options = {}) { emitter.emit(changeEvent, changes); - keys.forEach(key => { - sendFlagEvent(key, changes[key].current); - }); + // TEMPORARY - turn off sending of events when flags are acquired (it messes with + // integration testing, and I'm not sure why we're doing it anyway) + // keys.forEach(key => { + // sendFlagEvent(key, changes[key].current); + // }); } } From 6dd749f59426a2a022c824c13fc159408ee77868 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 15 May 2018 14:31:28 -0700 Subject: [PATCH 33/59] comment out more disabled logic --- src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index 36a2788f..fa2058ab 100644 --- a/src/index.js +++ b/src/index.js @@ -24,7 +24,7 @@ function initialize(env, user, options = {}) { const stream = Stream(streamUrl, environment, hash, options.useReport); const events = EventProcessor(eventsUrl + '/a/' + environment + '.gif', options, emitter); const requestor = Requestor(baseUrl, environment, options.useReport); - const seenRequests = {}; + // const seenRequests = {}; // temporarily disabled, see below let flags = typeof options.bootstrap === 'object' ? utils.transformValuesToVersionedValues(options.bootstrap) : {}; let goalTracker; let useLocalStorage; @@ -70,7 +70,7 @@ function initialize(env, user, options = {}) { function sendFlagEvent(key, value, defaultValue) { const user = ident.getUser(); - const cacheKey = JSON.stringify(value) + (user && user.key ? user.key : '') + key; + // const cacheKey = JSON.stringify(value) + (user && user.key ? user.key : '') + key; // see below const now = new Date(); // TEMPORARY - turn off caching/throttling logic which interferes with integration // const cached = seenRequests[cacheKey]; From 269fb5065a49c72e0d99d9eaeffb16b7f734580b Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 15 May 2018 14:54:25 -0700 Subject: [PATCH 34/59] debugging --- src/EventSender.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/EventSender.js b/src/EventSender.js index e2129134..cd125d18 100644 --- a/src/EventSender.js +++ b/src/EventSender.js @@ -56,6 +56,12 @@ export default function EventSender(eventsUrl) { } sender.sendEvents = function(events, sync) { + // TEMPORARY - debugging + for (var i: events) { + console.log("* sending event: " + JSON.stringify(events[i])); + } + // end debugging + const finalSync = sync === undefined ? false : sync; const chunks = utils.chunkUserEventsForUrl(MAX_URL_LENGTH - eventsUrl.length, events); const results = []; From 26b328e06287b9794c076d0c38f08bf690b23bf7 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 15 May 2018 14:55:22 -0700 Subject: [PATCH 35/59] typo --- src/EventSender.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EventSender.js b/src/EventSender.js index cd125d18..73c510a1 100644 --- a/src/EventSender.js +++ b/src/EventSender.js @@ -57,7 +57,7 @@ export default function EventSender(eventsUrl) { sender.sendEvents = function(events, sync) { // TEMPORARY - debugging - for (var i: events) { + for (var i = 0; i < events.length; i++) { console.log("* sending event: " + JSON.stringify(events[i])); } // end debugging From 39e76b626b70514d76ebdb08583c9a75541058fc Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 15 May 2018 14:57:46 -0700 Subject: [PATCH 36/59] linter --- src/EventSender.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/EventSender.js b/src/EventSender.js index 73c510a1..734d5b17 100644 --- a/src/EventSender.js +++ b/src/EventSender.js @@ -57,8 +57,8 @@ export default function EventSender(eventsUrl) { sender.sendEvents = function(events, sync) { // TEMPORARY - debugging - for (var i = 0; i < events.length; i++) { - console.log("* sending event: " + JSON.stringify(events[i])); + for (let i = 0; i < events.length; i++) { + console.log('* sending event: ' + JSON.stringify(events[i])); } // end debugging From df27ea6f524846b400cac52872cefbe8bfd0a938 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 15 May 2018 16:26:28 -0700 Subject: [PATCH 37/59] include variation in summary events --- src/EventSummarizer.js | 4 ++++ src/__tests__/EventProcessor-test.js | 4 ++-- src/__tests__/EventSummarizer-test.js | 7 +++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/EventSummarizer.js b/src/EventSummarizer.js index 3adba9e5..2e82259c 100644 --- a/src/EventSummarizer.js +++ b/src/EventSummarizer.js @@ -15,6 +15,7 @@ export default function EventSummarizer() { counters[counterKey] = { count: 1, key: event.key, + variation: event.variation, version: event.version, value: event.value, default: event.default, @@ -46,6 +47,9 @@ export default function EventSummarizer() { value: c.value, count: c.count, }; + if (c.variation !== undefined && c.variation !== null) { + counterOut.variation = c.variation; + } if (c.version) { counterOut.version = c.version; } else { diff --git a/src/__tests__/EventProcessor-test.js b/src/__tests__/EventProcessor-test.js index 93f1cdd0..05956fba 100644 --- a/src/__tests__/EventProcessor-test.js +++ b/src/__tests__/EventProcessor-test.js @@ -336,11 +336,11 @@ describe('EventProcessor', () => { expect(se.features).toEqual({ flagkey1: { default: 'default1', - counters: [{ version: 11, value: 'value1', count: 1 }], + counters: [{ version: 11, variation: 1, value: 'value1', count: 1 }], }, flagkey2: { default: 'default2', - counters: [{ version: 22, value: 'value2', count: 1 }], + counters: [{ version: 22, variation: 1, value: 'value2', count: 1 }], }, }); done(); diff --git a/src/__tests__/EventSummarizer-test.js b/src/__tests__/EventSummarizer-test.js index 71814f9e..a2b334ea 100644 --- a/src/__tests__/EventSummarizer-test.js +++ b/src/__tests__/EventSummarizer-test.js @@ -61,11 +61,14 @@ describe('EventSummarizer', () => { const expectedFeatures = { key1: { default: 111, - counters: [{ value: 100, version: 11, count: 2 }, { value: 200, version: 11, count: 1 }], + counters: [ + { value: 100, variation: 1, version: 11, count: 2 }, + { value: 200, variation: 2, version: 11, count: 1 } + ], }, key2: { default: 222, - counters: [{ value: 999, version: 22, count: 1 }], + counters: [{ value: 999, variation: 1, version: 22, count: 1 }], }, badkey: { default: 333, From 6b5af8e35d26b6133afc9c1cc32d70be162dd5e5 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 15 May 2018 16:29:04 -0700 Subject: [PATCH 38/59] linter --- src/__tests__/EventSummarizer-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/EventSummarizer-test.js b/src/__tests__/EventSummarizer-test.js index a2b334ea..4d506b18 100644 --- a/src/__tests__/EventSummarizer-test.js +++ b/src/__tests__/EventSummarizer-test.js @@ -63,7 +63,7 @@ describe('EventSummarizer', () => { default: 111, counters: [ { value: 100, variation: 1, version: 11, count: 2 }, - { value: 200, variation: 2, version: 11, count: 1 } + { value: 200, variation: 2, version: 11, count: 1 }, ], }, key2: { From 8607903b1687296fb4ddf4ea8637bfb7c14278ef Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 15 May 2018 16:26:28 -0700 Subject: [PATCH 39/59] include variation in summary events --- src/EventSummarizer.js | 4 ++++ src/__tests__/EventProcessor-test.js | 4 ++-- src/__tests__/EventSummarizer-test.js | 7 +++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/EventSummarizer.js b/src/EventSummarizer.js index 3adba9e5..2e82259c 100644 --- a/src/EventSummarizer.js +++ b/src/EventSummarizer.js @@ -15,6 +15,7 @@ export default function EventSummarizer() { counters[counterKey] = { count: 1, key: event.key, + variation: event.variation, version: event.version, value: event.value, default: event.default, @@ -46,6 +47,9 @@ export default function EventSummarizer() { value: c.value, count: c.count, }; + if (c.variation !== undefined && c.variation !== null) { + counterOut.variation = c.variation; + } if (c.version) { counterOut.version = c.version; } else { diff --git a/src/__tests__/EventProcessor-test.js b/src/__tests__/EventProcessor-test.js index 93f1cdd0..05956fba 100644 --- a/src/__tests__/EventProcessor-test.js +++ b/src/__tests__/EventProcessor-test.js @@ -336,11 +336,11 @@ describe('EventProcessor', () => { expect(se.features).toEqual({ flagkey1: { default: 'default1', - counters: [{ version: 11, value: 'value1', count: 1 }], + counters: [{ version: 11, variation: 1, value: 'value1', count: 1 }], }, flagkey2: { default: 'default2', - counters: [{ version: 22, value: 'value2', count: 1 }], + counters: [{ version: 22, variation: 1, value: 'value2', count: 1 }], }, }); done(); diff --git a/src/__tests__/EventSummarizer-test.js b/src/__tests__/EventSummarizer-test.js index 71814f9e..4d506b18 100644 --- a/src/__tests__/EventSummarizer-test.js +++ b/src/__tests__/EventSummarizer-test.js @@ -61,11 +61,14 @@ describe('EventSummarizer', () => { const expectedFeatures = { key1: { default: 111, - counters: [{ value: 100, version: 11, count: 2 }, { value: 200, version: 11, count: 1 }], + counters: [ + { value: 100, variation: 1, version: 11, count: 2 }, + { value: 200, variation: 2, version: 11, count: 1 }, + ], }, key2: { default: 222, - counters: [{ value: 999, version: 22, count: 1 }], + counters: [{ value: 999, variation: 1, version: 22, count: 1 }], }, badkey: { default: 333, From a22542f305e925914bea42846d5742ae2060c029 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 15 May 2018 17:18:42 -0700 Subject: [PATCH 40/59] don't generate feature request events for anything other than variation() --- src/index.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index fa2058ab..d3828048 100644 --- a/src/index.js +++ b/src/index.js @@ -149,6 +149,10 @@ function initialize(env, user, options = {}) { } function variation(key, defaultValue) { + return variationInternal(key, defaultValue, true); + } + + function variationInternal(key, defaultValue, sendEvent) { let value; if (flags && flags.hasOwnProperty(key) && !flags[key].deleted) { @@ -157,7 +161,9 @@ function initialize(env, user, options = {}) { value = defaultValue; } - sendFlagEvent(key, value, defaultValue); + if (sendEvent) { + sendFlagEvent(key, value, defaultValue); + } return value; } @@ -183,7 +189,8 @@ function initialize(env, user, options = {}) { for (const key in flags) { if (flags.hasOwnProperty(key)) { - results[key] = variation(key, null); + // TEMPORARY: turned off event generation for allFlags + results[key] = variationInternal(key, null, false); } } From 83720f3388a3300f0a84d668f5834d24412e17f4 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 15 May 2018 18:53:02 -0700 Subject: [PATCH 41/59] rm debugging --- src/EventSender.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/EventSender.js b/src/EventSender.js index 734d5b17..e2129134 100644 --- a/src/EventSender.js +++ b/src/EventSender.js @@ -56,12 +56,6 @@ export default function EventSender(eventsUrl) { } sender.sendEvents = function(events, sync) { - // TEMPORARY - debugging - for (let i = 0; i < events.length; i++) { - console.log('* sending event: ' + JSON.stringify(events[i])); - } - // end debugging - const finalSync = sync === undefined ? false : sync; const chunks = utils.chunkUserEventsForUrl(MAX_URL_LENGTH - eventsUrl.length, events); const results = []; From 7aacee44e8e6df83a4305ef5f4555aa982caa2b1 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 16 May 2018 09:34:16 -0700 Subject: [PATCH 42/59] add config options for suppressing unwanted event behavior --- src/index.js | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/index.js b/src/index.js index d3828048..e8b05944 100644 --- a/src/index.js +++ b/src/index.js @@ -19,12 +19,14 @@ function initialize(env, user, options = {}) { const streamUrl = options.streamUrl || 'https://clientstream.launchdarkly.com'; const hash = options.hash; const sendEvents = typeof options.sendEvents === 'undefined' ? true : config.sendEvents; + const suppressDuplicateEvents = !!options.suppressDuplicateEvents; + const sendEventsOnlyForVariation = !!options.sendEventsOnlyForVariation; const environment = env; const emitter = EventEmitter(); const stream = Stream(streamUrl, environment, hash, options.useReport); const events = EventProcessor(eventsUrl + '/a/' + environment + '.gif', options, emitter); const requestor = Requestor(baseUrl, environment, options.useReport); - // const seenRequests = {}; // temporarily disabled, see below + const seenRequests = {}; let flags = typeof options.bootstrap === 'object' ? utils.transformValuesToVersionedValues(options.bootstrap) : {}; let goalTracker; let useLocalStorage; @@ -70,16 +72,15 @@ function initialize(env, user, options = {}) { function sendFlagEvent(key, value, defaultValue) { const user = ident.getUser(); - // const cacheKey = JSON.stringify(value) + (user && user.key ? user.key : '') + key; // see below const now = new Date(); - // TEMPORARY - turn off caching/throttling logic which interferes with integration - // const cached = seenRequests[cacheKey]; - - // if (cached && now - cached < 300000 /* five minutes, in ms */) { - // return; - // } - - // seenRequests[cacheKey] = now; + if (suppressDuplicateEvents) { + const cacheKey = JSON.stringify(value) + (user && user.key ? user.key : '') + key; // see below + const cached = seenRequests[cacheKey]; + if (cached && now - cached < 300000) { // five minutes, in ms + return; + } + seenRequests[cacheKey] = now; + } const event = { kind: 'feature', @@ -189,8 +190,7 @@ function initialize(env, user, options = {}) { for (const key in flags) { if (flags.hasOwnProperty(key)) { - // TEMPORARY: turned off event generation for allFlags - results[key] = variationInternal(key, null, false); + results[key] = variationInternal(key, null, !sendEventsOnlyForVariation); } } @@ -320,11 +320,11 @@ function initialize(env, user, options = {}) { emitter.emit(changeEvent, changes); - // TEMPORARY - turn off sending of events when flags are acquired (it messes with - // integration testing, and I'm not sure why we're doing it anyway) - // keys.forEach(key => { - // sendFlagEvent(key, changes[key].current); - // }); + if (!sendEventsOnlyForVariation) { + keys.forEach(key => { + sendFlagEvent(key, changes[key].current); + }); + } } } From 81fcf0125f53790a2f877052e0e0450a34a29aef Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 16 May 2018 09:36:29 -0700 Subject: [PATCH 43/59] change option from suppress to allow --- src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index e8b05944..77341ad1 100644 --- a/src/index.js +++ b/src/index.js @@ -19,7 +19,7 @@ function initialize(env, user, options = {}) { const streamUrl = options.streamUrl || 'https://clientstream.launchdarkly.com'; const hash = options.hash; const sendEvents = typeof options.sendEvents === 'undefined' ? true : config.sendEvents; - const suppressDuplicateEvents = !!options.suppressDuplicateEvents; + const allowFrequentDuplicateEvents = !!options.allowFrequentDuplicateEvents; const sendEventsOnlyForVariation = !!options.sendEventsOnlyForVariation; const environment = env; const emitter = EventEmitter(); @@ -73,7 +73,7 @@ function initialize(env, user, options = {}) { function sendFlagEvent(key, value, defaultValue) { const user = ident.getUser(); const now = new Date(); - if (suppressDuplicateEvents) { + if (!allowFrequentDuplicateEvents) { const cacheKey = JSON.stringify(value) + (user && user.key ? user.key : '') + key; // see below const cached = seenRequests[cacheKey]; if (cached && now - cached < 300000) { // five minutes, in ms From c68b0d8302f83a565f58e404bc2a460e93373509 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 16 May 2018 09:41:17 -0700 Subject: [PATCH 44/59] linter --- src/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 77341ad1..365912af 100644 --- a/src/index.js +++ b/src/index.js @@ -76,7 +76,8 @@ function initialize(env, user, options = {}) { if (!allowFrequentDuplicateEvents) { const cacheKey = JSON.stringify(value) + (user && user.key ? user.key : '') + key; // see below const cached = seenRequests[cacheKey]; - if (cached && now - cached < 300000) { // five minutes, in ms + // cache TTL is five minutes + if (cached && now - cached < 300000) { return; } seenRequests[cacheKey] = now; From d70eefb08dafdbb1597bd83912daa957ecbf6ef7 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 16 May 2018 19:20:15 -0700 Subject: [PATCH 45/59] tolerate nulls in flags map --- src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index 365912af..0cfc303f 100644 --- a/src/index.js +++ b/src/index.js @@ -157,7 +157,7 @@ function initialize(env, user, options = {}) { function variationInternal(key, defaultValue, sendEvent) { let value; - if (flags && flags.hasOwnProperty(key) && !flags[key].deleted) { + if (flags && flags[key] && !flags[key].deleted) { value = flags[key].value === null ? defaultValue : flags[key].value; } else { value = defaultValue; @@ -289,7 +289,7 @@ function initialize(env, user, options = {}) { } for (const key in flags) { - if (flags.hasOwnProperty(key)) { + if (flags[key]) { if (newFlags[key] && newFlags[key].value !== flags[key].value) { changes[key] = { previous: flags[key].value, current: newFlags[key].value }; } else if (!newFlags[key] || newFlags[key].deleted) { From 0ad5864f3e990e30b2bfff40cf216ad273fd5248 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 16 May 2018 19:23:55 -0700 Subject: [PATCH 46/59] more null-toleration --- src/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/index.js b/src/index.js index 0cfc303f..aab6a9cb 100644 --- a/src/index.js +++ b/src/index.js @@ -289,7 +289,7 @@ function initialize(env, user, options = {}) { } for (const key in flags) { - if (flags[key]) { + if (flags.hasOwnProperty(key) && flags[key]) { if (newFlags[key] && newFlags[key].value !== flags[key].value) { changes[key] = { previous: flags[key].value, current: newFlags[key].value }; } else if (!newFlags[key] || newFlags[key].deleted) { @@ -298,7 +298,7 @@ function initialize(env, user, options = {}) { } } for (const key in newFlags) { - if (newFlags.hasOwnProperty(key) && (!flags[key] || flags[key].deleted)) { + if (newFlags.hasOwnProperty(key) && newFlags[key] && (!flags[key] || flags[key].deleted)) { changes[key] = { current: newFlags[key].value }; } } From 1382a8f0b9bd9c13fa4e0da0c11a1249dd15466e Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 17 May 2018 11:56:11 -0700 Subject: [PATCH 47/59] better handling of null/invalid user --- src/Identity.js | 3 ++ src/__tests__/LDClient-test.js | 66 ++++++++++++++++++++++++++++++++++ src/index.js | 33 ++++++++++------- 3 files changed, 89 insertions(+), 13 deletions(-) diff --git a/src/Identity.js b/src/Identity.js index 37c6a7a4..451311bf 100644 --- a/src/Identity.js +++ b/src/Identity.js @@ -1,6 +1,9 @@ import * as utils from './utils'; function sanitizeUser(u) { + if (!u) { + return null; + } const sane = utils.clone(u); if (sane.key) { sane.key = sane.key.toString(); diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index 1ba967c1..aa249ba8 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -683,5 +683,71 @@ describe('LDClient', () => { getLastRequest().respond(200, { 'Content-Type': 'application/json' }, '{"enable-foo": true}'); }); }); + + it('returns an error when identify is called with null user', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {} }); + + client.on('ready', () => { + client.identify(null).then( + () => { + throw Error("should not have succeeded"); + }, + () => { + done(); + } + ); + }); + }); + + it('returns an error when identify is called with user with no key', done => { + const client = LDClient.initialize(envName, user, { bootstrap: {} }); + + client.on('ready', () => { + client.identify({ country: "US" }).then( + () => { + throw Error("should not have succeeded"); + }, + () => { + done(); + } + ); + }); + }); + + it('returns default value for flag after identify is called with null user', done => { + const data = { foo: 'bar' }; + const client = LDClient.initialize(envName, user, { bootstrap: data }); + + client.on('ready', () => { + expect(client.variation('foo', 'x')).toEqual('bar'); + client.identify(null).then( + () => { + throw Error("should not have succeeded"); + }, + () => { + expect(client.variation('foo', 'x')).toEqual('x'); + done(); + } + ); + }); + }); + + it('returns default value for flag after identify is called with invalid user', done => { + const data = { foo: 'bar' }; + const client = LDClient.initialize(envName, user, { bootstrap: data }); + + client.on('ready', () => { + expect(client.variation('foo', 'x')).toEqual('bar'); + client.identify({ country: "US" }).then( + () => { + throw Error("should not have succeeded"); + }, + () => { + expect(client.variation('foo', 'x')).toEqual('x'); + done(); + } + ); + }); + }); }); }); diff --git a/src/index.js b/src/index.js index 365912af..ac12bbf8 100644 --- a/src/index.js +++ b/src/index.js @@ -126,19 +126,26 @@ function initialize(env, user, options = {}) { return utils.wrapPromiseCallback( new Promise((resolve, reject) => { ident.setUser(user); - requestor.fetchFlagSettings(ident.getUser(), hash, (err, settings) => { - if (err) { - emitter.maybeReportError(new errors.LDFlagFetchError(messages.errorFetchingFlags(err))); - return reject(err); - } - if (settings) { - updateSettings(settings); - } - resolve(settings); - if (subscribedToChangeEvents) { - connectStream(); - } - }); + if (!user || user.key === null || user.key === undefined) { + updateSettings({}); + const err = new errors.LDInvalidUserError(user ? messages.invalidUser() : messages.userNotSpecified()); + emitter.maybeReportError(err); + utils.onNextTick(() => reject(err)); + } else { + requestor.fetchFlagSettings(ident.getUser(), hash, (err, settings) => { + if (err) { + emitter.maybeReportError(new errors.LDFlagFetchError(messages.errorFetchingFlags(err))); + return reject(err); + } + if (settings) { + updateSettings(settings); + } + resolve(settings); + if (subscribedToChangeEvents) { + connectStream(); + } + }); + } }), onDone ); From b30304bc1592e34f4602082d7dd3a88d21a90e09 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 17 May 2018 11:59:25 -0700 Subject: [PATCH 48/59] linter --- src/__tests__/LDClient-test.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index aa249ba8..7892f65b 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -690,7 +690,7 @@ describe('LDClient', () => { client.on('ready', () => { client.identify(null).then( () => { - throw Error("should not have succeeded"); + throw Error('should not have succeeded'); }, () => { done(); @@ -703,9 +703,9 @@ describe('LDClient', () => { const client = LDClient.initialize(envName, user, { bootstrap: {} }); client.on('ready', () => { - client.identify({ country: "US" }).then( + client.identify({ country: 'US' }).then( () => { - throw Error("should not have succeeded"); + throw Error('should not have succeeded'); }, () => { done(); @@ -722,7 +722,7 @@ describe('LDClient', () => { expect(client.variation('foo', 'x')).toEqual('bar'); client.identify(null).then( () => { - throw Error("should not have succeeded"); + throw Error('should not have succeeded'); }, () => { expect(client.variation('foo', 'x')).toEqual('x'); @@ -738,9 +738,9 @@ describe('LDClient', () => { client.on('ready', () => { expect(client.variation('foo', 'x')).toEqual('bar'); - client.identify({ country: "US" }).then( + client.identify({ country: 'US' }).then( () => { - throw Error("should not have succeeded"); + throw Error('should not have succeeded'); }, () => { expect(client.variation('foo', 'x')).toEqual('x'); From 2fa24a0856c6bac1afaea5312d360ea934dbebc8 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 17 May 2018 12:07:00 -0700 Subject: [PATCH 49/59] comment --- src/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/index.js b/src/index.js index ac12bbf8..d1dd9937 100644 --- a/src/index.js +++ b/src/index.js @@ -127,6 +127,8 @@ function initialize(env, user, options = {}) { new Promise((resolve, reject) => { ident.setUser(user); if (!user || user.key === null || user.key === undefined) { + // If the user is invalid, we put ourselves in a state where no flags exist + // (so all evaluations will return default values). updateSettings({}); const err = new errors.LDInvalidUserError(user ? messages.invalidUser() : messages.userNotSpecified()); emitter.maybeReportError(err); From db5ca9895e2e3ea3d8ac686c208624b87a8bc323 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 17 May 2018 16:52:40 -0700 Subject: [PATCH 50/59] restore hasOwnProperty --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index aab6a9cb..5e7a4168 100644 --- a/src/index.js +++ b/src/index.js @@ -157,7 +157,7 @@ function initialize(env, user, options = {}) { function variationInternal(key, defaultValue, sendEvent) { let value; - if (flags && flags[key] && !flags[key].deleted) { + if (flags && flags.hasOwnProperty(key) && flags[key] && !flags[key].deleted) { value = flags[key].value === null ? defaultValue : flags[key].value; } else { value = defaultValue; From 0425fa69aa58f2137bf7e7a69cae427f364374c3 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 18 May 2018 13:43:57 -0700 Subject: [PATCH 51/59] use new flagVersion property for events --- src/__tests__/LDClient-test.js | 224 ++++++++++++++++++++++++++++++--- src/index.js | 11 +- 2 files changed, 216 insertions(+), 19 deletions(-) diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index 1ba967c1..9ea3fbcb 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -362,8 +362,114 @@ describe('LDClient', () => { }); }); + describe('event generation', () => { + function stubEventProcessor() { + const ep = { events: [] }; + ep.start = function() {}; + ep.flush = function() {}; + ep.stop = function() {}; + ep.enqueue = function(e) { ep.events.push(e); }; + return ep; + } + + function expectIdentifyEvent(e, user) { + expect(e.kind).toEqual('identify'); + expect(e.user).toEqual(user); + } + + function expectFeatureEvent(e, key, value, variation, version, defaultVal) { + expect(e.kind).toEqual('feature'); + expect(e.key).toEqual(key); + expect(e.value).toEqual(value); + expect(e.variation).toEqual(variation); + expect(e.version).toEqual(version); + expect(e.default).toEqual(defaultVal); + } + + it('sends an identify event at startup', done => { + const ep = stubEventProcessor(); + const server = sinon.fakeServer.create(); + server.respondWith([200, { 'Content-Type': 'application/json' }, + '{"foo":{"value":"a","variation":1,"version":2,"flagVersion":2000}}']); + const client = LDClient.initialize(envName, user, { eventProcessor: ep }); + + client.on('ready', () => { + expect(ep.events.length).toEqual(1); + expectIdentifyEvent(ep.events[0], user); + + done(); + }); + + server.respond(); + }); + + it('sends a feature event for variation()', done => { + const ep = stubEventProcessor(); + const server = sinon.fakeServer.create(); + server.respondWith([200, { 'Content-Type': 'application/json' }, + '{"foo":{"value":"a","variation":1,"version":2,"flagVersion":2000}}']); + const client = LDClient.initialize(envName, user, { eventProcessor: ep }); + + client.on('ready', () => { + client.variation('foo', 'x'); + + expect(ep.events.length).toEqual(2); + expectIdentifyEvent(ep.events[0], user); + expectFeatureEvent(ep.events[1], 'foo', 'a', 1, 2000, 'x'); + + done(); + }); + + server.respond(); + }); + + it('uses "version" instead of "flagVersion" in event if "flagVersion" is absent', done => { + const ep = stubEventProcessor(); + const server = sinon.fakeServer.create(); + server.respondWith([200, { 'Content-Type': 'application/json' }, + '{"foo":{"value":"a","variation":1,"version":2}}']); + const client = LDClient.initialize(envName, user, { eventProcessor: ep }); + + client.on('ready', () => { + client.variation('foo', 'x'); + + expect(ep.events.length).toEqual(2); + expectIdentifyEvent(ep.events[0], user); + expectFeatureEvent(ep.events[1], 'foo', 'a', 1, 2, 'x'); + + done(); + }); + + server.respond(); + }); + + it('omits event version if flag does not exist', done => { + const ep = stubEventProcessor(); + const server = sinon.fakeServer.create(); + server.respondWith([200, { 'Content-Type': 'application/json' }, + '{}']); + const client = LDClient.initialize(envName, user, { eventProcessor: ep }); + + client.on('ready', () => { + client.variation('foo', 'x'); + + expect(ep.events.length).toEqual(2); + expectIdentifyEvent(ep.events[0], user); + expectFeatureEvent(ep.events[1], 'foo', 'x', undefined, undefined, 'x'); + + done(); + }); + + server.respond(); + }); + }); + describe('event listening', () => { const streamUrl = 'https://clientstream.launchdarkly.com'; + + function streamEvents() { + return sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events; + } it('does not connect to the stream by default', done => { const client = LDClient.initialize(envName, user, { bootstrap: {} }); @@ -409,7 +515,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.ping(); + streamEvents().ping(); getLastRequest().respond( 200, { 'Content-Type': 'application/json' }, @@ -426,7 +532,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.put({ + streamEvents().put({ data: '{"enable-foo":{"value":true,"version":1}}', }); @@ -442,7 +548,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.put({ + streamEvents().put({ data: '{"enable-foo":{"value":true,"version":1}}', }); @@ -467,7 +573,7 @@ describe('LDClient', () => { done(); }); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.put({ + streamEvents().put({ data: '{"enable-foo":{"value":true,"version":1}}', }); }); @@ -484,7 +590,7 @@ describe('LDClient', () => { done(); }); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.put({ + streamEvents().put({ data: '{"enable-foo":{"value":true,"version":1}}', }); }); @@ -496,7 +602,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.patch({ + streamEvents().patch({ data: '{"key":"enable-foo","value":true,"version":1}', }); @@ -505,6 +611,94 @@ describe('LDClient', () => { }); }); + it('does not update flag if patch version < flag version', done => { + const server = sinon.fakeServer.create(); + server.respondWith([200, { 'Content-Type': 'application/json' }, + '{"enable-foo":{"value":"a","version":2}}']); + + const client = LDClient.initialize(envName, user); + client.on('ready', () => { + expect(client.variation('enable-foo')).toEqual("a"); + + client.on('change', () => {}); + + streamEvents().patch({ + data: '{"key":"enable-foo","value":"b","version":1}' + }); + + expect(client.variation('enable-foo')).toEqual("a"); + + done(); + }); + server.respond(); + }); + + it('does not update flag if patch version == flag version', done => { + const server = sinon.fakeServer.create(); + server.respondWith([200, { 'Content-Type': 'application/json' }, + '{"enable-foo":{"value":"a","version":2}}']); + + const client = LDClient.initialize(envName, user); + client.on('ready', () => { + expect(client.variation('enable-foo')).toEqual("a"); + + client.on('change', () => {}); + + streamEvents().patch({ + data: '{"key":"enable-foo","value":"b","version":1}' + }); + + expect(client.variation('enable-foo')).toEqual("a"); + + done(); + }); + server.respond(); + }); + + it('updates flag if patch has a version and flag has no version', done => { + const server = sinon.fakeServer.create(); + server.respondWith([200, { 'Content-Type': 'application/json' }, + '{"enable-foo":{"value":"a"}}']); + + const client = LDClient.initialize(envName, user); + client.on('ready', () => { + expect(client.variation('enable-foo')).toEqual("a"); + + client.on('change', () => {}); + + streamEvents().patch({ + data: '{"key":"enable-foo","value":"b","version":1}' + }); + + expect(client.variation('enable-foo')).toEqual("b"); + + done(); + }); + server.respond(); + }); + + it('updates flag if flag has a version and patch has no version', done => { + const server = sinon.fakeServer.create(); + server.respondWith([200, { 'Content-Type': 'application/json' }, + '{"enable-foo":{"value":"a","version":1}}']); + + const client = LDClient.initialize(envName, user); + client.on('ready', () => { + expect(client.variation('enable-foo')).toEqual("a"); + + client.on('change', () => {}); + + streamEvents().patch({ + data: '{"key":"enable-foo","value":"b"}' + }); + + expect(client.variation('enable-foo')).toEqual("b"); + + done(); + }); + server.respond(); + }); + it('updates local storage for patch message if using local storage', done => { window.localStorage.setItem(lsKey, '{"enable-foo":false}'); const client = LDClient.initialize(envName, user, { bootstrap: 'localstorage' }); @@ -512,7 +706,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.put({ + streamEvents().put({ data: '{"enable-foo":{"value":true,"version":1}}', }); @@ -537,7 +731,7 @@ describe('LDClient', () => { done(); }); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.patch({ + streamEvents().patch({ data: '{"key":"enable-foo","value":true,"version":1}', }); }); @@ -554,7 +748,7 @@ describe('LDClient', () => { done(); }); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.patch({ + streamEvents().patch({ data: '{"key":"enable-foo","value":true,"version":1}', }); }); @@ -572,7 +766,7 @@ describe('LDClient', () => { done(); }); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.patch({ + streamEvents().patch({ data: '{"key":"enable-foo","value":true,"version":1}', }); }); @@ -589,7 +783,7 @@ describe('LDClient', () => { done(); }); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.patch({ + streamEvents().patch({ data: '{"key":"enable-foo","value":true,"version":1}', }); }); @@ -601,7 +795,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.delete({ + streamEvents().delete({ data: '{"key":"enable-foo","version":1}', }); @@ -622,7 +816,7 @@ describe('LDClient', () => { done(); }); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.delete({ + streamEvents().delete({ data: '{"key":"enable-foo","version":1}', }); }); @@ -639,7 +833,7 @@ describe('LDClient', () => { done(); }); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.delete({ + streamEvents().delete({ data: '{"key":"enable-foo","version":1}', }); }); @@ -652,7 +846,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); - sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events.delete({ + streamEvents().delete({ data: '{"key":"enable-foo","version":1}', }); diff --git a/src/index.js b/src/index.js index 5e7a4168..05e1c2d1 100644 --- a/src/index.js +++ b/src/index.js @@ -24,7 +24,7 @@ function initialize(env, user, options = {}) { const environment = env; const emitter = EventEmitter(); const stream = Stream(streamUrl, environment, hash, options.useReport); - const events = EventProcessor(eventsUrl + '/a/' + environment + '.gif', options, emitter); + const events = options.eventProcessor || EventProcessor(eventsUrl + '/a/' + environment + '.gif', options, emitter); const requestor = Requestor(baseUrl, environment, options.useReport); const seenRequests = {}; let flags = typeof options.bootstrap === 'object' ? utils.transformValuesToVersionedValues(options.bootstrap) : {}; @@ -93,7 +93,7 @@ function initialize(env, user, options = {}) { }; const flag = flags[key]; if (flag) { - event.version = flag.version; + event.version = flag.flagVersion ? flag.flagVersion : flag.version; event.variation = flag.variation; event.trackEvents = flag.trackEvents; event.debugEventsUntilDate = flag.debugEventsUntilDate; @@ -253,9 +253,12 @@ function initialize(env, user, options = {}) { }, patch: function(e) { const data = JSON.parse(e.data); - if (!flags[data.key] || flags[data.key].version < data.version) { + // If both the flag and the patch have a version property, then the patch version must be + // greater than the flag version for us to accept the patch. If either one has no version + // then the patch always succeeds. + const oldFlag = flags[data.key]; + if (!oldFlag || !oldFlag.version || !data.version || oldFlag.version < data.version) { const mods = {}; - const oldFlag = flags[data.key]; const newFlag = Object.assign({}, data); delete newFlag['key']; flags[data.key] = newFlag; From 2d987a2850cc2867604c2e0a8e8de15840946525 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 18 May 2018 13:49:09 -0700 Subject: [PATCH 52/59] linter --- src/__tests__/LDClient-test.js | 78 ++++++++++++++++------------------ 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index 9ea3fbcb..ab822157 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -368,7 +368,9 @@ describe('LDClient', () => { ep.start = function() {}; ep.flush = function() {}; ep.stop = function() {}; - ep.enqueue = function(e) { ep.events.push(e); }; + ep.enqueue = function(e) { + ep.events.push(e); + }; return ep; } @@ -389,8 +391,11 @@ describe('LDClient', () => { it('sends an identify event at startup', done => { const ep = stubEventProcessor(); const server = sinon.fakeServer.create(); - server.respondWith([200, { 'Content-Type': 'application/json' }, - '{"foo":{"value":"a","variation":1,"version":2,"flagVersion":2000}}']); + server.respondWith([ + 200, + { 'Content-Type': 'application/json' }, + '{"foo":{"value":"a","variation":1,"version":2,"flagVersion":2000}}', + ]); const client = LDClient.initialize(envName, user, { eventProcessor: ep }); client.on('ready', () => { @@ -406,8 +411,11 @@ describe('LDClient', () => { it('sends a feature event for variation()', done => { const ep = stubEventProcessor(); const server = sinon.fakeServer.create(); - server.respondWith([200, { 'Content-Type': 'application/json' }, - '{"foo":{"value":"a","variation":1,"version":2,"flagVersion":2000}}']); + server.respondWith([ + 200, + { 'Content-Type': 'application/json' }, + '{"foo":{"value":"a","variation":1,"version":2,"flagVersion":2000}}', + ]); const client = LDClient.initialize(envName, user, { eventProcessor: ep }); client.on('ready', () => { @@ -426,8 +434,11 @@ describe('LDClient', () => { it('uses "version" instead of "flagVersion" in event if "flagVersion" is absent', done => { const ep = stubEventProcessor(); const server = sinon.fakeServer.create(); - server.respondWith([200, { 'Content-Type': 'application/json' }, - '{"foo":{"value":"a","variation":1,"version":2}}']); + server.respondWith([ + 200, + { 'Content-Type': 'application/json' }, + '{"foo":{"value":"a","variation":1,"version":2}}', + ]); const client = LDClient.initialize(envName, user, { eventProcessor: ep }); client.on('ready', () => { @@ -446,8 +457,7 @@ describe('LDClient', () => { it('omits event version if flag does not exist', done => { const ep = stubEventProcessor(); const server = sinon.fakeServer.create(); - server.respondWith([200, { 'Content-Type': 'application/json' }, - '{}']); + server.respondWith([200, { 'Content-Type': 'application/json' }, '{}']); const client = LDClient.initialize(envName, user, { eventProcessor: ep }); client.on('ready', () => { @@ -466,7 +476,7 @@ describe('LDClient', () => { describe('event listening', () => { const streamUrl = 'https://clientstream.launchdarkly.com'; - + function streamEvents() { return sources[`${streamUrl}/eval/${envName}/${encodedUser}`].__emitter._events; } @@ -602,9 +612,7 @@ describe('LDClient', () => { client.on('ready', () => { client.on('change', () => {}); - streamEvents().patch({ - data: '{"key":"enable-foo","value":true,"version":1}', - }); + streamEvents().patch({ data: '{"key":"enable-foo","value":true,"version":1}' }); expect(client.variation('enable-foo')).toEqual(true); done(); @@ -613,20 +621,17 @@ describe('LDClient', () => { it('does not update flag if patch version < flag version', done => { const server = sinon.fakeServer.create(); - server.respondWith([200, { 'Content-Type': 'application/json' }, - '{"enable-foo":{"value":"a","version":2}}']); + server.respondWith([200, { 'Content-Type': 'application/json' }, '{"enable-foo":{"value":"a","version":2}}']); const client = LDClient.initialize(envName, user); client.on('ready', () => { - expect(client.variation('enable-foo')).toEqual("a"); + expect(client.variation('enable-foo')).toEqual('a'); client.on('change', () => {}); - streamEvents().patch({ - data: '{"key":"enable-foo","value":"b","version":1}' - }); + streamEvents().patch({ data: '{"key":"enable-foo","value":"b","version":1}' }); - expect(client.variation('enable-foo')).toEqual("a"); + expect(client.variation('enable-foo')).toEqual('a'); done(); }); @@ -635,20 +640,17 @@ describe('LDClient', () => { it('does not update flag if patch version == flag version', done => { const server = sinon.fakeServer.create(); - server.respondWith([200, { 'Content-Type': 'application/json' }, - '{"enable-foo":{"value":"a","version":2}}']); + server.respondWith([200, { 'Content-Type': 'application/json' }, '{"enable-foo":{"value":"a","version":2}}']); const client = LDClient.initialize(envName, user); client.on('ready', () => { - expect(client.variation('enable-foo')).toEqual("a"); + expect(client.variation('enable-foo')).toEqual('a'); client.on('change', () => {}); - streamEvents().patch({ - data: '{"key":"enable-foo","value":"b","version":1}' - }); + streamEvents().patch({ data: '{"key":"enable-foo","value":"b","version":1}' }); - expect(client.variation('enable-foo')).toEqual("a"); + expect(client.variation('enable-foo')).toEqual('a'); done(); }); @@ -657,20 +659,17 @@ describe('LDClient', () => { it('updates flag if patch has a version and flag has no version', done => { const server = sinon.fakeServer.create(); - server.respondWith([200, { 'Content-Type': 'application/json' }, - '{"enable-foo":{"value":"a"}}']); + server.respondWith([200, { 'Content-Type': 'application/json' }, '{"enable-foo":{"value":"a"}}']); const client = LDClient.initialize(envName, user); client.on('ready', () => { - expect(client.variation('enable-foo')).toEqual("a"); + expect(client.variation('enable-foo')).toEqual('a'); client.on('change', () => {}); - streamEvents().patch({ - data: '{"key":"enable-foo","value":"b","version":1}' - }); + streamEvents().patch({ data: '{"key":"enable-foo","value":"b","version":1}' }); - expect(client.variation('enable-foo')).toEqual("b"); + expect(client.variation('enable-foo')).toEqual('b'); done(); }); @@ -679,20 +678,17 @@ describe('LDClient', () => { it('updates flag if flag has a version and patch has no version', done => { const server = sinon.fakeServer.create(); - server.respondWith([200, { 'Content-Type': 'application/json' }, - '{"enable-foo":{"value":"a","version":1}}']); + server.respondWith([200, { 'Content-Type': 'application/json' }, '{"enable-foo":{"value":"a","version":2}}']); const client = LDClient.initialize(envName, user); client.on('ready', () => { - expect(client.variation('enable-foo')).toEqual("a"); + expect(client.variation('enable-foo')).toEqual('a'); client.on('change', () => {}); - streamEvents().patch({ - data: '{"key":"enable-foo","value":"b"}' - }); + streamEvents().patch({ data: '{"key":"enable-foo","value":"b"}' }); - expect(client.variation('enable-foo')).toEqual("b"); + expect(client.variation('enable-foo')).toEqual('b'); done(); }); From 85be9eb15b87359143c1440e5cd62f62d8edd00d Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 18 May 2018 15:48:06 -0700 Subject: [PATCH 53/59] if CORS is available, post events to "bulk" endpoint, not to GIF endpoint --- src/EventProcessor.js | 4 +- src/EventSender.js | 56 ++++++++---- src/__tests__/EventProcessor-test.js | 35 ++++---- src/__tests__/EventSender-test.js | 126 ++++++++++++++++----------- src/index.js | 2 +- 5 files changed, 135 insertions(+), 88 deletions(-) diff --git a/src/EventProcessor.js b/src/EventProcessor.js index f2a738fa..f82feb32 100644 --- a/src/EventProcessor.js +++ b/src/EventProcessor.js @@ -4,9 +4,9 @@ import UserFilter from './UserFilter'; import * as errors from './errors'; import * as utils from './utils'; -export default function EventProcessor(eventsUrl, options = {}, emitter = null, sender = null) { +export default function EventProcessor(eventsUrl, environmentId, options = {}, emitter = null, sender = null) { const processor = {}; - const eventSender = sender || EventSender(eventsUrl); + const eventSender = sender || EventSender(eventsUrl, environmentId); const summarizer = EventSummarizer(); const userFilter = UserFilter(options); const inlineUsers = !!options.inlineUsersInEvents; diff --git a/src/EventSender.js b/src/EventSender.js index e2129134..bae56320 100644 --- a/src/EventSender.js +++ b/src/EventSender.js @@ -2,9 +2,20 @@ import * as utils from './utils'; const MAX_URL_LENGTH = 2000; -export default function EventSender(eventsUrl) { +export default function EventSender(eventsUrl, environmentId, forceHasCors, imageCreator) { + let hasCors; + const postUrl = eventsUrl + '/events/bulk/' + environmentId; + const imageUrl = eventsUrl + '/a/' + environmentId + '.gif'; const sender = {}; + function loadUrlUsingImage(src, onDone) { + const img = new Image(); + if (onDone) { + img.addEventListener('load', onDone); + } + img.src = src; + } + function getResponseInfo(xhr) { const ret = { status: xhr.status }; const dateStr = xhr.getResponseHeader('Date'); @@ -17,16 +28,13 @@ export default function EventSender(eventsUrl) { return ret; } - function sendChunk(events, sync) { - const src = eventsUrl + '?d=' + utils.base64URLEncode(JSON.stringify(events)); - + function sendChunk(events, usePost, sync) { + const createImage = imageCreator || loadUrlUsingImage; const send = onDone => { - const hasCors = 'withCredentials' in new XMLHttpRequest(); - // Detect browser support for CORS - if (hasCors) { - /* supports cross-domain requests */ + if (usePost) { const xhr = new XMLHttpRequest(); - xhr.open('GET', src, !sync); + xhr.open('POST', postUrl, !sync); + xhr.setRequestHeader('Content-Type', 'application/json'); if (!sync) { xhr.addEventListener('load', () => { @@ -34,15 +42,10 @@ export default function EventSender(eventsUrl) { }); } - xhr.send(); + xhr.send(JSON.stringify(events)); } else { - const img = new Image(); - - if (!sync) { - img.addEventListener('load', onDone); - } - - img.src = src; + const src = imageUrl + '?d=' + utils.base64URLEncode(JSON.stringify(events)); + createImage(src, sync ? null : onDone); } }; @@ -56,11 +59,26 @@ export default function EventSender(eventsUrl) { } sender.sendEvents = function(events, sync) { + // Detect browser support for CORS (can be overridden by tests) + if (hasCors === undefined) { + if (forceHasCors === undefined) { + hasCors = 'withCredentials' in new XMLHttpRequest(); + } else { + hasCors = forceHasCors; + } + } + const finalSync = sync === undefined ? false : sync; - const chunks = utils.chunkUserEventsForUrl(MAX_URL_LENGTH - eventsUrl.length, events); + let chunks; + if (hasCors) { + // no need to break up events into chunks if we can send a POST + chunks = [events]; + } else { + chunks = utils.chunkUserEventsForUrl(MAX_URL_LENGTH - eventsUrl.length, events); + } const results = []; for (let i = 0; i < chunks.length; i++) { - results.push(sendChunk(chunks[i], finalSync)); + results.push(sendChunk(chunks[i], hasCors, finalSync)); } return sync ? Promise.resolve() : Promise.all(results); }; diff --git a/src/__tests__/EventProcessor-test.js b/src/__tests__/EventProcessor-test.js index 05956fba..b4ed180f 100644 --- a/src/__tests__/EventProcessor-test.js +++ b/src/__tests__/EventProcessor-test.js @@ -9,6 +9,7 @@ describe('EventProcessor', () => { const user = { key: 'userKey', name: 'Red' }; const filteredUser = { key: 'userKey', privateAttrs: ['name'] }; const eventsUrl = '/fake-url'; + const envId = 'env'; mockEventSender.sendEvents = function(events, sync) { mockEventSender.calls.push({ @@ -61,7 +62,7 @@ describe('EventProcessor', () => { } it('should flush asynchronously', () => { - const processor = EventProcessor(eventsUrl, {}, null, mockEventSender); + const processor = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); const event = { kind: 'identify', key: user.key }; processor.enqueue(event); @@ -75,7 +76,7 @@ describe('EventProcessor', () => { }); it('should flush synchronously', () => { - const processor = EventProcessor(eventsUrl, {}, null, mockEventSender); + const processor = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); const user = { key: 'foo' }; const event = { kind: 'identify', key: user.key }; @@ -90,7 +91,7 @@ describe('EventProcessor', () => { }); it('should enqueue identify event', done => { - const ep = EventProcessor(eventsUrl, {}, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); const event = { kind: 'identify', creationDate: 1000, key: user.key, user: user }; ep.enqueue(event); ep.flush().then(() => { @@ -102,7 +103,7 @@ describe('EventProcessor', () => { it('filters user in identify event', done => { const config = { allAttributesPrivate: true }; - const ep = EventProcessor(eventsUrl, config, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, config, null, mockEventSender); const event = { kind: 'identify', creationDate: 1000, key: user.key, user: user }; ep.enqueue(event); ep.flush().then(() => { @@ -120,7 +121,7 @@ describe('EventProcessor', () => { }); it('queues individual feature event', done => { - const ep = EventProcessor(eventsUrl, {}, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); const event = { kind: 'feature', creationDate: 1000, @@ -141,7 +142,7 @@ describe('EventProcessor', () => { it('can include inline user in feature event', done => { const config = { inlineUsersInEvents: true }; - const ep = EventProcessor(eventsUrl, config, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, config, null, mockEventSender); const event = { kind: 'feature', creationDate: 1000, @@ -162,7 +163,7 @@ describe('EventProcessor', () => { it('filters user in feature event', done => { const config = { allAttributesPrivate: true, inlineUsersInEvents: true }; - const ep = EventProcessor(eventsUrl, config, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, config, null, mockEventSender); const event = { kind: 'feature', creationDate: 1000, @@ -182,7 +183,7 @@ describe('EventProcessor', () => { }); it('sets event kind to debug if event is temporarily in debug mode', done => { - const ep = EventProcessor(eventsUrl, {}, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); const futureTime = new Date().getTime() + 1000000; const e = { kind: 'feature', @@ -207,7 +208,7 @@ describe('EventProcessor', () => { }); it('can both track and debug an event', done => { - const ep = EventProcessor(eventsUrl, {}, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); const futureTime = new Date().getTime() + 1000000; const e = { kind: 'feature', @@ -233,7 +234,7 @@ describe('EventProcessor', () => { }); it('expires debug mode based on client time if client time is later than server time', done => { - const ep = EventProcessor(eventsUrl, {}, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); // Pick a server time that is somewhat behind the client time const serverTime = new Date().getTime() - 20000; @@ -270,7 +271,7 @@ describe('EventProcessor', () => { }); it('expires debug mode based on server time if server time is later than client time', done => { - const ep = EventProcessor(eventsUrl, {}, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); // Pick a server time that is somewhat ahead of the client time const serverTime = new Date().getTime() + 20000; @@ -307,7 +308,7 @@ describe('EventProcessor', () => { }); it('summarizes nontracked events', done => { - const ep = EventProcessor(eventsUrl, {}, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); function makeEvent(key, date, version, variation, value, defaultVal) { return { kind: 'feature', @@ -348,7 +349,7 @@ describe('EventProcessor', () => { }); it('queues custom event', done => { - const ep = EventProcessor(eventsUrl, {}, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); const e = { kind: 'custom', creationDate: 1000, @@ -368,7 +369,7 @@ describe('EventProcessor', () => { it('can include inline user in custom event', done => { const config = { inlineUsersInEvents: true }; - const ep = EventProcessor(eventsUrl, config, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, config, null, mockEventSender); const e = { kind: 'custom', creationDate: 1000, @@ -388,7 +389,7 @@ describe('EventProcessor', () => { it('filters user in custom event', done => { const config = { allAttributesPrivate: true, inlineUsersInEvents: true }; - const ep = EventProcessor(eventsUrl, config, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, config, null, mockEventSender); const e = { kind: 'custom', creationDate: 1000, @@ -407,7 +408,7 @@ describe('EventProcessor', () => { }); it('sends nothing if there are no events to flush', done => { - const ep = EventProcessor(eventsUrl, {}, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); ep.flush().then(() => { expect(mockEventSender.calls.length).toEqual(0); done(); @@ -415,7 +416,7 @@ describe('EventProcessor', () => { }); it('stops sending events after a 401 error', done => { - const ep = EventProcessor(eventsUrl, {}, null, mockEventSender); + const ep = EventProcessor(eventsUrl, envId, {}, null, mockEventSender); const e = { kind: 'identify', creationDate: 1000, user: user }; ep.enqueue(e); mockEventSender.status = 401; diff --git a/src/__tests__/EventSender-test.js b/src/__tests__/EventSender-test.js index a8940061..28ada581 100644 --- a/src/__tests__/EventSender-test.js +++ b/src/__tests__/EventSender-test.js @@ -7,8 +7,8 @@ describe('EventSender', () => { let sandbox; let xhr; let requests = []; - let warnSpy; const eventsUrl = '/fake-url'; + const envId = 'env'; beforeEach(() => { sandbox = sinon.sandbox.create(); @@ -17,19 +17,26 @@ describe('EventSender', () => { xhr.onCreate = function(xhr) { requests.push(xhr); }; - warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); }); afterEach(() => { sandbox.restore(); xhr.restore(); - warnSpy.mockRestore(); }); function lastRequest() { return requests[requests.length - 1]; } + function fakeImageCreator() { + const ret = function(url, onDone) { + ret.urls.push(url); + ret.onDone = onDone; + }; + ret.urls = []; + return ret; + } + function base64URLDecode(str) { let s = str; while (s.length % 4 !== 0) { @@ -40,64 +47,85 @@ describe('EventSender', () => { } function decodeOutputFromUrl(url) { - const prefix = eventsUrl + '?d='; + const prefix = eventsUrl + '/a/' + envId + '.gif?d='; if (!url.startsWith(prefix)) { throw 'URL "' + url + '" did not have expected prefix "' + prefix + '"'; } return JSON.parse(base64URLDecode(url.substring(prefix.length))); } - it('should send asynchronously', () => { - const sender = EventSender(eventsUrl); - const event = { kind: 'identify', key: 'userKey' }; - sender.sendEvents([event], false); - lastRequest().respond(); - expect(lastRequest().async).toEqual(true); - }); + describe('using image endpoint when CORS is not available', () => { + it('should encode events in a single chunk if they fit', () => { + const imageCreator = fakeImageCreator(); + const sender = EventSender(eventsUrl, envId, false, imageCreator); + const event1 = { kind: 'identify', key: 'userKey1' }; + const event2 = { kind: 'identify', key: 'userKey2' }; + const events = [event1, event2]; - it('should send synchronously', () => { - const sender = EventSender(eventsUrl); - const event = { kind: 'identify', key: 'userKey' }; - sender.sendEvents([event], true); - lastRequest().respond(); - expect(lastRequest().async).toEqual(false); - }); + sender.sendEvents(events, false); + + const urls = imageCreator.urls; + expect(urls.length).toEqual(1); + expect(decodeOutputFromUrl(urls[0])).toEqual(events); + }); + + it('should send events in multiple chunks if necessary', () => { + const imageCreator = fakeImageCreator(); + const sender = EventSender(eventsUrl, envId, false, imageCreator); + const events = []; + for (let i = 0; i < 80; i++) { + events.push({ kind: 'identify', key: 'thisIsALongUserKey' + i }); + } + + sender.sendEvents(events, false); - it('should encode events in a single chunk if they fit', done => { - const sender = EventSender(eventsUrl); - const event1 = { kind: 'identify', key: 'userKey1' }; - const event2 = { kind: 'identify', key: 'userKey2' }; - const events = [event1, event2]; - sender.sendEvents(events, true).then(() => { - expect(decodeOutputFromUrl(lastRequest().url)).toEqual(events); - done(); + const urls = imageCreator.urls; + expect(urls.length).toEqual(3); + expect(decodeOutputFromUrl(urls[0])).toEqual(events.slice(0, 31)); + expect(decodeOutputFromUrl(urls[1])).toEqual(events.slice(31, 62)); + expect(decodeOutputFromUrl(urls[2])).toEqual(events.slice(62, 80)); + }); + + it('should set a completion handler', () => { + const imageCreator = fakeImageCreator(); + const sender = EventSender(eventsUrl, envId, false, imageCreator); + const event1 = { kind: 'identify', key: 'userKey1' }; + + sender.sendEvents([event1], false); + + expect(imageCreator.onDone).toBeDefined(); }); - lastRequest().respond(); }); - it('should send events in multiple chunks if necessary', done => { - const sender = EventSender(eventsUrl); - const events = []; - for (let i = 0; i < 80; i++) { - events.push({ kind: 'identify', key: 'thisIsALongUserKey' + i }); - } - sender.sendEvents(events, true).then(() => { - const realRequests = []; - // The array of requests will also contain empty requests from our CORS-checking logic - for (let i = 0; i < requests.length; i++) { - if (requests[i].url) { - realRequests.push(requests[i]); - } + describe('using POST when CORS is available', () => { + it('should send asynchronously', () => { + const sender = EventSender(eventsUrl, envId, true); + const event = { kind: 'identify', key: 'userKey' }; + sender.sendEvents([event], false); + lastRequest().respond(); + expect(lastRequest().async).toEqual(true); + }); + + it('should send synchronously', () => { + const sender = EventSender(eventsUrl, envId, true); + const event = { kind: 'identify', key: 'userKey' }; + sender.sendEvents([event], true); + lastRequest().respond(); + expect(lastRequest().async).toEqual(false); + }); + + it('should send all events in request body', () => { + const sender = EventSender(eventsUrl, envId, true); + const events = []; + for (let i = 0; i < 80; i++) { + events.push({ kind: 'identify', key: 'thisIsALongUserKey' + i }); } - expect(realRequests.length).toEqual(3); - const output0 = decodeOutputFromUrl(realRequests[0].url); - const output1 = decodeOutputFromUrl(realRequests[1].url); - const output2 = decodeOutputFromUrl(realRequests[2].url); - expect(output0).toEqual(events.slice(0, 31)); - expect(output1).toEqual(events.slice(31, 62)); - expect(output2).toEqual(events.slice(62, 80)); - done(); + sender.sendEvents(events, false); + lastRequest().respond(); + const r = lastRequest(); + expect(r.url).toEqual(eventsUrl + '/events/bulk/' + envId); + expect(r.method).toEqual('POST'); + expect(JSON.parse(r.requestBody)).toEqual(events); }); - lastRequest().respond(); }); }); diff --git a/src/index.js b/src/index.js index 05e1c2d1..39305574 100644 --- a/src/index.js +++ b/src/index.js @@ -24,7 +24,7 @@ function initialize(env, user, options = {}) { const environment = env; const emitter = EventEmitter(); const stream = Stream(streamUrl, environment, hash, options.useReport); - const events = options.eventProcessor || EventProcessor(eventsUrl + '/a/' + environment + '.gif', options, emitter); + const events = options.eventProcessor || EventProcessor(eventsUrl, environment, options, emitter); const requestor = Requestor(baseUrl, environment, options.useReport); const seenRequests = {}; let flags = typeof options.bootstrap === 'object' ? utils.transformValuesToVersionedValues(options.bootstrap) : {}; From 868394dd4cca7eac2611c84f72b12695201c0edd Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Tue, 22 May 2018 14:41:34 -0700 Subject: [PATCH 54/59] revise null user logic to be consistent with mobile SDKs --- src/Identity.js | 3 --- src/__tests__/LDClient-test.js | 8 ++++---- src/index.js | 5 +---- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/Identity.js b/src/Identity.js index 451311bf..37c6a7a4 100644 --- a/src/Identity.js +++ b/src/Identity.js @@ -1,9 +1,6 @@ import * as utils from './utils'; function sanitizeUser(u) { - if (!u) { - return null; - } const sane = utils.clone(u); if (sane.key) { sane.key = sane.key.toString(); diff --git a/src/__tests__/LDClient-test.js b/src/__tests__/LDClient-test.js index 7892f65b..53507288 100644 --- a/src/__tests__/LDClient-test.js +++ b/src/__tests__/LDClient-test.js @@ -714,7 +714,7 @@ describe('LDClient', () => { }); }); - it('returns default value for flag after identify is called with null user', done => { + it('does not change flag values after identify is called with null user', done => { const data = { foo: 'bar' }; const client = LDClient.initialize(envName, user, { bootstrap: data }); @@ -725,14 +725,14 @@ describe('LDClient', () => { throw Error('should not have succeeded'); }, () => { - expect(client.variation('foo', 'x')).toEqual('x'); + expect(client.variation('foo', 'x')).toEqual('bar'); done(); } ); }); }); - it('returns default value for flag after identify is called with invalid user', done => { + it('does not change flag values after identify is called with invalid user', done => { const data = { foo: 'bar' }; const client = LDClient.initialize(envName, user, { bootstrap: data }); @@ -743,7 +743,7 @@ describe('LDClient', () => { throw Error('should not have succeeded'); }, () => { - expect(client.variation('foo', 'x')).toEqual('x'); + expect(client.variation('foo', 'x')).toEqual('bar'); done(); } ); diff --git a/src/index.js b/src/index.js index d1dd9937..4efc305f 100644 --- a/src/index.js +++ b/src/index.js @@ -125,15 +125,12 @@ function initialize(env, user, options = {}) { } return utils.wrapPromiseCallback( new Promise((resolve, reject) => { - ident.setUser(user); if (!user || user.key === null || user.key === undefined) { - // If the user is invalid, we put ourselves in a state where no flags exist - // (so all evaluations will return default values). - updateSettings({}); const err = new errors.LDInvalidUserError(user ? messages.invalidUser() : messages.userNotSpecified()); emitter.maybeReportError(err); utils.onNextTick(() => reject(err)); } else { + ident.setUser(user); requestor.fetchFlagSettings(ident.getUser(), hash, (err, settings) => { if (err) { emitter.maybeReportError(new errors.LDFlagFetchError(messages.errorFetchingFlags(err))); From 1f8bd390d0e044cf4b5a457c4c362a32ee64f2de Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 23 May 2018 11:27:19 -0700 Subject: [PATCH 55/59] rm unnecessary onNextTick --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 4efc305f..b9df1fdb 100644 --- a/src/index.js +++ b/src/index.js @@ -128,7 +128,7 @@ function initialize(env, user, options = {}) { if (!user || user.key === null || user.key === undefined) { const err = new errors.LDInvalidUserError(user ? messages.invalidUser() : messages.userNotSpecified()); emitter.maybeReportError(err); - utils.onNextTick(() => reject(err)); + reject(err); } else { ident.setUser(user); requestor.fetchFlagSettings(ident.getUser(), hash, (err, settings) => { From 523a296b2f8cdfcd81604d624ddd3c882944151f Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 24 May 2018 12:52:49 -0700 Subject: [PATCH 56/59] add custom user-agent (again) and event schema to headers --- src/EventSender.js | 2 ++ src/Requestor.js | 2 ++ src/__tests__/EventSender-test.js | 9 +++++++++ src/__tests__/Requestor-test.js | 19 +++++++++++++++++++ src/utils.js | 8 ++++++++ 5 files changed, 40 insertions(+) diff --git a/src/EventSender.js b/src/EventSender.js index bae56320..6b155f28 100644 --- a/src/EventSender.js +++ b/src/EventSender.js @@ -34,7 +34,9 @@ export default function EventSender(eventsUrl, environmentId, forceHasCors, imag if (usePost) { const xhr = new XMLHttpRequest(); xhr.open('POST', postUrl, !sync); + utils.addLDHeaders(xhr); xhr.setRequestHeader('Content-Type', 'application/json'); + xhr.setRequestHeader('X-LaunchDarkly-Event-Schema', '3'); if (!sync) { xhr.addEventListener('load', () => { diff --git a/src/Requestor.js b/src/Requestor.js index a6666c52..2ad50c19 100644 --- a/src/Requestor.js +++ b/src/Requestor.js @@ -29,9 +29,11 @@ function fetchJSON(endpoint, body, callback) { if (body) { xhr.open('REPORT', endpoint); xhr.setRequestHeader('Content-Type', 'application/json'); + utils.addLDHeaders(xhr); xhr.send(JSON.stringify(body)); } else { xhr.open('GET', endpoint); + utils.addLDHeaders(xhr); xhr.send(); } diff --git a/src/__tests__/EventSender-test.js b/src/__tests__/EventSender-test.js index 28ada581..5218a401 100644 --- a/src/__tests__/EventSender-test.js +++ b/src/__tests__/EventSender-test.js @@ -2,6 +2,7 @@ import Base64 from 'Base64'; import sinon from 'sinon'; import EventSender from '../EventSender'; +import * as utils from '../utils'; describe('EventSender', () => { let sandbox; @@ -127,5 +128,13 @@ describe('EventSender', () => { expect(r.method).toEqual('POST'); expect(JSON.parse(r.requestBody)).toEqual(events); }); + + it('should send custom user-agent header', () => { + const sender = EventSender(eventsUrl, envId, true); + const event = { kind: 'identify', key: 'userKey' }; + sender.sendEvents([event], true); + lastRequest().respond(); + expect(lastRequest().requestHeaders['X-LaunchDarkly-User-Agent']).toEqual(utils.getLDUserAgentString()); + }); }); }); diff --git a/src/__tests__/Requestor-test.js b/src/__tests__/Requestor-test.js index 406eb732..c2c0e30d 100644 --- a/src/__tests__/Requestor-test.js +++ b/src/__tests__/Requestor-test.js @@ -1,5 +1,6 @@ import sinon from 'sinon'; import Requestor from '../Requestor'; +import * as utils from '../utils'; describe('Requestor', () => { let server; @@ -83,4 +84,22 @@ describe('Requestor', () => { expect(handleFour.calledOnce).toEqual(true); expect(handleFive.calledOnce).toEqual(true); }); + + it('should send custom user-agent header in GET mode', () => { + const requestor = Requestor('http://requestee', 'FAKE_ENV', false); + const user = { key: 'foo' }; + requestor.fetchFlagSettings(user, 'hash1', sinon.spy()); + + expect(server.requests.length).toEqual(1); + expect(server.requests[0].requestHeaders['X-LaunchDarkly-User-Agent']).toEqual(utils.getLDUserAgentString()); + }); + + it('should send custom user-agent header in REPORT mode', () => { + const requestor = Requestor('http://requestee', 'FAKE_ENV', true); + const user = { key: 'foo' }; + requestor.fetchFlagSettings(user, 'hash1', sinon.spy()); + + expect(server.requests.length).toEqual(1); + expect(server.requests[0].requestHeaders['X-LaunchDarkly-User-Agent']).toEqual(utils.getLDUserAgentString()); + }); }); diff --git a/src/utils.js b/src/utils.js index 1ab799ae..a84a22d9 100644 --- a/src/utils.js +++ b/src/utils.js @@ -115,3 +115,11 @@ export function chunkUserEventsForUrl(maxLength, events) { return allChunks; } + +export function getLDUserAgentString() { + return 'JSClient/' + VERSION; +} + +export function addLDHeaders(xhr) { + xhr.setRequestHeader('X-LaunchDarkly-User-Agent', getLDUserAgentString()); +} From a3e9cb78fa2256230cec4459c717b10b8eea3e98 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Fri, 25 May 2018 17:55:49 -0700 Subject: [PATCH 57/59] prepare 2.0.0 release --- CHANGELOG.md | 17 +++++++++++++++++ package.json | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 17510709..d9069cf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,23 @@ All notable changes to the LaunchDarkly client-side JavaScript SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [2.0.0] - 2018-05-25 +### Changed +- To reduce the network bandwidth used for analytics events, feature request events are now sent as counters rather than individual events, and user details are now sent only at intervals rather than in each event. These behaviors can be modified through the LaunchDarkly UI and with the new configuration option `inlineUsersInEvents`. For more details, see [Analytics Data Stream Reference](https://docs.launchdarkly.com/v2.0/docs/analytics-data-stream-reference). +- In every function that takes an optional callback parameter, if you provide a callback, the function will not return a promise; a promise will be returned only if you omit the callback. Previously, it would always return a promise which would be resolved/rejected at the same time that the callback (if any) was called; this caused problems if you had not registered an error handler for the promise. +- When sending analytics events, if there is a connection error or an HTTP 5xx response, the client will try to send the events again one more time after a one-second delay. +- Analytics are now sent with an HTTP `POST` request if the browser supports CORS, or via image loading if it does not. Previously, they were always sent via image loading. + +### Added +- The new configuration option `sendEventsOnlyForVariation`, if set to `true`, causes analytics events for feature flags to be sent only when you call `variation`. Otherwise, the default behavior is to also send events when you call `allFlags`, and whenever a changed flag value is detected in streaming mode. +- The new configuration option `allowFrequentDuplicateEvents`, if set to `true`, turns off throttling for feature flag events. Otherwise, the default behavior is to block the sending of an analytics event if another event with the same flag key, flag value, and user key was generated within the last five minutes. + +### Fixed +- If `identify` is called with a null user, or a user with no key, the function no longer tries to do an HTTP request to the server (which would always fail); instead, it just returns an error. + +### Deprecated +- The configuration options `all_attributes_private` and `private_attribute_names` are deprecated. Use `allAttributesPrivate` and `privateAttributeNames` instead. + ## [1.7.4] - 2018-05-23 ### Fixed - Fixed a bug that caused events _not_ to be sent if `options.sendEvents` was explicitly set to `true`. diff --git a/package.json b/package.json index a22c7271..be47a700 100755 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ldclient-js", - "version": "1.7.4", + "version": "2.0.0", "description": "LaunchDarkly SDK for JavaScript", "author": "LaunchDarkly ", "license": "Apache-2.0", From 80b3c4a1fc294b0db0b2d1083be92c2158081954 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Wed, 30 May 2018 17:53:35 -0700 Subject: [PATCH 58/59] fix summary key computation --- src/EventSummarizer.js | 7 ++++- src/__tests__/EventSummarizer-test.js | 45 ++++++++++++++++++++------- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/EventSummarizer.js b/src/EventSummarizer.js index 2e82259c..2a5b833e 100644 --- a/src/EventSummarizer.js +++ b/src/EventSummarizer.js @@ -7,7 +7,12 @@ export default function EventSummarizer() { es.summarizeEvent = function(event) { if (event.kind === 'feature') { - const counterKey = event.key + ':' + (event.variation || '') + (event.version || ''); + const counterKey = + event.key + + ':' + + (event.variation !== null && event.variation !== undefined ? event.variation : '') + + ':' + + (event.version !== null && event.version !== undefined ? event.version : ''); const counterVal = counters[counterKey]; if (counterVal) { counterVal.count = counterVal.count + 1; diff --git a/src/__tests__/EventSummarizer-test.js b/src/__tests__/EventSummarizer-test.js index 4d506b18..98d323f3 100644 --- a/src/__tests__/EventSummarizer-test.js +++ b/src/__tests__/EventSummarizer-test.js @@ -31,20 +31,21 @@ describe('EventSummarizer', () => { expect(data.endDate).toEqual(2000); }); + function makeEvent(key, version, variation, value, defaultVal) { + return { + kind: 'feature', + creationDate: 1000, + key: key, + version: version, + user: user, + variation: variation, + value: value, + default: defaultVal, + }; + } + it('increments counters for feature events', () => { const es = EventSummarizer(); - function makeEvent(key, version, variation, value, defaultVal) { - return { - kind: 'feature', - creationDate: 1000, - key: key, - version: version, - user: user, - variation: variation, - value: value, - default: defaultVal, - }; - } const event1 = makeEvent('key1', 11, 1, 100, 111); const event2 = makeEvent('key1', 11, 2, 200, 111); const event3 = makeEvent('key2', 22, 1, 999, 222); @@ -77,4 +78,24 @@ describe('EventSummarizer', () => { }; expect(data.features).toEqual(expectedFeatures); }); + + it('distinguishes between zero and null/undefined in feature variation', () => { + const es = EventSummarizer(); + const event1 = makeEvent('key1', 11, 0, 100, 111); + const event2 = makeEvent('key1', 11, null, 111, 111); + const event3 = makeEvent('key1', 11, undefined, 111, 111); + es.summarizeEvent(event1); + es.summarizeEvent(event2); + es.summarizeEvent(event3); + const data = es.getSummary(); + + data.features.key1.counters.sort((a, b) => a.value - b.value); + const expectedFeatures = { + key1: { + default: 111, + counters: [{ variation: 0, value: 100, version: 11, count: 1 }, { value: 111, version: 11, count: 2 }], + }, + }; + expect(data.features).toEqual(expectedFeatures); + }); }); From 5ec40931c6c6a721dedc9785c3e14676af6eb7a0 Mon Sep 17 00:00:00 2001 From: Eli Bishop Date: Thu, 31 May 2018 16:58:23 -0700 Subject: [PATCH 59/59] version 2.1.0 --- CHANGELOG.md | 7 +++++++ package.json | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9069cf8..4d36b559 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,13 @@ All notable changes to the LaunchDarkly client-side JavaScript SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [2.1.0] - 2018-05-31 +### Added: +- The client now sends the current SDK version to LaunchDarkly in an HTTP header. This information will be visible in a future version of the LaunchDarkly UI. + +### Fixed: +- Fixed a bug that caused summary events to combine the counts for flag evaluations that produced the flag's first variation (variation index 0) with the counts for flag evaluations that fell through to the default value. + ## [2.0.0] - 2018-05-25 ### Changed - To reduce the network bandwidth used for analytics events, feature request events are now sent as counters rather than individual events, and user details are now sent only at intervals rather than in each event. These behaviors can be modified through the LaunchDarkly UI and with the new configuration option `inlineUsersInEvents`. For more details, see [Analytics Data Stream Reference](https://docs.launchdarkly.com/v2.0/docs/analytics-data-stream-reference). diff --git a/package.json b/package.json index be47a700..62a696fe 100755 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ldclient-js", - "version": "2.0.0", + "version": "2.1.0", "description": "LaunchDarkly SDK for JavaScript", "author": "LaunchDarkly ", "license": "Apache-2.0",