diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index e2a186eca..8ddc736eb 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -14,7 +14,7 @@ * limitations under the License. */ import { describe, it, expect, vi, MockInstance, beforeEach } from 'vitest'; -import { CMAB_FETCH_FAILED, DecisionService } from '.'; +import { CMAB_DUMMY_ENTITY_ID, CMAB_FETCH_FAILED, DecisionService } from '.'; import { getMockLogger } from '../../tests/mock/mock_logger'; import OptimizelyUserContext from '../../optimizely_user_context'; import { bucket } from '../bucketer'; @@ -140,10 +140,18 @@ const verifyBucketCall = ( variationIdMap, bucketingId, } = mockBucket.mock.calls[call][0]; + let expectedTrafficAllocation = experiment.trafficAllocation; + if (experiment.cmab) { + expectedTrafficAllocation = [{ + endOfRange: experiment.cmab.trafficAllocation, + entityId: CMAB_DUMMY_ENTITY_ID, + }]; + } + expect(experimentId).toBe(experiment.id); expect(experimentKey).toBe(experiment.key); expect(userId).toBe(user.getUserId()); - expect(trafficAllocationConfig).toBe(experiment.trafficAllocation); + expect(trafficAllocationConfig).toEqual(expectedTrafficAllocation); expect(experimentKeyMap).toBe(projectConfig.experimentKeyMap); expect(experimentIdMap).toBe(projectConfig.experimentIdMap); expect(groupIdMap).toBe(projectConfig.groupIdMap); @@ -1327,7 +1335,8 @@ describe('DecisionService', () => { }); }); - it('should get decision from the cmab service if the experiment is a cmab experiment', async () => { + it('should not return variation and should not call cmab service \ + for cmab experiment if user is not bucketed into it', async () => { const { decisionService, cmabService } = getDecisionService(); const config = createProjectConfig(getDecisionTestDatafile()); @@ -1340,6 +1349,57 @@ describe('DecisionService', () => { }, }); + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey == 'default-rollout-key') { + return { result: param.trafficAllocationConfig[0].entityId, reasons: [] } + } + return { + result: null, + reasons: [], + } + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + expect(value).toBeInstanceOf(Promise); + + const variation = (await value)[0]; + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['default-rollout-key'], + variation: config.variationIdMap['5007'], + decisionSource: DECISION_SOURCES.ROLLOUT, + }); + + verifyBucketCall(0, config, config.experimentKeyMap['exp_3'], user); + expect(cmabService.getDecision).not.toHaveBeenCalled(); + }); + + it('should get decision from the cmab service if the experiment is a cmab experiment \ + and user is bucketed into it', async () => { + const { decisionService, cmabService } = getDecisionService(); + const config = createProjectConfig(getDecisionTestDatafile()); + + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { + country: 'BD', + age: 80, // should satisfy audience condition for exp_3 which is cmab and not others + }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey == 'exp_3') { + return { result: param.trafficAllocationConfig[0].entityId, reasons: [] } + } + return { + result: null, + reasons: [], + } + }); + cmabService.getDecision.mockResolvedValue({ variationId: '5003', cmabUuid: 'uuid-test', @@ -1357,6 +1417,8 @@ describe('DecisionService', () => { decisionSource: DECISION_SOURCES.FEATURE_TEST, }); + verifyBucketCall(0, config, config.experimentKeyMap['exp_3'], user); + expect(cmabService.getDecision).toHaveBeenCalledTimes(1); expect(cmabService.getDecision).toHaveBeenCalledWith( config, @@ -1379,6 +1441,17 @@ describe('DecisionService', () => { }, }); + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey == 'exp_3') { + return { result: param.trafficAllocationConfig[0].entityId, reasons: [] } + } + return { + result: null, + reasons: [], + } + }); + cmabService.getDecision.mockResolvedValue({ variationId: '5003', cmabUuid: 'uuid-test', @@ -1424,6 +1497,17 @@ describe('DecisionService', () => { }, }); + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey == 'exp_3') { + return { result: param.trafficAllocationConfig[0].entityId, reasons: [] } + } + return { + result: null, + reasons: [], + } + }); + cmabService.getDecision.mockRejectedValue(new Error('I am an error')); const feature = config.featureKeyMap['flag_1']; @@ -1474,6 +1558,17 @@ describe('DecisionService', () => { userProfileServiceAsync?.save.mockImplementation(() => Promise.resolve()); + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey == 'exp_3') { + return { result: param.trafficAllocationConfig[0].entityId, reasons: [] } + } + return { + result: null, + reasons: [], + } + }); + cmabService.getDecision.mockResolvedValue({ variationId: '5003', cmabUuid: 'uuid-test', @@ -1552,6 +1647,17 @@ describe('DecisionService', () => { userProfileServiceAsync?.save.mockImplementation(() => Promise.resolve()); + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey == 'exp_3') { + return { result: param.trafficAllocationConfig[0].entityId, reasons: [] } + } + return { + result: null, + reasons: [], + } + }); + cmabService.getDecision.mockResolvedValue({ variationId: '5003', cmabUuid: 'uuid-test', @@ -1605,6 +1711,16 @@ describe('DecisionService', () => { userProfileServiceAsync?.save.mockRejectedValue(new Error('I am an error')); + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey == 'exp_3') { + return { result: param.trafficAllocationConfig[0].entityId, reasons: [] } + } + return { + result: null, + reasons: [], + } + }); cmabService.getDecision.mockResolvedValue({ variationId: '5003', @@ -1669,6 +1785,16 @@ describe('DecisionService', () => { userProfileServiceAsync?.lookup.mockResolvedValue(null); userProfileServiceAsync?.save.mockResolvedValue(null); + mockBucket.mockImplementation((param: BucketerParams) => { + const ruleKey = param.experimentKey; + if (ruleKey == 'exp_3') { + return { result: param.trafficAllocationConfig[0].entityId, reasons: [] } + } + return { + result: null, + reasons: [], + } + }); cmabService.getDecision.mockResolvedValue({ variationId: '5003', diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 82b6aa028..5d5e57da9 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -27,12 +27,12 @@ import { getExperimentFromId, getExperimentFromKey, getFlagVariationByKey, - getTrafficAllocation, getVariationIdFromExperimentAndVariationKey, getVariationFromId, getVariationKeyFromId, isActive, ProjectConfig, + getTrafficAllocation, } from '../../project_config/project_config'; import { AudienceEvaluator, createAudienceEvaluator } from '../audience_evaluator'; import * as stringValidator from '../../utils/string_value_validator'; @@ -44,6 +44,7 @@ import { FeatureFlag, OptimizelyDecideOption, OptimizelyUserContext, + TrafficAllocation, UserAttributes, UserProfile, UserProfileService, @@ -148,6 +149,9 @@ type VariationIdWithCmabParams = { cmabUuid?: string; }; export type DecideOptionsMap = Partial>; + +export const CMAB_DUMMY_ENTITY_ID= '$' + /** * Optimizely's decision service that determines which variation of an experiment the user will be allocated to. * @@ -355,6 +359,23 @@ export class DecisionService { reasons: [[CMAB_NOT_SUPPORTED_IN_SYNC]], }); } + + const userId = user.getUserId(); + const attributes = user.getAttributes(); + + const bucketingId = this.getBucketingId(userId, attributes); + const bucketerParams = this.buildBucketerParams(configObj, experiment, bucketingId, userId); + + const bucketerResult = bucket(bucketerParams); + + // this means the user is not in the cmab experiment + if (bucketerResult.result !== CMAB_DUMMY_ENTITY_ID) { + return Value.of(op, { + error: false, + result: {}, + reasons: bucketerResult.reasons, + }); + } const cmabPromise = this.cmabService.getDecision(configObj, user, experiment.id, decideOptions).then( (cmabDecision) => { @@ -573,6 +594,14 @@ export class DecisionService { bucketingId: string, userId: string ): BucketerParams { + let trafficAllocationConfig: TrafficAllocation[] = getTrafficAllocation(configObj, experiment.id); + if (experiment.cmab) { + trafficAllocationConfig = [{ + entityId: CMAB_DUMMY_ENTITY_ID, + endOfRange: experiment.cmab.trafficAllocation + }]; + } + return { bucketingId, experimentId: experiment.id, @@ -581,7 +610,7 @@ export class DecisionService { experimentKeyMap: configObj.experimentKeyMap, groupIdMap: configObj.groupIdMap, logger: this.logger, - trafficAllocationConfig: getTrafficAllocation(configObj, experiment.id), + trafficAllocationConfig, userId, variationIdMap: configObj.variationIdMap, } diff --git a/lib/project_config/project_config.spec.ts b/lib/project_config/project_config.spec.ts index 54aa75d97..5a0259ee4 100644 --- a/lib/project_config/project_config.spec.ts +++ b/lib/project_config/project_config.spec.ts @@ -249,17 +249,20 @@ describe('createProjectConfig - cmab experiments', () => { it('should populate cmab field correctly', function() { const datafile = testDatafile.getTestProjectConfig(); datafile.experiments[0].cmab = { - attributes: ['808797688', '808797689'], + attributeIds: ['808797688', '808797689'], + trafficAllocation: 3141, }; datafile.experiments[2].cmab = { - attributes: ['808797689'], + attributeIds: ['808797689'], + trafficAllocation: 1414, }; const configObj = projectConfig.createProjectConfig(datafile); const experiment0 = configObj.experiments[0]; expect(experiment0.cmab).toEqual({ + trafficAllocation: 3141, attributeIds: ['808797688', '808797689'], }); @@ -268,6 +271,7 @@ describe('createProjectConfig - cmab experiments', () => { const experiment2 = configObj.experiments[2]; expect(experiment2.cmab).toEqual({ + trafficAllocation: 1414, attributeIds: ['808797689'], }); }); diff --git a/lib/project_config/project_config.tests.js b/lib/project_config/project_config.tests.js index 6e93327cc..d69afda46 100644 --- a/lib/project_config/project_config.tests.js +++ b/lib/project_config/project_config.tests.js @@ -416,7 +416,7 @@ describe('lib/core/project_config', function() { assert.equal(ex.baseMessage, INVALID_EXPERIMENT_ID); assert.deepEqual(ex.params, ['invalidExperimentId']); }); - + describe('#getVariationIdFromExperimentAndVariationKey', function() { it('should return the variation id for the given experiment key and variation key', function() { assert.strictEqual( diff --git a/lib/project_config/project_config.ts b/lib/project_config/project_config.ts index 5a7674668..e91c4743a 100644 --- a/lib/project_config/project_config.ts +++ b/lib/project_config/project_config.ts @@ -157,15 +157,6 @@ export const createProjectConfig = function(datafileObj?: JSON, datafileStr: str projectConfig.__datafileStr = datafileStr === null ? JSON.stringify(datafileObj) : datafileStr; - /** rename cmab.attributes field from the datafile to cmab.attributeIds for each experiment */ - projectConfig.experiments.forEach(experiment => { - if (experiment.cmab) { - const attributes = (experiment.cmab as any).attributes; - delete (experiment.cmab as any).attributes; - experiment.cmab.attributeIds = attributes; - } - }); - /* * Conditions of audiences in projectConfig.typedAudiences are not * expected to be string-encoded as they are here in projectConfig.audiences. @@ -568,6 +559,7 @@ export const getExperimentFromKey = function(projectConfig: ProjectConfig, exper throw new OptimizelyError(EXPERIMENT_KEY_NOT_IN_DATAFILE, experimentKey); }; + /** * Given an experiment id, returns the traffic allocation within that experiment * @param {ProjectConfig} projectConfig Object representing project configuration @@ -890,7 +882,6 @@ export default { getVariationKeyFromId, getVariationIdFromExperimentAndVariationKey, getExperimentFromKey, - getTrafficAllocation, getExperimentFromId, getFlagVariationByKey, getFeatureFromKey, @@ -904,4 +895,5 @@ export default { isFeatureExperiment, toDatafile, tryCreatingProjectConfig, + getTrafficAllocation, }; diff --git a/lib/shared_types.ts b/lib/shared_types.ts index ea15b21e3..c203613a3 100644 --- a/lib/shared_types.ts +++ b/lib/shared_types.ts @@ -159,6 +159,7 @@ export interface Experiment { forcedVariations?: { [key: string]: string }; isRollout?: boolean; cmab?: { + trafficAllocation: number; attributeIds: string[]; }; }