diff --git a/lib/Onyx.ts b/lib/Onyx.ts index be5de3fc..680023be 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -220,12 +220,16 @@ function merge(key: TKey, changes: OnyxMergeInput): } mergeQueue[key] = [changes]; - mergeQueuePromise[key] = OnyxUtils.get(key).then((existingValue) => { + mergeQueuePromise[key] = Promise.resolve().then(() => { // Calls to Onyx.set after a merge will terminate the current merge process and clear the merge queue if (mergeQueue[key] == null) { return Promise.resolve(); } + // Read the existing value at merge application time (not at queue time) so that + // any intervening synchronous cache updates (e.g. from mergeCollection) are picked up. + const existingValue = OnyxUtils.get(key); + try { const validChanges = mergeQueue[key].filter((change) => { const {isCompatible, existingValueType, newValueType, isEmptyArrayCoercion} = utils.checkCompatibilityWithExistingValue(change, existingValue); @@ -313,92 +317,98 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { const defaultKeyStates = OnyxUtils.getDefaultKeyStates(); const initialKeys = Object.keys(defaultKeyStates); - const promise = OnyxUtils.getAllKeys() - .then((cachedKeys) => { - cache.clearNullishStorageKeys(); - - const keysToBeClearedFromStorage: OnyxKey[] = []; - const keyValuesToResetIndividually: KeyValueMapping = {}; - // We need to store old and new values for collection keys to properly notify subscribers when clearing Onyx - // because the notification process needs the old values in cache but at that point they will be already removed from it. - const keyValuesToResetAsCollection: Record< - OnyxKey, - {oldValues: Record; newValues: Record} - > = {}; - - const allKeys = new Set([...cachedKeys, ...initialKeys]); - - // The only keys that should not be cleared are: - // 1. Anything specifically passed in keysToPreserve (because some keys like language preferences, offline - // status, or activeClients need to remain in Onyx even when signed out) - // 2. Any keys with a default state (because they need to remain in Onyx as their default, and setting them - // to null would cause unknown behavior) - // 2.1 However, if a default key was explicitly set to null, we need to reset it to the default value - for (const key of allKeys) { - const isKeyToPreserve = keysToPreserve.some((preserveKey) => OnyxKeys.isKeyMatch(preserveKey, key)); - const isDefaultKey = key in defaultKeyStates; - - // If the key is being removed or reset to default: - // 1. Update it in the cache - // 2. Figure out whether it is a collection key or not, - // since collection key subscribers need to be updated differently - if (!isKeyToPreserve) { - const oldValue = cache.get(key); - const newValue = defaultKeyStates[key] ?? null; - if (newValue !== oldValue) { - cache.set(key, newValue); - - const collectionKey = OnyxKeys.getCollectionKey(key); - - if (collectionKey) { - if (!keyValuesToResetAsCollection[collectionKey]) { - keyValuesToResetAsCollection[collectionKey] = {oldValues: {}, newValues: {}}; - } - keyValuesToResetAsCollection[collectionKey].oldValues[key] = oldValue; - keyValuesToResetAsCollection[collectionKey].newValues[key] = newValue ?? undefined; - } else { - keyValuesToResetIndividually[key] = newValue ?? undefined; - } - } - } + const cachedKeys = OnyxUtils.getAllKeys(); + cache.clearNullishStorageKeys(); - if (isKeyToPreserve || isDefaultKey) { - continue; - } + // Clear pending merge queues so that any in-flight Onyx.merge() calls + // don't overwrite the default values we're about to set. + const mergeQueue = OnyxUtils.getMergeQueue(); + const mergeQueuePromise = OnyxUtils.getMergeQueuePromise(); + for (const key of Object.keys(mergeQueue)) { + delete mergeQueue[key]; + delete mergeQueuePromise[key]; + } - // If it isn't preserved and doesn't have a default, we'll remove it - keysToBeClearedFromStorage.push(key); + const keysToBeClearedFromStorage: OnyxKey[] = []; + const keyValuesToResetIndividually: KeyValueMapping = {}; + // We need to store old and new values for collection keys to properly notify subscribers when clearing Onyx + // because the notification process needs the old values in cache but at that point they will be already removed from it. + const keyValuesToResetAsCollection: Record< + OnyxKey, + {oldValues: Record; newValues: Record} + > = {}; + + const allKeys = new Set([...cachedKeys, ...initialKeys]); + + // The only keys that should not be cleared are: + // 1. Anything specifically passed in keysToPreserve (because some keys like language preferences, offline + // status, or activeClients need to remain in Onyx even when signed out) + // 2. Any keys with a default state (because they need to remain in Onyx as their default, and setting them + // to null would cause unknown behavior) + // 2.1 However, if a default key was explicitly set to null, we need to reset it to the default value + for (const key of allKeys) { + const isKeyToPreserve = keysToPreserve.some((preserveKey) => OnyxKeys.isKeyMatch(preserveKey, key)); + const isDefaultKey = key in defaultKeyStates; + + // If the key is being removed or reset to default: + // 1. Update it in the cache + // 2. Figure out whether it is a collection key or not, + // since collection key subscribers need to be updated differently + if (!isKeyToPreserve) { + const oldValue = cache.get(key); + const newValue = defaultKeyStates[key] ?? null; + if (newValue !== oldValue) { + cache.set(key, newValue); + + const collectionKey = OnyxKeys.getCollectionKey(key); + + if (collectionKey) { + if (!keyValuesToResetAsCollection[collectionKey]) { + keyValuesToResetAsCollection[collectionKey] = {oldValues: {}, newValues: {}}; + } + keyValuesToResetAsCollection[collectionKey].oldValues[key] = oldValue; + keyValuesToResetAsCollection[collectionKey].newValues[key] = newValue ?? undefined; + } else { + keyValuesToResetIndividually[key] = newValue ?? undefined; + } } + } - // Exclude RAM-only keys to prevent them from being saved to storage - const defaultKeyValuePairs = Object.entries( - Object.keys(defaultKeyStates) - .filter((key) => !keysToPreserve.some((preserveKey) => OnyxKeys.isKeyMatch(preserveKey, key)) && !OnyxKeys.isRamOnlyKey(key)) - .reduce((obj: KeyValueMapping, key) => { - // eslint-disable-next-line no-param-reassign - obj[key] = defaultKeyStates[key]; - return obj; - }, {}), - ); + if (isKeyToPreserve || isDefaultKey) { + continue; + } - // Remove only the items that we want cleared from storage, and reset others to default - for (const key of keysToBeClearedFromStorage) cache.drop(key); - return Storage.removeItems(keysToBeClearedFromStorage) - .then(() => connectionManager.refreshSessionID()) - .then(() => Storage.multiSet(defaultKeyValuePairs)) - .then(() => { - DevTools.clearState(keysToPreserve); - - // Notify the subscribers for each key/value group so they can receive the new values - for (const [key, value] of Object.entries(keyValuesToResetIndividually)) { - OnyxUtils.keyChanged(key, value); - } - for (const [key, value] of Object.entries(keyValuesToResetAsCollection)) { - OnyxUtils.keysChanged(key, value.newValues, value.oldValues); - } - }); - }) - .then(() => undefined); + // If it isn't preserved and doesn't have a default, we'll remove it + keysToBeClearedFromStorage.push(key); + } + + // Exclude RAM-only keys to prevent them from being saved to storage + const defaultKeyValuePairs = Object.entries( + Object.keys(defaultKeyStates) + .filter((key) => !keysToPreserve.some((preserveKey) => OnyxKeys.isKeyMatch(preserveKey, key)) && !OnyxKeys.isRamOnlyKey(key)) + .reduce((obj: KeyValueMapping, key) => { + // eslint-disable-next-line no-param-reassign + obj[key] = defaultKeyStates[key]; + return obj; + }, {}), + ); + + // Remove only the items that we want cleared from storage, and reset others to default + for (const key of keysToBeClearedFromStorage) cache.drop(key); + const promise = Storage.removeItems(keysToBeClearedFromStorage) + .then(() => connectionManager.refreshSessionID()) + .then(() => Storage.multiSet(defaultKeyValuePairs)) + .then(() => { + DevTools.clearState(keysToPreserve); + + // Notify the subscribers for each key/value group so they can receive the new values + for (const [key, value] of Object.entries(keyValuesToResetIndividually)) { + OnyxUtils.keyChanged(key, value); + } + for (const [key, value] of Object.entries(keyValuesToResetAsCollection)) { + OnyxUtils.keysChanged(key, value.newValues, value.oldValues); + } + }); return cache.captureTask(TASK.CLEAR, promise) as Promise; }); @@ -519,6 +529,11 @@ function update(data: Array>): Promise OnyxUtils.partialSetCollection({collectionKey, collection: batchedCollectionUpdates.set as OnyxSetCollectionInput})); + } if (!utils.isEmptyObject(batchedCollectionUpdates.merge)) { promises.push(() => OnyxUtils.mergeCollectionWithPatches({ @@ -529,9 +544,6 @@ function update(data: Array>): Promise OnyxUtils.partialSetCollection({collectionKey, collection: batchedCollectionUpdates.set as OnyxSetCollectionInput})); - } } for (const [key, operations] of Object.entries(updateQueue)) { diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index 725dbfd7..9bd3912a 100644 --- a/lib/OnyxCache.ts +++ b/lib/OnyxCache.ts @@ -346,18 +346,17 @@ class OnyxCache { * @param isCollectionKeyFn - Function to determine if a key is a collection key * @param getAllKeysFn - Function to get all keys, defaults to Storage.getAllKeys */ - addEvictableKeysToRecentlyAccessedList(isCollectionKeyFn: (key: OnyxKey) => boolean, getAllKeysFn: () => Promise>): Promise { - return getAllKeysFn().then((keys: Set) => { - for (const evictableKey of this.evictionAllowList) { - for (const key of keys) { - if (!OnyxKeys.isKeyMatch(evictableKey, key)) { - continue; - } - - this.addLastAccessedKey(key, isCollectionKeyFn(key)); + addEvictableKeysToRecentlyAccessedList(isCollectionKeyFn: (key: OnyxKey) => boolean, getAllKeysFn: () => Set): void { + const keys = getAllKeysFn(); + for (const evictableKey of this.evictionAllowList) { + for (const key of keys) { + if (!OnyxKeys.isKeyMatch(evictableKey, key)) { + continue; } + + this.addLastAccessedKey(key, isCollectionKeyFn(key)); } - }); + } } /** diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 016e5213..1db01e73 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -4,7 +4,7 @@ import _ from 'underscore'; import DevTools from './DevTools'; import * as Logger from './Logger'; import type Onyx from './Onyx'; -import cache, {TASK} from './OnyxCache'; +import cache from './OnyxCache'; import OnyxKeys from './OnyxKeys'; import * as Str from './Str'; import Storage from './storage'; @@ -251,155 +251,21 @@ function reduceCollectionWithSelector( } /** Get some data from the store */ -function get>(key: TKey): Promise { - // When we already have the value in cache - resolve right away - if (cache.hasCacheForKey(key)) { - return Promise.resolve(cache.get(key) as TValue); - } - - // RAM-only keys should never read from storage (they may have stale persisted data - // from before the key was migrated to RAM-only). Mark as nullish so future get() calls - // short-circuit via hasCacheForKey and avoid re-running this branch. - if (OnyxKeys.isRamOnlyKey(key)) { - cache.addNullishStorageKey(key); - return Promise.resolve(undefined as TValue); - } - - const taskName = `${TASK.GET}:${key}` as const; - - // When a value retrieving task for this key is still running hook to it - if (cache.hasPendingTask(taskName)) { - return cache.getTaskPromise(taskName) as Promise; - } - - // Otherwise retrieve the value from storage and capture a promise to aid concurrent usages - const promise = Storage.getItem(key) - .then((val) => { - if (skippableCollectionMemberIDs.size) { - try { - const [, collectionMemberID] = OnyxKeys.splitCollectionMemberKey(key); - if (skippableCollectionMemberIDs.has(collectionMemberID)) { - // The key is a skippable one, so we set the value to undefined. - // eslint-disable-next-line no-param-reassign - val = undefined as OnyxValue; - } - } catch (e) { - // The key is not a collection one or something went wrong during split, so we proceed with the function's logic. - } - } - - // Prefer cache over stale storage if a concurrent write populated it during the read. - const cachedValue = cache.get(key) as TValue; - if (cachedValue !== undefined) { - return cachedValue; - } - - if (val === undefined) { - cache.addNullishStorageKey(key); - return undefined; - } - - cache.set(key, val); - return val; - }) - .catch((err) => Logger.logInfo(`Unable to get item from persistent storage. Key: ${key} Error: ${err}`)); - - return cache.captureTask(taskName, promise) as Promise; +function get>(key: TKey): TValue { + return cache.get(key) as TValue; } -// multiGet the data first from the cache and then from the storage for the missing keys. -function multiGet(keys: CollectionKeyBase[]): Promise>> { - // Keys that are not in the cache - const missingKeys: OnyxKey[] = []; - - // Tasks that are pending - const pendingTasks: Array>> = []; - - // Keys for the tasks that are pending - const pendingKeys: OnyxKey[] = []; - - // Data to be sent back to the invoker +// multiGet the data from the cache for all given keys. +function multiGet(keys: CollectionKeyBase[]): Map> { const dataMap = new Map>(); - /** - * We are going to iterate over all the matching keys and check if we have the data in the cache. - * If we do then we add it to the data object. If we do not have them, then we check if there is a pending task - * for the key. If there is such task, then we add the promise to the pendingTasks array and the key to the pendingKeys - * array. If there is no pending task then we add the key to the missingKeys array. - * - * These missingKeys will be later used to multiGet the data from the storage. - */ for (const key of keys) { - // RAM-only keys should never read from storage as they may have stale persisted data - // from before the key was migrated to RAM-only. - if (OnyxKeys.isRamOnlyKey(key)) { - if (cache.hasCacheForKey(key)) { - dataMap.set(key, cache.get(key) as OnyxValue); - } - continue; - } - - const cacheValue = cache.get(key) as OnyxValue; - if (cacheValue) { - dataMap.set(key, cacheValue); - continue; - } - - const pendingKey = `${TASK.GET}:${key}` as const; - if (cache.hasPendingTask(pendingKey)) { - pendingTasks.push(cache.getTaskPromise(pendingKey) as Promise>); - pendingKeys.push(key); - } else { - missingKeys.push(key); + if (cache.hasCacheForKey(key)) { + dataMap.set(key, cache.get(key) as OnyxValue); } } - return ( - Promise.all(pendingTasks) - // Wait for all the pending tasks to resolve and then add the data to the data map. - .then((values) => { - for (const [index, value] of values.entries()) { - dataMap.set(pendingKeys[index], value); - } - - return Promise.resolve(); - }) - // Get the missing keys using multiGet from the storage. - .then(() => { - if (missingKeys.length === 0) { - return Promise.resolve(undefined); - } - - return Storage.multiGet(missingKeys); - }) - // Add the data from the missing keys to the data map and also merge it to the cache. - .then((values) => { - if (!values || values.length === 0) { - return dataMap; - } - - // temp object is used to merge the missing data into the cache - const temp: OnyxCollection = {}; - for (const [key, value] of values) { - if (skippableCollectionMemberIDs.size) { - try { - const [, collectionMemberID] = OnyxKeys.splitCollectionMemberKey(key); - if (skippableCollectionMemberIDs.has(collectionMemberID)) { - // The key is a skippable one, so we skip this iteration. - continue; - } - } catch (e) { - // The key is not a collection one or something went wrong during split, so we proceed with the function's logic. - } - } - - dataMap.set(key, value as OnyxValue); - temp[key] = value as OnyxValue; - } - cache.merge(temp); - return dataMap; - }) - ); + return dataMap; } /** @@ -408,8 +274,8 @@ 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) => get(key))) as Promise<{[Index in keyof Keys]: OnyxValue}>; +function tupleGet(keys: Keys): {[Index in keyof Keys]: OnyxValue} { + return keys.map((key) => get(key)) as {[Index in keyof Keys]: OnyxValue}; } /** @@ -442,30 +308,8 @@ function deleteKeyBySubscriptions(subscriptionID: number) { } /** Returns current key names stored in persisted storage */ -function getAllKeys(): Promise> { - // When we've already read stored keys, resolve right away - const cachedKeys = cache.getAllKeys(); - if (cachedKeys.size > 0) { - return Promise.resolve(cachedKeys); - } - - // When a value retrieving task for all keys is still running hook to it - if (cache.hasPendingTask(TASK.GET_ALL_KEYS)) { - return cache.getTaskPromise(TASK.GET_ALL_KEYS) as Promise>; - } - - // Otherwise retrieve the keys from storage and capture a promise to aid concurrent usages - const promise = Storage.getAllKeys().then((keys) => { - // Filter out RAM-only keys from storage results as they may be stale entries - // from before the key was migrated to RAM-only. - const filteredKeys = keys.filter((key) => !OnyxKeys.isRamOnlyKey(key)); - cache.setAllKeys(filteredKeys); - - // return the updated set of keys - return cache.getAllKeys(); - }); - - return cache.captureTask(TASK.GET_ALL_KEYS, promise) as Promise>; +function getAllKeys(): Set { + return cache.getAllKeys(); } /** @@ -750,9 +594,7 @@ function sendDataToConnection(mapping: CallbackToStateMapp * Gets the data for a given an array of matching keys, combines them into an object, and sends the result back to the subscriber. */ function getCollectionDataAndSendAsObject(matchingKeys: CollectionKeyBase[], mapping: CallbackToStateMapping): void { - multiGet(matchingKeys).then(() => { - sendDataToConnection(mapping, mapping.key); - }); + sendDataToConnection(mapping, mapping.key); } /** @@ -1147,16 +989,14 @@ function subscribeToKey(connectOptions: ConnectOptions { - for (const key of matchingKeys) { - sendDataToConnection(mapping, key as TKey); - } - }); + for (const key of matchingKeys) { + sendDataToConnection(mapping, key as TKey); + } return; } // If we are not subscribed to a collection key then there's only a single key to send an update for. - get(mapping.key).then(() => sendDataToConnection(mapping, mapping.key)); + sendDataToConnection(mapping, mapping.key); return; } @@ -1432,39 +1272,38 @@ function setCollectionWithRetry({collectionKey, } resultCollectionKeys = Object.keys(resultCollection); - return OnyxUtils.getAllKeys().then((persistedKeys) => { - const mutableCollection: OnyxInputKeyValueMapping = {...resultCollection}; + const persistedKeys = OnyxUtils.getAllKeys(); + const mutableCollection: OnyxInputKeyValueMapping = {...resultCollection}; - for (const key of persistedKeys) { - if (!key.startsWith(collectionKey)) { - continue; - } - if (resultCollectionKeys.includes(key)) { - continue; - } - - mutableCollection[key] = null; + for (const key of persistedKeys) { + if (!key.startsWith(collectionKey)) { + continue; + } + if (resultCollectionKeys.includes(key)) { + continue; } - const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true); - const previousCollection = OnyxUtils.getCachedCollection(collectionKey); + mutableCollection[key] = null; + } - for (const [key, value] of keyValuePairs) cache.set(key, value); + const keyValuePairs = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true); + const previousCollection = OnyxUtils.getCachedCollection(collectionKey); - keysChanged(collectionKey, mutableCollection, previousCollection); + for (const [key, value] of keyValuePairs) cache.set(key, value); - // RAM-only keys are not supposed to be saved to storage - if (OnyxKeys.isRamOnlyKey(collectionKey)) { - OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection); - return; - } + keysChanged(collectionKey, mutableCollection, previousCollection); - return Storage.multiSet(keyValuePairs) - .catch((error) => OnyxUtils.retryOperation(error, setCollectionWithRetry, {collectionKey, collection}, retryAttempt)) - .then(() => { - OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection); - }); - }); + // RAM-only keys are not supposed to be saved to storage + if (OnyxKeys.isRamOnlyKey(collectionKey)) { + OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection); + return Promise.resolve(); + } + + return Storage.multiSet(keyValuePairs) + .catch((error) => OnyxUtils.retryOperation(error, setCollectionWithRetry, {collectionKey, collection}, retryAttempt)) + .then(() => { + OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection); + }); } /** @@ -1515,103 +1354,94 @@ function mergeCollectionWithPatches( } 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, isProcessingCollectionUpdate); - return false; - } - return true; - }); + const persistedKeys = getAllKeys(); - const existingKeys = keys.filter((key) => persistedKeys.has(key)); + // Split to keys that exist in storage and keys that don't + const keys = resultCollectionKeys.filter((key) => { + if (resultCollection[key] === null) { + remove(key, isProcessingCollectionUpdate); + return false; + } + return true; + }); - const cachedCollectionForExistingKeys = getCachedCollection(collectionKey, existingKeys); + const existingKeys = keys.filter((key) => persistedKeys.has(key)); - const existingKeyCollection = existingKeys.reduce((obj: OnyxInputKeyValueMapping, key) => { - const {isCompatible, existingValueType, newValueType, isEmptyArrayCoercion} = utils.checkCompatibilityWithExistingValue( - resultCollection[key], - cachedCollectionForExistingKeys[key], - ); + const cachedCollectionForExistingKeys = getCachedCollection(collectionKey, existingKeys); - if (isEmptyArrayCoercion) { - // Merging an object into an empty array isn't semantically correct, but we allow it - // in case we accidentally encoded an empty object as an empty array in PHP. If you're - // looking at a bugbot from this message, we're probably missing that key in OnyxKeys::KEYS_REQUIRING_EMPTY_OBJECT - Logger.logAlert(`[ENSURE_BUGBOT] Onyx mergeCollection called on key "${key}" whose existing value is an empty array. Will coerce to object.`); - } - if (!isCompatible) { - Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'mergeCollection', existingValueType, newValueType)); - return obj; - } + const existingKeyCollection = existingKeys.reduce((obj: OnyxInputKeyValueMapping, key) => { + const {isCompatible, existingValueType, newValueType, isEmptyArrayCoercion} = utils.checkCompatibilityWithExistingValue(resultCollection[key], cachedCollectionForExistingKeys[key]); - // eslint-disable-next-line no-param-reassign - obj[key] = resultCollection[key]; - return obj; - }, {}) as Record>; + if (isEmptyArrayCoercion) { + // Merging an object into an empty array isn't semantically correct, but we allow it + // in case we accidentally encoded an empty object as an empty array in PHP. If you're + // looking at a bugbot from this message, we're probably missing that key in OnyxKeys::KEYS_REQUIRING_EMPTY_OBJECT + Logger.logAlert(`[ENSURE_BUGBOT] Onyx mergeCollection called on key "${key}" whose existing value is an empty array. Will coerce to object.`); + } + if (!isCompatible) { + Logger.logAlert(logMessages.incompatibleUpdateAlert(key, 'mergeCollection', existingValueType, newValueType)); + return obj; + } - const newCollection: Record> = {}; - for (const key of keys) { - if (persistedKeys.has(key)) { - continue; - } - newCollection[key] = resultCollection[key]; - } + // eslint-disable-next-line no-param-reassign + obj[key] = resultCollection[key]; + return obj; + }, {}) as Record>; - // 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); + const newCollection: Record> = {}; + for (const key of keys) { + if (persistedKeys.has(key)) { + continue; + } + newCollection[key] = resultCollection[key]; + } - // 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); + // 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); - const promises = []; + // 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); - // 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); + const promises = []; - // 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 - // We can skip this step for RAM-only keys as they should never be saved to storage - if (!OnyxKeys.isRamOnlyKey(collectionKey) && keyValuePairsForExistingCollection.length > 0) { - promises.push(Storage.multiMerge(keyValuePairsForExistingCollection)); - } + // We need to get the previously existing values so we can compare the new ones + // against them, to avoid unnecessary subscriber updates. + const previousCollection = Object.fromEntries(multiGet(existingKeys)); - // We can skip this step for RAM-only keys as they should never be saved to storage - if (!OnyxKeys.isRamOnlyKey(collectionKey) && keyValuePairsForNewCollection.length > 0) { - promises.push(Storage.multiSet(keyValuePairsForNewCollection)); - } + // 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 + // We can skip this step for RAM-only keys as they should never be saved to storage + if (!OnyxKeys.isRamOnlyKey(collectionKey) && keyValuePairsForExistingCollection.length > 0) { + promises.push(Storage.multiMerge(keyValuePairsForExistingCollection)); + } - // 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); - keysChanged(collectionKey, finalMergedCollection, previousCollection); - }); - - return Promise.all(promises) - .catch((error) => - retryOperation( - error, - mergeCollectionWithPatches, - {collectionKey, collection: resultCollection as OnyxMergeCollectionInput, mergeReplaceNullPatches, isProcessingCollectionUpdate}, - retryAttempt, - ), - ) - .then(() => { - sendActionToDevTools(METHOD.MERGE_COLLECTION, undefined, resultCollection); - return promiseUpdate; - }); - }) - .then(() => undefined); + // We can skip this step for RAM-only keys as they should never be saved to storage + if (!OnyxKeys.isRamOnlyKey(collectionKey) && 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}; + + // Merge original data to cache and update all subscribers + cache.merge(finalMergedCollection); + keysChanged(collectionKey, finalMergedCollection, previousCollection); + + return Promise.all(promises) + .catch((error) => + retryOperation( + error, + mergeCollectionWithPatches, + {collectionKey, collection: resultCollection as OnyxMergeCollectionInput, mergeReplaceNullPatches, isProcessingCollectionUpdate}, + retryAttempt, + ), + ) + .then(() => { + sendActionToDevTools(METHOD.MERGE_COLLECTION, undefined, resultCollection); + }); } /** @@ -1652,27 +1482,26 @@ function partialSetCollection({collectionKey, co } resultCollectionKeys = Object.keys(resultCollection); - return getAllKeys().then((persistedKeys) => { - const mutableCollection: OnyxInputKeyValueMapping = {...resultCollection}; - const existingKeys = resultCollectionKeys.filter((key) => persistedKeys.has(key)); - const previousCollection = getCachedCollection(collectionKey, existingKeys); - const keyValuePairs = prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true); + const persistedKeys = getAllKeys(); + const mutableCollection: OnyxInputKeyValueMapping = {...resultCollection}; + const existingKeys = resultCollectionKeys.filter((key) => persistedKeys.has(key)); + const previousCollection = getCachedCollection(collectionKey, existingKeys); + const keyValuePairs = prepareKeyValuePairsForStorage(mutableCollection, true, undefined, true); - for (const [key, value] of keyValuePairs) cache.set(key, value); + for (const [key, value] of keyValuePairs) cache.set(key, value); - keysChanged(collectionKey, mutableCollection, previousCollection); + keysChanged(collectionKey, mutableCollection, previousCollection); - if (OnyxKeys.isRamOnlyKey(collectionKey)) { - sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); - return; - } + if (OnyxKeys.isRamOnlyKey(collectionKey)) { + sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); + return Promise.resolve(); + } - return Storage.multiSet(keyValuePairs) - .catch((error) => retryOperation(error, partialSetCollection, {collectionKey, collection}, retryAttempt)) - .then(() => { - sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); - }); - }); + return Storage.multiSet(keyValuePairs) + .catch((error) => retryOperation(error, partialSetCollection, {collectionKey, collection}, retryAttempt)) + .then(() => { + sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); + }); } function logKeyChanged(onyxMethod: Extract, key: OnyxKey, value: unknown, hasChanged: boolean) { diff --git a/tests/perf-test/OnyxUtils.perf-test.ts b/tests/perf-test/OnyxUtils.perf-test.ts index afd03df7..29fe6b90 100644 --- a/tests/perf-test/OnyxUtils.perf-test.ts +++ b/tests/perf-test/OnyxUtils.perf-test.ts @@ -115,7 +115,7 @@ describe('OnyxUtils', () => { describe('get', () => { test('10k calls with heavy objects', async () => { - await measureAsyncFunction(() => Promise.all(mockedReportActionsKeys.map((key) => OnyxUtils.get(key))), { + await measureFunction(() => mockedReportActionsKeys.map((key) => OnyxUtils.get(key)), { beforeEach: async () => { await StorageMock.multiSet(Object.entries(mockedReportActionsMap).map(([k, v]) => [k, v])); }, @@ -126,7 +126,7 @@ describe('OnyxUtils', () => { describe('getAllKeys', () => { test('one call with 50k heavy objects', async () => { - await measureAsyncFunction(() => OnyxUtils.getAllKeys(), { + await measureFunction(() => OnyxUtils.getAllKeys(), { beforeEach: async () => { await StorageMock.multiSet(Object.entries(mockedReportActionsMap).map(([k, v]) => [k, v])); }, @@ -256,10 +256,10 @@ describe('OnyxUtils', () => { ...getRandomReportActions(ONYXKEYS.COLLECTION.EVICTABLE_TEST_KEY, 1000), }; const fakeMethodParameter = () => false; - const fakePromiseMethodParameter = () => Promise.resolve(new Set(Object.keys(data))); + const fakeGetAllKeysFn = () => new Set(Object.keys(data)); test('one call adding 1k keys', async () => { - await measureAsyncFunction(() => OnyxCache.addEvictableKeysToRecentlyAccessedList(fakeMethodParameter, fakePromiseMethodParameter), { + await measureFunction(() => OnyxCache.addEvictableKeysToRecentlyAccessedList(fakeMethodParameter, fakeGetAllKeysFn), { beforeEach: async () => { await Onyx.multiSet(data); }, @@ -545,7 +545,7 @@ describe('OnyxUtils', () => { describe('multiGet', () => { test('one call getting 10k heavy objects from storage', async () => { - await measureAsyncFunction(() => OnyxUtils.multiGet(mockedReportActionsKeys), { + await measureFunction(() => OnyxUtils.multiGet(mockedReportActionsKeys), { beforeEach: async () => { await StorageMock.multiSet(Object.entries(mockedReportActionsMap).map(([k, v]) => [k, v])); }, @@ -554,7 +554,7 @@ describe('OnyxUtils', () => { }); test('one call getting 10k heavy objects from cache', async () => { - await measureAsyncFunction(() => OnyxUtils.multiGet(mockedReportActionsKeys), { + await measureFunction(() => OnyxUtils.multiGet(mockedReportActionsKeys), { beforeEach: async () => { await Onyx.multiSet(mockedReportActionsMap); }, diff --git a/tests/perf-test/useOnyx.perf-test.tsx b/tests/perf-test/useOnyx.perf-test.tsx index 6cbc4159..5dc6be3c 100644 --- a/tests/perf-test/useOnyx.perf-test.tsx +++ b/tests/perf-test/useOnyx.perf-test.tsx @@ -4,7 +4,6 @@ import {Text, View} from 'react-native'; import {measureRenders} from 'reassure'; import type {FetchStatus, OnyxEntry, OnyxKey, OnyxValue, ResultMetadata, UseOnyxOptions} from '../../lib'; import Onyx, {useOnyx} from '../../lib'; -import StorageMock from '../../lib/storage'; import type {UseOnyxSelector} from '../../lib/useOnyx'; const ONYXKEYS = { @@ -80,23 +79,6 @@ describe('useOnyx', () => { }); }); - /** - * Expected renders: 2. - */ - test('data in storage but not yet in cache', async () => { - const key = ONYXKEYS.TEST_KEY; - await measureRenders(, { - beforeEach: async () => { - await StorageMock.setItem(key, 'test'); - }, - scenario: async () => { - await screen.findByText(dataMatcher(key, 'test')); - await screen.findByText(metadataStatusMatcher(key, 'loaded')); - }, - afterEach: clearOnyxAfterEachMeasure, - }); - }); - /** * Expected renders: 1. */ @@ -209,7 +191,7 @@ describe('useOnyx', () => { />, { beforeEach: async () => { - await StorageMock.setItem(key, 'test'); + await Onyx.set(key, 'test'); }, scenario: async () => { await screen.findByText(dataMatcher(key, undefined)); @@ -222,54 +204,6 @@ describe('useOnyx', () => { }); describe('multiple calls', () => { - /** - * Expected renders: 2. - */ - test('3 calls loading from storage', async () => { - function TestComponent() { - const [testKeyData, testKeyMetadata] = useOnyx(ONYXKEYS.TEST_KEY); - const [testKey2Data, testKey2Metadata] = useOnyx(ONYXKEYS.TEST_KEY_2); - const [testKey3Data, testKey3Metadata] = useOnyx(ONYXKEYS.TEST_KEY_3); - - return ( - - - - - - ); - } - - await measureRenders(, { - beforeEach: async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - await StorageMock.setItem(ONYXKEYS.TEST_KEY_2, 'test2'); - await StorageMock.setItem(ONYXKEYS.TEST_KEY_3, 'test3'); - }, - scenario: async () => { - await screen.findByText(dataMatcher(ONYXKEYS.TEST_KEY, 'test')); - await screen.findByText(metadataStatusMatcher(ONYXKEYS.TEST_KEY, 'loaded')); - await screen.findByText(dataMatcher(ONYXKEYS.TEST_KEY_2, 'test2')); - await screen.findByText(metadataStatusMatcher(ONYXKEYS.TEST_KEY_2, 'loaded')); - await screen.findByText(dataMatcher(ONYXKEYS.TEST_KEY_3, 'test3')); - await screen.findByText(metadataStatusMatcher(ONYXKEYS.TEST_KEY_3, 'loaded')); - }, - afterEach: clearOnyxAfterEachMeasure, - }); - }); - /** * Expected renders: 1. */ diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts index d32816e1..07b6ff94 100644 --- a/tests/unit/OnyxConnectionManagerTest.ts +++ b/tests/unit/OnyxConnectionManagerTest.ts @@ -2,7 +2,6 @@ import {act} from '@testing-library/react-native'; import Onyx from '../../lib'; import type {Connection} from '../../lib/OnyxConnectionManager'; import connectionManager from '../../lib/OnyxConnectionManager'; -import StorageMock from '../../lib/storage'; import type GenericCollection from '../utils/GenericCollection'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; @@ -69,7 +68,7 @@ describe('OnyxConnectionManager', () => { describe('connect / disconnect', () => { it('should connect to a key and fire the callback with its value', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); const connection = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); @@ -87,7 +86,7 @@ describe('OnyxConnectionManager', () => { }); it('should connect two times to the same key and fire both callbacks with its value', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); @@ -119,10 +118,7 @@ describe('OnyxConnectionManager', () => { [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: obj1, [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: obj2, } as GenericCollection; - await StorageMock.multiSet([ - [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, obj1], - [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`, obj2], - ]); + await Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, collection); const callback1 = jest.fn(); const connection1 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, callback: callback1}); @@ -151,7 +147,7 @@ describe('OnyxConnectionManager', () => { }); it('should connect to a key, connect some times more after first connection is made, and fire all subsequent callbacks immediately with its value', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); @@ -181,7 +177,7 @@ describe('OnyxConnectionManager', () => { }); it('should have the connection object already defined when triggering the callback of the second connection to the same key', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); @@ -208,7 +204,7 @@ describe('OnyxConnectionManager', () => { }); it('should create a separate connection to the same key when setting reuseConnection to false', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); @@ -223,7 +219,7 @@ describe('OnyxConnectionManager', () => { }); it('should create a separate connection to the same key when setting initWithStoredValues to false', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, initWithStoredValues: false, callback: callback1}); @@ -309,7 +305,7 @@ describe('OnyxConnectionManager', () => { }); it('should create a separate connection for the same key after a Onyx.clear() call', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); const callback1 = jest.fn(); connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); @@ -408,8 +404,8 @@ describe('OnyxConnectionManager', () => { describe('disconnectAll', () => { it('should disconnect all connections', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - await StorageMock.setItem(ONYXKEYS.TEST_KEY_2, 'test2'); + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); + await Onyx.set(ONYXKEYS.TEST_KEY_2, 'test2'); const callback1 = jest.fn(); const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: callback1}); @@ -435,8 +431,8 @@ describe('OnyxConnectionManager', () => { describe('refreshSessionID', () => { it('should create a separate connection for the same key if the session ID changes', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - await StorageMock.setItem(ONYXKEYS.TEST_KEY_2, 'test2'); + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); + await Onyx.set(ONYXKEYS.TEST_KEY_2, 'test2'); const connection1 = connectionManager.connect({key: ONYXKEYS.TEST_KEY, callback: jest.fn()}); diff --git a/tests/unit/onyxMultiMergeWebStorageTest.ts b/tests/unit/onyxMultiMergeWebStorageTest.ts index 84ea6c40..7a08be9f 100644 --- a/tests/unit/onyxMultiMergeWebStorageTest.ts +++ b/tests/unit/onyxMultiMergeWebStorageTest.ts @@ -35,57 +35,53 @@ describe('Onyx.mergeCollection() and WebStorage', () => { afterEach(() => Onyx.clear()); - it('merges two sets of data consecutively', () => { - StorageMock.setMockStore(initialData); - - // Given initial data in storage - expect(StorageMock.getMockStore().test_1).toEqual(initialTestObject); - expect(StorageMock.getMockStore().test_2).toEqual(initialTestObject); - expect(StorageMock.getMockStore().test_3).toEqual(initialTestObject); - - // And an empty cache values for the collection keys - expect(OnyxCache.get('test_1')).not.toBeDefined(); - expect(OnyxCache.get('test_2')).not.toBeDefined(); - expect(OnyxCache.get('test_3')).not.toBeDefined(); + it('merges two sets of data consecutively', () => + // Given initial data set through Onyx (populates both cache and storage) + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, initialData as GenericCollection) + .then(() => { + expect(OnyxCache.get('test_1')).toEqual(initialTestObject); + expect(OnyxCache.get('test_2')).toEqual(initialTestObject); + expect(OnyxCache.get('test_3')).toEqual(initialTestObject); - // When we merge additional data - const additionalDataOne = {b: 'b', c: 'c', e: [1, 2]}; - Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - test_1: additionalDataOne, - test_2: additionalDataOne, - test_3: additionalDataOne, - } as GenericCollection); + // When we merge additional data + const additionalDataOne = {b: 'b', c: 'c', e: [1, 2]}; + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: additionalDataOne, + test_2: additionalDataOne, + test_3: additionalDataOne, + } as GenericCollection); - // And call again consecutively with different data - const additionalDataTwo = {d: 'd', e: [2]}; - Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { - test_1: additionalDataTwo, - test_2: additionalDataTwo, - test_3: additionalDataTwo, - } as GenericCollection); + // And call again consecutively with different data + const additionalDataTwo = {d: 'd', e: [2]}; + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: additionalDataTwo, + test_2: additionalDataTwo, + test_3: additionalDataTwo, + } as GenericCollection); - return waitForPromisesToResolve().then(() => { - const finalObject = { - a: 'a', - b: 'b', - c: 'c', - d: 'd', - e: [2], - }; + return waitForPromisesToResolve(); + }) + .then(() => { + const finalObject = { + a: 'a', + b: 'b', + c: 'c', + d: 'd', + e: [2], + }; - // Then our new data should merge with the existing data in the cache - expect(OnyxCache.get('test_1')).toEqual(finalObject); - expect(OnyxCache.get('test_2')).toEqual(finalObject); - expect(OnyxCache.get('test_3')).toEqual(finalObject); + // Then our new data should merge with the existing data in the cache + expect(OnyxCache.get('test_1')).toEqual(finalObject); + expect(OnyxCache.get('test_2')).toEqual(finalObject); + expect(OnyxCache.get('test_3')).toEqual(finalObject); - // And the storage should reflect the same state - expect(StorageMock.getMockStore().test_1).toEqual(finalObject); - expect(StorageMock.getMockStore().test_2).toEqual(finalObject); - expect(StorageMock.getMockStore().test_3).toEqual(finalObject); - }); - }); + // And the storage should reflect the same state + expect(StorageMock.getMockStore().test_1).toEqual(finalObject); + expect(StorageMock.getMockStore().test_2).toEqual(finalObject); + expect(StorageMock.getMockStore().test_3).toEqual(finalObject); + })); - it('cache updates correctly when accessed again if keys are removed or evicted', () => { + it('cache and storage stay in sync after consecutive mergeCollection calls', () => { // Given empty storage expect(StorageMock.getMockStore().test_1).toBeFalsy(); expect(StorageMock.getMockStore().test_2).toBeFalsy(); @@ -114,11 +110,7 @@ describe('Onyx.mergeCollection() and WebStorage', () => { expect(StorageMock.getMockStore().test_2).toEqual(data); expect(StorageMock.getMockStore().test_3).toEqual(data); - // When we drop all the cache keys (but do not modify the underlying storage) and merge another object - OnyxCache.drop('test_1'); - OnyxCache.drop('test_2'); - OnyxCache.drop('test_3'); - + // When we merge another object on top of existing data const additionalData = {c: 'c'}; Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { test_1: additionalData, diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 5247cfc3..16a29f7f 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -65,17 +65,15 @@ describe('Onyx', () => { it('should remove key value from OnyxCache/Storage when set is called with null value', () => Onyx.set(ONYX_KEYS.OTHER_TEST, 42) - .then(() => OnyxUtils.getAllKeys()) - .then((keys) => { + .then(() => { + const keys = OnyxUtils.getAllKeys(); expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(true); return Onyx.set(ONYX_KEYS.OTHER_TEST, null); }) // Checks if cache value is removed. .then(() => { expect(cache.get(ONYX_KEYS.OTHER_TEST)).toBeUndefined(); - return OnyxUtils.getAllKeys(); - }) - .then((keys) => { + const keys = OnyxUtils.getAllKeys(); expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(false); })); @@ -3265,7 +3263,7 @@ describe('RAM-only keys should not read from storage', () => { }); await act(async () => waitForPromisesToResolve()); - const keys = await OnyxUtils.getAllKeys(); + const keys = OnyxUtils.getAllKeys(); expect(keys.has(ONYX_KEYS.RAM_ONLY_TEST_KEY)).toBe(false); expect(keys.has(`${ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`)).toBe(false); @@ -3437,7 +3435,7 @@ describe('RAM-only keys should not read from storage', () => { await Onyx.set(ramOnlyMember, {data: 'fresh_from_cache'}); // multiGet receives individual keys (e.g. collection members), not collection base keys - const result = await OnyxUtils.multiGet([normalMember, ramOnlyMember]); + const result = OnyxUtils.multiGet([normalMember, ramOnlyMember]); // Normal key should come from storage expect(result.get(normalMember)).toEqual('normal_from_storage'); diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index b4baa1f8..4ccd5e41 100644 --- a/tests/unit/useOnyxTest.ts +++ b/tests/unit/useOnyxTest.ts @@ -272,20 +272,6 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); }); - it('should initially return `undefined` while loading non-cached key, and then return value and loaded state', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - - const {result} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - expect(result.current[0]).toBeUndefined(); - expect(result.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - - expect(result.current[0]).toEqual('test'); - expect(result.current[1].status).toEqual('loaded'); - }); - it('should initially return undefined and then return cached value after multiple merge operations', async () => { Onyx.merge(ONYXKEYS.TEST_KEY, 'test1'); Onyx.merge(ONYXKEYS.TEST_KEY, 'test2'); @@ -339,12 +325,10 @@ describe('useOnyx', () => { }); it('should return updated state when connecting to the same regular key after an Onyx.clear() call', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - await act(async () => waitForPromisesToResolve()); - expect(result1.current[0]).toEqual('test'); expect(result1.current[1].status).toEqual('loaded'); @@ -374,12 +358,10 @@ describe('useOnyx', () => { }); it('should return updated state when connecting to the same colection member key after an Onyx.clear() call', async () => { - await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, 'test'); + await Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`, 'test'); const {result: result1} = renderHook(() => useOnyx(`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`)); - await act(async () => waitForPromisesToResolve()); - expect(result1.current[0]).toEqual('test'); expect(result1.current[1].status).toEqual('loaded'); @@ -944,39 +926,12 @@ describe('useOnyx', () => { }); describe('multiple usage', () => { - it('should connect to a key and load the value into cache, and return the value loaded in the next hook call', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); - - const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - expect(result1.current[0]).toBeUndefined(); - expect(result1.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - - expect(result1.current[0]).toEqual('test'); - expect(result1.current[1].status).toEqual('loaded'); - - const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - - expect(result2.current[0]).toEqual('test'); - expect(result2.current[1].status).toEqual('loaded'); - }); - - it('should connect to a key two times while data is loading from the cache, and return the value loaded to both of them', async () => { - await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test'); + it('should connect to a key two times and return the cached value to both of them', async () => { + await Onyx.set(ONYXKEYS.TEST_KEY, 'test'); const {result: result1} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); const {result: result2} = renderHook(() => useOnyx(ONYXKEYS.TEST_KEY)); - expect(result1.current[0]).toBeUndefined(); - expect(result1.current[1].status).toEqual('loading'); - - expect(result2.current[0]).toBeUndefined(); - expect(result2.current[1].status).toEqual('loading'); - - await act(async () => waitForPromisesToResolve()); - expect(result1.current[0]).toEqual('test'); expect(result1.current[1].status).toEqual('loaded'); @@ -1135,11 +1090,11 @@ describe('useOnyx', () => { expect(result.current[1].status).toEqual('loaded'); }); - it('should transition through loading and return value when switching from a skippable key to a valid one', async () => { + it('should return value immediately when switching from a skippable key to a valid one', async () => { // Seed a value for the skippable key — must stay invisible to the hook - await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`, {id: 'skippable'}); - // Seed the target valid key in storage only (not in cache) so the switch goes through loading - await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, {id: '1'}); + await Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`, {id: 'skippable'}); + // Seed the target valid key + await Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, {id: '1'}); const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id` as string}); @@ -1148,11 +1103,9 @@ describe('useOnyx', () => { expect(result.current[0]).toBeUndefined(); expect(result.current[1].status).toEqual('loaded'); - // Switch to a valid key whose value is in storage but not in cache — should transition through loading + // Switch to a valid key — value is available immediately from cache rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}1`); - expect(result.current[1].status).toEqual('loading'); - await act(async () => waitForPromisesToResolve()); expect(result.current[0]).toEqual({id: '1'});