diff --git a/API.md b/API.md index e766e551f..3fc28c26c 100644 --- a/API.md +++ b/API.md @@ -58,6 +58,11 @@ value will be saved to storage after the default value.

Sets a collection by replacing all existing collection members with new values. Any existing collection members not included in the new data will be removed.

+
get()
+

Reads a value out of the cache synchronously. A collection key reads every member.

+

Not a subscription, so the value never updates: use useOnyx for anything rendered. Returns undefined for a +key with no value, and for every key until init() has hydrated the cache.

+
@@ -257,3 +262,12 @@ Onyx.setCollection(ONYXKEYS.COLLECTION.REPORT, { [`${ONYXKEYS.COLLECTION.REPORT}2`]: report2, }); ``` + + +## get() +Reads a value out of the cache synchronously. A collection key reads every member. + +Not a subscription, so the value never updates: use `useOnyx` for anything rendered. Returns `undefined` for a +key with no value, and for every key until `init()` has hydrated the cache. + +**Kind**: global function diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 0a0d8abc9..60606ad6f 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -255,16 +255,17 @@ function merge(key: TKey, changes: OnyxMergeInput): } mergeQueue[key] = [changes]; - mergeQueuePromise[key] = OnyxUtils.get(key).then((valueFromGet) => { + 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(); } // Other writers (notably Onyx.update's mergeCollection path, which doesn't participate in mergeQueue) - // can land between get() resolving and this callback running. Applying the delta on top of the value - // captured back then and broadcasting it would overwrite those writes wholesale, so re-read the cache. - const existingValue = cache.hasCacheForKey(key) ? (cache.get(key) as OnyxInput | undefined) : valueFromGet; + // can land between this merge being queued and this callback running. Applying the delta on top of a + // value captured back then and broadcasting it would overwrite those writes wholesale, so read the + // existing value here, at merge application time. + const existingValue = OnyxUtils.get(key); try { const validChanges = mergeQueue[key].filter((change) => { @@ -353,92 +354,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; }); @@ -559,6 +566,11 @@ function update(data: Array>): Promise OnyxUtils.partialSetCollection({collectionKey, collection: batchedCollectionUpdates.set as OnyxSetCollectionInput})); + } if (!utils.isEmptyObject(batchedCollectionUpdates.merge)) { promises.push(() => OnyxUtils.mergeCollectionWithPatches({ @@ -568,9 +580,6 @@ function update(data: Array>): Promise OnyxUtils.partialSetCollection({collectionKey, collection: batchedCollectionUpdates.set as OnyxSetCollectionInput})); - } } for (const [key, operations] of Object.entries(updateQueue)) { @@ -611,11 +620,22 @@ function setCollection(collectionKey: TKey, coll return OnyxUtils.afterInit(() => OnyxUtils.setCollectionWithRetry({collectionKey, collection})); } +/** + * Reads a value out of the cache synchronously. A collection key reads every member. + * + * Not a subscription, so the value never updates: use `useOnyx` for anything rendered. Returns `undefined` for a + * key with no value, and for every key until `init()` has hydrated the cache. + */ +function get(key: TKey): OnyxValue { + return OnyxUtils.tryGetCachedValue(key) as OnyxValue; +} + const Onyx = { METHOD: OnyxUtils.METHOD, connect, connectWithoutView, disconnect, + get, set, multiSet, merge, diff --git a/lib/OnyxCache.ts b/lib/OnyxCache.ts index e105a0ab6..012b0dd8c 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 400cfbb2b..a2c790bfd 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 StorageCircuitBreaker from './StorageCircuitBreaker'; import Storage from './storage'; @@ -255,163 +255,23 @@ 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; - } - // hasCacheForKey catches cached falsy values (0, '', false, null) as cache hits, which // a truthy check on the value would miss. if (cache.hasCacheForKey(key)) { dataMap.set(key, cache.get(key) as OnyxValue); - 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); } } - 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. - } - } - - // Prefer cache over stale storage if a concurrent write populated it during - // the read — otherwise cache.merge(temp) below would resurrect dropped fields. - if (cache.hasCacheForKey(key)) { - dataMap.set(key, cache.get(key) as OnyxValue); - continue; - } - - dataMap.set(key, value as OnyxValue); - temp[key] = value as OnyxValue; - } - cache.merge(temp); - return dataMap; - }) - ); + return dataMap; } /** @@ -420,10 +280,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}; } /** @@ -456,30 +314,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(); } /** @@ -743,9 +579,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); } /** @@ -1189,7 +1023,7 @@ function subscribeToKey(connectOptions: ConnectOptions sendDataToConnection(mapping, mapping.key)); + sendDataToConnection(mapping, mapping.key); return; } @@ -1548,59 +1382,58 @@ 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; } - - const {pairs: keyValuePairs, keysToRemove: removalCandidates} = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true); - // Removals of keys that are neither cached nor persisted are no-ops and skipped. - const keysToRemove = removalCandidates.filter((key) => cache.get(key) !== undefined || persistedKeys.has(key)); - // Snapshot before cache mutations so keysChanged() can diff removed members. - const previousCollection = OnyxUtils.getCachedCollection(collectionKey); - - for (const [key, value] of keyValuePairs) cache.set(key, value); - for (const key of keysToRemove) cache.drop(key); - - // Skip subscriber notification on retry — already notified on attempt 0. - // Collection-root subscribers re-fire on every keysChanged by contract. - if (!retryAttempt) { - // Removed members are notified as undefined, matching mergeCollection/multiSet. - const partialForNotify = Object.fromEntries(Object.entries(mutableCollection).map(([key, value]) => [key, value ?? undefined])); - keysChanged(collectionKey, partialForNotify, previousCollection); + if (resultCollectionKeys.includes(key)) { + continue; } - // RAM-only keys are not supposed to be saved to storage - if (OnyxKeys.isRamOnlyKey(collectionKey)) { - OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection); - return; - } + mutableCollection[key] = null; + } - const inFlightKeys = new Set(keyValuePairs.map(([key]) => key)); + const {pairs: keyValuePairs, keysToRemove: removalCandidates} = OnyxUtils.prepareKeyValuePairsForStorage(mutableCollection, true); + // Removals of keys that are neither cached nor persisted are no-ops and skipped. + const keysToRemove = removalCandidates.filter((key) => cache.get(key) !== undefined || persistedKeys.has(key)); + // Snapshot before cache mutations so keysChanged() can diff removed members. + const previousCollection = OnyxUtils.getCachedCollection(collectionKey); - // One batched removal = one cross-tab sync event instead of one per key. A failed removal is - // logged, not retried — keysToRemove cannot be re-derived after the cache update. - const storagePromises = [Storage.multiSet(keyValuePairs)]; - if (keysToRemove.length > 0) { - storagePromises.push(Storage.removeItems(keysToRemove).catch((error) => Logger.logAlert(`setCollection failed to remove keys from storage. Error: ${error}`))); - } + for (const [key, value] of keyValuePairs) cache.set(key, value); + for (const key of keysToRemove) cache.drop(key); - return Promise.all(storagePromises) - .then(() => StorageCircuitBreaker.recordWriteSuccess()) - .catch((error) => OnyxUtils.retryOperation(error, setCollectionWithRetry, {collectionKey, collection}, retryAttempt, inFlightKeys)) - .then(() => { - OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection); - }); - }); + // Skip subscriber notification on retry — already notified on attempt 0. + // Collection-root subscribers re-fire on every keysChanged by contract. + if (!retryAttempt) { + // Removed members are notified as undefined, matching mergeCollection/multiSet. + const partialForNotify = Object.fromEntries(Object.entries(mutableCollection).map(([key, value]) => [key, value ?? undefined])); + keysChanged(collectionKey, partialForNotify, previousCollection); + } + + // 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(); + } + + const inFlightKeys = new Set(keyValuePairs.map(([key]) => key)); + + // One batched removal = one cross-tab sync event instead of one per key. A failed removal is + // logged, not retried — keysToRemove cannot be re-derived after the cache update. + const storagePromises = [Storage.multiSet(keyValuePairs)]; + if (keysToRemove.length > 0) { + storagePromises.push(Storage.removeItems(keysToRemove).catch((error) => Logger.logAlert(`setCollection failed to remove keys from storage. Error: ${error}`))); + } + + return Promise.all(storagePromises) + .then(() => StorageCircuitBreaker.recordWriteSuccess()) + .catch((error) => OnyxUtils.retryOperation(error, setCollectionWithRetry, {collectionKey, collection}, retryAttempt, inFlightKeys)) + .then(() => { + OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection); + }); } /** @@ -1650,157 +1483,142 @@ function mergeCollectionWithPatches( } resultCollectionKeys = Object.keys(resultCollection); - return getAllKeys() - .then((persistedKeys) => { - // Split to keys that exist in storage and keys that don't. Null members are collected - // for one batched removal below; nulls that are neither cached nor persisted are no-ops and skipped. - const keysToRemove: OnyxKey[] = []; - const keys = resultCollectionKeys.filter((key) => { - if (resultCollection[key] === null) { - if (cache.get(key) !== undefined || persistedKeys.has(key)) { - keysToRemove.push(key); - } - return false; - } - return true; - }); + const persistedKeys = getAllKeys(); - // Drop removed members before the pre-warm await below, so a concurrent write to one of - // these keys during the pre-warm is not wiped out by a late drop. - const removedPreviousValues: OnyxInputKeyValueMapping = {}; - for (const key of keysToRemove) { - removedPreviousValues[key] = cache.get(key); - cache.drop(key); + // Split to keys that exist in storage and keys that don't. Null members are collected + // for one batched removal below; nulls that are neither cached nor persisted are no-ops and skipped. + const keysToRemove: OnyxKey[] = []; + const keys = resultCollectionKeys.filter((key) => { + if (resultCollection[key] === null) { + if (cache.get(key) !== undefined || persistedKeys.has(key)) { + keysToRemove.push(key); } + return false; + } + return true; + }); - // One batched removal = one cross-tab sync event instead of one per key. Issued at drop time - // so a concurrent later write to a removed key persists after the removal. - const removalPromise = - !OnyxKeys.isRamOnlyKey(collectionKey) && keysToRemove.length > 0 - ? Storage.removeItems(keysToRemove).catch((error) => Logger.logAlert(`mergeCollection failed to remove keys from storage. Error: ${error}`)) - : undefined; - - const existingKeys = keys.filter((key) => persistedKeys.has(key)); + // Snapshot the previous values before dropping so keysChanged() below can diff removed members. + const removedPreviousValues: OnyxInputKeyValueMapping = {}; + for (const key of keysToRemove) { + removedPreviousValues[key] = cache.get(key); + cache.drop(key); + } - const cachedCollectionForExistingKeys = getCachedCollection(collectionKey, existingKeys); + // One batched removal = one cross-tab sync event instead of one per key. Issued at drop time + // so a concurrent later write to a removed key persists after the removal. + const removalPromise = + !OnyxKeys.isRamOnlyKey(collectionKey) && keysToRemove.length > 0 + ? Storage.removeItems(keysToRemove).catch((error) => Logger.logAlert(`mergeCollection failed to remove keys from storage. Error: ${error}`)) + : undefined; - const existingKeyCollection = existingKeys.reduce((obj: OnyxInputKeyValueMapping, key) => { - const {isCompatible, existingValueType, newValueType, isEmptyArrayCoercion} = utils.checkCompatibilityWithExistingValue( - resultCollection[key], - cachedCollectionForExistingKeys[key], - ); + const existingKeys = keys.filter((key) => persistedKeys.has(key)); - 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 cachedCollectionForExistingKeys = getCachedCollection(collectionKey, existingKeys); - // eslint-disable-next-line no-param-reassign - obj[key] = resultCollection[key]; - return obj; - }, {}) as Record>; + const existingKeyCollection = existingKeys.reduce((obj: OnyxInputKeyValueMapping, key) => { + const {isCompatible, existingValueType, newValueType, isEmptyArrayCoercion} = utils.checkCompatibilityWithExistingValue(resultCollection[key], cachedCollectionForExistingKeys[key]); - const newCollection: Record> = {}; - for (const key of keys) { - if (persistedKeys.has(key)) { - continue; - } - newCollection[key] = resultCollection[key]; - } + 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; + } - // 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 {pairs: keyValuePairsForExistingCollection} = prepareKeyValuePairsForStorage(existingKeyCollection, false, mergeReplaceNullPatches); + // eslint-disable-next-line no-param-reassign + obj[key] = resultCollection[key]; + return obj; + }, {}) as Record>; - // We can safely remove nested null values when using (multi-)set, - // because we will simply overwrite the existing values in storage. - const {pairs: keyValuePairsForNewCollection} = prepareKeyValuePairsForStorage(newCollection, true); + const newCollection: Record> = {}; + for (const key of keys) { + if (persistedKeys.has(key)) { + continue; + } + newCollection[key] = resultCollection[key]; + } - // finalMergedCollection contains all the keys that were merged, without the keys of incompatible updates - const finalMergedCollection = { - ...existingKeyCollection, - ...newCollection, - }; + // 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 {pairs: 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 {pairs: keyValuePairsForNewCollection} = prepareKeyValuePairsForStorage(newCollection, true); + + // finalMergedCollection contains all the keys that were merged, without the keys of incompatible updates + const finalMergedCollection = { + ...existingKeyCollection, + ...newCollection, + }; + + // No pre-warm read is needed before cache.merge(): the whole database is loaded into cache on init + // (see initializeWithDefaultKeyStates) and every write keeps cache and storage in step, so an + // existingKey (one present in cache.getAllKeys()) always has its value cached. + + // Snapshot previous values from the cache for keysChanged's diff, then update cache and notify + // subscribers synchronously BEFORE issuing storage writes. This matches the cache-first / + // storage-second invariant followed by every other Onyx write method (setWithRetry, applyMerge, + // setCollectionWithRetry, partialSetCollection, clear), ensuring subscribers still reflect the + // merged data even if the subsequent storage write fails. + const previousCollection = getCachedCollection(collectionKey, existingKeys); + + cache.merge(finalMergedCollection); + // Skip subscriber notification on retry — already notified on attempt 0. + // Collection-root subscribers re-fire on every keysChanged by contract. + if (!retryAttempt) { + const partialForNotify = keysToRemove.length > 0 ? {...finalMergedCollection, ...Object.fromEntries(keysToRemove.map((key) => [key, undefined]))} : finalMergedCollection; + const previousForNotify = keysToRemove.length > 0 ? {...previousCollection, ...removedPreviousValues} : previousCollection; + if (Object.keys(partialForNotify).length > 0) { + keysChanged(collectionKey, partialForNotify, previousForNotify); + } + } - // Pre-warm cache for cache-miss existingKeys so cache.merge() merges the new delta into - // the real previous storage value. Fast path (all warm) skips the pre-warm to preserve - // promise-chain depth; slow path batches the misses into one Storage.multiGet. - const hasColdExistingKey = existingKeys.some((key) => !cache.hasCacheForKey(key)); - // Swallow pre-warm read failures so a transient Storage.multiGet rejection doesn't - // skip the cache.merge() + keysChanged() below. Subscribers still see the merge even - // when storage reads fail. - const prewarmPromise = hasColdExistingKey - ? multiGet(existingKeys).catch((err) => Logger.logInfo(`mergeCollectionWithPatches pre-warm failed; proceeding with cache-only merge. Error: ${err}`)) - : Promise.resolve(); - return prewarmPromise.then(() => { - // Snapshot previous values from the (now-warm) cache for keysChanged's diff, then update - // cache and notify subscribers synchronously BEFORE issuing storage writes. This matches - // the cache-first / storage-second invariant followed by every other Onyx write method - // (setWithRetry, applyMerge, setCollectionWithRetry, partialSetCollection, clear), - // ensuring subscribers still reflect the merged data even if the subsequent storage - // write fails. - const previousCollection = getCachedCollection(collectionKey, existingKeys); - - cache.merge(finalMergedCollection); - // Skip subscriber notification on retry — already notified on attempt 0. - // Collection-root subscribers re-fire on every keysChanged by contract. - if (!retryAttempt) { - const partialForNotify = keysToRemove.length > 0 ? {...finalMergedCollection, ...Object.fromEntries(keysToRemove.map((key) => [key, undefined]))} : finalMergedCollection; - const previousForNotify = keysToRemove.length > 0 ? {...previousCollection, ...removedPreviousValues} : previousCollection; - if (Object.keys(partialForNotify).length > 0) { - keysChanged(collectionKey, partialForNotify, previousForNotify); - } - } + const promises = []; - const promises = []; + if (removalPromise) { + promises.push(removalPromise); + } - if (removalPromise) { - promises.push(removalPromise); - } + // New keys go through multiSet and existing keys through multiMerge. multiMerge on a + // missing key stores the value just like multiSet across all backends; splitting them lets + // multiSet strip nested nulls (the merge layer keeps them to delete nested storage keys). + // 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)); + } - // New keys go through multiSet and existing keys through multiMerge. multiMerge on a - // missing key stores the value just like multiSet across all backends; splitting them lets - // multiSet strip nested nulls (the merge layer keeps them to delete nested storage keys). - // 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 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)); + } - // 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)); - } + const inFlightKeys = new Set(Object.keys(finalMergedCollection)); - const inFlightKeys = new Set(Object.keys(finalMergedCollection)); - - return Promise.all(promises) - .then(() => StorageCircuitBreaker.recordWriteSuccess()) - .catch((error) => - retryOperation( - error, - mergeCollectionWithPatches, - { - collectionKey, - collection: resultCollection as OnyxMergeCollectionInput, - mergeReplaceNullPatches, - }, - retryAttempt, - inFlightKeys, - ), - ) - .then(() => { - sendActionToDevTools(METHOD.MERGE_COLLECTION, undefined, resultCollection); - }); - }); - }) - .then(() => undefined); + return Promise.all(promises) + .then(() => StorageCircuitBreaker.recordWriteSuccess()) + .catch((error) => + retryOperation( + error, + mergeCollectionWithPatches, + { + collectionKey, + collection: resultCollection as OnyxMergeCollectionInput, + mergeReplaceNullPatches, + }, + retryAttempt, + inFlightKeys, + ), + ) + .then(() => { + sendActionToDevTools(METHOD.MERGE_COLLECTION, undefined, resultCollection); + }); } /** @@ -1841,47 +1659,46 @@ 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 {pairs: keyValuePairs, keysToRemove: removalCandidates} = prepareKeyValuePairsForStorage(mutableCollection, true); - // Removals of keys that are neither cached nor persisted are no-ops and skipped. - const keysToRemove = removalCandidates.filter((key) => cache.get(key) !== undefined || persistedKeys.has(key)); - // Snapshot before cache mutations so keysChanged() can diff removed members. - const previousCollection = getCachedCollection(collectionKey, existingKeys); - - for (const [key, value] of keyValuePairs) cache.set(key, value); - for (const key of keysToRemove) cache.drop(key); - - // Skip subscriber notification on retry — already notified on attempt 0. - // Collection-root subscribers re-fire on every keysChanged by contract. - if (!retryAttempt) { - // Removed members are notified as undefined, matching mergeCollection/multiSet. - const partialForNotify = Object.fromEntries(Object.entries(mutableCollection).map(([key, value]) => [key, value ?? undefined])); - keysChanged(collectionKey, partialForNotify, previousCollection); - } + const persistedKeys = getAllKeys(); + const mutableCollection: OnyxInputKeyValueMapping = {...resultCollection}; + const existingKeys = resultCollectionKeys.filter((key) => persistedKeys.has(key)); + const {pairs: keyValuePairs, keysToRemove: removalCandidates} = prepareKeyValuePairsForStorage(mutableCollection, true); + // Removals of keys that are neither cached nor persisted are no-ops and skipped. + const keysToRemove = removalCandidates.filter((key) => cache.get(key) !== undefined || persistedKeys.has(key)); + // Snapshot before cache mutations so keysChanged() can diff removed members. + const previousCollection = getCachedCollection(collectionKey, existingKeys); - if (OnyxKeys.isRamOnlyKey(collectionKey)) { - sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); - return; - } + for (const [key, value] of keyValuePairs) cache.set(key, value); + for (const key of keysToRemove) cache.drop(key); - const inFlightKeys = new Set(keyValuePairs.map(([key]) => key)); + // Skip subscriber notification on retry — already notified on attempt 0. + // Collection-root subscribers re-fire on every keysChanged by contract. + if (!retryAttempt) { + // Removed members are notified as undefined, matching mergeCollection/multiSet. + const partialForNotify = Object.fromEntries(Object.entries(mutableCollection).map(([key, value]) => [key, value ?? undefined])); + keysChanged(collectionKey, partialForNotify, previousCollection); + } - // One batched removal = one cross-tab sync event instead of one per key. A failed removal is - // logged, not retried — keysToRemove cannot be re-derived after the cache update. - const storagePromises = [Storage.multiSet(keyValuePairs)]; - if (keysToRemove.length > 0) { - storagePromises.push(Storage.removeItems(keysToRemove).catch((error) => Logger.logAlert(`setCollection failed to remove keys from storage. Error: ${error}`))); - } + if (OnyxKeys.isRamOnlyKey(collectionKey)) { + sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); + return Promise.resolve(); + } - return Promise.all(storagePromises) - .then(() => StorageCircuitBreaker.recordWriteSuccess()) - .catch((error) => retryOperation(error, partialSetCollection, {collectionKey, collection}, retryAttempt, inFlightKeys)) - .then(() => { - sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); - }); - }); + const inFlightKeys = new Set(keyValuePairs.map(([key]) => key)); + + // One batched removal = one cross-tab sync event instead of one per key. A failed removal is + // logged, not retried — keysToRemove cannot be re-derived after the cache update. + const storagePromises = [Storage.multiSet(keyValuePairs)]; + if (keysToRemove.length > 0) { + storagePromises.push(Storage.removeItems(keysToRemove).catch((error) => Logger.logAlert(`setCollection failed to remove keys from storage. Error: ${error}`))); + } + + return Promise.all(storagePromises) + .then(() => StorageCircuitBreaker.recordWriteSuccess()) + .catch((error) => retryOperation(error, partialSetCollection, {collectionKey, collection}, retryAttempt, inFlightKeys)) + .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 87c7dd7cb..5c9ebce8f 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); }, @@ -544,7 +544,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])); }, @@ -553,7 +553,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 ce5488567..6e98c97aa 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. */ @@ -197,54 +179,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 664c96f28..f6b3cb0eb 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'; @@ -66,7 +65,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}); @@ -84,7 +83,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}); @@ -116,10 +115,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}); @@ -145,7 +141,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}); @@ -175,7 +171,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}); @@ -202,7 +198,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}); @@ -259,7 +255,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}); @@ -358,8 +354,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}); @@ -385,8 +381,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 84ea6c40d..7a08be9fe 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 a36c79ec2..9e5ed5bce 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -68,17 +68,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); })); @@ -2862,6 +2860,51 @@ describe('Onyx', () => { }); }); + describe('get', () => { + const memberOne = `${ONYX_KEYS.COLLECTION.TEST_KEY}1`; + const memberTwo = `${ONYX_KEYS.COLLECTION.TEST_KEY}2`; + + it('reads a value out of the cache synchronously', async () => { + await Onyx.merge(ONYX_KEYS.TEST_KEY, {id: 1, title: 'One'}); + + expect(Onyx.get(ONYX_KEYS.TEST_KEY)).toEqual({id: 1, title: 'One'}); + expect(Onyx.get(ONYX_KEYS.TEST_KEY)).toEqual(OnyxUtils.get(ONYX_KEYS.TEST_KEY)); + }); + + it('returns undefined for a key that has no value', () => { + expect(Onyx.get(ONYX_KEYS.TEST_KEY)).toBeUndefined(); + expect(Onyx.get(memberOne)).toBeUndefined(); + }); + + it('reads every member when given a collection key', async () => { + await Onyx.merge(memberOne, {id: 1, title: 'One'}); + await Onyx.merge(memberTwo, {id: 2, title: 'Two'}); + + expect(Onyx.get(ONYX_KEYS.COLLECTION.TEST_KEY)).toEqual({ + [memberOne]: {id: 1, title: 'One'}, + [memberTwo]: {id: 2, title: 'Two'}, + }); + }); + + it('agrees with a whole-collection subscriber', async () => { + await Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + [memberOne]: {id: 1, title: 'One'}, + [memberTwo]: {id: 2, title: 'Two'}, + } as GenericCollection); + + let subscribed: OnyxCollection; + connection = Onyx.connectWithoutView({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + callback: (collection) => { + subscribed = collection; + }, + }); + await waitForPromisesToResolve(); + + expect(Onyx.get(ONYX_KEYS.COLLECTION.TEST_KEY)).toEqual(subscribed); + }); + }); + describe('skippable collection member ids', () => { it('should skip the collection member id value when using Onyx.set()', async () => { let testKeyValue: unknown; @@ -3374,7 +3417,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); @@ -3681,7 +3724,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/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts index 0a20a7d21..3c955bec6 100644 --- a/tests/unit/onyxUtilsTest.ts +++ b/tests/unit/onyxUtilsTest.ts @@ -1187,7 +1187,7 @@ describe('OnyxUtils', () => { }); }); - describe('mergeCollection pre-warm', () => { + describe('mergeCollection cache-only reads', () => { // retryOperation tests above replace StorageMock methods without restoring them, leaving // rejecting mocks behind. Capture pristine refs at file-load time and restore in beforeEach // so our Onyx.set seeding actually reaches the in-memory storage provider. @@ -1205,17 +1205,7 @@ describe('OnyxUtils', () => { StorageMock.multiMerge = pristineMultiMerge; }); - // Make a key "cold" — value evicted from cache but still tracked as persisted. OnyxCache.drop - // also removes the key from `storageKeys`, so we re-register it afterwards to reliably hit - // the cold-but-persisted state regardless of getAllKeys()'s fallback path. - const evictFromCache = (...keys: string[]) => { - for (const key of keys) { - OnyxCache.drop(key); - OnyxCache.addKey(key); - } - }; - - it('fast path: skips storage reads entirely when every existing key is warm in cache', async () => { + it('skips storage reads entirely — the previous value comes from the cache', async () => { const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY; const existingKey1 = `${collectionKey}1`; const existingKey2 = `${collectionKey}2`; @@ -1232,8 +1222,8 @@ describe('OnyxUtils', () => { [existingKey2]: {value: 'merged-2'}, } as GenericCollection); - // With every existingKey warm, the diff swaps Promise.all(get) for Promise.resolve(), - // so no storage reads should happen during the pre-warm. + // Reads are synchronous and cache-only now, so mergeCollection never touches storage + // to resolve the previous value — only the subsequent writes hit storage. expect(multiGetSpy).not.toHaveBeenCalled(); expect(getItemSpy).not.toHaveBeenCalled(); @@ -1243,60 +1233,7 @@ describe('OnyxUtils', () => { expect(cached?.[existingKey2]).toEqual({value: 'merged-2'}); }); - it('slow path: batches cold existing keys into a single Storage.multiGet, with no individual getItem calls', async () => { - const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY; - const coldKey1 = `${collectionKey}1`; - const coldKey2 = `${collectionKey}2`; - const warmKey = `${collectionKey}3`; - - // Seed all three in storage, then evict two from cache so they are cold-but-persisted. - await Onyx.set(coldKey1, {value: 'persisted-1'}); - await Onyx.set(coldKey2, {value: 'persisted-2'}); - await Onyx.set(warmKey, {value: 'persisted-3'}); - evictFromCache(coldKey1, coldKey2); - - // Reset spies AFTER seeding so we only count calls made during mergeCollection itself. - const multiGetSpy = jest.spyOn(StorageMock, 'multiGet').mockClear(); - const getItemSpy = jest.spyOn(StorageMock, 'getItem').mockClear(); - - await Onyx.mergeCollection(collectionKey, { - [coldKey1]: {value: 'merged-1'}, - [coldKey2]: {value: 'merged-2'}, - [warmKey]: {value: 'merged-3'}, - } as GenericCollection); - - // OnyxUtils.multiGet filters to cache-missing keys before issuing Storage.multiGet, so we - // expect exactly one batched read containing only the cold keys (the warm key is skipped). - expect(multiGetSpy).toHaveBeenCalledTimes(1); - const requestedKeys = multiGetSpy.mock.calls[0][0] as string[]; - expect(requestedKeys.sort()).toEqual([coldKey1, coldKey2].sort()); - - // No individual Storage.getItem calls during pre-warm. Old code path would have fired one - // get() per existing key, each potentially landing in Storage.getItem on cache miss. - expect(getItemSpy).not.toHaveBeenCalled(); - }); - - it('slow path: cold-cache merge layers the new delta on top of existing storage data (no field drops)', async () => { - const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY; - const coldKey = `${collectionKey}1`; - - // Seed an object with multiple fields in storage, then evict from cache so the merge base - // must come from a storage read — not from `undefined`. - await Onyx.set(coldKey, {a: 1, b: 2}); - evictFromCache(coldKey); - - await Onyx.mergeCollection(collectionKey, { - [coldKey]: {c: 3}, - } as GenericCollection); - - // If the pre-warm did NOT populate the cache from storage, fastMerge would treat the - // previous value as undefined and the result would drop {a:1, b:2}. With the pre-warm - // running multiGet on the cold key, the merge layers {c:3} on top of {a:1, b:2}. - const cached = OnyxCache.getCollectionData(collectionKey); - expect(cached?.[coldKey]).toEqual({a: 1, b: 2, c: 3}); - }); - - it('warm cache: subscriber receives a single merged broadcast for an Onyx.update batch (no transient undefined)', async () => { + it('subscriber receives a single merged broadcast for an Onyx.update batch (no transient undefined)', async () => { const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY; const existingKey = `${collectionKey}1`; @@ -1320,88 +1257,44 @@ describe('OnyxUtils', () => { }, ]); - // The fast path resolves the pre-warm synchronously (Promise.resolve()), preserving the - // original promise-chain depth. The Onyx.update batch must therefore broadcast exactly + // The cache read resolves synchronously, so the Onyx.update batch broadcasts exactly // one merged value — not undefined first and the merged value on a later microtask. const broadcasts = collectionCallback.mock.calls.map((c) => c[0]); expect(broadcasts).toHaveLength(1); expect(broadcasts[0]?.[existingKey]).toEqual({value: 'merged'}); }); + }); - it('equivalence: warm-path and cold-path produce the same final cache state for the same merge', async () => { - const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY; - const memberKey = `${collectionKey}1`; - const delta = {value: 'after'} as const; - - // Warm-path run. - await Onyx.set(memberKey, {value: 'before', extra: 'kept'}); - await Onyx.mergeCollection(collectionKey, { - [memberKey]: delta, - } as GenericCollection); - const warmResult = OnyxCache.getCollectionData(collectionKey)?.[memberKey]; + describe('tryGetCachedValue', () => { + const memberOne = `${ONYXKEYS.COLLECTION.TEST_KEY}1`; + const memberTwo = `${ONYXKEYS.COLLECTION.TEST_KEY}2`; - // Reset and replay with a cold cache before the merge. - await Onyx.clear(); - await Onyx.set(memberKey, {value: 'before', extra: 'kept'}); - evictFromCache(memberKey); - await Onyx.mergeCollection(collectionKey, { - [memberKey]: delta, - } as GenericCollection); - const coldResult = OnyxCache.getCollectionData(collectionKey)?.[memberKey]; + it('accepts either shape of key, unlike get', async () => { + await Onyx.merge(memberOne, {id: 1, title: 'One'}); + await Onyx.merge(memberTwo, {id: 2, title: 'Two'}); - expect(warmResult).toEqual(coldResult); - expect(coldResult).toEqual({value: 'after', extra: 'kept'}); + expect(OnyxUtils.tryGetCachedValue(memberOne)).toEqual({id: 1, title: 'One'}); + expect(OnyxUtils.tryGetCachedValue(ONYXKEYS.COLLECTION.TEST_KEY)).toEqual({ + [memberOne]: {id: 1, title: 'One'}, + [memberTwo]: {id: 2, title: 'Two'}, + }); + expect(OnyxUtils.get(ONYXKEYS.COLLECTION.TEST_KEY)).toBeUndefined(); }); - it('preserves cache-first invariant when Storage.multiGet rejects on the slow path', async () => { - // A Storage.multiGet rejection during pre-warm must not skip the cache.merge() + - // keysChanged() that follow. Without the .catch() at the pre-warm call site, - // subscribers would miss the merge and Onyx.mergeCollection would reject. - const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY; - const coldMemberKey = `${collectionKey}1`; - const newMemberKey = `${collectionKey}2`; - - // Seed an existing member, then evict it from cache so it's "tracked but unloaded" — - // the slow path will try to multiGet it. - await Onyx.set(coldMemberKey, {value: 'persisted'}); - evictFromCache(coldMemberKey); + it('answers undefined for an empty collection while the store holds no key at all', () => { + expect(OnyxCache.getAllKeys().size).toBe(0); - // Connect and flush the subscriber's initial load BEFORE installing the rejecting mock — - // otherwise the connect's own multiGet (no .catch) consumes the mockRejectedValueOnce and - // leaks an unhandled rejection instead of exercising the merge pre-warm path. - const collectionCallback = jest.fn(); - Onyx.connect({ - key: collectionKey, - callback: collectionCallback, - }); - await waitForPromisesToResolve(); - collectionCallback.mockClear(); + // An unloaded store reads as "cannot tell yet" here and as empty through getCachedCollection. + expect(OnyxUtils.tryGetCachedValue(ONYXKEYS.COLLECTION.TEST_KEY)).toBeUndefined(); + expect(OnyxUtils.getCachedCollection(ONYXKEYS.COLLECTION.TEST_KEY)).toEqual({}); + }); - // The subscriber's connect re-populated cache, so re-evict to force the merge into - // the slow (cold-key) path. Then reject the next Storage.multiGet so the pre-warm - // read fails. - evictFromCache(coldMemberKey); - const transientError = new Error('Transient IndexedDB read error'); - StorageMock.multiGet = jest.fn(pristineMultiGet).mockRejectedValueOnce(transientError); - - // Outer promise must resolve, not reject, even when the pre-warm read fails. - let outerRejected: unknown = null; - const result = await Onyx.mergeCollection(collectionKey, { - [coldMemberKey]: {merged: true}, - [newMemberKey]: {value: 'new'}, - } as GenericCollection).catch((e: unknown) => { - outerRejected = e; - }); - expect(outerRejected).toBeNull(); - expect(result).toBeUndefined(); + it('answers an empty object for that same collection once any unrelated key exists', async () => { + await Onyx.merge(ONYXKEYS.TEST_KEY, {title: 'unrelated'}); - // cache.merge() + keysChanged() must still fire so subscribers see the merge. Use - // toMatchObject because a concurrent read may have re-populated the persisted value; - // what matters is that the new {merged: true} delta is applied on top. - expect(collectionCallback).toHaveBeenCalled(); - const lastBroadcast = collectionCallback.mock.calls.at(-1)?.[0] as Record | undefined; - expect(lastBroadcast?.[coldMemberKey]).toMatchObject({merged: true}); - expect(lastBroadcast?.[newMemberKey]).toEqual({value: 'new'}); + // Same empty collection, different answer, decided by an unrelated key: a caller has to treat undefined and {} alike. + expect(OnyxUtils.tryGetCachedValue(ONYXKEYS.COLLECTION.TEST_KEY)).toEqual({}); + expect(OnyxUtils.getCachedCollection(ONYXKEYS.COLLECTION.TEST_KEY)).toEqual({}); }); }); @@ -1439,27 +1332,6 @@ describe('OnyxUtils', () => { expect(getItemSpy).not.toHaveBeenCalled(); expect(result.get(falsyKey)).toBe(0); }); - - it('prefers cache when a concurrent write lands during the storage read', async () => { - // Concurrent write during multiGet's storage read must not be overwritten by the - // stale snapshot via cache.merge. - const key = `${ONYXKEYS.COLLECTION.TEST_KEY}race`; - - OnyxCache.drop(key); - OnyxCache.addKey(key); - - // Set cache inside the mock so it lands before Storage.multiGet's promise resolves — - // multiGet's .then() then sees a populated cache and skips writing the stale value. - StorageMock.multiGet = jest.fn().mockImplementation(() => { - OnyxCache.set(key, {fresh: 'data'}); - return Promise.resolve([[key, {stale: 'a', alsoStale: 'b'}]]); - }); - - const result = await OnyxUtils.multiGet([key]); - - expect(OnyxCache.get(key)).toEqual({fresh: 'data'}); - expect(result.get(key)).toEqual({fresh: 'data'}); - }); }); describe('storage eviction', () => { diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts index 837dcbdee..e1c43f6b0 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'); @@ -780,39 +762,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'); @@ -869,6 +824,52 @@ describe('useOnyx', () => { expect(result.current[0]).toBeUndefined(); expect(result.current[1].status).toEqual('loaded'); }); + + it('should return undefined and loaded state when switching from a valid key to a skippable one', async () => { + await Onyx.set(`${ONYXKEYS.COLLECTION.TEST_KEY}1`, {id: '1'}); + // Seed a value directly in storage for the skippable key. + // If the subscription is NOT skipped, Onyx would load this and return it. + // Asserting undefined below proves the subscription was actually suppressed. + await StorageMock.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`, {id: 'skippable'}); + + const {result, rerender} = renderHook((key: string) => useOnyx(key), {initialProps: `${ONYXKEYS.COLLECTION.TEST_KEY}1` as string}); + + await act(async () => waitForPromisesToResolve()); + + expect(result.current[0]).toEqual({id: '1'}); + expect(result.current[1].status).toEqual('loaded'); + + await act(async () => { + rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}skippable-id`); + }); + + await act(async () => waitForPromisesToResolve()); + + expect(result.current[0]).toBeUndefined(); + expect(result.current[1].status).toEqual('loaded'); + }); + + 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 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}); + + await act(async () => waitForPromisesToResolve()); + + expect(result.current[0]).toBeUndefined(); + expect(result.current[1].status).toEqual('loaded'); + + // Switch to a valid key — value is available immediately from cache + rerender(`${ONYXKEYS.COLLECTION.TEST_KEY}1`); + + await act(async () => waitForPromisesToResolve()); + + expect(result.current[0]).toEqual({id: '1'}); + expect(result.current[1].status).toEqual('loaded'); + }); }); describe('RAM-only keys', () => { @@ -1021,7 +1022,7 @@ describe('useOnyx', () => { expect(renderCount).toBe(2); }); - it('should render exactly twice when the key value is only present in storage', async () => { + it('should render exactly twice and not surface a value that is only present in storage (cache is the source of truth)', async () => { await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'storage_value'); let renderCount = 0; @@ -1032,7 +1033,9 @@ describe('useOnyx', () => { await act(async () => waitForPromisesToResolve()); - expect(result.current[0]).toEqual('storage_value'); + // Onyx reads are synchronous and cache-only now, so a value written directly to storage + // (bypassing the cache) is intentionally invisible to subscribers. + expect(result.current[0]).toBeUndefined(); expect(result.current[1].status).toEqual('loaded'); expect(renderCount).toBe(2); });