Addressing feedback

Author: aliabbasrizvi

packages/optimizely-sdk/lib/core/event_builder/index.js

@@ -58,7 +58,7 @@ function getCommonEventParams(options) {
   };
 
   fns.forOwn(attributes, function(attributeValue, attributeKey){
-    var attributeId = projectConfig.getAttributeId(options.configObj, attributeKey);
+    var attributeId = projectConfig.getAttributeId(options.configObj, attributeKey, options.logger);
     if (attributeId) {
       commonParams.visitors[0].attributes.push({
         entity_id: attributeId,

packages/optimizely-sdk/lib/core/event_builder/index.tests.js

@@ -27,13 +27,17 @@ var uuid = require('uuid');
 describe('lib/core/event_builder', function() {
   describe('APIs', function() {
 
+    var mockLogger;
     var configObj;
     var clock;
 
     beforeEach(function() {
       configObj = projectConfig.createProjectConfig(testData.getTestProjectConfig());
       clock = sinon.useFakeTimers(new Date().getTime());
       sinon.stub(uuid, 'v4').returns('a68cf1ad-0393-4e18-af87-efe8f01a7c9c');
+      mockLogger = {
+        log: sinon.stub(),
+      };
     });
 
     afterEach(function() {
@@ -279,6 +283,7 @@ describe('lib/core/event_builder', function() {
           experimentId: '111127',
           variationId: '111128',
           userId: 'testUser',
+          logger: mockLogger,
         };
 
         var actualParams = eventBuilder.getImpressionEvent(eventOptions);
@@ -345,14 +350,6 @@ describe('lib/core/event_builder', function() {
     });
 
     describe('getConversionEvent', function() {
-      var mockLogger;
-
-      beforeEach(function() {
-        mockLogger = {
-          log: sinon.stub(),
-        };
-      });
-
       it('should create proper params for getConversionEvent without attributes or event value', function() {
         var expectedParams = {
           url: 'https://logx.optimizely.com/v1/events',
@@ -606,7 +603,7 @@ describe('lib/core/event_builder', function() {
         };
 
         var actualParams = eventBuilder.getConversionEvent(eventOptions);
-
+        sinon.assert.calledOnce(mockLogger.log);
         assert.deepEqual(actualParams, expectedParams);
       });
 
@@ -949,14 +946,6 @@ describe('lib/core/event_builder', function() {
       });
 
       describe('createEventWithBucketingId', function () {
-        var mockLogger;
-
-        beforeEach(function () {
-          mockLogger = {
-            log: sinon.stub(),
-          };
-        });
-
         it('should send proper bucketingID with user attributes', function () {
           var expectedParams = {
             url: 'https://logx.optimizely.com/v1/events',

packages/optimizely-sdk/lib/core/project_config/index.js

@@ -19,6 +19,7 @@ var sprintf = require('sprintf');
 
 var EXPERIMENT_LAUNCHED_STATUS = 'Launched';
 var EXPERIMENT_RUNNING_STATUS = 'Running';
+var RESERVED_ATTRIBUTE_PREFIX = '$opt_';
 var MODULE_NAME = 'PROJECT_CONFIG';
 
 var ERROR_MESSAGES = enums.ERROR_MESSAGES;
@@ -132,15 +133,18 @@ module.exports = {
    * Get attribute ID for the provided attribute key
    * @param  {Object}      projectConfig Object representing project configuration
    * @param  {string}      attributeKey  Attribute key for which ID is to be determined
-   * @return {string|null} Attribute ID corresponding to the provided attribute key
+   * @param  {Object}      logger
+   * @return {string|null} Attribute ID corresponding to the provided attribute key. Attribute key if it is a reserved attribute.
    */
-  getAttributeId: function(projectConfig, attributeKey) {
+  getAttributeId: function(projectConfig, attributeKey, logger) {
     var attribute = projectConfig.attributeKeyMap[attributeKey];
     if (attribute) {
       return attribute.id;
-    } else if (enums.RESERVED_USER_ATTRIBUTES.indexOf(attributeKey) !== -1) {
+    } else if (attributeKey.startsWith(RESERVED_ATTRIBUTE_PREFIX)) {
       return attributeKey;
     }
+
+    logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.UNRECOGNIZED_ATTRIBUTE, MODULE_NAME, attributeKey));
     return null;
   },
 

packages/optimizely-sdk/lib/core/project_config/index.tests.js

@@ -216,9 +216,15 @@ describe('lib/core/project_config', function() {
   describe('projectConfig helper methods', function() {
     var testData = testDatafile.getTestProjectConfig();
     var configObj;
-
+    var createdLogger = logger.createLogger({logLevel: LOG_LEVEL.INFO});
+      
     beforeEach(function() {
       configObj = projectConfig.createProjectConfig(testData);
+      sinon.stub(createdLogger, 'log');
+    });
+
+    afterEach(function() {
+      createdLogger.log.restore();
     });
 
     it('should retrieve experiment ID for valid experiment key in getExperimentId', function() {
@@ -246,8 +252,15 @@ describe('lib/core/project_config', function() {
       assert.strictEqual(projectConfig.getAttributeId(configObj, 'browser_type'), '111094');
     });
 
+    it('should retrieve attribute ID for reserved attribute key in getAttributeId', function() {
+      assert.strictEqual(projectConfig.getAttributeId(configObj, '$opt_user_agent'), '$opt_user_agent');
+    });
+
     it('should return null for invalid attribute key in getAttributeId', function() {
-      assert.isNull(projectConfig.getAttributeId(configObj, 'invalidAttributeKey'));
+      assert.isNull(projectConfig.getAttributeId(configObj, 'invalidAttributeKey', createdLogger));
+      sinon.assert.calledWithExactly(createdLogger.log, 
+                                     LOG_LEVEL.DEBUG, 
+                                     'PROJECT_CONFIG: Unrecognized attribute invalidAttributeKey provided. Pruning before sending event to Optimizely.')
     });
 
     it('should retrieve event ID for valid event key in getEventId', function() {

packages/optimizely-sdk/lib/optimizely/index.js

@@ -181,6 +181,7 @@ Optimizely.prototype._sendImpressionEvent = function(experimentKey, variationKey
     experimentId: experimentId,
     userId: userId,
     variationId: variationId,
+    logger: this.logger,
   };
   var impressionEvent = eventBuilder.getImpressionEvent(impressionEventOptions);
   var dispatchedImpressionEventLogMessage = sprintf(LOG_MESSAGES.DISPATCH_IMPRESSION_EVENT,

packages/optimizely-sdk/lib/utils/enums/index.js

@@ -50,6 +50,7 @@ exports.ERROR_MESSAGES = {
   NO_JSON_PROVIDED: '%s: No JSON object to validate against schema.',
   NO_VARIATION_FOR_EXPERIMENT_KEY: '%s: No variation key %s defined in datafile for experiment %s.',
   UNDEFINED_ATTRIBUTE: '%s: Provided attribute: %s has an undefined value.',
+  UNRECOGNIZED_ATTRIBUTE: '%s: Unrecognized attribute %s provided. Pruning before sending event to Optimizely.',
   UNABLE_TO_CAST_VALUE: '%s: Unable to cast value %s to type %s, returning null.',
   USER_NOT_IN_FORCED_VARIATION: '%s: User %s is not in the forced variation map. Cannot remove their forced variation.',
   USER_PROFILE_LOOKUP_ERROR: '%s: Error while looking up user profile for user ID "%s": %s.',
@@ -134,11 +135,6 @@ exports.RESERVED_ATTRIBUTES = {
   USER_AGENT: '$opt_user_agent',
 };
 
-exports.RESERVED_USER_ATTRIBUTES = [
-  exports.RESERVED_ATTRIBUTES.BUCKETING_ID,
-  exports.RESERVED_ATTRIBUTES.USER_AGENT
-];
-
 exports.JAVASCRIPT_CLIENT_ENGINE = 'javascript-sdk';
 exports.NODE_CLIENT_ENGINE = 'node-sdk';
 exports.NODE_CLIENT_VERSION = '2.0.1';