diff --git a/API-INTERNAL.md b/API-INTERNAL.md index a9f75cbea..d413c0219 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -139,18 +139,13 @@ whatever it is we attempted to do.

broadcastUpdate()

Notifies subscribers and writes current value to cache

-
removeNullValues() ⇒
-

Removes a key from storage if the value is null. -Otherwise removes all nested null values in objects, -if shouldRemoveNestedNulls is true and returns the object.

-
prepareKeyValuePairsForStorage() ⇒

Storage expects array like: [["@MyApp_user", value_1], ["@MyApp_key", value_2]] This method transforms an object like {'@MyApp_user': myUserValue, '@MyApp_key': myKeyValue} to an array of key-value pairs in the above format and removes key-value pairs that are being set to null

-
applyMerge(changes)
-

Merges an array of changes with an existing value

+
mergeChanges(changes)
+

Merges an array of changes with an existing value or creates a single change

initializeWithDefaultKeyStates()

Merge user provided default key value pairs.

@@ -464,14 +459,6 @@ whatever it is we attempted to do. ## broadcastUpdate() Notifies subscribers and writes current value to cache -**Kind**: global function - - -## removeNullValues() ⇒ -Removes a key from storage if the value is null. -Otherwise removes all nested null values in objects, -if shouldRemoveNestedNulls is true and returns the object. - **Kind**: global function **Returns**: The value without null values and a boolean "wasRemoved", which indicates if the key got removed completely @@ -483,10 +470,10 @@ to an array of key-value pairs in the above format and removes key-value pairs t **Kind**: global function **Returns**: an array of key - value pairs <[key, value]> - + -## applyMerge(changes) -Merges an array of changes with an existing value +## mergeChanges(changes) +Merges an array of changes with an existing value or creates a single change **Kind**: global function diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 46170cdbf..e8612f37c 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -1,4 +1,3 @@ -/* eslint-disable no-continue */ import _ from 'underscore'; import * as Logger from './Logger'; import cache, {TASK} from './OnyxCache'; @@ -26,6 +25,7 @@ import type { OnyxValue, OnyxInput, OnyxMethodMap, + MultiMergeReplaceNullPatches, } from './types'; import OnyxUtils from './OnyxUtils'; import logMessages from './logMessages'; @@ -33,6 +33,7 @@ import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; import * as GlobalSettings from './GlobalSettings'; import decorateWithMetrics from './metrics'; +import OnyxMerge from './OnyxMerge'; /** Initialize the store with actions and listening for storage events */ function init({ @@ -170,38 +171,31 @@ function set(key: TKey, value: OnyxSetInput): Promis return Promise.resolve(); } - // If the value is null, we remove the key from storage - const {value: valueAfterRemoving, wasRemoved} = OnyxUtils.removeNullValues(key, value); - - const logSetCall = (hasChanged = true) => { - // Logging properties only since values could be sensitive things we don't want to log - Logger.logInfo(`set called for key: ${key}${_.isObject(value) ? ` properties: ${_.keys(value).join(',')}` : ''} hasChanged: ${hasChanged}`); - }; - - // Calling "OnyxUtils.removeNullValues" removes the key from storage and cache and updates the subscriber. + // If the change is null, we can just delete the key. // Therefore, we don't need to further broadcast and update the value so we can return early. - if (wasRemoved) { - logSetCall(); + if (value === null) { + OnyxUtils.remove(key); + Logger.logInfo(`set called for key: ${key} => null passed, so key was removed`); return Promise.resolve(); } - const valueWithoutNullValues = valueAfterRemoving as OnyxValue; - const hasChanged = cache.hasValueChanged(key, valueWithoutNullValues); + const valueWithoutNestedNullValues = utils.removeNestedNullValues(value) as OnyxValue; + const hasChanged = cache.hasValueChanged(key, valueWithoutNestedNullValues); - logSetCall(hasChanged); + Logger.logInfo(`set called for key: ${key}${_.isObject(value) ? ` properties: ${_.keys(value).join(',')}` : ''} hasChanged: ${hasChanged}`); // This approach prioritizes fast UI changes without waiting for data to be stored in device storage. - const updatePromise = OnyxUtils.broadcastUpdate(key, valueWithoutNullValues, hasChanged); + const updatePromise = OnyxUtils.broadcastUpdate(key, valueWithoutNestedNullValues, hasChanged); // If the value has not changed or the key got removed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. if (!hasChanged) { return updatePromise; } - return Storage.setItem(key, valueWithoutNullValues) - .catch((error) => OnyxUtils.evictStorageAndRetry(error, set, key, valueWithoutNullValues)) + return Storage.setItem(key, valueWithoutNestedNullValues) + .catch((error) => OnyxUtils.evictStorageAndRetry(error, set, key, valueWithoutNestedNullValues)) .then(() => { - OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNullValues); + OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNestedNullValues); return updatePromise; }); } @@ -313,8 +307,6 @@ function merge(key: TKey, changes: OnyxMergeInput): } try { - // We first only merge the changes, so we can provide these to the native implementation (SQLite uses only delta changes in "JSON_PATCH" to merge) - // We don't want to remove null values from the "batchedDeltaChanges", because SQLite uses them to remove keys from storage natively. const validChanges = mergeQueue[key].filter((change) => { const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(change, existingValue); if (!isCompatible) { @@ -326,53 +318,21 @@ function merge(key: TKey, changes: OnyxMergeInput): if (!validChanges.length) { return Promise.resolve(); } - const batchedDeltaChanges = OnyxUtils.applyMerge(undefined, validChanges, false); - // Case (1): When there is no existing value in storage, we want to set the value instead of merge it. - // Case (2): The presence of a top-level `null` in the merge queue instructs us to drop the whole existing value. - // In this case, we can't simply merge the batched changes with the existing value, because then the null in the merge queue would have no effect - const shouldSetValue = !existingValue || mergeQueue[key].includes(null); - - // Clean up the write queue, so we don't apply these changes again + // Clean up the write queue, so we don't apply these changes again. delete mergeQueue[key]; delete mergeQueuePromise[key]; - const logMergeCall = (hasChanged = true) => { - // Logging properties only since values could be sensitive things we don't want to log - Logger.logInfo(`merge called for key: ${key}${_.isObject(batchedDeltaChanges) ? ` properties: ${_.keys(batchedDeltaChanges).join(',')}` : ''} hasChanged: ${hasChanged}`); - }; - - // If the batched changes equal null, we want to remove the key from storage, to reduce storage size - const {wasRemoved} = OnyxUtils.removeNullValues(key, batchedDeltaChanges); - - // Calling "OnyxUtils.removeNullValues" removes the key from storage and cache and updates the subscriber. + // If the last change is null, we can just delete the key. // Therefore, we don't need to further broadcast and update the value so we can return early. - if (wasRemoved) { - logMergeCall(); + if (validChanges.at(-1) === null) { + Logger.logInfo(`merge called for key: ${key} => null passed, so key was removed`); + OnyxUtils.remove(key); return Promise.resolve(); } - // For providers that can't handle delta changes, we need to merge the batched changes with the existing value beforehand. - // The "preMergedValue" will be directly "set" in storage instead of being merged - // Therefore we merge the batched changes with the existing value to get the final merged value that will be stored. - // We can remove null values from the "preMergedValue", because "null" implicates that the user wants to remove a value from storage. - const preMergedValue = OnyxUtils.applyMerge(shouldSetValue ? undefined : existingValue, [batchedDeltaChanges], true); - - // In cache, we don't want to remove the key if it's null to improve performance and speed up the next merge. - const hasChanged = cache.hasValueChanged(key, preMergedValue); - - logMergeCall(hasChanged); - - // This approach prioritizes fast UI changes without waiting for data to be stored in device storage. - const updatePromise = OnyxUtils.broadcastUpdate(key, preMergedValue as OnyxValue, hasChanged); - - // If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. - if (!hasChanged) { - return updatePromise; - } - - return Storage.mergeItem(key, batchedDeltaChanges as OnyxValue, preMergedValue as OnyxValue, shouldSetValue).then(() => { - OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MERGE, key, changes, preMergedValue); + return OnyxMerge.applyMerge(key, existingValue, validChanges).then(({mergedValue, updatePromise}) => { + OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MERGE, key, changes, mergedValue); return updatePromise; }); } catch (error) { @@ -397,7 +357,11 @@ function merge(key: TKey, changes: OnyxMergeInput): * @param collectionKey e.g. `ONYXKEYS.COLLECTION.REPORT` * @param collection Object collection keyed by individual collection member keys and values */ -function mergeCollection(collectionKey: TKey, collection: OnyxMergeCollectionInput): Promise { +function mergeCollection( + collectionKey: TKey, + collection: OnyxMergeCollectionInput, + mergeReplaceNullPatches?: MultiMergeReplaceNullPatches, +): Promise { if (!OnyxUtils.isValidNonEmptyCollectionForMerge(collection)) { Logger.logInfo('mergeCollection() called with invalid or empty value. Skipping this update.'); return Promise.resolve(); @@ -447,10 +411,12 @@ function mergeCollection(collectionKey: TK const existingKeyCollection = existingKeys.reduce((obj: OnyxInputKeyValueMapping, key) => { const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(resultCollection[key], cachedCollectionForExistingKeys[key]); + if (!isCompatible) { Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'mergeCollection', existingValueType, newValueType)); return obj; } + // eslint-disable-next-line no-param-reassign obj[key] = resultCollection[key]; return obj; @@ -467,7 +433,7 @@ function mergeCollection(collectionKey: TK // When (multi-)merging the values with the existing values in storage, // we don't want to remove nested null values from the data that we pass to the storage layer, // because the storage layer uses them to remove nested keys from storage natively. - const keyValuePairsForExistingCollection = OnyxUtils.prepareKeyValuePairsForStorage(existingKeyCollection, false); + const keyValuePairsForExistingCollection = OnyxUtils.prepareKeyValuePairsForStorage(existingKeyCollection, false, mergeReplaceNullPatches); // We can safely remove nested null values when using (multi-)set, // because we will simply overwrite the existing values in storage. @@ -717,24 +683,31 @@ function update(data: OnyxUpdate[]): Promise { // Remove the collection-related key from the updateQueue so that it won't be processed individually. delete updateQueue[key]; - const updatedValue = OnyxUtils.applyMerge(undefined, operations, false); + const batchedChanges = OnyxUtils.mergeAndMarkChanges(operations); if (operations[0] === null) { // eslint-disable-next-line no-param-reassign - queue.set[key] = updatedValue; + queue.set[key] = batchedChanges.result; } else { // eslint-disable-next-line no-param-reassign - queue.merge[key] = updatedValue; + queue.merge[key] = batchedChanges.result; + if (batchedChanges.replaceNullPatches.length > 0) { + // eslint-disable-next-line no-param-reassign + queue.mergeReplaceNullPatches[key] = batchedChanges.replaceNullPatches; + } } return queue; }, { merge: {}, + mergeReplaceNullPatches: {}, set: {}, }, ); if (!utils.isEmptyObject(batchedCollectionUpdates.merge)) { - promises.push(() => mergeCollection(collectionKey, batchedCollectionUpdates.merge as Collection)); + promises.push(() => + mergeCollection(collectionKey, batchedCollectionUpdates.merge as Collection, batchedCollectionUpdates.mergeReplaceNullPatches), + ); } if (!utils.isEmptyObject(batchedCollectionUpdates.set)) { promises.push(() => multiSet(batchedCollectionUpdates.set)); @@ -742,13 +715,16 @@ function update(data: OnyxUpdate[]): Promise { }); Object.entries(updateQueue).forEach(([key, operations]) => { - const batchedChanges = OnyxUtils.applyMerge(undefined, operations, false); - if (operations[0] === null) { + const batchedChanges = OnyxUtils.mergeAndMarkChanges(operations).result; promises.push(() => set(key, batchedChanges)); - } else { - promises.push(() => merge(key, batchedChanges)); + return; } + + const mergePromises = operations.map((operation) => { + return merge(key, operation); + }); + promises.push(() => mergePromises.at(0) ?? Promise.resolve()); }); const snapshotPromises = OnyxUtils.updateSnapshots(data, merge); diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 30848b83c..e0d6bb8be 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -182,7 +182,12 @@ class OnyxCache { throw new Error('data passed to cache.merge() must be an Object of onyx key/value pairs'); } - this.storageMap = {...utils.fastMerge(this.storageMap, data)}; + this.storageMap = { + ...utils.fastMerge(this.storageMap, data, { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }).result, + }; Object.entries(data).forEach(([key, value]) => { this.addKey(key); diff --git a/lib/OnyxMerge/index.native.ts b/lib/OnyxMerge/index.native.ts new file mode 100644 index 000000000..afe7661c9 --- /dev/null +++ b/lib/OnyxMerge/index.native.ts @@ -0,0 +1,44 @@ +import _ from 'underscore'; +import * as Logger from '../Logger'; +import OnyxUtils from '../OnyxUtils'; +import type {OnyxKey, OnyxValue} from '../types'; +import cache from '../OnyxCache'; +import Storage from '../storage'; +import type {ApplyMerge} from './types'; + +const applyMerge: ApplyMerge = (key: TKey, existingValue: OnyxValue, validChanges: unknown[]) => { + // If any of the changes is null, we need to discard the existing value. + const baseValue = validChanges.includes(null) ? undefined : existingValue; + + // We first batch the changes into a single change with object removal marks, + // so that SQLite can merge the changes more efficiently. + const {result: batchedChanges, replaceNullPatches} = OnyxUtils.mergeAndMarkChanges(validChanges); + + // We then merge the batched changes with the existing value, because we need to final merged value to broadcast to subscribers. + const {result: mergedValue} = OnyxUtils.mergeChanges([batchedChanges], baseValue); + + // In cache, we don't want to remove the key if it's null to improve performance and speed up the next merge. + const hasChanged = cache.hasValueChanged(key, mergedValue); + + // Logging properties only since values could be sensitive things we don't want to log. + Logger.logInfo(`merge called for key: ${key}${_.isObject(mergedValue) ? ` properties: ${_.keys(mergedValue).join(',')}` : ''} hasChanged: ${hasChanged}`); + + // This approach prioritizes fast UI changes without waiting for data to be stored in device storage. + const updatePromise = OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue, hasChanged); + + // If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. + if (!hasChanged) { + return Promise.resolve({mergedValue, updatePromise}); + } + + return Storage.mergeItem(key, batchedChanges as OnyxValue, replaceNullPatches).then(() => ({ + mergedValue, + updatePromise, + })); +}; + +const OnyxMerge = { + applyMerge, +}; + +export default OnyxMerge; diff --git a/lib/OnyxMerge/index.ts b/lib/OnyxMerge/index.ts new file mode 100644 index 000000000..b08378884 --- /dev/null +++ b/lib/OnyxMerge/index.ts @@ -0,0 +1,36 @@ +import _ from 'underscore'; +import * as Logger from '../Logger'; +import OnyxUtils from '../OnyxUtils'; +import type {OnyxKey, OnyxValue} from '../types'; +import cache from '../OnyxCache'; +import Storage from '../storage'; +import type {ApplyMerge} from './types'; + +const applyMerge: ApplyMerge = (key: TKey, existingValue: OnyxValue, validChanges: unknown[]) => { + const {result: mergedValue} = OnyxUtils.mergeChanges(validChanges, existingValue); + + // In cache, we don't want to remove the key if it's null to improve performance and speed up the next merge. + const hasChanged = cache.hasValueChanged(key, mergedValue); + + // Logging properties only since values could be sensitive things we don't want to log. + Logger.logInfo(`merge called for key: ${key}${_.isObject(mergedValue) ? ` properties: ${_.keys(mergedValue).join(',')}` : ''} hasChanged: ${hasChanged}`); + + // This approach prioritizes fast UI changes without waiting for data to be stored in device storage. + const updatePromise = OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue, hasChanged); + + // If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. + if (!hasChanged) { + return Promise.resolve({mergedValue, updatePromise}); + } + + return Storage.setItem(key, mergedValue as OnyxValue).then(() => ({ + mergedValue, + updatePromise, + })); +}; + +const OnyxMerge = { + applyMerge, +}; + +export default OnyxMerge; diff --git a/lib/OnyxMerge/types.ts b/lib/OnyxMerge/types.ts new file mode 100644 index 000000000..51405c9ac --- /dev/null +++ b/lib/OnyxMerge/types.ts @@ -0,0 +1,10 @@ +import type {OnyxKey, OnyxValue} from '../types'; + +type ApplyMergeResult = { + mergedValue: OnyxValue; + updatePromise: Promise; +}; + +type ApplyMerge = (key: TKey, existingValue: OnyxValue, validChanges: unknown[]) => Promise; + +export type {ApplyMerge, ApplyMergeResult}; diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 0d86b0191..3b50f1740 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -19,6 +19,7 @@ import type { DefaultConnectOptions, KeyValueMapping, Mapping, + MultiMergeReplaceNullPatches, OnyxCollection, OnyxEntry, OnyxInput, @@ -28,12 +29,14 @@ import type { OnyxValue, Selector, } from './types'; +import type {FastMergeOptions, FastMergeResult} from './utils'; import utils from './utils'; import type {WithOnyxState} from './withOnyx/types'; import type {DeferredTask} from './createDeferredTask'; import createDeferredTask from './createDeferredTask'; import * as GlobalSettings from './GlobalSettings'; import decorateWithMetrics from './metrics'; +import type {StorageKeyValuePair} from './storage/providers/types'; // Method constants const METHOD = { @@ -1140,34 +1143,6 @@ function hasPendingMergeForKey(key: OnyxKey): boolean { return !!mergeQueue[key]; } -type RemoveNullValuesOutput | undefined> = { - value: Value; - wasRemoved: boolean; -}; - -/** - * Removes a key from storage if the value is null. - * Otherwise removes all nested null values in objects, - * if shouldRemoveNestedNulls is true and returns the object. - * - * @returns The value without null values and a boolean "wasRemoved", which indicates if the key got removed completely - */ -function removeNullValues | undefined>(key: OnyxKey, value: Value, shouldRemoveNestedNulls = true): RemoveNullValuesOutput { - if (value === null) { - remove(key); - return {value, wasRemoved: true}; - } - - if (value === undefined) { - return {value, wasRemoved: false}; - } - - // We can remove all null values in an object by merging it with itself - // utils.fastMerge recursively goes through the object and removes all null values - // Passing two identical objects as source and target to fastMerge will not change it, but only remove the null values - return {value: shouldRemoveNestedNulls ? utils.removeNestedNullValues(value) : value, wasRemoved: false}; -} - /** * Storage expects array like: [["@MyApp_user", value_1], ["@MyApp_key", value_2]] * This method transforms an object like {'@MyApp_user': myUserValue, '@MyApp_key': myKeyValue} @@ -1175,42 +1150,81 @@ function removeNullValues | undefined>(key: Ony * @return an array of key - value pairs <[key, value]> */ -function prepareKeyValuePairsForStorage(data: Record>, shouldRemoveNestedNulls: boolean): Array<[OnyxKey, OnyxInput]> { - return Object.entries(data).reduce]>>((pairs, [key, value]) => { - const {value: valueAfterRemoving, wasRemoved} = removeNullValues(key, value, shouldRemoveNestedNulls); +function prepareKeyValuePairsForStorage( + data: Record>, + shouldRemoveNestedNulls?: boolean, + replaceNullPatches?: MultiMergeReplaceNullPatches, +): StorageKeyValuePair[] { + const pairs: StorageKeyValuePair[] = []; + + Object.entries(data).forEach(([key, value]) => { + if (value === null) { + remove(key); + return; + } - if (!wasRemoved && valueAfterRemoving !== undefined) { - pairs.push([key, valueAfterRemoving]); + const valueWithoutNestedNullValues = shouldRemoveNestedNulls ?? true ? utils.removeNestedNullValues(value) : value; + + if (valueWithoutNestedNullValues !== undefined) { + pairs.push([key, valueWithoutNestedNullValues, replaceNullPatches?.[key]]); } + }); - return pairs; - }, []); + return pairs; +} + +function mergeChanges | undefined, TChange extends OnyxInput | undefined>(changes: TChange[], existingValue?: TValue): FastMergeResult { + return applyMerge('merge', changes, existingValue); +} + +function mergeAndMarkChanges | undefined, TChange extends OnyxInput | undefined>( + changes: TChange[], + existingValue?: TValue, +): FastMergeResult { + return applyMerge('mark', changes, existingValue); } /** - * Merges an array of changes with an existing value + * Merges an array of changes with an existing value or creates a single change * - * @param changes Array of changes that should be applied to the existing value + * @param changes Array of changes that should be merged + * @param existingValue The existing value that should be merged with the changes */ function applyMerge | undefined, TChange extends OnyxInput | undefined>( - existingValue: TValue, + mode: 'merge' | 'mark', changes: TChange[], - shouldRemoveNestedNulls: boolean, -): TChange { + existingValue?: TValue, +): FastMergeResult { const lastChange = changes?.at(-1); if (Array.isArray(lastChange)) { - return lastChange; + return {result: lastChange, replaceNullPatches: []}; } if (changes.some((change) => change && typeof change === 'object')) { // Object values are then merged one after the other - return changes.reduce((modifiedData, change) => utils.fastMerge(modifiedData, change, shouldRemoveNestedNulls), (existingValue || {}) as TChange); + return changes.reduce>( + (modifiedData, change) => { + const options: FastMergeOptions = mode === 'merge' ? {shouldRemoveNestedNulls: true, objectRemovalMode: 'replace'} : {objectRemovalMode: 'mark'}; + const {result, replaceNullPatches} = utils.fastMerge(modifiedData.result, change, options); + + // eslint-disable-next-line no-param-reassign + modifiedData.result = result; + // eslint-disable-next-line no-param-reassign + modifiedData.replaceNullPatches = [...modifiedData.replaceNullPatches, ...replaceNullPatches]; + + return modifiedData; + }, + { + result: (existingValue ?? {}) as TChange, + replaceNullPatches: [], + }, + ); } // If we have anything else we can't merge it so we'll // simply return the last value that was queued - return lastChange as TChange; + return {result: lastChange as TChange, replaceNullPatches: []}; } /** @@ -1220,7 +1234,9 @@ function initializeWithDefaultKeyStates(): Promise { return Storage.multiGet(Object.keys(defaultKeyStates)).then((pairs) => { const existingDataAsObject = Object.fromEntries(pairs); - const merged = utils.fastMerge(existingDataAsObject, defaultKeyStates); + const merged = utils.fastMerge(existingDataAsObject, defaultKeyStates, { + shouldRemoveNestedNulls: true, + }).result; cache.merge(merged ?? {}); Object.entries(merged ?? {}).forEach(([key, value]) => keyChanged(key, value, existingDataAsObject)); @@ -1462,9 +1478,9 @@ const OnyxUtils = { evictStorageAndRetry, broadcastUpdate, hasPendingMergeForKey, - removeNullValues, prepareKeyValuePairsForStorage, - applyMerge, + mergeChanges, + mergeAndMarkChanges, initializeWithDefaultKeyStates, getSnapshotKey, multiGet, diff --git a/lib/storage/InstanceSync/index.web.ts b/lib/storage/InstanceSync/index.web.ts index 67b309791..99a7fe325 100644 --- a/lib/storage/InstanceSync/index.web.ts +++ b/lib/storage/InstanceSync/index.web.ts @@ -5,7 +5,7 @@ */ import type {OnyxKey} from '../../types'; import NoopProvider from '../providers/NoopProvider'; -import type {KeyList, OnStorageKeyChanged} from '../providers/types'; +import type {StorageKeyList, OnStorageKeyChanged} from '../providers/types'; import type StorageProvider from '../providers/types'; const SYNC_ONYX = 'SYNC_ONYX'; @@ -19,7 +19,7 @@ function raiseStorageSyncEvent(onyxKey: OnyxKey) { global.localStorage.removeItem(SYNC_ONYX); } -function raiseStorageSyncManyKeysEvent(onyxKeys: KeyList) { +function raiseStorageSyncManyKeysEvent(onyxKeys: StorageKeyList) { onyxKeys.forEach((onyxKey) => { raiseStorageSyncEvent(onyxKey); }); @@ -54,12 +54,12 @@ const InstanceSync = { multiSet: raiseStorageSyncManyKeysEvent, mergeItem: raiseStorageSyncEvent, clear: (clearImplementation: () => void) => { - let allKeys: KeyList; + let allKeys: StorageKeyList; // The keys must be retrieved before storage is cleared or else the list of keys would be empty return storage .getAllKeys() - .then((keys: KeyList) => { + .then((keys: StorageKeyList) => { allKeys = keys; }) .then(() => clearImplementation()) diff --git a/lib/storage/index.ts b/lib/storage/index.ts index 938b615a5..97ec7ceed 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -116,9 +116,9 @@ const storage: Storage = { /** * Merging an existing value with a new one */ - mergeItem: (key, deltaChanges, preMergedValue, shouldSetValue = false) => + mergeItem: (key, change, replaceNullPatches) => tryOrDegradePerformance(() => { - const promise = provider.mergeItem(key, deltaChanges, preMergedValue, shouldSetValue); + const promise = provider.mergeItem(key, change, replaceNullPatches); if (shouldKeepInstancesSync) { return promise.then(() => InstanceSync.mergeItem(key)); diff --git a/lib/storage/providers/IDBKeyValProvider.ts b/lib/storage/providers/IDBKeyValProvider.ts index 9b749ab86..64d419a83 100644 --- a/lib/storage/providers/IDBKeyValProvider.ts +++ b/lib/storage/providers/IDBKeyValProvider.ts @@ -49,15 +49,18 @@ const provider: StorageProvider = { const upsertMany = pairsWithoutNull.map(([key, value], index) => { const prev = values[index]; - const newValue = utils.fastMerge(prev as Record, value as Record); + const newValue = utils.fastMerge(prev as Record, value as Record, { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }).result; return promisifyRequest(store.put(newValue, key)); }); - return Promise.all(upsertMany); + return Promise.all(upsertMany).then(() => undefined); }); }), - mergeItem(key, _deltaChanges, preMergedValue) { - // Since Onyx also merged the existing value with the changes, we can just set the value directly - return provider.setItem(key, preMergedValue); + mergeItem(key, change) { + // Since Onyx already merged the existing value with the changes, we can just set the value directly. + return provider.multiMerge([[key, change]]); }, multiSet: (pairs) => { const pairsWithoutNull = pairs.filter(([key, value]) => { @@ -67,7 +70,7 @@ const provider: StorageProvider = { } return true; - }); + }) as Array<[IDBValidKey, unknown]>; return setMany(pairsWithoutNull, idbKeyValStore); }, diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts index f67cff12a..1510d20cb 100644 --- a/lib/storage/providers/MemoryOnlyProvider.ts +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -1,7 +1,7 @@ import _ from 'underscore'; import utils from '../../utils'; import type StorageProvider from './types'; -import type {KeyValuePair} from './types'; +import type {StorageKeyValuePair} from './types'; import type {OnyxKey, OnyxValue} from '../../types'; type Store = Record>; @@ -49,7 +49,7 @@ const provider: StorageProvider = { new Promise((resolve) => { this.getItem(key).then((value) => resolve([key, value])); }), - ) as Array>; + ) as Array>; return Promise.all(getPromises); }, @@ -74,9 +74,9 @@ const provider: StorageProvider = { /** * Merging an existing value with a new one */ - mergeItem(key, _deltaChanges, preMergedValue) { - // Since Onyx already merged the existing value with the changes, we can just set the value directly - return this.setItem(key, preMergedValue); + mergeItem(key, change) { + // Since Onyx already merged the existing value with the changes, we can just set the value directly. + return this.multiMerge([[key, change]]); }, /** @@ -86,12 +86,15 @@ const provider: StorageProvider = { multiMerge(pairs) { _.forEach(pairs, ([key, value]) => { const existingValue = store[key] as Record; - const newValue = utils.fastMerge(existingValue, value as Record) as OnyxValue; + const newValue = utils.fastMerge(existingValue, value as Record, { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }).result as OnyxValue; set(key, newValue); }); - return Promise.resolve([]); + return Promise.resolve(); }, /** diff --git a/lib/storage/providers/NoopProvider.ts b/lib/storage/providers/NoopProvider.ts index f99af069c..ccbee65a6 100644 --- a/lib/storage/providers/NoopProvider.ts +++ b/lib/storage/providers/NoopProvider.ts @@ -54,7 +54,7 @@ const provider: StorageProvider = { * This function also removes all nested null values from an object. */ multiMerge() { - return Promise.resolve([]); + return Promise.resolve(); }, /** diff --git a/lib/storage/providers/SQLiteProvider.ts b/lib/storage/providers/SQLiteProvider.ts index e087b01eb..2183d8d04 100644 --- a/lib/storage/providers/SQLiteProvider.ts +++ b/lib/storage/providers/SQLiteProvider.ts @@ -2,12 +2,13 @@ * The SQLiteStorage provider stores everything in a key/value store by * converting the value to a JSON string */ -import type {BatchQueryResult, NitroSQLiteConnection} from 'react-native-nitro-sqlite'; +import type {BatchQueryCommand, NitroSQLiteConnection} from 'react-native-nitro-sqlite'; import {enableSimpleNullHandling, open} from 'react-native-nitro-sqlite'; import {getFreeDiskStorage} from 'react-native-device-info'; -import type StorageProvider from './types'; +import type {FastMergeReplaceNullPatch} from '../../utils'; import utils from '../../utils'; -import type {KeyList, KeyValuePairList} from './types'; +import type StorageProvider from './types'; +import type {StorageKeyList, StorageKeyValuePair} from './types'; // By default, NitroSQLite does not accept nullish values due to current limitations in Nitro Modules. // This flag enables a feature in NitroSQLite that allows for nullish values to be passed to operations, such as "execute" or "executeBatch". @@ -43,6 +44,20 @@ type PageCountResult = { const DB_NAME = 'OnyxDB'; let db: NitroSQLiteConnection; +function replacer(key: string, value: unknown) { + if (key === utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK) return undefined; + return value; +} + +function generateJSONReplaceSQLQueries(key: string, patches: FastMergeReplaceNullPatch[]): string[][] { + const queries = patches.map(([pathArray, value]) => { + const jsonPath = `$.${pathArray.join('.')}`; + return [jsonPath, JSON.stringify(value), key]; + }); + + return queries; +} + const provider: StorageProvider = { /** * The name of the provider that can be printed to the logs @@ -82,11 +97,11 @@ const provider: StorageProvider = { return db.executeAsync(command, keys).then(({rows}) => { // eslint-disable-next-line no-underscore-dangle const result = rows?._array.map((row) => [row.record_key, JSON.parse(row.valueJSON)]); - return (result ?? []) as KeyValuePairList; + return (result ?? []) as StorageKeyValuePair[]; }); }, setItem(key, value) { - return db.executeAsync('REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, ?);', [key, JSON.stringify(value)]); + return db.executeAsync('REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, ?);', [key, JSON.stringify(value)]).then(() => undefined); }, multiSet(pairs) { const query = 'REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, json(?));'; @@ -94,45 +109,66 @@ const provider: StorageProvider = { if (utils.isEmptyObject(params)) { return Promise.resolve(); } - return db.executeBatchAsync([{query, params}]); + return db.executeBatchAsync([{query, params}]).then(() => undefined); }, multiMerge(pairs) { - // Note: We use `ON CONFLICT DO UPDATE` here instead of `INSERT OR REPLACE INTO` - // so the new JSON value is merged into the old one if there's an existing value - const query = `INSERT INTO keyvaluepairs (record_key, valueJSON) - VALUES (:key, JSON(:value)) - ON CONFLICT DO UPDATE - SET valueJSON = JSON_PATCH(valueJSON, JSON(:value)); + const commands: BatchQueryCommand[] = []; + + const patchQuery = `INSERT INTO keyvaluepairs (record_key, valueJSON) + VALUES (:key, JSON(:value)) + ON CONFLICT DO UPDATE + SET valueJSON = JSON_PATCH(valueJSON, JSON(:value)); `; + const patchQueryArguments: string[][] = []; + const replaceQuery = `UPDATE keyvaluepairs + SET valueJSON = JSON_REPLACE(valueJSON, ?, JSON(?)) + WHERE record_key = ?; + `; + const replaceQueryArguments: string[][] = []; - const nonUndefinedPairs = pairs.filter((pair) => pair[1] !== undefined); - const params = nonUndefinedPairs.map((pair) => { - const value = JSON.stringify(pair[1]); - return [pair[0], value]; - }); + const nonNullishPairs = pairs.filter((pair) => pair[1] !== undefined); - return db.executeBatchAsync([{query, params}]); - }, - mergeItem(key, deltaChanges, preMergedValue, shouldSetValue) { - if (shouldSetValue) { - return this.setItem(key, preMergedValue) as Promise; + // eslint-disable-next-line @typescript-eslint/prefer-for-of + for (let i = 0; i < nonNullishPairs.length; i++) { + const [key, value, replaceNullPatches] = nonNullishPairs[i]; + + const valueAfterReplace = JSON.stringify(value, replacer); + patchQueryArguments.push([key, valueAfterReplace]); + + const patches = replaceNullPatches ?? []; + if (patches.length > 0) { + const queries = generateJSONReplaceSQLQueries(key, patches); + + if (queries.length > 0) { + replaceQueryArguments.push(...queries); + } + } + } + + commands.push({query: patchQuery, params: patchQueryArguments}); + if (replaceQueryArguments.length > 0) { + commands.push({query: replaceQuery, params: replaceQueryArguments}); } - return this.multiMerge([[key, deltaChanges]]) as Promise; + return db.executeBatchAsync(commands).then(() => undefined); + }, + mergeItem(key, change, replaceNullPatches) { + // Since Onyx already merged the existing value with the changes, we can just set the value directly. + return this.multiMerge([[key, change, replaceNullPatches]]); }, getAllKeys: () => db.executeAsync('SELECT record_key FROM keyvaluepairs;').then(({rows}) => { // eslint-disable-next-line no-underscore-dangle const result = rows?._array.map((row) => row.record_key); - return (result ?? []) as KeyList; + return (result ?? []) as StorageKeyList; }), - removeItem: (key) => db.executeAsync('DELETE FROM keyvaluepairs WHERE record_key = ?;', [key]), + removeItem: (key) => db.executeAsync('DELETE FROM keyvaluepairs WHERE record_key = ?;', [key]).then(() => undefined), removeItems: (keys) => { const placeholders = keys.map(() => '?').join(','); const query = `DELETE FROM keyvaluepairs WHERE record_key IN (${placeholders});`; - return db.executeAsync(query, keys); + return db.executeAsync(query, keys).then(() => undefined); }, - clear: () => db.executeAsync('DELETE FROM keyvaluepairs;', []), + clear: () => db.executeAsync('DELETE FROM keyvaluepairs;', []).then(() => undefined), getDatabaseSize() { return Promise.all([db.executeAsync('PRAGMA page_size;'), db.executeAsync('PRAGMA page_count;'), getFreeDiskStorage()]).then( ([pageSizeResult, pageCountResult, bytesRemaining]) => { diff --git a/lib/storage/providers/types.ts b/lib/storage/providers/types.ts index adbec3ff6..db7525aa5 100644 --- a/lib/storage/providers/types.ts +++ b/lib/storage/providers/types.ts @@ -1,9 +1,13 @@ -import type {BatchQueryResult, QueryResult} from 'react-native-nitro-sqlite'; import type {OnyxKey, OnyxValue} from '../../types'; +import type {FastMergeReplaceNullPatch} from '../../utils'; -type KeyValuePair = [OnyxKey, OnyxValue]; -type KeyList = OnyxKey[]; -type KeyValuePairList = KeyValuePair[]; +type StorageKeyValuePair = [key: OnyxKey, value: OnyxValue, replaceNullPatches?: FastMergeReplaceNullPatch[]]; +type StorageKeyList = OnyxKey[]; + +type DatabaseSize = { + bytesUsed: number; + bytesRemaining: number; +}; type OnStorageKeyChanged = (key: TKey, value: OnyxValue) => void; @@ -24,55 +28,53 @@ type StorageProvider = { /** * Get multiple key-value pairs for the given array of keys in a batch */ - multiGet: (keys: KeyList) => Promise; + multiGet: (keys: StorageKeyList) => Promise; /** * Sets the value for a given key. The only requirement is that the value should be serializable to JSON string */ - setItem: (key: TKey, value: OnyxValue) => Promise; + setItem: (key: TKey, value: OnyxValue) => Promise; /** * Stores multiple key-value pairs in a batch */ - multiSet: (pairs: KeyValuePairList) => Promise; + multiSet: (pairs: StorageKeyValuePair[]) => Promise; /** * Multiple merging of existing and new values in a batch */ - multiMerge: (pairs: KeyValuePairList) => Promise; + multiMerge: (pairs: StorageKeyValuePair[]) => Promise; /** - * Merges an existing value with a new one by leveraging JSON_PATCH - * @param deltaChanges - the delta for a specific key - * @param preMergedValue - the pre-merged data from `Onyx.applyMerge` - * @param shouldSetValue - whether the data should be set instead of merged + * Merges an existing value with a new one + * @param change - the change to merge with the existing value */ - mergeItem: (key: TKey, deltaChanges: OnyxValue, preMergedValue: OnyxValue, shouldSetValue?: boolean) => Promise; + mergeItem: (key: TKey, change: OnyxValue, replaceNullPatches?: FastMergeReplaceNullPatch[]) => Promise; /** * Returns all keys available in storage */ - getAllKeys: () => Promise; + getAllKeys: () => Promise; /** * Removes given key and its value from storage */ - removeItem: (key: OnyxKey) => Promise; + removeItem: (key: OnyxKey) => Promise; /** * Removes given keys and their values from storage */ - removeItems: (keys: KeyList) => Promise; + removeItems: (keys: StorageKeyList) => Promise; /** * Clears absolutely everything from storage */ - clear: () => Promise; + clear: () => Promise; /** * Gets the total bytes of the database file */ - getDatabaseSize: () => Promise<{bytesUsed: number; bytesRemaining: number}>; + getDatabaseSize: () => Promise; /** * @param onStorageKeyChanged Storage synchronization mechanism keeping all opened tabs in sync @@ -81,4 +83,4 @@ type StorageProvider = { }; export default StorageProvider; -export type {KeyList, KeyValuePair, KeyValuePairList, OnStorageKeyChanged}; +export type {StorageKeyList, StorageKeyValuePair, OnStorageKeyChanged}; diff --git a/lib/types.ts b/lib/types.ts index fa835666b..b6cd8a1af 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -3,6 +3,7 @@ import type {BuiltIns} from 'type-fest/source/internal'; import type OnyxUtils from './OnyxUtils'; import type {WithOnyxInstance, WithOnyxState} from './withOnyx/types'; import type {OnyxMethod} from './OnyxUtils'; +import type {FastMergeReplaceNullPatch} from './utils'; /** * Utility type that excludes `null` from the type `TValue`. @@ -485,11 +486,14 @@ type InitOptions = { // eslint-disable-next-line @typescript-eslint/no-explicit-any type GenericFunction = (...args: any[]) => any; +type MultiMergeReplaceNullPatches = {[TKey in OnyxKey]: FastMergeReplaceNullPatch[]}; + /** * Represents a combination of Merge and Set operations that should be executed in Onyx */ type MixedOperationsQueue = { merge: OnyxInputKeyValueMapping; + mergeReplaceNullPatches: MultiMergeReplaceNullPatches; set: OnyxInputKeyValueMapping; }; @@ -531,5 +535,6 @@ export type { OnyxValue, Selector, WithOnyxConnectOptions, + MultiMergeReplaceNullPatches, MixedOperationsQueue, }; diff --git a/lib/utils.ts b/lib/utils.ts index de82315bf..49e044860 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -3,29 +3,81 @@ import type {ConnectOptions, OnyxInput, OnyxKey} from './types'; type EmptyObject = Record; type EmptyValue = EmptyObject | null | undefined; -/** Checks whether the given object is an object and not null/undefined. */ -function isEmptyObject(obj: T | EmptyValue): obj is EmptyValue { - return typeof obj === 'object' && Object.keys(obj || {}).length === 0; -} +type FastMergeReplaceNullPatch = [string[], unknown]; + +type FastMergeOptions = { + /** If true, null object values will be removed. */ + shouldRemoveNestedNulls?: boolean; + + /** + * If set to "mark", we will mark objects that are set to null instead of simply removing them, + * so that we can batch changes together, without losing information about the object removal. + * If set to "replace", we will completely replace the marked objects with the new value instead of merging them. + * */ + objectRemovalMode?: 'mark' | 'replace' | 'none'; +}; + +type FastMergeMetadata = { + /** The path to the object that contains the internal "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag. */ + replaceNullPatches: FastMergeReplaceNullPatch[]; +}; + +type FastMergeResult = { + /** The result of the merge. */ + result: TValue; -// Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 + /** The path to the object that contains the internal "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag. */ + replaceNullPatches: FastMergeReplaceNullPatch[]; +}; + +const ONYX_INTERNALS__REPLACE_OBJECT_MARK = 'ONYX_INTERNALS__REPLACE_OBJECT_MARK'; /** - * Checks whether the given value can be merged. It has to be an object, but not an array, RegExp or Date. + * Merges two objects and removes null values if "shouldRemoveNestedNulls" is set to true + * + * We generally want to remove null values from objects written to disk and cache, because it decreases the amount of data stored in memory and on disk. */ -function isMergeableObject(value: unknown): value is Record { - const isNonNullObject = value != null ? typeof value === 'object' : false; - return isNonNullObject && !(value instanceof RegExp) && !(value instanceof Date) && !Array.isArray(value); +function fastMerge(target: TValue, source: TValue, options?: FastMergeOptions, metadata?: FastMergeMetadata, basePath: string[] = []): FastMergeResult { + if (!metadata) { + // eslint-disable-next-line no-param-reassign + metadata = { + replaceNullPatches: [], + }; + } + + // We have to ignore arrays and nullish values here, + // otherwise "mergeObject" will throw an error, + // because it expects an object as "source" + if (Array.isArray(source) || source === null || source === undefined) { + return {result: source, replaceNullPatches: metadata.replaceNullPatches}; + } + + const optionsWithDefaults: FastMergeOptions = { + shouldRemoveNestedNulls: options?.shouldRemoveNestedNulls ?? false, + objectRemovalMode: options?.objectRemovalMode ?? 'none', + }; + + const mergedValue = mergeObject(target, source as Record, optionsWithDefaults, metadata, basePath) as TValue; + + return {result: mergedValue, replaceNullPatches: metadata.replaceNullPatches}; } /** * Merges the source object into the target object. * @param target - The target object. * @param source - The source object. - * @param shouldRemoveNestedNulls - If true, null object values will be removed. + * @param options - The options for the merge. + * @param metadata - The metadata for the merge. + * @param basePath - The base path for the merge. * @returns - The merged object. */ -function mergeObject>(target: TObject | unknown | null | undefined, source: TObject, shouldRemoveNestedNulls = true): TObject { +function mergeObject>( + target: TObject | unknown | null | undefined, + source: TObject, + options: FastMergeOptions, + metadata: FastMergeMetadata, + basePath: string[], +): TObject { const destination: Record = {}; const targetObject = isMergeableObject(target) ? target : undefined; @@ -35,80 +87,87 @@ function mergeObject>(target: TObject | // If "shouldRemoveNestedNulls" is true, we want to remove null values from the merged object // and therefore we need to omit keys where either the source or target value is null. if (targetObject) { - // eslint-disable-next-line no-restricted-syntax, guard-for-in - for (const key in targetObject) { - const sourceValue = source?.[key]; - const targetValue = targetObject?.[key]; - - // If "shouldRemoveNestedNulls" is true, we want to remove null values from the merged object. - // Therefore, if either target or source value is null, we want to prevent the key from being set. - // targetValue should techincally never be "undefined", because it will always be a value from cache or storage - // and we never set "undefined" there. Still, if there targetValue is undefined we don't want to set - // the key explicitly to prevent loose undefined values in objects in cache and storage. - const isSourceOrTargetNull = targetValue === undefined || targetValue === null || sourceValue === null; - const shouldOmitTargetKey = shouldRemoveNestedNulls && isSourceOrTargetNull; - - if (!shouldOmitTargetKey) { - destination[key] = targetValue; + Object.keys(targetObject).forEach((key) => { + const targetProperty = targetObject?.[key]; + const sourceProperty = source?.[key]; + + // If "shouldRemoveNestedNulls" is true, we want to remove (nested) null values from the merged object. + // If either the source or target value is null, we want to omit the key from the merged object. + const shouldOmitNullishProperty = options.shouldRemoveNestedNulls && (targetProperty === null || sourceProperty === null); + + if (targetProperty === undefined || shouldOmitNullishProperty) { + return; } - } + + destination[key] = targetProperty; + }); } // After copying over all keys from the target object, we want to merge the source object into the destination object. - // eslint-disable-next-line no-restricted-syntax, guard-for-in - for (const key in source) { - const sourceValue = source?.[key] as Record; - const targetValue = targetObject?.[key]; - - // If undefined is passed as the source value for a key, we want to generally ignore it. - // If "shouldRemoveNestedNulls" is set to true and the source value is null, - // we don't want to set/merge the source value into the merged object. - const shouldIgnoreNullSourceValue = shouldRemoveNestedNulls && sourceValue === null; - const shouldOmitSourceKey = sourceValue === undefined || shouldIgnoreNullSourceValue; - - if (!shouldOmitSourceKey) { - // If the source value is a mergable object, we want to merge it into the target value. - // If "shouldRemoveNestedNulls" is true, "fastMerge" will recursively - // remove nested null values from the merged object. - // If source value is any other value we need to set the source value it directly. - if (isMergeableObject(sourceValue)) { - // If the target value is null or undefined, we need to fallback to an empty object, - // so that we can still use "fastMerge" to merge the source value, - // to ensure that nested null values are removed from the merged object. - const targetValueWithFallback = (targetValue ?? {}) as TObject; - destination[key] = fastMerge(targetValueWithFallback, sourceValue, shouldRemoveNestedNulls); - } else { - destination[key] = sourceValue; - } + Object.keys(source).forEach((key) => { + let targetProperty = targetObject?.[key]; + const sourceProperty = source?.[key] as Record; + + // If "shouldRemoveNestedNulls" is true, we want to remove (nested) null values from the merged object. + // If either the source value is null, we want to omit the key from the merged object. + const shouldOmitNullishProperty = options.shouldRemoveNestedNulls && sourceProperty === null; + + if (sourceProperty === undefined || shouldOmitNullishProperty) { + return; + } + + // If source value is not a mergable object, we need to set the source value it directly. + if (!isMergeableObject(sourceProperty)) { + destination[key] = sourceProperty; + return; } - } + + // If "shouldMarkRemovedObjects" is enabled and the previous merge change (targetProperty) is null, + // it means we want to fully replace this object when merging the batched changes with the Onyx value. + // To achieve this, we first mark these nested objects with an internal flag. + // When calling fastMerge again with "mark" removal mode, the marked objects will be removed. + if (options.objectRemovalMode === 'mark' && targetProperty === null) { + targetProperty = {[ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true}; + metadata.replaceNullPatches.push([[...basePath, key], {...sourceProperty}]); + } + + // Later, when merging the batched changes with the Onyx value, if a nested object of the batched changes + // has the internal flag set, we replace the entire destination object with the source one and remove + // the flag. + if (options.objectRemovalMode === 'replace' && sourceProperty[ONYX_INTERNALS__REPLACE_OBJECT_MARK]) { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + delete sourceProperty[ONYX_INTERNALS__REPLACE_OBJECT_MARK]; + destination[key] = sourceProperty; + return; + } + + destination[key] = fastMerge(targetProperty, sourceProperty, options, metadata, [...basePath, key]).result; + }); return destination as TObject; } +/** Checks whether the given object is an object and not null/undefined. */ +function isEmptyObject(obj: T | EmptyValue): obj is EmptyValue { + return typeof obj === 'object' && Object.keys(obj || {}).length === 0; +} + /** - * Merges two objects and removes null values if "shouldRemoveNestedNulls" is set to true - * - * We generally want to remove null values from objects written to disk and cache, because it decreases the amount of data stored in memory and on disk. - * On native, when merging an existing value with new changes, SQLite will use JSON_PATCH, which removes top-level nullish values. - * To be consistent with the behaviour for merge, we'll also want to remove null values for "set" operations. + * Checks whether the given value can be merged. It has to be an object, but not an array, RegExp or Date. + * Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1. */ -function fastMerge(target: TValue, source: TValue, shouldRemoveNestedNulls = true): TValue { - // We have to ignore arrays and nullish values here, - // otherwise "mergeObject" will throw an error, - // because it expects an object as "source" - if (Array.isArray(source) || source === null || source === undefined) { - return source; - } - - return mergeObject(target, source as Record, shouldRemoveNestedNulls) as TValue; +function isMergeableObject>(value: unknown): value is TObject { + const isNonNullObject = value != null ? typeof value === 'object' : false; + return isNonNullObject && !(value instanceof RegExp) && !(value instanceof Date) && !Array.isArray(value); } /** Deep removes the nested null values from the given value. */ function removeNestedNullValues | null>(value: TValue): TValue { if (typeof value === 'object' && !Array.isArray(value)) { - const objectValue = value as Record; - return fastMerge(objectValue, objectValue) as TValue; + return fastMerge(value, value, { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }).result; } return value; @@ -200,4 +259,15 @@ function hasWithOnyxInstance(mapping: ConnectOptions return 'withOnyxInstance' in mapping && mapping.withOnyxInstance; } -export default {isEmptyObject, fastMerge, formatActionName, removeNestedNullValues, checkCompatibilityWithExistingValue, pick, omit, hasWithOnyxInstance}; +export default { + fastMerge, + isEmptyObject, + formatActionName, + removeNestedNullValues, + checkCompatibilityWithExistingValue, + pick, + omit, + hasWithOnyxInstance, + ONYX_INTERNALS__REPLACE_OBJECT_MARK, +}; +export type {FastMergeResult, FastMergeReplaceNullPatch, FastMergeOptions}; diff --git a/tests/perf-test/OnyxUtils.perf-test.ts b/tests/perf-test/OnyxUtils.perf-test.ts index 997bf8f0e..737e61bd6 100644 --- a/tests/perf-test/OnyxUtils.perf-test.ts +++ b/tests/perf-test/OnyxUtils.perf-test.ts @@ -610,15 +610,6 @@ describe('OnyxUtils', () => { }); }); - describe('removeNullValues', () => { - test('one call with one heavy object', async () => { - const key = `${collectionKey}0`; - const reportAction = mockedReportActionsMap[`${collectionKey}0`]; - - await measureFunction(() => OnyxUtils.removeNullValues(key, reportAction, true)); - }); - }); - describe('prepareKeyValuePairsForStorage', () => { test('one call with 10k heavy objects', async () => { await measureFunction(() => OnyxUtils.prepareKeyValuePairsForStorage(mockedReportActionsMap, false)); @@ -634,9 +625,7 @@ describe('OnyxUtils', () => { const changedReportAction4 = createRandomReportAction(Number(reportAction.reportActionID)); const changedReportAction5 = createRandomReportAction(Number(reportAction.reportActionID)); - await measureFunction(() => - OnyxUtils.applyMerge(reportAction, [changedReportAction1, changedReportAction2, changedReportAction3, changedReportAction4, changedReportAction5], false), - ); + await measureFunction(() => OnyxUtils.mergeChanges([changedReportAction1, changedReportAction2, changedReportAction3, changedReportAction4, changedReportAction5], reportAction)); }); }); diff --git a/tests/perf-test/utils.perf-test.ts b/tests/perf-test/utils.perf-test.ts index 73f6e08f6..6b7458852 100644 --- a/tests/perf-test/utils.perf-test.ts +++ b/tests/perf-test/utils.perf-test.ts @@ -15,6 +15,10 @@ describe('[Utils.js]', () => { const target = getMockedPersonalDetails(1000); const source = getMockedPersonalDetails(500); - await measureFunction(() => utils.fastMerge(target, source, true)); + await measureFunction(() => + utils.fastMerge(target, source, { + shouldRemoveNestedNulls: true, + }), + ); }); }); diff --git a/tests/unit/fastMergeTest.ts b/tests/unit/fastMergeTest.ts index d20ba80ea..8821bdb1c 100644 --- a/tests/unit/fastMergeTest.ts +++ b/tests/unit/fastMergeTest.ts @@ -36,11 +36,27 @@ const testObjectWithNullValuesRemoved: DeepObject = { }, }; +const testMergeChanges: DeepObject[] = [ + { + b: { + d: { + h: 'h', + }, + }, + }, + { + b: { + d: null, + h: 'h', + }, + }, +]; + describe('fastMerge', () => { it('should merge an object with another object and remove nested null values', () => { - const result = utils.fastMerge(testObject, testObjectWithNullishValues); + const result = utils.fastMerge(testObject, testObjectWithNullishValues, {shouldRemoveNestedNulls: true}); - expect(result).toEqual({ + expect(result.result).toEqual({ a: 'a', b: { c: { @@ -55,9 +71,9 @@ describe('fastMerge', () => { }); it('should merge an object with another object and not remove nested null values', () => { - const result = utils.fastMerge(testObject, testObjectWithNullishValues, false); + const result = utils.fastMerge(testObject, testObjectWithNullishValues); - expect(result).toEqual({ + expect(result.result).toEqual({ a: 'a', b: { c: { @@ -73,9 +89,11 @@ describe('fastMerge', () => { }); it('should merge an object with an empty object and remove deeply nested null values', () => { - const result = utils.fastMerge({}, testObjectWithNullishValues); + const result = utils.fastMerge({}, testObjectWithNullishValues, { + shouldRemoveNestedNulls: true, + }); - expect(result).toEqual(testObjectWithNullValuesRemoved); + expect(result.result).toEqual(testObjectWithNullValuesRemoved); }); it('should remove null values by merging two identical objects with fastMerge', () => { @@ -86,15 +104,68 @@ describe('fastMerge', () => { it('should replace an object with an array', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any - const result = utils.fastMerge(testObject, [1, 2, 3] as any); + const result = utils.fastMerge(testObject, [1, 2, 3] as any, { + shouldRemoveNestedNulls: true, + }); - expect(result).toEqual([1, 2, 3]); + expect(result.result).toEqual([1, 2, 3]); }); it('should replace an array with an object', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any - const result = utils.fastMerge([1, 2, 3] as any, testObject); + const result = utils.fastMerge([1, 2, 3] as any, testObject, { + shouldRemoveNestedNulls: true, + }); + + expect(result.result).toEqual(testObject); + }); + + it('should add the "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag to the merged object when the change is set to null and "objectRemovalMode" is set to "mark"', () => { + const result = utils.fastMerge(testMergeChanges[1], testMergeChanges[0], { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'mark', + }); + + expect(result.result).toEqual({ + b: { + d: { + h: 'h', + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + }, + h: 'h', + }, + }); + expect(result.replaceNullPatches).toEqual([[['b', 'd'], {h: 'h'}]]); + }); - expect(result).toEqual(testObject); + it('should completely replace the target object with its source when the source has the "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag and "shouldReplaceMarkedObjects" is true', () => { + const result = utils.fastMerge( + testObject, + { + b: { + d: { + h: 'h', + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + }, + h: 'h', + }, + }, + { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }, + ); + + expect(result.result).toEqual({ + a: 'a', + b: { + c: 'c', + d: { + h: 'h', + }, + h: 'h', + g: 'g', + }, + }); }); }); diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 9c5972819..1644e2079 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1,9 +1,10 @@ import lodashClone from 'lodash/clone'; +import lodashCloneDeep from 'lodash/cloneDeep'; import Onyx from '../../lib'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import OnyxUtils from '../../lib/OnyxUtils'; import type OnyxCache from '../../lib/OnyxCache'; -import type {OnyxCollection, OnyxUpdate} from '../../lib/types'; +import type {DeepRecord, OnyxCollection, OnyxUpdate} from '../../lib/types'; import type GenericCollection from '../utils/GenericCollection'; import type {Connection} from '../../lib/OnyxConnectionManager'; @@ -1700,7 +1701,425 @@ describe('Onyx', () => { }); }); + it('should replace the old value after a null merge in the top-level object when batching updates', async () => { + let result: unknown; + connection = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_UPDATE, + waitForCollectionCallback: true, + callback: (value) => { + result = value; + }, + }); + + await Onyx.multiSet({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { + id: 'entry1', + someKey: 'someValue', + }, + }); + + const queuedUpdates: OnyxUpdate[] = [ + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + // Removing the entire object in this update. + // Any subsequent changes to this key should completely replace the old value. + value: null, + }, + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + // This change should completely replace `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1` old value. + value: { + someKey: 'someValueChanged', + }, + }, + ]; + + await Onyx.update(queuedUpdates); + + expect(result).toEqual({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { + someKey: 'someValueChanged', + }, + }); + }); + + describe('should replace the old value after a null merge in a nested property when batching updates', () => { + let result: unknown; + + beforeEach(() => { + connection = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_UPDATE, + waitForCollectionCallback: true, + callback: (value) => { + result = value; + }, + }); + }); + + it('replacing old object after null merge', async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const entry1: DeepRecord = { + sub_entry1: { + id: 'sub_entry1', + someKey: 'someValue', + }, + }; + await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1}); + + const entry1ExpectedResult = lodashCloneDeep(entry1); + const queuedUpdates: OnyxUpdate[] = []; + + queuedUpdates.push({ + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + value: { + // Removing the "sub_entry1" object in this update. + // Any subsequent changes to this object should completely replace the existing object in store. + sub_entry1: null, + }, + }); + delete entry1ExpectedResult.sub_entry1; + + queuedUpdates.push({ + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + value: { + // This change should completely replace "sub_entry1" existing object in store. + sub_entry1: { + newKey: 'newValue', + }, + }, + }); + entry1ExpectedResult.sub_entry1 = {newKey: 'newValue'}; + + await Onyx.update(queuedUpdates); + + expect(result).toEqual({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1ExpectedResult}); + }); + + it('setting new object after null merge', async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const entry1: DeepRecord = { + sub_entry1: { + id: 'sub_entry1', + someKey: 'someValue', + someNestedObject: { + someNestedKey: 'someNestedValue', + anotherNestedObject: { + anotherNestedKey: 'anotherNestedValue', + }, + }, + }, + }; + await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1}); + + const entry1ExpectedResult = lodashCloneDeep(entry1); + const queuedUpdates: OnyxUpdate[] = []; + + queuedUpdates.push({ + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + value: { + sub_entry1: { + someNestedObject: { + // Introducing a new "anotherNestedObject2" object in this update. + anotherNestedObject2: { + anotherNestedKey2: 'anotherNestedValue2', + }, + }, + }, + }, + }); + entry1ExpectedResult.sub_entry1.someNestedObject.anotherNestedObject2 = {anotherNestedKey2: 'anotherNestedValue2'}; + + queuedUpdates.push({ + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + value: { + sub_entry1: { + someNestedObject: { + // Removing the "anotherNestedObject2" object in this update. + // This property was only introduced in a previous update, so we don't need to care + // about an old existing value because there isn't one. + anotherNestedObject2: null, + }, + }, + }, + }); + delete entry1ExpectedResult.sub_entry1.someNestedObject.anotherNestedObject2; + + queuedUpdates.push({ + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + value: { + sub_entry1: { + someNestedObject: { + // Introducing the "anotherNestedObject2" object again with this update. + anotherNestedObject2: { + newNestedKey2: 'newNestedValue2', + }, + }, + }, + }, + }); + entry1ExpectedResult.sub_entry1.someNestedObject.anotherNestedObject2 = {newNestedKey2: 'newNestedValue2'}; + + await Onyx.update(queuedUpdates); + + expect(result).toEqual({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1ExpectedResult}); + }); + + it('setting new object after null merge of a primitive property', async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const entry1: DeepRecord = { + sub_entry1: { + id: 'sub_entry1', + someKey: 'someValue', + someNestedObject: { + someNestedKey: 'someNestedValue', + anotherNestedObject: { + anotherNestedKey: 'anotherNestedValue', + }, + }, + }, + }; + await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1}); + + const entry1ExpectedResult = lodashCloneDeep(entry1); + const queuedUpdates: OnyxUpdate[] = []; + + queuedUpdates.push({ + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + value: { + sub_entry1: { + someNestedObject: { + anotherNestedObject: { + // Removing the "anotherNestedKey" property in this update. + // This property's existing value in store is a primitive value, so we don't need to care + // about it when merging new values in any next updates. + anotherNestedKey: null, + }, + }, + }, + }, + }); + delete entry1ExpectedResult.sub_entry1.someNestedObject.anotherNestedObject.anotherNestedKey; + + queuedUpdates.push({ + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + value: { + sub_entry1: { + someNestedObject: { + anotherNestedObject: { + // Setting a new object to the "anotherNestedKey" property. + anotherNestedKey: { + newNestedKey: 'newNestedValue', + }, + }, + }, + }, + }, + }); + entry1ExpectedResult.sub_entry1.someNestedObject.anotherNestedObject.anotherNestedKey = {newNestedKey: 'newNestedValue'}; + + await Onyx.update(queuedUpdates); + + expect(result).toEqual({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1ExpectedResult}); + }); + }); + describe('merge', () => { + it('should replace the old value after a null merge in the top-level object when batching merges', async () => { + let result: unknown; + connection = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_UPDATE, + waitForCollectionCallback: true, + callback: (value) => { + result = value; + }, + }); + + await Onyx.multiSet({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { + id: 'entry1', + someKey: 'someValue', + }, + }); + + // Removing the entire object in this merge. + // Any subsequent changes to this key should completely replace the old value. + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, null); + + // This change should completely replace `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1` old value. + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + someKey: 'someValueChanged', + }); + + await waitForPromisesToResolve(); + + expect(result).toEqual({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: {someKey: 'someValueChanged'}}); + }); + + describe('should replace the old value after a null merge in a nested property when batching merges', () => { + let result: unknown; + + beforeEach(() => { + connection = Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_UPDATE, + waitForCollectionCallback: true, + callback: (value) => { + result = value; + }, + }); + }); + + it('replacing old object after null merge', async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const entry1: DeepRecord = { + sub_entry1: { + id: 'sub_entry1', + someKey: 'someValue', + }, + }; + await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1}); + + const entry1ExpectedResult = lodashCloneDeep(entry1); + + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + // Removing the "sub_entry1" object in this merge. + // Any subsequent changes to this object should completely replace the existing object in store. + sub_entry1: null, + }); + delete entry1ExpectedResult.sub_entry1; + + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + // This change should completely replace "sub_entry1" existing object in store. + sub_entry1: { + newKey: 'newValue', + }, + }); + entry1ExpectedResult.sub_entry1 = {newKey: 'newValue'}; + + await waitForPromisesToResolve(); + + expect(result).toEqual({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1ExpectedResult}); + }); + + it('setting new object after null merge', async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const entry1: DeepRecord = { + sub_entry1: { + id: 'sub_entry1', + someKey: 'someValue', + someNestedObject: { + someNestedKey: 'someNestedValue', + anotherNestedObject: { + anotherNestedKey: 'anotherNestedValue', + }, + }, + }, + }; + await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1}); + + const entry1ExpectedResult = lodashCloneDeep(entry1); + + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + sub_entry1: { + someNestedObject: { + // Introducing a new "anotherNestedObject2" object in this merge. + anotherNestedObject2: { + anotherNestedKey2: 'anotherNestedValue2', + }, + }, + }, + }); + entry1ExpectedResult.sub_entry1.someNestedObject.anotherNestedObject2 = {anotherNestedKey2: 'anotherNestedValue2'}; + + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + sub_entry1: { + someNestedObject: { + // Removing the "anotherNestedObject2" object in this merge. + // This property was only introduced in a previous merge, so we don't need to care + // about an old existing value because there isn't one. + anotherNestedObject2: null, + }, + }, + }); + delete entry1ExpectedResult.sub_entry1.someNestedObject.anotherNestedObject2; + + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + sub_entry1: { + someNestedObject: { + // Introducing the "anotherNestedObject2" object again with this update. + anotherNestedObject2: { + newNestedKey2: 'newNestedValue2', + }, + }, + }, + }); + entry1ExpectedResult.sub_entry1.someNestedObject.anotherNestedObject2 = {newNestedKey2: 'newNestedValue2'}; + + await waitForPromisesToResolve(); + + expect(result).toEqual({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1ExpectedResult}); + }); + + it('setting new object after null merge of a primitive property', async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const entry1: DeepRecord = { + sub_entry1: { + id: 'sub_entry1', + someKey: 'someValue', + someNestedObject: { + someNestedKey: 'someNestedValue', + anotherNestedObject: { + anotherNestedKey: 'anotherNestedValue', + }, + }, + }, + }; + await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1}); + + const entry1ExpectedResult = lodashCloneDeep(entry1); + + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + sub_entry1: { + someNestedObject: { + anotherNestedObject: { + // Removing the "anotherNestedKey" property in this merge. + // This property's existing value in store is a primitive value, so we don't need to care + // about it when merging new values in any next merges. + anotherNestedKey: null, + }, + }, + }, + }); + delete entry1ExpectedResult.sub_entry1.someNestedObject.anotherNestedObject.anotherNestedKey; + + Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, { + sub_entry1: { + someNestedObject: { + anotherNestedObject: { + // Setting a new object to the "anotherNestedKey" property. + anotherNestedKey: { + newNestedKey: 'newNestedValue', + }, + }, + }, + }, + }); + entry1ExpectedResult.sub_entry1.someNestedObject.anotherNestedObject.anotherNestedKey = {newNestedKey: 'newNestedValue'}; + + await waitForPromisesToResolve(); + + expect(result).toEqual({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1ExpectedResult}); + }); + }); + it('should remove a deeply nested null when merging an existing key', () => { let result: unknown; diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index 06e6be1d1..1fddf2672 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -1,5 +1,68 @@ import Onyx from '../../lib'; import OnyxUtils from '../../lib/OnyxUtils'; +import type {DeepRecord} from '../../lib/types'; +import utils from '../../lib/utils'; + +const testObject: DeepRecord = { + a: 'a', + b: { + c: 'c', + d: { + e: 'e', + f: 'f', + }, + g: 'g', + }, +}; + +const testMergeChanges: Array> = [ + { + b: { + d: { + h: 'h', + }, + }, + }, + { + b: { + // Removing "d" object. + d: null, + h: 'h', + }, + }, + { + b: { + // Adding back "d" property with a object. + // The "ONYX_INTERNALS__REPLACE_OBJECT_MARK" marker property should be added here when batching merge changes. + d: { + i: 'i', + }, + }, + }, + { + b: { + // Removing "d" object again. + d: null, + // Removing "g" object. + g: null, + }, + }, + { + b: { + // Adding back "d" property with a object. + // The "ONYX_INTERNALS__REPLACE_OBJECT_MARK" marker property should be added here when batching merge changes. + d: { + i: 'i', + j: 'j', + }, + // Adding back "g" property with a object. + // The "ONYX_INTERNALS__REPLACE_OBJECT_MARK" marker property should be added here when batching merge changes. + g: { + k: 'k', + }, + }, + }, +]; const ONYXKEYS = { TEST_KEY: 'test', @@ -87,4 +150,129 @@ describe('OnyxUtils', () => { }).toThrowError("Invalid '' key provided, only collection keys are allowed."); }); }); + + describe('mergeChanges', () => { + it("should return the last change if it's an array", () => { + const {result} = OnyxUtils.mergeChanges([...testMergeChanges, [0, 1, 2]], testObject); + + expect(result).toEqual([0, 1, 2]); + }); + + it("should return the last change if the changes aren't objects", () => { + const {result} = OnyxUtils.mergeChanges(['a', 0, 'b', 1], testObject); + + expect(result).toEqual(1); + }); + + it('should merge data correctly when applying batched changes', () => { + const batchedChanges: DeepRecord = { + b: { + d: { + i: 'i', + j: 'j', + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + }, + h: 'h', + g: { + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + k: 'k', + }, + }, + }; + + const {result} = OnyxUtils.mergeChanges([batchedChanges], testObject); + + expect(result).toEqual({ + a: 'a', + b: { + c: 'c', + d: { + i: 'i', + j: 'j', + }, + h: 'h', + g: { + k: 'k', + }, + }, + }); + }); + }); + + describe('mergeAndMarkChanges', () => { + it('should apply the replacement markers if the we have properties with objects being removed and added back during the changes', () => { + const {result, replaceNullPatches} = OnyxUtils.mergeAndMarkChanges(testMergeChanges); + + expect(result).toEqual({ + b: { + d: { + i: 'i', + j: 'j', + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + }, + h: 'h', + g: { + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + k: 'k', + }, + }, + }); + expect(replaceNullPatches).toEqual([ + [['b', 'd'], {i: 'i'}], + [['b', 'd'], {i: 'i', j: 'j'}], + [['b', 'g'], {k: 'k'}], + ]); + }); + + it('should 2', () => { + const {result, replaceNullPatches} = OnyxUtils.mergeAndMarkChanges([ + { + // Removing the "originalMessage" object in this update. + // Any subsequent changes to this object should completely replace the existing object in store. + originalMessage: null, + }, + { + // This change should completely replace "originalMessage" existing object in store. + originalMessage: { + errorMessage: 'newErrorMessage', + }, + receipt: { + // Removing the "nestedObject" object in this update. + // Any subsequent changes to this object should completely replace the existing object in store. + nestedObject: null, + }, + }, + { + receipt: { + receiptID: null, + filename: 'newFilename', + // This change should completely replace "receipt" existing object in store. + nestedObject: { + nestedKey2: 'newNestedKey2', + }, + }, + }, + ]); + + expect(result).toEqual({ + originalMessage: { + errorMessage: 'newErrorMessage', + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + }, + receipt: { + receiptID: null, + filename: 'newFilename', + nestedObject: { + nestedKey2: 'newNestedKey2', + [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, + }, + }, + }); + + expect(replaceNullPatches).toEqual([ + [['originalMessage'], {errorMessage: 'newErrorMessage'}], + [['receipt', 'nestedObject'], {nestedKey2: 'newNestedKey2'}], + ]); + }); + }); });