From cd864ed09eff2f774378b9f36a8fb9dc3f4951c8 Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Wed, 18 Oct 2023 16:34:42 +0200 Subject: [PATCH] fix: add sort to deep diff (#5084) Sort array items before running compare. Feature flag certain properties of strategy that were previously not present in the /api/admin/features endpoint. --- package.json | 2 + src/lib/db/index.ts | 7 +++- .../createExportImportService.ts | 1 + .../feature-toggle-row-converter.ts | 37 ++++++++++++++----- .../createFeatureToggleService.ts | 7 +++- src/lib/features/feature-toggle/deep-diff.ts | 21 ++++++----- .../feature-toggle/feature-toggle-store.ts | 13 ++++++- .../feature-toggle/tests/deep-diff.test.ts | 18 +++++++++ .../features/project/createProjectService.ts | 7 +++- .../last-seen/createLastSeenService.ts | 1 + 10 files changed, 90 insertions(+), 24 deletions(-) diff --git a/package.json b/package.json index 7a947c7bf5..85dd0cb555 100644 --- a/package.json +++ b/package.json @@ -123,6 +123,8 @@ "knex": "^2.4.2", "lodash.get": "^4.4.2", "lodash.groupby": "^4.6.0", + "lodash.isequal": "^4.5.0", + "lodash.sortby": "^4.7.0", "log4js": "^6.0.0", "make-fetch-happen": "^11.0.0", "memoizee": "^0.4.15", diff --git a/src/lib/db/index.ts b/src/lib/db/index.ts index 5d8f6a8d59..315faf4313 100644 --- a/src/lib/db/index.ts +++ b/src/lib/db/index.ts @@ -49,7 +49,12 @@ export const createStores = ( return { eventStore, - featureToggleStore: new FeatureToggleStore(db, eventBus, getLogger), + featureToggleStore: new FeatureToggleStore( + db, + eventBus, + getLogger, + config.flagResolver, + ), featureTypeStore: new FeatureTypeStore(db, getLogger), strategyStore: new StrategyStore(db, getLogger), clientApplicationsStore: new ClientApplicationsStore( diff --git a/src/lib/features/export-import-toggles/createExportImportService.ts b/src/lib/features/export-import-toggles/createExportImportService.ts index 89e423b1b7..2a5c435ba0 100644 --- a/src/lib/features/export-import-toggles/createExportImportService.ts +++ b/src/lib/features/export-import-toggles/createExportImportService.ts @@ -157,6 +157,7 @@ export const deferredExportImportTogglesService = ( db, eventBus, getLogger, + flagResolver, ); const tagStore = new TagStore(db, eventBus, getLogger); const tagTypeStore = new TagTypeStore(db, eventBus, getLogger); diff --git a/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts b/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts index 14181b93f7..d3b4385d49 100644 --- a/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts +++ b/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts @@ -5,12 +5,19 @@ import { FeatureToggle, IFeatureToggleQuery, ITag, + IFlagResolver, } from '../../../types'; import { mapValues, ensureStringValue } from '../../../util'; import { FeatureConfigurationClient } from '../types/feature-toggle-strategies-store-type'; export class FeatureToggleRowConverter { + private flagResolver: IFlagResolver; + + constructor(flagResolver: IFlagResolver) { + this.flagResolver = flagResolver; + } + isUnseenStrategyRow = ( feature: PartialDeep, row: Record, @@ -63,15 +70,27 @@ export class FeatureToggleRowConverter { }; rowToStrategy = (row: Record): IStrategyConfig => { - const strategy: IStrategyConfig = { - id: row.strategy_id, - name: row.strategy_name, - title: row.strategy_title, - constraints: row.constraints || [], - parameters: mapValues(row.parameters || {}, ensureStringValue), - sortOrder: row.sort_order, - disabled: row.strategy_disabled, - }; + let strategy: IStrategyConfig; + if (this.flagResolver.isEnabled('playgroundImprovements')) { + strategy = { + id: row.strategy_id, + name: row.strategy_name, + title: row.strategy_title, + constraints: row.constraints || [], + parameters: mapValues(row.parameters || {}, ensureStringValue), + sortOrder: row.sort_order, + disabled: row.strategy_disabled, + }; + } else { + strategy = { + id: row.strategy_id, + name: row.strategy_name, + constraints: row.constraints || [], + parameters: mapValues(row.parameters || {}, ensureStringValue), + sortOrder: row.sort_order, + }; + } + strategy.variants = row.strategy_variants || []; return strategy; }; diff --git a/src/lib/features/feature-toggle/createFeatureToggleService.ts b/src/lib/features/feature-toggle/createFeatureToggleService.ts index 1860dc272c..356bbb3892 100644 --- a/src/lib/features/feature-toggle/createFeatureToggleService.ts +++ b/src/lib/features/feature-toggle/createFeatureToggleService.ts @@ -64,7 +64,12 @@ export const createFeatureToggleService = ( getLogger, flagResolver, ); - const featureToggleStore = new FeatureToggleStore(db, eventBus, getLogger); + const featureToggleStore = new FeatureToggleStore( + db, + eventBus, + getLogger, + flagResolver, + ); const featureToggleClientStore = new FeatureToggleClientStore( db, eventBus, diff --git a/src/lib/features/feature-toggle/deep-diff.ts b/src/lib/features/feature-toggle/deep-diff.ts index f49414a644..9a80c8d095 100644 --- a/src/lib/features/feature-toggle/deep-diff.ts +++ b/src/lib/features/feature-toggle/deep-diff.ts @@ -1,3 +1,5 @@ +import sortBy from 'lodash.sortby'; + interface Difference { index: (string | number)[]; reason: string; @@ -18,8 +20,11 @@ export function deepDiff(arr1: any[], arr2: any[]): Difference[] | null { valueB: b, }); } else { - for (let i = 0; i < a.length; i++) { - compare(a[i], b[i], parentIndex.concat(i)); + const sortedA = sortBy(a, 'name'); + const sortedB = sortBy(b, 'name'); + + for (let i = 0; i < sortedA.length; i++) { + compare(sortedA[i], sortedB[i], parentIndex.concat(i)); } } } else if ( @@ -31,7 +36,10 @@ export function deepDiff(arr1: any[], arr2: any[]): Difference[] | null { const keysA = Object.keys(a); const keysB = Object.keys(b); - if (!arraysEqual(keysA, keysB)) { + if ( + keysA.length !== keysB.length || + !keysA.every((key) => keysB.includes(key)) + ) { diff.push({ index: parentIndex, reason: 'Different keys', @@ -53,13 +61,6 @@ export function deepDiff(arr1: any[], arr2: any[]): Difference[] | null { } } - function arraysEqual(a: any[], b: any[]): boolean { - return ( - a.length === b.length && - a.sort().every((val, index) => val === b.sort()[index]) - ); - } - compare(arr1, arr2, []); return diff.length > 0 ? diff : null; diff --git a/src/lib/features/feature-toggle/feature-toggle-store.ts b/src/lib/features/feature-toggle/feature-toggle-store.ts index 322b292e29..8157c16dc1 100644 --- a/src/lib/features/feature-toggle/feature-toggle-store.ts +++ b/src/lib/features/feature-toggle/feature-toggle-store.ts @@ -64,10 +64,19 @@ export default class FeatureToggleStore implements IFeatureToggleStore { private featureToggleRowConverter: FeatureToggleRowConverter; - constructor(db: Db, eventBus: EventEmitter, getLogger: LogProvider) { + private flagResolver: IFlagResolver; + + constructor( + db: Db, + eventBus: EventEmitter, + getLogger: LogProvider, + flagResolver: IFlagResolver, + ) { this.db = db; this.logger = getLogger('feature-toggle-store.ts'); - this.featureToggleRowConverter = new FeatureToggleRowConverter(); + this.featureToggleRowConverter = new FeatureToggleRowConverter( + flagResolver, + ); this.timer = (action) => metricsHelper.wrapTimer(eventBus, DB_TIME, { store: 'feature-toggle', diff --git a/src/lib/features/feature-toggle/tests/deep-diff.test.ts b/src/lib/features/feature-toggle/tests/deep-diff.test.ts index 8c53f57661..228a095b39 100644 --- a/src/lib/features/feature-toggle/tests/deep-diff.test.ts +++ b/src/lib/features/feature-toggle/tests/deep-diff.test.ts @@ -1,6 +1,24 @@ import { deepDiff } from '../deep-diff'; // Import the deepDiff function describe('deepDiff', () => { + test('should sort arrays by name before comparing', () => { + // Define two arrays that are identical except for the order of elements + const array1 = [ + { name: 'b', value: 2 }, + { name: 'a', value: 1 }, + ]; + const array2 = [ + { name: 'a', value: 1 }, + { name: 'b', value: 2 }, + ]; + + // If the function correctly sorts before comparing, there should be no differences + const result = deepDiff(array1, array2); + + // Assert that there is no difference + expect(result).toBeNull(); + }); + it('should return null for equal arrays', () => { const arr1 = [1, 2, 3]; const arr2 = [1, 2, 3]; diff --git a/src/lib/features/project/createProjectService.ts b/src/lib/features/project/createProjectService.ts index 01758643d1..fbafe4ae35 100644 --- a/src/lib/features/project/createProjectService.ts +++ b/src/lib/features/project/createProjectService.ts @@ -57,7 +57,12 @@ export const createProjectService = ( flagResolver, ); const groupStore = new GroupStore(db); - const featureToggleStore = new FeatureToggleStore(db, eventBus, getLogger); + const featureToggleStore = new FeatureToggleStore( + db, + eventBus, + getLogger, + flagResolver, + ); const featureTypeStore = new FeatureTypeStore(db, getLogger); const accountStore = new AccountStore(db, getLogger); const environmentStore = new EnvironmentStore(db, eventBus, getLogger); diff --git a/src/lib/services/client-metrics/last-seen/createLastSeenService.ts b/src/lib/services/client-metrics/last-seen/createLastSeenService.ts index 938a4d7078..e9fde30f3a 100644 --- a/src/lib/services/client-metrics/last-seen/createLastSeenService.ts +++ b/src/lib/services/client-metrics/last-seen/createLastSeenService.ts @@ -19,6 +19,7 @@ export const createLastSeenService = ( db, config.eventBus, config.getLogger, + config.flagResolver, ); return new LastSeenService({ lastSeenStore, featureToggleStore }, config);