diff --git a/evaluate_flag.js b/evaluate_flag.js index 11abe0a..86a1bc3 100644 --- a/evaluate_flag.js +++ b/evaluate_flag.js @@ -3,8 +3,12 @@ var dataKind = require('./versioned_data_kind'); var util = require('util'); var sha1 = require('node-sha1'); var async = require('async'); +var stringifyAttrs = require('./utils/stringifyAttrs'); var builtins = ['key', 'ip', 'country', 'email', 'firstName', 'lastName', 'avatar', 'name', 'anonymous']; +var userAttrsToStringifyForEvaluation = [ "key", "secondary" ]; +// Currently we are not stringifying the rest of the built-in attributes prior to evaluation, only for events. +// This is because it could affect evaluation results for existing users (ch35206). var noop = function(){}; @@ -22,8 +26,9 @@ function evaluate(flag, user, featureStore, cb) { return; } + var sanitizedUser = stringifyAttrs(user, userAttrsToStringifyForEvaluation); var events = []; - evalInternal(flag, user, featureStore, events, function(err, detail) { + evalInternal(flag, sanitizedUser, featureStore, events, function(err, detail) { cb(err, detail, events); }); } diff --git a/event_processor.js b/event_processor.js index cc27216..051ffb2 100644 --- a/event_processor.js +++ b/event_processor.js @@ -4,8 +4,11 @@ var EventSummarizer = require('./event_summarizer'); var UserFilter = require('./user_filter'); var errors = require('./errors'); var messages = require('./messages'); +var stringifyAttrs = require('./utils/stringifyAttrs'); var wrapPromiseCallback = require('./utils/wrapPromiseCallback'); +var userAttrsToStringifyForEvents = [ "key", "secondary", "ip", "country", "email", "firstName", "lastName", "avatar", "name" ]; + function EventProcessor(sdkKey, config, errorReporter) { var ep = {}; @@ -63,17 +66,17 @@ function EventProcessor(sdkKey, config, errorReporter) { out.reason = event.reason; } if (config.inlineUsersInEvents || debug) { - out.user = userFilter.filterUser(event.user); + out.user = processUser(event); } else { - out.userKey = event.user && event.user.key; + out.userKey = getUserKey(event); } return out; case 'identify': return { kind: 'identify', creationDate: event.creationDate, - key: event.user && event.user.key, - user: userFilter.filterUser(event.user) + key: getUserKey(event), + user: processUser(event) }; case 'custom': var out = { @@ -83,9 +86,9 @@ function EventProcessor(sdkKey, config, errorReporter) { data: event.data }; if (config.inlineUsersInEvents) { - out.user = userFilter.filterUser(event.user); + out.user = processUser(event); } else { - out.userKey = event.user && event.user.key; + out.userKey = getUserKey(event); } return out; default: @@ -93,6 +96,15 @@ function EventProcessor(sdkKey, config, errorReporter) { } } + function processUser(event) { + var filtered = userFilter.filterUser(event.user); + return stringifyAttrs(filtered, userAttrsToStringifyForEvents); + } + + function getUserKey(event) { + return event.user && String(event.user.key); + } + ep.sendEvent = function(event) { var addIndexEvent = false, addFullEvent = false, @@ -129,7 +141,7 @@ function EventProcessor(sdkKey, config, errorReporter) { enqueue({ kind: 'index', creationDate: event.creationDate, - user: userFilter.filterUser(event.user) + user: processUser(event) }); } if (addFullEvent) { diff --git a/index.js b/index.js index bf6e150..9563266 100644 --- a/index.js +++ b/index.js @@ -213,7 +213,6 @@ var newClient = function(sdkKey, config) { return resolve(errorResult('FLAG_NOT_FOUND', defaultVal)); } - sanitizeUser(user); if (user && user.key === "") { config.logger.warn("User key is blank. Flag evaluation will proceed, but the user will not be stored in LaunchDarkly"); } @@ -286,8 +285,6 @@ var newClient = function(sdkKey, config) { options = options || {}; } return wrapPromiseCallback(new Promise(function(resolve, reject) { - sanitizeUser(user); - if (this.isOffline()) { config.logger.info("allFlagsState() called in offline mode. Returning empty state."); return resolve(FlagsStateBuilder(false).build()); @@ -347,7 +344,6 @@ var newClient = function(sdkKey, config) { return; } - sanitizeUser(user); var event = {"key": eventName, "user": user, "kind": "custom", @@ -366,8 +362,7 @@ var newClient = function(sdkKey, config) { return; } - sanitizeUser(user); - var event = {"key": user.key, + var event = {"key": String(user.key), "kind": "identify", "user": user, "creationDate": new Date().getTime()}; @@ -434,13 +429,3 @@ function createProxyAgent(config) { return tunnel.httpOverHttp(options); } } - - -function sanitizeUser(u) { - if (!u) { - return; - } - if (u['key']) { - u['key'] = u['key'].toString(); - } -} diff --git a/test/evaluate_flag-test.js b/test/evaluate_flag-test.js index 61367da..00bdfb9 100644 --- a/test/evaluate_flag-test.js +++ b/test/evaluate_flag-test.js @@ -22,12 +22,6 @@ function defineSegment(segment, cb) { setTimeout(cb, 0); } -function evalBooleanFlag(flag, user, cb) { - evaluate.evaluate(flag, user, featureStore, function(err, result) { - cb(result); - }); -} - function makeFlagWithRules(rules, fallthrough) { if (!fallthrough) { fallthrough = { variation: 0 }; @@ -43,19 +37,23 @@ function makeFlagWithRules(rules, fallthrough) { }; } -function makeBooleanFlagWithOneClause(clause) { +function makeBooleanFlagWithRules(rules) { return { - key: 'feature', - on: true, - prerequisites: [], - rules: [ { clauses: [ clause ], variation: 1 } ], - targets: [], - salt: "", - fallthrough: { variation: 0 }, - offVariation: 0, - variations: [ false, true ], - version: 1 - }; + key: 'feature', + on: true, + prerequisites: [], + rules: rules, + targets: [], + salt: "", + fallthrough: { variation: 0 }, + offVariation: 0, + variations: [ false, true ], + version: 1 + }; +} + +function makeBooleanFlagWithOneClause(clause) { + return makeBooleanFlagWithRules([ { clauses: [ clause ], variation: 1 } ]); } function makeFlagWithSegmentMatch(segment) { @@ -372,6 +370,37 @@ describe('evaluate', function() { }); }); + it('coerces user key to string', function(done) { + var clause = { 'attribute': 'key', 'op': 'in', 'values': [ '999' ] }; + var flag = makeBooleanFlagWithOneClause(clause); + var user = { 'key': 999 }; + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail.value).toBe(true); + done(); + }); + }); + + it('coerces secondary key to string', function(done) { + // We can't really verify that the rollout calculation works correctly, but we can at least + // make sure it doesn't error out if there's a non-string secondary value (ch35189) + var rule = { + id: 'ruleid', + clauses: [ + { attribute: 'key', op: 'in', values: [ 'userkey' ] } + ], + rollout: { + salt: '', + variations: [ { weight: 100000, variation: 1 } ] + } + }; + var flag = makeBooleanFlagWithRules([ rule ]); + var user = { key: 'userkey', secondary: 999 }; + evaluate.evaluate(flag, user, featureStore, function(err, detail, events) { + expect(detail.value).toBe(true); + done(); + }); + }); + it('matches user from targets', function(done) { var flag = { key: 'feature0', diff --git a/test/event_processor-test.js b/test/event_processor-test.js index 75646ce..0d97a51 100644 --- a/test/event_processor-test.js +++ b/test/event_processor-test.js @@ -1,6 +1,5 @@ var nock = require('nock'); var EventProcessor = require('../event_processor'); -var EventEmitter = require('events').EventEmitter; describe('EventProcessor', function() { @@ -20,6 +19,10 @@ describe('EventProcessor', function() { }; var user = { key: 'userKey', name: 'Red' }; var filteredUser = { key: 'userKey', privateAttrs: [ 'name' ] }; + var numericUser = { key: 1, secondary: 2, ip: 3, country: 4, email: 5, firstName: 6, lastName: 7, + avatar: 8, name: 9, anonymous: false, custom: { age: 99 } }; + var stringifiedNumericUser = { key: '1', secondary: '2', ip: '3', country: '4', email: '5', firstName: '6', + lastName: '7', avatar: '8', name: '9', anonymous: false, custom: { age: 99 } }; afterEach(function() { if (ep) { @@ -70,7 +73,7 @@ describe('EventProcessor', function() { if (inlineUser) { expect(e.user).toEqual(inlineUser); } else { - expect(e.userKey).toEqual(source.user.key); + expect(e.userKey).toEqual(String(source.user.key)); } } @@ -123,6 +126,22 @@ describe('EventProcessor', function() { }); }); + it('stringifies user attributes in identify event', function(done) { + ep = EventProcessor(sdkKey, defaultConfig); + var e = { kind: 'identify', creationDate: 1000, user: numericUser }; + ep.sendEvent(e); + + flushAndGetRequest(function(output) { + expect(output).toEqual([{ + kind: 'identify', + creationDate: 1000, + key: stringifiedNumericUser.key, + user: stringifiedNumericUser + }]); + done(); + }); + }); + it('queues individual feature event with index event', function(done) { ep = EventProcessor(sdkKey, defaultConfig); var e = { kind: 'feature', creationDate: 1000, user: user, key: 'flagkey', @@ -154,6 +173,21 @@ describe('EventProcessor', function() { }); }); + it('stringifies user attributes in index event', function(done) { + ep = EventProcessor(sdkKey, defaultConfig); + var e = { kind: 'feature', creationDate: 1000, user: numericUser, key: 'flagkey', + version: 11, variation: 1, value: 'value', trackEvents: true }; + ep.sendEvent(e); + + flushAndGetRequest(function(output) { + expect(output.length).toEqual(3); + checkIndexEvent(output[0], e, stringifiedNumericUser); + checkFeatureEvent(output[1], e, false); + checkSummaryEvent(output[2]); + done(); + }); + }); + it('can include inline user in feature event', function(done) { var config = Object.assign({}, defaultConfig, { inlineUsersInEvents: true }); ep = EventProcessor(sdkKey, config); @@ -185,6 +219,21 @@ describe('EventProcessor', function() { }); }); + it('stringifies user attributes in feature event', function(done) { + var config = Object.assign({}, defaultConfig, { inlineUsersInEvents: true }); + ep = EventProcessor(sdkKey, config); + var e = { kind: 'feature', creationDate: 1000, user: numericUser, key: 'flagkey', + version: 11, variation: 1, value: 'value', trackEvents: true }; + ep.sendEvent(e); + + flushAndGetRequest(function(output) { + expect(output.length).toEqual(2); + checkFeatureEvent(output[0], e, false, stringifiedNumericUser); + checkSummaryEvent(output[1]); + done(); + }); + }); + it('can include reason in feature event', function(done) { var config = Object.assign({}, defaultConfig, { inlineUsersInEvents: true }); ep = EventProcessor(sdkKey, config); @@ -378,6 +427,20 @@ describe('EventProcessor', function() { }); }); + it('stringifies user attributes in custom event', function(done) { + var config = Object.assign({}, defaultConfig, { inlineUsersInEvents: true }); + ep = EventProcessor(sdkKey, config); + var e = { kind: 'custom', creationDate: 1000, user: numericUser, key: 'eventkey', + data: { thing: 'stuff' } }; + ep.sendEvent(e); + + flushAndGetRequest(function(output) { + expect(output.length).toEqual(1); + checkCustomEvent(output[0], e, stringifiedNumericUser); + done(); + }); + }); + it('filters user in custom event', function(done) { var config = Object.assign({}, defaultConfig, { allAttributesPrivate: true, inlineUsersInEvents: true }); diff --git a/utils/stringifyAttrs.js b/utils/stringifyAttrs.js new file mode 100644 index 0000000..ee77864 --- /dev/null +++ b/utils/stringifyAttrs.js @@ -0,0 +1,16 @@ + +module.exports = function stringifyAttrs(object, attrs) { + if (!object) { + return object; + } + var newObject; + for (var i in attrs) { + var attr = attrs[i]; + var value = object[attr]; + if (value !== undefined && typeof value !== 'string') { + newObject = newObject || Object.assign({}, object); + newObject[attr] = String(value); + } + } + return newObject || object; +}