fix: coalesce redundant GetMissingOnyxMessages while a fetch is in flight

src/libs/actions/OnyxUpdateManager/index.ts

@@ -160,8 +160,10 @@ function handleMissingOnyxUpdates<TKey extends OnyxKey>(onyxUpdatesFromServer: O
 
             Log.info('Client has not gotten reliable updates before so reconnecting the app to start the process');
 
-            // Since this is a full reconnectApp, we'll not apply the updates we received - those will come in the reconnect app request.
-            setMissingOnyxUpdatesQueryPromise(finalReconnectAppAfterActivatingReliableUpdates());
+            // A full reconnectApp does not apply the triggering update here; it comes back in the reconnect response.
+            // We still drain the deferred queue afterwards so an update enqueued while the reconnect is in flight is applied
+            // in order instead of being cleared by finalizeUpdatesAndResumeQueue.
+            setMissingOnyxUpdatesQueryPromise(finalReconnectAppAfterActivatingReliableUpdates().then(() => validateAndApplyDeferredUpdates(clientLastUpdateID)));
 
             return true;
         }
@@ -174,8 +176,8 @@ function handleMissingOnyxUpdates<TKey extends OnyxKey>(onyxUpdatesFromServer: O
         // Add the new update to the deferred updates
         enqueueDeferredOnyxUpdates(onyxUpdatesFromServer, {shouldPauseSequentialQueue: false});
 
-        // If there are deferred updates already, we don't need to fetch the missing updates again.
-        if (areDeferredUpdatesQueued || isFetchingForPendingUpdates) {
+        // If a fetch is already in progress, we don't need to start another one.
+        if (areDeferredUpdatesQueued || isFetchingForPendingUpdates || getMissingOnyxUpdatesQueryPromise()) {
             return false;
         }
 

tests/unit/OnyxUpdateManagerTest.ts

@@ -1,4 +1,5 @@
 import Onyx from 'react-native-onyx';
+import * as SequentialQueue from '@libs/Network/SequentialQueue';
 import type {AppActionsMock} from '@userActions/__mocks__/App';
 import type {OnyxUpdatesMock} from '@userActions/__mocks__/OnyxUpdates';
 import * as AppImport from '@userActions/App';
@@ -11,6 +12,7 @@ import * as OnyxUpdatesImport from '@userActions/OnyxUpdates';
 import ONYXKEYS from '@src/ONYXKEYS';
 import type {OnyxUpdatesFromServer} from '@src/types/onyx';
 import OnyxUpdateMockUtils from '../utils/OnyxUpdateMockUtils';
+import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
 
 jest.mock('@userActions/OnyxUpdates');
 jest.mock('@userActions/App');
@@ -68,6 +70,10 @@ describe('OnyxUpdateManager', () => {
         OnyxUpdateManager.resetDeferralLogicVariables();
     });
 
+    afterEach(() => {
+        jest.restoreAllMocks();
+    });
+
     it('should fetch missing Onyx updates once, defer updates and apply after missing updates', () => {
         OnyxUpdateManager.handleMissingOnyxUpdates(update3);
         OnyxUpdateManager.handleMissingOnyxUpdates(update4);
@@ -400,6 +406,88 @@ describe('OnyxUpdateManager', () => {
         });
     });
 
+    it('should not re-fetch missing updates when a duplicate arrives while a fetch is already in flight', async () => {
+        // Fire a duplicate mid-cycle, after the deferred queue is drained but while the fetch is still in flight, to simulate a concurrent Pusher update.
+        ApplyUpdates.mockValues.beforeApplyUpdates = async () => {
+            ApplyUpdates.mockValues.beforeApplyUpdates = undefined;
+            OnyxUpdateManager.handleMissingOnyxUpdates(update3);
+        };
+
+        OnyxUpdateManager.handleMissingOnyxUpdates(update3);
+        OnyxUpdateManager.handleMissingOnyxUpdates(update5);
+
+        return OnyxUpdateManager.queryPromise.then(() => {
+            // The mid-cycle duplicate must be coalesced, not fire its own GetMissingOnyxMessages.
+            expect(App.getMissingOnyxUpdates).toHaveBeenCalledTimes(2);
+            expect(App.getMissingOnyxUpdates).toHaveBeenNthCalledWith(1, 1, 2);
+            expect(App.getMissingOnyxUpdates).toHaveBeenNthCalledWith(2, 3, 4);
+        });
+    });
+
+    it('should resume the SequentialQueue exactly once when a duplicate arrives while a fetch is already in flight', async () => {
+        const unpauseSpy = jest.spyOn(SequentialQueue, 'unpause');
+
+        // Fire a duplicate mid-cycle, after the deferred queue is drained but while the fetch is still in flight, to simulate a concurrent Pusher update.
+        ApplyUpdates.mockValues.beforeApplyUpdates = async () => {
+            ApplyUpdates.mockValues.beforeApplyUpdates = undefined;
+            OnyxUpdateManager.handleMissingOnyxUpdates(update3);
+        };
+
+        OnyxUpdateManager.handleMissingOnyxUpdates(update3);
+        OnyxUpdateManager.handleMissingOnyxUpdates(update5);
+
+        // queryPromise resolves on the first finalize, so drain microtasks first: a leaked second fetch would attach a second finalize and resume the queue again.
+        return OnyxUpdateManager.queryPromise.then(waitForBatchedUpdates).then(() => {
+            expect(unpauseSpy).toHaveBeenCalledTimes(1);
+        });
+    });
+
+    it('should defer and later fetch a brand-new update that arrives while a fetch is already in flight', async () => {
+        // Fire a brand-new (higher) update mid-cycle, after the deferred queue is drained but while the fetch is still in flight, to simulate a concurrent Pusher update.
+        ApplyUpdates.mockValues.beforeApplyUpdates = async () => {
+            ApplyUpdates.mockValues.beforeApplyUpdates = undefined;
+            OnyxUpdateManager.handleMissingOnyxUpdates(update7);
+        };
+
+        OnyxUpdateManager.handleMissingOnyxUpdates(update3);
+        OnyxUpdateManager.handleMissingOnyxUpdates(update5);
+
+        return OnyxUpdateManager.queryPromise.then(() => {
+            // The new update must not fire its own parallel GetMissingOnyxMessages, but must still be fetched in order and applied (client reaches 7).
+            expect(App.getMissingOnyxUpdates).toHaveBeenCalledTimes(3);
+            expect(App.getMissingOnyxUpdates).toHaveBeenNthCalledWith(1, 1, 2);
+            expect(App.getMissingOnyxUpdates).toHaveBeenNthCalledWith(2, 3, 4);
+            expect(App.getMissingOnyxUpdates).toHaveBeenNthCalledWith(3, 5, 6);
+            expect(lastUpdateIDAppliedToClient).toBe(7);
+        });
+    });
+
+    it('should apply an update that arrives mid-reconnect instead of dropping it when the queue is finalized', async () => {
+        // First-ever reliable update: the client has never applied one, so the gap-handler takes the full-reconnect branch.
+        await Onyx.set(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, 0);
+
+        const reconnectSpy = jest.spyOn(App, 'finalReconnectAppAfterActivatingReliableUpdates').mockImplementation(async () => {
+            // The reconnect response advances the client to update 2.
+            await OnyxUpdates.apply(update2);
+            await waitForBatchedUpdates();
+
+            // A concurrent Pusher update lands while the reconnect promise is still in flight (after the client advanced to 2, before finalize).
+            OnyxUpdateManager.handleMissingOnyxUpdates(update3);
+        });
+
+        OnyxUpdateManager.handleMissingOnyxUpdates(update2);
+
+        return OnyxUpdateManager.queryPromise.then(() => {
+            // The mid-reconnect update must be applied in order, not cleared away by finalizeUpdatesAndResumeQueue (client reaches 3).
+            expect(lastUpdateIDAppliedToClient).toBe(3);
+
+            // It is contiguous with the reconnect response, so it must not trigger a redundant GetMissingOnyxMessages.
+            expect(App.getMissingOnyxUpdates).not.toHaveBeenCalled();
+
+            reconnectSpy.mockRestore();
+        });
+    });
+
     it('should apply deferred updates after fetching pending updates', () => {
         App.mockValues.missingOnyxUpdatesToBeApplied = [update2, update3];