From 8bb436da5f31d004c598ebd8cfa2a7eee416ea36 Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 01:29:51 +0300 Subject: [PATCH 01/13] feat: add mergeStrategy to RemoteEnv --- npm/ng-packs/packages/core/src/lib/models/config.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/npm/ng-packs/packages/core/src/lib/models/config.ts b/npm/ng-packs/packages/core/src/lib/models/config.ts index e3d6f6ab70..5f19907388 100644 --- a/npm/ng-packs/packages/core/src/lib/models/config.ts +++ b/npm/ng-packs/packages/core/src/lib/models/config.ts @@ -42,9 +42,14 @@ export namespace Config { } export type LocalizationParam = string | LocalizationWithDefault; + export type customMergeFn = ( + localEnv: Partial, + remoteEnv: any, + ) => Config.Environment; export interface RemoteEnv { url: string; + mergeStrategy: 'deepmerge' | 'overwrite' | customMergeFn; method?: string; headers?: ABP.Dictionary; } From c05fc996cfc69c984e859d8ba723d0e7c957ca55 Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 01:30:09 +0300 Subject: [PATCH 02/13] feat: add object-utils with deepMerge --- .../core/src/lib/utils/object-utils.ts | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 npm/ng-packs/packages/core/src/lib/utils/object-utils.ts diff --git a/npm/ng-packs/packages/core/src/lib/utils/object-utils.ts b/npm/ng-packs/packages/core/src/lib/utils/object-utils.ts new file mode 100644 index 0000000000..1dd21b208f --- /dev/null +++ b/npm/ng-packs/packages/core/src/lib/utils/object-utils.ts @@ -0,0 +1,51 @@ +export function deepMerge(target, source) { + if (isObjectAndNotArray(target) && isObjectAndNotArray(source)) { + return deepMergeRecursively(target, source); + } else if (isNullOrUndefined(target) && isNullOrUndefined(source)) { + return {}; + } else { + return isNullOrUndefined(source) ? target : source; + } +} + +function deepMergeRecursively(target, source) { + const shouldNotRecurse = + isNullOrUndefined(target) || + isNullOrUndefined(source) || // at least one not defined + isArray(target) || + isArray(source) || // at least one array + !isObject(target) || + !isObject(source); // at least one not an object; + + /** + * if we will not recurse any further, + * we will prioritize source if it is a defined value. + */ + if (shouldNotRecurse) { + return !isNullOrUndefined(source) ? source : target; + } + + const keysOfTarget = Object.keys(target); + const keysOfSource = Object.keys(source); + const uniqueKeys = new Set(keysOfTarget.concat(keysOfSource)); + return [...uniqueKeys].reduce((retVal, key) => { + retVal[key] = deepMergeRecursively(target[key], source[key]); + return retVal; + }, {}); +} + +function isNullOrUndefined(obj) { + return obj === null || obj === undefined; +} + +function isObject(obj) { + return obj instanceof Object; +} + +function isArray(obj) { + return Array.isArray(obj); +} + +function isObjectAndNotArray(obj) { + return isObject(obj) && !isArray(obj); +} From d187ebaa6f92dbd9ab6cd98e7f810b21aa6eac9a Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 01:30:43 +0300 Subject: [PATCH 03/13] feat: apply merge strategy on env --- .../core/src/lib/utils/environment-utils.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/npm/ng-packs/packages/core/src/lib/utils/environment-utils.ts b/npm/ng-packs/packages/core/src/lib/utils/environment-utils.ts index c8d92603f3..b8c182bcb4 100644 --- a/npm/ng-packs/packages/core/src/lib/utils/environment-utils.ts +++ b/npm/ng-packs/packages/core/src/lib/utils/environment-utils.ts @@ -5,6 +5,7 @@ import { catchError, switchMap } from 'rxjs/operators'; import { SetEnvironment } from '../actions/config.actions'; import { Config } from '../models/config'; import { RestOccurError } from '../actions/rest.actions'; +import { deepMerge } from './object-utils'; export function getRemoteEnv(injector: Injector, environment: Partial) { const { remoteEnv } = environment; @@ -18,7 +19,22 @@ export function getRemoteEnv(injector: Injector, environment: Partial(method, url, { headers }) .pipe( catchError(err => store.dispatch(new RestOccurError(err))), // TODO: Condiser get handle function from a provider - switchMap(env => store.dispatch(new SetEnvironment({ ...environment, ...env }))), + switchMap(env => store.dispatch(mergeEnvironments(environment, env, remoteEnv))), ) .toPromise(); } + +function mergeEnvironments( + local: Partial, + remote: any, + config: Config.RemoteEnv, +) { + switch (config.mergeStrategy) { + case 'deepmerge': + return new SetEnvironment(deepMerge(local, remote)); + case 'overwrite': + return new SetEnvironment(remote); + default: + return new SetEnvironment(config.mergeStrategy(local, remote)); + } +} From e9910d5970c7712caceafeab83b2a1d1a229bcc4 Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 01:30:59 +0300 Subject: [PATCH 04/13] test: env-utils and object-utils --- .../src/lib/tests/environment-utils.spec.ts | 91 +++++++---- .../core/src/lib/tests/object-utils.spec.ts | 150 ++++++++++++++++++ 2 files changed, 209 insertions(+), 32 deletions(-) create mode 100644 npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts diff --git a/npm/ng-packs/packages/core/src/lib/tests/environment-utils.spec.ts b/npm/ng-packs/packages/core/src/lib/tests/environment-utils.spec.ts index ab7cc0df0c..5599b61782 100644 --- a/npm/ng-packs/packages/core/src/lib/tests/environment-utils.spec.ts +++ b/npm/ng-packs/packages/core/src/lib/tests/environment-utils.spec.ts @@ -5,29 +5,8 @@ import { Store } from '@ngxs/store'; import { BehaviorSubject, of } from 'rxjs'; import { getRemoteEnv } from '../utils/environment-utils'; import { SetEnvironment } from '../actions/config.actions'; - -const environment = { - production: false, - hmr: false, - application: { - baseUrl: 'https://volosoft.com', - name: 'MyProjectName', - logoUrl: '', - }, - oAuthConfig: { - issuer: 'https://api.volosoft.com', - clientId: 'MyProjectName_App', - dummyClientSecret: '1q2w3e*', - scope: 'MyProjectName', - oidc: false, - requireHttps: true, - }, - apis: { - default: { - url: 'https://api.volosoft.com', - }, - }, -}; +import { Config } from '../models/config'; +import { deepMerge } from '../utils/object-utils'; @Component({ selector: 'abp-dummy', @@ -44,8 +23,58 @@ describe('EnvironmentUtils', () => { beforeEach(() => (spectator = createComponent())); - describe('#getRemoteEnv', async () => { - test('should call the remoteEnv URL and dispatch the SetEnvironment action ', async () => { + describe('#getRemoteEnv', () => { + const environment: Config.Environment = { + production: false, + hmr: false, + application: { + baseUrl: 'https://volosoft.com', + name: 'MyProjectName', + logoUrl: '', + }, + remoteEnv: { url: '/assets/appsettings.json', mergeStrategy: 'deepmerge' }, + oAuthConfig: { + issuer: 'https://api.volosoft.com', + clientId: 'MyProjectName_App', + dummyClientSecret: '1q2w3e*', + scope: 'MyProjectName', + oidc: false, + requireHttps: true, + }, + apis: { + default: { + url: 'https://api.volosoft.com', + }, + }, + }; + + const customEnv = { + application: { + baseUrl: 'https://custom-volosoft.com', + name: 'Custom-MyProjectName', + logoUrl: 'https://logourl/', + }, + apis: { + default: { + url: 'https://test-api.volosoft.com', + }, + }, + }; + + it('should call the remoteEnv URL and dispatch the SetEnvironment action for overwrite', () => { + setupTestAndRun({ mergeStrategy: 'deepmerge' }, deepMerge(environment, customEnv)); + }); + + it('should call the remoteEnv URL and dispatch the SetEnvironment action for deepmerge', () => { + setupTestAndRun({ mergeStrategy: 'overwrite' }, customEnv); + }); + + it('should call the remoteEnv URL and dispatch the SetEnvironment action for customFn', () => { + const someEnv = { apiUrl: 'https://some-api-url' } as any; + setupTestAndRun({ mergeStrategy: (_, __) => someEnv }, someEnv); + }); + + function setupTestAndRun(strategy: Pick, expectedValue) { const injector = spectator.inject(Injector); const injectorSpy = jest.spyOn(injector, 'get'); const store = spectator.inject(Store); @@ -56,16 +85,14 @@ describe('EnvironmentUtils', () => { injectorSpy.mockReturnValueOnce(http); injectorSpy.mockReturnValueOnce(store); - requestSpy.mockReturnValue(new BehaviorSubject(environment)); + requestSpy.mockReturnValue(new BehaviorSubject(customEnv)); dispatchSpy.mockReturnValue(of(true)); - const partialEnv = { remoteEnv: { url: '/assets/appsettings.json' } }; - getRemoteEnv(injector, partialEnv); + environment.remoteEnv.mergeStrategy = strategy.mergeStrategy; + getRemoteEnv(injector, environment); expect(requestSpy).toHaveBeenCalledWith('GET', '/assets/appsettings.json', { headers: {} }); - expect(dispatchSpy).toHaveBeenCalledWith( - new SetEnvironment({ ...environment, ...partialEnv }), - ); - }); + expect(dispatchSpy).toHaveBeenCalledWith(new SetEnvironment(expectedValue)); + } }); }); diff --git a/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts b/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts new file mode 100644 index 0000000000..efa1e73e19 --- /dev/null +++ b/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts @@ -0,0 +1,150 @@ +import { deepMerge } from '../utils/object-utils'; + +describe('DeepMerge', () => { + it('should return empty object when both inputs are null or undefined', () => { + expect(deepMerge(undefined, undefined)).toEqual({}); + expect(deepMerge(undefined, null)).toEqual({}); + expect(deepMerge(null, undefined)).toEqual({}); + expect(deepMerge(null, null)).toEqual({}); + }); + + it('should correctly return when any of the inputs is null or undefined', () => { + const differentTestValues = [10, false, '', 'test-string', { a: 1 }, [1, 2, 3], {}]; + differentTestValues.forEach(val => { + expect(deepMerge(undefined, val)).toEqual(val); + expect(deepMerge(null, val)).toEqual(val); + expect(deepMerge(val, undefined)).toEqual(val); + expect(deepMerge(val, null)).toEqual(val); + }); + }); + + it('should correctly return source if one of them is primitive', () => { + const differentTestValues = [ + { + target: 10, + source: false, + }, + { + target: false, + source: 20, + }, + { + target: 'string', + source: { a: 5 }, + }, + { + target: { b: 10 }, + source: 50, + }, + { + target: [1, 2, 3], + source: 40, + }, + { + target: { k: 60 }, + source: [4, 5, 6], + }, + ]; + + differentTestValues.forEach(val => + expect(deepMerge(val.target, val.source)).toEqual(val.source), + ); + }); + + it('should correctly return when both inputs are objects with different fields', () => { + const target = { a: 1 }; + const source = { b: 2 }; + const expected = { a: 1, b: 2 }; + expect(deepMerge(target, source)).toEqual(expected); + expect(deepMerge(source, target)).toEqual(expected); + }); + + it('should correctly return when both inputs are object with same fields but different values', () => { + const target = { a: 1 }; + const source = { a: 5 }; + expect(deepMerge(target, source)).toEqual(source); + expect(deepMerge(source, target)).toEqual(target); + }); + + it('should correctly merge shallow objects with different fields as well as some shared ones', () => { + const target = { a: 1, b: 2, c: 3 }; + const source = { a: 4, d: 5, e: 6 }; + expect(deepMerge(target, source)).toEqual({ a: 4, b: 2, c: 3, d: 5, e: 6 }); + }); + + it('should merge arrays', () => { + const firstArray = [1, 2, 3]; + const secondArray = [3, 4, 5, 6]; + expect(deepMerge(firstArray, secondArray)).toEqual(secondArray); + const target = { a: firstArray }; + const source = { a: secondArray }; + expect(deepMerge(target, source)).toEqual({ a: secondArray }); + }); + + it('should correctly merge nested objects', () => { + const target = { + a: { + b: { + c: { + d: 1, + g: 10, + q: undefined, + t: false, + }, + e: { + f: [1, 2, 3], + p: 'other-string', + }, + }, + }, + x: { + q: 'some-string', + }, + }; + const source = { + a: { + b: { + c: { + h: 30, + q: 45, + t: null, + }, + m: 20, + e: { + f: [20, 30, 40], + }, + }, + e: { k: [5, 6] }, + }, + z: { + y: true, + }, + }; + const expected = { + a: { + b: { + c: { + d: 1, + g: 10, + h: 30, + q: 45, + t: false, + }, + e: { + f: [20, 30, 40], + p: 'other-string', + }, + m: 20, + }, + e: { k: [5, 6] }, + }, + x: { + q: 'some-string', + }, + z: { + y: true, + }, + }; + expect(deepMerge(target, source)).toEqual(expected); + }); +}); From 7efebcb0618ea900635cd3145f07905eb997232f Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 08:29:04 +0300 Subject: [PATCH 05/13] test: improve test names of object-utils --- npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts b/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts index efa1e73e19..d234fe0132 100644 --- a/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts +++ b/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts @@ -18,7 +18,7 @@ describe('DeepMerge', () => { }); }); - it('should correctly return source if one of them is primitive', () => { + it('should correctly return source if one of them is primitive or an array', () => { const differentTestValues = [ { target: 10, @@ -72,7 +72,7 @@ describe('DeepMerge', () => { expect(deepMerge(target, source)).toEqual({ a: 4, b: 2, c: 3, d: 5, e: 6 }); }); - it('should merge arrays', () => { + it('should not merge arrays and return the latter', () => { const firstArray = [1, 2, 3]; const secondArray = [3, 4, 5, 6]; expect(deepMerge(firstArray, secondArray)).toEqual(secondArray); From dee43694b1cd4ced7eae04996c7c07d1c1051730 Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 08:37:09 +0300 Subject: [PATCH 06/13] refactor: improve readability of bool exp --- .../packages/core/src/lib/utils/object-utils.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/npm/ng-packs/packages/core/src/lib/utils/object-utils.ts b/npm/ng-packs/packages/core/src/lib/utils/object-utils.ts index 1dd21b208f..d4b475b7da 100644 --- a/npm/ng-packs/packages/core/src/lib/utils/object-utils.ts +++ b/npm/ng-packs/packages/core/src/lib/utils/object-utils.ts @@ -4,7 +4,7 @@ export function deepMerge(target, source) { } else if (isNullOrUndefined(target) && isNullOrUndefined(source)) { return {}; } else { - return isNullOrUndefined(source) ? target : source; + return exists(source) ? source : target; } } @@ -15,14 +15,14 @@ function deepMergeRecursively(target, source) { isArray(target) || isArray(source) || // at least one array !isObject(target) || - !isObject(source); // at least one not an object; + !isObject(source); // at least one not an object /** * if we will not recurse any further, * we will prioritize source if it is a defined value. */ if (shouldNotRecurse) { - return !isNullOrUndefined(source) ? source : target; + return exists(source) ? source : target; } const keysOfTarget = Object.keys(target); @@ -38,6 +38,10 @@ function isNullOrUndefined(obj) { return obj === null || obj === undefined; } +function exists(obj) { + return !isNullOrUndefined(obj); +} + function isObject(obj) { return obj instanceof Object; } From 69f15963c8613908377a0851806640e873383ff2 Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 10:53:09 +0300 Subject: [PATCH 07/13] refactor: move some funcs to common-utils --- .../core/src/lib/utils/common-utils.ts | 20 +++++++++++++++++ .../core/src/lib/utils/object-utils.ts | 22 ++----------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/npm/ng-packs/packages/core/src/lib/utils/common-utils.ts b/npm/ng-packs/packages/core/src/lib/utils/common-utils.ts index b8609be130..fb634d6dda 100644 --- a/npm/ng-packs/packages/core/src/lib/utils/common-utils.ts +++ b/npm/ng-packs/packages/core/src/lib/utils/common-utils.ts @@ -7,3 +7,23 @@ export function noop() { export function isUndefinedOrEmptyString(value: unknown): boolean { return value === undefined || value === ''; } + +export function isNullOrUndefined(obj) { + return obj === null || obj === undefined; +} + +export function exists(obj) { + return !isNullOrUndefined(obj); +} + +export function isObject(obj) { + return obj instanceof Object; +} + +export function isArray(obj) { + return Array.isArray(obj); +} + +export function isObjectAndNotArray(obj) { + return isObject(obj) && !isArray(obj); +} diff --git a/npm/ng-packs/packages/core/src/lib/utils/object-utils.ts b/npm/ng-packs/packages/core/src/lib/utils/object-utils.ts index d4b475b7da..b38c8a62ef 100644 --- a/npm/ng-packs/packages/core/src/lib/utils/object-utils.ts +++ b/npm/ng-packs/packages/core/src/lib/utils/object-utils.ts @@ -1,3 +1,5 @@ +import { isObjectAndNotArray, isNullOrUndefined, exists, isArray, isObject } from './common-utils'; + export function deepMerge(target, source) { if (isObjectAndNotArray(target) && isObjectAndNotArray(source)) { return deepMergeRecursively(target, source); @@ -33,23 +35,3 @@ function deepMergeRecursively(target, source) { return retVal; }, {}); } - -function isNullOrUndefined(obj) { - return obj === null || obj === undefined; -} - -function exists(obj) { - return !isNullOrUndefined(obj); -} - -function isObject(obj) { - return obj instanceof Object; -} - -function isArray(obj) { - return Array.isArray(obj); -} - -function isObjectAndNotArray(obj) { - return isObject(obj) && !isArray(obj); -} From 73cd8fe140a8866fb5931d47f7dcee2ae55919dd Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 10:53:54 +0300 Subject: [PATCH 08/13] test: test suites for new func set in common-utils --- .../core/src/lib/tests/common-utils.spec.ts | 96 ++++++++++++++++++- 1 file changed, 94 insertions(+), 2 deletions(-) diff --git a/npm/ng-packs/packages/core/src/lib/tests/common-utils.spec.ts b/npm/ng-packs/packages/core/src/lib/tests/common-utils.spec.ts index 60280e396b..359a521513 100644 --- a/npm/ng-packs/packages/core/src/lib/tests/common-utils.spec.ts +++ b/npm/ng-packs/packages/core/src/lib/tests/common-utils.spec.ts @@ -1,4 +1,12 @@ -import { isUndefinedOrEmptyString, noop } from '../utils'; +import { + isUndefinedOrEmptyString, + noop, + isNullOrUndefined, + exists, + isObject, + isArray, + isObjectAndNotArray, +} from '../utils'; describe('CommonUtils', () => { describe('#noop', () => { @@ -8,7 +16,7 @@ describe('CommonUtils', () => { }); }); - describe('#isUndefinedOrEmptyString', () => { + describe('#isUndefinedOrEmptyString & #exists', () => { test.each` value | expected ${null} | ${false} @@ -23,4 +31,88 @@ describe('CommonUtils', () => { expect(isUndefinedOrEmptyString(value)).toBe(expected); }); }); + + describe('#isNullOrUndefined & #exists', () => { + test.each` + value | expected + ${null} | ${true} + ${undefined} | ${true} + ${true} | ${false} + ${false} | ${false} + ${''} | ${false} + ${'test'} | ${false} + ${0} | ${false} + ${10} | ${false} + ${[]} | ${false} + ${[1, 2]} | ${false} + ${{}} | ${false} + ${{ a: 1 }} | ${false} + `( + 'should return $expected and !$expected (for #exists) when given parameter is $value', + ({ value, expected }) => { + expect(isNullOrUndefined(value)).toBe(expected); + expect(exists(value)).toBe(!expected); + }, + ); + }); + + describe('#isObject', () => { + test.each` + value | expected + ${null} | ${false} + ${undefined} | ${false} + ${true} | ${false} + ${false} | ${false} + ${''} | ${false} + ${'test'} | ${false} + ${0} | ${false} + ${10} | ${false} + ${[]} | ${true} + ${[1, 2]} | ${true} + ${{}} | ${true} + ${{ a: 1 }} | ${true} + `('should return $expected when given parameter is $value', ({ value, expected }) => { + expect(isObject(value)).toBe(expected); + }); + }); + + describe('#isArray', () => { + test.each` + value | expected + ${null} | ${false} + ${undefined} | ${false} + ${true} | ${false} + ${false} | ${false} + ${''} | ${false} + ${'test'} | ${false} + ${0} | ${false} + ${10} | ${false} + ${[]} | ${true} + ${[1, 2]} | ${true} + ${{}} | ${false} + ${{ a: 1 }} | ${false} + `('should return $expected when given parameter is $value', ({ value, expected }) => { + expect(isArray(value)).toBe(expected); + }); + }); + + describe('#isObjectAndNotArray', () => { + test.each` + value | expected + ${null} | ${false} + ${undefined} | ${false} + ${true} | ${false} + ${false} | ${false} + ${''} | ${false} + ${'test'} | ${false} + ${0} | ${false} + ${10} | ${false} + ${[]} | ${false} + ${[1, 2]} | ${false} + ${{}} | ${true} + ${{ a: 1 }} | ${true} + `('should return $expected when given parameter is $value', ({ value, expected }) => { + expect(isObjectAndNotArray(value)).toBe(expected); + }); + }); }); From 849b62669355d24b51fdd27a88a9d7fe4f410119 Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 10:54:15 +0300 Subject: [PATCH 09/13] feat: expose object-utils to outside world --- npm/ng-packs/packages/core/src/lib/utils/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/npm/ng-packs/packages/core/src/lib/utils/index.ts b/npm/ng-packs/packages/core/src/lib/utils/index.ts index 87b84ef800..383261fc07 100644 --- a/npm/ng-packs/packages/core/src/lib/utils/index.ts +++ b/npm/ng-packs/packages/core/src/lib/utils/index.ts @@ -10,6 +10,7 @@ export * from './lazy-load-utils'; export * from './localization-utils'; export * from './multi-tenancy-utils'; export * from './number-utils'; +export * from './object-utils'; export * from './route-utils'; export * from './rxjs-utils'; export * from './string-utils'; From f8bbc46e36ac44d449014e8ff276c7375981ee3a Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 10:54:38 +0300 Subject: [PATCH 10/13] fix: add null or undefined cases --- npm/ng-packs/packages/core/src/lib/utils/environment-utils.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/npm/ng-packs/packages/core/src/lib/utils/environment-utils.ts b/npm/ng-packs/packages/core/src/lib/utils/environment-utils.ts index b8c182bcb4..47a8f18b42 100644 --- a/npm/ng-packs/packages/core/src/lib/utils/environment-utils.ts +++ b/npm/ng-packs/packages/core/src/lib/utils/environment-utils.ts @@ -33,6 +33,8 @@ function mergeEnvironments( case 'deepmerge': return new SetEnvironment(deepMerge(local, remote)); case 'overwrite': + case null: + case undefined: return new SetEnvironment(remote); default: return new SetEnvironment(config.mergeStrategy(local, remote)); From 7225cad2c020c08ea2716955f85760cb7b0101e2 Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 11:25:52 +0300 Subject: [PATCH 11/13] test: utilize test.each for better readability --- .../src/lib/tests/environment-utils.spec.ts | 24 ++++--- .../core/src/lib/tests/object-utils.spec.ts | 68 ++++++++----------- 2 files changed, 41 insertions(+), 51 deletions(-) diff --git a/npm/ng-packs/packages/core/src/lib/tests/environment-utils.spec.ts b/npm/ng-packs/packages/core/src/lib/tests/environment-utils.spec.ts index 5599b61782..6dcfba17a7 100644 --- a/npm/ng-packs/packages/core/src/lib/tests/environment-utils.spec.ts +++ b/npm/ng-packs/packages/core/src/lib/tests/environment-utils.spec.ts @@ -61,18 +61,20 @@ describe('EnvironmentUtils', () => { }, }; - it('should call the remoteEnv URL and dispatch the SetEnvironment action for overwrite', () => { - setupTestAndRun({ mergeStrategy: 'deepmerge' }, deepMerge(environment, customEnv)); - }); + const someEnv = { apiUrl: 'https://some-api-url' } as any; + const customFn = (_, __) => someEnv; - it('should call the remoteEnv URL and dispatch the SetEnvironment action for deepmerge', () => { - setupTestAndRun({ mergeStrategy: 'overwrite' }, customEnv); - }); - - it('should call the remoteEnv URL and dispatch the SetEnvironment action for customFn', () => { - const someEnv = { apiUrl: 'https://some-api-url' } as any; - setupTestAndRun({ mergeStrategy: (_, __) => someEnv }, someEnv); - }); + test.each` + case | strategy | expected + ${'null'} | ${null} | ${customEnv} + ${'undefined'} | ${undefined} | ${customEnv} + ${'overwrite'} | ${'overwrite'} | ${customEnv} + ${'deepmerge'} | ${'deepmerge'} | ${deepMerge(environment, customEnv)} + ${'customFn'} | ${customFn} | ${someEnv} + `( + 'should call the remoteEnv URL and dispatch the SetEnvironment action for case $case ', + ({ strategy, expected }) => setupTestAndRun({ mergeStrategy: strategy }, expected), + ); function setupTestAndRun(strategy: Pick, expectedValue) { const injector = spectator.inject(Injector); diff --git a/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts b/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts index d234fe0132..38f096a283 100644 --- a/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts +++ b/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts @@ -8,48 +8,36 @@ describe('DeepMerge', () => { expect(deepMerge(null, null)).toEqual({}); }); - it('should correctly return when any of the inputs is null or undefined', () => { - const differentTestValues = [10, false, '', 'test-string', { a: 1 }, [1, 2, 3], {}]; - differentTestValues.forEach(val => { - expect(deepMerge(undefined, val)).toEqual(val); - expect(deepMerge(null, val)).toEqual(val); - expect(deepMerge(val, undefined)).toEqual(val); - expect(deepMerge(val, null)).toEqual(val); - }); + test.each` + value + ${10} + ${false} + ${''} + ${'test-string'} + ${{ a: 1 }} + ${[1, 2, 3]} + ${{}} + `('should correctly return when any of the inputs is null or undefined', val => { + expect(deepMerge(undefined, val)).toEqual(val); + expect(deepMerge(null, val)).toEqual(val); + expect(deepMerge(val, undefined)).toEqual(val); + expect(deepMerge(val, null)).toEqual(val); }); - it('should correctly return source if one of them is primitive or an array', () => { - const differentTestValues = [ - { - target: 10, - source: false, - }, - { - target: false, - source: 20, - }, - { - target: 'string', - source: { a: 5 }, - }, - { - target: { b: 10 }, - source: 50, - }, - { - target: [1, 2, 3], - source: 40, - }, - { - target: { k: 60 }, - source: [4, 5, 6], - }, - ]; - - differentTestValues.forEach(val => - expect(deepMerge(val.target, val.source)).toEqual(val.source), - ); - }); + test.each` + target | source + ${10} | ${false} + ${false} | ${20} + ${'some-string'} | ${{ a: 5 }} + ${{ b: 10 }} | ${50} + ${[1, 2, 3]} | ${40} + ${{ k: 60 }} | ${[4, 5, 6]} + `( + 'should correctly return source if one of them is primitive or an array', + ({ target, source }) => { + expect(deepMerge(target, source)).toEqual(source); + }, + ); it('should correctly return when both inputs are objects with different fields', () => { const target = { a: 1 }; From 2632f5c293fad472fdcdeebb85b5919ff5d2be24 Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 11:30:13 +0300 Subject: [PATCH 12/13] test: improve undefined or null test for deepMerge --- .../core/src/lib/tests/object-utils.spec.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts b/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts index 38f096a283..235d2e90ed 100644 --- a/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts +++ b/npm/ng-packs/packages/core/src/lib/tests/object-utils.spec.ts @@ -1,11 +1,14 @@ import { deepMerge } from '../utils/object-utils'; describe('DeepMerge', () => { - it('should return empty object when both inputs are null or undefined', () => { - expect(deepMerge(undefined, undefined)).toEqual({}); - expect(deepMerge(undefined, null)).toEqual({}); - expect(deepMerge(null, undefined)).toEqual({}); - expect(deepMerge(null, null)).toEqual({}); + test.each` + target | source + ${null} | ${null} + ${null} | ${undefined} + ${undefined} | ${null} + ${undefined} | ${undefined} + `('should return empty object when both inputs are $target and $source', ({ target, source }) => { + expect(deepMerge(target, source)).toEqual({}); }); test.each` From a20abfa23061917db8e2195fdb7f03dbb8acaec7 Mon Sep 17 00:00:00 2001 From: bnymncoskuner Date: Thu, 20 Aug 2020 14:01:12 +0300 Subject: [PATCH 13/13] test: undo wrongfully edited test name --- npm/ng-packs/packages/core/src/lib/tests/common-utils.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/ng-packs/packages/core/src/lib/tests/common-utils.spec.ts b/npm/ng-packs/packages/core/src/lib/tests/common-utils.spec.ts index 359a521513..05be159b9e 100644 --- a/npm/ng-packs/packages/core/src/lib/tests/common-utils.spec.ts +++ b/npm/ng-packs/packages/core/src/lib/tests/common-utils.spec.ts @@ -16,7 +16,7 @@ describe('CommonUtils', () => { }); }); - describe('#isUndefinedOrEmptyString & #exists', () => { + describe('#isUndefinedOrEmptyString', () => { test.each` value | expected ${null} | ${false}