From a37a8cb71891fde30e2436da7c47af55eb8cde5c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 14 Oct 2021 17:20:20 -0300 Subject: [PATCH 1/2] polishing and tests --- src/sdkClient/sdkClientMethodCS.ts | 4 +- src/sync/offline/LocalhostFromFile.ts | 9 +-- src/sync/offline/LocalhostFromObject.ts | 10 +-- .../localhost/__tests__/index.spec.ts | 62 +++++++++++++++++++ .../settingsValidation/localhost/builtin.ts | 16 +++++ .../localhost/{index.ts => pluggable.ts} | 7 ++- 6 files changed, 95 insertions(+), 13 deletions(-) create mode 100644 src/utils/settingsValidation/localhost/__tests__/index.spec.ts create mode 100644 src/utils/settingsValidation/localhost/builtin.ts rename src/utils/settingsValidation/localhost/{index.ts => pluggable.ts} (79%) diff --git a/src/sdkClient/sdkClientMethodCS.ts b/src/sdkClient/sdkClientMethodCS.ts index 80089705..aeb29720 100644 --- a/src/sdkClient/sdkClientMethodCS.ts +++ b/src/sdkClient/sdkClientMethodCS.ts @@ -59,7 +59,7 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? const sharedSdkReadiness = sdkReadinessManager.shared(readyTimeout); const sharedStorage = (storage as IStorageSyncCS).shared(matchingKey); - const sharedSyncManager = (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage); + const sharedSyncManager = syncManager && (syncManager as ISyncManagerCS).shared(matchingKey, sharedSdkReadiness.readinessManager, sharedStorage); // As shared clients reuse all the storage information, we don't need to check here if we // will use offline or online mode. We should stick with the original decision. @@ -74,7 +74,7 @@ export function sdkClientMethodCSFactory(params: ISdkClientFactoryParams): (key? validKey ); - sharedSyncManager.start(); + sharedSyncManager && sharedSyncManager.start(); log.info(NEW_SHARED_CLIENT); } else { diff --git a/src/sync/offline/LocalhostFromFile.ts b/src/sync/offline/LocalhostFromFile.ts index 2caff329..33e6c121 100644 --- a/src/sync/offline/LocalhostFromFile.ts +++ b/src/sync/offline/LocalhostFromFile.ts @@ -2,10 +2,11 @@ import { splitsParserFromFileFactory } from './splitsParser/splitsParserFromFile import { syncManagerOfflineFactory } from './syncManagerOffline'; import { SplitIO } from '../../types'; -// Factory of Localhost SyncManager based on yaml file. +// Singleton instance of the factory function for offline SyncManager from YAML file (a.k.a. localhostFromFile) // Requires Node 'fs' and 'path' APIs. +const localhostFromFile = syncManagerOfflineFactory(splitsParserFromFileFactory) as SplitIO.LocalhostFactory; +localhostFromFile.type = 'LocalhostFromFile'; + export function LocalhostFromFile(): SplitIO.LocalhostFactory { - const localhost = syncManagerOfflineFactory(splitsParserFromFileFactory) as SplitIO.LocalhostFactory; - localhost.type = 'LocalhostFromFile'; - return localhost; + return localhostFromFile; } diff --git a/src/sync/offline/LocalhostFromObject.ts b/src/sync/offline/LocalhostFromObject.ts index d1510996..b823e7ee 100644 --- a/src/sync/offline/LocalhostFromObject.ts +++ b/src/sync/offline/LocalhostFromObject.ts @@ -2,9 +2,11 @@ import { splitsParserFromSettingsFactory } from './splitsParser/splitsParserFrom import { syncManagerOfflineFactory } from './syncManagerOffline'; import { SplitIO } from '../../types'; -// Factory of Localhost SyncManager based on JS object. +// Singleton instance of the factory function for offline SyncManager from object (a.k.a. localhostFromObject) +// SDK instances instantiate their SyncManagers with the same factory +const localhostFromObject = syncManagerOfflineFactory(splitsParserFromSettingsFactory) as SplitIO.LocalhostFactory; +localhostFromObject.type = 'LocalhostFromObject'; + export function LocalhostFromObject(): SplitIO.LocalhostFactory { - const localhost = syncManagerOfflineFactory(splitsParserFromSettingsFactory) as SplitIO.LocalhostFactory; - localhost.type = 'LocalhostFromObject'; - return localhost; + return localhostFromObject; } diff --git a/src/utils/settingsValidation/localhost/__tests__/index.spec.ts b/src/utils/settingsValidation/localhost/__tests__/index.spec.ts new file mode 100644 index 00000000..663530dc --- /dev/null +++ b/src/utils/settingsValidation/localhost/__tests__/index.spec.ts @@ -0,0 +1,62 @@ +import { validateLocalhost } from '../pluggable'; +import { validateLocalhostWithDefault } from '../builtin'; +import { LocalhostFromObject } from '../../../../sync/offline/LocalhostFromObject'; +import { loggerMock as log } from '../../../../logger/__tests__/sdkLogger.mock'; + +const localhostModeObject = LocalhostFromObject(); + +describe('validateLocalhost, for slim SplitFactory', () => { + + afterEach(() => { + log.error.mockClear(); + }); + + test('if mode is LOCALHOST_MODE and localhostMode object is invalid or not provided, returns undefined and logs an error', () => { + expect(validateLocalhost({ log, sync: {}, mode: 'localhost' })).toBe(undefined); + expect(validateLocalhost({ log, sync: { localhostMode: null }, mode: 'localhost' })).toBe(undefined); + expect(validateLocalhost({ log, sync: { localhostMode: () => { } }, mode: 'localhost' })).toBe(undefined); + expect(log.error).toBeCalledTimes(3); // logs error if provided object is invalid + }); + + test('if mode is LOCALHOST_MODE and localhostMode object is valid, returns the provided object', () => { + expect(validateLocalhost({ log, sync: { localhostMode: localhostModeObject }, mode: 'localhost' })).toBe(localhostModeObject); + expect(log.error).not.toBeCalled(); + }); + + test('if mode is not LOCALHOST_MODE, returns the provided object (it is not validated)', () => { + expect(validateLocalhost({ log, sync: {}, mode: 'standalone' })).toBe(undefined); + expect(validateLocalhost({ log, sync: { localhostMode: 'INVALID_BUT_IGNORED' }, mode: 'standalone' })).toBe('INVALID_BUT_IGNORED'); + expect(log.error).not.toBeCalled(); + }); + +}); + +describe('validateLocalhostWithDefault, for full SplitFactory', () => { + + afterEach(() => { + log.error.mockClear(); + }); + + test('if mode is LOCALHOST_MODE and localhostMode object is not provided, returns default without logging an error', () => { + expect(validateLocalhostWithDefault({ log, sync: {}, mode: 'localhost' })).toBe(localhostModeObject); + expect(validateLocalhostWithDefault({ log, sync: { localhostMode: null }, mode: 'localhost' })).toBe(localhostModeObject); + expect(log.error).not.toBeCalled(); + }); + + test('if mode is LOCALHOST_MODE and localhostMode object is invalid, returns default and logs an error', () => { + expect(validateLocalhostWithDefault({ log, sync: { localhostMode: () => { } }, mode: 'localhost' })).toBe(localhostModeObject); + expect(log.error).toBeCalledTimes(1); // logs error if provided object is invalid + }); + + test('if mode is LOCALHOST_MODE and localhostMode object is valid, returns the provided object', () => { + expect(validateLocalhostWithDefault({ log, sync: { localhostMode: localhostModeObject }, mode: 'localhost' })).toBe(localhostModeObject); + expect(log.error).not.toBeCalled(); + }); + + test('if mode is not LOCALHOST_MODE, returns the provided object or the default one. Provided object is not validated and so no errors are logged', () => { + expect(validateLocalhostWithDefault({ log, sync: {}, mode: 'standalone' })).toBe(localhostModeObject); + expect(validateLocalhostWithDefault({ log, sync: { localhostMode: 'INVALID_BUT_IGNORED' }, mode: 'standalone' })).toBe('INVALID_BUT_IGNORED'); + expect(log.error).not.toBeCalled(); + }); + +}); diff --git a/src/utils/settingsValidation/localhost/builtin.ts b/src/utils/settingsValidation/localhost/builtin.ts new file mode 100644 index 00000000..35a30145 --- /dev/null +++ b/src/utils/settingsValidation/localhost/builtin.ts @@ -0,0 +1,16 @@ +import { ILogger } from '../../../logger/types'; +import { SDKMode, } from '../../../types'; +import { LocalhostFromObject } from '../../../sync/offline/LocalhostFromObject'; +import { validateLocalhost } from './pluggable'; + +/** + * This function validates `settings.sync.localhostMode` object + * + * @param {any} settings config object provided by the user to initialize the sdk + * + * @returns {Object} provided localhost mode module at `settings.sync.localhostMode` if valid, or a default LocalhostFromObject instance if not provided or invalid + */ +export function validateLocalhostWithDefault(settings: { log: ILogger, sync: { localhostMode?: any }, mode: SDKMode }) { + if (!settings.sync.localhostMode) return LocalhostFromObject(); + return validateLocalhost(settings) || LocalhostFromObject(); +} diff --git a/src/utils/settingsValidation/localhost/index.ts b/src/utils/settingsValidation/localhost/pluggable.ts similarity index 79% rename from src/utils/settingsValidation/localhost/index.ts rename to src/utils/settingsValidation/localhost/pluggable.ts index 7d5f9272..3231e612 100644 --- a/src/utils/settingsValidation/localhost/index.ts +++ b/src/utils/settingsValidation/localhost/pluggable.ts @@ -1,5 +1,6 @@ import { ERROR_LOCALHOST_MODULE_REQUIRED } from '../../../logger/constants'; -import { ISettings, } from '../../../types'; +import { ILogger } from '../../../logger/types'; +import { SDKMode, } from '../../../types'; import { LOCALHOST_MODE } from '../../constants'; /** @@ -7,9 +8,9 @@ import { LOCALHOST_MODE } from '../../constants'; * * @param {any} settings config object provided by the user to initialize the sdk * - * @returns {Object | undefined} provided localhost mode module at `settings.sync.localhostMode`, or undefined if it is not provided or invalid. + * @returns {Object | undefined} provided localhost mode module at `settings.sync.localhostMode`, or undefined if it is not provided or invalid */ -export function validateLocalhost(settings: ISettings) { +export function validateLocalhost(settings: { log: ILogger, sync: { localhostMode?: any}, mode: SDKMode }) { const localhostMode = settings.sync.localhostMode; // localhostMode.type is used for internal validation. Not considered part of the public API, and might be updated eventually. From 54e97fa5dcf07f6c4188d91178cb7ddd91b1a4b8 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 18 Oct 2021 12:16:48 -0300 Subject: [PATCH 2/2] updated readme --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index fd90814d..4d9dee55 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ To learn more about Split, contact hello@split.io, or get started with feature f Split has built and maintains SDKs for: -* .NET [Github](https://github.com/splitio/.net-core-client) [Docs](https://help.split.io/hc/en-us/articles/360020240172--NET-SDK) +* .NET [Github](https://github.com/splitio/dotnet-client) [Docs](https://help.split.io/hc/en-us/articles/360020240172--NET-SDK) * Android [Github](https://github.com/splitio/android-client) [Docs](https://help.split.io/hc/en-us/articles/360020343291-Android-SDK) * GO [Github](https://github.com/splitio/go-client) [Docs](https://help.split.io/hc/en-us/articles/360020093652-Go-SDK) * iOS [Github](https://github.com/splitio/ios-client) [Docs](https://help.split.io/hc/en-us/articles/360020401491-iOS-SDK) @@ -35,6 +35,7 @@ Split has built and maintains SDKs for: * PHP [Github](https://github.com/splitio/php-client) [Docs](https://help.split.io/hc/en-us/articles/360020350372-PHP-SDK) * Python [Github](https://github.com/splitio/python-client) [Docs](https://help.split.io/hc/en-us/articles/360020359652-Python-SDK) * React [Github](https://github.com/splitio/react-client) [Docs](https://help.split.io/hc/en-us/articles/360038825091-React-SDK) +* React Native [Github](https://github.com/splitio/react-native-client) [Docs](https://help.split.io/hc/en-us/articles/4406066357901-React-Native-SDK) * Redux [Github](https://github.com/splitio/redux-client) [Docs](https://help.split.io/hc/en-us/articles/360038851551-Redux-SDK) * Ruby [Github](https://github.com/splitio/ruby-client) [Docs](https://help.split.io/hc/en-us/articles/360020673251-Ruby-SDK)