diff --git a/lib/event_processor/batch_event_processor.spec.ts b/lib/event_processor/batch_event_processor.spec.ts index 30d8d1bac..4e955e364 100644 --- a/lib/event_processor/batch_event_processor.spec.ts +++ b/lib/event_processor/batch_event_processor.spec.ts @@ -94,7 +94,7 @@ describe('QueueingEventProcessor', async () => { await expect(processor.onRunning()).resolves.not.toThrow(); }); - it('should start dispatchRepeater and failedEventRepeater', () => { + it('should start failedEventRepeater', () => { const eventDispatcher = getMockDispatcher(); const dispatchRepeater = getMockRepeater(); const failedEventRepeater = getMockRepeater(); @@ -107,7 +107,6 @@ describe('QueueingEventProcessor', async () => { }); processor.start(); - expect(dispatchRepeater.start).toHaveBeenCalledOnce(); expect(failedEventRepeater.start).toHaveBeenCalledOnce(); }); @@ -167,7 +166,7 @@ describe('QueueingEventProcessor', async () => { processor.start(); await processor.onRunning(); - for(let i = 0; i < 100; i++) { + for(let i = 0; i < 99; i++) { const event = createImpressionEvent(`id-${i}`); await processor.process(event); } @@ -175,6 +174,25 @@ describe('QueueingEventProcessor', async () => { expect(eventDispatcher.dispatchEvent).toHaveBeenCalledTimes(0); }); + it('should start the dispatchRepeater if it is not running', async () => { + const eventDispatcher = getMockDispatcher(); + const dispatchRepeater = getMockRepeater(); + + const processor = new BatchEventProcessor({ + eventDispatcher, + dispatchRepeater, + batchSize: 100, + }); + + processor.start(); + await processor.onRunning(); + + const event = createImpressionEvent('id-1'); + await processor.process(event); + + expect(dispatchRepeater.start).toHaveBeenCalledOnce(); + }); + it('should dispatch events if queue is full and clear queue', async () => { const eventDispatcher = getMockDispatcher(); const mockDispatch: MockInstance = eventDispatcher.dispatchEvent; @@ -190,7 +208,7 @@ describe('QueueingEventProcessor', async () => { await processor.onRunning(); let events: ProcessableEvent[] = []; - for(let i = 0; i < 100; i++) { + for(let i = 0; i < 99; i++){ const event = createImpressionEvent(`id-${i}`); events.push(event); await processor.process(event); @@ -198,14 +216,16 @@ describe('QueueingEventProcessor', async () => { expect(eventDispatcher.dispatchEvent).toHaveBeenCalledTimes(0); - let event = createImpressionEvent('id-100'); + let event = createImpressionEvent('id-99'); + events.push(event); await processor.process(event); - + expect(eventDispatcher.dispatchEvent).toHaveBeenCalledTimes(1); expect(eventDispatcher.dispatchEvent.mock.calls[0][0]).toEqual(buildLogEvent(events)); - events = [event]; - for(let i = 101; i < 200; i++) { + events = []; + + for(let i = 100; i < 199; i++) { const event = createImpressionEvent(`id-${i}`); events.push(event); await processor.process(event); @@ -213,7 +233,8 @@ describe('QueueingEventProcessor', async () => { expect(eventDispatcher.dispatchEvent).toHaveBeenCalledTimes(1); - event = createImpressionEvent('id-200'); + event = createImpressionEvent('id-199'); + events.push(event); await processor.process(event); expect(eventDispatcher.dispatchEvent).toHaveBeenCalledTimes(2); @@ -257,6 +278,40 @@ describe('QueueingEventProcessor', async () => { expect(eventDispatcher.dispatchEvent.mock.calls[1][0]).toEqual(buildLogEvent([newEvent])); }); + it('should flush queue immediately regardless of batchSize, if event processor is disposable', async () => { + const eventDispatcher = getMockDispatcher(); + const mockDispatch: MockInstance = eventDispatcher.dispatchEvent; + mockDispatch.mockResolvedValue({}); + + const dispatchRepeater = getMockRepeater(); + const failedEventRepeater = getMockRepeater(); + + const processor = new BatchEventProcessor({ + eventDispatcher, + dispatchRepeater, + failedEventRepeater, + batchSize: 100, + }); + + processor.makeDisposable(); + processor.start(); + await processor.onRunning(); + + const events: ProcessableEvent[] = []; + const event = createImpressionEvent('id-1'); + events.push(event); + await processor.process(event); + + expect(eventDispatcher.dispatchEvent).toHaveBeenCalledTimes(1); + expect(eventDispatcher.dispatchEvent.mock.calls[0][0]).toEqual(buildLogEvent(events)); + expect(dispatchRepeater.reset).toHaveBeenCalledTimes(1); + expect(dispatchRepeater.start).not.toHaveBeenCalled(); + expect(failedEventRepeater.start).not.toHaveBeenCalled(); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + expect(processor.retryConfig?.maxRetries).toEqual(5); + }); + it('should store the event in the eventStore with increasing ids', async () => { const eventDispatcher = getMockDispatcher(); const eventStore = getMockSyncCache(); diff --git a/lib/event_processor/batch_event_processor.ts b/lib/event_processor/batch_event_processor.ts index 76e737a9d..a487f6cdf 100644 --- a/lib/event_processor/batch_event_processor.ts +++ b/lib/event_processor/batch_event_processor.ts @@ -30,6 +30,9 @@ import { areEventContextsEqual } from "./event_builder/user_event"; import { EVENT_PROCESSOR_STOPPED, FAILED_TO_DISPATCH_EVENTS, FAILED_TO_DISPATCH_EVENTS_WITH_ARG } from "../exception_messages"; import { sprintf } from "../utils/fns"; +export const DEFAULT_MIN_BACKOFF = 1000; +export const DEFAULT_MAX_BACKOFF = 32000; + export type EventWithId = { id: string; event: ProcessableEvent; @@ -209,7 +212,8 @@ export class BatchEventProcessor extends BaseService implements EventProcessor { if (!batch) { return; } - + + this.dispatchRepeater.reset(); this.dispatchBatch(batch, closing); } @@ -218,10 +222,6 @@ export class BatchEventProcessor extends BaseService implements EventProcessor { return Promise.reject('Event processor is not running'); } - if (this.eventQueue.length == this.batchSize) { - this.flush(); - } - const eventWithId = { id: this.idGenerator.getId(), event: event, @@ -232,29 +232,50 @@ export class BatchEventProcessor extends BaseService implements EventProcessor { if (this.eventQueue.length > 0 && !areEventContextsEqual(this.eventQueue[0].event, event)) { this.flush(); } - this.eventQueue.push(eventWithId); + + this.eventQueue.push(eventWithId); + + if (this.eventQueue.length == this.batchSize) { + this.flush(); + } else if (!this.dispatchRepeater.isRunning()) { + this.dispatchRepeater.start(); + } + } start(): void { if (!this.isNew()) { return; } + super.start(); this.state = ServiceState.Running; - this.dispatchRepeater.start(); - this.failedEventRepeater?.start(); + + if(!this.disposable) { + this.failedEventRepeater?.start(); + } this.retryFailedEvents(); this.startPromise.resolve(); } + makeDisposable(): void { + super.makeDisposable(); + this.batchSize = 1; + this.retryConfig = { + maxRetries: Math.min(this.retryConfig?.maxRetries ?? 5, 5), + backoffProvider: + this.retryConfig?.backoffProvider || + (() => new ExponentialBackoff(DEFAULT_MIN_BACKOFF, DEFAULT_MAX_BACKOFF, 500)), + } + } + stop(): void { if (this.isDone()) { return; } if (this.isNew()) { - // TOOD: replace message with imported constants this.startPromise.reject(new Error(EVENT_PROCESSOR_STOPPED)); } diff --git a/lib/event_processor/event_processor_factory.spec.ts b/lib/event_processor/event_processor_factory.spec.ts index 2f3d45408..938483f4f 100644 --- a/lib/event_processor/event_processor_factory.spec.ts +++ b/lib/event_processor/event_processor_factory.spec.ts @@ -15,8 +15,8 @@ */ import { describe, it, expect, beforeEach, vi, MockInstance } from 'vitest'; -import { DEFAULT_EVENT_BATCH_SIZE, DEFAULT_EVENT_FLUSH_INTERVAL, DEFAULT_MAX_BACKOFF, DEFAULT_MIN_BACKOFF, getBatchEventProcessor } from './event_processor_factory'; -import { BatchEventProcessor, BatchEventProcessorConfig, EventWithId } from './batch_event_processor'; +import { DEFAULT_EVENT_BATCH_SIZE, DEFAULT_EVENT_FLUSH_INTERVAL, getBatchEventProcessor } from './event_processor_factory'; +import { BatchEventProcessor, BatchEventProcessorConfig, EventWithId,DEFAULT_MAX_BACKOFF, DEFAULT_MIN_BACKOFF } from './batch_event_processor'; import { ExponentialBackoff, IntervalRepeater } from '../utils/repeater/repeater'; import { getMockSyncCache } from '../tests/mock/mock_cache'; import { LogLevel } from '../modules/logging'; diff --git a/lib/event_processor/event_processor_factory.ts b/lib/event_processor/event_processor_factory.ts index 8221e7dab..f64143cf8 100644 --- a/lib/event_processor/event_processor_factory.ts +++ b/lib/event_processor/event_processor_factory.ts @@ -19,14 +19,12 @@ import { StartupLog } from "../service"; import { ExponentialBackoff, IntervalRepeater } from "../utils/repeater/repeater"; import { EventDispatcher } from "./event_dispatcher/event_dispatcher"; import { EventProcessor } from "./event_processor"; -import { BatchEventProcessor, EventWithId, RetryConfig } from "./batch_event_processor"; +import { BatchEventProcessor, DEFAULT_MAX_BACKOFF, DEFAULT_MIN_BACKOFF, EventWithId, RetryConfig } from "./batch_event_processor"; import { AsyncPrefixCache, Cache, SyncPrefixCache } from "../utils/cache/cache"; export const DEFAULT_EVENT_BATCH_SIZE = 10; export const DEFAULT_EVENT_FLUSH_INTERVAL = 1000; export const DEFAULT_EVENT_MAX_QUEUE_SIZE = 10000; -export const DEFAULT_MIN_BACKOFF = 1000; -export const DEFAULT_MAX_BACKOFF = 32000; export const FAILED_EVENT_RETRY_INTERVAL = 20 * 1000; export const EVENT_STORE_PREFIX = 'optly_event:'; diff --git a/lib/exception_messages.ts b/lib/exception_messages.ts index f17fa2821..731607ff8 100644 --- a/lib/exception_messages.ts +++ b/lib/exception_messages.ts @@ -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'; diff --git a/lib/odp/event_manager/odp_event_manager.spec.ts b/lib/odp/event_manager/odp_event_manager.spec.ts index 67b874509..ff7efa5cb 100644 --- a/lib/odp/event_manager/odp_event_manager.spec.ts +++ b/lib/odp/event_manager/odp_event_manager.spec.ts @@ -207,6 +207,40 @@ describe('DefaultOdpEventManager', () => { } }); + it('should flush the queue immediately if disposable, regardless of the batchSize', async () => { + const apiManager = getMockApiManager(); + const repeater = getMockRepeater() + apiManager.sendEvents.mockResolvedValue({ statusCode: 200 }); + + const odpEventManager = new DefaultOdpEventManager({ + repeater, + apiManager: apiManager, + batchSize: 10, + retryConfig: { + maxRetries: 3, + backoffProvider: vi.fn(), + }, + }); + + odpEventManager.updateConfig({ + integrated: true, + odpConfig: config, + }); + odpEventManager.makeDisposable(); + odpEventManager.start(); + + await expect(odpEventManager.onRunning()).resolves.not.toThrow(); + + const event = makeEvent(0); + odpEventManager.sendEvent(event); + await exhaustMicrotasks(); + + expect(apiManager.sendEvents).toHaveBeenCalledTimes(1); + expect(apiManager.sendEvents).toHaveBeenNthCalledWith(1, config, [event]); + expect(repeater.reset).toHaveBeenCalledTimes(1); + expect(repeater.start).not.toHaveBeenCalled(); + }) + it('drops events and logs if the state is not running', async () => { const apiManager = getMockApiManager(); apiManager.sendEvents.mockResolvedValue({ statusCode: 200 }); diff --git a/lib/odp/event_manager/odp_event_manager.ts b/lib/odp/event_manager/odp_event_manager.ts index 6ebe5aaa0..76aec79be 100644 --- a/lib/odp/event_manager/odp_event_manager.ts +++ b/lib/odp/event_manager/odp_event_manager.ts @@ -107,6 +107,7 @@ export class DefaultOdpEventManager extends BaseService implements OdpEventManag } super.start(); + if (this.odpIntegrationConfig) { this.goToRunningState(); } else { @@ -114,6 +115,12 @@ export class DefaultOdpEventManager extends BaseService implements OdpEventManag } } + makeDisposable(): void { + super.makeDisposable(); + this.retryConfig.maxRetries = Math.min(this.retryConfig.maxRetries, 5); + this.batchSize = 1; + } + updateConfig(odpIntegrationConfig: OdpIntegrationConfig): void { if (this.isDone()) { return; diff --git a/lib/odp/odp_manager.spec.ts b/lib/odp/odp_manager.spec.ts index dc6e2b96b..8ffc2721d 100644 --- a/lib/odp/odp_manager.spec.ts +++ b/lib/odp/odp_manager.spec.ts @@ -51,6 +51,7 @@ const getMockOdpEventManager = () => { getState: vi.fn(), updateConfig: vi.fn(), sendEvent: vi.fn(), + makeDisposable: vi.fn(), }; }; @@ -696,5 +697,19 @@ describe('DefaultOdpManager', () => { eventManagerTerminatedPromise.reject(new Error(FAILED_TO_STOP)); await expect(odpManager.onTerminated()).rejects.toThrow(); }); + + it('should call makeDisposable() on eventManager when makeDisposable() is called on odpManager', async () => { + const eventManager = getMockOdpEventManager(); + const segmentManager = getMockOdpSegmentManager(); + + const odpManager = new DefaultOdpManager({ + segmentManager, + eventManager, + }); + + odpManager.makeDisposable(); + + expect(eventManager.makeDisposable).toHaveBeenCalled(); + }) }); diff --git a/lib/odp/odp_manager.ts b/lib/odp/odp_manager.ts index 05c476ff3..4029a3621 100644 --- a/lib/odp/odp_manager.ts +++ b/lib/odp/odp_manager.ts @@ -108,6 +108,11 @@ export class DefaultOdpManager extends BaseService implements OdpManager { }); } + makeDisposable(): void { + super.makeDisposable(); + this.eventManager.makeDisposable(); + } + private handleStartSuccess() { if (this.isDone()) { return; diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index 5ced36a08..1825bb9a2 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -16,15 +16,13 @@ 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'; +import { createOdpManager } from '../odp/odp_manager_factory.node'; describe('Optimizely', () => { const errorHandler = { handleError: function() {} }; @@ -34,19 +32,20 @@ describe('Optimizely', () => { }; const eventProcessor = getForwardingEventProcessor(eventDispatcher); - + const odpManager = createOdpManager({}); const logger = getMockLogger(); - const notificationCenter = createNotificationCenter({ logger, errorHandler }); - it('should pass ssr to the project config manager', () => { + it('should pass disposable options to the respective services', () => { const projectConfigManager = getMockProjectConfigManager({ initConfig: createProjectConfig(testData.getTestProjectConfig()), }); - vi.spyOn(projectConfigManager, 'setSsr'); + vi.spyOn(projectConfigManager, 'makeDisposable'); + vi.spyOn(eventProcessor, 'makeDisposable'); + vi.spyOn(odpManager, 'makeDisposable'); - const instance = new Optimizely({ + new Optimizely({ clientEngine: 'node-sdk', projectConfigManager, errorHandler, @@ -54,16 +53,13 @@ describe('Optimizely', () => { logger, notificationCenter, eventProcessor, - isSsr: true, + odpManager, + disposable: true, isValidInstance: true, }); - expect(projectConfigManager.setSsr).toHaveBeenCalledWith(true); - // 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.makeDisposable).toHaveBeenCalled(); + expect(eventProcessor.makeDisposable).toHaveBeenCalled(); + expect(odpManager.makeDisposable).toHaveBeenCalled(); }); }); diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index 34fa116f6..d71abfd3a 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -141,10 +141,20 @@ export default class Optimizely implements Client { this.errorHandler = config.errorHandler; this.isOptimizelyConfigValid = config.isValidInstance; this.logger = config.logger; + this.projectConfigManager = config.projectConfigManager; + this.notificationCenter = config.notificationCenter; this.odpManager = config.odpManager; this.vuidManager = config.vuidManager; + this.eventProcessor = config.eventProcessor; + + if(config.disposable) { + this.projectConfigManager.makeDisposable(); + this.eventProcessor?.makeDisposable(); + this.odpManager?.makeDisposable(); + } let decideOptionsArray = config.defaultDecideOptions ?? []; + if (!Array.isArray(decideOptionsArray)) { this.logger.log(LOG_LEVEL.DEBUG, INVALID_DEFAULT_DECIDE_OPTIONS, MODULE_NAME); decideOptionsArray = []; @@ -160,7 +170,6 @@ export default class Optimizely implements Client { } }); this.defaultDecideOptions = defaultDecideOptions; - this.projectConfigManager = config.projectConfigManager; this.disposeOnUpdate = this.projectConfigManager.onUpdate((configObj: projectConfig.ProjectConfig) => { this.logger.log( @@ -176,7 +185,6 @@ export default class Optimizely implements Client { this.updateOdpSettings(); }); - this.projectConfigManager.setSsr(config.isSsr) this.projectConfigManager.start(); const projectConfigManagerRunningPromise = this.projectConfigManager.onRunning(); @@ -198,10 +206,6 @@ export default class Optimizely implements Client { UNSTABLE_conditionEvaluators: config.UNSTABLE_conditionEvaluators, }); - this.notificationCenter = config.notificationCenter; - - this.eventProcessor = config.eventProcessor; - this.eventProcessor?.start(); const eventProcessorRunningPromise = this.eventProcessor ? this.eventProcessor.onRunning() : Promise.resolve(undefined); @@ -210,6 +214,8 @@ export default class Optimizely implements Client { this.notificationCenter.sendNotifications(NOTIFICATION_TYPES.LOG_EVENT, event); }); + this.odpManager?.start(); + this.readyPromise = Promise.all([ projectConfigManagerRunningPromise, eventProcessorRunningPromise, diff --git a/lib/project_config/polling_datafile_manager.spec.ts b/lib/project_config/polling_datafile_manager.spec.ts index 3efae54d7..642061d96 100644 --- a/lib/project_config/polling_datafile_manager.spec.ts +++ b/lib/project_config/polling_datafile_manager.spec.ts @@ -265,6 +265,30 @@ describe('PollingDatafileManager', () => { await expect(manager.onTerminated()).rejects.toThrow(); }); + it('retries min(initRetry, 5) amount of times if datafile manager is disposable', async () => { + const repeater = getMockRepeater(); + const requestHandler = getMockRequestHandler(); + const mockResponse = getMockAbortableRequest(Promise.reject('test error')); + requestHandler.makeRequest.mockReturnValue(mockResponse); + + const manager = new PollingDatafileManager({ + repeater, + requestHandler, + sdkKey: 'keyThatExists', + initRetry: 10, + }); + manager.makeDisposable(); + manager.start(); + + for(let i = 0; i < 6; i++) { + const ret = repeater.execute(i); + await expect(ret).rejects.toThrow(); + } + + expect(repeater.isRunning()).toBe(false); + expect(() => repeater.execute(6)).toThrow(); + }) + it('retries specified number of times before rejecting onRunning() and onTerminated() when autoupdate is false', async () => { const repeater = getMockRepeater(); const requestHandler = getMockRequestHandler(); @@ -470,6 +494,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(); diff --git a/lib/project_config/polling_datafile_manager.ts b/lib/project_config/polling_datafile_manager.ts index bde704029..62b17cfe4 100644 --- a/lib/project_config/polling_datafile_manager.ts +++ b/lib/project_config/polling_datafile_manager.ts @@ -96,6 +96,11 @@ export class PollingDatafileManager extends BaseService implements DatafileManag this.repeater.start(true); } + makeDisposable(): void { + super.makeDisposable(); + this.initRetryRemaining = Math.min(this.initRetryRemaining ?? 5, 5); + } + stop(): void { if (this.isDone()) { return; @@ -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(); } } diff --git a/lib/project_config/project_config_manager.spec.ts b/lib/project_config/project_config_manager.spec.ts index 7bed978ef..acd8538ee 100644 --- a/lib/project_config/project_config_manager.spec.ts +++ b/lib/project_config/project_config_manager.spec.ts @@ -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', () => { @@ -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(); @@ -538,6 +517,15 @@ describe('ProjectConfigManagerImpl', () => { expect(listener).toHaveBeenCalledTimes(1); }); + + it('should make datafileManager disposable if makeDisposable() is called', async () => { + const datafileManager = getMockDatafileManager({}); + vi.spyOn(datafileManager, 'makeDisposable'); + const manager = new ProjectConfigManagerImpl({ datafileManager }); + manager.makeDisposable(); + + expect(datafileManager.makeDisposable).toHaveBeenCalled(); + }) }); }); }); diff --git a/lib/project_config/project_config_manager.ts b/lib/project_config/project_config_manager.ts index 81ee87b78..a0ebbffdb 100644 --- a/lib/project_config/project_config_manager.ts +++ b/lib/project_config/project_config_manager.ts @@ -26,13 +26,10 @@ 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 { - // TODO: Don't use object type - // eslint-disable-next-line @typescript-eslint/ban-types - datafile?: string | object; + datafile?: string | Record; jsonSchemaValidator?: Transformer, datafileManager?: DatafileManager; logger?: LoggerFacade; @@ -40,7 +37,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): Fn; @@ -60,7 +56,6 @@ export class ProjectConfigManagerImpl extends BaseService implements ProjectConf public jsonSchemaValidator?: Transformer; public datafileManager?: DatafileManager; private eventEmitter: EventEmitter<{ update: ProjectConfig }> = new EventEmitter(); - private isSsr = false; constructor(config: ProjectConfigManagerConfig) { super(); @@ -77,17 +72,8 @@ 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; } @@ -110,6 +96,11 @@ export class ProjectConfigManagerImpl extends BaseService implements ProjectConf }); } + makeDisposable(): void { + super.makeDisposable(); + this.datafileManager?.makeDisposable(); + } + private handleInitError(error: Error): void { this.logger?.error(error); this.state = ServiceState.Failed; @@ -206,7 +197,6 @@ export class ProjectConfigManagerImpl extends BaseService implements ProjectConf } if (this.isNew() || this.isStarting()) { - // TOOD: replace message with imported constants this.startPromise.reject(new Error(DATAFILE_MANAGER_STOPPED)); } @@ -227,13 +217,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; - } } diff --git a/lib/service.ts b/lib/service.ts index 459488027..2d0877bee 100644 --- a/lib/service.ts +++ b/lib/service.ts @@ -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; + makeDisposable(): void; } export abstract class BaseService implements Service { @@ -59,7 +60,7 @@ export abstract class BaseService implements Service { protected stopPromise: ResolvablePromise; protected logger?: LoggerFacade; protected startupLogs: StartupLog[]; - + protected disposable = false; constructor(startupLogs: StartupLog[] = []) { this.state = ServiceState.New; this.startPromise = resolvablePromise(); @@ -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; } diff --git a/lib/shared_types.ts b/lib/shared_types.ts index b2ebad540..299dc9332 100644 --- a/lib/shared_types.ts +++ b/lib/shared_types.ts @@ -263,10 +263,10 @@ export interface OptimizelyOptions { sdkKey?: string; userProfileService?: UserProfileService | null; defaultDecideOptions?: OptimizelyDecideOption[]; - isSsr?:boolean; odpManager?: OdpManager; notificationCenter: DefaultNotificationCenter; vuidManager?: VuidManager + disposable?: boolean; } /** @@ -384,9 +384,9 @@ export interface Config { defaultDecideOptions?: OptimizelyDecideOption[]; clientEngine?: string; clientVersion?: string; - isSsr?: boolean; odpManager?: OdpManager; vuidManager?: VuidManager; + disposable?: boolean; } export type OptimizelyExperimentsMap = { diff --git a/lib/tests/mock/mock_project_config_manager.ts b/lib/tests/mock/mock_project_config_manager.ts index b76f71e2d..65c6268ab 100644 --- a/lib/tests/mock/mock_project_config_manager.ts +++ b/lib/tests/mock/mock_project_config_manager.ts @@ -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(), diff --git a/lib/tests/mock/mock_repeater.ts b/lib/tests/mock/mock_repeater.ts index adf6baf83..f70b0b477 100644 --- a/lib/tests/mock/mock_repeater.ts +++ b/lib/tests/mock/mock_repeater.ts @@ -31,7 +31,7 @@ export const getMockRepeater = () => { // throw if not running. This ensures tests cannot // do mock exection when the repeater is supposed to be not running. execute(failureCount: number): Promise { - if (!this.isRunning) throw new Error(); + if (!this.isRunning()) throw new Error(); const ret = this.handler?.(failureCount); ret?.catch(() => {}); return ret;