diff --git a/API-INTERNAL.md b/API-INTERNAL.md index a9f75cbea..8da520402 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -17,16 +17,13 @@
getDeferredInitTask()

Getter - returns the deffered init task.

-
getEvictionBlocklist()
-

Getter - returns the eviction block list.

-
getSkippableCollectionMemberIDs()

Getter - returns the skippable collection member IDs.

setSkippableCollectionMemberIDs()

Setter - sets the skippable collection member IDs.

-
initStoreValues(keys, initialKeyStates, evictableKeys)
+
initStoreValues(keys, initialKeyStates, evictableKeys, fullyMergedSnapshotKeys)

Sets the initial values for the Onyx store

maybeFlushBatchUpdates()
@@ -71,9 +68,6 @@ is associated with a collection of keys.

Checks to see if a provided key is the exact configured key of our connected subscriber or if the provided key is a collection member key (in case our configured key is a "collection key")

-
isEvictableKey()
-

Checks to see if this key has been flagged as safe for removal.

-
getCollectionKey(key)

Extracts the collection identifier of a given collection member key.

For example:

@@ -88,20 +82,6 @@ or if the provided key is a collection member key (in case our configured key is

Tries to get a value from the cache. If the value is not present in cache it will return the default value or undefined. If the requested key is a collection, it will return an object with all the collection members.

-
removeLastAccessedKey()
-

Remove a key from the recently accessed key list.

-
-
addLastAccessedKey()
-

Add a key to the list of recently accessed keys. The least -recently accessed key should be at the head and the most -recently accessed key at the tail.

-
-
addEvictableKeysToRecentlyAccessedList()
-

Take all the keys that are safe to evict and add them to -the recently accessed list when initializing the app. This -enables keys that have not recently been accessed to be -removed.

-
keysChanged()

When a collection of keys change, search for any callbacks matching the collection key and trigger those callbacks

@@ -139,18 +119,20 @@ 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, existingValue)
+

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

+
+
mergeAndMarkChanges(changes, existingValue)
+

Merges an array of changes with an existing value or creates a single change. +It will also mark deep nested objects that need to be entirely replaced during the merge.

+
+
mergeInternal(changes, existingValue)
+

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

initializeWithDefaultKeyStates()

Merge user provided default key value pairs.

@@ -167,6 +149,14 @@ to an array of key-value pairs in the above format and removes key-value pairs t
unsubscribeFromKey(subscriptionID)

Disconnects and removes the listener from the Onyx key.

+
mergeCollectionWithPatches(collectionKey, collection, mergeReplaceNullPatches)
+

Merges a collection based on their keys. +Serves as core implementation for Onyx.mergeCollection() public function, the difference being +that this internal function allows passing an additional mergeReplaceNullPatches parameter.

+
+
clearOnyxUtilsInternals()
+

Clear internal variables used in this file, useful in test environments.

+
@@ -192,12 +182,6 @@ Getter - returns the default key states. ## getDeferredInitTask() Getter - returns the deffered init task. -**Kind**: global function - - -## getEvictionBlocklist() -Getter - returns the eviction block list. - **Kind**: global function @@ -213,7 +197,7 @@ Setter - sets the skippable collection member IDs. **Kind**: global function -## initStoreValues(keys, initialKeyStates, evictableKeys) +## initStoreValues(keys, initialKeyStates, evictableKeys, fullyMergedSnapshotKeys) Sets the initial values for the Onyx store **Kind**: global function @@ -222,7 +206,8 @@ Sets the initial values for the Onyx store | --- | --- | | keys | `ONYXKEYS` constants object from Onyx.init() | | initialKeyStates | initial data to set when `init()` and `clear()` are called | -| evictableKeys | This is an array of keys (individual or collection patterns) that are eligible for automatic removal when storage limits are reached. | +| evictableKeys | This is an array of keys (individual or collection patterns) that when provided to Onyx are flagged as "safe" for removal. | +| fullyMergedSnapshotKeys | Array of snapshot collection keys where full merge is supported and data structure can be changed after merge. | @@ -318,12 +303,6 @@ or throws an Error if the key is not a collection one. Checks to see if a provided key is the exact configured key of our connected subscriber or if the provided key is a collection member key (in case our configured key is a "collection key") -**Kind**: global function - - -## isEvictableKey() -Checks to see if this key has been flagged as safe for removal. - **Kind**: global function @@ -349,29 +328,6 @@ For example: Tries to get a value from the cache. If the value is not present in cache it will return the default value or undefined. If the requested key is a collection, it will return an object with all the collection members. -**Kind**: global function - - -## removeLastAccessedKey() -Remove a key from the recently accessed key list. - -**Kind**: global function - - -## addLastAccessedKey() -Add a key to the list of recently accessed keys. The least -recently accessed key should be at the head and the most -recently accessed key at the tail. - -**Kind**: global function - - -## addEvictableKeysToRecentlyAccessedList() -Take all the keys that are safe to evict and add them to -the recently accessed list when initializing the app. This -enables keys that have not recently been accessed to be -removed. - **Kind**: global function @@ -465,15 +421,6 @@ whatever it is we attempted to do. 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 ## prepareKeyValuePairsForStorage() ⇒ @@ -483,16 +430,42 @@ 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]> - + + +## mergeChanges(changes, existingValue) +Merges an array of changes with an existing value or creates a single change. + +**Kind**: global function + +| Param | Description | +| --- | --- | +| changes | Array of changes that should be merged | +| existingValue | The existing value that should be merged with the changes | + + + +## mergeAndMarkChanges(changes, existingValue) +Merges an array of changes with an existing value or creates a single change. +It will also mark deep nested objects that need to be entirely replaced during the merge. + +**Kind**: global function + +| Param | Description | +| --- | --- | +| changes | Array of changes that should be merged | +| existingValue | The existing value that should be merged with the changes | -## applyMerge(changes) -Merges an array of changes with an existing value + + +## mergeInternal(changes, existingValue) +Merges an array of changes with an existing value or creates a single change. **Kind**: global function | Param | Description | | --- | --- | -| changes | Array of changes that should be applied to the existing value | +| changes | Array of changes that should be merged | +| existingValue | The existing value that should be merged with the changes | @@ -535,3 +508,24 @@ Disconnects and removes the listener from the Onyx key. | --- | --- | | subscriptionID | Subscription ID returned by calling `OnyxUtils.subscribeToKey()`. | + + +## mergeCollectionWithPatches(collectionKey, collection, mergeReplaceNullPatches) +Merges a collection based on their keys. +Serves as core implementation for `Onyx.mergeCollection()` public function, the difference being +that this internal function allows passing an additional `mergeReplaceNullPatches` parameter. + +**Kind**: global function + +| Param | Description | +| --- | --- | +| collectionKey | e.g. `ONYXKEYS.COLLECTION.REPORT` | +| collection | Object collection keyed by individual collection member keys and values | +| mergeReplaceNullPatches | Record where the key is a collection member key and the value is a list of tuples that we'll use to replace the nested objects of that collection member record with something else. | + + + +## clearOnyxUtilsInternals() +Clear internal variables used in this file, useful in test environments. + +**Kind**: global function diff --git a/API.md b/API.md index dc1fa112f..ec083bbbf 100644 --- a/API.md +++ b/API.md @@ -29,7 +29,7 @@ applied in the order they were called. Note: Onyx.set() calls do no Onyx.merge() and Onyx.set().

mergeCollection(collectionKey, collection)
-

Merges a collection based on their keys

+

Merges a collection based on their keys.

clear(keysToPreserve)

Clear out all the data in the store

@@ -158,7 +158,7 @@ Onyx.merge(ONYXKEYS.POLICY, {name: 'My Workspace'}); // -> {id: 1, name: 'My Wor ## mergeCollection(collectionKey, collection) -Merges a collection based on their keys +Merges a collection based on their keys. **Kind**: global function diff --git a/jest.config.js b/jest.config.js index b6f148ece..d904a33c4 100644 --- a/jest.config.js +++ b/jest.config.js @@ -15,4 +15,8 @@ module.exports = { testTimeout: 60000, transformIgnorePatterns: ['node_modules/(?!((@)?react-native|@ngneat/falso|uuid)/)'], testSequencer: './jest-sequencer.js', + moduleNameMapper: { + // Redirect all imports of OnyxMerge to its web version during unit tests. + '^(.*)/OnyxMerge$': '/lib/OnyxMerge/index.ts', + }, }; diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 02c48a854..a656f4fad 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -1,5 +1,3 @@ -/* eslint-disable no-continue */ -import _ from 'underscore'; import * as Logger from './Logger'; import cache, {TASK} from './OnyxCache'; import * as PerformanceUtils from './PerformanceUtils'; @@ -33,6 +31,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({ @@ -171,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); + OnyxUtils.logKeyRemoved(OnyxUtils.METHOD.SET, key); 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); + OnyxUtils.logKeyChanged(OnyxUtils.METHOD.SET, key, value, 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; }); } @@ -314,8 +306,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) { @@ -327,53 +317,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) { + OnyxUtils.remove(key); + OnyxUtils.logKeyRemoved(OnyxUtils.METHOD.MERGE, 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) { @@ -386,7 +344,7 @@ function merge(key: TKey, changes: OnyxMergeInput): } /** - * Merges a collection based on their keys + * Merges a collection based on their keys. * * @example * @@ -399,115 +357,7 @@ function merge(key: TKey, changes: OnyxMergeInput): * @param collection Object collection keyed by individual collection member keys and values */ function mergeCollection(collectionKey: TKey, collection: OnyxMergeCollectionInput): Promise { - if (!OnyxUtils.isValidNonEmptyCollectionForMerge(collection)) { - Logger.logInfo('mergeCollection() called with invalid or empty value. Skipping this update.'); - return Promise.resolve(); - } - - let resultCollection: OnyxInputKeyValueMapping = collection; - let resultCollectionKeys = Object.keys(resultCollection); - - // Confirm all the collection keys belong to the same parent - if (!OnyxUtils.doAllCollectionItemsBelongToSameParent(collectionKey, resultCollectionKeys)) { - return Promise.resolve(); - } - - const skippableCollectionMemberIDs = OnyxUtils.getSkippableCollectionMemberIDs(); - if (skippableCollectionMemberIDs.size) { - resultCollection = resultCollectionKeys.reduce((result: OnyxInputKeyValueMapping, key) => { - try { - const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key, collectionKey); - // If the collection member key is a skippable one we set its value to null. - // eslint-disable-next-line no-param-reassign - result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? resultCollection[key] : null; - } catch { - // Something went wrong during split, so we assign the data to result anyway. - // eslint-disable-next-line no-param-reassign - result[key] = resultCollection[key]; - } - - return result; - }, {}); - } - resultCollectionKeys = Object.keys(resultCollection); - - return OnyxUtils.getAllKeys() - .then((persistedKeys) => { - // Split to keys that exist in storage and keys that don't - const keys = resultCollectionKeys.filter((key) => { - if (resultCollection[key] === null) { - OnyxUtils.remove(key); - return false; - } - return true; - }); - - const existingKeys = keys.filter((key) => persistedKeys.has(key)); - - const cachedCollectionForExistingKeys = OnyxUtils.getCachedCollection(collectionKey, existingKeys); - - 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; - }, {}) as Record>; - - const newCollection: Record> = {}; - keys.forEach((key) => { - if (persistedKeys.has(key)) { - return; - } - newCollection[key] = resultCollection[key]; - }); - - // 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); - - // We can safely remove nested null values when using (multi-)set, - // because we will simply overwrite the existing values in storage. - const keyValuePairsForNewCollection = OnyxUtils.prepareKeyValuePairsForStorage(newCollection, true); - - const promises = []; - - // We need to get the previously existing values so we can compare the new ones - // against them, to avoid unnecessary subscriber updates. - const previousCollectionPromise = Promise.all(existingKeys.map((key) => OnyxUtils.get(key).then((value) => [key, value]))).then(Object.fromEntries); - - // 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)); - } - - if (keyValuePairsForNewCollection.length > 0) { - promises.push(Storage.multiSet(keyValuePairsForNewCollection)); - } - - // finalMergedCollection contains all the keys that were merged, without the keys of incompatible updates - const finalMergedCollection = {...existingKeyCollection, ...newCollection}; - - // Prefill cache if necessary by calling get() on any existing keys and then merge original data to cache - // and update all subscribers - const promiseUpdate = previousCollectionPromise.then((previousCollection) => { - cache.merge(finalMergedCollection); - return OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, finalMergedCollection, previousCollection); - }); - - return Promise.all(promises) - .catch((error) => OnyxUtils.evictStorageAndRetry(error, mergeCollection, collectionKey, resultCollection)) - .then(() => { - OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MERGE_COLLECTION, undefined, resultCollection); - return promiseUpdate; - }); - }) - .then(() => undefined); + return OnyxUtils.mergeCollectionWithPatches(collectionKey, collection); } /** @@ -718,24 +568,35 @@ 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(() => + OnyxUtils.mergeCollectionWithPatches( + collectionKey, + batchedCollectionUpdates.merge as Collection, + batchedCollectionUpdates.mergeReplaceNullPatches, + ), + ); } if (!utils.isEmptyObject(batchedCollectionUpdates.set)) { promises.push(() => multiSet(batchedCollectionUpdates.set)); @@ -743,13 +604,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.mergeChanges(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 f61ce4056..202720b80 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -220,7 +220,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..b796dfde4 --- /dev/null +++ b/lib/OnyxMerge/index.native.ts @@ -0,0 +1,48 @@ +import OnyxUtils from '../OnyxUtils'; +import type {OnyxInput, OnyxKey, OnyxValue} from '../types'; +import cache from '../OnyxCache'; +import Storage from '../storage'; +import type {ApplyMerge} from './types'; + +const applyMerge: ApplyMerge = | undefined, TChange extends OnyxInput | undefined>( + key: TKey, + existingValue: TValue, + validChanges: TChange[], +) => { + // If any of the changes is null, we need to discard the existing value. + const baseValue = validChanges.includes(null as TChange) ? 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. + OnyxUtils.logKeyChanged(OnyxUtils.METHOD.MERGE, key, mergedValue, 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}); + } + + // For native platforms we use `mergeItem` that will take advantage of JSON_PATCH and JSON_REPLACE SQL operations to + // merge the object in a performant way. + 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..2a648499b --- /dev/null +++ b/lib/OnyxMerge/index.ts @@ -0,0 +1,39 @@ +import cache from '../OnyxCache'; +import OnyxUtils from '../OnyxUtils'; +import Storage from '../storage'; +import type {OnyxInput, OnyxKey, OnyxValue} from '../types'; +import type {ApplyMerge} from './types'; + +const applyMerge: ApplyMerge = | undefined, TChange extends OnyxInput | undefined>( + key: TKey, + existingValue: TValue, + validChanges: TChange[], +) => { + 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. + OnyxUtils.logKeyChanged(OnyxUtils.METHOD.MERGE, key, mergedValue, 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}); + } + + // For web platforms we use `setItem` since the object was already merged with its changes before. + 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..c59b7892a --- /dev/null +++ b/lib/OnyxMerge/types.ts @@ -0,0 +1,14 @@ +import type {OnyxInput, OnyxKey} from '../types'; + +type ApplyMergeResult = { + mergedValue: TValue; + updatePromise: Promise; +}; + +type ApplyMerge = | undefined, TChange extends OnyxInput | null>( + key: TKey, + existingValue: TValue, + validChanges: TChange[], +) => Promise>; + +export type {ApplyMerge, ApplyMergeResult}; diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 28b7ee039..6e7581577 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -3,6 +3,7 @@ import {deepEqual} from 'fast-equals'; import lodashClone from 'lodash/clone'; import type {ValueOf} from 'type-fest'; import lodashPick from 'lodash/pick'; +import _ from 'underscore'; import DevTools from './DevTools'; import * as Logger from './Logger'; import type Onyx from './Onyx'; @@ -20,21 +21,26 @@ import type { DefaultConnectOptions, KeyValueMapping, Mapping, + MultiMergeReplaceNullPatches, OnyxCollection, OnyxEntry, OnyxInput, + OnyxInputKeyValueMapping, OnyxKey, OnyxMergeCollectionInput, OnyxUpdate, 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'; +import logMessages from './logMessages'; // Method constants const METHOD = { @@ -357,7 +363,7 @@ function multiGet(keys: CollectionKeyBase[]): Promise { if (skippableCollectionMemberIDs.size) { try { - const [, collectionMemberID] = OnyxUtils.splitCollectionMemberKey(key); + const [, collectionMemberID] = splitCollectionMemberKey(key); if (skippableCollectionMemberIDs.has(collectionMemberID)) { // The key is a skippable one, so we skip this iteration. return; @@ -383,7 +389,7 @@ function multiGet(keys: CollectionKeyBase[]): Promise|OnyxEntry>`, which is not what we want. This preserves the order of the keys provided. */ function tupleGet(keys: Keys): Promise<{[Index in keyof Keys]: OnyxValue}> { - return Promise.all(keys.map((key) => OnyxUtils.get(key))) as Promise<{[Index in keyof Keys]: OnyxValue}>; + return Promise.all(keys.map((key) => get(key))) as Promise<{[Index in keyof Keys]: OnyxValue}>; } /** @@ -1171,77 +1177,101 @@ 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. + * 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 * - * @returns The value without null values and a boolean "wasRemoved", which indicates if the key got removed completely + * @return an array of key - value pairs <[key, value]> */ -function removeNullValues | undefined>(key: OnyxKey, value: Value, shouldRemoveNestedNulls = true): RemoveNullValuesOutput { - if (value === null) { - remove(key); - return {value, wasRemoved: true}; - } +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 (value === undefined) { - return {value, wasRemoved: false}; - } + const valueWithoutNestedNullValues = shouldRemoveNestedNulls ?? true ? utils.removeNestedNullValues(value) : value; - // 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}; + if (valueWithoutNestedNullValues !== undefined) { + pairs.push([key, valueWithoutNestedNullValues, replaceNullPatches?.[key]]); + } + }); + + return pairs; } /** - * 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 - -* @return an array of key - value pairs <[key, value]> + * 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 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 mergeChanges | undefined, TChange extends OnyxInput | undefined>(changes: TChange[], existingValue?: TValue): FastMergeResult { + return mergeInternal('merge', changes, existingValue); +} - return pairs; - }, []); +/** + * Merges an array of changes with an existing value or creates a single change. + * It will also mark deep nested objects that need to be entirely replaced during the merge. + * + * @param changes Array of changes that should be merged + * @param existingValue The existing value that should be merged with the changes + */ +function mergeAndMarkChanges | undefined, TChange extends OnyxInput | undefined>( + changes: TChange[], + existingValue?: TValue, +): FastMergeResult { + return mergeInternal('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, +function mergeInternal | undefined, TChange extends OnyxInput | undefined>( + 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: []}; } /** @@ -1251,7 +1281,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)); @@ -1414,12 +1446,12 @@ function unsubscribeFromKey(subscriptionID: number): void { } function updateSnapshots(data: OnyxUpdate[], mergeFn: typeof Onyx.merge): Array<() => Promise> { - const snapshotCollectionKey = OnyxUtils.getSnapshotKey(); + const snapshotCollectionKey = getSnapshotKey(); if (!snapshotCollectionKey) return []; const promises: Array<() => Promise> = []; - const snapshotCollection = OnyxUtils.getCachedCollection(snapshotCollectionKey); + const snapshotCollection = getCachedCollection(snapshotCollectionKey); Object.entries(snapshotCollection).forEach(([snapshotEntryKey, snapshotEntryValue]) => { // Snapshots may not be present in cache. We don't know how to update them so we skip. @@ -1431,7 +1463,7 @@ function updateSnapshots(data: OnyxUpdate[], mergeFn: typeof Onyx.merge): Array< data.forEach(({key, value}) => { // snapshots are normal keys so we want to skip update if they are written to Onyx - if (OnyxUtils.isCollectionMemberKey(snapshotCollectionKey, key)) { + if (isCollectionMemberKey(snapshotCollectionKey, key)) { return; } @@ -1479,6 +1511,141 @@ function updateSnapshots(data: OnyxUpdate[], mergeFn: typeof Onyx.merge): Array< return promises; } +/** + * Merges a collection based on their keys. + * Serves as core implementation for `Onyx.mergeCollection()` public function, the difference being + * that this internal function allows passing an additional `mergeReplaceNullPatches` parameter. + * + * @param collectionKey e.g. `ONYXKEYS.COLLECTION.REPORT` + * @param collection Object collection keyed by individual collection member keys and values + * @param mergeReplaceNullPatches Record where the key is a collection member key and the value is a list of + * tuples that we'll use to replace the nested objects of that collection member record with something else. + */ +function mergeCollectionWithPatches( + collectionKey: TKey, + collection: OnyxMergeCollectionInput, + mergeReplaceNullPatches?: MultiMergeReplaceNullPatches, +): Promise { + if (!isValidNonEmptyCollectionForMerge(collection)) { + Logger.logInfo('mergeCollection() called with invalid or empty value. Skipping this update.'); + return Promise.resolve(); + } + + let resultCollection: OnyxInputKeyValueMapping = collection; + let resultCollectionKeys = Object.keys(resultCollection); + + // Confirm all the collection keys belong to the same parent + if (!doAllCollectionItemsBelongToSameParent(collectionKey, resultCollectionKeys)) { + return Promise.resolve(); + } + + if (skippableCollectionMemberIDs.size) { + resultCollection = resultCollectionKeys.reduce((result: OnyxInputKeyValueMapping, key) => { + try { + const [, collectionMemberID] = splitCollectionMemberKey(key, collectionKey); + // If the collection member key is a skippable one we set its value to null. + // eslint-disable-next-line no-param-reassign + result[key] = !skippableCollectionMemberIDs.has(collectionMemberID) ? resultCollection[key] : null; + } catch { + // Something went wrong during split, so we assign the data to result anyway. + // eslint-disable-next-line no-param-reassign + result[key] = resultCollection[key]; + } + + return result; + }, {}); + } + resultCollectionKeys = Object.keys(resultCollection); + + return getAllKeys() + .then((persistedKeys) => { + // Split to keys that exist in storage and keys that don't + const keys = resultCollectionKeys.filter((key) => { + if (resultCollection[key] === null) { + remove(key); + return false; + } + return true; + }); + + const existingKeys = keys.filter((key) => persistedKeys.has(key)); + + const cachedCollectionForExistingKeys = getCachedCollection(collectionKey, existingKeys); + + 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; + }, {}) as Record>; + + const newCollection: Record> = {}; + keys.forEach((key) => { + if (persistedKeys.has(key)) { + return; + } + newCollection[key] = resultCollection[key]; + }); + + // 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 = 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. + const keyValuePairsForNewCollection = prepareKeyValuePairsForStorage(newCollection, true); + + const promises = []; + + // We need to get the previously existing values so we can compare the new ones + // against them, to avoid unnecessary subscriber updates. + const previousCollectionPromise = Promise.all(existingKeys.map((key) => get(key).then((value) => [key, value]))).then(Object.fromEntries); + + // 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)); + } + + if (keyValuePairsForNewCollection.length > 0) { + promises.push(Storage.multiSet(keyValuePairsForNewCollection)); + } + + // finalMergedCollection contains all the keys that were merged, without the keys of incompatible updates + const finalMergedCollection = {...existingKeyCollection, ...newCollection}; + + // Prefill cache if necessary by calling get() on any existing keys and then merge original data to cache + // and update all subscribers + const promiseUpdate = previousCollectionPromise.then((previousCollection) => { + cache.merge(finalMergedCollection); + return scheduleNotifyCollectionSubscribers(collectionKey, finalMergedCollection, previousCollection); + }); + + return Promise.all(promises) + .catch((error) => evictStorageAndRetry(error, mergeCollectionWithPatches, collectionKey, resultCollection)) + .then(() => { + sendActionToDevTools(METHOD.MERGE_COLLECTION, undefined, resultCollection); + return promiseUpdate; + }); + }) + .then(() => undefined); +} + +function logKeyChanged(onyxMethod: Extract, key: OnyxKey, value: unknown, hasChanged: boolean) { + Logger.logInfo(`${onyxMethod} called for key: ${key}${_.isObject(value) ? ` properties: ${_.keys(value).join(',')}` : ''} hasChanged: ${hasChanged}`); +} + +function logKeyRemoved(onyxMethod: Extract, key: OnyxKey) { + Logger.logInfo(`${onyxMethod} called for key: ${key} => null passed, so key was removed`); +} + /** * Clear internal variables used in this file, useful in test environments. */ @@ -1522,9 +1689,9 @@ const OnyxUtils = { evictStorageAndRetry, broadcastUpdate, hasPendingMergeForKey, - removeNullValues, prepareKeyValuePairsForStorage, - applyMerge, + mergeChanges, + mergeAndMarkChanges, initializeWithDefaultKeyStates, getSnapshotKey, multiGet, @@ -1540,6 +1707,9 @@ const OnyxUtils = { addKeyToRecentlyAccessedIfNeeded, reduceCollectionWithSelector, updateSnapshots, + mergeCollectionWithPatches, + logKeyChanged, + logKeyRemoved, }; GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => { @@ -1561,8 +1731,6 @@ GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => { // @ts-expect-error Reassign getCollectionKeys = decorateWithMetrics(getCollectionKeys, 'OnyxUtils.getCollectionKeys'); // @ts-expect-error Reassign - addEvictableKeysToRecentlyAccessedList = decorateWithMetrics(cache.addEvictableKeysToRecentlyAccessedList, 'OnyxCache.addEvictableKeysToRecentlyAccessedList'); - // @ts-expect-error Reassign keysChanged = decorateWithMetrics(keysChanged, 'OnyxUtils.keysChanged'); // @ts-expect-error Reassign keyChanged = decorateWithMetrics(keyChanged, 'OnyxUtils.keyChanged'); 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..0fd2a63ec 100644 --- a/lib/storage/providers/IDBKeyValProvider.ts +++ b/lib/storage/providers/IDBKeyValProvider.ts @@ -49,15 +49,19 @@ 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 +71,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..2367e972e 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,16 @@ 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; 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..859176505 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,26 @@ type PageCountResult = { const DB_NAME = 'OnyxDB'; let db: NitroSQLiteConnection; +/** + * Prevents the stringifying of the object markers. + */ +function objectMarkRemover(key: string, value: unknown) { + if (key === utils.ONYX_INTERNALS__REPLACE_OBJECT_MARK) return undefined; + return value; +} + +/** + * Transforms the replace null patches into SQL queries to be passed to JSON_REPLACE. + */ +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 +103,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 +115,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[] = []; + + // Query to merge the change into the DB value. + 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 nonUndefinedPairs = pairs.filter((pair) => pair[1] !== undefined); - const params = nonUndefinedPairs.map((pair) => { - const value = JSON.stringify(pair[1]); - return [pair[0], value]; - }); + // Query to fully replace the nested objects of the DB value. + const replaceQuery = `UPDATE keyvaluepairs + SET valueJSON = JSON_REPLACE(valueJSON, ?, JSON(?)) + WHERE record_key = ?; + `; + const replaceQueryArguments: string[][] = []; - return db.executeBatchAsync([{query, params}]); - }, - mergeItem(key, deltaChanges, preMergedValue, shouldSetValue) { - if (shouldSetValue) { - return this.setItem(key, preMergedValue) as Promise; + const nonNullishPairs = pairs.filter((pair) => pair[1] !== undefined); + + for (const [key, value, replaceNullPatches] of nonNullishPairs) { + const changeWithoutMarkers = JSON.stringify(value, objectMarkRemover); + patchQueryArguments.push([key, changeWithoutMarkers]); + + const patches = replaceNullPatches ?? []; + if (patches.length > 0) { + const queries = generateJSONReplaceSQLQueries(key, patches); + + if (queries.length > 0) { + replaceQueryArguments.push(...queries); + } + } } - return this.multiMerge([[key, deltaChanges]]) as Promise; + commands.push({query: patchQuery, params: patchQueryArguments}); + if (replaceQueryArguments.length > 0) { + commands.push({query: replaceQuery, params: replaceQueryArguments}); + } + + 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 7fa6e23d1..5af416974 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`. @@ -493,11 +494,20 @@ type InitOptions = { // eslint-disable-next-line @typescript-eslint/no-explicit-any type GenericFunction = (...args: any[]) => any; +/** + * Represents a record where the key is a collection member key and the value is a list of + * tuples that we'll use to replace the nested objects of that collection member record with something else. + */ +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; }; @@ -539,5 +549,6 @@ export type { OnyxValue, Selector, WithOnyxConnectOptions, + MultiMergeReplaceNullPatches, MixedOperationsQueue, }; diff --git a/lib/utils.ts b/lib/utils.ts index 36d9226aa..c8e69e898 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -3,29 +3,88 @@ 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; -} +/** + * A tuple where the first value is the path to the nested object that contains the + * internal `ONYX_INTERNALS__REPLACE_OBJECT_MARK` flag, and the second value is the data we want to replace + * in that path. + * + * This tuple will be used in SQLiteProvider to replace the nested object using `JSON_REPLACE`. + * */ +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'; +}; -// Mostly copied from https://medium.com/@lubaka.a/how-to-remove-lodash-performance-improvement-b306669ad0e1 +type FastMergeMetadata = { + /** The list of tuples that will be used in SQLiteProvider to replace the nested objects using `JSON_REPLACE`. */ + replaceNullPatches: FastMergeReplaceNullPatch[]; +}; + +type FastMergeResult = { + /** The result of the merge. */ + result: TValue; + + /** The list of tuples that will be used in SQLiteProvider to replace the nested objects using `JSON_REPLACE`. */ + 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,73 +94,80 @@ 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 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 the source value is not a mergable object, we need to set the key 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]) { + // 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. + const sourcePropertyWithoutMark = {...sourceProperty}; + delete sourcePropertyWithoutMark.ONYX_INTERNALS__REPLACE_OBJECT_MARK; + destination[key] = sourcePropertyWithoutMark; + 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. */ @@ -222,4 +288,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 113d6f481..9b6f0bcaf 100644 --- a/tests/perf-test/OnyxUtils.perf-test.ts +++ b/tests/perf-test/OnyxUtils.perf-test.ts @@ -610,22 +610,13 @@ 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)); }); }); - describe('applyMerge', () => { + describe('mergeChanges', () => { test('one call merging 5 changes', async () => { const reportAction = mockedReportActionsMap[`${collectionKey}0`]; const changedReportAction1 = createRandomReportAction(Number(reportAction.reportActionID)); @@ -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 6bb239d16..ae572b951 100644 --- a/tests/perf-test/utils.perf-test.ts +++ b/tests/perf-test/utils.perf-test.ts @@ -25,7 +25,11 @@ describe('utils', () => { test('one call', async () => { const target = getRandomReportActions(collectionKey, 1000); const source = getRandomReportActions(collectionKey, 500); - await measureFunction(() => utils.fastMerge(target, source)); + await measureFunction(() => + utils.fastMerge(target, source, { + shouldRemoveNestedNulls: true, + }), + ); }); }); diff --git a/tests/types.ts b/tests/types.ts new file mode 100644 index 000000000..1aef55855 --- /dev/null +++ b/tests/types.ts @@ -0,0 +1,12 @@ +import type {DeepRecord} from '../lib/types'; + +// The types declared inside this file should be used only for testing. + +/** + * Utility type to represent a object that can accept any value and contain any deep objects. + */ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type GenericDeepRecord = DeepRecord; + +// eslint-disable-next-line import/prefer-default-export +export type {GenericDeepRecord}; diff --git a/tests/unit/fastMergeTest.ts b/tests/unit/fastMergeTest.ts index d20ba80ea..0823db572 100644 --- a/tests/unit/fastMergeTest.ts +++ b/tests/unit/fastMergeTest.ts @@ -1,7 +1,7 @@ -import type {DeepRecord} from '../../lib/types'; import utils from '../../lib/utils'; +import type {GenericDeepRecord} from '../types'; -type DeepObject = DeepRecord | unknown[]; +type DeepObject = GenericDeepRecord | unknown[]; const testObject: DeepObject = { a: 'a', @@ -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', () => { @@ -85,16 +103,67 @@ 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], { + 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], 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 "objectRemovalMode" is set to "replace"', () => { + 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 1092b5e59..5f4604950 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1,9 +1,12 @@ 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 StorageMock from '../../lib/storage'; import type {OnyxCollection, OnyxUpdate} from '../../lib/types'; +import type {GenericDeepRecord} from '../types'; import type GenericCollection from '../utils/GenericCollection'; import type {Connection} from '../../lib/OnyxConnectionManager'; @@ -857,7 +860,6 @@ describe('Onyx', () => { // When we pass it to Onyx.update // @ts-expect-error This is an invalid call to Onyx.update Onyx.update(data); - // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error) { if (error instanceof Error) { // Then we should expect the error message below @@ -874,7 +876,6 @@ describe('Onyx', () => { // When we pass it to Onyx.update // @ts-expect-error This is an invalid call to Onyx.update Onyx.update(data); - // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error) { if (error instanceof Error) { // Then we should expect the error message below @@ -1711,7 +1712,572 @@ 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'}}); + expect(await StorageMock.getItem(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`)).toEqual({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 () => { + const entry1: GenericDeepRecord = { + 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}); + expect(await StorageMock.getItem(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`)).toEqual(entry1ExpectedResult); + }); + + it('setting new object after null merge', async () => { + const entry1: GenericDeepRecord = { + 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}); + expect(await StorageMock.getItem(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`)).toEqual(entry1ExpectedResult); + }); + + it('setting new object after null merge of a primitive property', async () => { + const entry1: GenericDeepRecord = { + 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}); + expect(await StorageMock.getItem(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`)).toEqual(entry1ExpectedResult); + }); + + it('replacing nested object during updates', async () => { + const entry1: GenericDeepRecord | undefined = { + id: 'entry1', + someKey: 'someValue', + }; + await Onyx.multiSet({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: { + id: 'entry1', + someKey: 'someValue', + }, + }); + + let entry1ExpectedResult = lodashCloneDeep(entry1) as GenericDeepRecord | undefined; + const queuedUpdates: OnyxUpdate[] = []; + + queuedUpdates.push({ + 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, + }); + entry1ExpectedResult = undefined; + + queuedUpdates.push({ + 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', + someNestedObject: { + someNestedKey: 'someNestedValue', + }, + }, + }); + entry1ExpectedResult = {someKey: 'someValueChanged', someNestedObject: {someNestedKey: 'someNestedValue'}}; + + 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 key should completely replace the old update's value. + someNestedObject: null, + }, + }); + delete entry1ExpectedResult.someNestedObject; + + queuedUpdates.push({ + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, + onyxMethod: 'merge', + // This change should completely replace `someNestedObject` old update's value. + value: { + someNestedObject: { + someNestedKeyChanged: 'someNestedValueChange', + }, + }, + }); + entry1ExpectedResult.someNestedObject = {someNestedKeyChanged: 'someNestedValueChange'}; + + await Onyx.update(queuedUpdates); + + expect(result).toEqual({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1ExpectedResult}); + expect(await StorageMock.getItem(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`)).toEqual(entry1ExpectedResult); + }); + + describe('mergeCollection', () => { + it('replacing old object after null merge', async () => { + const entry1: GenericDeepRecord = { + sub_entry1: { + id: 'sub_entry1', + someKey: 'someValue', + }, + }; + + const entry2: GenericDeepRecord = { + sub_entry2: { + id: 'sub_entry2', + someKey: 'someValue', + }, + }; + await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1}); + await Onyx.multiSet({[`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`]: entry2}); + + const entry1ExpectedResult = lodashCloneDeep(entry1); + const entry2ExpectedResult = lodashCloneDeep(entry2); + 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, + }, + }, + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, + onyxMethod: 'merge', + value: { + // Removing the "sub_entry2" object in this update. + // Any subsequent changes to this object should completely replace the existing object in store. + sub_entry2: null, + }, + }, + ); + delete entry1ExpectedResult.sub_entry1; + delete entry2ExpectedResult.sub_entry2; + + 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', + }, + }, + }, + { + key: `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, + onyxMethod: 'merge', + value: { + // This change should completely replace "sub_entry2" existing object in store. + sub_entry2: { + newKey: 'newValue', + }, + }, + }, + ); + entry1ExpectedResult.sub_entry1 = {newKey: 'newValue'}; + entry2ExpectedResult.sub_entry2 = {newKey: 'newValue'}; + + await Onyx.update(queuedUpdates); + + expect(result).toEqual({ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`]: entry1ExpectedResult, + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`]: entry2ExpectedResult, + }); + expect(await StorageMock.multiGet([`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, `${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`])).toEqual([ + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`, entry1ExpectedResult], + [`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry2`, entry2ExpectedResult], + ]); + }); + }); + }); + 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'}}); + expect(await StorageMock.getItem(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`)).toEqual({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 () => { + const entry1: GenericDeepRecord = { + 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}); + expect(await StorageMock.getItem(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`)).toEqual(entry1ExpectedResult); + }); + + it('setting new object after null merge', async () => { + const entry1: GenericDeepRecord = { + 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}); + expect(await StorageMock.getItem(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`)).toEqual(entry1ExpectedResult); + }); + + it('setting new object after null merge of a primitive property', async () => { + const entry1: GenericDeepRecord = { + 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}); + expect(await StorageMock.getItem(`${ONYX_KEYS.COLLECTION.TEST_UPDATE}entry1`)).toEqual(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..521e28454 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 {GenericDeepRecord} from '../types'; +import utils from '../../lib/utils'; + +const testObject: GenericDeepRecord = { + a: 'a', + b: { + c: 'c', + d: { + e: 'e', + f: 'f', + }, + g: 'g', + }, +}; + +const testMergeChanges: GenericDeepRecord[] = [ + { + 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,78 @@ 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: GenericDeepRecord = { + 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 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'}], + ]); + }); + }); }); diff --git a/tests/utils/collections/reportActions.ts b/tests/utils/collections/reportActions.ts index 4fae80338..240fa654a 100644 --- a/tests/utils/collections/reportActions.ts +++ b/tests/utils/collections/reportActions.ts @@ -1,7 +1,7 @@ import {randAggregation, randBoolean, randWord} from '@ngneat/falso'; import {format} from 'date-fns'; import {createCollection} from './createCollection'; -import type {DeepRecord} from '../../../lib/types'; +import type {GenericDeepRecord} from '../../types'; const getRandomDate = (): string => { const randomTimestamp = Math.random() * new Date().getTime(); @@ -19,8 +19,7 @@ const getRandomReportActions = (collection: string, length = 10000) => length, ); -// eslint-disable-next-line @typescript-eslint/no-explicit-any -export default function createRandomReportAction(index: number): DeepRecord { +export default function createRandomReportAction(index: number): GenericDeepRecord { return { actionName: randWord(), reportActionID: index.toString(),