Merge pull request #39683 from margelo/@chrispader/GetMissingOnyxMess…

…ages-deferred-updates-after-revert

After revert: Defer local updates if there are missing updates and only call `GetMissingOnyxMessages` once

src/libs/Network/SequentialQueue.ts

@@ -156,6 +156,10 @@ function isRunning(): boolean {
     return isSequentialQueueRunning;
 }
 
+function isPaused(): boolean {
+    return isQueuePaused;
+}
+
 // Flush the queue when the connection resumes
 NetworkStore.onReconnection(flush);
 
@@ -191,4 +195,4 @@ function waitForIdle(): Promise<unknown> {
     return isReadyPromise;
 }
 
-export {flush, getCurrentRequest, isRunning, push, waitForIdle, pause, unpause};
+export {flush, getCurrentRequest, isRunning, isPaused, push, waitForIdle, pause, unpause};

src/libs/actions/OnyxUpdateManager.ts → src/libs/actions/OnyxUpdateManager/index.ts

@@ -3,11 +3,12 @@ import Onyx from 'react-native-onyx';
 import * as ActiveClientManager from '@libs/ActiveClientManager';
 import Log from '@libs/Log';
 import * as SequentialQueue from '@libs/Network/SequentialQueue';
-import CONST from '@src/CONST';
+import * as App from '@userActions/App';
 import ONYXKEYS from '@src/ONYXKEYS';
-import type {OnyxUpdatesFromServer} from '@src/types/onyx';
-import * as App from './App';
-import * as OnyxUpdates from './OnyxUpdates';
+import type {OnyxUpdatesFromServer, Response} from '@src/types/onyx';
+import {isValidOnyxUpdateFromServer} from '@src/types/onyx/OnyxUpdatesFromServer';
+import * as OnyxUpdateManagerUtils from './utils';
+import deferredUpdatesProxy from './utils/deferredUpdates';
 
 // This file is in charge of looking at the updateIDs coming from the server and comparing them to the last updateID that the client has.
 // If the client is behind the server, then we need to
@@ -19,15 +20,15 @@ import * as OnyxUpdates from './OnyxUpdates';
 // 6. Restart the sequential queue
 // 7. Restart the Onyx updates from Pusher
 // This will ensure that the client is up-to-date with the server and all the updates have been applied in the correct order.
-// It's important that this file is separate and not imported by OnyxUpdates.js, so that there are no circular dependencies. Onyx
-// is used as a pub/sub mechanism to break out of the circular dependency.
-// The circular dependency happens because this file calls API.GetMissingOnyxUpdates() which uses the SaveResponseInOnyx.js file
-// (as a middleware). Therefore, SaveResponseInOnyx.js can't import and use this file directly.
+// It's important that this file is separate and not imported by OnyxUpdates.js, so that there are no circular dependencies.
+// Onyx is used as a pub/sub mechanism to break out of the circular dependency.
+// The circular dependency happens because this file calls API.GetMissingOnyxUpdates() which uses the SaveResponseInOnyx.js file (as a middleware).
+// Therefore, SaveResponseInOnyx.js can't import and use this file directly.
 
-let lastUpdateIDAppliedToClient: number | null = 0;
+let lastUpdateIDAppliedToClient = 0;
 Onyx.connect({
     key: ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT,
-    callback: (value) => (lastUpdateIDAppliedToClient = value),
+    callback: (value) => (lastUpdateIDAppliedToClient = value ?? 0),
 });
 
 let isLoadingApp = false;
@@ -38,17 +39,40 @@ Onyx.connect({
     },
 });
 
+let queryPromise: Promise<Response | Response[] | void> | undefined;
+
+let resolveQueryPromiseWrapper: () => void;
+const createQueryPromiseWrapper = () =>
+    new Promise<void>((resolve) => {
+        resolveQueryPromiseWrapper = resolve;
+    });
+// eslint-disable-next-line import/no-mutable-exports
+let queryPromiseWrapper = createQueryPromiseWrapper();
+
+const resetDeferralLogicVariables = () => {
+    queryPromise = undefined;
+    deferredUpdatesProxy.deferredUpdates = {};
+};
+
+// This function will reset the query variables, unpause the SequentialQueue and log an info to the user.
+function finalizeUpdatesAndResumeQueue() {
+    console.debug('[OnyxUpdateManager] Done applying all updates');
+
+    resolveQueryPromiseWrapper();
+    queryPromiseWrapper = createQueryPromiseWrapper();
+
+    resetDeferralLogicVariables();
+    Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null);
+    SequentialQueue.unpause();
+}
+
 /**
  *
  * @param onyxUpdatesFromServer
  * @param clientLastUpdateID an optional override for the lastUpdateIDAppliedToClient
  * @returns
  */
-function handleOnyxUpdateGap(onyxUpdatesFromServer: OnyxEntry<OnyxUpdatesFromServer>, clientLastUpdateID = 0) {
-    // When there's no value, there's nothing to process, so let's return early.
-    if (!onyxUpdatesFromServer) {
-        return;
-    }
+function handleOnyxUpdateGap(onyxUpdatesFromServer: OnyxEntry<OnyxUpdatesFromServer>, clientLastUpdateID?: number) {
     // If isLoadingApp is positive it means that OpenApp command hasn't finished yet, and in that case
     // we don't have base state of the app (reports, policies, etc) setup. If we apply this update,
     // we'll only have them overriten by the openApp response. So let's skip it and return.
@@ -66,24 +90,15 @@ function handleOnyxUpdateGap(onyxUpdatesFromServer: OnyxEntry<OnyxUpdatesFromSer
         return;
     }
 
-    // Since we used the same key that used to store another object, let's confirm that the current object is
-    // following the new format before we proceed. If it isn't, then let's clear the object in Onyx.
-    if (
-        !(typeof onyxUpdatesFromServer === 'object' && !!onyxUpdatesFromServer) ||
-        !('type' in onyxUpdatesFromServer) ||
-        (!(onyxUpdatesFromServer.type === CONST.ONYX_UPDATE_TYPES.HTTPS && onyxUpdatesFromServer.request && onyxUpdatesFromServer.response) &&
-            !((onyxUpdatesFromServer.type === CONST.ONYX_UPDATE_TYPES.PUSHER || onyxUpdatesFromServer.type === CONST.ONYX_UPDATE_TYPES.AIRSHIP) && onyxUpdatesFromServer.updates))
-    ) {
-        console.debug('[OnyxUpdateManager] Invalid format found for updates, cleaning and unpausing the queue');
-        Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null);
-        SequentialQueue.unpause();
+    // When there is no value or an invalid value, there's nothing to process, so let's return early.
+    if (!isValidOnyxUpdateFromServer(onyxUpdatesFromServer)) {
         return;
     }
 
     const updateParams = onyxUpdatesFromServer;
     const lastUpdateIDFromServer = onyxUpdatesFromServer.lastUpdateID;
     const previousUpdateIDFromServer = onyxUpdatesFromServer.previousUpdateID;
-    const lastUpdateIDFromClient = clientLastUpdateID || lastUpdateIDAppliedToClient;
+    const lastUpdateIDFromClient = clientLastUpdateID ?? lastUpdateIDAppliedToClient ?? 0;
 
     // In cases where we received a previousUpdateID and it doesn't match our lastUpdateIDAppliedToClient
     // we need to perform one of the 2 possible cases:
@@ -92,32 +107,45 @@ function handleOnyxUpdateGap(onyxUpdatesFromServer: OnyxEntry<OnyxUpdatesFromSer
     // fully migrating to the reliable updates mode.
     // 2. This client already has the reliable updates mode enabled, but it's missing some updates and it
     // needs to fetch those.
-    let canUnpauseQueuePromise;
 
     // The flow below is setting the promise to a reconnect app to address flow (1) explained above.
     if (!lastUpdateIDFromClient) {
+        // If there is a ReconnectApp query in progress, we should not start another one.
+        if (queryPromise) {
+            return;
+        }
+
         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.
-        canUnpauseQueuePromise = App.finalReconnectAppAfterActivatingReliableUpdates();
+        queryPromise = App.finalReconnectAppAfterActivatingReliableUpdates();
     } else {
         // The flow below is setting the promise to a getMissingOnyxUpdates to address flow (2) explained above.
+
+        // Get the number of deferred updates before adding the new one
+        const existingDeferredUpdatesCount = Object.keys(deferredUpdatesProxy.deferredUpdates).length;
+
+        // Add the new update to the deferred updates
+        deferredUpdatesProxy.deferredUpdates[Number(updateParams.lastUpdateID)] = updateParams;
+
+        // If there are deferred updates already, we don't need to fetch the missing updates again.
+        if (existingDeferredUpdatesCount > 0) {
+            return;
+        }
+
         console.debug(`[OnyxUpdateManager] Client is behind the server by ${Number(previousUpdateIDFromServer) - lastUpdateIDFromClient} so fetching incremental updates`);
         Log.info('Gap detected in update IDs from server so fetching incremental updates', true, {
             lastUpdateIDFromServer,
             previousUpdateIDFromServer,
             lastUpdateIDFromClient,
         });
-        canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDFromClient, previousUpdateIDFromServer);
+
+        // Get the missing Onyx updates from the server and afterwards validate and apply the deferred updates.
+        // This will trigger recursive calls to "validateAndApplyDeferredUpdates" if there are gaps in the deferred updates.
+        queryPromise = App.getMissingOnyxUpdates(lastUpdateIDFromClient, previousUpdateIDFromServer).then(() => OnyxUpdateManagerUtils.validateAndApplyDeferredUpdates(clientLastUpdateID));
     }
 
-    canUnpauseQueuePromise.finally(() => {
-        OnyxUpdates.apply(updateParams).finally(() => {
-            console.debug('[OnyxUpdateManager] Done applying all updates');
-            Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null);
-            SequentialQueue.unpause();
-        });
-    });
+    queryPromise.finally(finalizeUpdatesAndResumeQueue);
 }
 
 export default () => {
@@ -128,4 +156,4 @@ export default () => {
     });
 };
 
-export {handleOnyxUpdateGap};
+export {handleOnyxUpdateGap, queryPromiseWrapper as queryPromise, resetDeferralLogicVariables};

src/libs/actions/OnyxUpdateManager/types.ts

@@ -0,0 +1,7 @@
+import type {OnyxUpdatesFromServer} from '@src/types/onyx';
+
+type DeferredUpdatesDictionary = Record<number, OnyxUpdatesFromServer>;
+
+type DetectGapAndSplitResult = {applicableUpdates: DeferredUpdatesDictionary; updatesAfterGaps: DeferredUpdatesDictionary; latestMissingUpdateID: number | undefined};
+
+export type {DeferredUpdatesDictionary, DetectGapAndSplitResult};

src/libs/actions/OnyxUpdateManager/utils/__mocks__/applyUpdates.ts

@@ -0,0 +1,34 @@
+import Onyx from 'react-native-onyx';
+import type {DeferredUpdatesDictionary} from '@libs/actions/OnyxUpdateManager/types';
+import ONYXKEYS from '@src/ONYXKEYS';
+import createProxyForObject from '@src/utils/createProxyForObject';
+
+let lastUpdateIDAppliedToClient = 0;
+Onyx.connect({
+    key: ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT,
+    callback: (value) => (lastUpdateIDAppliedToClient = value ?? 0),
+});
+
+type ApplyUpdatesMockValues = {
+    onApplyUpdates: ((updates: DeferredUpdatesDictionary) => Promise<void>) | undefined;
+};
+
+type ApplyUpdatesMock = {
+    applyUpdates: jest.Mock<Promise<[]>, [updates: DeferredUpdatesDictionary]>;
+    mockValues: ApplyUpdatesMockValues;
+};
+
+const mockValues: ApplyUpdatesMockValues = {
+    onApplyUpdates: undefined,
+};
+const mockValuesProxy = createProxyForObject(mockValues);
+
+const applyUpdates = jest.fn((updates: DeferredUpdatesDictionary) => {
+    const lastUpdateIdFromUpdates = Math.max(...Object.keys(updates).map(Number));
+    return (mockValuesProxy.onApplyUpdates === undefined ? Promise.resolve() : mockValuesProxy.onApplyUpdates(updates)).then(() =>
+        Onyx.set(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, Math.max(lastUpdateIDAppliedToClient, lastUpdateIdFromUpdates)),
+    );
+});
+
+export {applyUpdates, mockValuesProxy as mockValues};
+export type {ApplyUpdatesMock};

src/libs/actions/OnyxUpdateManager/utils/__mocks__/index.ts

@@ -0,0 +1,32 @@
+import type {DeferredUpdatesDictionary, DetectGapAndSplitResult} from '@libs/actions/OnyxUpdateManager/types';
+import createProxyForObject from '@src/utils/createProxyForObject';
+import type * as OnyxUpdateManagerUtilsImport from '..';
+import {applyUpdates} from './applyUpdates';
+
+const UtilsImplementation: typeof OnyxUpdateManagerUtilsImport = jest.requireActual('@libs/actions/OnyxUpdateManager/utils');
+
+type OnyxUpdateManagerUtilsMockValues = {
+    onValidateAndApplyDeferredUpdates: ((clientLastUpdateID?: number) => Promise<void>) | undefined;
+};
+
+type OnyxUpdateManagerUtilsMock = typeof UtilsImplementation & {
+    detectGapsAndSplit: jest.Mock<Promise<DetectGapAndSplitResult>, [updates: DeferredUpdatesDictionary, clientLastUpdateID?: number]>;
+    validateAndApplyDeferredUpdates: jest.Mock<Promise<void>, [clientLastUpdateID?: number]>;
+    mockValues: OnyxUpdateManagerUtilsMockValues;
+};
+
+const mockValues: OnyxUpdateManagerUtilsMockValues = {
+    onValidateAndApplyDeferredUpdates: undefined,
+};
+const mockValuesProxy = createProxyForObject(mockValues);
+
+const detectGapsAndSplit = jest.fn(UtilsImplementation.detectGapsAndSplit);
+
+const validateAndApplyDeferredUpdates = jest.fn((clientLastUpdateID?: number) =>
+    (mockValuesProxy.onValidateAndApplyDeferredUpdates === undefined ? Promise.resolve() : mockValuesProxy.onValidateAndApplyDeferredUpdates(clientLastUpdateID)).then(() =>
+        UtilsImplementation.validateAndApplyDeferredUpdates(clientLastUpdateID),
+    ),
+);
+
+export {applyUpdates, detectGapsAndSplit, validateAndApplyDeferredUpdates, mockValuesProxy as mockValues};
+export type {OnyxUpdateManagerUtilsMock};

src/libs/actions/OnyxUpdateManager/utils/applyUpdates.ts

@@ -0,0 +1,9 @@
+// We need to keep this in a separate file, so that we can mock this function in tests.
+import type {DeferredUpdatesDictionary} from '@libs/actions/OnyxUpdateManager/types';
+import * as OnyxUpdates from '@userActions/OnyxUpdates';
+
+// This function applies a list of updates to Onyx in order and resolves when all updates have been applied
+const applyUpdates = (updates: DeferredUpdatesDictionary) => Promise.all(Object.values(updates).map((update) => OnyxUpdates.apply(update)));
+
+// eslint-disable-next-line import/prefer-default-export
+export {applyUpdates};

src/libs/actions/OnyxUpdateManager/utils/deferredUpdates.ts

@@ -0,0 +1,8 @@
+import type {DeferredUpdatesDictionary} from '@libs/actions/OnyxUpdateManager/types';
+import createProxyForObject from '@src/utils/createProxyForObject';
+
+const deferredUpdatesValue = {deferredUpdates: {} as DeferredUpdatesDictionary};
+
+const deferredUpdatesProxy = createProxyForObject(deferredUpdatesValue);
+
+export default deferredUpdatesProxy;