diff --git a/src/libs/actions/OnyxUpdateManager/index.ts b/src/libs/actions/OnyxUpdateManager/index.ts index deb530547021..db00a55aa25b 100644 --- a/src/libs/actions/OnyxUpdateManager/index.ts +++ b/src/libs/actions/OnyxUpdateManager/index.ts @@ -75,7 +75,7 @@ function handleOnyxUpdateGap(onyxUpdatesFromServer: OnyxEntry((acc, [lastUpdateID, update]) => { - if (Number(lastUpdateID) >= firstUpdateAfterGapWithFallback) { - acc[Number(lastUpdateID)] = update; + // If there is a gap and we didn't detect two chained updates, "firstUpdateToBeAppliedAfterGap" will always be the the last deferred update. + // We will fetch all missing updates up to the previous update and can always apply the last deferred update. + const firstUpdateToBeAppliedAfterGap = firstUpdateIDAfterGaps ?? Number(updateValues[updateValues.length - 1].lastUpdateID); + + // Add all deferred updates after the gap(s) to "updatesAfterGaps". + // If "firstUpdateToBeAppliedAfterGap" is set to the last deferred update, the array will be empty. + Object.entries(pendingDeferredUpdates).forEach(([lastUpdateID, update]) => { + if (Number(lastUpdateID) < firstUpdateToBeAppliedAfterGap) { + return; } - return acc; + + updatesAfterGaps[Number(lastUpdateID)] = update; }, {}); } @@ -87,20 +118,16 @@ function validateAndApplyDeferredUpdates(clientLastUpdateID?: number, previousPa Log.info('[DeferredUpdates] Processing deferred updates', false, {lastUpdateIDFromClient, previousParams}); - // We only want to apply deferred updates that are newer than the last update that was applied to the client. - // At this point, the missing updates from "GetMissingOnyxUpdates" have been applied already, so we can safely filter out. - const pendingDeferredUpdates = DeferredOnyxUpdates.getUpdates({minUpdateID: lastUpdateIDFromClient}); + const {applicableUpdates, updatesAfterGaps, latestMissingUpdateID} = detectGapsAndSplit(lastUpdateIDFromClient); - // If there are no remaining deferred updates after filtering out outdated ones, - // we can just unpause the queue and return - if (Object.values(pendingDeferredUpdates).length === 0) { + // If there are no applicable deferred updates and no missing deferred updates, + // we don't need to apply or re-fetch any updates. We can just unpause the queue by resolving. + if (Object.values(applicableUpdates).length === 0 && latestMissingUpdateID === undefined) { return Promise.resolve(); } - const {applicableUpdates, updatesAfterGaps, latestMissingUpdateID} = detectGapsAndSplit(pendingDeferredUpdates, lastUpdateIDFromClient); - - // If we detected a gap in the deferred updates, only apply the deferred updates before the gap, - // re-fetch the missing updates and then apply the remaining deferred updates after the gap + // If newer updates got applied, we don't need to refetch for missing updates + // and will re-trigger the "validateAndApplyDeferredUpdates" process if (latestMissingUpdateID) { Log.info('[DeferredUpdates] Gap detected in deferred updates', false, {lastUpdateIDFromClient, latestMissingUpdateID}); @@ -117,7 +144,8 @@ function validateAndApplyDeferredUpdates(clientLastUpdateID?: number, previousPa DeferredOnyxUpdates.enqueue(updatesAfterGaps, {shouldPauseSequentialQueue: false}); - // If lastUpdateIDAppliedToClient got updated in the meantime, we will just retrigger the validation and application of the current deferred updates. + // If lastUpdateIDAppliedToClient got updated, we will just retrigger the validation + // and application of the current deferred updates. if (latestMissingUpdateID <= newLastUpdateIDFromClient) { validateAndApplyDeferredUpdates(undefined, {newLastUpdateIDFromClient, latestMissingUpdateID}) .then(() => resolve(undefined)) diff --git a/tests/unit/OnyxUpdateManagerTest.ts b/tests/unit/OnyxUpdateManagerTest.ts index 20b29ca18a56..2cfd1eec10c7 100644 --- a/tests/unit/OnyxUpdateManagerTest.ts +++ b/tests/unit/OnyxUpdateManagerTest.ts @@ -18,6 +18,7 @@ const ApplyUpdates = ApplyUpdatesImport as ApplyUpdatesMock; const OnyxUpdateManagerUtils = OnyxUpdateManagerUtilsImport as OnyxUpdateManagerUtilsMock; const mockUpdate3 = createOnyxMockUpdate(3); +const offsetedMockUpdate3 = createOnyxMockUpdate(3, undefined, 2); const mockUpdate4 = createOnyxMockUpdate(4); const mockUpdate5 = createOnyxMockUpdate(5); const mockUpdate6 = createOnyxMockUpdate(6); @@ -193,7 +194,7 @@ describe('OnyxUpdateManager', () => { // Even though there are multiple gaps in the deferred updates, we only want to fetch missing updates once per batch. expect(App.getMissingOnyxUpdates).toHaveBeenCalledTimes(1); - // validateAndApplyDeferredUpdates should be called twice, once for the initial deferred updates and once for the remaining deferred updates with gaps. + // validateAndApplyDeferredUpdates should be called once for the initial deferred updates // Unfortunately, we cannot easily count the calls of this function with Jest, since it recursively calls itself. // The intended assertion would look like this: // expect(OnyxUpdateManagerUtils.validateAndApplyDeferredUpdates).toHaveBeenCalledTimes(1); @@ -254,4 +255,28 @@ describe('OnyxUpdateManager', () => { expect(ApplyUpdates.applyUpdates).toHaveBeenNthCalledWith(2, {7: mockUpdate7}); }); }); + + it('should only fetch missing updates that are not outdated (older than already locally applied update)', () => { + OnyxUpdateManager.handleOnyxUpdateGap(offsetedMockUpdate3); + OnyxUpdateManager.handleOnyxUpdateGap(mockUpdate4); + + return OnyxUpdateManager.queryPromise.then(() => { + // After all missing and deferred updates have been applied, the lastUpdateIDAppliedToClient should be 4. + expect(lastUpdateIDAppliedToClient).toBe(4); + + // validateAndApplyDeferredUpdates should be called once for the initial deferred updates + // Unfortunately, we cannot easily count the calls of this function with Jest, since it recursively calls itself. + // The intended assertion would look like this: + // expect(OnyxUpdateManagerUtils.validateAndApplyDeferredUpdates).toHaveBeenCalledTimes(1); + + // There should be only one call to applyUpdates. The call should contain all the deferred update, + // since the locally applied updates have changed in the meantime. + expect(ApplyUpdates.applyUpdates).toHaveBeenCalledTimes(1); + // eslint-disable-next-line @typescript-eslint/naming-convention + expect(ApplyUpdates.applyUpdates).toHaveBeenCalledWith({3: offsetedMockUpdate3, 4: mockUpdate4}); + + // There are no gaps in the deferred updates, therefore only one call to getMissingOnyxUpdates should be triggered + expect(App.getMissingOnyxUpdates).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/tests/utils/createOnyxMockUpdate.ts b/tests/utils/createOnyxMockUpdate.ts index f8e28e364cf7..e50918b01faf 100644 --- a/tests/utils/createOnyxMockUpdate.ts +++ b/tests/utils/createOnyxMockUpdate.ts @@ -1,10 +1,10 @@ import type {OnyxUpdate} from 'react-native-onyx'; import type {OnyxUpdatesFromServer} from '@src/types/onyx'; -const createOnyxMockUpdate = (lastUpdateID: number, successData: OnyxUpdate[] = []): OnyxUpdatesFromServer => ({ +const createOnyxMockUpdate = (lastUpdateID: number, successData: OnyxUpdate[] = [], previousUpdateIDOffset = 1): OnyxUpdatesFromServer => ({ type: 'https', lastUpdateID, - previousUpdateID: lastUpdateID - 1, + previousUpdateID: lastUpdateID - previousUpdateIDOffset, request: { command: 'TestCommand', successData, @@ -15,7 +15,7 @@ const createOnyxMockUpdate = (lastUpdateID: number, successData: OnyxUpdate[] = response: { jsonCode: 200, lastUpdateID, - previousUpdateID: lastUpdateID - 1, + previousUpdateID: lastUpdateID - previousUpdateIDOffset, onyxData: successData, }, });