Merge pull request #24607 from Expensify/tgolen-reorder-onyxupdates

[No QA] Ensure that no other updates are made to Onyx while missing Onyx updates are fetched and applied

src/libs/API.js

@@ -36,9 +36,8 @@ Request.use(Middleware.SaveResponseInOnyx);
  * @param {Object} [onyxData.optimisticData] - Onyx instructions that will be passed to Onyx.update() before the request is made.
  * @param {Object} [onyxData.successData] - Onyx instructions that will be passed to Onyx.update() when the response has jsonCode === 200.
  * @param {Object} [onyxData.failureData] - Onyx instructions that will be passed to Onyx.update() when the response has jsonCode !== 200.
- * @param {Boolean} [prioritizeRequest] Whether or not the request should be prioritized at the front of the queue or placed onto the back of the queue
  */
-function write(command, apiCommandParameters = {}, onyxData = {}, prioritizeRequest = false) {
+function write(command, apiCommandParameters = {}, onyxData = {}) {
     Log.info('Called API write', false, {command, ...apiCommandParameters});
 
     // Optimistically update Onyx
@@ -71,7 +70,7 @@ function write(command, apiCommandParameters = {}, onyxData = {}, prioritizeRequ
     };
 
     // Write commands can be saved and retried, so push it to the SequentialQueue
-    SequentialQueue.push(request, prioritizeRequest);
+    SequentialQueue.push(request);
 }
 
 /**

src/libs/Network/SequentialQueue.js

@@ -19,6 +19,7 @@ resolveIsReadyPromise();
 
 let isSequentialQueueRunning = false;
 let currentRequest = null;
+let isQueuePaused = false;
 
 /**
  * Process any persisted requests, when online, one at a time until the queue is empty.
@@ -30,6 +31,11 @@ let currentRequest = null;
  * @returns {Promise}
  */
 function process() {
+    // When the queue is paused, return early. This prevents any new requests from happening. The queue will be flushed again when the queue is unpaused.
+    if (isQueuePaused) {
+        return Promise.resolve();
+    }
+
     const persistedRequests = PersistedRequests.getAll();
     if (_.isEmpty(persistedRequests) || NetworkStore.isOffline()) {
         return Promise.resolve();
@@ -57,6 +63,11 @@ function process() {
 }
 
 function flush() {
+    // When the queue is paused, return early. This will keep an requests in the queue and they will get flushed again when the queue is unpaused
+    if (isQueuePaused) {
+        return;
+    }
+
     if (isSequentialQueueRunning || _.isEmpty(PersistedRequests.getAll())) {
         return;
     }
@@ -101,11 +112,10 @@ NetworkStore.onReconnection(flush);
 
 /**
  * @param {Object} request
- * @param {Boolean} [front] whether or not the request should be placed in the front of the queue
  */
-function push(request, front = false) {
+function push(request) {
     // Add request to Persisted Requests so that it can be retried if it fails
-    PersistedRequests.save([request], front);
+    PersistedRequests.save([request]);
 
     // If we are offline we don't need to trigger the queue to empty as it will happen when we come back online
     if (NetworkStore.isOffline()) {
@@ -139,4 +149,30 @@ function waitForIdle() {
     return isReadyPromise;
 }
 
-export {flush, getCurrentRequest, isRunning, push, waitForIdle};
+/**
+ * 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/actions/App.js

@@ -18,6 +18,7 @@ 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;
@@ -211,20 +212,22 @@ function reconnectApp(updateIDFrom = 0) {
  * 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
  * @param {Number} [updateIDTo] the ID of the Onyx update that we want to fetch up to
+ * @return {Promise}
  */
 function getMissingOnyxUpdates(updateIDFrom = 0, updateIDTo = 0) {
     console.debug(`[OnyxUpdates] Fetching missing updates updateIDFrom: ${updateIDFrom} and updateIDTo: ${updateIDTo}`);
 
-    API.write(
+    // It is SUPER BAD FORM to return promises from action methods.
+    // DO NOT FOLLOW THIS PATTERN!!!!!
+    // It was absolutely necessary in order to block OnyxUpdates while fetching the missing updates from the server or else the udpates aren't applied in the proper order.
+    // eslint-disable-next-line rulesdir/no-api-side-effects-method
+    return API.makeRequestWithSideEffects(
         'GetMissingOnyxMessages',
         {
             updateIDFrom,
             updateIDTo,
         },
         getOnyxDataForOpenOrReconnect(),
-
-        // Set this to true so that the request will be prioritized at the front of the sequential queue
-        true,
     );
 }
 
@@ -259,7 +262,8 @@ Onyx.connect({
                 previousUpdateIDFromServer,
                 lastUpdateIDAppliedToClient,
             });
-            getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastUpdateIDFromServer);
+            SequentialQueue.pause();
+            getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastUpdateIDFromServer).finally(SequentialQueue.unpause);
         }
 
         if (lastUpdateIDFromServer > lastUpdateIDAppliedToClient) {

src/libs/actions/PersistedRequests.js

@@ -15,11 +15,10 @@ function clear() {
 
 /**
  * @param {Array} requestsToPersist
- * @param {Boolean} [front] whether or not the request should go in the front of the queue
  */
-function save(requestsToPersist, front = false) {
+function save(requestsToPersist) {
     if (persistedRequests.length) {
-        persistedRequests = front ? persistedRequests.unshift(...requestsToPersist) : persistedRequests.concat(requestsToPersist);
+        persistedRequests = persistedRequests.concat(requestsToPersist);
     } else {
         persistedRequests = requestsToPersist;
     }