diff --git a/API-INTERNAL.md b/API-INTERNAL.md index 59208f80f..f5e6f119b 100644 --- a/API-INTERNAL.md +++ b/API-INTERNAL.md @@ -79,17 +79,6 @@ run out of storage the least recently accessed key can be removed.

getCollectionDataAndSendAsObject()

Gets the data for a given an array of matching keys, combines them into an object, and sends the result back to the subscriber.

-
prepareSubscriberUpdate(callback)
-

Delays promise resolution until the next macrotask to prevent race condition if the key subscription is in progress.

-
-
scheduleSubscriberUpdate()
-

Schedules an update that will be appended to the macro task queue (so it doesn't update the subscribers immediately).

-
-
scheduleNotifyCollectionSubscribers()
-

This method is similar to scheduleSubscriberUpdate but it is built for working specifically with collections -so that keysChanged() is triggered for the collection and not keyChanged(). If this was not done, then the -subscriber callbacks receive the data in a different format than they normally expect and it breaks code.

-
remove()

Remove a key from Onyx and update the subscribers

@@ -350,35 +339,6 @@ run out of storage the least recently accessed key can be removed. ## getCollectionDataAndSendAsObject() Gets the data for a given an array of matching keys, combines them into an object, and sends the result back to the subscriber. -**Kind**: global function - - -## prepareSubscriberUpdate(callback) -Delays promise resolution until the next macrotask to prevent race condition if the key subscription is in progress. - -**Kind**: global function - -| Param | Description | -| --- | --- | -| callback | The keyChanged/keysChanged callback | - - - -## scheduleSubscriberUpdate() -Schedules an update that will be appended to the macro task queue (so it doesn't update the subscribers immediately). - -**Kind**: global function -**Example** -```js -scheduleSubscriberUpdate(key, value, subscriber => subscriber.initWithStoredValues === false) -``` - - -## scheduleNotifyCollectionSubscribers() -This method is similar to scheduleSubscriberUpdate but it is built for working specifically with collections -so that keysChanged() is triggered for the collection and not keyChanged(). If this was not done, then the -subscriber callbacks receive the data in a different format than they normally expect and it breaks code. - **Kind**: global function diff --git a/lib/Onyx.ts b/lib/Onyx.ts index b7a9b7976..77ce04852 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -262,9 +262,8 @@ function merge(key: TKey, changes: OnyxMergeInput): return Promise.resolve(); } - return OnyxMerge.applyMerge(key, existingValue, validChanges).then(({mergedValue, updatePromise}) => { + return OnyxMerge.applyMerge(key, existingValue, validChanges).then(({mergedValue}) => { OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MERGE, key, changes, mergedValue); - return updatePromise; }); } catch (error) { Logger.logAlert(`An error occurred while applying merge for key: ${key}, Error: ${error}`); @@ -376,16 +375,6 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { keysToBeClearedFromStorage.push(key); } - const updatePromises: Array> = []; - - // Notify the subscribers for each key/value group so they can receive the new values - for (const [key, value] of Object.entries(keyValuesToResetIndividually)) { - updatePromises.push(OnyxUtils.scheduleSubscriberUpdate(key, value)); - } - for (const [key, value] of Object.entries(keyValuesToResetAsCollection)) { - updatePromises.push(OnyxUtils.scheduleNotifyCollectionSubscribers(key, value.newValues, value.oldValues)); - } - // Exclude RAM-only keys to prevent them from being saved to storage const defaultKeyValuePairs = Object.entries( Object.keys(defaultKeyStates) @@ -404,7 +393,14 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { .then(() => Storage.multiSet(defaultKeyValuePairs)) .then(() => { DevTools.clearState(keysToPreserve); - return Promise.all(updatePromises); + + // 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); diff --git a/lib/OnyxMerge/index.native.ts b/lib/OnyxMerge/index.native.ts index 558532a7d..c01c1524a 100644 --- a/lib/OnyxMerge/index.native.ts +++ b/lib/OnyxMerge/index.native.ts @@ -27,21 +27,20 @@ const applyMerge: ApplyMerge = , hasChanged); + OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue, hasChanged); const shouldSkipStorageOperations = !hasChanged || OnyxKeys.isRamOnlyKey(key); // If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. // If the key is marked as RAM-only, it should not be saved nor updated in the storage. if (shouldSkipStorageOperations) { - return Promise.resolve({mergedValue, updatePromise}); + return Promise.resolve({mergedValue}); } // For native platforms we use `mergeItem` that will take advantage of JSON_PATCH and JSON_REPLACE SQL operations to // merge the object in a performant way. return Storage.mergeItem(key, batchedChanges as OnyxValue, replaceNullPatches).then(() => ({ mergedValue, - updatePromise, })); }; diff --git a/lib/OnyxMerge/index.ts b/lib/OnyxMerge/index.ts index ddf5525c8..dd6c86b5e 100644 --- a/lib/OnyxMerge/index.ts +++ b/lib/OnyxMerge/index.ts @@ -19,20 +19,19 @@ const applyMerge: ApplyMerge = , hasChanged); + OnyxUtils.broadcastUpdate(key, mergedValue as OnyxValue, hasChanged); const shouldSkipStorageOperations = !hasChanged || OnyxKeys.isRamOnlyKey(key); // If the value has not changed, calling Storage.setItem() would be redundant and a waste of performance, so return early instead. // If the key is marked as RAM-only, it should not be saved nor updated in the storage. if (shouldSkipStorageOperations) { - return Promise.resolve({mergedValue, updatePromise}); + return Promise.resolve({mergedValue}); } // For web platforms we use `setItem` since the object was already merged with its changes before. return Storage.setItem(key, mergedValue as OnyxValue).then(() => ({ mergedValue, - updatePromise, })); }; diff --git a/lib/OnyxMerge/types.ts b/lib/OnyxMerge/types.ts index c59b7892a..e53d8ff32 100644 --- a/lib/OnyxMerge/types.ts +++ b/lib/OnyxMerge/types.ts @@ -2,7 +2,6 @@ import type {OnyxInput, OnyxKey} from '../types'; type ApplyMergeResult = { mergedValue: TValue; - updatePromise: Promise; }; type ApplyMerge = | undefined, TChange extends OnyxInput | null>( diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 9851e4ff2..bbcd65ce4 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -1,4 +1,4 @@ -import {deepEqual} from 'fast-equals'; +import {deepEqual, shallowEqual} from 'fast-equals'; import type {ValueOf} from 'type-fest'; import _ from 'underscore'; import DevTools from './DevTools'; @@ -72,9 +72,6 @@ type OnyxMethod = ValueOf; let mergeQueue: Record>> = {}; let mergeQueuePromise: Record> = {}; -// Used to schedule subscriber update to the macro task queue -let nextMacrotaskPromise: Promise | null = null; - // Holds a mapping of all the React components that want their state subscribed to a store key let callbackToStateMapping: Record> = {}; @@ -85,7 +82,7 @@ let onyxKeyToSubscriptionIDs = new Map(); let defaultKeyStates: Record> = {}; // Used for comparison with a new update to avoid invoking the Onyx.connect callback with the same data. -let lastConnectionCallbackData = new Map>(); +let lastConnectionCallbackData = new Map; matchedKey: OnyxKey | undefined}>(); let snapshotKey: OnyxKey | null = null; @@ -579,41 +576,47 @@ function keysChanged( // Regular Onyx.connect() subscriber found. if (typeof subscriber.callback === 'function') { - // If they are subscribed to the collection key and using waitForCollectionCallback then we'll - // send the whole cached collection. - if (isSubscribedToCollectionKey) { - if (subscriber.waitForCollectionCallback) { - subscriber.callback(cachedCollection, subscriber.key, partialCollection); - continue; - } + try { + // If they are subscribed to the collection key and using waitForCollectionCallback then we'll + // send the whole cached collection. + if (isSubscribedToCollectionKey) { + lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); - // If they are not using waitForCollectionCallback then we notify the subscriber with - // the new merged data but only for any keys in the partial collection. - const dataKeys = Object.keys(partialCollection ?? {}); - for (const dataKey of dataKeys) { - if (deepEqual(cachedCollection[dataKey], previousCollection[dataKey])) { + if (subscriber.waitForCollectionCallback) { + subscriber.callback(cachedCollection, subscriber.key, partialCollection); continue; } - subscriber.callback(cachedCollection[dataKey], dataKey); + // If they are not using waitForCollectionCallback then we notify the subscriber with + // the new merged data but only for any keys in the partial collection. + const dataKeys = Object.keys(partialCollection ?? {}); + for (const dataKey of dataKeys) { + if (deepEqual(cachedCollection[dataKey], previousCollection[dataKey])) { + continue; + } + + subscriber.callback(cachedCollection[dataKey], dataKey); + } + continue; } - continue; - } - // And if the subscriber is specifically only tracking a particular collection member key then we will - // notify them with the cached data for that key only. - if (isSubscribedToCollectionMemberKey) { - if (deepEqual(cachedCollection[subscriber.key], previousCollection[subscriber.key])) { + // And if the subscriber is specifically only tracking a particular collection member key then we will + // notify them with the cached data for that key only. + if (isSubscribedToCollectionMemberKey) { + if (deepEqual(cachedCollection[subscriber.key], previousCollection[subscriber.key])) { + continue; + } + + const subscriberCallback = subscriber.callback as DefaultConnectCallback; + subscriberCallback(cachedCollection[subscriber.key], subscriber.key as TKey); + lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection[subscriber.key], matchedKey: subscriber.key}); continue; } - const subscriberCallback = subscriber.callback as DefaultConnectCallback; - subscriberCallback(cachedCollection[subscriber.key], subscriber.key as TKey); - lastConnectionCallbackData.set(subscriber.subscriptionID, cachedCollection[subscriber.key]); continue; + } catch (error) { + Logger.logAlert(`[OnyxUtils.keysChanged] Subscriber callback threw an error for key '${collectionKey}': ${error}`); } - - continue; } } } @@ -663,32 +666,40 @@ function keyChanged( // Subscriber is a regular call to connect() and provided a callback if (typeof subscriber.callback === 'function') { - if (lastConnectionCallbackData.has(subscriber.subscriptionID) && lastConnectionCallbackData.get(subscriber.subscriptionID) === value) { - continue; - } - - if (OnyxKeys.isCollectionKey(subscriber.key) && subscriber.waitForCollectionCallback) { - // Skip individual key changes for collection callbacks during collection updates - // to prevent duplicate callbacks - the collection update will handle this properly - if (isProcessingCollectionUpdate) { + try { + const lastData = lastConnectionCallbackData.get(subscriber.subscriptionID); + if (lastData && lastData.matchedKey === key && lastData.value === value) { continue; } - let cachedCollection = cachedCollections[subscriber.key]; - if (!cachedCollection) { - cachedCollection = getCachedCollection(subscriber.key); - cachedCollections[subscriber.key] = cachedCollection; + if (OnyxKeys.isCollectionKey(subscriber.key) && subscriber.waitForCollectionCallback) { + // Skip individual key changes for collection callbacks during collection updates + // to prevent duplicate callbacks - the collection update will handle this properly + if (isProcessingCollectionUpdate) { + continue; + } + let cachedCollection = cachedCollections[subscriber.key]; + + if (!cachedCollection) { + cachedCollection = getCachedCollection(subscriber.key); + cachedCollections[subscriber.key] = cachedCollection; + } + + cachedCollection[key] = value; + lastConnectionCallbackData.set(subscriber.subscriptionID, {value: cachedCollection, matchedKey: subscriber.key}); + subscriber.callback(cachedCollection, subscriber.key, {[key]: value}); + continue; } - cachedCollection[key] = value; - subscriber.callback(cachedCollection, subscriber.key, {[key]: value}); + const subscriberCallback = subscriber.callback as DefaultConnectCallback; + subscriberCallback(value, key); + + lastConnectionCallbackData.set(subscriber.subscriptionID, {value, matchedKey: key}); continue; + } catch (error) { + Logger.logAlert(`[OnyxUtils.keyChanged] Subscriber callback threw an error for key '${key}': ${error}`); } - const subscriberCallback = subscriber.callback as DefaultConnectCallback; - subscriberCallback(value, key); - - lastConnectionCallbackData.set(subscriber.subscriptionID, value); continue; } @@ -699,24 +710,36 @@ function keyChanged( /** * Sends the data obtained from the keys to the connection. */ -function sendDataToConnection(mapping: CallbackToStateMapping, value: OnyxValue | null, matchedKey: TKey | undefined): void { +function sendDataToConnection(mapping: CallbackToStateMapping, matchedKey: TKey | undefined): void { // If the mapping no longer exists then we should not send any data. // This means our subscriber was disconnected. if (!callbackToStateMapping[mapping.subscriptionID]) { return; } + // Always read the latest value from cache to avoid stale or duplicate data. + // For collection subscribers with waitForCollectionCallback, read the full collection. + // For individual key subscribers, read just that key's value. + let value: OnyxValue | undefined; + if (OnyxKeys.isCollectionKey(mapping.key) && mapping.waitForCollectionCallback) { + const collection = getCachedCollection(mapping.key); + value = Object.keys(collection).length > 0 ? (collection as OnyxValue) : undefined; + } else { + value = cache.get(matchedKey ?? mapping.key) as OnyxValue; + } + // For regular callbacks, we never want to pass null values, but always just undefined if a value is not set in cache or storage. - const valueToPass = value === null ? undefined : value; - const lastValue = lastConnectionCallbackData.get(mapping.subscriptionID); - lastConnectionCallbackData.get(mapping.subscriptionID); + value = value === null ? undefined : value; + const lastData = lastConnectionCallbackData.get(mapping.subscriptionID); - // If the value has not changed we do not need to trigger the callback - if (lastConnectionCallbackData.has(mapping.subscriptionID) && valueToPass === lastValue) { + // If the value has not changed for the same key we do not need to trigger the callback. + // We compare matchedKey to avoid suppressing callbacks for different collection members + // that happen to have shallow-equal values (e.g. during hydration racing with set()). + if (lastData && lastData.matchedKey === matchedKey && shallowEqual(lastData.value, value)) { return; } - (mapping as DefaultConnectOptions).callback?.(valueToPass, matchedKey as TKey); + (mapping as DefaultConnectOptions).callback?.(value, matchedKey as TKey); } /** @@ -739,63 +762,17 @@ function addKeyToRecentlyAccessedIfNeeded(key: TKey): void * 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((dataMap) => { - const data = Object.fromEntries(dataMap.entries()) as OnyxValue; - sendDataToConnection(mapping, data, mapping.key); + multiGet(matchingKeys).then(() => { + sendDataToConnection(mapping, mapping.key); }); } -/** - * Delays promise resolution until the next macrotask to prevent race condition if the key subscription is in progress. - * - * @param callback The keyChanged/keysChanged callback - * */ -function prepareSubscriberUpdate(callback: () => void): Promise { - if (!nextMacrotaskPromise) { - nextMacrotaskPromise = new Promise((resolve) => { - setTimeout(() => { - nextMacrotaskPromise = null; - resolve(); - }, 0); - }); - } - return Promise.all([nextMacrotaskPromise, Promise.resolve().then(callback)]).then(); -} - -/** - * Schedules an update that will be appended to the macro task queue (so it doesn't update the subscribers immediately). - * - * @example - * scheduleSubscriberUpdate(key, value, subscriber => subscriber.initWithStoredValues === false) - */ -function scheduleSubscriberUpdate( - key: TKey, - value: OnyxValue, - canUpdateSubscriber: (subscriber?: CallbackToStateMapping) => boolean = () => true, - isProcessingCollectionUpdate = false, -): Promise { - return prepareSubscriberUpdate(() => keyChanged(key, value, canUpdateSubscriber, isProcessingCollectionUpdate)); -} - -/** - * This method is similar to scheduleSubscriberUpdate but it is built for working specifically with collections - * so that keysChanged() is triggered for the collection and not keyChanged(). If this was not done, then the - * subscriber callbacks receive the data in a different format than they normally expect and it breaks code. - */ -function scheduleNotifyCollectionSubscribers( - key: TKey, - value: OnyxCollection, - previousValue?: OnyxCollection, -): Promise { - return prepareSubscriberUpdate(() => keysChanged(key, value, previousValue)); -} - /** * Remove a key from Onyx and update the subscribers */ function remove(key: TKey, isProcessingCollectionUpdate?: boolean): Promise { cache.drop(key); - scheduleSubscriberUpdate(key, undefined as OnyxValue, undefined, isProcessingCollectionUpdate); + keyChanged(key, undefined as OnyxValue, undefined, isProcessingCollectionUpdate); if (OnyxKeys.isRamOnlyKey(key)) { return Promise.resolve(); @@ -866,7 +843,7 @@ function retryOperation(error: Error, on /** * Notifies subscribers and writes current value to cache */ -function broadcastUpdate(key: TKey, value: OnyxValue, hasChanged?: boolean): Promise { +function broadcastUpdate(key: TKey, value: OnyxValue, hasChanged?: boolean): void { // Update subscribers if the cached value has changed, or when the subscriber specifically requires // all updates regardless of value changes (indicated by initWithStoredValues set to false). if (hasChanged) { @@ -875,7 +852,7 @@ function broadcastUpdate(key: TKey, value: OnyxValue cache.addToAccessedKeys(key); } - return scheduleSubscriberUpdate(key, value, (subscriber) => hasChanged || subscriber?.initWithStoredValues === false).then(() => undefined); + keyChanged(key, value, (subscriber) => hasChanged || subscriber?.initWithStoredValues === false); } function hasPendingMergeForKey(key: OnyxKey): boolean { @@ -1149,7 +1126,7 @@ function subscribeToKey(connectOptions: ConnectOptions(connectOptions: ConnectOptions { - for (const [key, val] of values.entries()) { - sendDataToConnection(mapping, val as OnyxValue, key as TKey); + multiGet(matchingKeys).then(() => { + 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((val) => sendDataToConnection(mapping, val as OnyxValue, mapping.key)); + get(mapping.key).then(() => sendDataToConnection(mapping, mapping.key)); return; } @@ -1335,24 +1312,23 @@ function setWithRetry({key, value, options}: SetParams OnyxUtils.retryOperation(error, setWithRetry, {key, value: valueWithoutNestedNullValues, options}, retryAttempt)) .then(() => { OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET, key, valueWithoutNestedNullValues); - return updatePromise; }); } @@ -1386,17 +1362,17 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom const keyValuePairsToSet = OnyxUtils.prepareKeyValuePairsForStorage(newData, true); - const updatePromises = keyValuePairsToSet.map(([key, value]) => { + for (const [key, value] of keyValuePairsToSet) { // When we use multiSet to set a key we want to clear the current delta changes from Onyx.merge that were queued // before the value was set. If Onyx.merge is currently reading the old value from storage, it will then not apply the changes. if (OnyxUtils.hasPendingMergeForKey(key)) { delete OnyxUtils.getMergeQueue()[key]; } - // Update cache and optimistically inform subscribers on the next tick + // Update cache and optimistically inform subscribers cache.set(key, value); - return OnyxUtils.scheduleSubscriberUpdate(key, value); - }); + keyChanged(key, value); + } const keyValuePairsToStore = keyValuePairsToSet.filter((keyValuePair) => { const [key] = keyValuePair; @@ -1408,9 +1384,7 @@ function multiSetWithRetry(data: OnyxMultiSetInput, retryAttempt?: number): Prom .catch((error) => OnyxUtils.retryOperation(error, multiSetWithRetry, newData, retryAttempt)) .then(() => { OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.MULTI_SET, undefined, newData); - return Promise.all(updatePromises); - }) - .then(() => undefined); + }); } /** @@ -1471,19 +1445,18 @@ function setCollectionWithRetry({collectionKey, for (const [key, value] of keyValuePairs) cache.set(key, value); - const updatePromise = OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection); + keysChanged(collectionKey, mutableCollection, 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 updatePromise; + return; } return Storage.multiSet(keyValuePairs) .catch((error) => OnyxUtils.retryOperation(error, setCollectionWithRetry, {collectionKey, collection}, retryAttempt)) .then(() => { OnyxUtils.sendActionToDevTools(OnyxUtils.METHOD.SET_COLLECTION, undefined, mutableCollection); - return updatePromise; }); }); } @@ -1615,7 +1588,7 @@ function mergeCollectionWithPatches( // and update all subscribers const promiseUpdate = previousCollectionPromise.then((previousCollection) => { cache.merge(finalMergedCollection); - return scheduleNotifyCollectionSubscribers(collectionKey, finalMergedCollection, previousCollection); + keysChanged(collectionKey, finalMergedCollection, previousCollection); }); return Promise.all(promises) @@ -1681,18 +1654,17 @@ function partialSetCollection({collectionKey, co for (const [key, value] of keyValuePairs) cache.set(key, value); - const updatePromise = scheduleNotifyCollectionSubscribers(collectionKey, mutableCollection, previousCollection); + keysChanged(collectionKey, mutableCollection, previousCollection); if (OnyxKeys.isRamOnlyKey(collectionKey)) { sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); - return updatePromise; + return; } return Storage.multiSet(keyValuePairs) .catch((error) => retryOperation(error, partialSetCollection, {collectionKey, collection}, retryAttempt)) .then(() => { sendActionToDevTools(METHOD.SET_COLLECTION, undefined, mutableCollection); - return updatePromise; }); }); } @@ -1733,8 +1705,6 @@ const OnyxUtils = { keyChanged, sendDataToConnection, getCollectionDataAndSendAsObject, - scheduleSubscriberUpdate, - scheduleNotifyCollectionSubscribers, remove, reportStorageQuota, retryOperation, diff --git a/tests/perf-test/OnyxUtils.perf-test.ts b/tests/perf-test/OnyxUtils.perf-test.ts index a26d7d695..2fc05ed36 100644 --- a/tests/perf-test/OnyxUtils.perf-test.ts +++ b/tests/perf-test/OnyxUtils.perf-test.ts @@ -368,7 +368,6 @@ describe('OnyxUtils', () => { subscriptionID, callback: jest.fn(), }, - mockedReportActionsMap, undefined, ), { @@ -432,66 +431,6 @@ describe('OnyxUtils', () => { }); }); - describe('scheduleSubscriberUpdate', () => { - test('10k calls scheduling updates', async () => { - const subscriptionMap = new Map(); - - const changedReportActions = Object.fromEntries( - Object.entries(mockedReportActionsMap).map(([k, v]) => [k, createRandomReportAction(Number(v.reportActionID))] as const), - ) as GenericCollection; - - await measureAsyncFunction(() => Promise.all(Object.entries(changedReportActions).map(([key, value]) => OnyxUtils.scheduleSubscriberUpdate(key, value))), { - beforeEach: async () => { - await Onyx.multiSet(mockedReportActionsMap); - for (const key of mockedReportActionsKeys) { - const id = OnyxUtils.subscribeToKey({key, callback: jest.fn(), initWithStoredValues: false}); - subscriptionMap.set(key, id); - } - }, - afterEach: async () => { - for (const key of mockedReportActionsKeys) { - const id = subscriptionMap.get(key); - if (id) { - OnyxUtils.unsubscribeFromKey(id); - } - } - subscriptionMap.clear(); - await clearOnyxAfterEachMeasure(); - }, - }); - }); - }); - - describe('scheduleNotifyCollectionSubscribers', () => { - test('one call with 10k heavy objects to update 10k subscribers', async () => { - const subscriptionMap = new Map(); - - const changedReportActions = Object.fromEntries( - Object.entries(mockedReportActionsMap).map(([k, v]) => [k, createRandomReportAction(Number(v.reportActionID))] as const), - ) as GenericCollection; - - await measureAsyncFunction(() => OnyxUtils.scheduleNotifyCollectionSubscribers(collectionKey, changedReportActions, mockedReportActionsMap), { - beforeEach: async () => { - await Onyx.multiSet(mockedReportActionsMap); - for (const key of mockedReportActionsKeys) { - const id = OnyxUtils.subscribeToKey({key, callback: jest.fn(), initWithStoredValues: false}); - subscriptionMap.set(key, id); - } - }, - afterEach: async () => { - for (const key of mockedReportActionsKeys) { - const id = subscriptionMap.get(key); - if (id) { - OnyxUtils.unsubscribeFromKey(id); - } - } - subscriptionMap.clear(); - await clearOnyxAfterEachMeasure(); - }, - }); - }); - }); - describe('remove', () => { test('10k calls', async () => { await measureAsyncFunction(() => Promise.all(mockedReportActionsKeys.map((key) => OnyxUtils.remove(key))), { @@ -535,7 +474,7 @@ describe('OnyxUtils', () => { const reportAction = mockedReportActionsMap[`${collectionKey}0`]; const changedReportAction = createRandomReportAction(Number(reportAction.reportActionID)); - await measureAsyncFunction(() => OnyxUtils.broadcastUpdate(key, changedReportAction, true), { + await measureFunction(() => OnyxUtils.broadcastUpdate(key, changedReportAction, true), { beforeEach: async () => { await Onyx.set(key, reportAction); }, diff --git a/tests/unit/collectionHydrationTest.ts b/tests/unit/collectionHydrationTest.ts new file mode 100644 index 000000000..64de44412 --- /dev/null +++ b/tests/unit/collectionHydrationTest.ts @@ -0,0 +1,165 @@ +import StorageMock from '../../lib/storage'; +import Onyx from '../../lib'; +import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; + +const ONYX_KEYS = { + COLLECTION: { + TEST_KEY: 'test_', + }, + SINGLE_KEY: 'single', +}; + +describe('Collection hydration with connect() followed by immediate set()', () => { + beforeEach(async () => { + // ===== Session 1 ===== + // Data is written to persistent storage (simulates a previous app session). + await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {id: 1, title: 'Test One'}); + await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`, {id: 2, title: 'Test Two'}); + await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`, {id: 3, title: 'Test Three'}); + await StorageMock.setItem(ONYX_KEYS.SINGLE_KEY, {title: 'old'}); + + // ===== Session 2 ===== + // App restarts. Onyx.init() calls getAllKeys() which populates storageKeys + // with all 3 keys, but their values are NOT read into cache yet. + Onyx.init({keys: ONYX_KEYS}); + }); + + afterEach(() => Onyx.clear()); + + test('waitForCollectionCallback=true should deliver full collection from storage', async () => { + const mockCallback = jest.fn(); + + // A component connects to the collection (starts async hydration via multiGet). + Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + waitForCollectionCallback: true, + callback: mockCallback, + }); + + Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {id: 1, title: 'Updated Test One'}); + + await waitForPromisesToResolve(); + + // The subscriber should eventually receive ALL collection members. + // The async hydration reads test_2 and test_3 from storage. + const lastCall = mockCallback.mock.calls[mockCallback.mock.calls.length - 1][0]; + expect(lastCall).toHaveProperty(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); + expect(lastCall).toHaveProperty(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); + expect(lastCall).toHaveProperty(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`); + + // Verify the updated value is present (not stale) + expect(lastCall[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]).toEqual({id: 1, title: 'Updated Test One'}); + }); + + test('waitForCollectionCallback=false should deliver all shallow-equal collection members when set() races with hydration', async () => { + // Clear existing storage and set up shallow-equal values for all members + await StorageMock.clear(); + await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {status: 'active'}); + await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`, {status: 'active'}); + await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`, {status: 'active'}); + // Re-init so Onyx picks up the new storage keys + Onyx.init({keys: ONYX_KEYS}); + + const mockCallback = jest.fn(); + + Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + waitForCollectionCallback: false, + callback: mockCallback, + }); + + // set() with the same shallow-equal value — this fires keyChanged synchronously, + // populating lastConnectionCallbackData before the hydration multiGet resolves. + Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {status: 'active'}); + + await waitForPromisesToResolve(); + + const deliveredKeys = new Set(); + for (const call of mockCallback.mock.calls) { + const [, key] = call; + if (key) { + deliveredKeys.add(key); + } + } + + // ALL three members must be delivered, even though their values are shallow-equal. + expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); + expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); + expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`); + }); + + test('single key: set() with non-shallow-equal value should not be overwritten by stale hydration', async () => { + const mockCallback = jest.fn(); + + Onyx.connect({ + key: ONYX_KEYS.SINGLE_KEY, + callback: mockCallback, + }); + + // Immediately update the key with a non-shallow-equal + Onyx.set(ONYX_KEYS.SINGLE_KEY, {title: 'new'}); + + await waitForPromisesToResolve(); + + // The LAST value delivered to the subscriber must be the fresh one, not the stale storage value + const lastValue = mockCallback.mock.calls[mockCallback.mock.calls.length - 1][0]; + expect(lastValue).toEqual({title: 'new'}); + }); + + test('collection key: set() with non-shallow-equal value should not be regressed by hydration multiGet', async () => { + const mockCallback = jest.fn(); + + Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + waitForCollectionCallback: true, + callback: mockCallback, + }); + + // Update key 1 with a non-shallow-equal value while hydration multiGet is in-flight + Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {id: 1, title: 'Freshly Updated'}); + + await waitForPromisesToResolve(); + + const lastCall = mockCallback.mock.calls[mockCallback.mock.calls.length - 1][0]; + + // The final collection snapshot must have the fresh value, not the stale storage one + expect(lastCall[`${ONYX_KEYS.COLLECTION.TEST_KEY}1`]).toEqual({id: 1, title: 'Freshly Updated'}); + // Other members should still be present from storage + expect(lastCall[`${ONYX_KEYS.COLLECTION.TEST_KEY}2`]).toEqual({id: 2, title: 'Test Two'}); + expect(lastCall[`${ONYX_KEYS.COLLECTION.TEST_KEY}3`]).toEqual({id: 3, title: 'Test Three'}); + }); + + test('waitForCollectionCallback=false should deliver all collection members from storage', async () => { + const mockCallback = jest.fn(); + + // A component connects to the collection (callback fires per key, not batched). + Onyx.connect({ + key: ONYX_KEYS.COLLECTION.TEST_KEY, + waitForCollectionCallback: false, + callback: mockCallback, + }); + + Onyx.set(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`, {id: 1, title: 'Updated Test One'}); + + await waitForPromisesToResolve(); + + // With waitForCollectionCallback=false, the callback fires per key individually. + // Collect all keys that were delivered across all calls. + const deliveredKeys = new Set(); + for (const call of mockCallback.mock.calls) { + const [, key] = call; + if (key) { + deliveredKeys.add(key); + } + } + + expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}1`); + expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}2`); + expect(deliveredKeys).toContain(`${ONYX_KEYS.COLLECTION.TEST_KEY}3`); + + // Verify the updated value is present (not stale) by finding the last call for key 1 + const key1Calls = mockCallback.mock.calls.filter((call) => call[1] === `${ONYX_KEYS.COLLECTION.TEST_KEY}1`); + const lastKey1Value = key1Calls[key1Calls.length - 1][0]; + expect(lastKey1Value).toEqual({id: 1, title: 'Updated Test One'}); + }); +}); diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 390f1a165..bfafd0768 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -1552,10 +1552,9 @@ describe('Onyx', () => { return waitForPromisesToResolve(); }) .then(() => { - expect(collectionCallback).toHaveBeenCalledTimes(3); + expect(collectionCallback).toHaveBeenCalledTimes(2); expect(collectionCallback).toHaveBeenNthCalledWith(1, {[cat]: initialValue}, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue}); - expect(collectionCallback).toHaveBeenNthCalledWith(2, {[cat]: initialValue}, ONYX_KEYS.COLLECTION.ANIMALS, undefined); - expect(collectionCallback).toHaveBeenNthCalledWith(3, collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue, [dog]: {name: 'Rex'}}); + expect(collectionCallback).toHaveBeenNthCalledWith(2, collectionDiff, ONYX_KEYS.COLLECTION.ANIMALS, {[cat]: initialValue, [dog]: {name: 'Rex'}}); // Cat hasn't changed from its original value, expect only the initial connect callback expect(catCallback).toHaveBeenCalledTimes(1);