From d21f52e542bb789dd73bb320bbdfb3294a4e42f8 Mon Sep 17 00:00:00 2001 From: jordangarcia Date: Tue, 23 Oct 2018 11:57:24 -0700 Subject: [PATCH 1/9] WIP Add proposed solution example --- app.js | 44 +++++++++++++++++ .../lib/core/decision_service/index.js | 47 ++++++++++--------- .../optimizely-sdk/lib/optimizely/index.js | 15 ++++++ 3 files changed, 83 insertions(+), 23 deletions(-) create mode 100644 app.js diff --git a/app.js b/app.js new file mode 100644 index 000000000..bd736e8be --- /dev/null +++ b/app.js @@ -0,0 +1,44 @@ +const fs = require('fs'); +const prompts = require('prompts') +const optimizely = require('@optimizely/optimizely-sdk'); +const datafile = require('./datafile.json') +const userProfiles = require('./userProfiles.json') + +const EXPERIMENT_KEY = 'checkout_flow_test' +const userProfile = {} +const userProfileService ={ + lookup: function(userId, cb) { + //console.log('looked up', userProfiles[userId]) + + cb(null, userProfiles[userId]) + }, + save: function(res) { + //return + profiles = JSON.parse(fs.readFileSync('userProfiles.json')) || {} + profiles[res.user_id] = res + fs.writeFileSync('userProfiles.json', JSON.stringify(profiles, null, ' '), 'utf8') + } +} +const client = optimizely.createInstance({ + datafile, + userProfileService, +}); + + +;(async function() { + //const values = await prompts({ + //type: 'text', + //name: 'userId', + //message: 'User ID', + //}) + + for (var i = 0; i < 5; i++) { + const n = `jordan-${i}` + client.activateWithUserProfileService(EXPERIMENT_KEY, n, {}, function(err, res) { + + console.log(n, res) + }) + } + + +})(); diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index 3aca9b89c..7ccbfc159 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.js @@ -61,6 +61,7 @@ function DecisionService(options) { * @return {string|null} the variation the user is bucketed into. */ DecisionService.prototype.getVariation = function(experimentKey, userId, attributes) { + attributes = attributes || {} // by default, the bucketing ID should be the user ID var bucketingId = this._getBucketingId(userId, attributes); @@ -79,8 +80,8 @@ DecisionService.prototype.getVariation = function(experimentKey, userId, attribu } // check for sticky bucketing - var userProfile = this.__getUserProfile(userId); - variation = this.__getStoredVariation(experiment, userProfile); + var experimentBucketMap = attributes.experiment_bucket_map || {} + variation = this.__getStoredVariation(experiment, experimentBucketMap); if (!!variation) { this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.RETURNING_STORED_VARIATION, MODULE_NAME, variation.key, experimentKey, userId)); return variation.key; @@ -99,9 +100,9 @@ DecisionService.prototype.getVariation = function(experimentKey, userId, attribu } // persist bucketing - this.__saveUserProfile(userProfile, experiment, variation); + this.__saveUserProfile(experiment, variation, userId, experimentBucketMap); - return variation.key; + return variation.key }; /** @@ -189,18 +190,14 @@ DecisionService.prototype.__buildBucketerParams = function(experimentKey, bucket * @param {Object} userProfile * @return {Object} the stored variation or null if the user profile does not have one for the given experiment */ -DecisionService.prototype.__getStoredVariation = function(experiment, userProfile) { - if (!userProfile || !userProfile.experiment_bucket_map) { - return null; - } - - if (userProfile.experiment_bucket_map.hasOwnProperty(experiment.id)) { - var decision = userProfile.experiment_bucket_map[experiment.id]; +DecisionService.prototype.__getStoredVariation = function(experiment, experimentBucketMap) { + if (experimentBucketMap.hasOwnProperty(experiment.id)) { + var decision = experimentBucketMap[experiment.id]; var variationId = decision.variation_id; if (this.configObj.variationIdMap.hasOwnProperty(variationId)) { return this.configObj.variationIdMap[decision.variation_id]; } else { - this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SAVED_VARIATION_NOT_FOUND, MODULE_NAME, userProfile.user_id, variationId, experiment.key)); + this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SAVED_VARIATION_NOT_FOUND, MODULE_NAME, variationId, experiment.key)); } } @@ -212,22 +209,21 @@ DecisionService.prototype.__getStoredVariation = function(experiment, userProfil * @param {string} userId * @return {Object} the stored user profile or an empty one if not found */ -DecisionService.prototype.__getUserProfile = function(userId) { +DecisionService.prototype.getUserProfile = function(userId, cb) { var userProfile = { user_id: userId, experiment_bucket_map: {}, }; if (!this.userProfileService) { - return userProfile; + return cb(null, userProfile); } try { - userProfile = this.userProfileService.lookup(userId) || userProfile; // only assign if the lookup is successful + this.userProfileService.lookup(userId, cb) } catch (ex) { this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.USER_PROFILE_LOOKUP_ERROR, MODULE_NAME, userId, ex.message)); } - return userProfile; }; /** @@ -236,20 +232,25 @@ DecisionService.prototype.__getUserProfile = function(userId) { * @param {Object} experiment * @param {Object} variation */ -DecisionService.prototype.__saveUserProfile = function(userProfile, experiment, variation) { +DecisionService.prototype.__saveUserProfile = function(experiment, variation, userId, experimentBucketMap) { if (!this.userProfileService) { return; } try { - userProfile.experiment_bucket_map[experiment.id] = { - variation_id: variation.id, - }; + newBucketMap = fns.cloneDeep(experimentBucketMap) + newBucketMap[experiment.id] = { + variation_id: variation.key + } + + this.userProfileService.save({ + user_id: userId, + experiment_bucket_map: newBucketMap, + }); - this.userProfileService.save(userProfile); - this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SAVED_VARIATION, MODULE_NAME, variation.key, experiment.key, userProfile.user_id)); + this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SAVED_VARIATION, MODULE_NAME, variation.key, experiment.key, userId)); } catch (ex) { - this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.USER_PROFILE_SAVE_ERROR, MODULE_NAME, userProfile.user_id, ex.message)); + this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.USER_PROFILE_SAVE_ERROR, MODULE_NAME, userId, ex.message)); } }; diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index 8c81d2347..fd9b3293f 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -106,6 +106,21 @@ function Optimizely(config) { }); } +Optimizely.prototype.activateWithUserProfileService = function (experimentKey, userId, attributes, cb) { + this.decisionService.getUserProfile(userId, function(err, res) { + var experiment_bucket_map = {} + if (res && res.experiment_bucket_map) { + experiment_bucket_map = res.experiment_bucket_map + } + + const variation = this.activate(experimentKey, userId, fns.assignIn({}, attributes, { + experiment_bucket_map, + })) + + cb(null, variation) + }.bind(this)) +} + /** * Buckets visitor and sends impression event to Optimizely. * @param {string} experimentKey From ba243109602a3a4d212dbe223efe94d890036aad Mon Sep 17 00:00:00 2001 From: jordangarcia Date: Wed, 31 Oct 2018 15:10:38 -0700 Subject: [PATCH 2/9] Respect both attributes and userProfileService --- .../lib/core/decision_service/index.js | 30 ++++++++++++++----- .../optimizely-sdk/lib/optimizely/index.js | 15 ---------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index 7ccbfc159..0eedf0c1b 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.js @@ -27,6 +27,8 @@ var ERROR_MESSAGES = enums.ERROR_MESSAGES; var LOG_LEVEL = enums.LOG_LEVEL; var LOG_MESSAGES = enums.LOG_MESSAGES; var DECISION_SOURCES = enums.DECISION_SOURCES; +var STICKY_BUCKETING_KEY = '$opt_experiment_bucket_map'; + /** @@ -80,8 +82,8 @@ DecisionService.prototype.getVariation = function(experimentKey, userId, attribu } // check for sticky bucketing - var experimentBucketMap = attributes.experiment_bucket_map || {} - variation = this.__getStoredVariation(experiment, experimentBucketMap); + const experimentBucketMap = this.__resolveExperimentBucketMap(userId, attributes); + variation = this.__getStoredVariation(experiment, userId, experimentBucketMap); if (!!variation) { this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.RETURNING_STORED_VARIATION, MODULE_NAME, variation.key, experimentKey, userId)); return variation.key; @@ -105,6 +107,18 @@ DecisionService.prototype.getVariation = function(experimentKey, userId, attribu return variation.key }; +/** + * Merges attributes from attributes[STICKY_BUCKETING_KEY] and userProfileService + * @param {Object} attributes + * @return {Object} finalized copy of experiment_bucket_map + */ +DecisionService.prototype.__resolveExperimentBucketMap = function(userId, attributes) { + var userProfile = this.__getUserProfile(userId) || {}; + var attributeExperimentBucketMap = attributes[STICKY_BUCKETING_KEY]; + return fns.assignIn({}, userProfile.experiment_bucket_map, attributeExperimentBucketMap); +} + + /** * Checks whether the experiment is running or launched * @param {string} experimentKey Key of experiment being validated @@ -190,14 +204,14 @@ DecisionService.prototype.__buildBucketerParams = function(experimentKey, bucket * @param {Object} userProfile * @return {Object} the stored variation or null if the user profile does not have one for the given experiment */ -DecisionService.prototype.__getStoredVariation = function(experiment, experimentBucketMap) { +DecisionService.prototype.__getStoredVariation = function(experiment, userId, experimentBucketMap) { if (experimentBucketMap.hasOwnProperty(experiment.id)) { var decision = experimentBucketMap[experiment.id]; var variationId = decision.variation_id; if (this.configObj.variationIdMap.hasOwnProperty(variationId)) { return this.configObj.variationIdMap[decision.variation_id]; } else { - this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SAVED_VARIATION_NOT_FOUND, MODULE_NAME, variationId, experiment.key)); + this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.SAVED_VARIATION_NOT_FOUND, MODULE_NAME, userId, variationId, experiment.key)); } } @@ -209,18 +223,18 @@ DecisionService.prototype.__getStoredVariation = function(experiment, experiment * @param {string} userId * @return {Object} the stored user profile or an empty one if not found */ -DecisionService.prototype.getUserProfile = function(userId, cb) { +DecisionService.prototype.__getUserProfile = function(userId) { var userProfile = { user_id: userId, experiment_bucket_map: {}, }; if (!this.userProfileService) { - return cb(null, userProfile); + return userProfile; } try { - this.userProfileService.lookup(userId, cb) + return this.userProfileService.lookup(userId) } catch (ex) { this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.USER_PROFILE_LOOKUP_ERROR, MODULE_NAME, userId, ex.message)); } @@ -240,7 +254,7 @@ DecisionService.prototype.__saveUserProfile = function(experiment, variation, us try { newBucketMap = fns.cloneDeep(experimentBucketMap) newBucketMap[experiment.id] = { - variation_id: variation.key + variation_id: variation.id } this.userProfileService.save({ diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index fd9b3293f..8c81d2347 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -106,21 +106,6 @@ function Optimizely(config) { }); } -Optimizely.prototype.activateWithUserProfileService = function (experimentKey, userId, attributes, cb) { - this.decisionService.getUserProfile(userId, function(err, res) { - var experiment_bucket_map = {} - if (res && res.experiment_bucket_map) { - experiment_bucket_map = res.experiment_bucket_map - } - - const variation = this.activate(experimentKey, userId, fns.assignIn({}, attributes, { - experiment_bucket_map, - })) - - cb(null, variation) - }.bind(this)) -} - /** * Buckets visitor and sends impression event to Optimizely. * @param {string} experimentKey From fd2c39d8fda018f5800bfba4fad2c658960d9c9b Mon Sep 17 00:00:00 2001 From: jordangarcia Date: Wed, 31 Oct 2018 15:45:20 -0700 Subject: [PATCH 3/9] Add decisionService tests for attribute sticky bucketing --- .../lib/core/decision_service/index.tests.js | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.tests.js b/packages/optimizely-sdk/lib/core/decision_service/index.tests.js index 04a050b77..724464fe0 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.tests.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.tests.js @@ -87,6 +87,22 @@ describe('lib/core/decision_service', function() { assert.strictEqual(mockLogger.log.args[0][1], 'DECISION_SERVICE: Experiment testExperimentNotRunning is not running.'); }); + describe('when attributes.$opt_experiment_bucket_map is supplied', function() { + it('should respect the sticky bucketing information for attributes', function() { + bucketerStub.returns('111128'); // ID of the 'control' variation from `test_data` + const attributes = { + $opt_experiment_bucket_map: { + '111127': { + 'variation_id': '111129' // ID of the 'variation' variation + }, + }, + }; + + assert.strictEqual('variation', decisionServiceInstance.getVariation('testExperiment', 'decision_service_user', attributes)); + sinon.assert.notCalled(bucketerStub); + }); + }); + describe('when a user profile service is provided', function () { var userProfileServiceInstance = null; var userProfileLookupStub; @@ -252,6 +268,102 @@ describe('lib/core/decision_service', function() { }, }); }); + + describe('when passing `attributes.$opt_experiment_bucket_map`', function() { + it('should respect attributes over the userProfileService for the matching experiment id', function () { + userProfileLookupStub.returns({ + user_id: 'decision_service_user', + experiment_bucket_map: { + '111127': { + 'variation_id': '111128' // ID of the 'control' variation + }, + }, + }); + + const attributes = { + $opt_experiment_bucket_map: { + '111127': { + 'variation_id': '111129' // ID of the 'variation' variation + }, + }, + }; + + + assert.strictEqual('variation', decisionServiceInstance.getVariation('testExperiment', 'decision_service_user', attributes)); + sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user'); + sinon.assert.notCalled(bucketerStub); + assert.strictEqual(mockLogger.log.args[0][1], 'PROJECT_CONFIG: User decision_service_user is not in the forced variation map.'); + assert.strictEqual(mockLogger.log.args[1][1], 'DECISION_SERVICE: Returning previously activated variation \"variation\" of experiment \"testExperiment\" for user \"decision_service_user\" from user profile.'); + }); + + it('should respect ignore attributes for a different experiment id', function () { + userProfileLookupStub.returns({ + user_id: 'decision_service_user', + experiment_bucket_map: { + '111127': { + 'variation_id': '111128' // ID of the 'control' variation + }, + }, + }); + + const attributes = { + $opt_experiment_bucket_map: { + '122227': { + 'variation_id': '122229' // ID of the 'variationWithAudience' variation + }, + }, + }; + + assert.strictEqual('control', decisionServiceInstance.getVariation('testExperiment', 'decision_service_user', attributes)); + sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user'); + sinon.assert.notCalled(bucketerStub); + assert.strictEqual(mockLogger.log.args[0][1], 'PROJECT_CONFIG: User decision_service_user is not in the forced variation map.'); + assert.strictEqual(mockLogger.log.args[1][1], 'DECISION_SERVICE: Returning previously activated variation \"control\" of experiment \"testExperiment\" for user \"decision_service_user\" from user profile.'); + }); + + it('should use attributes when the userProfileLookup returns null', function () { + userProfileLookupStub.returns({ + user_id: 'decision_service_user', + experiment_bucket_map: { + '122227': { + 'variation_id': '122229' // ID of the 'variationWithAudience' variation + }, + } + }) + + const attributes = { + $opt_experiment_bucket_map: { + '111127': { + 'variation_id': '111129' // ID of the 'variation' variation + }, + }, + }; + + assert.strictEqual('variation', decisionServiceInstance.getVariation('testExperiment', 'decision_service_user', attributes)); + sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user'); + sinon.assert.notCalled(bucketerStub); + assert.strictEqual(mockLogger.log.args[0][1], 'PROJECT_CONFIG: User decision_service_user is not in the forced variation map.'); + assert.strictEqual(mockLogger.log.args[1][1], 'DECISION_SERVICE: Returning previously activated variation \"variation\" of experiment \"testExperiment\" for user \"decision_service_user\" from user profile.'); + }); + + it('should use attributes when the userProfileLookup variations for other experiments', function () { + userProfileLookupStub.returns(null) + + const attributes = { + $opt_experiment_bucket_map: { + '111127': { + 'variation_id': '111129' // ID of the 'variation' variation + }, + }, + }; + + assert.strictEqual('variation', decisionServiceInstance.getVariation('testExperiment', 'decision_service_user', attributes)); + sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user'); + sinon.assert.notCalled(bucketerStub); + assert.strictEqual(mockLogger.log.args[0][1], 'PROJECT_CONFIG: User decision_service_user is not in the forced variation map.'); + assert.strictEqual(mockLogger.log.args[1][1], 'DECISION_SERVICE: Returning previously activated variation \"variation\" of experiment \"testExperiment\" for user \"decision_service_user\" from user profile.'); + }); + }); }); }); @@ -459,6 +571,7 @@ describe('lib/core/decision_service', function() { 'test_user', userAttributesWithBucketingId )); + sinon.assert.calledWithExactly(userProfileLookupStub, 'test_user'); }); }); From f4847df74068cb769686f5e30282d6e9cb27b3b9 Mon Sep 17 00:00:00 2001 From: jordangarcia Date: Wed, 31 Oct 2018 16:23:09 -0700 Subject: [PATCH 4/9] Add API level tests --- .../optimizely-sdk/lib/optimizely/index.tests.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/optimizely-sdk/lib/optimizely/index.tests.js b/packages/optimizely-sdk/lib/optimizely/index.tests.js index a6f251d05..ad4b57267 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely/index.tests.js @@ -639,6 +639,22 @@ describe('lib/optimizely', function() { JSON.stringify(expectedObj.params))); }); + describe('when experiment_bucket_map attribute is present', function() { + it('should call activate and respect attribute experiment_bucket_map', function() { + bucketStub.returns('111128'); // id of "control" variation + var activate = optlyInstance.activate('testExperiment', 'testUser', { + $opt_experiment_bucket_map: { + '111127': { + variation_id: '111129', // id of "variation" variation + }, + }, + }); + + assert.strictEqual(activate, 'variation'); + sinon.assert.notCalled(bucketer.bucket); + }); + }); + it('should call bucketer and dispatchEvent with proper args and return variation key if user is in grouped experiment', function() { bucketStub.returns('662'); var activate = optlyInstance.activate('groupExperiment2', 'testUser'); From c57c0cbd34f94a7089913d3ad3c09f80976e1bdf Mon Sep 17 00:00:00 2001 From: jordangarcia Date: Wed, 31 Oct 2018 16:23:43 -0700 Subject: [PATCH 5/9] Remove app.js --- app.js | 44 -------------------------------------------- 1 file changed, 44 deletions(-) delete mode 100644 app.js diff --git a/app.js b/app.js deleted file mode 100644 index bd736e8be..000000000 --- a/app.js +++ /dev/null @@ -1,44 +0,0 @@ -const fs = require('fs'); -const prompts = require('prompts') -const optimizely = require('@optimizely/optimizely-sdk'); -const datafile = require('./datafile.json') -const userProfiles = require('./userProfiles.json') - -const EXPERIMENT_KEY = 'checkout_flow_test' -const userProfile = {} -const userProfileService ={ - lookup: function(userId, cb) { - //console.log('looked up', userProfiles[userId]) - - cb(null, userProfiles[userId]) - }, - save: function(res) { - //return - profiles = JSON.parse(fs.readFileSync('userProfiles.json')) || {} - profiles[res.user_id] = res - fs.writeFileSync('userProfiles.json', JSON.stringify(profiles, null, ' '), 'utf8') - } -} -const client = optimizely.createInstance({ - datafile, - userProfileService, -}); - - -;(async function() { - //const values = await prompts({ - //type: 'text', - //name: 'userId', - //message: 'User ID', - //}) - - for (var i = 0; i < 5; i++) { - const n = `jordan-${i}` - client.activateWithUserProfileService(EXPERIMENT_KEY, n, {}, function(err, res) { - - console.log(n, res) - }) - } - - -})(); From c7a107f4e394923cbc4ea5fa4c6cc55f0872e3d7 Mon Sep 17 00:00:00 2001 From: jordangarcia Date: Wed, 31 Oct 2018 16:38:00 -0700 Subject: [PATCH 6/9] Fix lint --- .../lib/core/decision_service/index.js | 14 +++++++------- .../lib/core/decision_service/index.tests.js | 16 ++++++++-------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index 0eedf0c1b..550ea4d49 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.js @@ -63,7 +63,7 @@ function DecisionService(options) { * @return {string|null} the variation the user is bucketed into. */ DecisionService.prototype.getVariation = function(experimentKey, userId, attributes) { - attributes = attributes || {} + attributes = attributes || {}; // by default, the bucketing ID should be the user ID var bucketingId = this._getBucketingId(userId, attributes); @@ -82,7 +82,7 @@ DecisionService.prototype.getVariation = function(experimentKey, userId, attribu } // check for sticky bucketing - const experimentBucketMap = this.__resolveExperimentBucketMap(userId, attributes); + var experimentBucketMap = this.__resolveExperimentBucketMap(userId, attributes); variation = this.__getStoredVariation(experiment, userId, experimentBucketMap); if (!!variation) { this.logger.log(LOG_LEVEL.INFO, sprintf(LOG_MESSAGES.RETURNING_STORED_VARIATION, MODULE_NAME, variation.key, experimentKey, userId)); @@ -104,7 +104,7 @@ DecisionService.prototype.getVariation = function(experimentKey, userId, attribu // persist bucketing this.__saveUserProfile(experiment, variation, userId, experimentBucketMap); - return variation.key + return variation.key; }; /** @@ -116,7 +116,7 @@ DecisionService.prototype.__resolveExperimentBucketMap = function(userId, attrib var userProfile = this.__getUserProfile(userId) || {}; var attributeExperimentBucketMap = attributes[STICKY_BUCKETING_KEY]; return fns.assignIn({}, userProfile.experiment_bucket_map, attributeExperimentBucketMap); -} +}; /** @@ -234,7 +234,7 @@ DecisionService.prototype.__getUserProfile = function(userId) { } try { - return this.userProfileService.lookup(userId) + return this.userProfileService.lookup(userId); } catch (ex) { this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.USER_PROFILE_LOOKUP_ERROR, MODULE_NAME, userId, ex.message)); } @@ -252,10 +252,10 @@ DecisionService.prototype.__saveUserProfile = function(experiment, variation, us } try { - newBucketMap = fns.cloneDeep(experimentBucketMap) + newBucketMap = fns.cloneDeep(experimentBucketMap); newBucketMap[experiment.id] = { variation_id: variation.id - } + }; this.userProfileService.save({ user_id: userId, diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.tests.js b/packages/optimizely-sdk/lib/core/decision_service/index.tests.js index 724464fe0..a3b48c283 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.tests.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.tests.js @@ -90,7 +90,7 @@ describe('lib/core/decision_service', function() { describe('when attributes.$opt_experiment_bucket_map is supplied', function() { it('should respect the sticky bucketing information for attributes', function() { bucketerStub.returns('111128'); // ID of the 'control' variation from `test_data` - const attributes = { + var attributes = { $opt_experiment_bucket_map: { '111127': { 'variation_id': '111129' // ID of the 'variation' variation @@ -280,7 +280,7 @@ describe('lib/core/decision_service', function() { }, }); - const attributes = { + var attributes = { $opt_experiment_bucket_map: { '111127': { 'variation_id': '111129' // ID of the 'variation' variation @@ -306,7 +306,7 @@ describe('lib/core/decision_service', function() { }, }); - const attributes = { + var attributes = { $opt_experiment_bucket_map: { '122227': { 'variation_id': '122229' // ID of the 'variationWithAudience' variation @@ -329,9 +329,9 @@ describe('lib/core/decision_service', function() { 'variation_id': '122229' // ID of the 'variationWithAudience' variation }, } - }) + }); - const attributes = { + var attributes = { $opt_experiment_bucket_map: { '111127': { 'variation_id': '111129' // ID of the 'variation' variation @@ -347,9 +347,9 @@ describe('lib/core/decision_service', function() { }); it('should use attributes when the userProfileLookup variations for other experiments', function () { - userProfileLookupStub.returns(null) + userProfileLookupStub.returns(null); - const attributes = { + var attributes = { $opt_experiment_bucket_map: { '111127': { 'variation_id': '111129' // ID of the 'variation' variation @@ -588,7 +588,7 @@ describe('lib/core/decision_service', function() { 'browser_type': 'safari', '$opt_bucketing_id': 50 }; - + beforeEach(function() { sinon.stub(mockLogger, 'log'); configObj = projectConfig.createProjectConfig(testData); From c571d2b667e9285e718e99b9835a3335d40f2dd2 Mon Sep 17 00:00:00 2001 From: jordangarcia Date: Thu, 1 Nov 2018 10:15:31 -0700 Subject: [PATCH 7/9] Respond to PR feedback --- .../lib/core/decision_service/index.js | 9 +++++---- .../lib/core/decision_service/index.tests.js | 14 +++++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index 550ea4d49..97273f05d 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.js @@ -63,7 +63,6 @@ function DecisionService(options) { * @return {string|null} the variation the user is bucketed into. */ DecisionService.prototype.getVariation = function(experimentKey, userId, attributes) { - attributes = attributes || {}; // by default, the bucketing ID should be the user ID var bucketingId = this._getBucketingId(userId, attributes); @@ -113,6 +112,7 @@ DecisionService.prototype.getVariation = function(experimentKey, userId, attribu * @return {Object} finalized copy of experiment_bucket_map */ DecisionService.prototype.__resolveExperimentBucketMap = function(userId, attributes) { + attributes = attributes || {} var userProfile = this.__getUserProfile(userId) || {}; var attributeExperimentBucketMap = attributes[STICKY_BUCKETING_KEY]; return fns.assignIn({}, userProfile.experiment_bucket_map, attributeExperimentBucketMap); @@ -201,7 +201,8 @@ DecisionService.prototype.__buildBucketerParams = function(experimentKey, bucket /** * Get the stored variation from the user profile for the given experiment * @param {Object} experiment - * @param {Object} userProfile + * @param {String} userId + * @param {Object} experimentBucketMap mapping experiment => { variation_id: } * @return {Object} the stored variation or null if the user profile does not have one for the given experiment */ DecisionService.prototype.__getStoredVariation = function(experiment, userId, experimentBucketMap) { @@ -221,7 +222,7 @@ DecisionService.prototype.__getStoredVariation = function(experiment, userId, ex /** * Get the user profile with the given user ID * @param {string} userId - * @return {Object} the stored user profile or an empty one if not found + * @return {Object|undefined} the stored user profile or undefined if one isn't found */ DecisionService.prototype.__getUserProfile = function(userId) { var userProfile = { @@ -252,7 +253,7 @@ DecisionService.prototype.__saveUserProfile = function(experiment, variation, us } try { - newBucketMap = fns.cloneDeep(experimentBucketMap); + var newBucketMap = fns.cloneDeep(experimentBucketMap); newBucketMap[experiment.id] = { variation_id: variation.id }; diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.tests.js b/packages/optimizely-sdk/lib/core/decision_service/index.tests.js index a3b48c283..2bb4b74c9 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.tests.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.tests.js @@ -296,11 +296,11 @@ describe('lib/core/decision_service', function() { assert.strictEqual(mockLogger.log.args[1][1], 'DECISION_SERVICE: Returning previously activated variation \"variation\" of experiment \"testExperiment\" for user \"decision_service_user\" from user profile.'); }); - it('should respect ignore attributes for a different experiment id', function () { + it('should ignore attributes for a different experiment id', function () { userProfileLookupStub.returns({ user_id: 'decision_service_user', experiment_bucket_map: { - '111127': { + '111127': { // 'testExperiment' ID 'variation_id': '111128' // ID of the 'control' variation }, }, @@ -308,7 +308,7 @@ describe('lib/core/decision_service', function() { var attributes = { $opt_experiment_bucket_map: { - '122227': { + '122227': { // other experiment ID 'variation_id': '122229' // ID of the 'variationWithAudience' variation }, }, @@ -321,11 +321,11 @@ describe('lib/core/decision_service', function() { assert.strictEqual(mockLogger.log.args[1][1], 'DECISION_SERVICE: Returning previously activated variation \"control\" of experiment \"testExperiment\" for user \"decision_service_user\" from user profile.'); }); - it('should use attributes when the userProfileLookup returns null', function () { + it('should use attributes when the userProfileLookup variations for other experiments', function () { userProfileLookupStub.returns({ user_id: 'decision_service_user', experiment_bucket_map: { - '122227': { + '122227': { // other experiment ID 'variation_id': '122229' // ID of the 'variationWithAudience' variation }, } @@ -333,7 +333,7 @@ describe('lib/core/decision_service', function() { var attributes = { $opt_experiment_bucket_map: { - '111127': { + '111127': { // 'testExperiment' ID 'variation_id': '111129' // ID of the 'variation' variation }, }, @@ -346,7 +346,7 @@ describe('lib/core/decision_service', function() { assert.strictEqual(mockLogger.log.args[1][1], 'DECISION_SERVICE: Returning previously activated variation \"variation\" of experiment \"testExperiment\" for user \"decision_service_user\" from user profile.'); }); - it('should use attributes when the userProfileLookup variations for other experiments', function () { + it('should use attributes when the userProfileLookup returns null', function () { userProfileLookupStub.returns(null); var attributes = { From 04dca741866b07b389e69dec4b32fa15e8849fbd Mon Sep 17 00:00:00 2001 From: jordangarcia Date: Thu, 1 Nov 2018 10:17:14 -0700 Subject: [PATCH 8/9] Move enum --- packages/optimizely-sdk/lib/core/decision_service/index.js | 3 +-- packages/optimizely-sdk/lib/utils/enums/index.js | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index 97273f05d..ee064d6ed 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.js @@ -27,7 +27,6 @@ var ERROR_MESSAGES = enums.ERROR_MESSAGES; var LOG_LEVEL = enums.LOG_LEVEL; var LOG_MESSAGES = enums.LOG_MESSAGES; var DECISION_SOURCES = enums.DECISION_SOURCES; -var STICKY_BUCKETING_KEY = '$opt_experiment_bucket_map'; @@ -114,7 +113,7 @@ DecisionService.prototype.getVariation = function(experimentKey, userId, attribu DecisionService.prototype.__resolveExperimentBucketMap = function(userId, attributes) { attributes = attributes || {} var userProfile = this.__getUserProfile(userId) || {}; - var attributeExperimentBucketMap = attributes[STICKY_BUCKETING_KEY]; + var attributeExperimentBucketMap = attributes[enums.CONTROL_ATTRIBUTES.STICKY_BUCKETING_KEY]; return fns.assignIn({}, userProfile.experiment_bucket_map, attributeExperimentBucketMap); }; diff --git a/packages/optimizely-sdk/lib/utils/enums/index.js b/packages/optimizely-sdk/lib/utils/enums/index.js index cdd42d12c..65636dd66 100644 --- a/packages/optimizely-sdk/lib/utils/enums/index.js +++ b/packages/optimizely-sdk/lib/utils/enums/index.js @@ -136,6 +136,7 @@ exports.CONTROL_ATTRIBUTES = { BOT_FILTERING: '$opt_bot_filtering', BUCKETING_ID: '$opt_bucketing_id', USER_AGENT: '$opt_user_agent', + STICKY_BUCKETING_KEY: '$opt_experiment_bucket_map', }; exports.JAVASCRIPT_CLIENT_ENGINE = 'javascript-sdk'; From 1b8c5e85c297d2dbff9879107a971bcf8892b49a Mon Sep 17 00:00:00 2001 From: jordangarcia Date: Wed, 7 Nov 2018 15:19:34 -0800 Subject: [PATCH 9/9] Address PR comments --- packages/optimizely-sdk/lib/core/decision_service/index.js | 3 ++- packages/optimizely-sdk/lib/utils/enums/index.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index ee064d6ed..be26bb2f0 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.js @@ -198,7 +198,7 @@ DecisionService.prototype.__buildBucketerParams = function(experimentKey, bucket }; /** - * Get the stored variation from the user profile for the given experiment + * Pull the stored variation out of the experimentBucketMap for an experiment/userId * @param {Object} experiment * @param {String} userId * @param {Object} experimentBucketMap mapping experiment => { variation_id: } @@ -245,6 +245,7 @@ DecisionService.prototype.__getUserProfile = function(userId) { * @param {Object} userProfile * @param {Object} experiment * @param {Object} variation + * @param {Object} experimentBucketMap */ DecisionService.prototype.__saveUserProfile = function(experiment, variation, userId, experimentBucketMap) { if (!this.userProfileService) { diff --git a/packages/optimizely-sdk/lib/utils/enums/index.js b/packages/optimizely-sdk/lib/utils/enums/index.js index 65636dd66..f316e24c1 100644 --- a/packages/optimizely-sdk/lib/utils/enums/index.js +++ b/packages/optimizely-sdk/lib/utils/enums/index.js @@ -135,8 +135,8 @@ exports.RESERVED_EVENT_KEYWORDS = { exports.CONTROL_ATTRIBUTES = { BOT_FILTERING: '$opt_bot_filtering', BUCKETING_ID: '$opt_bucketing_id', - USER_AGENT: '$opt_user_agent', STICKY_BUCKETING_KEY: '$opt_experiment_bucket_map', + USER_AGENT: '$opt_user_agent', }; exports.JAVASCRIPT_CLIENT_ENGINE = 'javascript-sdk';