diff --git a/API-INTERNAL.md b/API-INTERNAL.md index d8665de66..ab9dd07cf 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 c89d24f38..4e4e3c4dd 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -1,4 +1,3 @@ -/* eslint-disable no-continue */ import _ from 'underscore'; import lodashPick from 'lodash/pick'; import * as Logger from './Logger'; @@ -27,6 +26,7 @@ import type { OnyxValue, OnyxInput, OnyxMethodMap, + MultiMergeReplaceNullPatches, } from './types'; import OnyxUtils from './OnyxUtils'; import logMessages from './logMessages'; @@ -34,6 +34,7 @@ import type {Connection} from './OnyxConnectionManager'; import connectionManager from './OnyxConnectionManager'; import * as GlobalSettings from './GlobalSettings'; import decorateWithMetrics from './metrics'; +import OnyxMerge from './OnyxMerge/index.native'; /** Initialize the store with actions and listening for storage events */ function init({ @@ -41,7 +42,7 @@ function init({ initialKeyStates = {}, safeEvictionKeys = [], maxCachedKeysCount = 1000, - shouldSyncMultipleInstances = Boolean(global.localStorage), + shouldSyncMultipleInstances = !!global.localStorage, debugSetState = false, enablePerformanceMetrics = false, skippableCollectionMemberIDs = [], @@ -169,38 +170,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; }); } @@ -307,7 +301,6 @@ function merge(key: TKey, changes: OnyxMergeInput): } try { - // We first only merge the changes, so we use OnyxUtils.batchMergeChanges() to combine all the changes into just one. const validChanges = mergeQueue[key].filter((change) => { const {isCompatible, existingValueType, newValueType} = utils.checkCompatibilityWithExistingValue(change, existingValue); if (!isCompatible) { @@ -319,54 +312,21 @@ function merge(key: TKey, changes: OnyxMergeInput): if (!validChanges.length) { return Promise.resolve(); } - const batchedDeltaChanges = OnyxUtils.batchMergeChanges(validChanges).result; - - // 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. 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(); } - // If "shouldSetValue" is true, it means that we want to completely replace the existing value with the batched changes, - // so we pass `undefined` to OnyxUtils.applyMerge() first parameter to make it use "batchedDeltaChanges" to - // create a new object for us. - // If "shouldSetValue" is false, it means that we want to merge the batched changes into the existing value, - // so we pass "existingValue" to the first parameter. - const resultValue = OnyxUtils.applyMerge(shouldSetValue ? undefined : existingValue, [batchedDeltaChanges]); - - // 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, resultValue); - - logMergeCall(hasChanged); - - // This approach prioritizes fast UI changes without waiting for data to be stored in device storage. - const updatePromise = OnyxUtils.broadcastUpdate(key, resultValue 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, resultValue as OnyxValue).then(() => { - OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MERGE, key, changes, resultValue); + return OnyxMerge.applyMerge(key, existingValue, validChanges).then(({mergedValue, updatePromise}) => { + OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MERGE, key, changes, mergedValue); return updatePromise; }); } catch (error) { @@ -394,7 +354,7 @@ function merge(key: TKey, changes: OnyxMergeInput): function mergeCollection( collectionKey: TKey, collection: OnyxMergeCollectionInput, - mergeReplaceNullPatches?: MixedOperationsQueue['mergeReplaceNullPatches'], + mergeReplaceNullPatches?: MultiMergeReplaceNullPatches, ): Promise { if (!OnyxUtils.isValidNonEmptyCollectionForMerge(collection)) { Logger.logInfo('mergeCollection() called with invalid or empty value. Skipping this update.'); @@ -445,10 +405,12 @@ function mergeCollection( 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; @@ -465,7 +427,7 @@ function mergeCollection( // 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. @@ -480,7 +442,7 @@ function mergeCollection( // New keys will be added via multiSet while existing keys will be updated using multiMerge // This is because setting a key that doesn't exist yet with multiMerge will throw errors if (keyValuePairsForExistingCollection.length > 0) { - promises.push(Storage.multiMerge(keyValuePairsForExistingCollection, mergeReplaceNullPatches)); + promises.push(Storage.multiMerge(keyValuePairsForExistingCollection)); } if (keyValuePairsForNewCollection.length > 0) { @@ -774,7 +736,7 @@ 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 batchedChanges = OnyxUtils.batchMergeChanges(operations); + const batchedChanges = OnyxUtils.mergeAndMarkChanges(operations); if (operations[0] === null) { // eslint-disable-next-line no-param-reassign queue.set[key] = batchedChanges.result; @@ -806,13 +768,16 @@ function update(data: OnyxUpdate[]): Promise { }); Object.entries(updateQueue).forEach(([key, operations]) => { - const batchedChanges = OnyxUtils.batchMergeChanges(operations).result; - 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 = updateSnapshots(data); diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 691cedfee..adc52778b 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -164,7 +164,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, true, false, true).result}; + this.storageMap = { + ...utils.fastMerge(this.storageMap, data, { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }).result, + }; Object.entries(data).forEach(([key, value]) => { this.addKey(key); @@ -227,6 +232,10 @@ class OnyxCache { const temp = []; while (numKeysToRemove > 0) { const value = iterator.next().value; + if (value === undefined) { + break; + } + temp.push(value); numKeysToRemove--; } diff --git a/lib/OnyxMerge/index.native.ts b/lib/OnyxMerge/index.native.ts new file mode 100644 index 000000000..bf323b53d --- /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, ApplyMergeResult} from './types'; + +const applyMerge: ApplyMerge = (key: TKey, existingValue: OnyxValue, validChanges: unknown[]): Promise => { + // 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..ea53203dd --- /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, ApplyMergeResult} from './types'; + +const applyMerge: ApplyMerge = (key: TKey, existingValue: OnyxValue, validChanges: unknown[]): Promise => { + 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 93b6bf651..9ac4e881b 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -20,6 +20,7 @@ import type { DefaultConnectOptions, KeyValueMapping, Mapping, + MultiMergeReplaceNullPatches, OnyxCollection, OnyxEntry, OnyxInput, @@ -28,13 +29,14 @@ import type { OnyxValue, Selector, } from './types'; -import type {FastMergeResult} from './utils'; +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 = { @@ -1204,34 +1206,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} @@ -1239,41 +1213,44 @@ 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); - - if (!wasRemoved && valueAfterRemoving !== undefined) { - pairs.push([key, valueAfterRemoving]); +function prepareKeyValuePairsForStorage( + data: Record>, + shouldRemoveNestedNulls?: boolean, + replaceNullPatches?: MultiMergeReplaceNullPatches, +): StorageKeyValuePair[] { + const pairs: StorageKeyValuePair[] = []; + + Object.entries(data).forEach(([key, value]) => { + if (value === null) { + remove(key); + return; } - return pairs; - }, []); -} + const valueWithoutNestedNullValues = shouldRemoveNestedNulls ?? true ? utils.removeNestedNullValues(value) : value; -/** - * Merges an array of changes with an existing value - * - * @param changes Array of changes that should be applied to the existing value - */ -function applyMerge | undefined, TChange extends OnyxInput | undefined>(existingValue: TValue, changes: TChange[]): TChange { - const lastChange = changes?.at(-1); + if (valueWithoutNestedNullValues !== undefined) { + pairs.push([key, valueWithoutNestedNullValues, replaceNullPatches?.[key]]); + } + }); - if (Array.isArray(lastChange)) { - return lastChange; - } + return pairs; +} - 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, true, false, true).result, (existingValue || {}) as TChange); - } +function mergeChanges | undefined>(changes: TValue[], existingValue?: TValue): FastMergeResult { + return applyMerge('merge', changes, existingValue); +} - // 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; +function mergeAndMarkChanges | undefined>(changes: TValue[], existingValue?: TValue): FastMergeResult { + return applyMerge('mark', changes, existingValue); } -function batchMergeChanges | undefined>(changes: TChange[]): FastMergeResult { +/** + * Merges an array of changes with an existing value or creates a single change + * + * @param changes Array of changes that should be merged + * @param existingValue The existing value that should be merged with the changes + */ +function applyMerge | undefined>(mode: 'merge' | 'mark', changes: TValue[], existingValue?: TValue): FastMergeResult { const lastChange = changes?.at(-1); if (Array.isArray(lastChange)) { @@ -1282,17 +1259,20 @@ function batchMergeChanges | undefined>(chang if (changes.some((change) => change && typeof change === 'object')) { // Object values are then merged one after the other - return changes.reduce>( + return changes.reduce>( (modifiedData, change) => { - const fastMergeResult = utils.fastMerge(modifiedData.result, change, false, true, false); + 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 = fastMergeResult.result; + modifiedData.result = result; // eslint-disable-next-line no-param-reassign - modifiedData.replaceNullPatches = [...modifiedData.replaceNullPatches, ...fastMergeResult.replaceNullPatches]; + modifiedData.replaceNullPatches = [...modifiedData.replaceNullPatches, ...replaceNullPatches]; + return modifiedData; }, { - result: {} as TChange, + result: (existingValue ?? {}) as TValue, replaceNullPatches: [], }, ); @@ -1300,7 +1280,7 @@ function batchMergeChanges | undefined>(chang // If we have anything else we can't merge it so we'll // simply return the last value that was queued - return {result: lastChange as TChange, replaceNullPatches: []}; + return {result: lastChange as TValue, replaceNullPatches: []}; } /** @@ -1310,7 +1290,9 @@ function initializeWithDefaultKeyStates(): Promise { return Storage.multiGet(Object.keys(defaultKeyStates)).then((pairs) => { const existingDataAsObject = Object.fromEntries(pairs); - const merged = utils.fastMerge(existingDataAsObject, defaultKeyStates, true, false, false).result; + const merged = utils.fastMerge(existingDataAsObject, defaultKeyStates, { + shouldRemoveNestedNulls: true, + }).result; cache.merge(merged ?? {}); Object.entries(merged ?? {}).forEach(([key, value]) => keyChanged(key, value, existingDataAsObject)); @@ -1373,7 +1355,7 @@ function subscribeToKey(connectOptions: ConnectOptions { 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 33befed1d..f9b9e2d57 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, preMergedValue) => + mergeItem: (key, change) => tryOrDegradePerformance(() => { - const promise = provider.mergeItem(key, preMergedValue); + const promise = provider.mergeItem(key, change); if (shouldKeepInstancesSync) { return promise.then(() => InstanceSync.mergeItem(key)); diff --git a/lib/storage/providers/IDBKeyValProvider.ts b/lib/storage/providers/IDBKeyValProvider.ts index 09fa6af5a..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, true, false, true).result; + 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, preMergedValue) { + mergeItem(key, change) { // Since Onyx already merged the existing value with the changes, we can just set the value directly. - return provider.setItem(key, preMergedValue); + 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 7f1b1c2fb..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, preMergedValue) { + mergeItem(key, change) { // Since Onyx already merged the existing value with the changes, we can just set the value directly. - return this.setItem(key, preMergedValue); + 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, true, false, true).result 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 d3dd7d7de..bdcde2b76 100644 --- a/lib/storage/providers/SQLiteProvider.ts +++ b/lib/storage/providers/SQLiteProvider.ts @@ -3,12 +3,12 @@ * converting the value to a JSON string */ import {getFreeDiskStorage} from 'react-native-device-info'; -import type {BatchQueryResult, QuickSQLiteConnection, SQLBatchTuple} from 'react-native-quick-sqlite'; +import type {QuickSQLiteConnection, SQLBatchTuple} from 'react-native-quick-sqlite'; import {open} from 'react-native-quick-sqlite'; import type {FastMergeReplaceNullPatch} from '../../utils'; import utils from '../../utils'; import type StorageProvider from './types'; -import type {KeyList, KeyValuePairList} from './types'; +import type {StorageKeyList, StorageKeyValuePair} from './types'; const DB_NAME = 'OnyxDB'; let db: QuickSQLiteConnection; @@ -61,20 +61,20 @@ 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 stringifiedPairs = pairs.map((pair) => [pair[0], JSON.stringify(pair[1] === undefined ? null : pair[1])]); if (utils.isEmptyObject(stringifiedPairs)) { return Promise.resolve(); } - return db.executeBatchAsync([['REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, json(?));', stringifiedPairs]]); + return db.executeBatchAsync([['REPLACE INTO keyvaluepairs (record_key, valueJSON) VALUES (?, json(?));', stringifiedPairs]]).then(() => undefined); }, - multiMerge(pairs, mergeReplaceNullPatches) { + multiMerge(pairs) { const commands: SQLBatchTuple[] = []; const patchQuery = `INSERT INTO keyvaluepairs (record_key, valueJSON) @@ -93,13 +93,14 @@ const provider: StorageProvider = { // eslint-disable-next-line @typescript-eslint/prefer-for-of for (let i = 0; i < nonNullishPairs.length; i++) { - const pair = nonNullishPairs[i]; - const value = JSON.stringify(pair[1], replacer); - patchQueryArguments.push([pair[0], value]); + const [key, value, replaceNullPatches] = nonNullishPairs[i]; - const patches = mergeReplaceNullPatches?.[pair[0]] ?? []; + const valueAfterReplace = JSON.stringify(value, replacer); + patchQueryArguments.push([key, valueAfterReplace]); + + const patches = replaceNullPatches ?? []; if (patches.length > 0) { - const queries = generateJSONReplaceSQLQueries(pair[0], patches); + const queries = generateJSONReplaceSQLQueries(key, patches); if (queries.length > 0) { replaceQueryArguments.push(...queries); @@ -112,25 +113,25 @@ const provider: StorageProvider = { commands.push([replaceQuery, replaceQueryArguments]); } - return db.executeBatchAsync(commands); + return db.executeBatchAsync(commands).then(() => undefined); }, - mergeItem(key, preMergedValue) { + mergeItem(key, change, replaceNullPatches) { // Since Onyx already merged the existing value with the changes, we can just set the value directly. - return this.setItem(key, preMergedValue) as Promise; + 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]) => { const pageSize: number = pageSizeResult.rows?.item(0).page_size; diff --git a/lib/storage/providers/types.ts b/lib/storage/providers/types.ts index 6304d0866..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-quick-sqlite'; -import type {MixedOperationsQueue, OnyxKey, OnyxValue} from '../../types'; +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,53 +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, mergeReplaceNullPatches?: MixedOperationsQueue['mergeReplaceNullPatches']) => Promise; + multiMerge: (pairs: StorageKeyValuePair[]) => Promise; /** * Merges an existing value with a new one - * @param preMergedValue - the pre-merged data from `Onyx.applyMerge` + * @param change - the change to merge with the existing value */ - mergeItem: (key: TKey, preMergedValue: OnyxValue) => 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 @@ -79,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 cd5556b89..7dbdb1ca7 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -486,12 +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: {[TKey in OnyxKey]: FastMergeReplaceNullPatch[]}; + mergeReplaceNullPatches: MultiMergeReplaceNullPatches; set: OnyxInputKeyValueMapping; }; @@ -533,5 +535,6 @@ export type { OnyxValue, Selector, WithOnyxConnectOptions, + MultiMergeReplaceNullPatches, MixedOperationsQueue, }; diff --git a/lib/utils.ts b/lib/utils.ts index 88091f6cd..9a282cf12 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -7,51 +7,81 @@ type EmptyValue = EmptyObject | null | undefined; 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 loosing 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'; + + /** If true, any nested objects that contains the internal "ONYX_INTERNALS__REPLACE_OBJECT_MARK" flag will be completely replaced instead of merged. */ + shouldReplaceMarkedObjects?: boolean; +}; + 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; + + /** 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 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; -} - -// Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 - /** - * 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 isBatchingMergeChanges - If true, it means that we are batching merge changes before applying - * them to the Onyx value, so we must use a special logic to handle these changes. - * @param shouldReplaceMarkedObjects - If true, any nested objects that contains the internal "ONYX_INTERNALS__REPLACE_OBJECT_MARK" - * flag will be completely replaced instead of merged. + * @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: boolean, - isBatchingMergeChanges: boolean, - shouldReplaceMarkedObjects: boolean, + options: FastMergeOptions, metadata: FastMergeMetadata, - basePath: string[] = [], + basePath: string[], ): TObject { const destination: Record = {}; @@ -62,119 +92,91 @@ function mergeObject>( // 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; - - // If we are batching merge changes and the previous merge change (targetValue) 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. With the desired objects - // marked, when calling this method again with "shouldReplaceMarkedObjects" set to true we can proceed - // effectively replace them in the next condition. - if (isBatchingMergeChanges && targetValue === null) { - (targetValueWithFallback as Record)[ONYX_INTERNALS__REPLACE_OBJECT_MARK] = true; - metadata.replaceNullPatches.push([[...basePath, key], {...sourceValue}]); - } - - // Then, 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 (shouldReplaceMarkedObjects && sourceValue[ONYX_INTERNALS__REPLACE_OBJECT_MARK]) { - // We do a spread here in order to have a new object reference and allow us to delete the internal flag - // of the merged object only. - destination[key] = {...sourceValue}; - delete (destination[key] as Record).ONYX_INTERNALS__REPLACE_OBJECT_MARK; - } else { - // For the normal situations we'll just call `fastMerge()` again to merge the nested object. - destination[key] = fastMerge(targetValueWithFallback, sourceValue, shouldRemoveNestedNulls, isBatchingMergeChanges, shouldReplaceMarkedObjects, metadata, [ - ...basePath, - key, - ]).result; - } - } 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 "shouldReplaceMarkedObjects" enabled, 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]) { + // We do a spread here in order to have a new object reference and allow us to delete the internal flag + // of the merged object only. + // eslint-disable-next-line @typescript-eslint/no-unused-vars + delete sourceProperty[ONYX_INTERNALS__REPLACE_OBJECT_MARK]; + // const {ONYX_INTERNALS__REPLACE_OBJECT_MARK: _mark, ...sourcePropertyWithoutMark} = sourceProperty; + destination[key] = sourceProperty; + return; } - } + + destination[key] = fastMerge(targetProperty, sourceProperty, options, metadata, [...basePath, key]).result; + }); return destination as TObject; } -/** - * 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 fastMerge( - target: TValue, - source: TValue, - shouldRemoveNestedNulls: boolean, - isBatchingMergeChanges: boolean, - shouldReplaceMarkedObjects: boolean, - 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}; - } +/** 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; +} - const mergedValue = mergeObject(target, source as Record, shouldRemoveNestedNulls, isBatchingMergeChanges, shouldReplaceMarkedObjects, metadata, basePath) as TValue; +// Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 - return {result: mergedValue, replaceNullPatches: metadata.replaceNullPatches}; +/** + * Checks whether the given value can be merged. It has to be an object, but not an array, RegExp or Date. + */ +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)) { - return fastMerge(value, value, true, false, true).result; + return fastMerge(value, value, { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }).result; } return value; @@ -268,8 +270,8 @@ function hasWithOnyxInstance(mapping: ConnectOptions } export default { - isEmptyObject, fastMerge, + isEmptyObject, formatActionName, removeNestedNullValues, checkCompatibilityWithExistingValue, @@ -278,4 +280,4 @@ export default { hasWithOnyxInstance, ONYX_INTERNALS__REPLACE_OBJECT_MARK, }; -export type {FastMergeResult, FastMergeReplaceNullPatch}; +export type {FastMergeResult, FastMergeReplaceNullPatch, FastMergeOptions}; diff --git a/tests/perf-test/utils.perf-test.ts b/tests/perf-test/utils.perf-test.ts index b3ab893c7..6df36df08 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, false, false)); + await measureFunction(() => + utils.fastMerge(target, source, { + shouldRemoveNestedNulls: true, + }), + ); }); }); diff --git a/tests/unit/fastMergeTest.ts b/tests/unit/fastMergeTest.ts index b70a2d56f..8821bdb1c 100644 --- a/tests/unit/fastMergeTest.ts +++ b/tests/unit/fastMergeTest.ts @@ -54,7 +54,7 @@ const testMergeChanges: DeepObject[] = [ describe('fastMerge', () => { it('should merge an object with another object and remove nested null values', () => { - const result = utils.fastMerge(testObject, testObjectWithNullishValues, true, false, false); + const result = utils.fastMerge(testObject, testObjectWithNullishValues, {shouldRemoveNestedNulls: true}); expect(result.result).toEqual({ a: 'a', @@ -71,7 +71,7 @@ describe('fastMerge', () => { }); it('should merge an object with another object and not remove nested null values', () => { - const result = utils.fastMerge(testObject, testObjectWithNullishValues, false, false, false); + const result = utils.fastMerge(testObject, testObjectWithNullishValues); expect(result.result).toEqual({ a: 'a', @@ -89,7 +89,9 @@ describe('fastMerge', () => { }); it('should merge an object with an empty object and remove deeply nested null values', () => { - const result = utils.fastMerge({}, testObjectWithNullishValues, true, false, false); + const result = utils.fastMerge({}, testObjectWithNullishValues, { + shouldRemoveNestedNulls: true, + }); expect(result.result).toEqual(testObjectWithNullValuesRemoved); }); @@ -102,20 +104,27 @@ 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, true, false, false); + const result = utils.fastMerge(testObject, [1, 2, 3] as any, { + shouldRemoveNestedNulls: true, + }); 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, true, false, false); + 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 target object when its source is set to null and "isBatchingMergeChanges" is true', () => { - const result = utils.fastMerge(testMergeChanges[1], testMergeChanges[0], true, true, false); + 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: { @@ -141,9 +150,10 @@ describe('fastMerge', () => { h: 'h', }, }, - true, - false, - true, + { + shouldRemoveNestedNulls: true, + objectRemovalMode: 'replace', + }, ); expect(result.result).toEqual({ diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index d16c0f169..2e065fe71 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -151,15 +151,15 @@ describe('OnyxUtils', () => { }); }); - describe('applyMerge', () => { + describe('mergeChanges', () => { it("should return the last change if it's an array", () => { - const result = OnyxUtils.applyMerge(testObject, [...testMergeChanges, [0, 1, 2]]); + const {result} = OnyxUtils.mergeAndMarkChanges([...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.applyMerge(testObject, ['a', 0, 'b', 1]); + const {result} = OnyxUtils.mergeChanges(['a', 0, 'b', 1], testObject); expect(result).toEqual(1); }); @@ -180,7 +180,7 @@ describe('OnyxUtils', () => { }, }; - const result = OnyxUtils.applyMerge(testObject, [batchedChanges]); + const {result} = OnyxUtils.mergeChanges([batchedChanges], testObject); expect(result).toEqual({ a: 'a', @@ -199,11 +199,11 @@ describe('OnyxUtils', () => { }); }); - describe('batchMergeChanges', () => { + 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 = OnyxUtils.batchMergeChanges(testMergeChanges); + const {result, replaceNullPatches} = OnyxUtils.mergeAndMarkChanges(testMergeChanges); - expect(result.result).toEqual({ + expect(result).toEqual({ b: { d: { i: 'i', @@ -217,7 +217,7 @@ describe('OnyxUtils', () => { }, }, }); - expect(result.replaceNullPatches).toEqual([ + expect(replaceNullPatches).toEqual([ [['b', 'd'], {i: 'i'}], [['b', 'd'], {i: 'i', j: 'j'}], [['b', 'g'], {k: 'k'}], @@ -225,7 +225,7 @@ describe('OnyxUtils', () => { }); it('should 2', () => { - const result = OnyxUtils.batchMergeChanges([ + 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. @@ -254,7 +254,7 @@ describe('OnyxUtils', () => { }, ]); - expect(result.result).toEqual({ + expect(result).toEqual({ originalMessage: { errorMessage: 'newErrorMessage', [utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK]: true, @@ -268,7 +268,8 @@ describe('OnyxUtils', () => { }, }, }); - expect(result.replaceNullPatches).toEqual([ + + expect(replaceNullPatches).toEqual([ [['originalMessage'], {errorMessage: 'newErrorMessage'}], [['receipt', 'nestedObject'], {nestedKey2: 'newNestedKey2'}], ]);