From 3791c16134e8a32fbbff1252b8f2a442f4dfd5f3 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Tue, 14 Dec 2021 22:09:06 +0500 Subject: [PATCH 1/4] Removed getflagVariationByKey from optimizely and moved it to project config also instead of getting variation from rule --- .../lib/core/decision_service/index.ts | 7 +++--- .../lib/core/project_config/index.ts | 21 ++++++++++++++++++ .../optimizely-sdk/lib/optimizely/index.ts | 22 +------------------ .../lib/optimizely_user_context/index.ts | 12 +++++++--- packages/optimizely-sdk/lib/shared_types.ts | 5 ++++- 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.ts b/packages/optimizely-sdk/lib/core/decision_service/index.ts index c32cb1bb1..11925c090 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.ts +++ b/packages/optimizely-sdk/lib/core/decision_service/index.ts @@ -31,6 +31,7 @@ import { getExperimentAudienceConditions, getExperimentFromId, getExperimentFromKey, + getFlagVariationByKey, getTrafficAllocation, getVariationIdFromExperimentAndVariationKey, getVariationFromId, @@ -614,7 +615,7 @@ export class DecisionService { decideReasons.push(...decisionVariation.reasons); variationKey = decisionVariation.result; if (variationKey) { - const variation = experiment.variationKeyMap[variationKey]; + const variation = getFlagVariationByKey(configObj, feature.key, variationKey); variationForFeatureExperiment = { experiment: experiment, variation: variation, @@ -1016,7 +1017,7 @@ export class DecisionService { const decideReasons: (string | number)[][] = []; // check forced decision first - const forcedDecisionResponse = user.findValidatedForcedDecision(flagKey, rule.key); + const forcedDecisionResponse = user.findValidatedForcedDecision(configObj, flagKey, rule.key); decideReasons.push(...forcedDecisionResponse.reasons); const forcedVariaton = forcedDecisionResponse.result; @@ -1048,7 +1049,7 @@ export class DecisionService { // check forced decision first const rule = rules[ruleIndex]; - const forcedDecisionResponse = user.findValidatedForcedDecision(flagKey, rule.key); + const forcedDecisionResponse = user.findValidatedForcedDecision(configObj, flagKey, rule.key); decideReasons.push(...forcedDecisionResponse.reasons); const forcedVariaton = forcedDecisionResponse.result; diff --git a/packages/optimizely-sdk/lib/core/project_config/index.ts b/packages/optimizely-sdk/lib/core/project_config/index.ts index 8874652b9..a0f24e585 100644 --- a/packages/optimizely-sdk/lib/core/project_config/index.ts +++ b/packages/optimizely-sdk/lib/core/project_config/index.ts @@ -508,6 +508,26 @@ export const getExperimentFromId = function( return null; }; +/** +* Returns flag variation for specified flagKey and variationKey +* @param {flagKey} string +* @param {variationKey} string +* @return {OptimizelyVariation|null} +*/ +export const getFlagVariationByKey = function(projectConfig: ProjectConfig, flagKey: string, variationKey: string): OptimizelyVariation | null { + if (!projectConfig) { + return null; + } + + const variations = projectConfig.flagVariationsMap[flagKey]; + const result = find(variations, item => item.key === variationKey) + if (result) { + return result; + } + + return null; +}; + /** * Get feature from provided feature key. Log an error if no feature exists in * the project config with the given key. @@ -817,6 +837,7 @@ export default { getExperimentFromKey, getTrafficAllocation, getExperimentFromId, + getFlagVariationByKey, getFeatureFromKey, getVariableForFeature, getVariableValueForVariation, diff --git a/packages/optimizely-sdk/lib/optimizely/index.ts b/packages/optimizely-sdk/lib/optimizely/index.ts index 2996f027f..f52388f3a 100644 --- a/packages/optimizely-sdk/lib/optimizely/index.ts +++ b/packages/optimizely-sdk/lib/optimizely/index.ts @@ -1481,7 +1481,7 @@ export default class Optimizely { const allDecideOptions = this.getAllDecideOptions(options); - const forcedDecisionResponse = user.findValidatedForcedDecision(key); + const forcedDecisionResponse = user.findValidatedForcedDecision(configObj, key); reasons.push(...forcedDecisionResponse.reasons); const variation = forcedDecisionResponse.result; if (variation) { @@ -1672,24 +1672,4 @@ export default class Optimizely { return this.decideForKeys(user, allFlagKeys, options); } - /** - * Returns flag variation for specified flagKey and variationKey - * @param {flagKey} string - * @param {variationKey} string - * @return {OptimizelyVariation|null} - */ - getFlagVariationByKey(flagKey: string, variationKey: string): OptimizelyVariation | null { - const configObj = this.projectConfigManager.getConfig(); - if (!configObj) { - return null; - } - - const variations = configObj.flagVariationsMap[flagKey]; - const result = find(variations, item => item.key === variationKey) - if (result) { - return result; - } - - return null; - } } diff --git a/packages/optimizely-sdk/lib/optimizely_user_context/index.ts b/packages/optimizely-sdk/lib/optimizely_user_context/index.ts index a7a3d4a62..d8b81cc41 100644 --- a/packages/optimizely-sdk/lib/optimizely_user_context/index.ts +++ b/packages/optimizely-sdk/lib/optimizely_user_context/index.ts @@ -26,6 +26,10 @@ import { UserAttributes, Variation } from '../../lib/shared_types'; +import { + getFlagVariationByKey, + ProjectConfig, +} from '../core/project_config'; import { LOG_MESSAGES, CONTROL_ATTRIBUTES } from '../utils/enums'; const logger = getLogger(); @@ -211,22 +215,24 @@ export default class OptimizelyUserContext { /** * Finds a validated forced decision for specific flagKey and optional ruleKey. + * @param {ProjectConfig}config A projectConfig. * @param {string} flagKey A flagKey. * @param {ruleKey} ruleKey A ruleKey (optional). * @return {DecisionResponse} DecisionResponse object containing valid variation object and decide reasons. */ findValidatedForcedDecision( + config: ProjectConfig, flagKey: string, - ruleKey?: string + ruleKey?: string, ): DecisionResponse { const decideReasons: (string | number)[][] = []; const forcedDecision = this.findForcedDecision({ flagKey, ruleKey }); let variation = null; let variationKey; - if (forcedDecision) { + if (config && forcedDecision) { variationKey = forcedDecision.variationKey; - variation = this.optimizely.getFlagVariationByKey(flagKey, variationKey); + variation = getFlagVariationByKey(config, flagKey, variationKey); if (variation) { if (ruleKey) { logger.info( diff --git a/packages/optimizely-sdk/lib/shared_types.ts b/packages/optimizely-sdk/lib/shared_types.ts index a8f4a0a60..4c88af5d8 100644 --- a/packages/optimizely-sdk/lib/shared_types.ts +++ b/packages/optimizely-sdk/lib/shared_types.ts @@ -18,6 +18,8 @@ import { EventProcessor } from '@optimizely/js-sdk-event-processor'; import { NotificationCenter } from '@optimizely/js-sdk-utils'; +import { ProjectConfig } from './core/project_config'; + export interface BucketerParams { experimentId: string; experimentKey: string; @@ -381,8 +383,9 @@ export interface OptimizelyUserContext { ): { [key: string]: OptimizelyDecision }; trackEvent(eventName: string, eventTags?: EventTags): void; findValidatedForcedDecision( + config: ProjectConfig, flagKey: string, - ruleKey?: string, + ruleKey?: string ): DecisionResponse; setForcedDecision(context: OptimizelyDecisionContext, decision: OptimizelyForcedDecision): boolean; getForcedDecision(context: OptimizelyDecisionContext): OptimizelyForcedDecision | null; From d7b2a9ea9f990f4eab3f2eb407195cac981877e6 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Tue, 14 Dec 2021 22:29:26 +0500 Subject: [PATCH 2/4] fix --- packages/optimizely-sdk/lib/core/decision_service/index.ts | 6 +++++- packages/optimizely-sdk/lib/core/project_config/index.ts | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/optimizely-sdk/lib/core/decision_service/index.ts b/packages/optimizely-sdk/lib/core/decision_service/index.ts index 11925c090..c7e5e9cb7 100644 --- a/packages/optimizely-sdk/lib/core/decision_service/index.ts +++ b/packages/optimizely-sdk/lib/core/decision_service/index.ts @@ -615,7 +615,11 @@ export class DecisionService { decideReasons.push(...decisionVariation.reasons); variationKey = decisionVariation.result; if (variationKey) { - const variation = getFlagVariationByKey(configObj, feature.key, variationKey); + let variation = null; + variation = experiment.variationKeyMap[variationKey]; + if (!variation) { + variation = getFlagVariationByKey(configObj, feature.key, variationKey); + } variationForFeatureExperiment = { experiment: experiment, variation: variation, diff --git a/packages/optimizely-sdk/lib/core/project_config/index.ts b/packages/optimizely-sdk/lib/core/project_config/index.ts index a0f24e585..2ea2b8b41 100644 --- a/packages/optimizely-sdk/lib/core/project_config/index.ts +++ b/packages/optimizely-sdk/lib/core/project_config/index.ts @@ -512,9 +512,9 @@ export const getExperimentFromId = function( * Returns flag variation for specified flagKey and variationKey * @param {flagKey} string * @param {variationKey} string -* @return {OptimizelyVariation|null} +* @return {Variation|null} */ -export const getFlagVariationByKey = function(projectConfig: ProjectConfig, flagKey: string, variationKey: string): OptimizelyVariation | null { +export const getFlagVariationByKey = function(projectConfig: ProjectConfig, flagKey: string, variationKey: string): Variation | null { if (!projectConfig) { return null; } From 61a7969703f250bb7ae732483a0ca428a34116f2 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 15 Dec 2021 20:58:11 +0500 Subject: [PATCH 3/4] Added unit test --- .../optimizely_user_context/index.tests.js | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/optimizely-sdk/lib/optimizely_user_context/index.tests.js b/packages/optimizely-sdk/lib/optimizely_user_context/index.tests.js index eb13aad69..8c4bf0f8a 100644 --- a/packages/optimizely-sdk/lib/optimizely_user_context/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely_user_context/index.tests.js @@ -176,7 +176,39 @@ describe('lib/optimizely_user_context', function() { ); assert.deepEqual(decision, fakeDecision); }); - }); + + it('should return an expected decision object when forced decision is called and variation of different experiment but same flag key', function() { + var flagKey = 'feature_1'; + var ruleKey = 'exp_with_audience'; + var variationKey = '3324490633'; + + var fakeDecision = { + variationKey: variationKey, + enabled: true, + variables: {}, + ruleKey: ruleKey, + flagKey: flagKey, + userContext: 'fakeUserContext', + reasons: [], + }; + fakeOptimizely = { + decide: sinon.stub().returns(fakeDecision) + }; + var user = new OptimizelyUserContext({ + optimizely: fakeOptimizely, + userId, + }); + user.setForcedDecision({ flagKey: flagKey, ruleKey }, { variationKey }); + var decision = user.decide(flagKey, options); + sinon.assert.calledWithExactly( + fakeOptimizely.decide, + user, + flagKey, + options + ); + assert.deepEqual(decision, fakeDecision); + }); + }); describe('#decideForKeys', function() { it('should return an expected decision results object', function() { From 3b87b7d31e97e40dc9c965881873e29dd04caab3 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Fri, 17 Dec 2021 19:04:10 +0500 Subject: [PATCH 4/4] Fixed unit test --- .../optimizely_user_context/index.tests.js | 49 +++++++------------ 1 file changed, 17 insertions(+), 32 deletions(-) diff --git a/packages/optimizely-sdk/lib/optimizely_user_context/index.tests.js b/packages/optimizely-sdk/lib/optimizely_user_context/index.tests.js index 8c4bf0f8a..a7d49d9a0 100644 --- a/packages/optimizely-sdk/lib/optimizely_user_context/index.tests.js +++ b/packages/optimizely-sdk/lib/optimizely_user_context/index.tests.js @@ -176,38 +176,6 @@ describe('lib/optimizely_user_context', function() { ); assert.deepEqual(decision, fakeDecision); }); - - it('should return an expected decision object when forced decision is called and variation of different experiment but same flag key', function() { - var flagKey = 'feature_1'; - var ruleKey = 'exp_with_audience'; - var variationKey = '3324490633'; - - var fakeDecision = { - variationKey: variationKey, - enabled: true, - variables: {}, - ruleKey: ruleKey, - flagKey: flagKey, - userContext: 'fakeUserContext', - reasons: [], - }; - fakeOptimizely = { - decide: sinon.stub().returns(fakeDecision) - }; - var user = new OptimizelyUserContext({ - optimizely: fakeOptimizely, - userId, - }); - user.setForcedDecision({ flagKey: flagKey, ruleKey }, { variationKey }); - var decision = user.decide(flagKey, options); - sinon.assert.calledWithExactly( - fakeOptimizely.decide, - user, - flagKey, - options - ); - assert.deepEqual(decision, fakeDecision); - }); }); describe('#decideForKeys', function() { @@ -413,6 +381,23 @@ describe('lib/optimizely_user_context', function() { optlyInstance.notificationCenter.sendNotifications.restore(); }); + it('should return an expected decision object when forced decision is called and variation of different experiment but same flag key', function() { + var flagKey = 'feature_1'; + var ruleKey = 'exp_with_audience'; + var variationKey = '3324490633'; + + var user = optlyInstance.createUserContext(userId); + user.setForcedDecision({ flagKey: flagKey, ruleKey }, { variationKey }); + var decision = user.decide(flagKey, options); + + assert.equal(decision.variationKey, variationKey); + assert.equal(decision.ruleKey, ruleKey); + assert.equal(decision.enabled, true); + assert.equal(decision.userContext.getUserId(), userId); + assert.deepEqual(decision.userContext.getAttributes(), {}); + assert.deepEqual(Object.keys(decision.userContext.forcedDecisionsMap).length, 1); + }); + it('should return forced decision object when forced decision is set for a flag and do NOT dispatch an event with DISABLE_DECISION_EVENT passed in decide options', function() { var user = optlyInstance.createUserContext(userId); var featureKey = 'feature_1';