From 649962b12bcb65fcfc38c6fb105de291de9167dc Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Tue, 18 Sep 2018 21:46:57 +0500 Subject: [PATCH 1/7] Add attribute types support. --- .../core/condition_evaluator/index.tests.js | 16 +++++++++- .../lib/core/decision_service/index.js | 32 +++++++++++++------ .../lib/core/project_config/index.tests.js | 5 ++- .../optimizely-sdk/lib/tests/test_data.js | 9 ++++++ .../optimizely-sdk/lib/utils/enums/index.js | 2 ++ 5 files changed, 53 insertions(+), 11 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/condition_evaluator/index.tests.js b/packages/optimizely-sdk/lib/core/condition_evaluator/index.tests.js index cea5ff2f5..52bc02aff 100644 --- a/packages/optimizely-sdk/lib/core/condition_evaluator/index.tests.js +++ b/packages/optimizely-sdk/lib/core/condition_evaluator/index.tests.js @@ -1,5 +1,5 @@ /** - * Copyright 2016, Optimizely + * Copyright 2016, 2018, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,9 @@ var conditionEvaluator = require('./'); var browserConditionSafari = {'name': 'browser_type', 'value': 'safari'}; var deviceConditionIphone = {'name': 'device_model', 'value': 'iphone6'}; +var booleanCondition = {'name': 'is_firefox', 'value': true}; +var integerCondition = {'name': 'num_users', 'value': 10}; +var doubleCondition = {'name': 'pi_value', 'value': 3.14}; describe('lib/core/condition_evaluator', function() { describe('APIs', function() { @@ -39,6 +42,17 @@ describe('lib/core/condition_evaluator', function() { assert.isFalse(conditionEvaluator.evaluate(['and', browserConditionSafari], userAttributes)); }); + it('should evaluate typed attributes', function() { + var userAttributes = { + browser_type: 'safari', + is_firefox: true, + num_users: 10, + pi_value: 3.14, + }; + + assert.isTrue(conditionEvaluator.evaluate(['and', browserConditionSafari, booleanCondition, integerCondition, doubleCondition], userAttributes)); + }); + describe('and evaluation', function() { it('should return true when ALL conditions evaluate to true', function() { var userAttributes = { diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index 214473316..3ffb7dd4b 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.js @@ -62,15 +62,7 @@ function DecisionService(options) { */ DecisionService.prototype.getVariation = function(experimentKey, userId, attributes) { // by default, the bucketing ID should be the user ID - var bucketingId = userId; - - // If the bucketing ID key is defined in attributes, than use that in place of the userID for the murmur hash key - if (!fns.isEmpty(attributes)) { - if (attributes.hasOwnProperty(enums.CONTROL_ATTRIBUTES.BUCKETING_ID)) { - bucketingId = attributes[enums.CONTROL_ATTRIBUTES.BUCKETING_ID]; - this.logger.log(LOG_LEVEL.DEBUG, sprintf('Setting the bucketing ID to %s.', bucketingId)); - } - } + var bucketingId = this._getBucketingId(userId, attributes); if (!this.__checkIfExperimentIsActive(experimentKey, userId)) { return null; @@ -432,6 +424,28 @@ DecisionService.prototype._getVariationForRollout = function(feature, userId, at }; }; +/** + * Get bucketing Id from user attributes. + * @param {String} userId + * @param {Object} attributes + * @returns {String} Bucketing Id if it is a string type in attributes, user Id otherwise. + */ +DecisionService.prototype._getBucketingId = function(userId, attributes) { + var bucketingId = userId; + + // If the bucketing ID key is defined in attributes, than use that in place of the userID for the murmur hash key + if (!fns.isEmpty(attributes) && attributes.hasOwnProperty(enums.CONTROL_ATTRIBUTES.BUCKETING_ID)) { + if (typeof attributes[enums.CONTROL_ATTRIBUTES.BUCKETING_ID] === 'string') { + bucketingId = attributes[enums.CONTROL_ATTRIBUTES.BUCKETING_ID]; + this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.VALID_BUCKETING_ID, bucketingId)); + } else { + this.logger.log(LOG_LEVEL.WARNING, LOG_MESSAGES.BUCKETING_ID_NOT_STRING); + } + } + + return bucketingId; +}; + module.exports = { /** * Creates an instance of the DecisionService. diff --git a/packages/optimizely-sdk/lib/core/project_config/index.tests.js b/packages/optimizely-sdk/lib/core/project_config/index.tests.js index 5daaef45c..9fbda9f67 100644 --- a/packages/optimizely-sdk/lib/core/project_config/index.tests.js +++ b/packages/optimizely-sdk/lib/core/project_config/index.tests.js @@ -1,5 +1,5 @@ /** - * Copyright 2016-2017, Optimizely + * Copyright 2016-2018, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -70,6 +70,9 @@ describe('lib/core/project_config', function() { var expectedAttributeKeyMap = { browser_type: testData.attributes[0], + boolean_key: testData.attributes[1], + integer_key: testData.attributes[2], + double_key: testData.attributes[3], }; assert.deepEqual(configObj.attributeKeyMap, expectedAttributeKeyMap); diff --git a/packages/optimizely-sdk/lib/tests/test_data.js b/packages/optimizely-sdk/lib/tests/test_data.js index ac0ca1878..85ca8e0bf 100644 --- a/packages/optimizely-sdk/lib/tests/test_data.js +++ b/packages/optimizely-sdk/lib/tests/test_data.js @@ -239,6 +239,15 @@ var config = { attributes: [{ key: 'browser_type', id: '111094' + }, { + id: "323434545", + key: "boolean_key" + }, { + id: "616727838", + key: "integer_key" + }, { + id: "808797686", + key: "double_key" } ], audiences: [{ diff --git a/packages/optimizely-sdk/lib/utils/enums/index.js b/packages/optimizely-sdk/lib/utils/enums/index.js index f54bba1fd..25507fe88 100644 --- a/packages/optimizely-sdk/lib/utils/enums/index.js +++ b/packages/optimizely-sdk/lib/utils/enums/index.js @@ -123,6 +123,8 @@ exports.LOG_MESSAGES = { VALID_USER_PROFILE_SERVICE: '%s: Valid user profile service provided.', VARIATION_REMOVED_FOR_USER: '%s: Variation mapped to experiment %s has been removed for user %s.', VARIABLE_REQUESTED_WITH_WRONG_TYPE: '%s: Requested variable type "%s", but variable is of type "%s". Use correct API to retrieve value. Returning None.', + VALID_BUCKETING_ID: '%s: BucketingId is valid: "%s"', + BUCKETING_ID_NOT_STRING: 'BucketingID attribute is not a string. Defaulted to userId', }; exports.RESERVED_EVENT_KEYWORDS = { From 402f8e859ae23957abed8865e3207bdba35ccad4 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Wed, 19 Sep 2018 21:58:43 +0500 Subject: [PATCH 2/7] Add unit tests for attribute types support and event payload attributes validation. --- .../core/condition_evaluator/index.tests.js | 2 +- .../lib/core/decision_service/index.js | 6 +- .../lib/core/decision_service/index.tests.js | 47 ++++++- .../lib/core/event_builder/index.js | 21 +-- .../lib/core/event_builder/index.tests.js | 132 ++++++++++++++++++ .../lib/optimizely/index.tests.js | 78 +++++++++++ .../lib/utils/attributes_validator/index.js | 8 +- .../utils/attributes_validator/index.tests.js | 31 +++- .../optimizely-sdk/lib/utils/enums/index.js | 2 +- 9 files changed, 310 insertions(+), 17 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/condition_evaluator/index.tests.js b/packages/optimizely-sdk/lib/core/condition_evaluator/index.tests.js index 52bc02aff..8ea30c2d5 100644 --- a/packages/optimizely-sdk/lib/core/condition_evaluator/index.tests.js +++ b/packages/optimizely-sdk/lib/core/condition_evaluator/index.tests.js @@ -42,7 +42,7 @@ describe('lib/core/condition_evaluator', function() { assert.isFalse(conditionEvaluator.evaluate(['and', browserConditionSafari], userAttributes)); }); - it('should evaluate typed attributes', function() { + it('should evaluate different typed attributes', function() { var userAttributes = { browser_type: 'safari', is_firefox: true, diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index 3ffb7dd4b..1648da390 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.js @@ -432,14 +432,14 @@ DecisionService.prototype._getVariationForRollout = function(feature, userId, at */ DecisionService.prototype._getBucketingId = function(userId, attributes) { var bucketingId = userId; - + // If the bucketing ID key is defined in attributes, than use that in place of the userID for the murmur hash key if (!fns.isEmpty(attributes) && attributes.hasOwnProperty(enums.CONTROL_ATTRIBUTES.BUCKETING_ID)) { if (typeof attributes[enums.CONTROL_ATTRIBUTES.BUCKETING_ID] === 'string') { bucketingId = attributes[enums.CONTROL_ATTRIBUTES.BUCKETING_ID]; - this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.VALID_BUCKETING_ID, bucketingId)); + this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.VALID_BUCKETING_ID, MODULE_NAME, bucketingId)); } else { - this.logger.log(LOG_LEVEL.WARNING, LOG_MESSAGES.BUCKETING_ID_NOT_STRING); + this.logger.log(LOG_LEVEL.WARNING, sprintf(LOG_MESSAGES.BUCKETING_ID_NOT_STRING, MODULE_NAME)); } } 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 b757f1b5d..197778ce2 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.tests.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.tests.js @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2017, Optimizely, Inc. and contributors * + * Copyright 2017-2018, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -461,6 +461,51 @@ describe('lib/core/decision_service', function() { }); }); + describe('_getBucketingId', function() { + var configObj; + var decisionService; + var mockLogger = logger.createLogger({logLevel: LOG_LEVEL.INFO}); + var userId = 'testUser1'; + var userAttributesWithBucketingId = { + 'browser_type': 'firefox', + '$opt_bucketing_id': '123456789' + }; + var userAttributesWithInvalidBucketingId = { + 'browser_type': 'safari', + '$opt_bucketing_id': 50 + }; + + beforeEach(function() { + sinon.stub(mockLogger, 'log'); + configObj = projectConfig.createProjectConfig(testData); + decisionService = DecisionService.createDecisionService({ + configObj: configObj, + logger: mockLogger, + }); + }); + + afterEach(function() { + mockLogger.log.restore(); + }); + + it('should return userId if bucketingId is not defined in user attributes', function() { + assert.strictEqual(userId, decisionService._getBucketingId(userId, null)); + assert.strictEqual(userId, decisionService._getBucketingId(userId, {'browser_type': 'safari'})); + }); + + it('should log warning in case of invalid bucketingId', function() { + assert.strictEqual(userId, decisionService._getBucketingId(userId, userAttributesWithInvalidBucketingId)); + assert.strictEqual(1, mockLogger.log.callCount); + assert.strictEqual(mockLogger.log.args[0][1], 'DECISION_SERVICE: BucketingID attribute is not a string. Defaulted to userId'); + }); + + it('should return correct bucketingId when provided in attributes', function() { + assert.strictEqual('123456789', decisionService._getBucketingId(userId, userAttributesWithBucketingId)); + assert.strictEqual(1, mockLogger.log.callCount); + assert.strictEqual(mockLogger.log.args[0][1], 'DECISION_SERVICE: BucketingId is valid: "123456789"'); + }); + }); + describe('feature management', function() { describe('#getVariationForFeature', function() { var configObj; diff --git a/packages/optimizely-sdk/lib/core/event_builder/index.js b/packages/optimizely-sdk/lib/core/event_builder/index.js index 6fd03d258..fc242c4c8 100644 --- a/packages/optimizely-sdk/lib/core/event_builder/index.js +++ b/packages/optimizely-sdk/lib/core/event_builder/index.js @@ -17,6 +17,7 @@ var enums = require('../../utils/enums'); var fns = require('../../utils/fns'); var eventTagUtils = require('../../utils/event_tag_utils'); var projectConfig = require('../project_config'); +var attributeValidator = require('../../utils/attributes_validator'); var ACTIVATE_EVENT_KEY = 'campaign_activated'; var CUSTOM_ATTRIBUTE_FEATURE_TYPE = 'custom'; @@ -58,15 +59,17 @@ function getCommonEventParams(options) { anonymize_ip: anonymize_ip, }; - fns.forOwn(attributes, function(attributeValue, attributeKey){ - var attributeId = projectConfig.getAttributeId(options.configObj, attributeKey, options.logger); - if (attributeId) { - commonParams.visitors[0].attributes.push({ - entity_id: attributeId, - key: attributeKey, - type: CUSTOM_ATTRIBUTE_FEATURE_TYPE, - value: attributes[attributeKey], - }); + fns.forOwn(attributes, function(attributeValue, attributeKey) { + if (attributeValidator.isAttributeValid(attributeKey, attributeValue)) { + var attributeId = projectConfig.getAttributeId(options.configObj, attributeKey, options.logger); + if (attributeId) { + commonParams.visitors[0].attributes.push({ + entity_id: attributeId, + key: attributeKey, + type: CUSTOM_ATTRIBUTE_FEATURE_TYPE, + value: attributes[attributeKey], + }); + } } }); diff --git a/packages/optimizely-sdk/lib/core/event_builder/index.tests.js b/packages/optimizely-sdk/lib/core/event_builder/index.tests.js index 08ab9fa73..cd2c31b93 100644 --- a/packages/optimizely-sdk/lib/core/event_builder/index.tests.js +++ b/packages/optimizely-sdk/lib/core/event_builder/index.tests.js @@ -405,6 +405,138 @@ describe('lib/core/event_builder', function() { assert.deepEqual(actualParams, expectedParams); }); + + it('should create proper params for getImpressionEvent with typed attributes', function() { + var expectedParams = { + url: 'https://logx.optimizely.com/v1/events', + httpVerb: 'POST', + params: { + 'account_id': '12001', + 'project_id': '111001', + 'visitors': [{ + 'attributes': [{ + 'entity_id': '111094', + 'key': 'browser_type', + 'type': 'custom', + 'value': 'Chrome' + }, { + 'entity_id': '323434545', + 'key': 'boolean_key', + 'type': 'custom', + 'value': true + }, { + 'entity_id': '616727838', + 'key': 'integer_key', + 'type': 'custom', + 'value': 10 + }, { + 'entity_id': '808797686', + 'key': 'double_key', + 'type': 'custom', + 'value': 3.14 + }], + 'visitor_id': 'testUser', + 'snapshots': [{ + 'decisions': [{ + 'variation_id': '111128', + 'experiment_id': '111127', + 'campaign_id': '4' + }], + 'events': [{ + 'timestamp': Math.round(new Date().getTime()), + 'entity_id': '4', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'key': 'campaign_activated' + }] + }] + }], + 'revision': '42', + 'client_name': 'node-sdk', + 'client_version': packageJSON.version, + 'anonymize_ip': false, + } + }; + + var eventOptions = { + attributes: { + 'browser_type': 'Chrome', + 'boolean_key': true, + 'integer_key': 10, + 'double_key': 3.14, + }, + clientEngine: 'node-sdk', + clientVersion: packageJSON.version, + configObj: configObj, + experimentId: '111127', + variationId: '111128', + userId: 'testUser', + }; + + var actualParams = eventBuilder.getImpressionEvent(eventOptions); + + assert.deepEqual(actualParams, expectedParams); + }); + + it('should remove invalid params from impression event payload', function() { + var expectedParams = { + url: 'https://logx.optimizely.com/v1/events', + httpVerb: 'POST', + params: { + 'account_id': '12001', + 'project_id': '111001', + 'visitors': [{ + 'attributes': [{ + 'entity_id': '111094', + 'key': 'browser_type', + 'type': 'custom', + 'value': 'Chrome' + }, { + 'entity_id': '616727838', + 'key': 'integer_key', + 'type': 'custom', + 'value': 10 + }], + 'visitor_id': 'testUser', + 'snapshots': [{ + 'decisions': [{ + 'variation_id': '111128', + 'experiment_id': '111127', + 'campaign_id': '4' + }], + 'events': [{ + 'timestamp': Math.round(new Date().getTime()), + 'entity_id': '4', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c', + 'key': 'campaign_activated' + }] + }] + }], + 'revision': '42', + 'client_name': 'node-sdk', + 'client_version': packageJSON.version, + 'anonymize_ip': false, + } + }; + + var eventOptions = { + attributes: { + 'browser_type': 'Chrome', + 'integer_key': 10, + 'boolean_key': null, + 'double_key': [1, 2, 3], + }, + clientEngine: 'node-sdk', + clientVersion: packageJSON.version, + configObj: configObj, + experimentId: '111127', + variationId: '111128', + userId: 'testUser', + }; + + var actualParams = eventBuilder.getImpressionEvent(eventOptions); + + assert.deepEqual(actualParams, expectedParams); + }); }); describe('getConversionEvent', function() { diff --git a/packages/optimizely-sdk/lib/optimizely/index.tests.js b/packages/optimizely-sdk/lib/optimizely/index.tests.js index b5b671393..bab0e4c55 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely/index.tests.js @@ -557,6 +557,84 @@ describe('lib/optimizely', function() { JSON.stringify(expectedObj.params))); }); + it('should call activate and dispatchEvent with typed attributes and return variation key', function() { + bucketStub.returns('122229'); + var activate = optlyInstance.activate('testExperimentWithAudiences', 'testUser', { + browser_type: 'firefox', + boolean_key: true, + integer_key: 10, + double_key: 3.14, + }); + assert.strictEqual(activate, 'variationWithAudience'); + + sinon.assert.calledOnce(bucketer.bucket); + sinon.assert.calledOnce(eventDispatcher.dispatchEvent); + sinon.assert.calledTwice(createdLogger.log); + + var expectedObj = { + url: 'https://logx.optimizely.com/v1/events', + httpVerb: 'POST', + params: { + 'account_id': '12001', + 'project_id': '111001', + 'visitors': [{ + 'snapshots': [{ + 'decisions': [{ + 'campaign_id': '5', + 'experiment_id': '122227', + 'variation_id': '122229' + }], + 'events': [{ + 'entity_id': '5', + 'timestamp': Math.round(new Date().getTime()), + 'key': 'campaign_activated', + 'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c' + }] + }], + 'visitor_id': 'testUser', + 'attributes': [{ + 'entity_id': '111094', + 'key': 'browser_type', + 'type': 'custom', + 'value': 'firefox' + }, { + 'entity_id': '323434545', + 'key': 'boolean_key', + 'type': 'custom', + 'value': true + }, { + 'entity_id': '616727838', + 'key': 'integer_key', + 'type': 'custom', + 'value': 10 + }, { + 'entity_id': '808797686', + 'key': 'double_key', + 'type': 'custom', + 'value': 3.14 + }], + }], + 'revision': '42', + 'client_name': 'node-sdk', + 'client_version': enums.NODE_CLIENT_VERSION, + 'anonymize_ip': false, + } + }; + + var eventDispatcherCall = eventDispatcher.dispatchEvent.args[0]; + assert.deepEqual(eventDispatcherCall[0], expectedObj); + + var logMessage1 = createdLogger.log.args[0][1]; + assert.strictEqual(logMessage1, sprintf(LOG_MESSAGES.USER_HAS_NO_FORCED_VARIATION, + 'PROJECT_CONFIG', + 'testUser')); + var logMessage2 = createdLogger.log.args[1][1]; + assert.strictEqual(logMessage2, sprintf(LOG_MESSAGES.DISPATCH_IMPRESSION_EVENT, + 'OPTIMIZELY', + expectedObj.url, + JSON.stringify(expectedObj.params))); + }); + 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'); diff --git a/packages/optimizely-sdk/lib/utils/attributes_validator/index.js b/packages/optimizely-sdk/lib/utils/attributes_validator/index.js index 7e38c1ade..bbd8965a6 100644 --- a/packages/optimizely-sdk/lib/utils/attributes_validator/index.js +++ b/packages/optimizely-sdk/lib/utils/attributes_validator/index.js @@ -1,5 +1,5 @@ /** - * Copyright 2016, Optimizely + * Copyright 2016, 2018, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,6 +24,8 @@ var lodashForOwn = require('lodash/forOwn'); var ERROR_MESSAGES = require('../enums').ERROR_MESSAGES; var MODULE_NAME = 'ATTRIBUTES_VALIDATOR'; +var VALID_ATTRIBUTE_TYPES = ['string', 'boolean', 'number']; + module.exports = { /** * Validates user's provided attributes @@ -43,4 +45,8 @@ module.exports = { throw new Error(sprintf(ERROR_MESSAGES.INVALID_ATTRIBUTES, MODULE_NAME)); } }, + + isAttributeValid: function(attributeKey, attributeValue) { + return typeof attributeKey === 'string' && attributeKey !== '' && VALID_ATTRIBUTE_TYPES.indexOf(typeof attributeValue) !== -1; + } }; diff --git a/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js b/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js index 6abd5862e..bb8a93e84 100644 --- a/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js +++ b/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js @@ -1,5 +1,5 @@ /** - * Copyright 2016, Optimizely + * Copyright 2016, 2018, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,7 @@ var chai = require('chai'); var assert = chai.assert; var sprintf = require('sprintf-js').sprintf; var attributesValidator = require('./'); +var lodashForOwn = require('lodash/forOwn'); var ERROR_MESSAGES = require('../enums').ERROR_MESSAGES; @@ -60,5 +61,33 @@ describe('lib/utils/attributes_validator', function() { }, sprintf(ERROR_MESSAGES.UNDEFINED_ATTRIBUTE, 'ATTRIBUTES_VALIDATOR', attributeKey)); }); }); + + describe('isAttributeValid', function() { + it('isAttributeValid returns true for valid values', function() { + var userAttributes = { + 'browser_type': 'Chrome', + 'is_firefox': false, + 'num_users': 10, + 'pi_value': 3.14, + }; + + lodashForOwn(userAttributes, function(value, key) { + assert.isTrue(attributesValidator.isAttributeValid(key, value)); + }); + }); + + it('isAttributeValid returns false for invalid values', function() { + var userAttributes = { + '': 'javascript', + 'null': null, + 'objects': {a: 'b'}, + 'pi_value': [1, 2, 3], + }; + + lodashForOwn(userAttributes, function(value, key) { + assert.isFalse(attributesValidator.isAttributeValid(key, value)); + }); + }); + }); }); }); diff --git a/packages/optimizely-sdk/lib/utils/enums/index.js b/packages/optimizely-sdk/lib/utils/enums/index.js index 25507fe88..f867b23df 100644 --- a/packages/optimizely-sdk/lib/utils/enums/index.js +++ b/packages/optimizely-sdk/lib/utils/enums/index.js @@ -124,7 +124,7 @@ exports.LOG_MESSAGES = { VARIATION_REMOVED_FOR_USER: '%s: Variation mapped to experiment %s has been removed for user %s.', VARIABLE_REQUESTED_WITH_WRONG_TYPE: '%s: Requested variable type "%s", but variable is of type "%s". Use correct API to retrieve value. Returning None.', VALID_BUCKETING_ID: '%s: BucketingId is valid: "%s"', - BUCKETING_ID_NOT_STRING: 'BucketingID attribute is not a string. Defaulted to userId', + BUCKETING_ID_NOT_STRING: '%s: BucketingID attribute is not a string. Defaulted to userId', }; exports.RESERVED_EVENT_KEYWORDS = { From 68db7de5b06b709e7d3c9a4db18ba598cdc18a92 Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Thu, 20 Sep 2018 15:58:55 -0700 Subject: [PATCH 3/7] Attribute validation for log-tier --- .../lib/core/decision_service/index.js | 2 +- .../optimizely-sdk/lib/core/event_builder/index.js | 1 + .../lib/core/event_builder/index.tests.js | 5 +++++ .../lib/utils/attributes_validator/index.js | 2 +- .../lib/utils/attributes_validator/index.tests.js | 12 ++++++------ 5 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.js b/packages/optimizely-sdk/lib/core/decision_service/index.js index 1648da390..13a8d4cf6 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.js +++ b/packages/optimizely-sdk/lib/core/decision_service/index.js @@ -434,7 +434,7 @@ DecisionService.prototype._getBucketingId = function(userId, attributes) { var bucketingId = userId; // If the bucketing ID key is defined in attributes, than use that in place of the userID for the murmur hash key - if (!fns.isEmpty(attributes) && attributes.hasOwnProperty(enums.CONTROL_ATTRIBUTES.BUCKETING_ID)) { + if ((attributes != null && typeof attributes === 'object') && attributes.hasOwnProperty(enums.CONTROL_ATTRIBUTES.BUCKETING_ID)) { if (typeof attributes[enums.CONTROL_ATTRIBUTES.BUCKETING_ID] === 'string') { bucketingId = attributes[enums.CONTROL_ATTRIBUTES.BUCKETING_ID]; this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.VALID_BUCKETING_ID, MODULE_NAME, bucketingId)); diff --git a/packages/optimizely-sdk/lib/core/event_builder/index.js b/packages/optimizely-sdk/lib/core/event_builder/index.js index fc242c4c8..bb06d2a8a 100644 --- a/packages/optimizely-sdk/lib/core/event_builder/index.js +++ b/packages/optimizely-sdk/lib/core/event_builder/index.js @@ -59,6 +59,7 @@ function getCommonEventParams(options) { anonymize_ip: anonymize_ip, }; + // Omit attribute values that are not supported by the log endpoint. fns.forOwn(attributes, function(attributeValue, attributeKey) { if (attributeValidator.isAttributeValid(attributeKey, attributeValue)) { var attributeId = projectConfig.getAttributeId(options.configObj, attributeKey, options.logger); diff --git a/packages/optimizely-sdk/lib/core/event_builder/index.tests.js b/packages/optimizely-sdk/lib/core/event_builder/index.tests.js index cd2c31b93..dffe9419b 100644 --- a/packages/optimizely-sdk/lib/core/event_builder/index.tests.js +++ b/packages/optimizely-sdk/lib/core/event_builder/index.tests.js @@ -495,6 +495,11 @@ describe('lib/core/event_builder', function() { 'key': 'integer_key', 'type': 'custom', 'value': 10 + }, { + 'entity_id': '323434545', + 'key': 'boolean_key', + 'type': 'custom', + 'value': null }], 'visitor_id': 'testUser', 'snapshots': [{ diff --git a/packages/optimizely-sdk/lib/utils/attributes_validator/index.js b/packages/optimizely-sdk/lib/utils/attributes_validator/index.js index bbd8965a6..14a7be11b 100644 --- a/packages/optimizely-sdk/lib/utils/attributes_validator/index.js +++ b/packages/optimizely-sdk/lib/utils/attributes_validator/index.js @@ -47,6 +47,6 @@ module.exports = { }, isAttributeValid: function(attributeKey, attributeValue) { - return typeof attributeKey === 'string' && attributeKey !== '' && VALID_ATTRIBUTE_TYPES.indexOf(typeof attributeValue) !== -1; + return typeof attributeKey === 'string' && (attributeValue === null || VALID_ATTRIBUTE_TYPES.indexOf(typeof attributeValue) !== -1); } }; diff --git a/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js b/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js index bb8a93e84..f4c336ad8 100644 --- a/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js +++ b/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js @@ -17,7 +17,7 @@ var chai = require('chai'); var assert = chai.assert; var sprintf = require('sprintf-js').sprintf; var attributesValidator = require('./'); -var lodashForOwn = require('lodash/forOwn'); +var fns = require('./../fns/'); var ERROR_MESSAGES = require('../enums').ERROR_MESSAGES; @@ -69,22 +69,22 @@ describe('lib/utils/attributes_validator', function() { 'is_firefox': false, 'num_users': 10, 'pi_value': 3.14, + 'null': null, + '': 'javascript', }; - lodashForOwn(userAttributes, function(value, key) { + fns.forOwn(userAttributes, function(value, key) { assert.isTrue(attributesValidator.isAttributeValid(key, value)); }); }); it('isAttributeValid returns false for invalid values', function() { - var userAttributes = { - '': 'javascript', - 'null': null, + var userAttributes = { 'objects': {a: 'b'}, 'pi_value': [1, 2, 3], }; - lodashForOwn(userAttributes, function(value, key) { + fns.forOwn(userAttributes, function(value, key) { assert.isFalse(attributesValidator.isAttributeValid(key, value)); }); }); From 0ee73f19d1e70d0e0f0dc9b563c77104e1fc80b9 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Fri, 21 Sep 2018 17:52:43 +0500 Subject: [PATCH 4/7] Remove attributes null value check. --- .../optimizely-sdk/lib/core/event_builder/index.tests.js | 4 ++-- .../optimizely-sdk/lib/utils/attributes_validator/index.js | 2 +- .../lib/utils/attributes_validator/index.tests.js | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/event_builder/index.tests.js b/packages/optimizely-sdk/lib/core/event_builder/index.tests.js index dffe9419b..7e0dd7798 100644 --- a/packages/optimizely-sdk/lib/core/event_builder/index.tests.js +++ b/packages/optimizely-sdk/lib/core/event_builder/index.tests.js @@ -499,7 +499,7 @@ describe('lib/core/event_builder', function() { 'entity_id': '323434545', 'key': 'boolean_key', 'type': 'custom', - 'value': null + 'value': false }], 'visitor_id': 'testUser', 'snapshots': [{ @@ -527,7 +527,7 @@ describe('lib/core/event_builder', function() { attributes: { 'browser_type': 'Chrome', 'integer_key': 10, - 'boolean_key': null, + 'boolean_key': false, 'double_key': [1, 2, 3], }, clientEngine: 'node-sdk', diff --git a/packages/optimizely-sdk/lib/utils/attributes_validator/index.js b/packages/optimizely-sdk/lib/utils/attributes_validator/index.js index 14a7be11b..fa241045e 100644 --- a/packages/optimizely-sdk/lib/utils/attributes_validator/index.js +++ b/packages/optimizely-sdk/lib/utils/attributes_validator/index.js @@ -47,6 +47,6 @@ module.exports = { }, isAttributeValid: function(attributeKey, attributeValue) { - return typeof attributeKey === 'string' && (attributeValue === null || VALID_ATTRIBUTE_TYPES.indexOf(typeof attributeValue) !== -1); + return typeof attributeKey === 'string' && VALID_ATTRIBUTE_TYPES.indexOf(typeof attributeValue) !== -1; } }; diff --git a/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js b/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js index f4c336ad8..91ca320c6 100644 --- a/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js +++ b/packages/optimizely-sdk/lib/utils/attributes_validator/index.tests.js @@ -69,7 +69,6 @@ describe('lib/utils/attributes_validator', function() { 'is_firefox': false, 'num_users': 10, 'pi_value': 3.14, - 'null': null, '': 'javascript', }; @@ -79,9 +78,10 @@ describe('lib/utils/attributes_validator', function() { }); it('isAttributeValid returns false for invalid values', function() { - var userAttributes = { + var userAttributes = { + 'null': null, 'objects': {a: 'b'}, - 'pi_value': [1, 2, 3], + 'array': [1, 2, 3] }; fns.forOwn(userAttributes, function(value, key) { From 77b40235739387a77bdc356e59eeec16e5ef348c Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Fri, 21 Sep 2018 11:34:39 -0700 Subject: [PATCH 5/7] indented correctly. --- packages/optimizely-sdk/lib/tests/test_data.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/optimizely-sdk/lib/tests/test_data.js b/packages/optimizely-sdk/lib/tests/test_data.js index 85ca8e0bf..71b7fa724 100644 --- a/packages/optimizely-sdk/lib/tests/test_data.js +++ b/packages/optimizely-sdk/lib/tests/test_data.js @@ -247,7 +247,7 @@ var config = { key: "integer_key" }, { id: "808797686", - key: "double_key" + key: "double_key" } ], audiences: [{ From 03c5c578c1ed9a9fdc07942818cf998d56285dab Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Fri, 21 Sep 2018 14:41:33 -0700 Subject: [PATCH 6/7] activate shouldn't filter attributes for notification listener --- packages/optimizely-sdk/lib/optimizely/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index 590f4cb0e..030959c81 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -238,8 +238,7 @@ Optimizely.prototype.track = function(eventKey, userId, attributes, eventTags) { return; } - // remove null values from attributes and eventTags - attributes = this.__filterEmptyValues(attributes); + // remove null values from eventTags eventTags = this.__filterEmptyValues(eventTags); var conversionEventOptions = { From 30472a350d0a255da00150cfce89ec4ebbc7d4e4 Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Fri, 21 Sep 2018 14:57:33 -0700 Subject: [PATCH 7/7] activate/track shouldn't filter attributes for notification listener --- packages/optimizely-sdk/lib/optimizely/index.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/optimizely-sdk/lib/optimizely/index.js b/packages/optimizely-sdk/lib/optimizely/index.js index 030959c81..0e312deba 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.js +++ b/packages/optimizely-sdk/lib/optimizely/index.js @@ -136,10 +136,7 @@ Optimizely.prototype.activate = function (experimentKey, userId, attributes) { this.logger.log(LOG_LEVEL.DEBUG, shouldNotDispatchActivateLogMessage); return variationKey; } - - // remove null values from attributes - attributes = this.__filterEmptyValues(attributes); - + this._sendImpressionEvent(experimentKey, variationKey, userId, attributes); return variationKey;