Skip to content

[FSSDK-10993] additional cleanup for project config manager #977

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 5 commits into from
Dec 11, 2024
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
3 changes: 2 additions & 1 deletion lib/project_config/config_manager_factory.browser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ vi.mock('../utils/http_request_handler/browser_request_handler', () => {
import { getPollingConfigManager, PollingConfigManagerConfig, PollingConfigManagerFactoryOptions } from './config_manager_factory';
import { createPollingProjectConfigManager } from './config_manager_factory.browser';
import { BrowserRequestHandler } from '../utils/http_request_handler/browser_request_handler';
import { getMockSyncCache } from '../tests/mock/mock_cache';

describe('createPollingConfigManager', () => {
const mockGetPollingConfigManager = vi.mocked(getPollingConfigManager);
Expand Down Expand Up @@ -76,7 +77,7 @@ describe('createPollingConfigManager', () => {
autoUpdate: true,
urlTemplate: 'urlTemplate',
datafileAccessToken: 'datafileAccessToken',
cache: { get: vi.fn(), set: vi.fn(), contains: vi.fn(), remove: vi.fn() },
cache: getMockSyncCache<string>(),
};

const projectConfigManager = createPollingProjectConfigManager(config);
Expand Down
4 changes: 2 additions & 2 deletions lib/project_config/config_manager_factory.node.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ vi.mock('../utils/http_request_handler/node_request_handler', () => {
import { getPollingConfigManager, PollingConfigManagerConfig } from './config_manager_factory';
import { createPollingProjectConfigManager } from './config_manager_factory.node';
import { NodeRequestHandler } from '../utils/http_request_handler/node_request_handler';
import { DEFAULT_AUTHENTICATED_URL_TEMPLATE, DEFAULT_URL_TEMPLATE } from './constant';
import { getMockSyncCache } from '../tests/mock/mock_cache';

describe('createPollingConfigManager', () => {
const mockGetPollingConfigManager = vi.mocked(getPollingConfigManager);
Expand Down Expand Up @@ -77,7 +77,7 @@ describe('createPollingConfigManager', () => {
autoUpdate: false,
urlTemplate: 'urlTemplate',
datafileAccessToken: 'datafileAccessToken',
cache: { get: vi.fn(), set: vi.fn(), contains: vi.fn(), remove: vi.fn() },
cache: getMockSyncCache(),
};

const projectConfigManager = createPollingProjectConfigManager(config);
Expand Down
39 changes: 15 additions & 24 deletions lib/project_config/config_manager_factory.react_native.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ async function mockRequireAsyncStorage() {
M._load_original = M._load;
M._load = (uri: string, parent: string) => {
if (uri === '@react-native-async-storage/async-storage') {
if (isAsyncStorageAvailable) return {};
if (isAsyncStorageAvailable) return { default: {} };
throw new Error('Module not found: @react-native-async-storage/async-storage');
}
return M._load_original(uri, parent);
Expand All @@ -47,25 +47,30 @@ vi.mock('../utils/http_request_handler/browser_request_handler', () => {
return { BrowserRequestHandler };
});

vi.mock('../plugins/key_value_cache/reactNativeAsyncStorageCache', () => {
const ReactNativeAsyncStorageCache = vi.fn();
return { default: ReactNativeAsyncStorageCache };
vi.mock('../utils/cache/async_storage_cache.react_native', async (importOriginal) => {
const original: any = await importOriginal();
const OriginalAsyncStorageCache = original.AsyncStorageCache;
const MockAsyncStorageCache = vi.fn().mockImplementation(function (this: any, ...args) {
Object.setPrototypeOf(this, new OriginalAsyncStorageCache(...args));
});
return { AsyncStorageCache: MockAsyncStorageCache };
});

import { getPollingConfigManager, PollingConfigManagerConfig } from './config_manager_factory';
import { createPollingProjectConfigManager } from './config_manager_factory.react_native';
import { BrowserRequestHandler } from '../utils/http_request_handler/browser_request_handler';
import ReactNativeAsyncStorageCache from '../plugins/key_value_cache/reactNativeAsyncStorageCache';
import { AsyncStorageCache } from '../utils/cache/async_storage_cache.react_native';
import { getMockSyncCache } from '../tests/mock/mock_cache';

describe('createPollingConfigManager', () => {
const mockGetPollingConfigManager = vi.mocked(getPollingConfigManager);
const MockBrowserRequestHandler = vi.mocked(BrowserRequestHandler);
const MockReactNativeAsyncStorageCache = vi.mocked(ReactNativeAsyncStorageCache);
const MockAsyncStorageCache = vi.mocked(AsyncStorageCache);

beforeEach(() => {
mockGetPollingConfigManager.mockClear();
MockBrowserRequestHandler.mockClear();
MockReactNativeAsyncStorageCache.mockClear();
MockAsyncStorageCache.mockClear();
});

it('creates and returns the instance by calling getPollingConfigManager', () => {
Expand Down Expand Up @@ -110,7 +115,7 @@ describe('createPollingConfigManager', () => {
createPollingProjectConfigManager(config);

expect(
Object.is(mockGetPollingConfigManager.mock.calls[0][0].cache, MockReactNativeAsyncStorageCache.mock.instances[0])
Object.is(mockGetPollingConfigManager.mock.calls[0][0].cache, MockAsyncStorageCache.mock.instances[0])
).toBe(true);
});

Expand All @@ -123,7 +128,7 @@ describe('createPollingConfigManager', () => {
autoUpdate: false,
urlTemplate: 'urlTemplate',
datafileAccessToken: 'datafileAccessToken',
cache: { get: vi.fn(), set: vi.fn(), contains: vi.fn(), remove: vi.fn() },
cache: getMockSyncCache(),
};

createPollingProjectConfigManager(config);
Expand All @@ -133,38 +138,24 @@ describe('createPollingConfigManager', () => {

it('Should not throw error if a cache is present in the config, and async storage is not available', async () => {
isAsyncStorageAvailable = false;
const { default: ReactNativeAsyncStorageCache } = await vi.importActual<
typeof import('../plugins/key_value_cache/reactNativeAsyncStorageCache')
>('../plugins/key_value_cache/reactNativeAsyncStorageCache');
const config = {
sdkKey: 'sdkKey',
requestHandler: { makeRequest: vi.fn() },
cache: { get: vi.fn(), set: vi.fn(), contains: vi.fn(), remove: vi.fn() },
cache: getMockSyncCache<string>(),
};

MockReactNativeAsyncStorageCache.mockImplementationOnce(() => {
return new ReactNativeAsyncStorageCache();
});

expect(() => createPollingProjectConfigManager(config)).not.toThrow();
isAsyncStorageAvailable = true;
});

it('should throw an error if cache is not present in the config, and async storage is not available', async () => {
isAsyncStorageAvailable = false;

const { default: ReactNativeAsyncStorageCache } = await vi.importActual<
typeof import('../plugins/key_value_cache/reactNativeAsyncStorageCache')
>('../plugins/key_value_cache/reactNativeAsyncStorageCache');
const config = {
sdkKey: 'sdkKey',
requestHandler: { makeRequest: vi.fn() },
};

MockReactNativeAsyncStorageCache.mockImplementationOnce(() => {
return new ReactNativeAsyncStorageCache();
});

expect(() => createPollingProjectConfigManager(config)).toThrowError(
'Module not found: @react-native-async-storage/async-storage'
);
Expand Down
4 changes: 2 additions & 2 deletions lib/project_config/config_manager_factory.react_native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
import { getPollingConfigManager, PollingConfigManagerConfig } from "./config_manager_factory";
import { BrowserRequestHandler } from "../utils/http_request_handler/browser_request_handler";
import { ProjectConfigManager } from "./project_config_manager";
import ReactNativeAsyncStorageCache from "../plugins/key_value_cache/reactNativeAsyncStorageCache";
import { AsyncStorageCache } from "../utils/cache/async_storage_cache.react_native";

export const createPollingProjectConfigManager = (config: PollingConfigManagerConfig): ProjectConfigManager => {
const defaultConfig = {
autoUpdate: true,
requestHandler: new BrowserRequestHandler(),
cache: config.cache || new ReactNativeAsyncStorageCache()
cache: config.cache || new AsyncStorageCache(),
};

return getPollingConfigManager({ ...defaultConfig, ...config });
Expand Down
34 changes: 30 additions & 4 deletions lib/project_config/config_manager_factory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ import { ProjectConfigManagerImpl } from './project_config_manager';
import { PollingDatafileManager } from './polling_datafile_manager';
import { ExponentialBackoff, IntervalRepeater } from '../utils/repeater/repeater';
import { getPollingConfigManager } from './config_manager_factory';
import { DEFAULT_UPDATE_INTERVAL } from './constant';
import { DEFAULT_UPDATE_INTERVAL, UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE } from './constant';
import { getMockSyncCache } from '../tests/mock/mock_cache';
import { LogLevel } from '../modules/logging';

describe('getPollingConfigManager', () => {
const MockProjectConfigManagerImpl = vi.mocked(ProjectConfigManagerImpl);
Expand Down Expand Up @@ -73,7 +75,32 @@ describe('getPollingConfigManager', () => {
};
getPollingConfigManager(config);
expect(MockIntervalRepeater.mock.calls[0][0]).toBe(DEFAULT_UPDATE_INTERVAL);
expect(MockPollingDatafileManager.mock.calls[0][0].updateInterval).toBe(DEFAULT_UPDATE_INTERVAL);
});

it('adds a startup log if the update interval is below the minimum', () => {
const config = {
sdkKey: 'abcd',
requestHandler: { makeRequest: vi.fn() },
updateInterval: 10000,
};
getPollingConfigManager(config);
const startupLogs = MockPollingDatafileManager.mock.calls[0][0].startupLogs;
expect(startupLogs).toEqual(expect.arrayContaining([{
level: LogLevel.WARNING,
message: UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE,
params: [],
}]));
});

it('does not add any startup log if the update interval above the minimum', () => {
const config = {
sdkKey: 'abcd',
requestHandler: { makeRequest: vi.fn() },
updateInterval: 40000,
};
getPollingConfigManager(config);
const startupLogs = MockPollingDatafileManager.mock.calls[0][0].startupLogs;
expect(startupLogs).toEqual([]);
});

it('uses the provided options', () => {
Expand All @@ -86,7 +113,7 @@ describe('getPollingConfigManager', () => {
autoUpdate: true,
urlTemplate: 'urlTemplate',
datafileAccessToken: 'datafileAccessToken',
cache: { get: vi.fn(), set: vi.fn(), contains: vi.fn(), remove: vi.fn() },
cache: getMockSyncCache<string>(),
};

getPollingConfigManager(config);
Expand All @@ -96,7 +123,6 @@ describe('getPollingConfigManager', () => {
expect(MockPollingDatafileManager).toHaveBeenNthCalledWith(1, expect.objectContaining({
sdkKey: config.sdkKey,
autoUpdate: config.autoUpdate,
updateInterval: config.updateInterval,
urlTemplate: config.urlTemplate,
datafileAccessToken: config.datafileAccessToken,
requestHandler: config.requestHandler,
Expand Down
19 changes: 16 additions & 3 deletions lib/project_config/config_manager_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@ import { Transformer } from "../utils/type";
import { DatafileManagerConfig } from "./datafile_manager";
import { ProjectConfigManagerImpl, ProjectConfigManager } from "./project_config_manager";
import { PollingDatafileManager } from "./polling_datafile_manager";
import PersistentKeyValueCache from "../plugins/key_value_cache/persistentKeyValueCache";
import { Cache } from "../utils/cache/cache";
import { DEFAULT_UPDATE_INTERVAL } from './constant';
import { ExponentialBackoff, IntervalRepeater } from "../utils/repeater/repeater";
import { StartupLog } from "../service";
import { MIN_UPDATE_INTERVAL, UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE } from './constant';
import { LogLevel } from "../modules/logging";

export type StaticConfigManagerConfig = {
datafile: string,
Expand All @@ -42,7 +45,7 @@ export type PollingConfigManagerConfig = {
updateInterval?: number;
urlTemplate?: string;
datafileAccessToken?: string;
cache?: PersistentKeyValueCache;
cache?: Cache<string>;
};

export type PollingConfigManagerFactoryOptions = PollingConfigManagerConfig & { requestHandler: RequestHandler };
Expand All @@ -55,15 +58,25 @@ export const getPollingConfigManager = (
const backoff = new ExponentialBackoff(1000, updateInterval, 500);
const repeater = new IntervalRepeater(updateInterval, backoff);

const startupLogs: StartupLog[] = []

if (updateInterval < MIN_UPDATE_INTERVAL) {
startupLogs.push({
level: LogLevel.WARNING,
message: UPDATE_INTERVAL_BELOW_MINIMUM_MESSAGE,
params: [],
});
}

const datafileManagerConfig: DatafileManagerConfig = {
sdkKey: opt.sdkKey,
autoUpdate: opt.autoUpdate,
updateInterval: updateInterval,
urlTemplate: opt.urlTemplate,
datafileAccessToken: opt.datafileAccessToken,
requestHandler: opt.requestHandler,
cache: opt.cache,
repeater,
startupLogs,
};

const datafileManager = new PollingDatafileManager(datafileManagerConfig);
Expand Down
9 changes: 4 additions & 5 deletions lib/project_config/datafile_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Service } from '../service';
import PersistentKeyValueCache from '../plugins/key_value_cache/persistentKeyValueCache';
import { Service, StartupLog } from '../service';
import { Cache } from '../utils/cache/cache';
import { RequestHandler } from '../utils/http_request_handler/http';
import { Fn, Consumer } from '../utils/type';
import { Repeater } from '../utils/repeater/repeater';
Expand All @@ -30,12 +30,11 @@ export type DatafileManagerConfig = {
requestHandler: RequestHandler;
autoUpdate?: boolean;
sdkKey: string;
/** Polling interval in milliseconds to check for datafile updates. */
updateInterval?: number;
urlTemplate?: string;
cache?: PersistentKeyValueCache;
cache?: Cache<string>;
datafileAccessToken?: string;
initRetry?: number;
repeater: Repeater;
logger?: LoggerFacade;
startupLogs?: StartupLog[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ import { assert } from 'chai';
import { cloneDeep } from 'lodash';
import sinon from 'sinon';

import { createOptimizelyConfig, OptimizelyConfig } from './';
import { createProjectConfig } from '../../project_config/project_config';
import { createOptimizelyConfig, OptimizelyConfig } from './optimizely_config';
import { createProjectConfig } from './project_config';
import {
getTestProjectConfigWithFeatures,
getTypedAudiencesConfig,
getSimilarRuleKeyConfig,
getSimilarExperimentKeyConfig,
getDuplicateExperimentKeyConfig,
} from '../../tests/test_data';
} from '../tests/test_data';

var datafile = getTestProjectConfigWithFeatures();
var typedAudienceDatafile = getTypedAudiencesConfig();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { LoggerFacade, getLogger } from '../../modules/logging';
import { ProjectConfig } from '../../project_config/project_config';
import { DEFAULT_OPERATOR_TYPES } from '../condition_tree_evaluator';
import { LoggerFacade, getLogger } from '../modules/logging';
import { ProjectConfig } from '../project_config/project_config';
import { DEFAULT_OPERATOR_TYPES } from '../core/condition_tree_evaluator';
import {
Audience,
Experiment,
Expand All @@ -32,7 +32,7 @@ import {
Rollout,
Variation,
VariationVariable,
} from '../../shared_types';
} from '../shared_types';

interface FeatureVariablesMap {
[key: string]: FeatureVariable[];
Expand Down
Loading
Loading