Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Aug 16, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/libs/API.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

/**
Expand Down
44 changes: 40 additions & 4 deletions src/libs/Network/SequentialQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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};
14 changes: 9 additions & 5 deletions src/libs/actions/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
);
}

Expand Down Expand Up @@ -259,7 +262,8 @@ Onyx.connect({
previousUpdateIDFromServer,
lastUpdateIDAppliedToClient,
});
getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastUpdateIDFromServer);
SequentialQueue.pause();
getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastUpdateIDFromServer).then(SequentialQueue.unpause);
danieldoglas marked this conversation as resolved.
Show resolved Hide resolved
}

if (lastUpdateIDFromServer > lastUpdateIDAppliedToClient) {
Expand Down
5 changes: 2 additions & 3 deletions src/libs/actions/PersistedRequests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down