Skip to content

[FSSDK-10711] Make use of VUID as an opt-in #950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 39 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
6b4f556
chore: update devcontainer config
mikechu-optimizely Oct 11, 2024
cc0c730
feat: add enableVuid to ODP options
mikechu-optimizely Oct 11, 2024
e1f2ee1
feat: add vuid removal function
mikechu-optimizely Oct 11, 2024
f4d5ed5
feat: use enableVuid option in BrowserOdpManager
mikechu-optimizely Oct 11, 2024
eed0d9b
doc: update warn when vuid is not enabled
mikechu-optimizely Oct 11, 2024
898c5f5
chore: oops still need jest until merge to `master`
mikechu-optimizely Oct 11, 2024
e15702a
chore: grrr remove vitest extensions for now
mikechu-optimizely Oct 11, 2024
763d387
test: fix jest configs in settings.json for VSCode
mikechu-optimizely Oct 11, 2024
005936b
test: cover explicit enablement of vuid
mikechu-optimizely Oct 11, 2024
8aee289
fix: code to pass tests
mikechu-optimizely Oct 11, 2024
9432c63
test: correct failing tests by enableVuid: true
mikechu-optimizely Oct 11, 2024
be04392
lint: fixes
mikechu-optimizely Oct 11, 2024
1196193
test: add coverage to VUID
mikechu-optimizely Oct 11, 2024
24e2b36
fix: PR requested updates
mikechu-optimizely Oct 14, 2024
9f1f6b8
refactor: WIP moving VUID out of ODP
mikechu-optimizely Oct 15, 2024
059db4c
refactor: remove remaining vuid from ODP Manager
mikechu-optimizely Oct 15, 2024
8cfb58a
refactor: VuidManager to a standard class from singleton
mikechu-optimizely Oct 15, 2024
dd1b434
refactor: ODP managers
mikechu-optimizely Oct 15, 2024
e84d353
refactor: WIP init VuidManager from opti client
mikechu-optimizely Oct 15, 2024
d85ec98
refactor: handle registerVuid
mikechu-optimizely Oct 15, 2024
a24a6af
refactor+doc: make readonly
mikechu-optimizely Oct 15, 2024
cbbf028
refactor: remove `this`
mikechu-optimizely Oct 15, 2024
83b997c
test: fixed + associated code corrections
mikechu-optimizely Oct 15, 2024
dad3a79
chore: include *.tests.js in jest
mikechu-optimizely Oct 15, 2024
26d962b
test: remove test that's no longer valid
mikechu-optimizely Oct 15, 2024
e970500
test: fix them so they run
mikechu-optimizely Oct 15, 2024
4200ff8
fix: PR requested changes
mikechu-optimizely Oct 15, 2024
6991743
chore: another correction to jest config
mikechu-optimizely Oct 15, 2024
ec4ae90
fix: PR requested change
mikechu-optimizely Oct 16, 2024
9b7fee0
test: skipping select tests for future PR.
mikechu-optimizely Oct 16, 2024
eb48fc8
revert: test changes
mikechu-optimizely Oct 16, 2024
640b67d
test: skip non-critical path tests
mikechu-optimizely Oct 16, 2024
67267f6
doc: Put TODOs on skipped test
mikechu-optimizely Oct 16, 2024
295394a
revert: auto-formatting & doc additions for ref readability
mikechu-optimizely Oct 16, 2024
f0d3c22
revert: more auto-format & clean ups (making it dirty again :-)
mikechu-optimizely Oct 16, 2024
339ecfb
revert: even more auto-formatting
mikechu-optimizely Oct 16, 2024
ccbdd4e
revert: last few auto-format put back ins
mikechu-optimizely Oct 16, 2024
0c6e57f
revert: missing import
mikechu-optimizely Oct 16, 2024
f382071
updates
raju-opti Oct 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"dbaeumer.vscode-eslint",
"eamodio.gitlens",
"esbenp.prettier-vscode",
"Gruntfuggly.todo-tree",
"github.vscode-github-actions",
"Orta.vscode-jest",
"ms-vscode.test-adapter-converter"
Expand Down
9 changes: 5 additions & 4 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"jest.rootPath": "/workspaces/javascript-sdk/packages/optimizely-sdk",
"jest.rootPath": "/workspaces/javascript-sdk",
"jest.jestCommandLine": "./node_modules/.bin/jest",
"jest.autoRevealOutput": "on-exec-error",
"editor.tabSize": 2
}
"jest.outputConfig": "test-results-based",
"editor.tabSize": 2,
"jest.runMode": "deferred"
}
10 changes: 9 additions & 1 deletion lib/core/odp/odp_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { OptimizelySegmentOption } from './optimizely_segment_option';
import { invalidOdpDataFound } from './odp_utils';
import { OdpEvent } from './odp_event';
import { resolvablePromise, ResolvablePromise } from '../../utils/promise/resolvablePromise';
import { OdpOptions } from '../../shared_types';

/**
* Manager for handling internal all business logic related to
Expand Down Expand Up @@ -97,22 +98,29 @@ export abstract class OdpManager implements IOdpManager {
*/
odpIntegrationConfig?: OdpIntegrationConfig;

/**
* ODP initialization options
*/
protected odpOptions?: OdpOptions;

// TODO: Consider accepting logger as a parameter and initializing it in constructor instead
constructor({
odpIntegrationConfig,
segmentManager,
eventManager,
logger,
odpOptions,
}: {
odpIntegrationConfig?: OdpIntegrationConfig;
segmentManager: IOdpSegmentManager;
eventManager: IOdpEventManager;
logger: LogHandler;
odpOptions?: OdpOptions;
}) {
this.segmentManager = segmentManager;
this.eventManager = eventManager;
this.logger = logger;

this.odpOptions = odpOptions;
this.configPromise = resolvablePromise();

const readinessDependencies: PromiseLike<unknown>[] = [this.configPromise];
Expand Down
39 changes: 19 additions & 20 deletions lib/optimizely/index.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,25 @@
/****************************************************************************
* Copyright 2020-2024, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
* You may obtain a copy of the License at *
* *
* https://www.apache.org/licenses/LICENSE-2.0 *
* *
* Unless required by applicable law or agreed to in writing, software *
* distributed under the License is distributed on an "AS IS" BASIS, *
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. *
* See the License for the specific language governing permissions and *
* limitations under the License. *
***************************************************************************/
/**
* Copyright 2020-2024, Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { LoggerFacade, ErrorHandler } from '../modules/logging';
import { sprintf, objectValues } from '../utils/fns';
import { NotificationCenter } from '../core/notification_center';
import { EventProcessor } from '../modules/event_processor';

import { IOdpManager } from '../core/odp/odp_manager';
import { OdpConfig } from '../core/odp/odp_config';
import { OdpEvent } from '../core/odp/odp_event';
import { OptimizelySegmentOption } from '../core/odp/optimizely_segment_option';

Expand Down Expand Up @@ -1328,12 +1327,12 @@ export default class Optimizely implements Client {
});
this.readyTimeouts = {};
return eventProcessorStoppedPromise.then(
function() {
function () {
return {
success: true,
};
},
function(err) {
function (err) {
return {
success: false,
reason: String(err),
Expand Down Expand Up @@ -1404,7 +1403,7 @@ export default class Optimizely implements Client {
});
};
const readyTimeout = setTimeout(onReadyTimeout, timeoutValue);
const onClose = function() {
const onClose = function () {
resolveTimeoutPromise({
success: false,
reason: 'Instance closed',
Expand Down Expand Up @@ -1765,7 +1764,7 @@ export default class Optimizely implements Client {
}

if (!this.odpManager.isVuidEnabled()) {
this.logger.log(LOG_LEVEL.WARNING, 'getVuid() unavailable for this platform', MODULE_NAME);
this.logger.log(LOG_LEVEL.WARNING, 'getVuid() unavailable for this platform or was not explicitly enabled.', MODULE_NAME);
return undefined;
}

Expand Down
27 changes: 11 additions & 16 deletions lib/plugins/odp_manager/index.browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,12 @@ import {
ODP_USER_KEY,
REQUEST_TIMEOUT_ODP_SEGMENTS_MS,
REQUEST_TIMEOUT_ODP_EVENTS_MS,
LOG_MESSAGES,
} from '../../utils/enums';
import { getLogger, LogHandler, LogLevel } from '../../modules/logging';
import { getLogger, LogHandler } from '../../modules/logging';

import { BrowserRequestHandler } from './../../utils/http_request_handler/browser_request_handler';

import BrowserAsyncStorageCache from '../key_value_cache/browserAsyncStorageCache';
import PersistentKeyValueCache from '../key_value_cache/persistentKeyValueCache';
import { BrowserLRUCache } from '../../utils/lru_cache';

import { VuidManager } from './../vuid_manager/index';
Expand All @@ -53,14 +51,14 @@ interface BrowserOdpManagerConfig {
// Client-side Browser Plugin for ODP Manager
export class BrowserOdpManager extends OdpManager {
static cache = new BrowserAsyncStorageCache();
vuidManager?: VuidManager;
vuid?: string;
vuid?: string = "";

constructor(options: {
odpIntegrationConfig?: OdpIntegrationConfig;
segmentManager: IOdpSegmentManager;
eventManager: IOdpEventManager;
logger: LogHandler;
odpOptions?: OdpOptions;
}) {
super(options);
}
Expand All @@ -73,13 +71,12 @@ export class BrowserOdpManager extends OdpManager {
clientEngine = clientEngine || JAVASCRIPT_CLIENT_ENGINE;
clientVersion = clientVersion || CLIENT_VERSION;

let odpConfig : OdpConfig | undefined = undefined;
let odpConfig: OdpConfig | undefined = undefined;
if (odpIntegrationConfig?.integrated) {
odpConfig = odpIntegrationConfig.odpConfig;
}

let customSegmentRequestHandler;

if (odpOptions?.segmentsRequestHandler) {
customSegmentRequestHandler = odpOptions.segmentsRequestHandler;
} else {
Expand All @@ -90,24 +87,22 @@ export class BrowserOdpManager extends OdpManager {
}

let segmentManager: IOdpSegmentManager;

if (odpOptions?.segmentManager) {
segmentManager = odpOptions.segmentManager;
} else {
segmentManager = new OdpSegmentManager(
odpOptions?.segmentsCache ||
new BrowserLRUCache<string, string[]>({
maxSize: odpOptions?.segmentsCacheSize,
timeout: odpOptions?.segmentsCacheTimeout,
}),
new BrowserLRUCache<string, string[]>({
maxSize: odpOptions?.segmentsCacheSize,
timeout: odpOptions?.segmentsCacheTimeout,
}),
new OdpSegmentApiManager(customSegmentRequestHandler, logger),
logger,
odpConfig
);
}

let customEventRequestHandler;

if (odpOptions?.eventRequestHandler) {
customEventRequestHandler = odpOptions.eventRequestHandler;
} else {
Expand All @@ -118,7 +113,6 @@ export class BrowserOdpManager extends OdpManager {
}

let eventManager: IOdpEventManager;

if (odpOptions?.eventManager) {
eventManager = odpOptions.eventManager;
} else {
Expand All @@ -140,6 +134,7 @@ export class BrowserOdpManager extends OdpManager {
segmentManager,
eventManager,
logger,
odpOptions,
});
}

Expand All @@ -148,7 +143,7 @@ export class BrowserOdpManager extends OdpManager {
* accesses or creates new VUID from Browser cache
*/
protected async initializeVuid(): Promise<void> {
const vuidManager = await VuidManager.instance(BrowserOdpManager.cache);
const vuidManager = await VuidManager.instance(BrowserOdpManager.cache, this.odpOptions);
this.vuid = vuidManager.vuid;
}

Expand Down Expand Up @@ -194,7 +189,7 @@ export class BrowserOdpManager extends OdpManager {
}

isVuidEnabled(): boolean {
return true;
return this.odpOptions?.enableVuid || false;
}

getVuid(): string | undefined {
Expand Down
16 changes: 10 additions & 6 deletions lib/plugins/vuid_manager/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2022-2023, Optimizely
* Copyright 2022-2024, Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -14,6 +14,7 @@
* limitations under the License.
*/

import { OdpOptions } from '../../shared_types';
import { uuid } from '../../utils/fns';
import PersistentKeyValueCache from '../key_value_cache/persistentKeyValueCache';

Expand Down Expand Up @@ -43,7 +44,7 @@ export class VuidManager implements IVuidManager {
* Current VUID value being used
* @private
*/
private _vuid: string;
private _vuid = '';

/**
* Get the current VUID value being used
Expand All @@ -52,9 +53,7 @@ export class VuidManager implements IVuidManager {
return this._vuid;
}

private constructor() {
this._vuid = '';
}
private constructor() { }

/**
* Instance of the VUID Manager
Expand All @@ -67,11 +66,16 @@ export class VuidManager implements IVuidManager {
* @param cache Caching mechanism to use for persisting the VUID outside working memory *
* @returns An instance of VuidManager
*/
static async instance(cache: PersistentKeyValueCache): Promise<VuidManager> {
static async instance(cache: PersistentKeyValueCache, options?: OdpOptions): Promise<VuidManager> {
if (!this._instance) {
this._instance = new VuidManager();
}

if (!options?.enableVuid) {
cache.remove(this._instance._keyForVuid);
return this._instance;
}

if (!this._instance._vuid) {
await this._instance.load(cache);
}
Expand Down
1 change: 1 addition & 0 deletions lib/shared_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export interface DatafileOptions {

export interface OdpOptions {
disabled?: boolean;
enableVuid?: boolean;
segmentsCache?: ICache<string, string[]>;
segmentsCacheSize?: number;
segmentsCacheTimeout?: number;
Expand Down
23 changes: 19 additions & 4 deletions tests/odpManager.browser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,32 @@ describe('OdpManager', () => {
resetCalls(mockSegmentManager);
});

const browserOdpManagerInstance = () =>
BrowserOdpManager.createInstance({
it('should have an empty string for VUID on BrowserOdpManager initialization', async () => {
const browserOdpManager = BrowserOdpManager.createInstance({
odpOptions: {
eventManager: fakeEventManager,
segmentManager: fakeSegmentManager,
// enableVuid: false, // Note: VUID is not explicitly enabled
},
});

it('should create VUID automatically on BrowserOdpManager initialization', async () => {
const browserOdpManager = browserOdpManagerInstance();
const vuidManager = await VuidManager.instance(BrowserOdpManager.cache);

expect(vuidManager.vuid).toBe("");
expect(browserOdpManager.vuid).toBe("")
});

it('should create VUID automatically on BrowserOdpManager initialization if VUID is explicitly enabled', async () => {
const browserOdpManager = BrowserOdpManager.createInstance({
odpOptions: {
eventManager: fakeEventManager,
segmentManager: fakeSegmentManager,
enableVuid: true,
},
});

const vuidManager = await VuidManager.instance(BrowserOdpManager.cache, {enableVuid: true});

expect(browserOdpManager.vuid).toBe(vuidManager.vuid);
});

Expand Down
29 changes: 25 additions & 4 deletions tests/vuidManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ describe('VuidManager', () => {

await cache.remove('optimizely-odp');

const manager1 = await VuidManager.instance(cache);
const manager1 = await VuidManager.instance(cache, {enableVuid: true});
const vuid1 = manager1.vuid;

const manager2 = await VuidManager.instance(cache);
const manager2 = await VuidManager.instance(cache, {enableVuid: true});
const vuid2 = manager2.vuid;

expect(vuid1).toStrictEqual(vuid2);
Expand All @@ -83,7 +83,7 @@ describe('VuidManager', () => {
it('should handle no valid optimizely-vuid in the cache', async () => {
when(mockCache.get(anyString())).thenResolve(undefined);

const manager = await VuidManager.instance(instance(mockCache)); // load() called initially
const manager = await VuidManager.instance(instance(mockCache), {enableVuid: true}); // load() called initially

verify(mockCache.get(anyString())).once();
verify(mockCache.set(anyString(), anything())).once();
Expand All @@ -93,10 +93,31 @@ describe('VuidManager', () => {
it('should create a new vuid if old VUID from cache is not valid', async () => {
when(mockCache.get(anyString())).thenResolve('vuid-not-valid');

const manager = await VuidManager.instance(instance(mockCache));
const manager = await VuidManager.instance(instance(mockCache), {enableVuid: true});

verify(mockCache.get(anyString())).once();
verify(mockCache.set(anyString(), anything())).once();
expect(VuidManager.isVuid(manager.vuid)).toBe(true);
});

it('should call remove when enableVuid is not specified', async () => {
const manager = await VuidManager.instance(instance(mockCache));

verify(mockCache.remove(anyString())).once();
expect(manager.vuid).toBe('');
});

it('should call remove when enableVuid is false', async () => {
const manager = await VuidManager.instance(instance(mockCache), {enableVuid: false});

verify(mockCache.remove(anyString())).once();
expect(manager.vuid).toBe('');
});

it('should never call remove when enableVuid is true', async () => {
const manager = await VuidManager.instance(instance(mockCache), {enableVuid: true});

verify(mockCache.remove(anyString())).never();
expect(VuidManager.isVuid(manager.vuid)).toBe(true);
});
});
Loading