Skip to content

Commit d7c714b

Browse files
raju-optiraju-opti
committed
fix decision service
1 parent 6150ce2 commit d7c714b

File tree

3 files changed

+62
-107
lines changed

3 files changed

+62
-107
lines changed

‎lib/core/decision_service/index.tests.js

Lines changed: 61 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,12 @@ import {
5555
USER_MEETS_CONDITIONS_FOR_TARGETING_RULE,
5656
USER_NOT_BUCKETED_INTO_TARGETING_RULE,
5757
USER_NOT_IN_ROLLOUT,
58-
VALID_BUCKETING_ID
58+
VALID_BUCKETING_ID,
59+
SAVED_USER_VARIATION,
60+
SAVED_VARIATION_NOT_FOUND
5961
} from '../../log_messages';
6062
import { mock } from 'node:test';
61-
import { BUCKETING_ID_NOT_STRING } from '../../error_messages';
63+
import { BUCKETING_ID_NOT_STRING, USER_PROFILE_LOOKUP_ERROR, USER_PROFILE_SAVE_ERROR } from '../../error_messages';
6264

6365
var testData = getTestProjectConfig();
6466
var testDataWithFeatures = getTestProjectConfigWithFeatures();
@@ -344,14 +346,20 @@ describe('lib/core/decision_service', function() {
344346
);
345347
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
346348
sinon.assert.calledOnce(bucketerStub);
347-
assert.strictEqual(
348-
buildLogMessageFromArgs(mockLogger.log.args[0]),
349-
'DECISION_SERVICE: User decision_service_user is not in the forced variation map.'
350-
);
351-
assert.strictEqual(
352-
buildLogMessageFromArgs(mockLogger.log.args[1]),
353-
'DECISION_SERVICE: User decision_service_user was previously bucketed into variation with ID not valid variation for experiment testExperiment, but no matching variation was found.'
349+
// assert.strictEqual(
350+
// buildLogMessageFromArgs(mockLogger.log.args[0]),
351+
// 'DECISION_SERVICE: User decision_service_user is not in the forced variation map.'
352+
// );
353+
assert.deepEqual(mockLogger.debug.args[0], [USER_HAS_NO_FORCED_VARIATION, 'decision_service_user']);
354+
355+
sinon.assert.calledWith(
356+
mockLogger.info,
357+
SAVED_VARIATION_NOT_FOUND,
358+
'decision_service_user',
359+
'not valid variation',
360+
'testExperiment'
354361
);
362+
355363
// make sure we save the decision
356364
sinon.assert.calledWith(userProfileSaveStub, {
357365
user_id: 'decision_service_user',
@@ -382,7 +390,7 @@ describe('lib/core/decision_service', function() {
382390
);
383391
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
384392
sinon.assert.calledOnce(bucketerStub);
385-
assert.strictEqual(5, mockLogger.log.callCount);
393+
386394
sinon.assert.calledWith(userProfileServiceInstance.save, {
387395
user_id: 'decision_service_user',
388396
experiment_bucket_map: {
@@ -391,14 +399,11 @@ describe('lib/core/decision_service', function() {
391399
},
392400
},
393401
});
394-
assert.strictEqual(
395-
buildLogMessageFromArgs(mockLogger.log.args[0]),
396-
'DECISION_SERVICE: User decision_service_user is not in the forced variation map.'
397-
);
398-
assert.strictEqual(
399-
buildLogMessageFromArgs(mockLogger.log.args[4]),
400-
'DECISION_SERVICE: Saved user profile for user "decision_service_user".'
401-
);
402+
403+
404+
assert.deepEqual(mockLogger.debug.args[0], [USER_HAS_NO_FORCED_VARIATION, 'decision_service_user']);
405+
406+
assert.deepEqual(mockLogger.info.lastCall.args, [SAVED_USER_VARIATION, 'decision_service_user']);
402407
});
403408

404409
it('should log an error message if "lookup" throws an error', function() {
@@ -417,14 +422,10 @@ describe('lib/core/decision_service', function() {
417422
);
418423
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
419424
sinon.assert.calledOnce(bucketerStub); // should still go through with bucketing
420-
assert.strictEqual(
421-
buildLogMessageFromArgs(mockLogger.log.args[0]),
422-
'DECISION_SERVICE: Error while looking up user profile for user ID "decision_service_user": I am an error.'
423-
);
424-
assert.strictEqual(
425-
buildLogMessageFromArgs(mockLogger.log.args[1]),
426-
'DECISION_SERVICE: User decision_service_user is not in the forced variation map.'
427-
);
425+
426+
assert.deepEqual(mockLogger.error.args[0], [USER_PROFILE_LOOKUP_ERROR, 'decision_service_user', 'I am an error']);
427+
428+
assert.deepEqual(mockLogger.debug.args[0], [USER_HAS_NO_FORCED_VARIATION, 'decision_service_user']);
428429
});
429430

430431
it('should log an error message if "save" throws an error', function() {
@@ -444,15 +445,10 @@ describe('lib/core/decision_service', function() {
444445
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
445446
sinon.assert.calledOnce(bucketerStub); // should still go through with bucketing
446447

447-
assert.strictEqual(5, mockLogger.log.callCount);
448-
assert.strictEqual(
449-
buildLogMessageFromArgs(mockLogger.log.args[0]),
450-
'DECISION_SERVICE: User decision_service_user is not in the forced variation map.'
451-
);
452-
assert.strictEqual(
453-
buildLogMessageFromArgs(mockLogger.log.args[4]),
454-
'DECISION_SERVICE: Error while saving user profile for user ID "decision_service_user": I am an error.'
455-
);
448+
449+
assert.deepEqual(mockLogger.debug.args[0], [USER_HAS_NO_FORCED_VARIATION, 'decision_service_user']);
450+
451+
assert.deepEqual(mockLogger.error.args[0], [USER_PROFILE_SAVE_ERROR, 'decision_service_user', 'I am an error']);
456452

457453
// make sure that we save the decision
458454
sinon.assert.calledWith(userProfileSaveStub, {
@@ -499,14 +495,10 @@ describe('lib/core/decision_service', function() {
499495
);
500496
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
501497
sinon.assert.notCalled(bucketerStub);
502-
assert.strictEqual(
503-
buildLogMessageFromArgs(mockLogger.log.args[0]),
504-
'DECISION_SERVICE: User decision_service_user is not in the forced variation map.'
505-
);
506-
assert.strictEqual(
507-
buildLogMessageFromArgs(mockLogger.log.args[1]),
508-
'DECISION_SERVICE: Returning previously activated variation "variation" of experiment "testExperiment" for user "decision_service_user" from user profile.'
509-
);
498+
499+
assert.deepEqual(mockLogger.debug.args[0], [USER_HAS_NO_FORCED_VARIATION, 'decision_service_user']);
500+
501+
assert.deepEqual(mockLogger.info.args[0], [RETURNING_STORED_VARIATION, 'variation', 'testExperiment', 'decision_service_user']);
510502
});
511503

512504
it('should ignore attributes for a different experiment id', function() {
@@ -544,14 +536,10 @@ describe('lib/core/decision_service', function() {
544536
);
545537
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
546538
sinon.assert.notCalled(bucketerStub);
547-
assert.strictEqual(
548-
buildLogMessageFromArgs(mockLogger.log.args[0]),
549-
'DECISION_SERVICE: User decision_service_user is not in the forced variation map.'
550-
);
551-
assert.strictEqual(
552-
buildLogMessageFromArgs(mockLogger.log.args[1]),
553-
'DECISION_SERVICE: Returning previously activated variation "control" of experiment "testExperiment" for user "decision_service_user" from user profile.'
554-
);
539+
540+
assert.deepEqual(mockLogger.debug.args[0], [USER_HAS_NO_FORCED_VARIATION, 'decision_service_user']);
541+
542+
assert.deepEqual(mockLogger.info.args[0], [RETURNING_STORED_VARIATION, 'control', 'testExperiment', 'decision_service_user']);
555543
});
556544

557545
it('should use attributes when the userProfileLookup variations for other experiments', function() {
@@ -589,14 +577,10 @@ describe('lib/core/decision_service', function() {
589577
);
590578
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
591579
sinon.assert.notCalled(bucketerStub);
592-
assert.strictEqual(
593-
buildLogMessageFromArgs(mockLogger.log.args[0]),
594-
'DECISION_SERVICE: User decision_service_user is not in the forced variation map.'
595-
);
596-
assert.strictEqual(
597-
buildLogMessageFromArgs(mockLogger.log.args[1]),
598-
'DECISION_SERVICE: Returning previously activated variation "variation" of experiment "testExperiment" for user "decision_service_user" from user profile.'
599-
);
580+
581+
assert.deepEqual(mockLogger.debug.args[0], [USER_HAS_NO_FORCED_VARIATION, 'decision_service_user']);
582+
583+
assert.deepEqual(mockLogger.info.args[0], [RETURNING_STORED_VARIATION, 'variation', 'testExperiment', 'decision_service_user']);
600584
});
601585

602586
it('should use attributes when the userProfileLookup returns null', function() {
@@ -625,14 +609,10 @@ describe('lib/core/decision_service', function() {
625609
);
626610
sinon.assert.calledWith(userProfileLookupStub, 'decision_service_user');
627611
sinon.assert.notCalled(bucketerStub);
628-
assert.strictEqual(
629-
buildLogMessageFromArgs(mockLogger.log.args[0]),
630-
'DECISION_SERVICE: User decision_service_user is not in the forced variation map.'
631-
);
632-
assert.strictEqual(
633-
buildLogMessageFromArgs(mockLogger.log.args[1]),
634-
'DECISION_SERVICE: Returning previously activated variation "variation" of experiment "testExperiment" for user "decision_service_user" from user profile.'
635-
);
612+
613+
assert.deepEqual(mockLogger.debug.args[0], [USER_HAS_NO_FORCED_VARIATION, 'decision_service_user']);
614+
615+
assert.deepEqual(mockLogger.info.args[0], [RETURNING_STORED_VARIATION, 'variation', 'testExperiment', 'decision_service_user']);
636616
});
637617
});
638618
});
@@ -671,8 +651,6 @@ describe('lib/core/decision_service', function() {
671651
};
672652

673653
assert.deepEqual(bucketerParams, expectedParams);
674-
675-
sinon.assert.notCalled(mockLogger.log);
676654
});
677655
});
678656

@@ -708,15 +686,10 @@ describe('lib/core/decision_service', function() {
708686
''
709687
).result
710688
);
711-
assert.strictEqual(2, mockLogger.log.callCount);
712-
assert.strictEqual(
713-
buildLogMessageFromArgs(mockLogger.log.args[0]),
714-
'DECISION_SERVICE: Evaluating audiences for experiment "testExperimentWithAudiences": ["11154"].'
715-
);
716-
assert.strictEqual(
717-
buildLogMessageFromArgs(mockLogger.log.args[1]),
718-
'DECISION_SERVICE: Audiences for experiment testExperimentWithAudiences collectively evaluated to TRUE.'
719-
);
689+
690+
assert.deepEqual(mockLogger.debug.args[0], [EVALUATING_AUDIENCES_COMBINED, 'experiment', 'testExperimentWithAudiences', JSON.stringify(["11154"])]);
691+
692+
assert.deepEqual(mockLogger.info.args[0], [AUDIENCE_EVALUATION_RESULT_COMBINED, 'experiment', 'testExperimentWithAudiences', 'TRUE']);
720693
});
721694

722695
it('should return decision response with result true when experiment has no audience', function() {
@@ -732,15 +705,9 @@ describe('lib/core/decision_service', function() {
732705
);
733706
assert.isTrue(__audienceEvaluateSpy.alwaysReturned(true));
734707

735-
assert.strictEqual(2, mockLogger.log.callCount);
736-
assert.strictEqual(
737-
buildLogMessageFromArgs(mockLogger.log.args[0]),
738-
'DECISION_SERVICE: Evaluating audiences for experiment "testExperiment": [].'
739-
);
740-
assert.strictEqual(
741-
buildLogMessageFromArgs(mockLogger.log.args[1]),
742-
'DECISION_SERVICE: Audiences for experiment testExperiment collectively evaluated to TRUE.'
743-
);
708+
assert.deepEqual(mockLogger.debug.args[0], [EVALUATING_AUDIENCES_COMBINED, 'experiment', 'testExperiment', JSON.stringify([])]);
709+
710+
assert.deepEqual(mockLogger.info.args[0], [AUDIENCE_EVALUATION_RESULT_COMBINED, 'experiment', 'testExperiment', 'TRUE']);
744711
});
745712

746713
it('should return decision response with result false when audience conditions can not be evaluated', function() {
@@ -756,15 +723,9 @@ describe('lib/core/decision_service', function() {
756723
);
757724
assert.isTrue(__audienceEvaluateSpy.alwaysReturned(false));
758725

759-
assert.strictEqual(2, mockLogger.log.callCount);
760-
assert.strictEqual(
761-
buildLogMessageFromArgs(mockLogger.log.args[0]),
762-
'DECISION_SERVICE: Evaluating audiences for experiment "testExperimentWithAudiences": ["11154"].'
763-
);
764-
assert.strictEqual(
765-
buildLogMessageFromArgs(mockLogger.log.args[1]),
766-
'DECISION_SERVICE: Audiences for experiment testExperimentWithAudiences collectively evaluated to FALSE.'
767-
);
726+
assert.deepEqual(mockLogger.debug.args[0], [EVALUATING_AUDIENCES_COMBINED, 'experiment', 'testExperimentWithAudiences', JSON.stringify(["11154"])]);
727+
728+
assert.deepEqual(mockLogger.info.args[0], [AUDIENCE_EVALUATION_RESULT_COMBINED, 'experiment', 'testExperimentWithAudiences', 'FALSE']);
768729
});
769730

770731
it('should return decision response with result false when audience conditions are not met', function() {
@@ -780,15 +741,10 @@ describe('lib/core/decision_service', function() {
780741
);
781742
assert.isTrue(__audienceEvaluateSpy.alwaysReturned(false));
782743

783-
assert.strictEqual(2, mockLogger.log.callCount);
784-
assert.strictEqual(
785-
buildLogMessageFromArgs(mockLogger.log.args[0]),
786-
'DECISION_SERVICE: Evaluating audiences for experiment "testExperimentWithAudiences": ["11154"].'
787-
);
788-
assert.strictEqual(
789-
buildLogMessageFromArgs(mockLogger.log.args[1]),
790-
'DECISION_SERVICE: Audiences for experiment testExperimentWithAudiences collectively evaluated to FALSE.'
791-
);
744+
745+
assert.deepEqual(mockLogger.debug.args[0], [EVALUATING_AUDIENCES_COMBINED, 'experiment', 'testExperimentWithAudiences', JSON.stringify(["11154"])]);
746+
747+
assert.deepEqual(mockLogger.info.args[0], [AUDIENCE_EVALUATION_RESULT_COMBINED, 'experiment', 'testExperimentWithAudiences', 'FALSE']);
792748
});
793749
});
794750

‎lib/core/decision_service/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,6 @@ export class DecisionService {
550550
} catch (ex: any) {
551551
this.logger?.error(
552552
USER_PROFILE_LOOKUP_ERROR,
553-
MODULE_NAME,
554553
userId,
555554
ex.message,
556555
);

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@
7474
"clean:win": "(if exist dist rd /s/q dist)",
7575
"lint": "tsc --noEmit && eslint 'lib/**/*.js' 'lib/**/*.ts'",
7676
"test-vitest": "tsc --noEmit --p tsconfig.spec.json && vitest run",
77-
"test-mocha": "TS_NODE_COMPILER_OPTIONS='{\"module\": \"commonjs\" }' mocha -r ts-node/register -r lib/tests/exit_on_unhandled_rejection.js 'lib/**/*.tests.ts' 'lib/**/decision_service/*.tests.js'",
77+
"test-mocha": "TS_NODE_COMPILER_OPTIONS='{\"module\": \"commonjs\" }' mocha -r ts-node/register -r lib/tests/exit_on_unhandled_rejection.js 'lib/**/*.tests.ts' 'lib/**/**/*.tests.js'",
7878
"test": "npm run test-mocha && npm run test-vitest",
7979
"posttest": "npm run lint",
8080
"test-ci": "npm run test-xbrowser && npm run test-umdbrowser",

0 commit comments

Comments
 (0)