Skip to content

Commit 2937fe0

Browse files
authored
[FSSDK-11510] add validation to factories (#1060)
also updated createInstance to throw in case of validation errors
1 parent 0391b47 commit 2937fe0

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+554
-211
lines changed

lib/client_factory.spec.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/**
2+
* Copyright 2025, Optimizely
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { describe, it, expect } from 'vitest';
18+
19+
import { getOptimizelyInstance } from './client_factory';
20+
import { createStaticProjectConfigManager } from './project_config/config_manager_factory';
21+
import Optimizely from './optimizely';
22+
23+
describe('getOptimizelyInstance', () => {
24+
it('should throw if the projectConfigManager is not a valid ProjectConfigManager', () => {
25+
expect(() => getOptimizelyInstance({
26+
projectConfigManager: undefined as any,
27+
requestHandler: {} as any,
28+
})).toThrow('Invalid config manager');
29+
30+
expect(() => getOptimizelyInstance({
31+
projectConfigManager: null as any,
32+
requestHandler: {} as any,
33+
})).toThrow('Invalid config manager');
34+
35+
expect(() => getOptimizelyInstance({
36+
projectConfigManager: 'abc' as any,
37+
requestHandler: {} as any,
38+
})).toThrow('Invalid config manager');
39+
40+
expect(() => getOptimizelyInstance({
41+
projectConfigManager: 123 as any,
42+
requestHandler: {} as any,
43+
})).toThrow('Invalid config manager');
44+
45+
expect(() => getOptimizelyInstance({
46+
projectConfigManager: {} as any,
47+
requestHandler: {} as any,
48+
})).toThrow('Invalid config manager');
49+
});
50+
51+
it('should return an instance of Optimizely if a valid projectConfigManager is provided', () => {
52+
const optimizelyInstance = getOptimizelyInstance({
53+
projectConfigManager: createStaticProjectConfigManager({
54+
datafile: '{}',
55+
}),
56+
requestHandler: {} as any,
57+
});
58+
59+
expect(optimizelyInstance).toBeInstanceOf(Optimizely);
60+
});
61+
});

lib/client_factory.ts

Lines changed: 42 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,7 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
17-
import { LoggerFacade } from "./logging/logger";
1816
import { Client, Config } from "./shared_types";
19-
import { Maybe } from "./utils/type";
20-
import configValidator from './utils/config_validator';
2117
import { extractLogger } from "./logging/logger_factory";
2218
import { extractErrorNotifier } from "./error/error_notifier_factory";
2319
import { extractConfigManager } from "./project_config/config_manager_factory";
@@ -35,61 +31,50 @@ export type OptimizelyFactoryConfig = Config & {
3531
requestHandler: RequestHandler;
3632
}
3733

38-
export const getOptimizelyInstance = (config: OptimizelyFactoryConfig): Client | null => {
39-
let logger: Maybe<LoggerFacade>;
40-
41-
try {
42-
logger = config.logger ? extractLogger(config.logger) : undefined;
43-
44-
configValidator.validate(config);
45-
46-
const {
47-
clientEngine,
48-
clientVersion,
49-
jsonSchemaValidator,
50-
userProfileService,
51-
userProfileServiceAsync,
52-
defaultDecideOptions,
53-
disposable,
54-
requestHandler,
55-
} = config;
56-
57-
const errorNotifier = config.errorNotifier ? extractErrorNotifier(config.errorNotifier) : undefined;
58-
59-
const projectConfigManager = extractConfigManager(config.projectConfigManager);
60-
const eventProcessor = config.eventProcessor ? extractEventProcessor(config.eventProcessor) : undefined;
61-
const odpManager = config.odpManager ? extractOdpManager(config.odpManager) : undefined;
62-
const vuidManager = config.vuidManager ? extractVuidManager(config.vuidManager) : undefined;
34+
export const getOptimizelyInstance = (config: OptimizelyFactoryConfig): Client => {
35+
const {
36+
clientEngine,
37+
clientVersion,
38+
jsonSchemaValidator,
39+
userProfileService,
40+
userProfileServiceAsync,
41+
defaultDecideOptions,
42+
disposable,
43+
requestHandler,
44+
} = config;
45+
46+
const projectConfigManager = extractConfigManager(config.projectConfigManager);
47+
const eventProcessor = extractEventProcessor(config.eventProcessor);
48+
const odpManager = extractOdpManager(config.odpManager);
49+
const vuidManager = extractVuidManager(config.vuidManager);
50+
const errorNotifier = extractErrorNotifier(config.errorNotifier);
51+
const logger = extractLogger(config.logger);
6352

64-
const cmabClient = new DefaultCmabClient({
65-
requestHandler,
66-
});
53+
const cmabClient = new DefaultCmabClient({
54+
requestHandler,
55+
});
6756

68-
const cmabService = new DefaultCmabService({
69-
cmabClient,
70-
cmabCache: new InMemoryLruCache<CmabCacheValue>(DEFAULT_CMAB_CACHE_SIZE, DEFAULT_CMAB_CACHE_TIMEOUT),
71-
});
57+
const cmabService = new DefaultCmabService({
58+
cmabClient,
59+
cmabCache: new InMemoryLruCache<CmabCacheValue>(DEFAULT_CMAB_CACHE_SIZE, DEFAULT_CMAB_CACHE_TIMEOUT),
60+
});
7261

73-
const optimizelyOptions = {
74-
cmabService,
75-
clientEngine: clientEngine || JAVASCRIPT_CLIENT_ENGINE,
76-
clientVersion: clientVersion || CLIENT_VERSION,
77-
jsonSchemaValidator,
78-
userProfileService,
79-
userProfileServiceAsync,
80-
defaultDecideOptions,
81-
disposable,
82-
logger,
83-
errorNotifier,
84-
projectConfigManager,
85-
eventProcessor,
86-
odpManager,
87-
vuidManager,
88-
};
62+
const optimizelyOptions = {
63+
cmabService,
64+
clientEngine: clientEngine || JAVASCRIPT_CLIENT_ENGINE,
65+
clientVersion: clientVersion || CLIENT_VERSION,
66+
jsonSchemaValidator,
67+
userProfileService,
68+
userProfileServiceAsync,
69+
defaultDecideOptions,
70+
disposable,
71+
logger,
72+
errorNotifier,
73+
projectConfigManager,
74+
eventProcessor,
75+
odpManager,
76+
vuidManager,
77+
};
8978

90-
return new Optimizely(optimizelyOptions);
91-
} catch (e) {
92-
logger?.error(e);
93-
return null;
94-
}
79+
return new Optimizely(optimizelyOptions);
9580
}

lib/core/decision_service/index.tests.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import {
2525
LOG_LEVEL,
2626
DECISION_SOURCES,
2727
} from '../../utils/enums';
28-
import { getForwardingEventProcessor } from '../../event_processor/forwarding_event_processor';
28+
import { getForwardingEventProcessor } from '../../event_processor/event_processor_factory';
2929
import { createNotificationCenter } from '../../notification_center';
3030
import Optimizely from '../../optimizely';
3131
import OptimizelyUserContext from '../../optimizely_user_context';

lib/entrypoint.test-d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ import { Maybe } from './utils/type';
5959

6060
export type Entrypoint = {
6161
// client factory
62-
createInstance: (config: Config) => Client | null;
62+
createInstance: (config: Config) => Client;
6363

6464
// config manager related exports
6565
createStaticProjectConfigManager: (config: StaticConfigManagerConfig) => OpaqueConfigManager;

lib/entrypoint.universal.test-d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ import { UniversalOdpManagerOptions } from './odp/odp_manager_factory.universal'
5555

5656
export type UniversalEntrypoint = {
5757
// client factory
58-
createInstance: (config: UniversalConfig) => Client | null;
58+
createInstance: (config: UniversalConfig) => Client;
5959

6060
// config manager related exports
6161
createStaticProjectConfigManager: (config: StaticConfigManagerConfig) => OpaqueConfigManager;
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* Copyright 2025, Optimizely
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
import { describe, it, expect } from 'vitest';
18+
import { createErrorNotifier } from './error_notifier_factory';
19+
20+
describe('createErrorNotifier', () => {
21+
it('should throw errors for invalid error handlers', () => {
22+
expect(() => createErrorNotifier(null as any)).toThrow('Invalid error handler');
23+
expect(() => createErrorNotifier(undefined as any)).toThrow('Invalid error handler');
24+
25+
26+
expect(() => createErrorNotifier('abc' as any)).toThrow('Invalid error handler');
27+
expect(() => createErrorNotifier(123 as any)).toThrow('Invalid error handler');
28+
29+
expect(() => createErrorNotifier({} as any)).toThrow('Invalid error handler');
30+
31+
expect(() => createErrorNotifier({ handleError: 'abc' } as any)).toThrow('Invalid error handler');
32+
});
33+
});

lib/error/error_notifier_factory.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,35 @@
1414
* limitations under the License.
1515
*/
1616
import { errorResolver } from "../message/message_resolver";
17+
import { Maybe } from "../utils/type";
1718
import { ErrorHandler } from "./error_handler";
1819
import { DefaultErrorNotifier } from "./error_notifier";
1920

21+
export const INVALID_ERROR_HANDLER = 'Invalid error handler';
22+
2023
const errorNotifierSymbol = Symbol();
2124

2225
export type OpaqueErrorNotifier = {
2326
[errorNotifierSymbol]: unknown;
2427
};
2528

29+
const validateErrorHandler = (errorHandler: ErrorHandler) => {
30+
if (!errorHandler || typeof errorHandler !== 'object' || typeof errorHandler.handleError !== 'function') {
31+
throw new Error(INVALID_ERROR_HANDLER);
32+
}
33+
}
34+
2635
export const createErrorNotifier = (errorHandler: ErrorHandler): OpaqueErrorNotifier => {
36+
validateErrorHandler(errorHandler);
2737
return {
2838
[errorNotifierSymbol]: new DefaultErrorNotifier(errorHandler, errorResolver),
2939
}
3040
}
3141

32-
export const extractErrorNotifier = (errorNotifier: OpaqueErrorNotifier): DefaultErrorNotifier => {
33-
return errorNotifier[errorNotifierSymbol] as DefaultErrorNotifier;
42+
export const extractErrorNotifier = (errorNotifier: Maybe<OpaqueErrorNotifier>): Maybe<DefaultErrorNotifier> => {
43+
if (!errorNotifier || typeof errorNotifier !== 'object') {
44+
return undefined;
45+
}
46+
47+
return errorNotifier[errorNotifierSymbol] as Maybe<DefaultErrorNotifier>;
3448
}

lib/event_processor/event_dispatcher/event_dispatcher_factory.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ import { RequestHandler } from '../../utils/http_request_handler/http';
1818
import { DefaultEventDispatcher } from './default_dispatcher';
1919
import { EventDispatcher } from './event_dispatcher';
2020

21+
import { validateRequestHandler } from '../../utils/http_request_handler/request_handler_validator';
22+
2123
export const createEventDispatcher = (requestHander: RequestHandler): EventDispatcher => {
24+
validateRequestHandler(requestHander);
2225
return new DefaultEventDispatcher(requestHander);
2326
}

lib/event_processor/event_processor_factory.browser.spec.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,18 @@ vi.mock('./default_dispatcher.browser', () => {
1919
return { default: {} };
2020
});
2121

22-
vi.mock('./forwarding_event_processor', () => {
23-
const getForwardingEventProcessor = vi.fn().mockImplementation(() => {
24-
return {};
25-
});
26-
return { getForwardingEventProcessor };
27-
});
28-
2922
vi.mock('./event_processor_factory', async (importOriginal) => {
3023
const getBatchEventProcessor = vi.fn().mockImplementation(() => {
3124
return {};
3225
});
3326
const getOpaqueBatchEventProcessor = vi.fn().mockImplementation(() => {
3427
return {};
3528
});
29+
const getForwardingEventProcessor = vi.fn().mockImplementation(() => {
30+
return {};
31+
});
3632
const original: any = await importOriginal();
37-
return { ...original, getBatchEventProcessor, getOpaqueBatchEventProcessor };
33+
return { ...original, getBatchEventProcessor, getOpaqueBatchEventProcessor, getForwardingEventProcessor };
3834
});
3935

4036
vi.mock('../utils/cache/local_storage_cache.browser', () => {
@@ -50,9 +46,8 @@ import defaultEventDispatcher from './event_dispatcher/default_dispatcher.browse
5046
import { LocalStorageCache } from '../utils/cache/local_storage_cache.browser';
5147
import { SyncPrefixStore } from '../utils/cache/store';
5248
import { createForwardingEventProcessor, createBatchEventProcessor } from './event_processor_factory.browser';
53-
import { EVENT_STORE_PREFIX, extractEventProcessor, FAILED_EVENT_RETRY_INTERVAL } from './event_processor_factory';
49+
import { EVENT_STORE_PREFIX, extractEventProcessor, getForwardingEventProcessor, FAILED_EVENT_RETRY_INTERVAL } from './event_processor_factory';
5450
import sendBeaconEventDispatcher from './event_dispatcher/send_beacon_dispatcher.browser';
55-
import { getForwardingEventProcessor } from './forwarding_event_processor';
5651
import browserDefaultEventDispatcher from './event_dispatcher/default_dispatcher.browser';
5752
import { getOpaqueBatchEventProcessor } from './event_processor_factory';
5853

lib/event_processor/event_processor_factory.browser.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
17-
import { getForwardingEventProcessor } from './forwarding_event_processor';
1816
import { EventDispatcher } from './event_dispatcher/event_dispatcher';
1917
import { EventProcessor } from './event_processor';
2018
import { EventWithId } from './batch_event_processor';
@@ -23,6 +21,7 @@ import {
2321
BatchEventProcessorOptions,
2422
OpaqueEventProcessor,
2523
wrapEventProcessor,
24+
getForwardingEventProcessor,
2625
} from './event_processor_factory';
2726
import defaultEventDispatcher from './event_dispatcher/default_dispatcher.browser';
2827
import sendBeaconEventDispatcher from './event_dispatcher/send_beacon_dispatcher.browser';

lib/event_processor/event_processor_factory.node.spec.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,16 @@ vi.mock('./default_dispatcher.node', () => {
1919
return { default: {} };
2020
});
2121

22-
vi.mock('./forwarding_event_processor', () => {
23-
const getForwardingEventProcessor = vi.fn().mockReturnValue({});
24-
return { getForwardingEventProcessor };
25-
});
26-
2722
vi.mock('./event_processor_factory', async (importOriginal) => {
2823
const getBatchEventProcessor = vi.fn().mockImplementation(() => {
2924
return {};
3025
});
3126
const getOpaqueBatchEventProcessor = vi.fn().mockImplementation(() => {
3227
return {};
3328
});
29+
const getForwardingEventProcessor = vi.fn().mockReturnValue({});
3430
const original: any = await importOriginal();
35-
return { ...original, getBatchEventProcessor, getOpaqueBatchEventProcessor };
31+
return { ...original, getBatchEventProcessor, getOpaqueBatchEventProcessor, getForwardingEventProcessor };
3632
});
3733

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

4642
import { createBatchEventProcessor, createForwardingEventProcessor } from './event_processor_factory.node';
47-
import { getForwardingEventProcessor } from './forwarding_event_processor';
4843
import nodeDefaultEventDispatcher from './event_dispatcher/default_dispatcher.node';
49-
import { EVENT_STORE_PREFIX, extractEventProcessor, FAILED_EVENT_RETRY_INTERVAL } from './event_processor_factory';
44+
import { EVENT_STORE_PREFIX, extractEventProcessor, getForwardingEventProcessor, FAILED_EVENT_RETRY_INTERVAL } from './event_processor_factory';
5045
import { getOpaqueBatchEventProcessor } from './event_processor_factory';
5146
import { AsyncStore, AsyncPrefixStore, SyncStore, SyncPrefixStore } from '../utils/cache/store';
5247
import { AsyncStorageCache } from '../utils/cache/async_storage_cache.react_native';

lib/event_processor/event_processor_factory.node.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16-
import { getForwardingEventProcessor } from './forwarding_event_processor';
1716
import { EventDispatcher } from './event_dispatcher/event_dispatcher';
1817
import defaultEventDispatcher from './event_dispatcher/default_dispatcher.node';
1918
import {
@@ -23,6 +22,7 @@ import {
2322
getPrefixEventStore,
2423
OpaqueEventProcessor,
2524
wrapEventProcessor,
25+
getForwardingEventProcessor,
2626
} from './event_processor_factory';
2727

2828
export const DEFAULT_EVENT_BATCH_SIZE = 10;

0 commit comments

Comments
 (0)