Skip to content

Commit c41493e

Browse files
msohailhussainmikeproeng37
msohailhussain
authored and
mikeproeng37
committed
feat(eventbuilder): Track attributes with valid attribute types (#166)
1 parent feb34eb commit c41493e

File tree

11 files changed

+364
-23
lines changed

11 files changed

+364
-23
lines changed

‎packages/optimizely-sdk/lib/core/condition_evaluator/index.tests.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2016, Optimizely
2+
* Copyright 2016, 2018, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,6 +19,9 @@ var conditionEvaluator = require('./');
1919

2020
var browserConditionSafari = {'name': 'browser_type', 'value': 'safari'};
2121
var deviceConditionIphone = {'name': 'device_model', 'value': 'iphone6'};
22+
var booleanCondition = {'name': 'is_firefox', 'value': true};
23+
var integerCondition = {'name': 'num_users', 'value': 10};
24+
var doubleCondition = {'name': 'pi_value', 'value': 3.14};
2225

2326
describe('lib/core/condition_evaluator', function() {
2427
describe('APIs', function() {
@@ -39,6 +42,17 @@ describe('lib/core/condition_evaluator', function() {
3942
assert.isFalse(conditionEvaluator.evaluate(['and', browserConditionSafari], userAttributes));
4043
});
4144

45+
it('should evaluate different typed attributes', function() {
46+
var userAttributes = {
47+
browser_type: 'safari',
48+
is_firefox: true,
49+
num_users: 10,
50+
pi_value: 3.14,
51+
};
52+
53+
assert.isTrue(conditionEvaluator.evaluate(['and', browserConditionSafari, booleanCondition, integerCondition, doubleCondition], userAttributes));
54+
});
55+
4256
describe('and evaluation', function() {
4357
it('should return true when ALL conditions evaluate to true', function() {
4458
var userAttributes = {

‎packages/optimizely-sdk/lib/core/decision_service/index.js

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,7 @@ function DecisionService(options) {
6262
*/
6363
DecisionService.prototype.getVariation = function(experimentKey, userId, attributes) {
6464
// by default, the bucketing ID should be the user ID
65-
var bucketingId = userId;
66-
67-
// If the bucketing ID key is defined in attributes, than use that in place of the userID for the murmur hash key
68-
if (!fns.isEmpty(attributes)) {
69-
if (attributes.hasOwnProperty(enums.CONTROL_ATTRIBUTES.BUCKETING_ID)) {
70-
bucketingId = attributes[enums.CONTROL_ATTRIBUTES.BUCKETING_ID];
71-
this.logger.log(LOG_LEVEL.DEBUG, sprintf('Setting the bucketing ID to %s.', bucketingId));
72-
}
73-
}
65+
var bucketingId = this._getBucketingId(userId, attributes);
7466

7567
if (!this.__checkIfExperimentIsActive(experimentKey, userId)) {
7668
return null;
@@ -432,6 +424,28 @@ DecisionService.prototype._getVariationForRollout = function(feature, userId, at
432424
};
433425
};
434426

427+
/**
428+
* Get bucketing Id from user attributes.
429+
* @param {String} userId
430+
* @param {Object} attributes
431+
* @returns {String} Bucketing Id if it is a string type in attributes, user Id otherwise.
432+
*/
433+
DecisionService.prototype._getBucketingId = function(userId, attributes) {
434+
var bucketingId = userId;
435+
436+
// If the bucketing ID key is defined in attributes, than use that in place of the userID for the murmur hash key
437+
if ((attributes != null && typeof attributes === 'object') && attributes.hasOwnProperty(enums.CONTROL_ATTRIBUTES.BUCKETING_ID)) {
438+
if (typeof attributes[enums.CONTROL_ATTRIBUTES.BUCKETING_ID] === 'string') {
439+
bucketingId = attributes[enums.CONTROL_ATTRIBUTES.BUCKETING_ID];
440+
this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.VALID_BUCKETING_ID, MODULE_NAME, bucketingId));
441+
} else {
442+
this.logger.log(LOG_LEVEL.WARNING, sprintf(LOG_MESSAGES.BUCKETING_ID_NOT_STRING, MODULE_NAME));
443+
}
444+
}
445+
446+
return bucketingId;
447+
};
448+
435449
module.exports = {
436450
/**
437451
* Creates an instance of the DecisionService.

‎packages/optimizely-sdk/lib/core/decision_service/index.tests.js

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/****************************************************************************
2-
* Copyright 2017, Optimizely, Inc. and contributors *
2+
* Copyright 2017-2018, Optimizely, Inc. and contributors *
33
* *
44
* Licensed under the Apache License, Version 2.0 (the "License"); *
55
* you may not use this file except in compliance with the License. *
@@ -461,6 +461,51 @@ describe('lib/core/decision_service', function() {
461461
});
462462
});
463463

464+
describe('_getBucketingId', function() {
465+
var configObj;
466+
var decisionService;
467+
var mockLogger = logger.createLogger({logLevel: LOG_LEVEL.INFO});
468+
var userId = 'testUser1';
469+
var userAttributesWithBucketingId = {
470+
'browser_type': 'firefox',
471+
'$opt_bucketing_id': '123456789'
472+
};
473+
var userAttributesWithInvalidBucketingId = {
474+
'browser_type': 'safari',
475+
'$opt_bucketing_id': 50
476+
};
477+
478+
beforeEach(function() {
479+
sinon.stub(mockLogger, 'log');
480+
configObj = projectConfig.createProjectConfig(testData);
481+
decisionService = DecisionService.createDecisionService({
482+
configObj: configObj,
483+
logger: mockLogger,
484+
});
485+
});
486+
487+
afterEach(function() {
488+
mockLogger.log.restore();
489+
});
490+
491+
it('should return userId if bucketingId is not defined in user attributes', function() {
492+
assert.strictEqual(userId, decisionService._getBucketingId(userId, null));
493+
assert.strictEqual(userId, decisionService._getBucketingId(userId, {'browser_type': 'safari'}));
494+
});
495+
496+
it('should log warning in case of invalid bucketingId', function() {
497+
assert.strictEqual(userId, decisionService._getBucketingId(userId, userAttributesWithInvalidBucketingId));
498+
assert.strictEqual(1, mockLogger.log.callCount);
499+
assert.strictEqual(mockLogger.log.args[0][1], 'DECISION_SERVICE: BucketingID attribute is not a string. Defaulted to userId');
500+
});
501+
502+
it('should return correct bucketingId when provided in attributes', function() {
503+
assert.strictEqual('123456789', decisionService._getBucketingId(userId, userAttributesWithBucketingId));
504+
assert.strictEqual(1, mockLogger.log.callCount);
505+
assert.strictEqual(mockLogger.log.args[0][1], 'DECISION_SERVICE: BucketingId is valid: "123456789"');
506+
});
507+
});
508+
464509
describe('feature management', function() {
465510
describe('#getVariationForFeature', function() {
466511
var configObj;

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

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ var enums = require('../../utils/enums');
1717
var fns = require('../../utils/fns');
1818
var eventTagUtils = require('../../utils/event_tag_utils');
1919
var projectConfig = require('../project_config');
20+
var attributeValidator = require('../../utils/attributes_validator');
2021

2122
var ACTIVATE_EVENT_KEY = 'campaign_activated';
2223
var CUSTOM_ATTRIBUTE_FEATURE_TYPE = 'custom';
@@ -58,15 +59,18 @@ function getCommonEventParams(options) {
5859
anonymize_ip: anonymize_ip,
5960
};
6061

61-
fns.forOwn(attributes, function(attributeValue, attributeKey){
62-
var attributeId = projectConfig.getAttributeId(options.configObj, attributeKey, options.logger);
63-
if (attributeId) {
64-
commonParams.visitors[0].attributes.push({
65-
entity_id: attributeId,
66-
key: attributeKey,
67-
type: CUSTOM_ATTRIBUTE_FEATURE_TYPE,
68-
value: attributes[attributeKey],
69-
});
62+
// Omit attribute values that are not supported by the log endpoint.
63+
fns.forOwn(attributes, function(attributeValue, attributeKey) {
64+
if (attributeValidator.isAttributeValid(attributeKey, attributeValue)) {
65+
var attributeId = projectConfig.getAttributeId(options.configObj, attributeKey, options.logger);
66+
if (attributeId) {
67+
commonParams.visitors[0].attributes.push({
68+
entity_id: attributeId,
69+
key: attributeKey,
70+
type: CUSTOM_ATTRIBUTE_FEATURE_TYPE,
71+
value: attributes[attributeKey],
72+
});
73+
}
7074
}
7175
});
7276

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

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,143 @@ describe('lib/core/event_builder', function() {
405405

406406
assert.deepEqual(actualParams, expectedParams);
407407
});
408+
409+
it('should create proper params for getImpressionEvent with typed attributes', function() {
410+
var expectedParams = {
411+
url: 'https://logx.optimizely.com/v1/events',
412+
httpVerb: 'POST',
413+
params: {
414+
'account_id': '12001',
415+
'project_id': '111001',
416+
'visitors': [{
417+
'attributes': [{
418+
'entity_id': '111094',
419+
'key': 'browser_type',
420+
'type': 'custom',
421+
'value': 'Chrome'
422+
}, {
423+
'entity_id': '323434545',
424+
'key': 'boolean_key',
425+
'type': 'custom',
426+
'value': true
427+
}, {
428+
'entity_id': '616727838',
429+
'key': 'integer_key',
430+
'type': 'custom',
431+
'value': 10
432+
}, {
433+
'entity_id': '808797686',
434+
'key': 'double_key',
435+
'type': 'custom',
436+
'value': 3.14
437+
}],
438+
'visitor_id': 'testUser',
439+
'snapshots': [{
440+
'decisions': [{
441+
'variation_id': '111128',
442+
'experiment_id': '111127',
443+
'campaign_id': '4'
444+
}],
445+
'events': [{
446+
'timestamp': Math.round(new Date().getTime()),
447+
'entity_id': '4',
448+
'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c',
449+
'key': 'campaign_activated'
450+
}]
451+
}]
452+
}],
453+
'revision': '42',
454+
'client_name': 'node-sdk',
455+
'client_version': packageJSON.version,
456+
'anonymize_ip': false,
457+
}
458+
};
459+
460+
var eventOptions = {
461+
attributes: {
462+
'browser_type': 'Chrome',
463+
'boolean_key': true,
464+
'integer_key': 10,
465+
'double_key': 3.14,
466+
},
467+
clientEngine: 'node-sdk',
468+
clientVersion: packageJSON.version,
469+
configObj: configObj,
470+
experimentId: '111127',
471+
variationId: '111128',
472+
userId: 'testUser',
473+
};
474+
475+
var actualParams = eventBuilder.getImpressionEvent(eventOptions);
476+
477+
assert.deepEqual(actualParams, expectedParams);
478+
});
479+
480+
it('should remove invalid params from impression event payload', function() {
481+
var expectedParams = {
482+
url: 'https://logx.optimizely.com/v1/events',
483+
httpVerb: 'POST',
484+
params: {
485+
'account_id': '12001',
486+
'project_id': '111001',
487+
'visitors': [{
488+
'attributes': [{
489+
'entity_id': '111094',
490+
'key': 'browser_type',
491+
'type': 'custom',
492+
'value': 'Chrome'
493+
}, {
494+
'entity_id': '616727838',
495+
'key': 'integer_key',
496+
'type': 'custom',
497+
'value': 10
498+
}, {
499+
'entity_id': '323434545',
500+
'key': 'boolean_key',
501+
'type': 'custom',
502+
'value': false
503+
}],
504+
'visitor_id': 'testUser',
505+
'snapshots': [{
506+
'decisions': [{
507+
'variation_id': '111128',
508+
'experiment_id': '111127',
509+
'campaign_id': '4'
510+
}],
511+
'events': [{
512+
'timestamp': Math.round(new Date().getTime()),
513+
'entity_id': '4',
514+
'uuid': 'a68cf1ad-0393-4e18-af87-efe8f01a7c9c',
515+
'key': 'campaign_activated'
516+
}]
517+
}]
518+
}],
519+
'revision': '42',
520+
'client_name': 'node-sdk',
521+
'client_version': packageJSON.version,
522+
'anonymize_ip': false,
523+
}
524+
};
525+
526+
var eventOptions = {
527+
attributes: {
528+
'browser_type': 'Chrome',
529+
'integer_key': 10,
530+
'boolean_key': false,
531+
'double_key': [1, 2, 3],
532+
},
533+
clientEngine: 'node-sdk',
534+
clientVersion: packageJSON.version,
535+
configObj: configObj,
536+
experimentId: '111127',
537+
variationId: '111128',
538+
userId: 'testUser',
539+
};
540+
541+
var actualParams = eventBuilder.getImpressionEvent(eventOptions);
542+
543+
assert.deepEqual(actualParams, expectedParams);
544+
});
408545
});
409546

410547
describe('getConversionEvent', function() {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2016-2017, Optimizely
2+
* Copyright 2016-2018, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -70,6 +70,9 @@ describe('lib/core/project_config', function() {
7070

7171
var expectedAttributeKeyMap = {
7272
browser_type: testData.attributes[0],
73+
boolean_key: testData.attributes[1],
74+
integer_key: testData.attributes[2],
75+
double_key: testData.attributes[3],
7376
};
7477

7578
assert.deepEqual(configObj.attributeKeyMap, expectedAttributeKeyMap);

0 commit comments

Comments
 (0)