Skip to content

Commit

Permalink
[FSSDK-11003] disposable project config manager
Browse files Browse the repository at this point in the history
  • Loading branch information
junaed-optimizely committed Jan 7, 2025
1 parent 51e8c1a commit 060cbc3
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 62 deletions.
1 change: 0 additions & 1 deletion lib/exception_messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export const DATAFILE_MANAGER_STOPPED = 'Datafile manager stopped before it coul
export const DATAFILE_MANAGER_FAILED_TO_START = 'Datafile manager failed to start';
export const FAILED_TO_FETCH_DATAFILE = 'Failed to fetch datafile';
export const FAILED_TO_STOP = 'Failed to stop';
export const YOU_MUST_PROVIDE_DATAFILE_IN_SSR = 'You must provide datafile in SSR';
export const YOU_MUST_PROVIDE_AT_LEAST_ONE_OF_SDKKEY_OR_DATAFILE = 'You must provide at least one of sdkKey or datafile';
export const RETRY_CANCELLED = 'Retry cancelled';
export const REQUEST_TIMEOUT = 'Request timeout';
Expand Down
1 change: 1 addition & 0 deletions lib/odp/odp_manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const getMockOdpEventManager = () => {
getState: vi.fn(),
updateConfig: vi.fn(),
sendEvent: vi.fn(),
makeDisposable: vi.fn(),
};
};

Expand Down
15 changes: 6 additions & 9 deletions lib/optimizely/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@
import { describe, it, expect, vi } from 'vitest';
import Optimizely from '.';
import { getMockProjectConfigManager } from '../tests/mock/mock_project_config_manager';
import * as logger from '../plugins/logger';
import * as jsonSchemaValidator from '../utils/json_schema_validator';
import { LOG_LEVEL } from '../common_exports';
import { createNotificationCenter } from '../notification_center';
import testData from '../tests/test_data';
import { getForwardingEventProcessor } from '../event_processor/forwarding_event_processor';
import { LoggerFacade } from '../modules/logging';
import { createProjectConfig } from '../project_config/project_config';
import { getMockLogger } from '../tests/mock/mock_logger';

Expand All @@ -39,12 +36,12 @@ describe('Optimizely', () => {

const notificationCenter = createNotificationCenter({ logger, errorHandler });

it('should pass ssr to the project config manager', () => {
it('should pass disposable option to the project config manager', () => {
const projectConfigManager = getMockProjectConfigManager({
initConfig: createProjectConfig(testData.getTestProjectConfig()),
});

vi.spyOn(projectConfigManager, 'setSsr');
vi.spyOn(projectConfigManager, 'makeDisposable');

const instance = new Optimizely({
clientEngine: 'node-sdk',
Expand All @@ -54,16 +51,16 @@ describe('Optimizely', () => {
logger,
notificationCenter,
eventProcessor,
isSsr: true,
disposable: true,
isValidInstance: true,
});

expect(projectConfigManager.setSsr).toHaveBeenCalledWith(true);
expect(projectConfigManager.makeDisposable).toHaveBeenCalled();
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
expect(instance.getProjectConfig()).toBe(projectConfigManager.config);
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
expect(projectConfigManager.isSsr).toBe(true);
expect(projectConfigManager.disposable).toBe(true);
});
});
5 changes: 4 additions & 1 deletion lib/optimizely/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,10 @@ export default class Optimizely implements Client {
this.updateOdpSettings();
});

this.projectConfigManager.setSsr(config.isSsr)
if(config.disposable) {
this.projectConfigManager.makeDisposable();
}

this.projectConfigManager.start();
const projectConfigManagerRunningPromise = this.projectConfigManager.onRunning();

Expand Down
19 changes: 19 additions & 0 deletions lib/project_config/polling_datafile_manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,25 @@ describe('PollingDatafileManager', () => {
expect(repeater.stop).toHaveBeenCalled();
});

it('stops repeater after successful initialization if disposable is true', async () => {
const repeater = getMockRepeater();
const requestHandler = getMockRequestHandler();
const mockResponse = getMockAbortableRequest(Promise.resolve({ statusCode: 200, body: '{"foo": "bar"}', headers: {} }));
requestHandler.makeRequest.mockReturnValueOnce(mockResponse);

const manager = new PollingDatafileManager({
repeater,
requestHandler,
sdkKey: 'keyThatExists',
});
manager.makeDisposable();
manager.start();
repeater.execute(0);

await expect(manager.onRunning()).resolves.not.toThrow();
expect(repeater.stop).toHaveBeenCalled();
});

it('saves the datafile in cache', async () => {
const repeater = getMockRepeater();
const requestHandler = getMockRequestHandler();
Expand Down
8 changes: 7 additions & 1 deletion lib/project_config/polling_datafile_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ export class PollingDatafileManager extends BaseService implements DatafileManag
return;
}

// If disposable, reset the retry count to 5
if(this.disposable) {
this.initRetryRemaining = 5;
}

super.start();
this.state = ServiceState.Starting;
this.setDatafileFromCacheIfAvailable();
Expand Down Expand Up @@ -162,7 +167,8 @@ export class PollingDatafileManager extends BaseService implements DatafileManag
if (datafile) {
this.handleDatafile(datafile);
// if autoUpdate is off, don't need to sync datafile any more
if (!this.autoUpdate) {
// if disposable, stop the repeater after the first successful fetch
if (!this.autoUpdate || this.disposable) {
this.repeater.stop();
}
}
Expand Down
21 changes: 0 additions & 21 deletions lib/project_config/project_config_manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,17 +165,6 @@ describe('ProjectConfigManagerImpl', () => {
await manager.onRunning();
expect(manager.getConfig()).toEqual(createProjectConfig(testData.getTestProjectConfig()));
});

it('should not start datafileManager if isSsr is true and return correct config', () => {
const datafileManager = getMockDatafileManager({});
vi.spyOn(datafileManager, 'start');
const manager = new ProjectConfigManagerImpl({ datafile: testData.getTestProjectConfig(), datafileManager });
manager.setSsr(true);
manager.start();

expect(manager.getConfig()).toEqual(createProjectConfig(testData.getTestProjectConfig()));
expect(datafileManager.start).not.toHaveBeenCalled();
});
});

describe('when datafile is invalid', () => {
Expand Down Expand Up @@ -409,16 +398,6 @@ describe('ProjectConfigManagerImpl', () => {
expect(logger.error).toHaveBeenCalled();
});

it('should reject onRunning() and log error if isSsr is true and datafile is not provided', async () =>{
const logger = getMockLogger();
const manager = new ProjectConfigManagerImpl({ logger, datafileManager: getMockDatafileManager({})});
manager.setSsr(true);
manager.start();

await expect(manager.onRunning()).rejects.toThrow();
expect(logger.error).toHaveBeenCalled();
});

it('should reject onRunning() and log error if the datafile version is not supported', async () => {
const logger = getMockLogger();
const datafile = testData.getUnsupportedVersionConfig();
Expand Down
27 changes: 5 additions & 22 deletions lib/project_config/project_config_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {
DATAFILE_MANAGER_FAILED_TO_START,
DATAFILE_MANAGER_STOPPED,
YOU_MUST_PROVIDE_AT_LEAST_ONE_OF_SDKKEY_OR_DATAFILE,
YOU_MUST_PROVIDE_DATAFILE_IN_SSR,
} from '../exception_messages';

interface ProjectConfigManagerConfig {
Expand All @@ -40,7 +39,6 @@ interface ProjectConfigManagerConfig {

export interface ProjectConfigManager extends Service {
setLogger(logger: LoggerFacade): void;
setSsr(isSsr?: boolean): void;
getConfig(): ProjectConfig | undefined;
getOptimizelyConfig(): OptimizelyConfig | undefined;
onUpdate(listener: Consumer<ProjectConfig>): Fn;
Expand All @@ -60,7 +58,6 @@ export class ProjectConfigManagerImpl extends BaseService implements ProjectConf
public jsonSchemaValidator?: Transformer<unknown, boolean>;
public datafileManager?: DatafileManager;
private eventEmitter: EventEmitter<{ update: ProjectConfig }> = new EventEmitter();
private isSsr = false;

constructor(config: ProjectConfigManagerConfig) {
super();
Expand All @@ -77,24 +74,19 @@ export class ProjectConfigManagerImpl extends BaseService implements ProjectConf

this.state = ServiceState.Starting;

if(this.isSsr) {
// If isSsr is true, we don't need to poll for datafile updates
this.datafileManager = undefined
}

if (!this.datafile && !this.datafileManager) {
const errorMessage = this.isSsr
? YOU_MUST_PROVIDE_DATAFILE_IN_SSR
: YOU_MUST_PROVIDE_AT_LEAST_ONE_OF_SDKKEY_OR_DATAFILE;

this.handleInitError(new Error(errorMessage));
this.handleInitError(new Error(YOU_MUST_PROVIDE_AT_LEAST_ONE_OF_SDKKEY_OR_DATAFILE));
return;
}

if (this.datafile) {
this.handleNewDatafile(this.datafile, true);
}

if(this.disposable) {
this.datafileManager?.makeDisposable();
}

this.datafileManager?.start();

// This handles the case where the datafile manager starts successfully. The
Expand Down Expand Up @@ -227,13 +219,4 @@ export class ProjectConfigManagerImpl extends BaseService implements ProjectConf
this.stopPromise.reject(err);
});
}

/**
* Set the isSsr flag to indicate if the project config manager is being used in a server side rendering environment
* @param {Boolean} isSsr
* @returns {void}
*/
setSsr(isSsr: boolean): void {
this.isSsr = isSsr;
}
}
7 changes: 6 additions & 1 deletion lib/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export interface Service {
// either by failing to start or stop.
// It will resolve if the service is stopped successfully.
onTerminated(): Promise<void>;
makeDisposable(): void;
}

export abstract class BaseService implements Service {
Expand All @@ -59,7 +60,7 @@ export abstract class BaseService implements Service {
protected stopPromise: ResolvablePromise<void>;
protected logger?: LoggerFacade;
protected startupLogs: StartupLog[];

protected disposable = false;
constructor(startupLogs: StartupLog[] = []) {
this.state = ServiceState.New;
this.startPromise = resolvablePromise();
Expand All @@ -71,6 +72,10 @@ export abstract class BaseService implements Service {
this.stopPromise.promise.catch(() => {});
}

makeDisposable(): void {
this.disposable = true;
}

setLogger(logger: LoggerFacade): void {
this.logger = logger;
}
Expand Down
4 changes: 2 additions & 2 deletions lib/shared_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,10 +263,10 @@ export interface OptimizelyOptions {
sdkKey?: string;
userProfileService?: UserProfileService | null;
defaultDecideOptions?: OptimizelyDecideOption[];
isSsr?:boolean;
odpManager?: OdpManager;
notificationCenter: DefaultNotificationCenter;
vuidManager?: VuidManager
disposable?:boolean;
}

/**
Expand Down Expand Up @@ -384,9 +384,9 @@ export interface Config {
defaultDecideOptions?: OptimizelyDecideOption[];
clientEngine?: string;
clientVersion?: string;
isSsr?: boolean;
odpManager?: OdpManager;
vuidManager?: VuidManager;
disposable?:boolean;
}

export type OptimizelyExperimentsMap = {
Expand Down
8 changes: 4 additions & 4 deletions lib/tests/mock/mock_project_config_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ type MockOpt = {

export const getMockProjectConfigManager = (opt: MockOpt = {}): ProjectConfigManager => {
return {
isSsr: false,
disposable: false,
config: opt.initConfig,
start: () => {},
setSsr: function(isSsr:boolean) {
this.isSsr = isSsr;
makeDisposable(){
this.disposable = true;
},
start: () => {},
onRunning: () => opt.onRunning || Promise.resolve(),
stop: () => {},
onTerminated: () => opt.onTerminated || Promise.resolve(),
Expand Down

0 comments on commit 060cbc3

Please sign in to comment.