Skip to content

Commit 41fed0d

Browse files
authored
[FSSDK-11132] make entity validation configurable in bucketer (#1071)
* [FSSDK-11132] make entity validation configurable in bucketer for cmab, we are using a dummy entityId which is not a real variation id. We need to skip entity validation for cmab experiments.
1 parent b73e364 commit 41fed0d

File tree

6 files changed

+29
-10
lines changed

6 files changed

+29
-10
lines changed

lib/core/bucketer/index.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ describe('excluding groups', () => {
8080
experimentIdMap: configObj.experimentIdMap,
8181
groupIdMap: configObj.groupIdMap,
8282
logger: mockLogger,
83+
validateEntity: true,
8384
};
8485

8586
vi.spyOn(bucketValueGenerator, 'generateBucketValue')
@@ -127,6 +128,7 @@ describe('including groups: random', () => {
127128
groupIdMap: configObj.groupIdMap,
128129
logger: mockLogger,
129130
userId: 'testUser',
131+
validateEntity: true,
130132
};
131133
});
132134

@@ -228,6 +230,7 @@ describe('including groups: overlapping', () => {
228230
groupIdMap: configObj.groupIdMap,
229231
logger: mockLogger,
230232
userId: 'testUser',
233+
validateEntity: true,
231234
};
232235
});
233236

@@ -280,6 +283,7 @@ describe('bucket value falls into empty traffic allocation ranges', () => {
280283
experimentIdMap: configObj.experimentIdMap,
281284
groupIdMap: configObj.groupIdMap,
282285
logger: mockLogger,
286+
validateEntity: true,
283287
};
284288
});
285289

@@ -329,6 +333,7 @@ describe('traffic allocation has invalid variation ids', () => {
329333
experimentIdMap: configObj.experimentIdMap,
330334
groupIdMap: configObj.groupIdMap,
331335
logger: mockLogger,
336+
validateEntity: true,
332337
};
333338
});
334339

@@ -359,6 +364,7 @@ describe('testBucketWithBucketingId', () => {
359364
variationIdMap: configObj.variationIdMap,
360365
experimentIdMap: configObj.experimentIdMap,
361366
groupIdMap: configObj.groupIdMap,
367+
validateEntity: true,
362368
};
363369
});
364370

lib/core/bucketer/index.tests.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ describe('lib/core/bucketer', function () {
7474
experimentIdMap: configObj.experimentIdMap,
7575
groupIdMap: configObj.groupIdMap,
7676
logger: createdLogger,
77+
validateEntity: true,
7778
};
7879
sinon
7980
.stub(bucketValueGenerator, 'generateBucketValue')
@@ -115,6 +116,7 @@ describe('lib/core/bucketer', function () {
115116
experimentIdMap: configObj.experimentIdMap,
116117
groupIdMap: configObj.groupIdMap,
117118
logger: createdLogger,
119+
validateEntity: true,
118120
};
119121
bucketerStub = sinon.stub(bucketValueGenerator, 'generateBucketValue');
120122
});
@@ -135,6 +137,7 @@ describe('lib/core/bucketer', function () {
135137
groupIdMap: configObj.groupIdMap,
136138
logger: createdLogger,
137139
userId: 'testUser',
140+
validateEntity: true,
138141
};
139142
});
140143

@@ -225,6 +228,7 @@ describe('lib/core/bucketer', function () {
225228
groupIdMap: configObj.groupIdMap,
226229
logger: createdLogger,
227230
userId: 'testUser',
231+
validateEntity: true,
228232
};
229233
});
230234

@@ -269,6 +273,7 @@ describe('lib/core/bucketer', function () {
269273
experimentIdMap: configObj.experimentIdMap,
270274
groupIdMap: configObj.groupIdMap,
271275
logger: createdLogger,
276+
validateEntity: true,
272277
};
273278
});
274279

@@ -316,6 +321,7 @@ describe('lib/core/bucketer', function () {
316321
experimentIdMap: configObj.experimentIdMap,
317322
groupIdMap: configObj.groupIdMap,
318323
logger: createdLogger,
324+
validateEntity: true,
319325
};
320326
});
321327

@@ -365,6 +371,7 @@ describe('lib/core/bucketer', function () {
365371
experimentIdMap: configObj.experimentIdMap,
366372
groupIdMap: configObj.groupIdMap,
367373
logger: createdLogger,
374+
validateEntity: true,
368375
};
369376
});
370377

lib/core/bucketer/index.ts

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -138,17 +138,16 @@ export const bucket = function(bucketerParams: BucketerParams): DecisionResponse
138138
]);
139139

140140
const entityId = _findBucket(bucketValue, bucketerParams.trafficAllocationConfig);
141-
if (entityId !== null) {
142-
if (!bucketerParams.variationIdMap[entityId]) {
143-
if (entityId) {
144-
bucketerParams.logger?.warn(INVALID_VARIATION_ID);
145-
decideReasons.push([INVALID_VARIATION_ID]);
146-
}
147-
return {
148-
result: null,
149-
reasons: decideReasons,
150-
};
141+
142+
if (bucketerParams.validateEntity && entityId !== null && !bucketerParams.variationIdMap[entityId]) {
143+
if (entityId) {
144+
bucketerParams.logger?.warn(INVALID_VARIATION_ID);
145+
decideReasons.push([INVALID_VARIATION_ID]);
151146
}
147+
return {
148+
result: null,
149+
reasons: decideReasons,
150+
};
152151
}
153152

154153
return {

lib/core/decision_service/index.tests.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,7 @@ describe('lib/core/decision_service', function() {
653653
experimentIdMap: configObj.experimentIdMap,
654654
experimentKeyMap: configObj.experimentKeyMap,
655655
groupIdMap: configObj.groupIdMap,
656+
validateEntity: true,
656657
};
657658

658659
assert.deepEqual(bucketerParams, expectedParams);

lib/core/decision_service/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -594,12 +594,16 @@ export class DecisionService {
594594
bucketingId: string,
595595
userId: string
596596
): BucketerParams {
597+
let validateEntity = true;
598+
597599
let trafficAllocationConfig: TrafficAllocation[] = getTrafficAllocation(configObj, experiment.id);
598600
if (experiment.cmab) {
599601
trafficAllocationConfig = [{
600602
entityId: CMAB_DUMMY_ENTITY_ID,
601603
endOfRange: experiment.cmab.trafficAllocation
602604
}];
605+
606+
validateEntity = false;
603607
}
604608

605609
return {
@@ -613,6 +617,7 @@ export class DecisionService {
613617
trafficAllocationConfig,
614618
userId,
615619
variationIdMap: configObj.variationIdMap,
620+
validateEntity,
616621
}
617622
}
618623

lib/shared_types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export interface BucketerParams {
6464
variationIdMap: { [id: string]: Variation };
6565
logger?: LoggerFacade;
6666
bucketingId: string;
67+
validateEntity?: boolean;
6768
}
6869

6970
export interface DecisionResponse<T> {

0 commit comments

Comments
 (0)