diff --git a/API-INTERNAL.md b/API-INTERNAL.md index df46837f7..f5e7f52b7 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -33,6 +33,9 @@ is reused by every connection.

getDefaultKeyStates()

Getter - returns the default key states.

+
getNullFreeDefaultKeyState()
+

Getter - returns the default key state without nested nulls, cached so clear() reuses the same references.

+
getDeferredInitTask()

Getter - returns the deffered init task.

@@ -206,6 +209,12 @@ Getter - returns the merge queue promise. ## getDefaultKeyStates() Getter - returns the default key states. +**Kind**: global function + + +## getNullFreeDefaultKeyState() +Getter - returns the default key state without nested nulls, cached so clear() reuses the same references. + **Kind**: global function diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 3a05db78a..35be6bcf0 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -64,13 +64,15 @@ function init({ const individual: Array<[OnyxKey, OnyxEntry]> = []; const collectionBatches = new Map>; previous: NonUndefined>}>(); - for (const [key, value] of pairs) { + for (const [key, valueFromStorage] of pairs) { // RAM-only keys should never sync from storage as they may have stale persisted data // from before the key was migrated to RAM-only. if (OnyxKeys.isRamOnlyKey(key)) { continue; } + const value = utils.needsNormalization(valueFromStorage) ? utils.removeNestedNullValues(valueFromStorage) : valueFromStorage; + const collectionKey = OnyxKeys.getCollectionKey(key); const isCollectionMember = !!collectionKey && OnyxKeys.isCollectionMemberKey(collectionKey, key); @@ -487,7 +489,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { // since collection key subscribers need to be updated differently if (!isKeyToPreserve) { const oldValue = cache.get(key); - const newValue = defaultKeyStates[key] ?? null; + const newValue = OnyxUtils.getNullFreeDefaultKeyState(key); valuesSetByClear[key] = newValue; if (newValue !== oldValue) { cache.set(key, newValue); diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 4e5cd19cf..ed557f585 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -76,6 +76,9 @@ const mergeQueuesWithStaleRead = new WeakSet>>(); // Optional user-provided key value states set when Onyx initializes or clears let defaultKeyStates: Record> = {}; +// Cache for getNullFreeDefaultKeyState() +let nullFreeDefaultKeyStates: Record> = {}; + let snapshotKey: OnyxKey | null = null; // Connections can be made before `Onyx.init`. They would wait for this task before resolving. @@ -123,6 +126,14 @@ function getDefaultKeyStates(): Record> { return defaultKeyStates; } +/** Getter - returns the default key state without nested nulls, cached so clear() reuses the same references. */ +function getNullFreeDefaultKeyState(key: OnyxKey): OnyxValue { + if (!(key in nullFreeDefaultKeyStates)) { + nullFreeDefaultKeyStates[key] = utils.removeNestedNullValues(defaultKeyStates[key] ?? null); + } + return nullFreeDefaultKeyStates[key]; +} + /** * Getter - returns the deffered init task. */ @@ -191,6 +202,7 @@ function initStoreValues(keys: DeepRecord, initialKeyStates: Pa // Set our default key states to use when initializing and clearing Onyx data defaultKeyStates = initialKeyStates; + nullFreeDefaultKeyStates = {}; DevTools.initState(initialKeyStates); @@ -781,7 +793,7 @@ function prepareKeyValuePairsForStorage( continue; } - const valueWithoutNestedNullValues = shouldRemoveNestedNulls ?? true ? utils.removeNestedNullValues(value) : value; + const valueWithoutNestedNullValues = shouldRemoveNestedNulls ?? true ? utils.removeNestedNullValues(value, cache.get(key)) : value; if (valueWithoutNestedNullValues !== undefined) { pairs.push([key, valueWithoutNestedNullValues, replaceNullPatches?.[key]]); @@ -1119,7 +1131,7 @@ function setWithRetry({key, value, options}: SetParams; + const valueWithoutNestedNullValues = utils.removeNestedNullValues(value, existingValue) as OnyxValue; const hasChanged = options?.skipCacheCheck ? true : cache.hasValueChanged(key, valueWithoutNestedNullValues); OnyxUtils.logKeyChanged(OnyxUtils.METHOD.SET, key, value, hasChanged); @@ -1697,6 +1709,7 @@ const OnyxUtils = { getMergeQueue, getMergeQueuePromise, getDefaultKeyStates, + getNullFreeDefaultKeyState, getDeferredInitTask, afterInit, initStoreValues, diff --git a/lib/storage/providers/IDBKeyValProvider/index.ts b/lib/storage/providers/IDBKeyValProvider/index.ts index 546c9acd2..cbb50c1cf 100644 --- a/lib/storage/providers/IDBKeyValProvider/index.ts +++ b/lib/storage/providers/IDBKeyValProvider/index.ts @@ -154,7 +154,14 @@ const provider: StorageProvider = { return ( IDB.get(key, provider.store) // idb-keyval returns undefined for missing items, but this needs to return null so that idb-keyval does the same thing as SQLiteStorage. - .then((val) => (val === undefined ? null : val)) + .then((val) => { + if (val === undefined) { + return null; + } + + // Stored values can hold nested nulls, but values handed to the cache must not. + return utils.needsNormalization(val) ? utils.removeNestedNullValues(val) : val; + }) ); }, removeItem(key) { diff --git a/lib/storage/providers/SQLiteProvider.ts b/lib/storage/providers/SQLiteProvider.ts index c01844985..da13ed76a 100644 --- a/lib/storage/providers/SQLiteProvider.ts +++ b/lib/storage/providers/SQLiteProvider.ts @@ -215,7 +215,10 @@ const provider: StorageProvider = { return null; } - return JSON.parse(result.valueJSON); + const value = JSON.parse(result.valueJSON); + + // valueJSON is a number for numeric values, so the type is checked before includes(). + return typeof value === 'object' && value !== null && result.valueJSON.includes('null') ? utils.removeNestedNullValues(value) : value; }); }, multiGet(keys) { diff --git a/lib/utils.ts b/lib/utils.ts index 6235ddf03..164ae4792 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -1,5 +1,6 @@ import {deepEqual} from 'fast-equals'; import type {OnyxInput, OnyxKey} from './types'; +import * as Logger from './Logger'; type EmptyObject = Record; type EmptyValue = EmptyObject | null | undefined; @@ -176,7 +177,8 @@ function mergeObject>( // of the merged object only. const sourcePropertyWithoutMark = {...sourceProperty}; delete sourcePropertyWithoutMark.ONYX_INTERNALS__REPLACE_OBJECT_MARK; - destination[key] = sourcePropertyWithoutMark; + // Built in "mark" mode, so it can still hold nested nulls. + destination[key] = options.shouldRemoveNestedNulls ? removeNestedNullValues(sourcePropertyWithoutMark) : sourcePropertyWithoutMark; continue; } @@ -264,12 +266,25 @@ function needsNormalization(value: unknown): boolean { return false; } -/** Deep removes the nested null values from the given value. Returns the original reference if no nulls were found. */ -function removeNestedNullValues | null>(value: TValue): TValue { +/** + * Deep removes nested null values from the given value, leaving arrays as they are. Returns the original reference if no nulls were found. + * + * Objects that are the same reference as the object at the same path in `nullFreeReference` are returned without being traversed, + * so `nullFreeReference` must hold no nested nulls outside arrays (pass the live cache value). + */ +function removeNestedNullValues | null>(value: TValue, nullFreeReference?: unknown): TValue { if (value === null || value === undefined || typeof value !== 'object' || Array.isArray(value)) { return value; } + if (value === nullFreeReference) { + if (process.env.NODE_ENV === 'development' && needsNormalization(value)) { + Logger.logAlert('removeNestedNullValues skipped a cached subtree that holds nested null values. Onyx values must not be mutated in place.'); + } + return value; + } + + const reference = nullFreeReference !== null && typeof nullFreeReference === 'object' && !Array.isArray(nullFreeReference) ? (nullFreeReference as Record) : undefined; let hasChanged = false; const result: Record = {}; @@ -283,7 +298,7 @@ function removeNestedNullValues | null>(value: } if (typeof propertyValue === 'object' && !Array.isArray(propertyValue)) { - const cleaned = removeNestedNullValues(propertyValue); + const cleaned = removeNestedNullValues(propertyValue, reference?.[key]); if (cleaned !== propertyValue) { hasChanged = true; } diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 6b07e1c0f..7e465feaf 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -13,6 +13,7 @@ import createDeferredTask from '../../lib/createDeferredTask'; import * as Logger from '../../lib/Logger'; import onyxSubscriptionManager from '../../lib/OnyxSubscriptionManager'; import OnyxUtils from '../../lib/OnyxUtils'; +import utils from '../../lib/utils'; import StorageMock from '../../lib/storage'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; @@ -3878,6 +3879,143 @@ describe('Onyx', () => { await expect(Onyx.multiGet([])).resolves.toEqual([]); }); }); + + describe('null-free cache', () => { + it('should remove nested nulls when the native merge path replaces an object built in mark mode', () => { + // Given a batch that removes an object and then sets a new one holding a nested null (native applyMerge batches like this) + const {result: batchedChanges} = OnyxUtils.mergeAndMarkChanges([{a: null}, {a: {b: 1, c: null}}]); + + // When the batched change is merged into the existing value + const {result: mergedValue} = OnyxUtils.mergeChanges([batchedChanges], {a: {x: 1}}); + + // Then the replacing object has its nested null removed, so it can be cached + expect(mergedValue).toStrictEqual({a: {b: 1}}); + }); + + it('should alert in development when a skipped cached subtree was mutated to hold a null', async () => { + // Given a cached subtree that app code mutated in place, which Onyx does not support + await Onyx.set(ONYX_KEYS.TEST_KEY, {a: {x: 1}}); + const cachedValue = cache.get(ONYX_KEYS.TEST_KEY) as GenericDeepRecord; + (cachedValue.a as GenericDeepRecord).x = null; + const logAlertSpy = jest.spyOn(Logger, 'logAlert'); + const previousNodeEnv = process.env.NODE_ENV; + process.env.NODE_ENV = 'development'; + + try { + // When a value sharing that subtree is set + await Onyx.set(ONYX_KEYS.TEST_KEY, {...cachedValue, b: 1}); + + // Then the mutation is reported instead of silently persisting the null + expect(logAlertSpy).toHaveBeenCalledWith(expect.stringContaining('skipped a cached subtree that holds nested null values')); + } finally { + process.env.NODE_ENV = previousNodeEnv; + logAlertSpy.mockRestore(); + } + }); + + it('should keep every cached value free of nested nulls across random writes', async () => { + // Given random sequences of set, merge, multiSet, mergeCollection and update writes that carry nested nulls + let state = 7; + const random = () => { + state = (state * 1664525 + 1013904223) % 4294967296; + return state / 4294967296; + }; + const randomValue = (depth: number): unknown => { + const roll = random(); + if (roll < 0.2) { + return null; + } + if (roll < 0.45 || depth === 0) { + return Math.floor(random() * 10); + } + const value: Record = {}; + for (let i = 0; i < 1 + Math.floor(random() * 3); i++) { + value[`p${Math.floor(random() * 4)}`] = randomValue(depth - 1); + } + return value; + }; + const keys = [ONYX_KEYS.TEST_KEY, `${ONYX_KEYS.COLLECTION.TEST_KEY}1`, `${ONYX_KEYS.COLLECTION.TEST_KEY}2`, `${ONYX_KEYS.COLLECTION.TEST_KEY}3`]; + const randomKey = () => keys[Math.floor(random() * keys.length)]; + const randomObject = () => ({...(randomValue(3) as Record), id: 1}); + + for (let step = 0; step < 300; step++) { + const roll = random(); + if (roll < 0.25) { + Onyx.set(randomKey(), randomObject()); + } else if (roll < 0.5) { + Onyx.merge(randomKey(), randomObject()); + } else if (roll < 0.6) { + Onyx.multiSet({[randomKey()]: randomObject()}); + } else if (roll < 0.75) { + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + [`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]: randomObject(), + [`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]: randomObject(), + } as GenericCollection); + } else if (roll < 0.88) { + Onyx.update([ + {onyxMethod: Onyx.METHOD.MERGE, key: randomKey(), value: random() < 0.3 ? null : randomObject()}, + {onyxMethod: Onyx.METHOD.MERGE, key: randomKey(), value: randomObject()}, + {onyxMethod: random() < 0.5 ? Onyx.METHOD.SET : Onyx.METHOD.MERGE, key: randomKey(), value: randomObject()}, + ]); + } else { + // Removes a nested object and replaces it with a new one holding nulls in the same batch (replace mode) + const member = `${ONYX_KEYS.COLLECTION.TEST_KEY}${1 + Math.floor(random() * 3)}`; + Onyx.update([ + {onyxMethod: Onyx.METHOD.MERGE, key: member, value: {p0: null}}, + {onyxMethod: Onyx.METHOD.MERGE, key: member, value: {p0: {q: 1, r: null, s: {t: null}}}}, + {onyxMethod: Onyx.METHOD.MERGE, key: randomKey(), value: randomObject()}, + ]); + } + if (step % 10 === 9) { + await waitForPromisesToResolve(); + + // Then no cached value holds a nested null + for (const key of keys) { + const value = cache.get(key); + expect(value === undefined || value === null || !utils.needsNormalization(value)).toBe(true); + } + } + } + }); + + it('should not cache nested nulls of an object that replaced a removed object in a batched update', async () => { + const member1 = `${ONYX_KEYS.COLLECTION.TEST_KEY}1`; + const member2 = `${ONYX_KEYS.COLLECTION.TEST_KEY}2`; + await Onyx.multiSet({[member1]: {a: {x: 1}}, [member2]: {a: {x: 1}}}); + + await Onyx.update([ + {onyxMethod: Onyx.METHOD.MERGE, key: member1, value: {a: null}}, + {onyxMethod: Onyx.METHOD.MERGE, key: member1, value: {a: {b: 1, c: null}}}, + {onyxMethod: Onyx.METHOD.MERGE, key: member2, value: {z: 1}}, + ]); + + expect(cache.get(member1)).toStrictEqual({a: {b: 1}}); + }); + + it('should keep the cached subtrees and remove new nulls when setting a value built from the cached one', async () => { + await Onyx.set(ONYX_KEYS.TEST_KEY, {a: {x: 1}, b: {y: 2}}); + const cachedValue = cache.get(ONYX_KEYS.TEST_KEY) as GenericDeepRecord; + + await Onyx.set(ONYX_KEYS.TEST_KEY, {...cachedValue, b: {y: 3, z: null}, c: null}); + + const newValue = cache.get(ONYX_KEYS.TEST_KEY) as GenericDeepRecord; + expect(newValue).toStrictEqual({a: {x: 1}, b: {y: 3}}); + expect(newValue.a).toBe(cachedValue.a); + expect(await StorageMock.getItem(ONYX_KEYS.TEST_KEY)).toStrictEqual({a: {x: 1}, b: {y: 3}}); + }); + + it('should keep the cached subtrees and remove new nulls when multi-setting values built from the cached ones', async () => { + await Onyx.set(ONYX_KEYS.TEST_KEY, {a: {x: 1}}); + const cachedValue = cache.get(ONYX_KEYS.TEST_KEY) as GenericDeepRecord; + + await Onyx.multiSet({[ONYX_KEYS.TEST_KEY]: {...cachedValue, b: {y: null}}, [ONYX_KEYS.OTHER_TEST]: {c: {d: null}}}); + + const newValue = cache.get(ONYX_KEYS.TEST_KEY) as GenericDeepRecord; + expect(newValue).toStrictEqual({a: {x: 1}, b: {}}); + expect(newValue.a).toBe(cachedValue.a); + expect(cache.get(ONYX_KEYS.OTHER_TEST)).toStrictEqual({c: {}}); + }); + }); }); // Separate describe block for Onyx.init to control initialization during each test. @@ -4535,3 +4673,68 @@ describe('get() should prefer cache over stale storage', () => { expect(value).toHaveProperty('lastVisitTime', '2025-01-01'); }); }); + +describe('null-free cache with init options', () => { + let cache: typeof OnyxCache; + + beforeEach(() => { + Object.assign(OnyxUtils.getDeferredInitTask(), createDeferredTask()); + cache = require('../../lib/OnyxCache').default; + }); + + afterEach(() => { + jest.restoreAllMocks(); + return Onyx.clear(); + }); + + it('should reset a key to its default without nested nulls, keeping the same reference on every clear', async () => { + // Given a default key state that holds nested nulls + Onyx.init({ + keys: ONYX_KEYS, + initialKeyStates: {[ONYX_KEYS.OTHER_TEST]: {errors: null, nested: {a: null, b: 1}, isLoading: false}}, + }); + await act(async () => waitForPromisesToResolve()); + await Onyx.merge(ONYX_KEYS.OTHER_TEST, {isLoading: true}); + const callback = jest.fn(); + const connection = Onyx.connect({key: ONYX_KEYS.OTHER_TEST, callback}); + await act(async () => waitForPromisesToResolve()); + callback.mockClear(); + + // When Onyx is cleared twice + await Onyx.clear(); + const valueAfterFirstClear = cache.get(ONYX_KEYS.OTHER_TEST); + await Onyx.clear(); + + // Then the default has no nested nulls and the second clear keeps the same reference without notifying again + expect(valueAfterFirstClear).toStrictEqual({nested: {b: 1}, isLoading: false}); + expect(cache.get(ONYX_KEYS.OTHER_TEST)).toBe(valueAfterFirstClear); + expect(callback).toHaveBeenCalledTimes(1); + + connection.unsubscribe(); + }); + + it('should remove nested nulls from a value synced from another instance', async () => { + // Given Onyx syncing values written by other instances + Onyx.init({keys: ONYX_KEYS, shouldSyncMultipleInstances: true}); + await act(async () => waitForPromisesToResolve()); + const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls.at(-1)?.[0]; + let receivedValue: unknown; + const connection = Onyx.connect({ + key: ONYX_KEYS.TEST_KEY, + callback: (value) => { + receivedValue = value; + }, + }); + await act(async () => waitForPromisesToResolve()); + + // When another instance stores a value that holds nested nulls + syncCallback([[ONYX_KEYS.TEST_KEY, {a: {b: null, c: 1}, d: null}]]); + await act(async () => waitForPromisesToResolve()); + + // Then the cache and subscribers get the value without the nested nulls + expect(cache.get(ONYX_KEYS.TEST_KEY)).toStrictEqual({a: {c: 1}}); + expect(receivedValue).toStrictEqual({a: {c: 1}}); + + connection.unsubscribe(); + }); +}); diff --git a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts index 0c4c9416d..921372288 100644 --- a/tests/unit/storage/providers/IDBKeyvalProviderTest.ts +++ b/tests/unit/storage/providers/IDBKeyvalProviderTest.ts @@ -81,6 +81,17 @@ describe('IDBKeyValProvider', () => { it('should return null if there is no stored value for the key', async () => { expect(await IDBKeyValProvider.getItem(ONYXKEYS.TEST_KEY)).toBeNull(); }); + + it('should remove nested nulls from a stored value', async () => { + // Given a value in storage that holds nested nulls + await IDB.set(ONYXKEYS.TEST_KEY_3, {a: 1, b: null, nested: {c: null, d: 'd'}, list: [null, {e: null}]}, IDBKeyValProvider.store); + + // When it is read + const value = await IDBKeyValProvider.getItem(ONYXKEYS.TEST_KEY_3); + + // Then the nested nulls are removed, and arrays are left as they are + expect(value).toEqual({a: 1, nested: {d: 'd'}, list: [null, {e: null}]}); + }); }); describe('multiGet', () => { diff --git a/tests/unit/storage/providers/SQLiteProviderTest.ts b/tests/unit/storage/providers/SQLiteProviderTest.ts index e4ef73686..2b0112ace 100644 --- a/tests/unit/storage/providers/SQLiteProviderTest.ts +++ b/tests/unit/storage/providers/SQLiteProviderTest.ts @@ -223,6 +223,17 @@ describe('SQLiteProvider', () => { it('should return null if there is no stored value for the key', async () => { expect(await SQLiteProvider.getItem(ONYXKEYS.TEST_KEY)).toBeNull(); }); + + it('should remove nested nulls from a stored value', async () => { + // Given a stored value with nested nulls, which JSON_REPLACE patches can write + await SQLiteProvider.store?.executeAsync('REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, ?);', [ + ONYXKEYS.TEST_KEY_3, + JSON.stringify({a: 1, b: null, nested: {c: null, d: 'd'}, list: [null, {e: null}]}), + ]); + + // Then the value is handed out without the nested nulls, and arrays are left as they are + expect(await SQLiteProvider.getItem(ONYXKEYS.TEST_KEY_3)).toEqual({a: 1, nested: {d: 'd'}, list: [null, {e: null}]}); + }); }); describe('multiGet', () => { @@ -429,6 +440,28 @@ describe('SQLiteProvider', () => { expect((stored.outer as Record).nested).not.toHaveProperty('oldKey'); expect((stored.outer as Record).nested).not.toHaveProperty('keepMe'); }); + + it('should apply a replacement nested inside an earlier replacement', async () => { + await SQLiteProvider.setItem(ONYXKEYS.TEST_KEY_3, {outer: {oldKey: 'gone'}}); + + // Given the patches from a batch of merges {outer: null}, {outer: {nested: null}}, {outer: {nested: {newKey: 'newValue'}}} + const change = { + outer: { + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + nested: {[utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, newKey: 'newValue'}, + }, + }; + const replaceNullPatches: Array<[string[], unknown]> = [ + [['outer'], {nested: null}], + [['outer', 'nested'], {newKey: 'newValue'}], + ]; + + // When the patches are applied + await SQLiteProvider.multiMerge([[ONYXKEYS.TEST_KEY_3, change, replaceNullPatches]]); + + // Then the second patch finds the path kept by the first one + expect(await SQLiteProvider.getItem(ONYXKEYS.TEST_KEY_3)).toEqual({outer: {nested: {newKey: 'newValue'}}}); + }); }); describe('mergeItem', () => { diff --git a/tests/unit/utilsTest.ts b/tests/unit/utilsTest.ts index e15f99ed2..f6b60bddd 100644 --- a/tests/unit/utilsTest.ts +++ b/tests/unit/utilsTest.ts @@ -187,6 +187,16 @@ describe('utils', () => { }); }); + it('should remove nested nulls from the replacing object only when "shouldRemoveNestedNulls" is true', () => { + const source = {b: {d: {[utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, h: 'h', i: null, j: {k: null}}}}; + + const withRemoval = utils.fastMerge(testObject, source, {shouldRemoveNestedNulls: true, objectRemovalMode: 'replace'}); + expect(withRemoval.result).toStrictEqual({a: 'a', b: {c: 'c', d: {h: 'h', j: {}}, g: 'g'}}); + + const withoutRemoval = utils.fastMerge(testObject, source, {objectRemovalMode: 'replace'}); + expect(withoutRemoval.result).toStrictEqual({a: 'a', b: {c: 'c', d: {h: 'h', i: null, j: {k: null}}, g: 'g'}}); + }); + test.each([ ['a string', 'value'], ['a number', 1000], @@ -572,6 +582,66 @@ describe('utils', () => { const result = utils.removeNestedNullValues(value); expect(result).toBe(value); }); + + it('should only copy the objects along the path of a removed null', () => { + const untouched = {x: {y: 1}}; + const sibling = {z: 2}; + const value = {a: untouched, b: {c: {d: null, e: sibling}}, f: [null]}; + const result = utils.removeNestedNullValues(value) as GenericDeepRecord; + expect(result).toStrictEqual({a: {x: {y: 1}}, b: {c: {e: {z: 2}}}, f: [null]}); + expect(result.a).toBe(untouched); + expect((result.b as GenericDeepRecord).c).not.toBe(value.b.c); + expect(((result.b as GenericDeepRecord).c as GenericDeepRecord).e).toBe(sibling); + expect(result.f).toBe(value.f); + expect(value.b.c).toHaveProperty('d', null); + }); + + it('should keep the key order when the first removal happens after other keys', () => { + const value = {a: 1, b: {c: 1}, d: null, e: 'e', f: undefined, g: {h: null}}; + const result = utils.removeNestedNullValues(value); + expect(Object.keys(result)).toEqual(['a', 'b', 'e', 'g']); + expect(result).toStrictEqual({a: 1, b: {c: 1}, e: 'e', g: {}}); + expect((result as GenericDeepRecord).b).toBe(value.b); + }); + }); + + describe('nullFreeReference', () => { + it('should return the value as is when it is the reference itself', () => { + const value = {a: {b: 1}}; + expect(utils.removeNestedNullValues(value, value)).toBe(value); + }); + + it('should not traverse subtrees that are shared with the reference', () => { + let reads = 0; + const shared = { + get x() { + reads++; + return 1; + }, + }; + const value = {a: shared, b: null, c: {d: null}}; + const result = utils.removeNestedNullValues(value, {a: shared, c: {}}) as GenericDeepRecord; + expect(reads).toBe(0); + expect(result.a).toBe(shared); + expect(Object.keys(result)).toEqual(['a', 'c']); + expect(result.c).toStrictEqual({}); + }); + + it('should still remove nulls from nested subtrees that differ from the reference', () => { + const sharedD = {e: 1}; + const reference = {a: {b: {c: 1}, d: sharedD}, f: 1}; + const value = {a: {b: {c: null, g: 2}, d: sharedD}, f: null}; + const result = utils.removeNestedNullValues(value, reference) as GenericDeepRecord; + expect(result).toStrictEqual({a: {b: {g: 2}, d: {e: 1}}}); + expect((result.a as GenericDeepRecord).d).toBe(sharedD); + }); + + it('should ignore references that are not plain objects', () => { + const value = {0: {a: null}}; + expect(utils.removeNestedNullValues(value, [value[0]])).toStrictEqual({0: {}}); + expect(utils.removeNestedNullValues(value, 'string')).toStrictEqual({0: {}}); + expect(utils.removeNestedNullValues(value, null)).toStrictEqual({0: {}}); + }); }); }); @@ -654,3 +724,78 @@ describe('utils', () => { }); }); }); + +/** Small deterministic PRNG so failures are reproducible. */ +function createRandom(seed: number) { + let state = seed % 4294967296 || 1; + return () => { + state = (state * 1664525 + 1013904223) % 4294967296; + return state / 4294967296; + }; +} + +type Value = unknown; + +function randomLeaf(random: () => number): Value { + const roll = random(); + if (roll < 0.25) { + return Math.floor(random() * 100); + } + if (roll < 0.5) { + return `s${Math.floor(random() * 100)}`; + } + if (roll < 0.65) { + return [Math.floor(random() * 10), null, {a: null}]; + } + if (roll < 0.8) { + return random() < 0.5; + } + return null; +} + +function randomObject(random: () => number, depth: number): Record { + const object: Record = {}; + const size = 1 + Math.floor(random() * 6); + for (let i = 0; i < size; i++) { + object[`k${Math.floor(random() * 8)}`] = depth > 0 && random() < 0.4 ? randomObject(random, depth - 1) : randomLeaf(random); + } + return object; +} + +/** Builds a value that shares random subtrees (by reference) with `reference`, plus random new or changed parts. */ +function deriveValue(random: () => number, reference: Record, depth: number): Record { + const value: Record = {}; + for (const key of Object.keys(reference)) { + const roll = random(); + const referenceProperty = reference[key]; + if (roll < 0.5) { + value[key] = referenceProperty; + } else if (roll < 0.7 && depth > 0 && referenceProperty && typeof referenceProperty === 'object' && !Array.isArray(referenceProperty)) { + value[key] = deriveValue(random, referenceProperty as Record, depth - 1); + } else if (roll < 0.9) { + value[key] = depth > 0 && random() < 0.4 ? randomObject(random, depth - 1) : randomLeaf(random); + } + } + if (random() < 0.5) { + value[`n${Math.floor(random() * 5)}`] = depth > 0 && random() < 0.5 ? randomObject(random, depth - 1) : randomLeaf(random); + } + return value; +} + +describe('removeNestedNullValues with a null-free reference', () => { + it('returns the same result as without a reference for any null-free reference', () => { + // Given 5000 random values that share random subtrees with a null-free reference (like a cached value) + const random = createRandom(42); + for (let i = 0; i < 5000; i++) { + const reference = utils.removeNestedNullValues(randomObject(random, 3)) as Record; + const value = deriveValue(random, reference, 3); + + // When nulls are removed with and without the reference + const withReference = utils.removeNestedNullValues(value as never, reference); + const withoutReference = utils.removeNestedNullValues(value as never); + + // Then the content is identical, because skipping a shared subtree must never skip a null + expect(withReference).toStrictEqual(withoutReference); + } + }); +});