Skip to content

[FSSDK-11510] add validation to factories #1060

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 3 commits into from
May 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 61 additions & 0 deletions lib/client_factory.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* Copyright 2025, 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 { describe, it, expect } from 'vitest';

import { getOptimizelyInstance } from './client_factory';
import { createStaticProjectConfigManager } from './project_config/config_manager_factory';
import Optimizely from './optimizely';

describe('getOptimizelyInstance', () => {
it('should throw if the projectConfigManager is not a valid ProjectConfigManager', () => {
expect(() => getOptimizelyInstance({
projectConfigManager: undefined as any,

Check warning on line 26 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 26 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 26 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 26 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type
requestHandler: {} as any,

Check warning on line 27 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 27 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 27 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 27 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type
})).toThrow('Invalid config manager');

expect(() => getOptimizelyInstance({
projectConfigManager: null as any,

Check warning on line 31 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 31 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 31 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 31 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type
requestHandler: {} as any,

Check warning on line 32 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 32 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 32 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 32 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type
})).toThrow('Invalid config manager');

expect(() => getOptimizelyInstance({
projectConfigManager: 'abc' as any,

Check warning on line 36 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 36 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 36 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 36 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type
requestHandler: {} as any,

Check warning on line 37 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 37 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 37 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 37 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type
})).toThrow('Invalid config manager');

expect(() => getOptimizelyInstance({
projectConfigManager: 123 as any,

Check warning on line 41 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 41 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 41 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 41 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type
requestHandler: {} as any,

Check warning on line 42 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 42 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 42 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 42 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type
})).toThrow('Invalid config manager');

expect(() => getOptimizelyInstance({
projectConfigManager: {} as any,

Check warning on line 46 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 46 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 46 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 46 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type
requestHandler: {} as any,

Check warning on line 47 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

Check warning on line 47 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (18)

Unexpected any. Specify a different type

Check warning on line 47 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (20)

Unexpected any. Specify a different type

Check warning on line 47 in lib/client_factory.spec.ts

View workflow job for this annotation

GitHub Actions / unit_tests (22)

Unexpected any. Specify a different type
})).toThrow('Invalid config manager');
});

it('should return an instance of Optimizely if a valid projectConfigManager is provided', () => {
const optimizelyInstance = getOptimizelyInstance({
projectConfigManager: createStaticProjectConfigManager({
datafile: '{}',
}),
requestHandler: {} as any,
});

expect(optimizelyInstance).toBeInstanceOf(Optimizely);
});
});
99 changes: 42 additions & 57 deletions lib/client_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { LoggerFacade } from "./logging/logger";
import { Client, Config } from "./shared_types";
import { Maybe } from "./utils/type";
import configValidator from './utils/config_validator';
import { extractLogger } from "./logging/logger_factory";
import { extractErrorNotifier } from "./error/error_notifier_factory";
import { extractConfigManager } from "./project_config/config_manager_factory";
Expand All @@ -35,61 +31,50 @@ export type OptimizelyFactoryConfig = Config & {
requestHandler: RequestHandler;
}

export const getOptimizelyInstance = (config: OptimizelyFactoryConfig): Client | null => {
let logger: Maybe<LoggerFacade>;

try {
logger = config.logger ? extractLogger(config.logger) : undefined;

configValidator.validate(config);

const {
clientEngine,
clientVersion,
jsonSchemaValidator,
userProfileService,
userProfileServiceAsync,
defaultDecideOptions,
disposable,
requestHandler,
} = config;

const errorNotifier = config.errorNotifier ? extractErrorNotifier(config.errorNotifier) : undefined;

const projectConfigManager = extractConfigManager(config.projectConfigManager);
const eventProcessor = config.eventProcessor ? extractEventProcessor(config.eventProcessor) : undefined;
const odpManager = config.odpManager ? extractOdpManager(config.odpManager) : undefined;
const vuidManager = config.vuidManager ? extractVuidManager(config.vuidManager) : undefined;
export const getOptimizelyInstance = (config: OptimizelyFactoryConfig): Client => {
const {
clientEngine,
clientVersion,
jsonSchemaValidator,
userProfileService,
userProfileServiceAsync,
defaultDecideOptions,
disposable,
requestHandler,
} = config;

const projectConfigManager = extractConfigManager(config.projectConfigManager);
const eventProcessor = extractEventProcessor(config.eventProcessor);
const odpManager = extractOdpManager(config.odpManager);
const vuidManager = extractVuidManager(config.vuidManager);
const errorNotifier = extractErrorNotifier(config.errorNotifier);
const logger = extractLogger(config.logger);

const cmabClient = new DefaultCmabClient({
requestHandler,
});
const cmabClient = new DefaultCmabClient({
requestHandler,
});

const cmabService = new DefaultCmabService({
cmabClient,
cmabCache: new InMemoryLruCache<CmabCacheValue>(DEFAULT_CMAB_CACHE_SIZE, DEFAULT_CMAB_CACHE_TIMEOUT),
});
const cmabService = new DefaultCmabService({
cmabClient,
cmabCache: new InMemoryLruCache<CmabCacheValue>(DEFAULT_CMAB_CACHE_SIZE, DEFAULT_CMAB_CACHE_TIMEOUT),
});

const optimizelyOptions = {
cmabService,
clientEngine: clientEngine || JAVASCRIPT_CLIENT_ENGINE,
clientVersion: clientVersion || CLIENT_VERSION,
jsonSchemaValidator,
userProfileService,
userProfileServiceAsync,
defaultDecideOptions,
disposable,
logger,
errorNotifier,
projectConfigManager,
eventProcessor,
odpManager,
vuidManager,
};
const optimizelyOptions = {
cmabService,
clientEngine: clientEngine || JAVASCRIPT_CLIENT_ENGINE,
clientVersion: clientVersion || CLIENT_VERSION,
jsonSchemaValidator,
userProfileService,
userProfileServiceAsync,
defaultDecideOptions,
disposable,
logger,
errorNotifier,
projectConfigManager,
eventProcessor,
odpManager,
vuidManager,
};

return new Optimizely(optimizelyOptions);
} catch (e) {
logger?.error(e);
return null;
}
return new Optimizely(optimizelyOptions);
}
2 changes: 1 addition & 1 deletion lib/core/decision_service/index.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
LOG_LEVEL,
DECISION_SOURCES,
} from '../../utils/enums';
import { getForwardingEventProcessor } from '../../event_processor/forwarding_event_processor';
import { getForwardingEventProcessor } from '../../event_processor/event_processor_factory';
import { createNotificationCenter } from '../../notification_center';
import Optimizely from '../../optimizely';
import OptimizelyUserContext from '../../optimizely_user_context';
Expand Down
2 changes: 1 addition & 1 deletion lib/entrypoint.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ import { Maybe } from './utils/type';

export type Entrypoint = {
// client factory
createInstance: (config: Config) => Client | null;
createInstance: (config: Config) => Client;

// config manager related exports
createStaticProjectConfigManager: (config: StaticConfigManagerConfig) => OpaqueConfigManager;
Expand Down
2 changes: 1 addition & 1 deletion lib/entrypoint.universal.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ import { UniversalOdpManagerOptions } from './odp/odp_manager_factory.universal'

export type UniversalEntrypoint = {
// client factory
createInstance: (config: UniversalConfig) => Client | null;
createInstance: (config: UniversalConfig) => Client;

// config manager related exports
createStaticProjectConfigManager: (config: StaticConfigManagerConfig) => OpaqueConfigManager;
Expand Down
33 changes: 33 additions & 0 deletions lib/error/error_notifier_factory.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* Copyright 2025, 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 { describe, it, expect } from 'vitest';
import { createErrorNotifier } from './error_notifier_factory';

describe('createErrorNotifier', () => {
it('should throw errors for invalid error handlers', () => {
expect(() => createErrorNotifier(null as any)).toThrow('Invalid error handler');
expect(() => createErrorNotifier(undefined as any)).toThrow('Invalid error handler');


expect(() => createErrorNotifier('abc' as any)).toThrow('Invalid error handler');
expect(() => createErrorNotifier(123 as any)).toThrow('Invalid error handler');

expect(() => createErrorNotifier({} as any)).toThrow('Invalid error handler');

expect(() => createErrorNotifier({ handleError: 'abc' } as any)).toThrow('Invalid error handler');
});
});
18 changes: 16 additions & 2 deletions lib/error/error_notifier_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,35 @@
* limitations under the License.
*/
import { errorResolver } from "../message/message_resolver";
import { Maybe } from "../utils/type";
import { ErrorHandler } from "./error_handler";
import { DefaultErrorNotifier } from "./error_notifier";

export const INVALID_ERROR_HANDLER = 'Invalid error handler';

const errorNotifierSymbol = Symbol();

export type OpaqueErrorNotifier = {
[errorNotifierSymbol]: unknown;
};

const validateErrorHandler = (errorHandler: ErrorHandler) => {
if (!errorHandler || typeof errorHandler !== 'object' || typeof errorHandler.handleError !== 'function') {
throw new Error(INVALID_ERROR_HANDLER);
}
}

export const createErrorNotifier = (errorHandler: ErrorHandler): OpaqueErrorNotifier => {
validateErrorHandler(errorHandler);
return {
[errorNotifierSymbol]: new DefaultErrorNotifier(errorHandler, errorResolver),
}
}

export const extractErrorNotifier = (errorNotifier: OpaqueErrorNotifier): DefaultErrorNotifier => {
return errorNotifier[errorNotifierSymbol] as DefaultErrorNotifier;
export const extractErrorNotifier = (errorNotifier: Maybe<OpaqueErrorNotifier>): Maybe<DefaultErrorNotifier> => {
if (!errorNotifier || typeof errorNotifier !== 'object') {
return undefined;
}

return errorNotifier[errorNotifierSymbol] as Maybe<DefaultErrorNotifier>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import { RequestHandler } from '../../utils/http_request_handler/http';
import { DefaultEventDispatcher } from './default_dispatcher';
import { EventDispatcher } from './event_dispatcher';

import { validateRequestHandler } from '../../utils/http_request_handler/request_handler_validator';

export const createEventDispatcher = (requestHander: RequestHandler): EventDispatcher => {
validateRequestHandler(requestHander);
return new DefaultEventDispatcher(requestHander);
}
15 changes: 5 additions & 10 deletions lib/event_processor/event_processor_factory.browser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,18 @@ vi.mock('./default_dispatcher.browser', () => {
return { default: {} };
});

vi.mock('./forwarding_event_processor', () => {
const getForwardingEventProcessor = vi.fn().mockImplementation(() => {
return {};
});
return { getForwardingEventProcessor };
});

vi.mock('./event_processor_factory', async (importOriginal) => {
const getBatchEventProcessor = vi.fn().mockImplementation(() => {
return {};
});
const getOpaqueBatchEventProcessor = vi.fn().mockImplementation(() => {
return {};
});
const getForwardingEventProcessor = vi.fn().mockImplementation(() => {
return {};
});
const original: any = await importOriginal();
return { ...original, getBatchEventProcessor, getOpaqueBatchEventProcessor };
return { ...original, getBatchEventProcessor, getOpaqueBatchEventProcessor, getForwardingEventProcessor };
});

vi.mock('../utils/cache/local_storage_cache.browser', () => {
Expand All @@ -50,9 +46,8 @@ import defaultEventDispatcher from './event_dispatcher/default_dispatcher.browse
import { LocalStorageCache } from '../utils/cache/local_storage_cache.browser';
import { SyncPrefixStore } from '../utils/cache/store';
import { createForwardingEventProcessor, createBatchEventProcessor } from './event_processor_factory.browser';
import { EVENT_STORE_PREFIX, extractEventProcessor, FAILED_EVENT_RETRY_INTERVAL } from './event_processor_factory';
import { EVENT_STORE_PREFIX, extractEventProcessor, getForwardingEventProcessor, FAILED_EVENT_RETRY_INTERVAL } from './event_processor_factory';
import sendBeaconEventDispatcher from './event_dispatcher/send_beacon_dispatcher.browser';
import { getForwardingEventProcessor } from './forwarding_event_processor';
import browserDefaultEventDispatcher from './event_dispatcher/default_dispatcher.browser';
import { getOpaqueBatchEventProcessor } from './event_processor_factory';

Expand Down
3 changes: 1 addition & 2 deletions lib/event_processor/event_processor_factory.browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { getForwardingEventProcessor } from './forwarding_event_processor';
import { EventDispatcher } from './event_dispatcher/event_dispatcher';
import { EventProcessor } from './event_processor';
import { EventWithId } from './batch_event_processor';
Expand All @@ -23,6 +21,7 @@ import {
BatchEventProcessorOptions,
OpaqueEventProcessor,
wrapEventProcessor,
getForwardingEventProcessor,
} from './event_processor_factory';
import defaultEventDispatcher from './event_dispatcher/default_dispatcher.browser';
import sendBeaconEventDispatcher from './event_dispatcher/send_beacon_dispatcher.browser';
Expand Down
11 changes: 3 additions & 8 deletions lib/event_processor/event_processor_factory.node.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,16 @@ vi.mock('./default_dispatcher.node', () => {
return { default: {} };
});

vi.mock('./forwarding_event_processor', () => {
const getForwardingEventProcessor = vi.fn().mockReturnValue({});
return { getForwardingEventProcessor };
});

vi.mock('./event_processor_factory', async (importOriginal) => {
const getBatchEventProcessor = vi.fn().mockImplementation(() => {
return {};
});
const getOpaqueBatchEventProcessor = vi.fn().mockImplementation(() => {
return {};
});
const getForwardingEventProcessor = vi.fn().mockReturnValue({});
const original: any = await importOriginal();
return { ...original, getBatchEventProcessor, getOpaqueBatchEventProcessor };
return { ...original, getBatchEventProcessor, getOpaqueBatchEventProcessor, getForwardingEventProcessor };
});

vi.mock('../utils/cache/async_storage_cache.react_native', () => {
Expand All @@ -44,9 +40,8 @@ vi.mock('../utils/cache/store', () => {
});

import { createBatchEventProcessor, createForwardingEventProcessor } from './event_processor_factory.node';
import { getForwardingEventProcessor } from './forwarding_event_processor';
import nodeDefaultEventDispatcher from './event_dispatcher/default_dispatcher.node';
import { EVENT_STORE_PREFIX, extractEventProcessor, FAILED_EVENT_RETRY_INTERVAL } from './event_processor_factory';
import { EVENT_STORE_PREFIX, extractEventProcessor, getForwardingEventProcessor, FAILED_EVENT_RETRY_INTERVAL } from './event_processor_factory';
import { getOpaqueBatchEventProcessor } from './event_processor_factory';
import { AsyncStore, AsyncPrefixStore, SyncStore, SyncPrefixStore } from '../utils/cache/store';
import { AsyncStorageCache } from '../utils/cache/async_storage_cache.react_native';
Expand Down
2 changes: 1 addition & 1 deletion lib/event_processor/event_processor_factory.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { getForwardingEventProcessor } from './forwarding_event_processor';
import { EventDispatcher } from './event_dispatcher/event_dispatcher';
import defaultEventDispatcher from './event_dispatcher/default_dispatcher.node';
import {
Expand All @@ -23,6 +22,7 @@ import {
getPrefixEventStore,
OpaqueEventProcessor,
wrapEventProcessor,
getForwardingEventProcessor,
} from './event_processor_factory';

export const DEFAULT_EVENT_BATCH_SIZE = 10;
Expand Down
Loading