From d32cd8b94179e3ddcccb8e55dd21f88dba0f2cfa Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 26 Mar 2024 13:09:49 +0100 Subject: [PATCH 01/26] defer updates if missing updates are fetched --- src/libs/actions/OnyxUpdateManager.ts | 78 +++++++++++++++++++++++++-- 1 file changed, 75 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 9c6f30cc5e9e..e33d94cd574d 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -3,6 +3,7 @@ import Log from '@libs/Log'; import * as SequentialQueue from '@libs/Network/SequentialQueue'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; +import type {OnyxUpdatesFromServer} from '@src/types/onyx'; import * as App from './App'; import * as OnyxUpdates from './OnyxUpdates'; @@ -27,6 +28,8 @@ Onyx.connect({ callback: (value) => (lastUpdateIDAppliedToClient = value), }); +let deferredUpdates: OnyxUpdatesFromServer[] = []; + export default () => { console.debug('[OnyxUpdateManager] Listening for updates from the server'); Onyx.connect({ @@ -68,30 +71,99 @@ export default () => { SequentialQueue.pause(); let canUnpauseQueuePromise; + let updateAfterReconnectApp: OnyxUpdatesFromServer | undefined; + // The flow below is setting the promise to a reconnect app to address flow (1) explained above. if (!lastUpdateIDAppliedToClient) { Log.info('Client has not gotten reliable updates before so reconnecting the app to start the process'); + updateAfterReconnectApp = updateParams; + // 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(); } 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 = deferredUpdates.length; + // Add the new update to the deferred updates + deferredUpdates.push(updateParams); + + // If there are already deferred updates, then 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) - lastUpdateIDAppliedToClient} so fetching incremental updates`); Log.info('Gap detected in update IDs from server so fetching incremental updates', true, { lastUpdateIDFromServer, previousUpdateIDFromServer, lastUpdateIDAppliedToClient, }); + + // Trigger "getMissingOnyxUpdates" to get the missing updates from the server canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer); } - canUnpauseQueuePromise.finally(() => { - OnyxUpdates.apply(updateParams).finally(() => { + function applyDeferredUpdates() { + // If lastUpdateIDAppliedToClient is null, then no updates were applied, + // therefore something must have gone wrong and we should handle the problem. + if (!lastUpdateIDAppliedToClient) { + // TODO: Handle this case + return; + } + + function unpauseQueueAndReset() { console.debug('[OnyxUpdateManager] Done applying all updates'); Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null); SequentialQueue.unpause(); + } + + // If "updateAfterReconnectApp" is set, it means that case (1) was triggered and we only need to apply the one update, not the deferred ones. + if (updateAfterReconnectApp) { + OnyxUpdates.apply(updateAfterReconnectApp).finally(unpauseQueueAndReset); + updateAfterReconnectApp = undefined; + return; + } + + // If case (2) was triggered, then we need to sort the deferred updates by lastUpdateID and apply them in order. + const sortedDeferredUpdates = deferredUpdates.sort((a, b) => { + const aLastUpdateID = Number(a.lastUpdateID) || 0; + const bLastUpdateID = Number(b.lastUpdateID) || 0; + + return aLastUpdateID - bLastUpdateID; }); - }); + + // In order for the updates to be applied correctly in order, we need to check + // if there are any gaps in the updates that we received from the server + + // the deferred ones that were pushed before. + function validateDeferredUpdates(): boolean { + const earliestDeferredUpdateId = Number(sortedDeferredUpdates[0].lastUpdateID) || 0; + + if (lastUpdateIDAppliedToClient >= earliestDeferredUpdateId) { + console.error("The client's lastUpdateID is greater than or equal to the earliest deferred update's lastUpdateID"); + + return false; + } + + return true; + } + + if (!validateDeferredUpdates()) { + // Re-Trigger "getMissingOnyxUpdates" because we detected a gap in the fetched + deferred updates + canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer).finally(applyDeferredUpdates); + return; + } + + console.log({sortedDeferredUpdates}); + + Promise.all(sortedDeferredUpdates.map((update) => OnyxUpdates.apply(update))).finally(() => { + deferredUpdates = []; + unpauseQueueAndReset(); + }); + } + + canUnpauseQueuePromise.finally(applyDeferredUpdates); }, }); }; From e41558ec4b5b49af88e96e8e654bf89be0e64f03 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 26 Mar 2024 14:22:42 +0100 Subject: [PATCH 02/26] simplify --- src/libs/actions/OnyxUpdateManager.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index e33d94cd574d..394ddaef6cff 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -28,8 +28,6 @@ Onyx.connect({ callback: (value) => (lastUpdateIDAppliedToClient = value), }); -let deferredUpdates: OnyxUpdatesFromServer[] = []; - export default () => { console.debug('[OnyxUpdateManager] Listening for updates from the server'); Onyx.connect({ @@ -72,6 +70,7 @@ export default () => { let canUnpauseQueuePromise; let updateAfterReconnectApp: OnyxUpdatesFromServer | undefined; + const deferredUpdates: OnyxUpdatesFromServer[] = []; // The flow below is setting the promise to a reconnect app to address flow (1) explained above. if (!lastUpdateIDAppliedToClient) { @@ -122,7 +121,6 @@ export default () => { // If "updateAfterReconnectApp" is set, it means that case (1) was triggered and we only need to apply the one update, not the deferred ones. if (updateAfterReconnectApp) { OnyxUpdates.apply(updateAfterReconnectApp).finally(unpauseQueueAndReset); - updateAfterReconnectApp = undefined; return; } @@ -157,10 +155,7 @@ export default () => { console.log({sortedDeferredUpdates}); - Promise.all(sortedDeferredUpdates.map((update) => OnyxUpdates.apply(update))).finally(() => { - deferredUpdates = []; - unpauseQueueAndReset(); - }); + Promise.all(sortedDeferredUpdates.map((update) => OnyxUpdates.apply(update))).finally(unpauseQueueAndReset); } canUnpauseQueuePromise.finally(applyDeferredUpdates); From 931527cc947ea8327c42cc90f3a9956f92bbab6b Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 26 Mar 2024 14:26:28 +0100 Subject: [PATCH 03/26] move deferred updates out of callback --- src/libs/actions/OnyxUpdateManager.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 394ddaef6cff..3a8e6d97876f 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -28,6 +28,9 @@ Onyx.connect({ callback: (value) => (lastUpdateIDAppliedToClient = value), }); +let deferedUpdateBeforeReconnect: OnyxUpdatesFromServer | undefined; +let deferredUpdates: OnyxUpdatesFromServer[] = []; + export default () => { console.debug('[OnyxUpdateManager] Listening for updates from the server'); Onyx.connect({ @@ -69,14 +72,11 @@ export default () => { SequentialQueue.pause(); let canUnpauseQueuePromise; - let updateAfterReconnectApp: OnyxUpdatesFromServer | undefined; - const deferredUpdates: OnyxUpdatesFromServer[] = []; - // The flow below is setting the promise to a reconnect app to address flow (1) explained above. if (!lastUpdateIDAppliedToClient) { Log.info('Client has not gotten reliable updates before so reconnecting the app to start the process'); - updateAfterReconnectApp = updateParams; + deferedUpdateBeforeReconnect = updateParams; // 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(); @@ -119,8 +119,9 @@ export default () => { } // If "updateAfterReconnectApp" is set, it means that case (1) was triggered and we only need to apply the one update, not the deferred ones. - if (updateAfterReconnectApp) { - OnyxUpdates.apply(updateAfterReconnectApp).finally(unpauseQueueAndReset); + if (deferedUpdateBeforeReconnect) { + OnyxUpdates.apply(deferedUpdateBeforeReconnect).finally(unpauseQueueAndReset); + deferedUpdateBeforeReconnect = undefined; return; } @@ -155,7 +156,10 @@ export default () => { console.log({sortedDeferredUpdates}); - Promise.all(sortedDeferredUpdates.map((update) => OnyxUpdates.apply(update))).finally(unpauseQueueAndReset); + Promise.all(sortedDeferredUpdates.map((update) => OnyxUpdates.apply(update))).finally(() => { + unpauseQueueAndReset(); + deferredUpdates = []; + }); } canUnpauseQueuePromise.finally(applyDeferredUpdates); From 5088441c94e671a235341fd83caaa6e3bad8cff6 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 26 Mar 2024 14:31:48 +0100 Subject: [PATCH 04/26] simplify comments --- src/libs/actions/OnyxUpdateManager.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 3a8e6d97876f..84d6af16359a 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -100,10 +100,11 @@ export default () => { lastUpdateIDAppliedToClient, }); - // Trigger "getMissingOnyxUpdates" to get the missing updates from the server + // Get the missing Onyx updates from the server canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer); } + // This function will apply the deferred updates in order after the missing updates are fetched function applyDeferredUpdates() { // If lastUpdateIDAppliedToClient is null, then no updates were applied, // therefore something must have gone wrong and we should handle the problem. @@ -148,8 +149,8 @@ export default () => { return true; } + // If we detect a gap in the fetched + deferred updates, re-fetch the missing updates if (!validateDeferredUpdates()) { - // Re-Trigger "getMissingOnyxUpdates" because we detected a gap in the fetched + deferred updates canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer).finally(applyDeferredUpdates); return; } From 9d818d3621e4473b085b9c3da6b969f59bb35f4a Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 26 Mar 2024 19:10:32 +0100 Subject: [PATCH 05/26] fix: apply deferred updates in order --- src/libs/actions/OnyxUpdateManager.ts | 43 ++++++++++++++------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 84d6af16359a..6aebd92476b6 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -3,7 +3,7 @@ import Log from '@libs/Log'; import * as SequentialQueue from '@libs/Network/SequentialQueue'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {OnyxUpdatesFromServer} from '@src/types/onyx'; +import type {OnyxUpdatesFromServer, Response} from '@src/types/onyx'; import * as App from './App'; import * as OnyxUpdates from './OnyxUpdates'; @@ -28,7 +28,7 @@ Onyx.connect({ callback: (value) => (lastUpdateIDAppliedToClient = value), }); -let deferedUpdateBeforeReconnect: OnyxUpdatesFromServer | undefined; +let deferredUpdateBeforeReconnect: OnyxUpdatesFromServer | undefined; let deferredUpdates: OnyxUpdatesFromServer[] = []; export default () => { @@ -70,13 +70,13 @@ export default () => { // applied in their correct and specific order. If this queue was not paused, then there would be a lot of // onyx data being applied while we are fetching the missing updates and that would put them all out of order. SequentialQueue.pause(); - let canUnpauseQueuePromise; + let canUnpauseQueuePromise: Promise; // The flow below is setting the promise to a reconnect app to address flow (1) explained above. if (!lastUpdateIDAppliedToClient) { Log.info('Client has not gotten reliable updates before so reconnecting the app to start the process'); - deferedUpdateBeforeReconnect = updateParams; + deferredUpdateBeforeReconnect = updateParams; // 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(); @@ -120,9 +120,9 @@ export default () => { } // If "updateAfterReconnectApp" is set, it means that case (1) was triggered and we only need to apply the one update, not the deferred ones. - if (deferedUpdateBeforeReconnect) { - OnyxUpdates.apply(deferedUpdateBeforeReconnect).finally(unpauseQueueAndReset); - deferedUpdateBeforeReconnect = undefined; + if (deferredUpdateBeforeReconnect) { + OnyxUpdates.apply(deferredUpdateBeforeReconnect).finally(unpauseQueueAndReset); + deferredUpdateBeforeReconnect = undefined; return; } @@ -134,29 +134,30 @@ export default () => { return aLastUpdateID - bLastUpdateID; }); - // In order for the updates to be applied correctly in order, we need to check - // if there are any gaps in the updates that we received from the server + - // the deferred ones that were pushed before. - function validateDeferredUpdates(): boolean { - const earliestDeferredUpdateId = Number(sortedDeferredUpdates[0].lastUpdateID) || 0; - - if (lastUpdateIDAppliedToClient >= earliestDeferredUpdateId) { - console.error("The client's lastUpdateID is greater than or equal to the earliest deferred update's lastUpdateID"); - - return false; + // In order for the deferred updates to be applied correctly in order, + // we need to check if there are any gaps deferred updates. + function validateUpdatesAreChained(): boolean { + for (let i = 0; i < sortedDeferredUpdates.length - 1; i++) { + const reversedDeferredUpdates = sortedDeferredUpdates.toReversed(); + const currentUpdate = reversedDeferredUpdates[i]; + const previousUpdate = reversedDeferredUpdates[i + 1]; + + if (Number(previousUpdate.lastUpdateID) !== Number(currentUpdate.previousUpdateID)) { + return false; + } } return true; } // If we detect a gap in the fetched + deferred updates, re-fetch the missing updates - if (!validateDeferredUpdates()) { - canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer).finally(applyDeferredUpdates); + if (!validateUpdatesAreChained()) { + const lastDeferedUpdateId = Number(sortedDeferredUpdates.at(-1)?.lastUpdateID) ?? 0; + + canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastDeferedUpdateId).finally(applyDeferredUpdates); return; } - console.log({sortedDeferredUpdates}); - Promise.all(sortedDeferredUpdates.map((update) => OnyxUpdates.apply(update))).finally(() => { unpauseQueueAndReset(); deferredUpdates = []; From f21ea78ea5b2bfc1299c4fad47a12f31de2fc694 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 27 Mar 2024 11:44:23 +0100 Subject: [PATCH 06/26] simplify and improve applying deferred updates --- src/libs/actions/OnyxUpdateManager.ts | 67 +++++++++++++-------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 6aebd92476b6..6b90f37ccc1f 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -29,7 +29,7 @@ Onyx.connect({ }); let deferredUpdateBeforeReconnect: OnyxUpdatesFromServer | undefined; -let deferredUpdates: OnyxUpdatesFromServer[] = []; +let deferredUpdates: Record = {}; export default () => { console.debug('[OnyxUpdateManager] Listening for updates from the server'); @@ -84,9 +84,9 @@ export default () => { // 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 = deferredUpdates.length; + const existingDeferredUpdatesCount = Object.keys(deferredUpdates).length; // Add the new update to the deferred updates - deferredUpdates.push(updateParams); + deferredUpdates[Number(updateParams.lastUpdateID)] = updateParams; // If there are already deferred updates, then we don't need to fetch the missing updates again if (existingDeferredUpdatesCount > 0) { @@ -104,13 +104,12 @@ export default () => { canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer); } - // This function will apply the deferred updates in order after the missing updates are fetched + // This function will apply the deferred updates in order after the missing updates are fetched and applied function applyDeferredUpdates() { - // If lastUpdateIDAppliedToClient is null, then no updates were applied, - // therefore something must have gone wrong and we should handle the problem. - if (!lastUpdateIDAppliedToClient) { - // TODO: Handle this case - return; + // If "updateAfterReconnectApp" is set, it means that case (1) was triggered and we need to apply the update that was queued before the reconnectApp + if (deferredUpdateBeforeReconnect) { + OnyxUpdates.apply(deferredUpdateBeforeReconnect); + deferredUpdateBeforeReconnect = undefined; } function unpauseQueueAndReset() { @@ -119,46 +118,46 @@ export default () => { SequentialQueue.unpause(); } - // If "updateAfterReconnectApp" is set, it means that case (1) was triggered and we only need to apply the one update, not the deferred ones. - if (deferredUpdateBeforeReconnect) { - OnyxUpdates.apply(deferredUpdateBeforeReconnect).finally(unpauseQueueAndReset); - deferredUpdateBeforeReconnect = undefined; + const deferredUpdateValues = Object.values(deferredUpdates); + // If there are no deferred updates, then we can just unpause the queue and return + if (deferredUpdateValues.length === 0) { + unpauseQueueAndReset(); return; } - // If case (2) was triggered, then we need to sort the deferred updates by lastUpdateID and apply them in order. - const sortedDeferredUpdates = deferredUpdates.sort((a, b) => { - const aLastUpdateID = Number(a.lastUpdateID) || 0; - const bLastUpdateID = Number(b.lastUpdateID) || 0; - - return aLastUpdateID - bLastUpdateID; - }); - + let lastValidDeferredUpdateID = 0; // In order for the deferred updates to be applied correctly in order, // we need to check if there are any gaps deferred updates. - function validateUpdatesAreChained(): boolean { - for (let i = 0; i < sortedDeferredUpdates.length - 1; i++) { - const reversedDeferredUpdates = sortedDeferredUpdates.toReversed(); - const currentUpdate = reversedDeferredUpdates[i]; - const previousUpdate = reversedDeferredUpdates[i + 1]; - - if (Number(previousUpdate.lastUpdateID) !== Number(currentUpdate.previousUpdateID)) { - return false; + function validateDeferredUpdatesAreChained(): boolean { + let isFirst = true; + for (const update of deferredUpdateValues) { + // If it's the first one, we need to skip it since we won't have the previousUpdateID + // if the previousUpdateID key exists, then we can just move on since there's no gap + if (isFirst || deferredUpdates[Number(update.previousUpdateID)]) { + lastValidDeferredUpdateID = Number(update.lastUpdateID); + isFirst = false; + // eslint-disable-next-line no-continue + continue; } + + return false; } return true; } - // If we detect a gap in the fetched + deferred updates, re-fetch the missing updates - if (!validateUpdatesAreChained()) { - const lastDeferedUpdateId = Number(sortedDeferredUpdates.at(-1)?.lastUpdateID) ?? 0; + // If we detect a gap in the deferred updates, re-fetch the missing updates. + if (!validateDeferredUpdatesAreChained()) { + // lastUpdateIDAppliedToClient should technically not be possible to be null, after the missing updates have been applied, therefore we don't need to handle this case + if (!lastUpdateIDAppliedToClient) { + return; + } - canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastDeferedUpdateId).finally(applyDeferredUpdates); + canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastValidDeferredUpdateID).finally(applyDeferredUpdates); return; } - Promise.all(sortedDeferredUpdates.map((update) => OnyxUpdates.apply(update))).finally(() => { + Promise.all(deferredUpdateValues.map((update) => OnyxUpdates.apply(update))).finally(() => { unpauseQueueAndReset(); deferredUpdates = []; }); From 3cc017e89dca35df3627b7f4cfdf50040b382137 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 27 Mar 2024 11:59:46 +0100 Subject: [PATCH 07/26] check if deferred updates are older than the currently applied one --- src/libs/actions/OnyxUpdateManager.ts | 33 +++++++++++++++++++-------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 6b90f37ccc1f..bbca6c6c007e 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -106,21 +106,39 @@ export default () => { // This function will apply the deferred updates in order after the missing updates are fetched and applied function applyDeferredUpdates() { + // It should not be possible for lastUpdateIDAppliedToClient to be null, after the missing updates have been applied, therefore we don't need to handle this case + if (!lastUpdateIDAppliedToClient) { + return; + } + // If "updateAfterReconnectApp" is set, it means that case (1) was triggered and we need to apply the update that was queued before the reconnectApp if (deferredUpdateBeforeReconnect) { OnyxUpdates.apply(deferredUpdateBeforeReconnect); deferredUpdateBeforeReconnect = undefined; } + // We only want to apply deferred updates that are newer than the last update that was applied to the client. + // At this point, the missing updates from "GetMissingOnyxUpdates" have been applied already, so we can safely filter out. + const pendingDeferredUpdates = Object.fromEntries( + Object.entries(deferredUpdates).filter(([lastUpdateID]) => { + // Same as above, it should not be possible for lastUpdateIDAppliedToClient to be null. + // Still, if so we want to keep the deferred update in the list + if (!lastUpdateIDAppliedToClient) { + return true; + } + return (Number(lastUpdateID) ?? 0) > lastUpdateIDAppliedToClient; + }), + ); + const pendingDeferredUpdateValues = Object.values(pendingDeferredUpdates); + function unpauseQueueAndReset() { console.debug('[OnyxUpdateManager] Done applying all updates'); Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null); SequentialQueue.unpause(); } - const deferredUpdateValues = Object.values(deferredUpdates); // If there are no deferred updates, then we can just unpause the queue and return - if (deferredUpdateValues.length === 0) { + if (pendingDeferredUpdateValues.length === 0) { unpauseQueueAndReset(); return; } @@ -130,10 +148,10 @@ export default () => { // we need to check if there are any gaps deferred updates. function validateDeferredUpdatesAreChained(): boolean { let isFirst = true; - for (const update of deferredUpdateValues) { + for (const update of pendingDeferredUpdateValues) { // If it's the first one, we need to skip it since we won't have the previousUpdateID // if the previousUpdateID key exists, then we can just move on since there's no gap - if (isFirst || deferredUpdates[Number(update.previousUpdateID)]) { + if (isFirst || pendingDeferredUpdates[Number(update.previousUpdateID)]) { lastValidDeferredUpdateID = Number(update.lastUpdateID); isFirst = false; // eslint-disable-next-line no-continue @@ -148,16 +166,11 @@ export default () => { // If we detect a gap in the deferred updates, re-fetch the missing updates. if (!validateDeferredUpdatesAreChained()) { - // lastUpdateIDAppliedToClient should technically not be possible to be null, after the missing updates have been applied, therefore we don't need to handle this case - if (!lastUpdateIDAppliedToClient) { - return; - } - canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastValidDeferredUpdateID).finally(applyDeferredUpdates); return; } - Promise.all(deferredUpdateValues.map((update) => OnyxUpdates.apply(update))).finally(() => { + Promise.all(pendingDeferredUpdateValues.map((update) => OnyxUpdates.apply(update))).finally(() => { unpauseQueueAndReset(); deferredUpdates = []; }); From ce9027ccc46569e3487f8520f6b4597add1905ee Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Wed, 27 Mar 2024 12:09:14 +0100 Subject: [PATCH 08/26] improve code --- src/libs/actions/OnyxUpdateManager.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index bbca6c6c007e..1297db104e05 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -137,7 +137,8 @@ export default () => { SequentialQueue.unpause(); } - // If there are no deferred updates, then we can just unpause the queue and return + // If there are no remaining deferred updates after filtering out outdated ones, + // we can just unpause the queue and return if (pendingDeferredUpdateValues.length === 0) { unpauseQueueAndReset(); return; @@ -146,7 +147,7 @@ export default () => { let lastValidDeferredUpdateID = 0; // In order for the deferred updates to be applied correctly in order, // we need to check if there are any gaps deferred updates. - function validateDeferredUpdatesAreChained(): boolean { + function areDeferredUpdatesChained(): boolean { let isFirst = true; for (const update of pendingDeferredUpdateValues) { // If it's the first one, we need to skip it since we won't have the previousUpdateID @@ -165,7 +166,7 @@ export default () => { } // If we detect a gap in the deferred updates, re-fetch the missing updates. - if (!validateDeferredUpdatesAreChained()) { + if (!areDeferredUpdatesChained()) { canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastValidDeferredUpdateID).finally(applyDeferredUpdates); return; } From c3034e01095e01de08122d216ab16bf729d841c0 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Fri, 29 Mar 2024 13:29:18 +0100 Subject: [PATCH 09/26] fix: refetching logic --- src/libs/actions/OnyxUpdateManager.ts | 68 +++++++++++++++++++-------- 1 file changed, 49 insertions(+), 19 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 1297db104e05..2dd0c3df54f6 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -28,8 +28,10 @@ Onyx.connect({ callback: (value) => (lastUpdateIDAppliedToClient = value), }); +type DeferredUpdatesDictionary = Record; + let deferredUpdateBeforeReconnect: OnyxUpdatesFromServer | undefined; -let deferredUpdates: Record = {}; +let deferredUpdates: DeferredUpdatesDictionary = {}; export default () => { console.debug('[OnyxUpdateManager] Listening for updates from the server'); @@ -105,7 +107,7 @@ export default () => { } // This function will apply the deferred updates in order after the missing updates are fetched and applied - function applyDeferredUpdates() { + function validateAndApplyDeferredUpdates() { // It should not be possible for lastUpdateIDAppliedToClient to be null, after the missing updates have been applied, therefore we don't need to handle this case if (!lastUpdateIDAppliedToClient) { return; @@ -144,40 +146,68 @@ export default () => { return; } - let lastValidDeferredUpdateID = 0; + type DetectGapAndSplitResult = {beforeGap: DeferredUpdatesDictionary; afterGap: DeferredUpdatesDictionary; latestMissingUpdateID: number | undefined}; // In order for the deferred updates to be applied correctly in order, // we need to check if there are any gaps deferred updates. - function areDeferredUpdatesChained(): boolean { + function detectGapAndSplit(): DetectGapAndSplitResult { let isFirst = true; + const beforeGap: DeferredUpdatesDictionary = {}; + let afterGap: DeferredUpdatesDictionary = pendingDeferredUpdates; + for (const update of pendingDeferredUpdateValues) { - // If it's the first one, we need to skip it since we won't have the previousUpdateID - // if the previousUpdateID key exists, then we can just move on since there's no gap - if (isFirst || pendingDeferredUpdates[Number(update.previousUpdateID)]) { - lastValidDeferredUpdateID = Number(update.lastUpdateID); - isFirst = false; - // eslint-disable-next-line no-continue - continue; + // If it's the first update, we need to check that the previousUpdateID of the fetched update is the same as the lastUpdateIDAppliedToClient + // If any update's previousUpdateID doesn't match the lastUpdateID from the previous update, there's a gap in the deferred updates + if (isFirst && update.previousUpdateID !== lastUpdateIDAppliedToClient) { + break; + } + + if (pendingDeferredUpdates[Number(update.previousUpdateID)]) { + beforeGap[Number(update.lastUpdateID)] = update; + // eslint-disable-next-line @typescript-eslint/no-unused-vars + afterGap = Object.fromEntries(Object.entries(afterGap).filter(([_key, u]) => u.lastUpdateID === update.lastUpdateID)); + } else { + break; } - return false; + isFirst = false; + } + + let latestMissingUpdateID: number | undefined; + const afterGapValues = Object.values(afterGap); + if (afterGapValues.length > 0 && afterGapValues[0]) { + latestMissingUpdateID = Number(afterGapValues[0].previousUpdateID) || 0; } - return true; + return {beforeGap, afterGap, latestMissingUpdateID}; } - // If we detect a gap in the deferred updates, re-fetch the missing updates. - if (!areDeferredUpdatesChained()) { - canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, lastValidDeferredUpdateID).finally(applyDeferredUpdates); + const {beforeGap, afterGap, latestMissingUpdateID} = detectGapAndSplit(); + + const applyUpdates = (updates: DeferredUpdatesDictionary) => Promise.all(Object.values(updates).map((update) => OnyxUpdates.apply(update))); + + // If we detect a gap in the deferred updates, only apply the deferred updates before the gap, + // re-fetch the missing updates and then apply thedeferred updates again. + if (latestMissingUpdateID) { + applyUpdates(beforeGap).finally(() => { + // Same as above, we can ignore this case because + // it should not be possible for lastUpdateIDAppliedToClient to be null. + if (!lastUpdateIDAppliedToClient) { + return; + } + + deferredUpdates = afterGap; + canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID).finally(validateAndApplyDeferredUpdates); + }); return; } - Promise.all(pendingDeferredUpdateValues.map((update) => OnyxUpdates.apply(update))).finally(() => { + applyUpdates(pendingDeferredUpdates).finally(() => { unpauseQueueAndReset(); - deferredUpdates = []; + deferredUpdates = {}; }); } - canUnpauseQueuePromise.finally(applyDeferredUpdates); + canUnpauseQueuePromise.finally(validateAndApplyDeferredUpdates); }, }); }; From b9afdf359158d64fb45fad3ae46b96c94d7b79cc Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Fri, 29 Mar 2024 15:07:11 +0100 Subject: [PATCH 10/26] fix: comments and logic --- src/libs/actions/OnyxUpdateManager.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 2dd0c3df54f6..06f61940cb91 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -106,7 +106,7 @@ export default () => { canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer); } - // This function will apply the deferred updates in order after the missing updates are fetched and applied + // This function will check for gaps in the deferred updates and apply the updates in order after the missing updates are fetched and applied function validateAndApplyDeferredUpdates() { // It should not be possible for lastUpdateIDAppliedToClient to be null, after the missing updates have been applied, therefore we don't need to handle this case if (!lastUpdateIDAppliedToClient) { @@ -135,6 +135,7 @@ export default () => { function unpauseQueueAndReset() { console.debug('[OnyxUpdateManager] Done applying all updates'); + deferredUpdates = {}; Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null); SequentialQueue.unpause(); } @@ -186,7 +187,7 @@ export default () => { const applyUpdates = (updates: DeferredUpdatesDictionary) => Promise.all(Object.values(updates).map((update) => OnyxUpdates.apply(update))); // If we detect a gap in the deferred updates, only apply the deferred updates before the gap, - // re-fetch the missing updates and then apply thedeferred updates again. + // re-fetch the missing updates and then apply the deferred updates again. if (latestMissingUpdateID) { applyUpdates(beforeGap).finally(() => { // Same as above, we can ignore this case because @@ -196,15 +197,14 @@ export default () => { } deferredUpdates = afterGap; - canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID).finally(validateAndApplyDeferredUpdates); + App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID) + .then(() => applyUpdates(afterGap)) + .then(unpauseQueueAndReset); }); return; } - applyUpdates(pendingDeferredUpdates).finally(() => { - unpauseQueueAndReset(); - deferredUpdates = {}; - }); + applyUpdates(pendingDeferredUpdates).finally(unpauseQueueAndReset); } canUnpauseQueuePromise.finally(validateAndApplyDeferredUpdates); From 6a5c9221c5d94b985ababa19d28689c1cd0cde3c Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 1 Apr 2024 15:19:18 +0200 Subject: [PATCH 11/26] restructure file --- src/libs/actions/OnyxUpdateManager.ts | 209 +++++++++++++------------- 1 file changed, 104 insertions(+), 105 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 06f61940cb91..f54a3f9d2b69 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -33,6 +33,98 @@ type DeferredUpdatesDictionary = Record; let deferredUpdateBeforeReconnect: OnyxUpdatesFromServer | undefined; let deferredUpdates: DeferredUpdatesDictionary = {}; +function unpauseQueueAndReset() { + console.debug('[OnyxUpdateManager] Done applying all updates'); + deferredUpdates = {}; + Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null); + SequentialQueue.unpause(); +} +const applyUpdates = (updates: DeferredUpdatesDictionary) => Promise.all(Object.values(updates).map((update) => OnyxUpdates.apply(update))); + +type DetectGapAndSplitResult = {beforeGap: DeferredUpdatesDictionary; afterGap: DeferredUpdatesDictionary; latestMissingUpdateID: number | undefined}; +// In order for the deferred updates to be applied correctly in order, +// we need to check if there are any gaps deferred updates. +function detectGapAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSplitResult { + let isFirst = true; + const beforeGap: DeferredUpdatesDictionary = {}; + let afterGap: DeferredUpdatesDictionary = updates; + + for (const update of Object.values(updates)) { + // If it's the first update, we need to check that the previousUpdateID of the fetched update is the same as the lastUpdateIDAppliedToClient + // If any update's previousUpdateID doesn't match the lastUpdateID from the previous update, there's a gap in the deferred updates + if (isFirst && update.previousUpdateID !== lastUpdateIDAppliedToClient) { + break; + } + + if (updates[Number(update.previousUpdateID)]) { + beforeGap[Number(update.lastUpdateID)] = update; + // eslint-disable-next-line @typescript-eslint/no-unused-vars + afterGap = Object.fromEntries(Object.entries(afterGap).filter(([_key, u]) => u.lastUpdateID === update.lastUpdateID)); + } else { + break; + } + + isFirst = false; + } + + let latestMissingUpdateID: number | undefined; + const afterGapValues = Object.values(afterGap); + if (afterGapValues.length > 0 && afterGapValues[0]) { + latestMissingUpdateID = Number(afterGapValues[0].previousUpdateID) || 0; + } + + return {beforeGap, afterGap, latestMissingUpdateID}; +} + +// This function will check for gaps in the deferred updates and +// apply the updates in order after the missing updates are fetched and applied +function validateAndApplyDeferredUpdates(): Promise { + // We only want to apply deferred updates that are newer than the last update that was applied to the client. + // At this point, the missing updates from "GetMissingOnyxUpdates" have been applied already, so we can safely filter out. + const pendingDeferredUpdates = Object.fromEntries( + Object.entries(deferredUpdates).filter(([lastUpdateID]) => { + // It should not be possible for lastUpdateIDAppliedToClient to be null, + // after the missing updates have been applied. + // Still, if so we want to keep the deferred update in the list + if (!lastUpdateIDAppliedToClient) { + return true; + } + return (Number(lastUpdateID) ?? 0) > lastUpdateIDAppliedToClient; + }), + ); + const pendingDeferredUpdateValues = Object.values(pendingDeferredUpdates); + + // If there are no remaining deferred updates after filtering out outdated ones, + // we can just unpause the queue and return + if (pendingDeferredUpdateValues.length === 0) { + return Promise.resolve(); + } + + const {beforeGap, afterGap, latestMissingUpdateID} = detectGapAndSplit(pendingDeferredUpdates); + + // If we detect a gap in the deferred updates, only apply the deferred updates before the gap, + // re-fetch the missing updates and then apply the deferred updates again. + if (latestMissingUpdateID) { + return new Promise((resolve, reject) => { + applyUpdates(beforeGap).then(() => { + // Same as above, we can ignore this case because + // it should not be possible for lastUpdateIDAppliedToClient to be null. + if (!lastUpdateIDAppliedToClient) { + return; + } + + deferredUpdates = afterGap; + App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID) + .then(() => applyUpdates(afterGap)) + .then(() => resolve()) + .catch(reject); + }); + }); + } + + return applyUpdates(pendingDeferredUpdates).then(() => undefined); +} + export default () => { console.debug('[OnyxUpdateManager] Listening for updates from the server'); Onyx.connect({ @@ -72,7 +164,7 @@ export default () => { // applied in their correct and specific order. If this queue was not paused, then there would be a lot of // onyx data being applied while we are fetching the missing updates and that would put them all out of order. SequentialQueue.pause(); - let canUnpauseQueuePromise: Promise; + let queryPromise: Promise; // The flow below is setting the promise to a reconnect app to address flow (1) explained above. if (!lastUpdateIDAppliedToClient) { @@ -81,7 +173,15 @@ export default () => { deferredUpdateBeforeReconnect = updateParams; // 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().then(() => { + // If "updateAfterReconnectApp" is set, it means that case (1) was triggered and we need to apply the update that was queued before the reconnectApp + if (!deferredUpdateBeforeReconnect) { + return; + } + + OnyxUpdates.apply(deferredUpdateBeforeReconnect); + deferredUpdateBeforeReconnect = undefined; + }); } else { // The flow below is setting the promise to a getMissingOnyxUpdates to address flow (2) explained above. @@ -103,111 +203,10 @@ export default () => { }); // Get the missing Onyx updates from the server - canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer); - } - - // This function will check for gaps in the deferred updates and apply the updates in order after the missing updates are fetched and applied - function validateAndApplyDeferredUpdates() { - // It should not be possible for lastUpdateIDAppliedToClient to be null, after the missing updates have been applied, therefore we don't need to handle this case - if (!lastUpdateIDAppliedToClient) { - return; - } - - // If "updateAfterReconnectApp" is set, it means that case (1) was triggered and we need to apply the update that was queued before the reconnectApp - if (deferredUpdateBeforeReconnect) { - OnyxUpdates.apply(deferredUpdateBeforeReconnect); - deferredUpdateBeforeReconnect = undefined; - } - - // We only want to apply deferred updates that are newer than the last update that was applied to the client. - // At this point, the missing updates from "GetMissingOnyxUpdates" have been applied already, so we can safely filter out. - const pendingDeferredUpdates = Object.fromEntries( - Object.entries(deferredUpdates).filter(([lastUpdateID]) => { - // Same as above, it should not be possible for lastUpdateIDAppliedToClient to be null. - // Still, if so we want to keep the deferred update in the list - if (!lastUpdateIDAppliedToClient) { - return true; - } - return (Number(lastUpdateID) ?? 0) > lastUpdateIDAppliedToClient; - }), - ); - const pendingDeferredUpdateValues = Object.values(pendingDeferredUpdates); - - function unpauseQueueAndReset() { - console.debug('[OnyxUpdateManager] Done applying all updates'); - deferredUpdates = {}; - Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null); - SequentialQueue.unpause(); - } - - // If there are no remaining deferred updates after filtering out outdated ones, - // we can just unpause the queue and return - if (pendingDeferredUpdateValues.length === 0) { - unpauseQueueAndReset(); - return; - } - - type DetectGapAndSplitResult = {beforeGap: DeferredUpdatesDictionary; afterGap: DeferredUpdatesDictionary; latestMissingUpdateID: number | undefined}; - // In order for the deferred updates to be applied correctly in order, - // we need to check if there are any gaps deferred updates. - function detectGapAndSplit(): DetectGapAndSplitResult { - let isFirst = true; - const beforeGap: DeferredUpdatesDictionary = {}; - let afterGap: DeferredUpdatesDictionary = pendingDeferredUpdates; - - for (const update of pendingDeferredUpdateValues) { - // If it's the first update, we need to check that the previousUpdateID of the fetched update is the same as the lastUpdateIDAppliedToClient - // If any update's previousUpdateID doesn't match the lastUpdateID from the previous update, there's a gap in the deferred updates - if (isFirst && update.previousUpdateID !== lastUpdateIDAppliedToClient) { - break; - } - - if (pendingDeferredUpdates[Number(update.previousUpdateID)]) { - beforeGap[Number(update.lastUpdateID)] = update; - // eslint-disable-next-line @typescript-eslint/no-unused-vars - afterGap = Object.fromEntries(Object.entries(afterGap).filter(([_key, u]) => u.lastUpdateID === update.lastUpdateID)); - } else { - break; - } - - isFirst = false; - } - - let latestMissingUpdateID: number | undefined; - const afterGapValues = Object.values(afterGap); - if (afterGapValues.length > 0 && afterGapValues[0]) { - latestMissingUpdateID = Number(afterGapValues[0].previousUpdateID) || 0; - } - - return {beforeGap, afterGap, latestMissingUpdateID}; - } - - const {beforeGap, afterGap, latestMissingUpdateID} = detectGapAndSplit(); - - const applyUpdates = (updates: DeferredUpdatesDictionary) => Promise.all(Object.values(updates).map((update) => OnyxUpdates.apply(update))); - - // If we detect a gap in the deferred updates, only apply the deferred updates before the gap, - // re-fetch the missing updates and then apply the deferred updates again. - if (latestMissingUpdateID) { - applyUpdates(beforeGap).finally(() => { - // Same as above, we can ignore this case because - // it should not be possible for lastUpdateIDAppliedToClient to be null. - if (!lastUpdateIDAppliedToClient) { - return; - } - - deferredUpdates = afterGap; - App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID) - .then(() => applyUpdates(afterGap)) - .then(unpauseQueueAndReset); - }); - return; - } - - applyUpdates(pendingDeferredUpdates).finally(unpauseQueueAndReset); + queryPromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer).then(validateAndApplyDeferredUpdates); } - canUnpauseQueuePromise.finally(validateAndApplyDeferredUpdates); + queryPromise.finally(unpauseQueueAndReset); }, }); }; From 4f35d1ec0e858934ec1651d965af49ac7cc1db3d Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 1 Apr 2024 16:10:37 +0200 Subject: [PATCH 12/26] fix: re-write to detect possible multiple gaps in the deferred updates --- src/libs/actions/OnyxUpdateManager.ts | 80 ++++++++++++++++++--------- 1 file changed, 55 insertions(+), 25 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index f54a3f9d2b69..773b875d7491 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -29,51 +29,72 @@ Onyx.connect({ }); type DeferredUpdatesDictionary = Record; - let deferredUpdateBeforeReconnect: OnyxUpdatesFromServer | undefined; let deferredUpdates: DeferredUpdatesDictionary = {}; +// This function will unpause the SequentialQueue, log an info and reset the deferred updates function unpauseQueueAndReset() { console.debug('[OnyxUpdateManager] Done applying all updates'); deferredUpdates = {}; Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null); SequentialQueue.unpause(); } + +// 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))); -type DetectGapAndSplitResult = {beforeGap: DeferredUpdatesDictionary; afterGap: DeferredUpdatesDictionary; latestMissingUpdateID: number | undefined}; +type DetectGapAndSplitResult = {applicableUpdates: DeferredUpdatesDictionary; updatesAfterGaps: DeferredUpdatesDictionary; latestMissingUpdateID: number | undefined}; + // In order for the deferred updates to be applied correctly in order, // we need to check if there are any gaps deferred updates. -function detectGapAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSplitResult { - let isFirst = true; - const beforeGap: DeferredUpdatesDictionary = {}; - let afterGap: DeferredUpdatesDictionary = updates; +function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSplitResult { + const updateValues = Object.values(updates); + const applicableUpdates: DeferredUpdatesDictionary = {}; - for (const update of Object.values(updates)) { + let gapExists = false; + let firstUpdateAfterGaps: number | undefined; + + for (const [index, update] of updateValues.entries()) { // If it's the first update, we need to check that the previousUpdateID of the fetched update is the same as the lastUpdateIDAppliedToClient // If any update's previousUpdateID doesn't match the lastUpdateID from the previous update, there's a gap in the deferred updates + const isFirst = index === 0; if (isFirst && update.previousUpdateID !== lastUpdateIDAppliedToClient) { break; } if (updates[Number(update.previousUpdateID)]) { - beforeGap[Number(update.lastUpdateID)] = update; - // eslint-disable-next-line @typescript-eslint/no-unused-vars - afterGap = Object.fromEntries(Object.entries(afterGap).filter(([_key, u]) => u.lastUpdateID === update.lastUpdateID)); + // If a gap exists, we will not add any more updates to the applicable updates + // Instead, we we have to keep track of the first update after all detected gaps + if (gapExists) { + // If "firstUpdateAfterGaps" hasn't been set yet and there was a gap, + // we need to set it to the first update after all gaps + if (!firstUpdateAfterGaps) { + firstUpdateAfterGaps = Number(update.previousUpdateID); + } + } else { + // If no gap exists yet and the previousUpdateID of the current update is found in the deferred updates, we can add it to the applicable updates + applicableUpdates[Number(update.lastUpdateID)] = update; + } } else { - break; + // When we find a(nother) gap, we need to set the flag to true and reset the "firstUpdateAfterGaps" variable, so that we can continue searching for the next update after all gaps + gapExists = true; + firstUpdateAfterGaps = undefined; } + } - isFirst = false; + let updatesAfterGaps: DeferredUpdatesDictionary = {}; + if (gapExists && firstUpdateAfterGaps) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + updatesAfterGaps = Object.fromEntries(Object.entries(updates).filter(([lastUpdateID]) => Number(lastUpdateID) >= firstUpdateAfterGaps!)); } let latestMissingUpdateID: number | undefined; - const afterGapValues = Object.values(afterGap); - if (afterGapValues.length > 0 && afterGapValues[0]) { - latestMissingUpdateID = Number(afterGapValues[0].previousUpdateID) || 0; + const updatesAfterGapValues = Object.values(updatesAfterGaps); + if (updatesAfterGapValues.length > 0 && updatesAfterGapValues[0]) { + latestMissingUpdateID = Number(updatesAfterGapValues[0].previousUpdateID) || 0; } - return {beforeGap, afterGap, latestMissingUpdateID}; + return {applicableUpdates, updatesAfterGaps, latestMissingUpdateID}; } // This function will check for gaps in the deferred updates and @@ -85,7 +106,7 @@ function validateAndApplyDeferredUpdates(): Promise { Object.entries(deferredUpdates).filter(([lastUpdateID]) => { // It should not be possible for lastUpdateIDAppliedToClient to be null, // after the missing updates have been applied. - // Still, if so we want to keep the deferred update in the list + // If still so we want to keep the deferred update in the list. if (!lastUpdateIDAppliedToClient) { return true; } @@ -100,29 +121,37 @@ function validateAndApplyDeferredUpdates(): Promise { return Promise.resolve(); } - const {beforeGap, afterGap, latestMissingUpdateID} = detectGapAndSplit(pendingDeferredUpdates); + const {applicableUpdates, updatesAfterGaps, latestMissingUpdateID} = detectGapsAndSplit(pendingDeferredUpdates); - // If we detect a gap in the deferred updates, only apply the deferred updates before the gap, - // re-fetch the missing updates and then apply the deferred updates again. + // If we detected a gap in the deferred updates, only apply the deferred updates before the gap, + // re-fetch the missing updates and then apply the remaining deferred updates after the gap if (latestMissingUpdateID) { return new Promise((resolve, reject) => { - applyUpdates(beforeGap).then(() => { + deferredUpdates = {}; + applyUpdates(applicableUpdates).then(() => { // Same as above, we can ignore this case because // it should not be possible for lastUpdateIDAppliedToClient to be null. if (!lastUpdateIDAppliedToClient) { return; } - deferredUpdates = afterGap; + // After we have applied the applicable updates, there might have been new deferred updates added. + // In the next (recursive) call of "validateAndApplyDeferredUpdates", + // the initial "updatesAfterGaps" and all new deferred updates will be applied in order, + // as long as there was no new gap detected. Otherwise repeat the process. + deferredUpdates = {...deferredUpdates, ...updatesAfterGaps}; + + // Then we can fetch the missing updates and apply them App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID) - .then(() => applyUpdates(afterGap)) + .then(validateAndApplyDeferredUpdates) .then(() => resolve()) .catch(reject); }); }); } - return applyUpdates(pendingDeferredUpdates).then(() => undefined); + // If there are no gaps in the deferred updates, we can apply all deferred updates in order + return applyUpdates(applicableUpdates).then(() => undefined); } export default () => { @@ -202,7 +231,8 @@ export default () => { lastUpdateIDAppliedToClient, }); - // Get the missing Onyx updates from the server + // 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(lastUpdateIDAppliedToClient, previousUpdateIDFromServer).then(validateAndApplyDeferredUpdates); } From 095f09e285f1c431727683a0a3f8e3b6dd84dbb4 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 1 Apr 2024 20:20:35 +0200 Subject: [PATCH 13/26] simplified conditions --- src/libs/actions/OnyxUpdateManager.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 773b875d7491..7da9f9b4a566 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -55,15 +55,15 @@ function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSpl let firstUpdateAfterGaps: number | undefined; for (const [index, update] of updateValues.entries()) { - // If it's the first update, we need to check that the previousUpdateID of the fetched update is the same as the lastUpdateIDAppliedToClient - // If any update's previousUpdateID doesn't match the lastUpdateID from the previous update, there's a gap in the deferred updates const isFirst = index === 0; - if (isFirst && update.previousUpdateID !== lastUpdateIDAppliedToClient) { - break; - } - if (updates[Number(update.previousUpdateID)]) { - // If a gap exists, we will not add any more updates to the applicable updates + // If any update's previousUpdateID doesn't match the lastUpdateID from the previous update, the deferred updates aren't chained and there's a gap. + // For the first update, we need to check that the previousUpdateID of the fetched update is the same as the lastUpdateIDAppliedToClient. + // For any other updates, we need to check if the previousUpdateID of the current update is found in the deferred updates. + // If an update is chained, we can add it to the applicable updates. + const isChained = isFirst ? update.previousUpdateID === lastUpdateIDAppliedToClient : updates[Number(update.previousUpdateID)]; + if (isChained) { + // If a gap exists already, we will not add any more updates to the applicable updates // Instead, we we have to keep track of the first update after all detected gaps if (gapExists) { // If "firstUpdateAfterGaps" hasn't been set yet and there was a gap, @@ -76,7 +76,8 @@ function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSpl applicableUpdates[Number(update.lastUpdateID)] = update; } } else { - // When we find a(nother) gap, we need to set the flag to true and reset the "firstUpdateAfterGaps" variable, so that we can continue searching for the next update after all gaps + // When we find a (new) gap, we need to set "gapExists" to true and reset the "firstUpdateAfterGaps" variable, + // so that we can continue searching for the next update after all gaps gapExists = true; firstUpdateAfterGaps = undefined; } From 6c748ac0ad15d139841b6f9381dac817f8e44bde Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Mon, 1 Apr 2024 20:23:19 +0200 Subject: [PATCH 14/26] further improve comments --- src/libs/actions/OnyxUpdateManager.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 7da9f9b4a566..21f226dd887c 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -63,16 +63,15 @@ function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSpl // If an update is chained, we can add it to the applicable updates. const isChained = isFirst ? update.previousUpdateID === lastUpdateIDAppliedToClient : updates[Number(update.previousUpdateID)]; if (isChained) { - // If a gap exists already, we will not add any more updates to the applicable updates - // Instead, we we have to keep track of the first update after all detected gaps + // If a gap exists already, we will not add any more updates to the applicable updates. + // Instead, we we have to find the first (chained) update after all gaps. if (gapExists) { - // If "firstUpdateAfterGaps" hasn't been set yet and there was a gap, - // we need to set it to the first update after all gaps + // If "firstUpdateAfterGaps" hasn't been set yet and there was a gap, we need to set it to the first update after all gaps. if (!firstUpdateAfterGaps) { firstUpdateAfterGaps = Number(update.previousUpdateID); } } else { - // If no gap exists yet and the previousUpdateID of the current update is found in the deferred updates, we can add it to the applicable updates + // If no gap exists yet, we can add the update to the applicable updates applicableUpdates[Number(update.lastUpdateID)] = update; } } else { From b5deb28fc21994e1c637aada0fa881ffa0faf06f Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 11:51:11 +0200 Subject: [PATCH 15/26] fix: minor problems --- src/libs/actions/OnyxUpdateManager.ts | 37 ++++++++++++++------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 21f226dd887c..aeabfd29e043 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -53,6 +53,7 @@ function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSpl let gapExists = false; let firstUpdateAfterGaps: number | undefined; + let latestMissingUpdateID: number | undefined; for (const [index, update] of updateValues.entries()) { const isFirst = index === 0; @@ -79,21 +80,24 @@ function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSpl // so that we can continue searching for the next update after all gaps gapExists = true; firstUpdateAfterGaps = undefined; + + // If there is a gap, it means the previous update is the latest missing update. + latestMissingUpdateID = Number(update.previousUpdateID); } } + // When there was no "firstUpdateAfterGaps" set yet, we need to set it to the last update in the list, + // because we will fetch all missing updates up to the previous one and can then always apply the lst update. + if (!firstUpdateAfterGaps) { + firstUpdateAfterGaps = Number(updateValues[updateValues.length - 1].lastUpdateID); + } + let updatesAfterGaps: DeferredUpdatesDictionary = {}; if (gapExists && firstUpdateAfterGaps) { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion updatesAfterGaps = Object.fromEntries(Object.entries(updates).filter(([lastUpdateID]) => Number(lastUpdateID) >= firstUpdateAfterGaps!)); } - let latestMissingUpdateID: number | undefined; - const updatesAfterGapValues = Object.values(updatesAfterGaps); - if (updatesAfterGapValues.length > 0 && updatesAfterGapValues[0]) { - latestMissingUpdateID = Number(updatesAfterGapValues[0].previousUpdateID) || 0; - } - return {applicableUpdates, updatesAfterGaps, latestMissingUpdateID}; } @@ -113,11 +117,10 @@ function validateAndApplyDeferredUpdates(): Promise { return (Number(lastUpdateID) ?? 0) > lastUpdateIDAppliedToClient; }), ); - const pendingDeferredUpdateValues = Object.values(pendingDeferredUpdates); // If there are no remaining deferred updates after filtering out outdated ones, // we can just unpause the queue and return - if (pendingDeferredUpdateValues.length === 0) { + if (Object.values(pendingDeferredUpdates).length === 0) { return Promise.resolve(); } @@ -129,23 +132,21 @@ function validateAndApplyDeferredUpdates(): Promise { return new Promise((resolve, reject) => { deferredUpdates = {}; applyUpdates(applicableUpdates).then(() => { - // Same as above, we can ignore this case because - // it should not be possible for lastUpdateIDAppliedToClient to be null. - if (!lastUpdateIDAppliedToClient) { - return; - } - // After we have applied the applicable updates, there might have been new deferred updates added. // In the next (recursive) call of "validateAndApplyDeferredUpdates", // the initial "updatesAfterGaps" and all new deferred updates will be applied in order, // as long as there was no new gap detected. Otherwise repeat the process. deferredUpdates = {...deferredUpdates, ...updatesAfterGaps}; + // It should not be possible for lastUpdateIDAppliedToClient to be null, therefore we can ignore this case. + // If lastUpdateIDAppliedToClient got updated in the meantime, we will just retrigger the validation and application of the current deferred updates. + if (!lastUpdateIDAppliedToClient || latestMissingUpdateID <= lastUpdateIDAppliedToClient) { + validateAndApplyDeferredUpdates().then(resolve).catch(reject); + return; + } + // Then we can fetch the missing updates and apply them - App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID) - .then(validateAndApplyDeferredUpdates) - .then(() => resolve()) - .catch(reject); + App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID).then(validateAndApplyDeferredUpdates).then(resolve).catch(reject); }); }); } From 2d087866bf4737b5d2da40644eb1988a881389a1 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 11:54:32 +0200 Subject: [PATCH 16/26] remove unnecessary code --- src/libs/actions/OnyxUpdateManager.ts | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index aeabfd29e043..ad5debacf9c2 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -29,7 +29,6 @@ Onyx.connect({ }); type DeferredUpdatesDictionary = Record; -let deferredUpdateBeforeReconnect: OnyxUpdatesFromServer | undefined; let deferredUpdates: DeferredUpdatesDictionary = {}; // This function will unpause the SequentialQueue, log an info and reset the deferred updates @@ -103,7 +102,7 @@ function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSpl // This function will check for gaps in the deferred updates and // apply the updates in order after the missing updates are fetched and applied -function validateAndApplyDeferredUpdates(): Promise { +function validateAndApplyDeferredUpdates(): Promise { // We only want to apply deferred updates that are newer than the last update that was applied to the client. // At this point, the missing updates from "GetMissingOnyxUpdates" have been applied already, so we can safely filter out. const pendingDeferredUpdates = Object.fromEntries( @@ -152,7 +151,7 @@ function validateAndApplyDeferredUpdates(): Promise { } // If there are no gaps in the deferred updates, we can apply all deferred updates in order - return applyUpdates(applicableUpdates).then(() => undefined); + return applyUpdates(applicableUpdates); } export default () => { @@ -194,24 +193,14 @@ export default () => { // applied in their correct and specific order. If this queue was not paused, then there would be a lot of // onyx data being applied while we are fetching the missing updates and that would put them all out of order. SequentialQueue.pause(); - let queryPromise: Promise; + let queryPromise: Promise; // The flow below is setting the promise to a reconnect app to address flow (1) explained above. if (!lastUpdateIDAppliedToClient) { Log.info('Client has not gotten reliable updates before so reconnecting the app to start the process'); - deferredUpdateBeforeReconnect = updateParams; - // Since this is a full reconnectApp, we'll not apply the updates we received - those will come in the reconnect app request. - queryPromise = App.finalReconnectAppAfterActivatingReliableUpdates().then(() => { - // If "updateAfterReconnectApp" is set, it means that case (1) was triggered and we need to apply the update that was queued before the reconnectApp - if (!deferredUpdateBeforeReconnect) { - return; - } - - OnyxUpdates.apply(deferredUpdateBeforeReconnect); - deferredUpdateBeforeReconnect = undefined; - }); + queryPromise = App.finalReconnectAppAfterActivatingReliableUpdates(); } else { // The flow below is setting the promise to a getMissingOnyxUpdates to address flow (2) explained above. From 25c86239212462ea7b51f41df4ad75ea39c61962 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 11:55:37 +0200 Subject: [PATCH 17/26] improve comment --- src/libs/actions/OnyxUpdateManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index ad5debacf9c2..c960e2233425 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -209,7 +209,7 @@ export default () => { // Add the new update to the deferred updates deferredUpdates[Number(updateParams.lastUpdateID)] = updateParams; - // If there are already deferred updates, then we don't need to fetch the missing updates again + // If there are deferred updates already, we don't need to fetch the missing updates again. if (existingDeferredUpdatesCount > 0) { return; } From 776981079a066477e7e177971f2ba20a81e5673a Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 12:00:01 +0200 Subject: [PATCH 18/26] fix: check for queryPromise --- src/libs/actions/OnyxUpdateManager.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index c960e2233425..9bc85faf1cff 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -28,12 +28,15 @@ Onyx.connect({ callback: (value) => (lastUpdateIDAppliedToClient = value), }); +let queryPromise: Promise | undefined; + type DeferredUpdatesDictionary = Record; let deferredUpdates: DeferredUpdatesDictionary = {}; -// This function will unpause the SequentialQueue, log an info and reset the deferred updates -function unpauseQueueAndReset() { +// 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'); + queryPromise = undefined; deferredUpdates = {}; Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null); SequentialQueue.unpause(); @@ -163,6 +166,11 @@ export default () => { return; } + // If there is a query of either ReconnectApp or GetMissingOnyxUpdates in progress, we should not start another one. + if (queryPromise) { + 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 ( @@ -193,7 +201,6 @@ export default () => { // applied in their correct and specific order. If this queue was not paused, then there would be a lot of // onyx data being applied while we are fetching the missing updates and that would put them all out of order. SequentialQueue.pause(); - let queryPromise: Promise; // The flow below is setting the promise to a reconnect app to address flow (1) explained above. if (!lastUpdateIDAppliedToClient) { @@ -226,7 +233,7 @@ export default () => { queryPromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer).then(validateAndApplyDeferredUpdates); } - queryPromise.finally(unpauseQueueAndReset); + queryPromise.finally(finalizeUpdatesAndResumeQueue); }, }); }; From 47613cf0d3fede7bbf1ae0405934102afeb46770 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 12:02:17 +0200 Subject: [PATCH 19/26] update comment --- src/libs/actions/OnyxUpdateManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 9bc85faf1cff..e789e06fbf4e 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -88,8 +88,8 @@ function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSpl } } - // When there was no "firstUpdateAfterGaps" set yet, we need to set it to the last update in the list, - // because we will fetch all missing updates up to the previous one and can then always apply the lst update. + // When "firstUpdateAfterGaps" is not set yet, we need to set it to the last update in the list, + // because we will fetch all missing updates up to the previous one and can then always apply the last update in the deferred updates. if (!firstUpdateAfterGaps) { firstUpdateAfterGaps = Number(updateValues[updateValues.length - 1].lastUpdateID); } From fb9ac392760b8e78067e4b38855bdd154f069075 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 12:04:21 +0200 Subject: [PATCH 20/26] update comment --- src/libs/actions/OnyxUpdateManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index e789e06fbf4e..2ca93f967818 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -67,7 +67,7 @@ function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSpl const isChained = isFirst ? update.previousUpdateID === lastUpdateIDAppliedToClient : updates[Number(update.previousUpdateID)]; if (isChained) { // If a gap exists already, we will not add any more updates to the applicable updates. - // Instead, we we have to find the first (chained) update after all gaps. + // Instead, once there are two chained updates again, we can set "firstUpdateAfterGaps" to the first update after the current gap. if (gapExists) { // If "firstUpdateAfterGaps" hasn't been set yet and there was a gap, we need to set it to the first update after all gaps. if (!firstUpdateAfterGaps) { From b9692d1eb40c7ad640c3384f211011397d4e3997 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 12:04:43 +0200 Subject: [PATCH 21/26] move lines --- src/libs/actions/OnyxUpdateManager.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 2ca93f967818..c8f354981fec 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -45,10 +45,9 @@ function finalizeUpdatesAndResumeQueue() { // 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))); -type DetectGapAndSplitResult = {applicableUpdates: DeferredUpdatesDictionary; updatesAfterGaps: DeferredUpdatesDictionary; latestMissingUpdateID: number | undefined}; - // In order for the deferred updates to be applied correctly in order, // we need to check if there are any gaps deferred updates. +type DetectGapAndSplitResult = {applicableUpdates: DeferredUpdatesDictionary; updatesAfterGaps: DeferredUpdatesDictionary; latestMissingUpdateID: number | undefined}; function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSplitResult { const updateValues = Object.values(updates); const applicableUpdates: DeferredUpdatesDictionary = {}; From 25147725bcbdffebaed4c178da161e61ac1bc37a Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 13:25:14 +0200 Subject: [PATCH 22/26] fix: comments --- src/libs/actions/OnyxUpdateManager.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index c8f354981fec..aa5ae50f947b 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -46,7 +46,7 @@ function finalizeUpdatesAndResumeQueue() { const applyUpdates = (updates: DeferredUpdatesDictionary) => Promise.all(Object.values(updates).map((update) => OnyxUpdates.apply(update))); // In order for the deferred updates to be applied correctly in order, -// we need to check if there are any gaps deferred updates. +// we need to check if there are any gaps between deferred updates. type DetectGapAndSplitResult = {applicableUpdates: DeferredUpdatesDictionary; updatesAfterGaps: DeferredUpdatesDictionary; latestMissingUpdateID: number | undefined}; function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSplitResult { const updateValues = Object.values(updates); @@ -120,7 +120,7 @@ function validateAndApplyDeferredUpdates(): Promise { ); // If there are no remaining deferred updates after filtering out outdated ones, - // we can just unpause the queue and return + // we can just unpause the queue and return if (Object.values(pendingDeferredUpdates).length === 0) { return Promise.resolve(); } @@ -212,6 +212,7 @@ export default () => { // Get the number of deferred updates before adding the new one const existingDeferredUpdatesCount = Object.keys(deferredUpdates).length; + // Add the new update to the deferred updates deferredUpdates[Number(updateParams.lastUpdateID)] = updateParams; From c0fe3644b7e5fad0f9f7508f3e78900bc79faed3 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 14:39:53 +0200 Subject: [PATCH 23/26] fix: wrong early return --- src/libs/actions/OnyxUpdateManager.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index aa5ae50f947b..a065efaf49f9 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -165,11 +165,6 @@ export default () => { return; } - // If there is a query of either ReconnectApp or GetMissingOnyxUpdates in progress, we should not start another one. - if (queryPromise) { - 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 ( @@ -203,6 +198,11 @@ export default () => { // The flow below is setting the promise to a reconnect app to address flow (1) explained above. if (!lastUpdateIDAppliedToClient) { + // 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. From 252f0ab447048893ba3b2d86f778e6b391483463 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 14:57:15 +0200 Subject: [PATCH 24/26] fix: missing return --- src/libs/actions/OnyxUpdateManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index a065efaf49f9..7b472e67b70e 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -147,7 +147,7 @@ function validateAndApplyDeferredUpdates(): Promise { } // Then we can fetch the missing updates and apply them - App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID).then(validateAndApplyDeferredUpdates).then(resolve).catch(reject); + return App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID).then(validateAndApplyDeferredUpdates).then(resolve).catch(reject); }); }); } From c1ca79b8a815695430cd8e284d5c349bc3275ab0 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 15:08:07 +0200 Subject: [PATCH 25/26] Revert "fix: missing return" This reverts commit 252f0ab447048893ba3b2d86f778e6b391483463. --- src/libs/actions/OnyxUpdateManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index 7b472e67b70e..a065efaf49f9 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -147,7 +147,7 @@ function validateAndApplyDeferredUpdates(): Promise { } // Then we can fetch the missing updates and apply them - return App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID).then(validateAndApplyDeferredUpdates).then(resolve).catch(reject); + App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, latestMissingUpdateID).then(validateAndApplyDeferredUpdates).then(resolve).catch(reject); }); }); } From 67f322e67056d3f53ca6ee37f316037544bc6823 Mon Sep 17 00:00:00 2001 From: Christoph Pader Date: Tue, 2 Apr 2024 19:43:13 +0200 Subject: [PATCH 26/26] Update OnyxUpdateManager.ts Co-authored-by: Eric Han <117511920+eh2077@users.noreply.github.com> --- src/libs/actions/OnyxUpdateManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/OnyxUpdateManager.ts b/src/libs/actions/OnyxUpdateManager.ts index a065efaf49f9..45cb2b78ecde 100644 --- a/src/libs/actions/OnyxUpdateManager.ts +++ b/src/libs/actions/OnyxUpdateManager.ts @@ -63,7 +63,7 @@ function detectGapsAndSplit(updates: DeferredUpdatesDictionary): DetectGapAndSpl // For the first update, we need to check that the previousUpdateID of the fetched update is the same as the lastUpdateIDAppliedToClient. // For any other updates, we need to check if the previousUpdateID of the current update is found in the deferred updates. // If an update is chained, we can add it to the applicable updates. - const isChained = isFirst ? update.previousUpdateID === lastUpdateIDAppliedToClient : updates[Number(update.previousUpdateID)]; + const isChained = isFirst ? update.previousUpdateID === lastUpdateIDAppliedToClient : !!updates[Number(update.previousUpdateID)]; if (isChained) { // If a gap exists already, we will not add any more updates to the applicable updates. // Instead, once there are two chained updates again, we can set "firstUpdateAfterGaps" to the first update after the current gap.