Skip to content

Commit b94a8e5

Browse files
committed
[FSSDK-11113] make Optimizely class instance of Service interface
1 parent 446d8f2 commit b94a8e5

File tree

6 files changed

+70
-78
lines changed

6 files changed

+70
-78
lines changed

lib/event_processor/forwarding_event_processor.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { buildLogEvent } from './event_builder/log_event';
2323
import { BaseService, ServiceState } from '../service';
2424
import { EventEmitter } from '../utils/event_emitter/event_emitter';
2525
import { Consumer, Fn } from '../utils/type';
26-
import { SERVICE_STOPPED_BEFORE_IT_WAS_STARTED } from 'error_message';
26+
import { SERVICE_STOPPED_BEFORE_RUNNING } from 'error_message';
2727
import { OptimizelyError } from '../error/optimizly_error';
2828
class ForwardingEventProcessor extends BaseService implements EventProcessor {
2929
private dispatcher: EventDispatcher;
@@ -56,7 +56,7 @@ class ForwardingEventProcessor extends BaseService implements EventProcessor {
5656
}
5757

5858
if (this.isNew()) {
59-
this.startPromise.reject(new OptimizelyError(SERVICE_STOPPED_BEFORE_IT_WAS_STARTED));
59+
this.startPromise.reject(new OptimizelyError(SERVICE_STOPPED_BEFORE_RUNNING));
6060
}
6161

6262
this.state = ServiceState.Terminated;

lib/message/error_message.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export const DATAFILE_MANAGER_STOPPED = 'Datafile manager stopped before it coul
9595
export const FAILED_TO_FETCH_DATAFILE = 'Failed to fetch datafile';
9696
export const NO_SDKKEY_OR_DATAFILE = 'At least one of sdkKey or datafile must be provided';
9797
export const RETRY_CANCELLED = 'Retry cancelled';
98-
export const SERVICE_STOPPED_BEFORE_IT_WAS_STARTED = 'Service stopped before it was started';
98+
export const SERVICE_STOPPED_BEFORE_RUNNING = 'Service stopped before running';
9999
export const ONLY_POST_REQUESTS_ARE_SUPPORTED = 'Only POST requests are supported';
100100
export const SEND_BEACON_FAILED = 'sendBeacon failed';
101101
export const FAILED_TO_DISPATCH_EVENTS = 'Failed to dispatch events'

lib/optimizely/index.tests.js

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,26 +37,15 @@ import {
3737
FEATURE_NOT_ENABLED_FOR_USER,
3838
INVALID_CLIENT_ENGINE,
3939
INVALID_DEFAULT_DECIDE_OPTIONS,
40-
INVALID_OBJECT,
4140
NOT_ACTIVATING_USER,
42-
USER_HAS_NO_FORCED_VARIATION,
43-
USER_HAS_NO_FORCED_VARIATION_FOR_EXPERIMENT,
44-
USER_MAPPED_TO_FORCED_VARIATION,
45-
USER_RECEIVED_DEFAULT_VARIABLE_VALUE,
4641
VALID_USER_PROFILE_SERVICE,
47-
VARIATION_REMOVED_FOR_USER,
4842
} from 'log_message';
4943
import {
50-
EXPERIMENT_KEY_NOT_IN_DATAFILE,
51-
INVALID_ATTRIBUTES,
5244
NOT_TRACKING_USER,
5345
EVENT_KEY_NOT_FOUND,
5446
INVALID_EXPERIMENT_KEY,
55-
INVALID_INPUT_FORMAT,
56-
NO_VARIATION_FOR_EXPERIMENT_KEY,
57-
USER_NOT_IN_FORCED_VARIATION,
58-
INSTANCE_CLOSED,
5947
ONREADY_TIMEOUT,
48+
SERVICE_STOPPED_BEFORE_RUNNING
6049
} from 'error_message';
6150

6251
import {
@@ -77,6 +66,8 @@ import {
7766
} from '../core/decision_service';
7867

7968
import { USER_BUCKETED_INTO_EXPERIMENT_IN_GROUP } from '../core/bucketer';
69+
import { read } from 'fs';
70+
import { resolvablePromise } from '../utils/promise/resolvablePromise';
8071

8172
var LOG_LEVEL = enums.LOG_LEVEL;
8273
var DECISION_SOURCES = enums.DECISION_SOURCES;
@@ -9463,7 +9454,7 @@ describe('lib/optimizely', function() {
94639454
var readyPromise = optlyInstance.onReady();
94649455
clock.tick(300001);
94659456
return readyPromise.then(() => {
9466-
return Promise.reject(new Error(PROMISE_SHOULD_NOT_HAVE_RESOLVED));
9457+
return Promise.reject(new Error('PROMISE_SHOULD_NOT_HAVE_RESOLVED'));
94679458
}, (err) => {
94689459
assert.equal(err.baseMessage, ONREADY_TIMEOUT);
94699460
assert.deepEqual(err.params, [ 30000 ]);
@@ -9485,18 +9476,25 @@ describe('lib/optimizely', function() {
94859476
eventProcessor,
94869477
});
94879478
var readyPromise = optlyInstance.onReady({ timeout: 100 });
9479+
94889480
optlyInstance.close();
9481+
94899482
return readyPromise.then(() => {
9490-
return Promise.reject(new Error(PROMISE_SHOULD_NOT_HAVE_RESOLVED));
9483+
return Promise.reject(new Error('PROMISE_SHOULD_NOT_HAVE_RESOLVED'));
94919484
}, (err) => {
9492-
assert.equal(err.baseMessage, INSTANCE_CLOSED);
9485+
assert.equal(err.baseMessage, SERVICE_STOPPED_BEFORE_RUNNING);
94939486
});
94949487
});
94959488

94969489
it('can be called several times with different timeout values and the returned promises behave correctly', function() {
9490+
const onRunning = resolvablePromise();
9491+
94979492
optlyInstance = new Optimizely({
94989493
clientEngine: 'node-sdk',
9499-
projectConfigManager: getMockProjectConfigManager(),
9494+
projectConfigManager: getMockProjectConfigManager({
9495+
onRunning: onRunning.promise,
9496+
}),
9497+
95009498
eventProcessor,
95019499
jsonSchemaValidator: jsonSchemaValidator,
95029500
logger: createdLogger,
@@ -9510,16 +9508,16 @@ describe('lib/optimizely', function() {
95109508
var readyPromise3 = optlyInstance.onReady({ timeout: 300 });
95119509
clock.tick(101);
95129510
return readyPromise1
9513-
.then(function() {
9511+
.catch(function() {
95149512
clock.tick(100);
95159513
return readyPromise2;
95169514
})
9517-
.then(function() {
9515+
.catch(function() {
95189516
// readyPromise3 has not resolved yet because only 201 ms have elapsed.
95199517
// Calling close on the instance should resolve readyPromise3
9520-
optlyInstance.close();
9518+
optlyInstance.close().catch(() => {});
95219519
return readyPromise3;
9522-
});
9520+
}).catch(() => {});
95239521
});
95249522

95259523
it('clears the timeout when the project config manager ready promise fulfills', function() {

lib/optimizely/index.ts

Lines changed: 47 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ import {
7777
NOT_TRACKING_USER,
7878
VARIABLE_REQUESTED_WITH_WRONG_TYPE,
7979
ONREADY_TIMEOUT,
80-
INSTANCE_CLOSED
80+
INSTANCE_CLOSED,
81+
SERVICE_STOPPED_BEFORE_RUNNING
8182
} from 'error_message';
8283

8384
import {
@@ -133,10 +134,8 @@ export type OptimizelyOptions = {
133134
}
134135

135136
export default class Optimizely extends BaseService implements Client {
136-
// readyTimeout is specified as any to make this work in both browser & Node
137-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
138-
private readyTimeouts: { [key: string]: { readyTimeout: any; onClose: () => void } };
139-
private nextReadyTimeoutId: number;
137+
private cleanupTasks: Map<number, Fn> = new Map();
138+
private nextCleanupTaskId = 0;
140139
private clientEngine: string;
141140
private clientVersion: string;
142141
private errorNotifier?: ErrorNotifier;
@@ -233,22 +232,25 @@ export default class Optimizely extends BaseService implements Client {
233232

234233
this.notificationCenter = createNotificationCenter({ logger: this.logger, errorNotifier: this.errorNotifier });
235234

236-
this.readyTimeouts = {};
237-
this.nextReadyTimeoutId = 0;
238-
239235
this.start();
240236
}
241237

242238
start(): void {
239+
if (!this.isNew()) {
240+
return;
241+
}
242+
243243
super.start();
244244

245245
this.state = ServiceState.Starting;
246246
this.projectConfigManager.start();
247247
this.eventProcessor?.start();
248248
this.odpManager?.start();
249249

250+
const configManOnRunning = this.projectConfigManager.onRunning();
251+
250252
Promise.all([
251-
this.projectConfigManager.onRunning(),
253+
configManOnRunning,
252254
this.eventProcessor ? this.eventProcessor.onRunning() : Promise.resolve(),
253255
this.odpManager ? this.odpManager.onRunning() : Promise.resolve(),
254256
this.vuidManager ? this.vuidManager.initialize() : Promise.resolve(),
@@ -260,11 +262,13 @@ export default class Optimizely extends BaseService implements Client {
260262
if (vuid) {
261263
this.odpManager?.setVuid(vuid);
262264
}
265+
}, (err) => {
266+
this.state = ServiceState.Failed;
267+
this.errorReporter.report(err);
268+
this.startPromise.reject(err);
263269
});
264270
}
265271

266-
267-
268272
/**
269273
* Returns the project configuration retrieved from projectConfigManager
270274
* @return {projectConfig.ProjectConfig}
@@ -1233,24 +1237,30 @@ export default class Optimizely extends BaseService implements Client {
12331237
*
12341238
* @return {Promise}
12351239
*/
1236-
close(): Promise<unknown> {
1240+
close(): Promise<unknown> {
12371241
this.stop();
12381242
return this.onTerminated();
12391243
}
12401244

12411245
stop(): void {
1246+
if (this.isDone()) {
1247+
return;
1248+
}
1249+
1250+
this.startPromise.promise.then(() => {}, (err) => {});
1251+
1252+
if (!this.isRunning()) {
1253+
this.startPromise.reject(new OptimizelyError(SERVICE_STOPPED_BEFORE_RUNNING));
1254+
}
1255+
12421256
this.state = ServiceState.Stopping;
12431257

12441258
this.projectConfigManager.stop();
12451259
this.eventProcessor?.stop();
12461260
this.odpManager?.stop();
12471261
this.notificationCenter.clearAllNotificationListeners();
12481262

1249-
Object.keys(this.readyTimeouts).forEach((readyTimeoutId: string) => {
1250-
const readyTimeoutRecord = this.readyTimeouts[readyTimeoutId];
1251-
clearTimeout(readyTimeoutRecord.readyTimeout);
1252-
readyTimeoutRecord.onClose();
1253-
});
1263+
this.cleanupTasks.forEach((onClose) => onClose());
12541264

12551265
Promise.all([
12561266
this.projectConfigManager.onTerminated(),
@@ -1305,30 +1315,26 @@ export default class Optimizely extends BaseService implements Client {
13051315
}
13061316

13071317
const timeoutPromise = resolvablePromise();
1318+
timeoutPromise.promise.catch(() => {});
13081319

1309-
const timeoutId = this.nextReadyTimeoutId++;
1320+
const cleanupTaskId = this.nextCleanupTaskId++;
13101321

13111322
const onReadyTimeout = () => {
1312-
delete this.readyTimeouts[timeoutId];
1323+
this.cleanupTasks.delete(cleanupTaskId);
13131324
timeoutPromise.reject(new OptimizelyError(ONREADY_TIMEOUT, timeoutValue));
13141325
};
13151326

13161327
const readyTimeout = setTimeout(onReadyTimeout, timeoutValue);
1317-
const onClose = function() {
1318-
timeoutPromise.reject(new OptimizelyError(INSTANCE_CLOSED));
1319-
};
1320-
1321-
this.readyTimeouts[timeoutId] = {
1322-
readyTimeout: readyTimeout,
1323-
onClose: onClose,
1324-
};
1325-
1326-
this.onRunning().then(() => {
1328+
1329+
this.cleanupTasks.set(cleanupTaskId, () => {
13271330
clearTimeout(readyTimeout);
1328-
delete this.readyTimeouts[timeoutId];
1331+
timeoutPromise.reject(new OptimizelyError(INSTANCE_CLOSED));
13291332
});
13301333

1331-
return Promise.race([this.onRunning(), timeoutPromise]);
1334+
return Promise.race([this.onRunning().then(() => {
1335+
clearTimeout(readyTimeout);
1336+
this.cleanupTasks.delete(cleanupTaskId);
1337+
}), timeoutPromise]);
13321338
}
13331339

13341340
//============ decide ============//
@@ -1351,12 +1357,19 @@ export default class Optimizely extends BaseService implements Client {
13511357
return null;
13521358
}
13531359

1354-
return new OptimizelyUserContext({
1360+
const userContext = new OptimizelyUserContext({
13551361
optimizely: this,
13561362
userId: userIdentifier,
13571363
attributes,
1358-
shouldIdentifyUser: true,
13591364
});
1365+
1366+
this.onRunning().then(() => {
1367+
if (this.odpManager && this.isOdpIntegrated()) {
1368+
this.odpManager.identifyUser(userIdentifier);
1369+
}
1370+
}).catch(() => {});
1371+
1372+
return userContext;
13601373
}
13611374

13621375
/**
@@ -1375,7 +1388,6 @@ export default class Optimizely extends BaseService implements Client {
13751388
optimizely: this,
13761389
userId,
13771390
attributes,
1378-
shouldIdentifyUser: false,
13791391
});
13801392
}
13811393

@@ -1662,9 +1674,7 @@ export default class Optimizely extends BaseService implements Client {
16621674
* @param {string} userId
16631675
*/
16641676
public identifyUser(userId: string): void {
1665-
if (this.odpManager && this.isOdpIntegrated()) {
1666-
this.odpManager.identifyUser(userId);
1667-
}
1677+
16681678
}
16691679

16701680
/**

lib/optimizely_user_context/index.ts

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ interface OptimizelyUserContextConfig {
3030
optimizely: Optimizely;
3131
userId: string;
3232
attributes?: UserAttributes;
33-
shouldIdentifyUser?: boolean;
3433
}
3534

3635
export interface IOptimizelyUserContext {
@@ -57,25 +56,11 @@ export default class OptimizelyUserContext implements IOptimizelyUserContext {
5756
private forcedDecisionsMap: { [key: string]: { [key: string]: OptimizelyForcedDecision } };
5857
private _qualifiedSegments: string[] | null = null;
5958

60-
constructor({ optimizely, userId, attributes, shouldIdentifyUser = true }: OptimizelyUserContextConfig) {
59+
constructor({ optimizely, userId, attributes }: OptimizelyUserContextConfig) {
6160
this.optimizely = optimizely;
6261
this.userId = userId;
6362
this.attributes = { ...attributes } ?? {};
6463
this.forcedDecisionsMap = {};
65-
66-
if (shouldIdentifyUser) {
67-
this.optimizely.onReady().then(() => {
68-
this.identifyUser();
69-
}).catch(() => {});
70-
}
71-
}
72-
73-
/**
74-
* On user context instantiation, fire event to attempt to identify user to ODP.
75-
* Note: This fails if ODP is not enabled.
76-
*/
77-
private identifyUser(): void {
78-
this.optimizely.identifyUser(this.userId);
7964
}
8065

8166
/**
@@ -235,7 +220,6 @@ export default class OptimizelyUserContext implements IOptimizelyUserContext {
235220

236221
private cloneUserContext(): OptimizelyUserContext {
237222
const userContext = new OptimizelyUserContext({
238-
shouldIdentifyUser: false,
239223
optimizely: this.getOptimizely(),
240224
userId: this.getUserId(),
241225
attributes: this.getAttributes(),

lib/service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ export abstract class BaseService implements Service {
6666
this.startPromise = resolvablePromise();
6767
this.stopPromise = resolvablePromise();
6868
this.startupLogs = startupLogs;
69-
69+
7070
// avoid unhandled promise rejection
7171
this.startPromise.promise.catch(() => {});
7272
this.stopPromise.promise.catch(() => {});

0 commit comments

Comments
 (0)