Skip to content

Commit ecaa0ee

Browse files
opti-jnguyenopti-jnguyen
committed
Final review round fixes + patch unit tests
1 parent 4b74b87 commit ecaa0ee

File tree

9 files changed

+115
-45
lines changed

9 files changed

+115
-45
lines changed

‎packages/optimizely-sdk/lib/index.browser.tests.js

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,6 @@ class MockLocalStorage {
5555
}
5656
}
5757

58-
const createBrowserOdpManager = config => {
59-
return new BrowserOdpManager({
60-
disable: config?.disabled || false,
61-
segmentsCache: new BrowserLRUCache({
62-
maxSize: config?.segmentsCacheSize,
63-
timeout: config?.segmentsCacheTimeout,
64-
}),
65-
segmentManager: config?.segmentManager,
66-
eventManager: config?.eventManager,
67-
logger: config?.logger,
68-
});
69-
};
70-
7158
if (!global.window) {
7259
try {
7360
global.window = {
@@ -607,7 +594,7 @@ describe('javascript-sdk (Browser)', function() {
607594
sandbox.restore();
608595
});
609596

610-
it('should send identify event when initialized with odp enabled', () => {
597+
it('should send identify event by default when initialized', () => {
611598
new OptimizelyUserContext({
612599
optimizely: fakeOptimizely,
613600
userId: testFsUserId,
@@ -625,9 +612,11 @@ describe('javascript-sdk (Browser)', function() {
625612
eventDispatcher: fakeEventDispatcher,
626613
eventBatchSize: null,
627614
logger,
628-
odpManager: createBrowserOdpManager({
629-
disabled: true,
615+
odpManager: BrowserOdpManager.createBrowserOdpManager({
630616
logger,
617+
odpOptions: {
618+
disabled: true,
619+
},
631620
}),
632621
});
633622

@@ -641,9 +630,11 @@ describe('javascript-sdk (Browser)', function() {
641630
eventDispatcher: fakeEventDispatcher,
642631
eventBatchSize: null,
643632
logger,
644-
odpManager: createBrowserOdpManager({
645-
segmentsCacheSize: 10,
633+
odpManager: BrowserOdpManager.createBrowserOdpManager({
646634
logger,
635+
odpOptions: {
636+
segmentsCacheSize: 10,
637+
},
647638
}),
648639
});
649640

@@ -661,9 +652,11 @@ describe('javascript-sdk (Browser)', function() {
661652
eventDispatcher: fakeEventDispatcher,
662653
eventBatchSize: null,
663654
logger,
664-
odpManager: createBrowserOdpManager({
665-
segmentsCacheTimeout: 10,
655+
odpManager: BrowserOdpManager.createBrowserOdpManager({
666656
logger,
657+
odpOptions: {
658+
segmentsCacheTimeout: 10,
659+
},
667660
}),
668661
});
669662

@@ -681,10 +674,12 @@ describe('javascript-sdk (Browser)', function() {
681674
eventDispatcher: fakeEventDispatcher,
682675
eventBatchSize: null,
683676
logger,
684-
odpManager: createBrowserOdpManager({
685-
segmentsCacheSize: 10,
686-
segmentsCacheTimeout: 10,
677+
odpManager: BrowserOdpManager.createBrowserOdpManager({
687678
logger,
679+
odpOptions: {
680+
segmentsCacheSize: 10,
681+
segmentsCacheTimeout: 10,
682+
},
688683
}),
689684
});
690685

@@ -713,9 +708,11 @@ describe('javascript-sdk (Browser)', function() {
713708
eventDispatcher: fakeEventDispatcher,
714709
eventBatchSize: null,
715710
logger,
716-
odpManager: createBrowserOdpManager({
717-
segmentManager: fakeSegmentManager,
711+
odpManager: BrowserOdpManager.createBrowserOdpManager({
718712
logger,
713+
odpOptions: {
714+
segmentManager: fakeSegmentManager,
715+
},
719716
}),
720717
});
721718

@@ -741,9 +738,11 @@ describe('javascript-sdk (Browser)', function() {
741738
eventDispatcher: fakeEventDispatcher,
742739
eventBatchSize: null,
743740
logger,
744-
odpManager: createBrowserOdpManager({
745-
eventManager: fakeEventManager,
741+
odpManager: BrowserOdpManager.createBrowserOdpManager({
746742
logger,
743+
odpOptions: {
744+
eventManager: fakeEventManager,
745+
},
747746
}),
748747
});
749748

@@ -789,9 +788,11 @@ describe('javascript-sdk (Browser)', function() {
789788
eventDispatcher: fakeEventDispatcher,
790789
eventBatchSize: null,
791790
logger,
792-
odpManager: createBrowserOdpManager({
793-
disabled: true,
791+
odpManager: BrowserOdpManager.createBrowserOdpManager({
794792
logger,
793+
odpOptions: {
794+
disabled: true,
795+
},
795796
}),
796797
});
797798

‎packages/optimizely-sdk/lib/index.browser.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ const createInstance = function(config: Config): Client | null {
125125
: undefined,
126126
notificationCenter,
127127
isValidInstance: isValidInstance,
128-
odpManager: config.odpManager || new BrowserOdpManager({ disable: false, logger }),
128+
odpManager: BrowserOdpManager.createBrowserOdpManager({ logger, odpOptions: config.odpOptions }),
129129
};
130130

131131
const optimizely = new BrowserOptimizely(optimizelyOptions);

‎packages/optimizely-sdk/lib/optimizely/index.tests.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,7 +1008,7 @@ describe('lib/optimizely', function() {
10081008
var activate = optlyInstance.activate('testExperiment', 'user1');
10091009
assert.strictEqual(activate, 'control');
10101010

1011-
sinon.assert.calledThrice(Optimizely.prototype.validateInputs);
1011+
sinon.assert.calledTwice(Optimizely.prototype.validateInputs);
10121012

10131013
var logMessage0 = buildLogMessageFromArgs(createdLogger.log.args[0]);
10141014
assert.strictEqual(
@@ -1920,7 +1920,7 @@ describe('lib/optimizely', function() {
19201920
var getVariation = optlyInstance.getVariation('testExperiment', 'user1');
19211921
assert.strictEqual(getVariation, 'control');
19221922

1923-
sinon.assert.calledTwice(Optimizely.prototype.validateInputs);
1923+
sinon.assert.calledOnce(Optimizely.prototype.validateInputs);
19241924

19251925
sinon.assert.calledTwice(createdLogger.log);
19261926

‎packages/optimizely-sdk/lib/optimizely/index.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ export default class Optimizely {
485485
const variationKey = this.decisionService.getVariation(
486486
configObj,
487487
experiment,
488-
this.createUserContext(userId, attributes) as OptimizelyUserContext
488+
this.createInternalUserContext(userId, attributes) as OptimizelyUserContext
489489
).result;
490490
const decisionNotificationType = projectConfig.isFeatureExperiment(configObj, experiment.id)
491491
? DECISION_NOTIFICATION_TYPES.FEATURE_TEST
@@ -657,7 +657,7 @@ export default class Optimizely {
657657
}
658658

659659
let sourceInfo = {};
660-
const user = this.createUserContext(userId, attributes) as OptimizelyUserContext;
660+
const user = this.createInternalUserContext(userId, attributes) as OptimizelyUserContext;
661661
const decisionObj = this.decisionService.getVariationForFeature(configObj, feature, user).result;
662662
const decisionSource = decisionObj.decisionSource;
663663
const experimentKey = decision.getExperimentKey(decisionObj);
@@ -833,7 +833,7 @@ export default class Optimizely {
833833
return null;
834834
}
835835

836-
const user = this.createUserContext(userId, attributes) as OptimizelyUserContext;
836+
const user = this.createInternalUserContext(userId, attributes) as OptimizelyUserContext;
837837
const decisionObj = this.decisionService.getVariationForFeature(configObj, featureFlag, user).result;
838838
const featureEnabled = decision.getFeatureEnabledFromVariation(decisionObj);
839839
const variableValue = this.getFeatureVariableValueFromVariation(
@@ -1165,7 +1165,7 @@ export default class Optimizely {
11651165
return null;
11661166
}
11671167

1168-
const user = this.createUserContext(userId, attributes) as OptimizelyUserContext;
1168+
const user = this.createInternalUserContext(userId, attributes) as OptimizelyUserContext;
11691169

11701170
const decisionObj = this.decisionService.getVariationForFeature(configObj, featureFlag, user).result;
11711171
const featureEnabled = decision.getFeatureEnabledFromVariation(decisionObj);
@@ -1438,6 +1438,26 @@ export default class Optimizely {
14381438
return null;
14391439
}
14401440

1441+
return new OptimizelyUserContext({
1442+
optimizely: this,
1443+
userId,
1444+
attributes,
1445+
shouldIdentifyUser: true,
1446+
});
1447+
}
1448+
1449+
/**
1450+
* Creates an internal context of the user for which decision APIs will be called.
1451+
*
1452+
* A user context will be created successfully even when the SDK is not fully configured yet, so no
1453+
* this.isValidInstance() check is performed here.
1454+
*
1455+
* @param {string} userId The user ID to be used for bucketing.
1456+
* @param {UserAttributes} attributes Optional user attributes.
1457+
* @return {OptimizelyUserContext|null} An OptimizelyUserContext associated with this OptimizelyClient or
1458+
* null if provided inputs are invalid
1459+
*/
1460+
private createInternalUserContext(userId: string, attributes?: UserAttributes): OptimizelyUserContext | null {
14411461
return new OptimizelyUserContext({
14421462
optimizely: this,
14431463
userId,

‎packages/optimizely-sdk/lib/optimizely_user_context/index.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@ export default class OptimizelyUserContext {
3939
private forcedDecisionsMap: { [key: string]: { [key: string]: OptimizelyForcedDecision } };
4040
private _qualifiedSegments: string[] = [];
4141

42-
constructor({ optimizely, userId, attributes, shouldIdentifyUser }: OptimizelyUserContextConfig) {
42+
constructor({ optimizely, userId, attributes, shouldIdentifyUser = true }: OptimizelyUserContextConfig) {
4343
this.optimizely = optimizely;
4444
this.userId = userId;
4545
this.attributes = { ...attributes } ?? {};
4646
this.forcedDecisionsMap = {};
4747

48-
if (shouldIdentifyUser == null || shouldIdentifyUser) {
48+
if (shouldIdentifyUser) {
4949
this.identifyUser();
5050
}
5151
}
@@ -54,7 +54,7 @@ export default class OptimizelyUserContext {
5454
* On user context instantiation, fire event to attempt to identify user to ODP.
5555
* Note: This fails if ODP is not enabled.
5656
*/
57-
identifyUser(): void {
57+
private identifyUser(): void {
5858
this.optimizely.identifyUser(this.userId);
5959
}
6060

‎packages/optimizely-sdk/lib/plugins/key_value_cache/browserAsyncStorageCache.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
import { ERROR_MESSAGES } from './../../utils/enums/index';
21
/**
3-
* Copyright 2022, Optimizely
2+
* Copyright 2022-2023, Optimizely
43
*
54
* Licensed under the Apache License, Version 2.0 (the "License");
65
* you may not use this file except in compliance with the License.
@@ -18,6 +17,7 @@ import { ERROR_MESSAGES } from './../../utils/enums/index';
1817
import { tryWithLocalStorage } from '../../utils/local_storage/tryLocalStorage';
1918
import PersistentKeyValueCache from './persistentKeyValueCache';
2019
import { getLogger } from '../../modules/logging';
20+
import { ERROR_MESSAGES } from './../../utils/enums/index';
2121

2222
export default class BrowserAsyncStorageCache implements PersistentKeyValueCache {
2323
logger = getLogger();

‎packages/optimizely-sdk/lib/plugins/odp_manager/index.browser.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616

1717
import { BROWSER_CLIENT_VERSION, ERROR_MESSAGES, JAVASCRIPT_CLIENT_ENGINE, ODP_USER_KEY } from '../../utils/enums';
18-
import { getLogger, LogHandler, LogLevel } from '../../modules/logging';
18+
import { getLogger, LoggerFacade, LogHandler, LogLevel } from '../../modules/logging';
1919
import { BrowserRequestHandler } from './../../utils/http_request_handler/browser_request_handler';
2020

2121
import BrowserAsyncStorageCache from '../key_value_cache/browserAsyncStorageCache';
@@ -28,6 +28,7 @@ import { OdpManager } from '../../core/odp/odp_manager';
2828
import { OdpEvent } from '../../core/odp/odp_event';
2929
import { OdpEventManager } from '../../core/odp/odp_event_manager';
3030
import { OdpSegmentManager } from '../../core/odp/odp_segment_manager';
31+
import { OdpOptions } from '../../shared_types';
3132

3233
interface BrowserOdpManagerConfig {
3334
disable: boolean;
@@ -121,4 +122,29 @@ export class BrowserOdpManager extends OdpManager {
121122

122123
super.sendEvent({ type, action, identifiers: identifiersWithVuid, data });
123124
}
125+
126+
public static createBrowserOdpManager({
127+
logger = getLogger(),
128+
odpOptions,
129+
}: {
130+
logger: LoggerFacade;
131+
odpOptions?: OdpOptions;
132+
}): BrowserOdpManager {
133+
if (!odpOptions) {
134+
return new BrowserOdpManager({ disable: false, logger });
135+
}
136+
137+
return new BrowserOdpManager({
138+
disable: odpOptions.disabled || false,
139+
segmentsCache:
140+
odpOptions?.segmentsCache ||
141+
new BrowserLRUCache<string, string[]>({
142+
maxSize: odpOptions.segmentsCacheSize,
143+
timeout: odpOptions.segmentsCacheTimeout,
144+
}),
145+
segmentManager: odpOptions.segmentManager,
146+
eventManager: odpOptions.eventManager,
147+
logger,
148+
});
149+
}
124150
}

‎packages/optimizely-sdk/lib/shared_types.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ import { NotificationCenter as NotificationCenterImpl } from './core/notificatio
2020
import { NOTIFICATION_TYPES, ODP_EVENT_ACTION } from './utils/enums';
2121

2222
import { OdpManager } from './core/odp/odp_manager';
23+
import { OdpSegmentManager } from './core/odp/odp_segment_manager';
24+
import { LRUCache } from './utils/lru_cache';
25+
import { OdpEventManager } from './core/odp/odp_event_manager';
2326

2427
export interface BucketerParams {
2528
experimentId: string;
@@ -76,6 +79,15 @@ export interface DatafileOptions {
7679
datafileAccessToken?: string;
7780
}
7881

82+
export interface OdpOptions {
83+
disabled?: boolean;
84+
segmentsCache?: LRUCache<string, string[]>;
85+
segmentsCacheSize?: number;
86+
segmentsCacheTimeout?: number;
87+
segmentManager?: OdpSegmentManager;
88+
eventManager?: OdpEventManager;
89+
}
90+
7991
export interface ListenerPayload {
8092
userId: string;
8193
attributes?: UserAttributes;
@@ -285,6 +297,12 @@ export interface OptimizelyVariable {
285297
value: string;
286298
}
287299

300+
export interface BrowserClient extends Client {
301+
getVuid(): string;
302+
// TODO: In the future, will add a function to allow overriding the VUID.
303+
createUserContext(userId?: string, attributes?: UserAttributes): OptimizelyUserContext | null;
304+
}
305+
288306
export interface Client {
289307
notificationCenter: NotificationCenter;
290308
createUserContext(userId: string, attributes?: UserAttributes): OptimizelyUserContext | null;
@@ -359,7 +377,7 @@ export interface Config extends ConfigLite {
359377
eventFlushInterval?: number; // Maximum time for an event to be enqueued
360378
eventMaxQueueSize?: number; // Maximum size for the event queue
361379
sdkKey?: string;
362-
odpManager?: OdpManager;
380+
odpOptions?: OdpOptions;
363381
}
364382

365383
/**

‎packages/optimizely-sdk/lib/utils/lru_cache/browser_lru_cache.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,20 @@
1414
* limitations under the License.
1515
*/
1616

17-
import LRUCache, { ISegmentsCacheConfig, LRUCacheConfig } from './lru_cache';
17+
import LRUCache, { ISegmentsCacheConfig } from './lru_cache';
18+
19+
export interface BrowserLRUCacheConfig {
20+
maxSize?: number;
21+
timeout?: number;
22+
}
1823

1924
export const BrowserLRUCacheConfig: ISegmentsCacheConfig = {
2025
DEFAULT_CAPACITY: 100,
2126
DEFAULT_TIMEOUT_SECS: 600,
2227
};
2328

2429
export class BrowserLRUCache<K, V> extends LRUCache<K, V> {
25-
constructor(config?: LRUCacheConfig) {
30+
constructor(config?: BrowserLRUCacheConfig) {
2631
super({
2732
maxSize: config?.maxSize || BrowserLRUCacheConfig.DEFAULT_CAPACITY,
2833
timeout: config?.timeout || BrowserLRUCacheConfig.DEFAULT_TIMEOUT_SECS * 1000,

0 commit comments

Comments
 (0)