Merge pull request #25455 from Expensify/tgolen-ordered-updates

Apply Onyx updates in an ordered fashion

src/App.js

@@ -24,6 +24,7 @@ import {CurrentReportIDContextProvider} from './components/withCurrentReportID';
 import {EnvironmentProvider} from './components/withEnvironment';
 import * as Session from './libs/actions/Session';
 import useDefaultDragAndDrop from './hooks/useDefaultDragAndDrop';
+import OnyxUpdateManager from './libs/actions/OnyxUpdateManager';
 
 // For easier debugging and development, when we are in web we expose Onyx to the window, so you can more easily set data into Onyx
 if (window && Environment.isDevelopment()) {
@@ -42,6 +43,7 @@ const fill = {flex: 1};
 
 function App() {
     useDefaultDragAndDrop();
+    OnyxUpdateManager();
     return (
         <GestureHandlerRootView style={fill}>
             <ComposeProviders

src/CONST.ts

@@ -2629,6 +2629,10 @@ const CONST = {
         DEFAULT_COORDINATE: [-122.4021, 37.7911],
         STYLE_URL: 'mapbox://styles/expensify/cllcoiqds00cs01r80kp34tmq',
     },
+    ONYX_UPDATE_TYPES: {
+        HTTPS: 'https',
+        PUSHER: 'pusher',
+    },
 } as const;
 
 export default CONST;

src/ONYXKEYS.ts

@@ -367,7 +367,7 @@ type OnyxValues = {
     [ONYXKEYS.SELECTED_TAB]: string;
     [ONYXKEYS.RECEIPT_MODAL]: OnyxTypes.ReceiptModal;
     [ONYXKEYS.MAPBOX_ACCESS_TOKEN]: OnyxTypes.MapboxAccessToken;
-    [ONYXKEYS.ONYX_UPDATES_FROM_SERVER]: number;
+    [ONYXKEYS.ONYX_UPDATES_FROM_SERVER]: OnyxTypes.OnyxUpdatesFromServer;
     [ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT]: number;
     [ONYXKEYS.MAX_CANVAS_AREA]: number;
     [ONYXKEYS.MAX_CANVAS_HEIGHT]: number;

src/libs/API.js

@@ -21,7 +21,8 @@ Request.use(Middleware.RecheckConnection);
 // Reauthentication - Handles jsonCode 407 which indicates an expired authToken. We need to reauthenticate and get a new authToken with our stored credentials.
 Request.use(Middleware.Reauthentication);
 
-// SaveResponseInOnyx - Merges either the successData or failureData into Onyx depending on if the call was successful or not
+// SaveResponseInOnyx - Merges either the successData or failureData into Onyx depending on if the call was successful or not. This needs to be the LAST middleware we use, don't add any
+// middlewares after this, because the SequentialQueue depends on the result of this middleware to pause the queue (if needed) to bring the app to an up-to-date state.
 Request.use(Middleware.SaveResponseInOnyx);
 
 /**

src/libs/Middleware/SaveResponseInOnyx.js

@@ -1,34 +1,32 @@
-import Onyx from 'react-native-onyx';
 import _ from 'underscore';
 import CONST from '../../CONST';
 import ONYXKEYS from '../../ONYXKEYS';
-import * as QueuedOnyxUpdates from '../actions/QueuedOnyxUpdates';
 import * as MemoryOnlyKeys from '../actions/MemoryOnlyKeys/MemoryOnlyKeys';
 import * as OnyxUpdates from '../actions/OnyxUpdates';
 
+// If we're executing any of these requests, we don't need to trigger our OnyxUpdates flow to update the current data even if our current value is out of
+// date because all these requests are updating the app to the most current state.
+const requestsToIgnoreLastUpdateID = ['OpenApp', 'ReconnectApp', 'GetMissingOnyxMessages'];
+
 /**
- * @param {Promise} response
+ * @param {Promise} requestResponse
  * @param {Object} request
  * @returns {Promise}
  */
-function SaveResponseInOnyx(response, request) {
-    return response.then((responseData) => {
-        // Make sure we have response data (i.e. response isn't a promise being passed down to us by a failed retry request and responseData undefined)
-        if (!responseData) {
+function SaveResponseInOnyx(requestResponse, request) {
+    return requestResponse.then((response) => {
+        // Make sure we have response data (i.e. response isn't a promise being passed down to us by a failed retry request and response undefined)
+        if (!response) {
             return;
         }
+        const onyxUpdates = response.onyxData;
 
-        // The data for this response comes in two different formats:
-        // 1. Original format - this is what was sent before the RELIABLE_UPDATES project and will go away once RELIABLE_UPDATES is fully complete
-        //     - The data is an array of objects, where each object is an onyx update
-        //       Example: [{onyxMethod: 'whatever', key: 'foo', value: 'bar'}]
-        // 1. Reliable updates format - this is what was sent with the RELIABLE_UPDATES project and will be the format from now on
-        //     - The data is an object, containing updateIDs from the server and an array of onyx updates (this array is the same format as the original format above)
-        //       Example: {lastUpdateID: 1, previousUpdateID: 0, onyxData: [{onyxMethod: 'whatever', key: 'foo', value: 'bar'}]}
-        //       NOTE: This is slightly different than the format of the pusher event data, where pusher has "updates" and HTTPS responses have "onyxData" (long story)
+        // Sometimes we call requests that are successfull but they don't have any response or any success/failure data to set. Let's return early since
+        // we don't need to store anything here.
+        if (!onyxUpdates && !request.successData && !request.failureData) {
+            return Promise.resolve(response);
+        }
 
-        // Supports both the old format and the new format
-        const onyxUpdates = _.isArray(responseData) ? responseData : responseData.onyxData;
         // If there is an OnyxUpdate for using memory only keys, enable them
         _.find(onyxUpdates, ({key, value}) => {
             if (key !== ONYXKEYS.IS_USING_MEMORY_ONLY_KEYS || !value) {
@@ -39,30 +37,26 @@ function SaveResponseInOnyx(response, request) {
             return true;
         });
 
-        // Save the update IDs to Onyx so they can be used to fetch incremental updates if the client gets out of sync from the server
-        OnyxUpdates.saveUpdateIDs(Number(responseData.lastUpdateID || 0), Number(responseData.previousUpdateID || 0));
+        const responseToApply = {
+            type: CONST.ONYX_UPDATE_TYPES.HTTPS,
+            lastUpdateID: Number(response.lastUpdateID || 0),
+            previousUpdateID: Number(response.previousUpdateID || 0),
+            request,
+            response,
+        };
 
-        // For most requests we can immediately update Onyx. For write requests we queue the updates and apply them after the sequential queue has flushed to prevent a replay effect in
-        // the UI. See https://github.com/Expensify/App/issues/12775 for more info.
-        const updateHandler = request.data.apiRequestType === CONST.API_REQUEST_TYPE.WRITE ? QueuedOnyxUpdates.queueOnyxUpdates : Onyx.update;
+        if (_.includes(requestsToIgnoreLastUpdateID, request.command) || !OnyxUpdates.doesClientNeedToBeUpdated(Number(response.previousUpdateID || 0))) {
+            return OnyxUpdates.apply(responseToApply);
+        }
 
-        // First apply any onyx data updates that are being sent back from the API. We wait for this to complete and then
-        // apply successData or failureData. This ensures that we do not update any pending, loading, or other UI states contained
-        // in successData/failureData until after the component has received and API data.
-        const onyxDataUpdatePromise = responseData.onyxData ? updateHandler(responseData.onyxData) : Promise.resolve();
+        // Save the update IDs to Onyx so they can be used to fetch incremental updates if the client gets out of sync from the server
+        OnyxUpdates.saveUpdateInformation(responseToApply);
 
-        return onyxDataUpdatePromise
-            .then(() => {
-                // Handle the request's success/failure data (client-side data)
-                if (responseData.jsonCode === 200 && request.successData) {
-                    return updateHandler(request.successData);
-                }
-                if (responseData.jsonCode !== 200 && request.failureData) {
-                    return updateHandler(request.failureData);
-                }
-                return Promise.resolve();
-            })
-            .then(() => responseData);
+        // Ensure the queue is paused while the client resolves the gap in onyx updates so that updates are guaranteed to happen in a specific order.
+        return Promise.resolve({
+            ...response,
+            shouldPauseQueue: true,
+        });
     });
 }
 

src/libs/Network/SequentialQueue.js

@@ -21,6 +21,30 @@ let isSequentialQueueRunning = false;
 let currentRequest = null;
 let isQueuePaused = false;
 
+/**
+ * Puts the queue into a paused state so that no requests will be processed
+ */
+function pause() {
+    if (isQueuePaused) {
+        return;
+    }
+
+    console.debug('[SequentialQueue] Pausing the queue');
+    isQueuePaused = true;
+}
+
+/**
+ * Gets the current Onyx queued updates, apply them and clear the queue if the queue is not paused.
+ */
+function flushOnyxUpdatesQueue() {
+    // The only situation where the queue is paused is if we found a gap between the app current data state and our server's. If that happens,
+    // we'll trigger async calls to make the client updated again. While we do that, we don't want to insert anything in Onyx.
+    if (isQueuePaused) {
+        return;
+    }
+    QueuedOnyxUpdates.flushQueue();
+}
+
 /**
  * Process any persisted requests, when online, one at a time until the queue is empty.
  *
@@ -44,7 +68,12 @@ function process() {
 
     // Set the current request to a promise awaiting its processing so that getCurrentRequest can be used to take some action after the current request has processed.
     currentRequest = Request.processWithMiddleware(requestToProcess, true)
-        .then(() => {
+        .then((response) => {
+            // A response might indicate that the queue should be paused. This happens when a gap in onyx updates is detected between the client and the server and
+            // that gap needs resolved before the queue can continue.
+            if (response.shouldPauseQueue) {
+                pause();
+            }
             PersistedRequests.remove(requestToProcess);
             RequestThrottle.clear();
             return process();
@@ -94,12 +123,27 @@ function flush() {
                 isSequentialQueueRunning = false;
                 resolveIsReadyPromise();
                 currentRequest = null;
-                Onyx.update(QueuedOnyxUpdates.getQueuedUpdates()).then(QueuedOnyxUpdates.clear);
+                flushOnyxUpdatesQueue();
             });
         },
     });
 }
 
+/**
+ * Unpauses the queue and flushes all the requests that were in it or were added to it while paused
+ */
+function unpause() {
+    if (!isQueuePaused) {
+        return;
+    }
+
+    const numberOfPersistedRequests = PersistedRequests.getAll().length || 0;
+    console.debug(`[SequentialQueue] Unpausing the queue and flushing ${numberOfPersistedRequests} requests`);
+    isQueuePaused = false;
+    flushOnyxUpdatesQueue();
+    flush();
+}
+
 /**
  * @returns {Boolean}
  */
@@ -149,30 +193,4 @@ function waitForIdle() {
     return isReadyPromise;
 }
 
-/**
- * Puts the queue into a paused state so that no requests will be processed
- */
-function pause() {
-    if (isQueuePaused) {
-        return;
-    }
-
-    console.debug('[SequentialQueue] Pausing the queue');
-    isQueuePaused = true;
-}
-
-/**
- * Unpauses the queue and flushes all the requests that were in it or were added to it while paused
- */
-function unpause() {
-    if (!isQueuePaused) {
-        return;
-    }
-
-    const numberOfPersistedRequests = PersistedRequests.getAll().length || 0;
-    console.debug(`[SequentialQueue] Unpausing the queue and flushing ${numberOfPersistedRequests} requests`);
-    isQueuePaused = false;
-    flush();
-}
-
 export {flush, getCurrentRequest, isRunning, push, waitForIdle, pause, unpause};

src/libs/PusherUtils.js

@@ -18,12 +18,13 @@ function subscribeToMultiEvent(eventType, callback) {
 /**
  * @param {String} eventType
  * @param {Mixed} data
+ * @returns {Promise}
  */
 function triggerMultiEventHandler(eventType, data) {
     if (!multiEventCallbackMapping[eventType]) {
-        return;
+        return Promise.resolve();
     }
-    multiEventCallbackMapping[eventType](data);
+    return multiEventCallbackMapping[eventType](data);
 }
 
 /**

src/libs/actions/App.js

@@ -18,7 +18,6 @@ import * as Session from './Session';
 import * as ReportActionsUtils from '../ReportActionsUtils';
 import Timing from './Timing';
 import * as Browser from '../Browser';
-import * as SequentialQueue from '../Network/SequentialQueue';
 
 let currentUserAccountID;
 let currentUserEmail;
@@ -208,6 +207,35 @@ function reconnectApp(updateIDFrom = 0) {
     });
 }
 
+/**
+ * Fetches data when the app will call reconnectApp without params for the last time. This is a separate function
+ * because it will follow patterns that are not recommended so we can be sure we're not putting the app in a unusable
+ * state because of race conditions between reconnectApp and other pusher updates being applied at the same time.
+ * @return {Promise}
+ */
+function finalReconnectAppAfterActivatingReliableUpdates() {
+    console.debug(`[OnyxUpdates] Executing last reconnect app with promise`);
+    return getPolicyParamsForOpenOrReconnect().then((policyParams) => {
+        const params = {...policyParams};
+
+        // When the app reconnects we do a fast "sync" of the LHN and only return chats that have new messages. We achieve this by sending the most recent reportActionID.
+        // we have locally. And then only update the user about chats with messages that have occurred after that reportActionID.
+        //
+        // - Look through the local report actions and reports to find the most recently modified report action or report.
+        // - We send this to the server so that it can compute which new chats the user needs to see and return only those as an optimization.
+        Timing.start(CONST.TIMING.CALCULATE_MOST_RECENT_LAST_MODIFIED_ACTION);
+        params.mostRecentReportActionLastModified = ReportActionsUtils.getMostRecentReportActionLastModified();
+        Timing.end(CONST.TIMING.CALCULATE_MOST_RECENT_LAST_MODIFIED_ACTION, '', 500);
+
+        // It is SUPER BAD FORM to return promises from action methods.
+        // DO NOT FOLLOW THIS PATTERN!!!!!
+        // It was absolutely necessary in order to not break the app while migrating to the new reliable updates pattern. This method will be removed
+        // as soon as we have everyone migrated to the reliableUpdate beta.
+        // eslint-disable-next-line rulesdir/no-api-side-effects-method
+        return API.makeRequestWithSideEffects('ReconnectApp', params, getOnyxDataForOpenOrReconnect());
+    });
+}
+
 /**
  * Fetches data when the client has discovered it missed some Onyx updates from the server
  * @param {Number} [updateIDFrom] the ID of the Onyx update that we want to start fetching from
@@ -231,48 +259,6 @@ function getMissingOnyxUpdates(updateIDFrom = 0, updateIDTo = 0) {
     );
 }
 
-// The next 40ish lines of code are used for detecting when there is a gap of OnyxUpdates between what was last applied to the client and the updates the server has.
-// When a gap is detected, the missing updates are fetched from the API.
-
-// These key needs to be separate from ONYXKEYS.ONYX_UPDATES_FROM_SERVER so that it can be updated without triggering the callback when the server IDs are updated
-let lastUpdateIDAppliedToClient = 0;
-Onyx.connect({
-    key: ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT,
-    callback: (val) => (lastUpdateIDAppliedToClient = val),
-});
-
-Onyx.connect({
-    key: ONYXKEYS.ONYX_UPDATES_FROM_SERVER,
-    callback: (val) => {
-        if (!val) {
-            return;
-        }
-
-        const {lastUpdateIDFromServer, previousUpdateIDFromServer} = val;
-        console.debug('[OnyxUpdates] Received lastUpdateID from server', lastUpdateIDFromServer);
-        console.debug('[OnyxUpdates] Received previousUpdateID from server', previousUpdateIDFromServer);
-        console.debug('[OnyxUpdates] Last update ID applied to the client', lastUpdateIDAppliedToClient);
-
-        // If the previous update from the server does not match the last update the client got, then the client is missing some updates.
-        // getMissingOnyxUpdates will fetch updates starting from the last update this client got and going to the last update the server sent.
-        if (lastUpdateIDAppliedToClient && previousUpdateIDFromServer && lastUpdateIDAppliedToClient < previousUpdateIDFromServer) {
-            console.debug('[OnyxUpdates] Gap detected in update IDs so fetching incremental updates');
-            Log.info('Gap detected in update IDs from server so fetching incremental updates', true, {
-                lastUpdateIDFromServer,
-                previousUpdateIDFromServer,
-                lastUpdateIDAppliedToClient,
-            });
-            SequentialQueue.pause();
-            getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastUpdateIDFromServer).finally(SequentialQueue.unpause);
-        }
-
-        if (lastUpdateIDFromServer > lastUpdateIDAppliedToClient) {
-            // Update this value so that it matches what was just received from the server
-            Onyx.merge(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, lastUpdateIDFromServer || 0);
-        }
-    },
-});
-
 /**
  * This promise is used so that deeplink component know when a transition is end.
  * This is necessary because we want to begin deeplink redirection after the transition is end.
@@ -484,4 +470,6 @@ export {
     beginDeepLinkRedirect,
     beginDeepLinkRedirectAfterTransition,
     createWorkspaceAndNavigateToIt,
+    getMissingOnyxUpdates,
+    finalReconnectAppAfterActivatingReliableUpdates,
 };