From 89af5bc6e0b0bcbe2970c836986c36999207ecd8 Mon Sep 17 00:00:00 2001 From: rowheat02 Date: Thu, 24 Jul 2025 19:33:37 +0545 Subject: [PATCH 1/3] Refactor map comparison logic with rule-based utilities and update tests for improved flexibility --- web/client/utils/MapUtils.js | 154 +++++++++++++------- web/client/utils/__tests__/MapUtils-test.js | 152 +++++++++++++------ 2 files changed, 209 insertions(+), 97 deletions(-) diff --git a/web/client/utils/MapUtils.js b/web/client/utils/MapUtils.js index 48b00d0c7b..b198aab353 100644 --- a/web/client/utils/MapUtils.js +++ b/web/client/utils/MapUtils.js @@ -833,71 +833,125 @@ export const getIdFromUri = (uri, regex = /data\/(\d+)/) => { return findDataDigit && findDataDigit.length && findDataDigit.length > 1 ? findDataDigit[1] : null; }; + /** - * Method for cleanup map object from uneseccary fields which - * updated map contains and were set on map render - * @param {object} obj + * Determines if a field should be included in the comparison based on picked fields and exclusion rules. + * @param {string} path - The full path to the field (e.g., 'root.obj.key'). + * @param {string} key - The key of the field being checked. + * @param {*} value - The value of the field. + * @param {object} rules - The rules object containing pickedFields and excludes. + * @param {string[]} rules.pickedFields - Array of field paths to include in the comparison. + * @param {object} rules.excludes - Object mapping parent paths to arrays of keys to exclude. + * @returns {boolean} True if the field should be included, false otherwise. */ - -export const prepareMapObjectToCompare = obj => { - const skippedKeys = ['apiKey', 'time', 'args', 'fixed']; - const shouldBeSkipped = (key) => skippedKeys.reduce((p, n) => p || key === n, false); - Object.keys(obj).forEach(key => { - const value = obj[key]; - const type = typeof value; - if (type === "object" && value !== null && !shouldBeSkipped(key)) { - prepareMapObjectToCompare(value); - if (!Object.keys(value).length) { - delete obj[key]; - } - } else if (type === "undefined" || !value || shouldBeSkipped(key)) { - delete obj[key]; +export const filterFieldByRules = (path, key, value, { pickedFields = [], excludes = {} }) => { + if (value === undefined) { + return false; + } + if (pickedFields.some((field) => field.includes(path) || path.includes(field))) { + // Fix: check parent path for excludes + const parentPath = path.substring(0, path.lastIndexOf('.')); + if (excludes[parentPath] === undefined) { + return true; } - }); + if (excludes[parentPath] && excludes[parentPath].includes(key)) { + return false; + } + return true; + } + return false; }; /** - * Method added for support old key with objects provided for compareMapChanges feature - * like text_serch_config - * @param {object} obj - * @param {string} oldKey - * @param {string} newKey + * Prepares object entries for comparison by applying aliasing, filtering, and sorting. + * @param {object} obj - The object whose entries are to be prepared. + * @param {object} rules - The rules object containing aliases, pickedFields, and excludes. + * @param {string} parentKey - The parent key path for the current object. + * @returns {Array} Array of [key, value] pairs, filtered and sorted for comparison. */ -export const updateObjectFieldKey = (obj, oldKey, newKey) => { - if (obj[oldKey]) { - Object.defineProperty(obj, newKey, Object.getOwnPropertyDescriptor(obj, oldKey)); - delete obj[oldKey]; +export const prepareObjectEntries = (obj, rules, parentKey) => { + const safeObj = obj || {}; + // First filter using the original keys, then apply aliasing + return Object.entries(safeObj) + .filter(([key, value]) => filterFieldByRules(`${parentKey}.${key}`, key, value, rules)) + .map(([key, value]) => [rules.aliases && rules.aliases[key] || key, value]) + .sort((a, b) => { + if (a[0] < b[0]) { return -1; } + if (a[0] > b[0]) { return 1; } + return 0; + }); +}; + +// function that checks if a field has changed ( also includes the rules to prepare object for comparision) +export const recursiveIsChangedWithRules = (a, b, rules, parentKey = 'root') => { + // strictly equal + if (a === b) { + return false; + } + + // Handle arrays + if (Array.isArray(a)) { + if (!Array.isArray(b) || a.length !== b.length) { + return true; + } + // same reference + if (a === b) return false; + for (let i = 0; i < a.length; i++) { + if (recursiveIsChangedWithRules(a[i], b[i], rules, `${parentKey}[]`)) return true; + } + return false; + } + + // Handle objects + if (typeof a === 'object' && a !== null) { + // Prepare entries only if needed + const aEntries = prepareObjectEntries(a, rules, parentKey); + const bEntries = prepareObjectEntries(b || {}, rules, parentKey); + if (aEntries.length !== bEntries.length) { + return true; + } + for (let i = 0; i < aEntries.length; i++) { + const [key, value] = aEntries[i]; + if (recursiveIsChangedWithRules(value, bEntries[i]?.[1], rules, `${parentKey}.${key}`)) { + return true; + } + } + return false; } + + // Fallback for primitives + if (!isEqual(a, b)) { + return true; + } + return false; }; /** - * Feature for map change recognition. Returns value of isEqual method from lodash - * @param {object} map1 original map before changes - * @param {object} map2 updated map - * @returns {boolean} + * @param {object} map1 - The original map configuration object. + * @param {object} map2 - The updated map configuration object. + * @returns {boolean} True if the considered fields are equal, false otherwise. */ export const compareMapChanges = (map1 = {}, map2 = {}) => { const pickedFields = [ - 'map.layers', - 'map.backgrounds', - 'map.text_search_config', - 'map.bookmark_search_config', - 'map.text_serch_config', - 'map.zoom', - 'widgetsConfig', - 'swipe' + 'root.map.layers', + 'root.map.backgrounds', + 'root.map.text_search_config', + 'root.map.bookmark_search_config', + 'root.map.text_serch_config', + 'root.map.zoom', + 'root.widgetsConfig', + 'root.swipe' ]; - const filteredMap1 = pick(cloneDeep(map1), pickedFields); - const filteredMap2 = pick(cloneDeep(map2), pickedFields); - // ABOUT: used for support text_serch_config field in old maps - updateObjectFieldKey(filteredMap1.map, 'text_serch_config', 'text_search_config'); - updateObjectFieldKey(filteredMap2.map, 'text_serch_config', 'text_search_config'); - - prepareMapObjectToCompare(filteredMap1); - prepareMapObjectToCompare(filteredMap2); - return isEqual(filteredMap1, filteredMap2); -}; + const aliases = { + text_serch_config: 'text_search_config' + }; + const excludes = { + 'root.map.layers[]': ['apiKey', 'time', 'args', 'fixed'] + }; + const isSame = !recursiveIsChangedWithRules(map1, map2, { pickedFields, aliases, excludes }, 'root'); + return isSame; +}; /** * creates utilities for registering, fetching, executing hooks * used to override default ones in order to have a local hooks object @@ -1022,8 +1076,6 @@ export default { isSimpleGeomType, getSimpleGeomType, getIdFromUri, - prepareMapObjectToCompare, - updateObjectFieldKey, compareMapChanges, clearHooks, getResolutionObject, diff --git a/web/client/utils/__tests__/MapUtils-test.js b/web/client/utils/__tests__/MapUtils-test.js index 554d83a596..a944a50367 100644 --- a/web/client/utils/__tests__/MapUtils-test.js +++ b/web/client/utils/__tests__/MapUtils-test.js @@ -32,8 +32,6 @@ import { getIdFromUri, getSimpleGeomType, isSimpleGeomType, - prepareMapObjectToCompare, - updateObjectFieldKey, compareMapChanges, mergeMapConfigs, addRootParentGroup, @@ -43,7 +41,10 @@ import { reprojectZoom, getRandomPointInCRS, convertResolution, - getExactZoomFromResolution + getExactZoomFromResolution, + recursiveIsChangedWithRules, + filterFieldByRules, + prepareObjectEntries } from '../MapUtils'; import { VisualizationModes } from '../MapTypeUtils'; @@ -1943,7 +1944,8 @@ describe('Test the MapUtils', () => { }, "catalogServices": {}, "widgetsConfig": {}, - "mapInfoConfiguration": {} + "mapInfoConfiguration": {}, + swipe: {} }; const map2 = { "version": 2, @@ -1991,48 +1993,6 @@ describe('Test the MapUtils', () => { expect(compareMapChanges(map1, map2)).toBeTruthy(); }); - it('test prepareMapObjectToCompare', () => { - const obj1 = { time: new Date().toISOString() }; - const obj2 = { apiKey: 'some api key' }; - const obj3 = { test: undefined }; - const obj4 = { test: null }; - const obj5 = { test: false }; - const obj6 = { test: {} }; - const obj7 = { fixed: false }; - const obj8 = { args: 'some api key' }; - prepareMapObjectToCompare(obj1); - prepareMapObjectToCompare(obj2); - prepareMapObjectToCompare(obj3); - prepareMapObjectToCompare(obj4); - prepareMapObjectToCompare(obj5); - prepareMapObjectToCompare(obj6); - prepareMapObjectToCompare(obj7); - prepareMapObjectToCompare(obj8); - expect(Object.keys(obj1).indexOf('time')).toBe(-1); - expect(Object.keys(obj2).indexOf('apiKey')).toBe(-1); - expect(Object.keys(obj3).indexOf('test')).toBe(-1); - expect(Object.keys(obj4).indexOf('test')).toBe(-1); - expect(Object.keys(obj5).indexOf('test')).toBe(-1); - expect(Object.keys(obj6).indexOf('test')).toBe(-1); - expect(Object.keys(obj7).indexOf('fixed')).toBe(-1); - expect(Object.keys(obj8).indexOf('args')).toBe(-1); - }); - - it('test updateObjectFieldKey', () => { - const origin = { test1: 'test', test2: 'test' }; - const clone = JSON.parse(JSON.stringify(origin)); - const clone2 = JSON.parse(JSON.stringify(origin)); - const clone3 = JSON.parse(JSON.stringify(origin)); - updateObjectFieldKey(clone); - updateObjectFieldKey(clone2, 'test1', 'test3'); - updateObjectFieldKey(clone3, 'test3', 'test4'); - expect(clone.test1).toBe(origin.test1); - expect(clone.test2).toBe(origin.test2); - expect(clone2.test1).toNotExist(); - expect(clone2.test3).toExist(); - expect(clone3.test3).toNotExist(); - expect(clone3.test4).toNotExist(); - }); it('mergeMapConfigs', () => { const testBackground = { @@ -2439,4 +2399,104 @@ describe('Test the MapUtils', () => { expect(getExactZoomFromResolution(50000, resolutions)).toEqual(1.6465589981535294); expect(getExactZoomFromResolution(10000, resolutions)).toEqual(3.9684870930408915); }); + +}); + +describe('recursiveIsChangedWithRules', () => { + it('ignores excluded keys', () => { + const rules = { + pickedFields: ['root.obj'], + excludes: { 'root.obj': ['x'] } + }; + expect(recursiveIsChangedWithRules({ x: 1, y: 2 }, { y: 2 }, rules, 'root.obj')).toBe(false); + }); + it('treats alias keys as equal', () => { + const rules = { + pickedFields: ['root.obj'], + aliases: { old: 'new' } + }; + expect(recursiveIsChangedWithRules({ old: 1 }, { 'new': 1 }, rules, 'root.obj')).toBe(false); + }); + it('only compares picked fields', () => { + const rules = { + pickedFields: ['root.obj.a'], + excludes: {} + }; + expect(recursiveIsChangedWithRules({ a: 1, b: 2 }, { a: 1, b: 3 }, rules, 'root.obj')).toBe(false); + }); + it('works with nested structures and exclusions', () => { + const rules = { + pickedFields: ['root.arr'], + excludes: { 'root.arr[]': ['skip'] } + }; + const a = { arr: [{ keep: 1, skip: 2 }] }; + const b = { arr: [{ keep: 1, skip: 3 }] }; + expect(recursiveIsChangedWithRules(a, b, rules, 'root')).toBe(false); + }); + it('detects changes in nested structures not excluded', () => { + const rules = { + pickedFields: ['root.arr'], + excludes: { 'root.arr[]': ['skip'] } + }; + const a = { arr: [{ keep: 1, skip: 2 }] }; + const b = { arr: [{ keep: 2, skip: 2 }] }; + expect(recursiveIsChangedWithRules(a, b, rules, 'root')).toBe(true); + }); +}); + +describe('filterFieldByRules', () => { + it('returns false if value is undefined', () => { + const rules = { pickedFields: ['root.obj'], excludes: {} }; + expect(filterFieldByRules('root.obj.x', 'x', undefined, rules)).toBe(false); + }); + it('returns true if path is in pickedFields and not excluded', () => { + const rules = { pickedFields: ['root.obj'], excludes: {} }; + expect(filterFieldByRules('root.obj.x', 'x', 1, rules)).toBe(true); + }); + it('returns false if path is in pickedFields but key is excluded', () => { + const rules = { pickedFields: ['root.obj'], excludes: { 'root.obj': ['x'] } }; + expect(filterFieldByRules('root.obj.x', 'x', 1, rules)).toBe(false); + }); + it('returns false if path is not in pickedFields', () => { + const rules = { pickedFields: ['root.other'], excludes: {} }; + expect(filterFieldByRules('root.obj.x', 'x', 1, rules)).toBe(false); + }); +}); + +describe('prepareObjectEntries', () => { + it('returns filtered and sorted entries with aliasing', () => { + const obj = { a: 1, b: 2, c: 3 }; + const rules = { + pickedFields: ['root.a', 'root.b'], + excludes: {}, + aliases: { 'a': 'x', 'b': 'y' } + }; + const entries = prepareObjectEntries(obj, rules, 'root'); + expect(entries).toEqual([ + ['x', 1], + ['y', 2] + ]); + }); + it('excludes keys as per rules', () => { + const obj = { a: 1, b: 2 }; + const rules = { + pickedFields: ['root.obj'], + excludes: { 'root.obj': ['b'] }, + aliases: {} + }; + const entries = prepareObjectEntries(obj, rules, 'root.obj'); + expect(entries).toEqual([ + ['a', 1] + ]); + }); + it('returns empty array if no picked fields match', () => { + const obj = { a: 1 }; + const rules = { + pickedFields: ['root.other'], + excludes: {}, + aliases: {} + }; + const entries = prepareObjectEntries(obj, rules, 'root.obj'); + expect(entries).toEqual([]); + }); }); From 8c1fb1d2ca18e538dac00870764481382145256e Mon Sep 17 00:00:00 2001 From: rowheat02 Date: Fri, 25 Jul 2025 13:38:28 +0545 Subject: [PATCH 2/3] fix: tests --- web/client/epics/__tests__/pendingChanges-test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/web/client/epics/__tests__/pendingChanges-test.js b/web/client/epics/__tests__/pendingChanges-test.js index d67ac51643..b2529788a9 100644 --- a/web/client/epics/__tests__/pendingChanges-test.js +++ b/web/client/epics/__tests__/pendingChanges-test.js @@ -76,7 +76,8 @@ describe('comparePendingChanges', () => { "catalogURL": "url", "useForElevation": false, "hidden": false, - "params": {} + "params": {}, + "expanded": false } ], "groups": [], From 49a0603301f6295feba208b4f73a90dd65022506 Mon Sep 17 00:00:00 2001 From: rowheat02 Date: Mon, 28 Jul 2025 20:40:46 +0545 Subject: [PATCH 3/3] fix: replaced isEqual from loadash to js strich check --- web/client/utils/MapUtils.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/web/client/utils/MapUtils.js b/web/client/utils/MapUtils.js index b198aab353..df397a51d6 100644 --- a/web/client/utils/MapUtils.js +++ b/web/client/utils/MapUtils.js @@ -918,12 +918,8 @@ export const recursiveIsChangedWithRules = (a, b, rules, parentKey = 'root') => } return false; } - // Fallback for primitives - if (!isEqual(a, b)) { - return true; - } - return false; + return a !== b; }; /**