Skip to content

Commit c4531e6

Browse files
authored
fix (forced variations): Move forced variation methods and state out of project config and into decision service (#24)
Summary: This is a solution for persisting forced variation state when the project config is updated from datafile management. Just as in Java, we moved the forced variation map and forced variation methods into decision service. Test plan: - Existing unit tests are passing with no substantial changes: the changes I made in existing unit tests only update log message assertions to reflect the new location of forced variation methods (previously in project config, now in decision service). - I moved forced variation method tests to the decision service tests file, and updated the method signatures to remove the logger argument (decision service already has a reference to its own logger). - I added a few more unit tests covering cases where a previously set variation or experiment no longer exists on a new project config object.
1 parent 08f451f commit c4531e6

File tree

6 files changed

+353
-348
lines changed

6 files changed

+353
-348
lines changed

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

Lines changed: 143 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ var bucketer = require('../bucketer');
1919
var enums = require('../../utils/enums');
2020
var fns = require('../../utils/fns');
2121
var projectConfig = require('../project_config');
22+
var stringValidator = require('../../utils/string_value_validator');
2223

2324
var sprintf = require('@optimizely/js-sdk-utils').sprintf;
2425

@@ -50,6 +51,7 @@ var DECISION_SOURCES = enums.DECISION_SOURCES;
5051
function DecisionService(options) {
5152
this.userProfileService = options.userProfileService || null;
5253
this.logger = options.logger;
54+
this.forcedVariationMap = {};
5355
}
5456

5557
/**
@@ -68,7 +70,7 @@ DecisionService.prototype.getVariation = function(configObj, experimentKey, user
6870
return null;
6971
}
7072
var experiment = configObj.experimentKeyMap[experimentKey];
71-
var forcedVariationKey = projectConfig.getForcedVariation(configObj, experimentKey, userId, this.logger);
73+
var forcedVariationKey = this.getForcedVariation(configObj, experimentKey, userId);
7274
if (!!forcedVariationKey) {
7375
return forcedVariationKey;
7476
}
@@ -468,6 +470,146 @@ DecisionService.prototype._getBucketingId = function(userId, attributes) {
468470
return bucketingId;
469471
};
470472

473+
/**
474+
* Removes forced variation for given userId and experimentKey
475+
* @param {string} userId String representing the user id
476+
* @param {number} experimentId Number representing the experiment id
477+
* @param {string} experimentKey Key representing the experiment id
478+
* @throws If the user id is not valid or not in the forced variation map
479+
*/
480+
DecisionService.prototype.removeForcedVariation = function(userId, experimentId, experimentKey) {
481+
if (!userId) {
482+
throw new Error(sprintf(ERROR_MESSAGES.INVALID_USER_ID, MODULE_NAME));
483+
}
484+
485+
if (this.forcedVariationMap.hasOwnProperty(userId)) {
486+
delete this.forcedVariationMap[userId][experimentId];
487+
this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.VARIATION_REMOVED_FOR_USER, MODULE_NAME, experimentKey, userId));
488+
} else {
489+
throw new Error(sprintf(ERROR_MESSAGES.USER_NOT_IN_FORCED_VARIATION, MODULE_NAME, userId));
490+
}
491+
};
492+
493+
/**
494+
* Sets forced variation for given userId and experimentKey
495+
* @param {string} userId String representing the user id
496+
* @param {number} experimentId Number representing the experiment id
497+
* @param {number} variationId Number representing the variation id
498+
* @throws If the user id is not valid
499+
*/
500+
DecisionService.prototype.__setInForcedVariationMap = function(userId, experimentId, variationId) {
501+
if (this.forcedVariationMap.hasOwnProperty(userId)) {
502+
this.forcedVariationMap[userId][experimentId] = variationId;
503+
} else {
504+
this.forcedVariationMap[userId] = {};
505+
this.forcedVariationMap[userId][experimentId] = variationId;
506+
}
507+
508+
this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.USER_MAPPED_TO_FORCED_VARIATION, MODULE_NAME, variationId, experimentId, userId));
509+
};
510+
511+
/**
512+
* Gets the forced variation key for the given user and experiment.
513+
* @param {Object} configObj Object representing project configuration
514+
* @param {string} experimentKey Key for experiment.
515+
* @param {string} userId The user Id.
516+
* @return {string|null} Variation The variation which the given user and experiment should be forced into.
517+
*/
518+
DecisionService.prototype.getForcedVariation = function(configObj, experimentKey, userId) {
519+
var experimentToVariationMap = this.forcedVariationMap[userId];
520+
if (!experimentToVariationMap) {
521+
this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.USER_HAS_NO_FORCED_VARIATION, MODULE_NAME, userId));
522+
return null;
523+
}
524+
525+
var experimentId;
526+
try {
527+
var experiment = projectConfig.getExperimentFromKey(configObj, experimentKey);
528+
if (experiment.hasOwnProperty('id')) {
529+
experimentId = experiment['id'];
530+
} else {
531+
// catching improperly formatted experiments
532+
this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.IMPROPERLY_FORMATTED_EXPERIMENT, MODULE_NAME, experimentKey));
533+
return null;
534+
}
535+
} catch (ex) {
536+
// catching experiment not in datafile
537+
this.logger.log(LOG_LEVEL.ERROR, ex.message);
538+
return null;
539+
}
540+
541+
var variationId = experimentToVariationMap[experimentId];
542+
if (!variationId) {
543+
this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.USER_HAS_NO_FORCED_VARIATION_FOR_EXPERIMENT, MODULE_NAME, experimentKey, userId));
544+
return null;
545+
}
546+
547+
var variationKey = projectConfig.getVariationKeyFromId(configObj, variationId);
548+
if (variationKey) {
549+
this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.USER_HAS_FORCED_VARIATION, MODULE_NAME, variationKey, experimentKey, userId));
550+
} else {
551+
this.logger.log(LOG_LEVEL.DEBUG, sprintf(LOG_MESSAGES.USER_HAS_NO_FORCED_VARIATION_FOR_EXPERIMENT, MODULE_NAME, experimentKey, userId));
552+
}
553+
554+
return variationKey;
555+
};
556+
557+
/**
558+
* Sets the forced variation for a user in a given experiment
559+
* @param {Object} configObj Object representing project configuration
560+
* @param {string} experimentKey Key for experiment.
561+
* @param {string} userId The user Id.
562+
* @param {string} variationKey Key for variation. If null, then clear the existing experiment-to-variation mapping
563+
* @return {boolean} A boolean value that indicates if the set completed successfully.
564+
*/
565+
DecisionService.prototype.setForcedVariation = function(configObj, experimentKey, userId, variationKey) {
566+
if (variationKey != null && !stringValidator.validate(variationKey)) {
567+
this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.INVALID_VARIATION_KEY, MODULE_NAME));
568+
return false;
569+
}
570+
571+
var experimentId;
572+
try {
573+
var experiment = projectConfig.getExperimentFromKey(configObj, experimentKey);
574+
if (experiment.hasOwnProperty('id')) {
575+
experimentId = experiment['id'];
576+
} else {
577+
// catching improperly formatted experiments
578+
this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.IMPROPERLY_FORMATTED_EXPERIMENT, MODULE_NAME, experimentKey));
579+
return false;
580+
}
581+
} catch (ex) {
582+
// catching experiment not in datafile
583+
this.logger.log(LOG_LEVEL.ERROR, ex.message);
584+
return false;
585+
}
586+
587+
if (variationKey == null) {
588+
try {
589+
this.removeForcedVariation(userId, experimentId, experimentKey, this.logger);
590+
return true;
591+
} catch (ex) {
592+
this.logger.log(LOG_LEVEL.ERROR, ex.message);
593+
return false;
594+
}
595+
}
596+
597+
var variationId = projectConfig.getVariationIdFromExperimentAndVariationKey(configObj, experimentKey, variationKey);
598+
599+
if (!variationId) {
600+
this.logger.log(LOG_LEVEL.ERROR, sprintf(ERROR_MESSAGES.NO_VARIATION_FOR_EXPERIMENT_KEY, MODULE_NAME, variationKey, experimentKey));
601+
return false;
602+
}
603+
604+
try {
605+
this.__setInForcedVariationMap(userId, experimentId, variationId);
606+
return true;
607+
} catch (ex) {
608+
this.logger.log(LOG_LEVEL.ERROR, ex.message);
609+
return false;
610+
}
611+
};
612+
471613
module.exports = {
472614
/**
473615
* Creates an instance of the DecisionService.

0 commit comments

Comments
 (0)