From 14755ebf2720da96c2ff61c6c021c25bab353bf7 Mon Sep 17 00:00:00 2001 From: Carlos Barros <765936+barros001@users.noreply.github.com> Date: Mon, 6 Nov 2023 16:31:35 -0500 Subject: [PATCH 1/9] ensure push events are processed in the same order as they are received --- src/libs/actions/OnyxUpdates.ts | 14 ++++++++++++-- src/libs/actions/User.js | 34 +++++++++++++++++++-------------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/libs/actions/OnyxUpdates.ts b/src/libs/actions/OnyxUpdates.ts index b44b485ac60f..16212bfae106 100644 --- a/src/libs/actions/OnyxUpdates.ts +++ b/src/libs/actions/OnyxUpdates.ts @@ -15,6 +15,8 @@ Onyx.connect({ callback: (val) => (lastUpdateIDAppliedToClient = val), }); +let pusherEventsQueuePromise = Promise.resolve(); + function applyHTTPSOnyxUpdates(request: Request, response: Response) { console.debug('[OnyxUpdateManager] Applying https update'); // For most requests we can immediately update Onyx. For write requests we queue the updates and apply them after the sequential queue has flushed to prevent a replay effect in @@ -45,10 +47,18 @@ function applyHTTPSOnyxUpdates(request: Request, response: Response) { function applyPusherOnyxUpdates(updates: OnyxUpdateEvent[]) { console.debug('[OnyxUpdateManager] Applying pusher update'); - const pusherEventPromises = updates.map((update) => PusherUtils.triggerMultiEventHandler(update.eventType, update.data)); - return Promise.all(pusherEventPromises).then(() => { + + for (const update of updates) { + pusherEventsQueuePromise = pusherEventsQueuePromise.then(() => { + return PusherUtils.triggerMultiEventHandler(update.eventType, update.data); + }); + } + + pusherEventsQueuePromise = pusherEventsQueuePromise.then(() => { console.debug('[OnyxUpdateManager] Done applying Pusher update'); }); + + return pusherEventsQueuePromise; } /** diff --git a/src/libs/actions/User.js b/src/libs/actions/User.js index f7375a5583a6..9849c54dffd7 100644 --- a/src/libs/actions/User.js +++ b/src/libs/actions/User.js @@ -522,20 +522,26 @@ function subscribeToUserEvents() { }); // Handles Onyx updates coming from Pusher through the mega multipleEvents. - PusherUtils.subscribeToMultiEvent(Pusher.TYPE.MULTIPLE_EVENT_TYPE.ONYX_API_UPDATE, (pushJSON) => - SequentialQueue.getCurrentRequest().then(() => { - // If we don't have the currentUserAccountID (user is logged out) we don't want to update Onyx with data from Pusher - if (!currentUserAccountID) { - return; - } - - const onyxUpdatePromise = Onyx.update(pushJSON); - triggerNotifications(pushJSON); - - // Return a promise when Onyx is done updating so that the OnyxUpdatesManager can properly apply all - // the onyx updates in order - return onyxUpdatePromise; - }), + PusherUtils.subscribeToMultiEvent( + Pusher.TYPE.MULTIPLE_EVENT_TYPE.ONYX_API_UPDATE, + (pushJSON) => + new Promise((resolve) => + SequentialQueue.getCurrentRequest().then(() => { + // If we don't have the currentUserAccountID (user is logged out) we don't want to update Onyx with data from Pusher + if (!currentUserAccountID) { + return resolve(); + } + + const onyxUpdatePromise = Onyx.update(pushJSON); + triggerNotifications(pushJSON); + + // Return a promise when Onyx is done updating so that the OnyxUpdatesManager can properly apply all + // the onyx updates in order + return onyxUpdatePromise.then(() => { + resolve(onyxUpdatePromise); + }); + }), + ), ); } From 49557b37cbd7ff0893c08864af627845dae24f51 Mon Sep 17 00:00:00 2001 From: Carlos Barros <765936+barros001@users.noreply.github.com> Date: Mon, 6 Nov 2023 21:11:48 -0500 Subject: [PATCH 2/9] removed redundant promise --- src/libs/actions/User.js | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/libs/actions/User.js b/src/libs/actions/User.js index 9849c54dffd7..536c68f2cf01 100644 --- a/src/libs/actions/User.js +++ b/src/libs/actions/User.js @@ -522,26 +522,20 @@ function subscribeToUserEvents() { }); // Handles Onyx updates coming from Pusher through the mega multipleEvents. - PusherUtils.subscribeToMultiEvent( - Pusher.TYPE.MULTIPLE_EVENT_TYPE.ONYX_API_UPDATE, - (pushJSON) => - new Promise((resolve) => - SequentialQueue.getCurrentRequest().then(() => { - // If we don't have the currentUserAccountID (user is logged out) we don't want to update Onyx with data from Pusher - if (!currentUserAccountID) { - return resolve(); - } - - const onyxUpdatePromise = Onyx.update(pushJSON); - triggerNotifications(pushJSON); - - // Return a promise when Onyx is done updating so that the OnyxUpdatesManager can properly apply all - // the onyx updates in order - return onyxUpdatePromise.then(() => { - resolve(onyxUpdatePromise); - }); - }), - ), + PusherUtils.subscribeToMultiEvent(Pusher.TYPE.MULTIPLE_EVENT_TYPE.ONYX_API_UPDATE, (pushJSON) => + SequentialQueue.getCurrentRequest().then(() => { + // If we don't have the currentUserAccountID (user is logged out) we don't want to update Onyx with data from Pusher + if (!currentUserAccountID) { + return + } + + const onyxUpdatePromise = Onyx.update(pushJSON); + triggerNotifications(pushJSON); + + // Return a promise when Onyx is done updating so that the OnyxUpdatesManager can properly apply all + // the onyx updates in order + return onyxUpdatePromise; + }), ); } From f756a64754adc7dd60513ec789a21820ae30528a Mon Sep 17 00:00:00 2001 From: Carlos Barros <765936+barros001@users.noreply.github.com> Date: Mon, 6 Nov 2023 21:13:38 -0500 Subject: [PATCH 3/9] removed unnecessary return --- src/libs/actions/OnyxUpdates.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libs/actions/OnyxUpdates.ts b/src/libs/actions/OnyxUpdates.ts index 16212bfae106..8d07755be357 100644 --- a/src/libs/actions/OnyxUpdates.ts +++ b/src/libs/actions/OnyxUpdates.ts @@ -49,9 +49,7 @@ function applyPusherOnyxUpdates(updates: OnyxUpdateEvent[]) { console.debug('[OnyxUpdateManager] Applying pusher update'); for (const update of updates) { - pusherEventsQueuePromise = pusherEventsQueuePromise.then(() => { - return PusherUtils.triggerMultiEventHandler(update.eventType, update.data); - }); + pusherEventsQueuePromise = pusherEventsQueuePromise.then(() => PusherUtils.triggerMultiEventHandler(update.eventType, update.data)); } pusherEventsQueuePromise = pusherEventsQueuePromise.then(() => { From 0244379fb3ff868a5079ed0be96d2c9474f756de Mon Sep 17 00:00:00 2001 From: Carlos Barros <765936+barros001@users.noreply.github.com> Date: Mon, 6 Nov 2023 21:14:28 -0500 Subject: [PATCH 4/9] added missing ; --- src/libs/actions/User.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/User.js b/src/libs/actions/User.js index 536c68f2cf01..f7375a5583a6 100644 --- a/src/libs/actions/User.js +++ b/src/libs/actions/User.js @@ -526,7 +526,7 @@ function subscribeToUserEvents() { SequentialQueue.getCurrentRequest().then(() => { // If we don't have the currentUserAccountID (user is logged out) we don't want to update Onyx with data from Pusher if (!currentUserAccountID) { - return + return; } const onyxUpdatePromise = Onyx.update(pushJSON); From 89267cbf93308384233fa3e64b31178031d09172 Mon Sep 17 00:00:00 2001 From: Carlos Barros <765936+barros001@users.noreply.github.com> Date: Thu, 9 Nov 2023 10:46:36 -0500 Subject: [PATCH 5/9] use reduce instead of for --- src/libs/actions/OnyxUpdates.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libs/actions/OnyxUpdates.ts b/src/libs/actions/OnyxUpdates.ts index 8d07755be357..8a855698a5b7 100644 --- a/src/libs/actions/OnyxUpdates.ts +++ b/src/libs/actions/OnyxUpdates.ts @@ -15,7 +15,7 @@ Onyx.connect({ callback: (val) => (lastUpdateIDAppliedToClient = val), }); -let pusherEventsQueuePromise = Promise.resolve(); +let pusherEventsPromise = Promise.resolve(); function applyHTTPSOnyxUpdates(request: Request, response: Response) { console.debug('[OnyxUpdateManager] Applying https update'); @@ -48,15 +48,15 @@ function applyHTTPSOnyxUpdates(request: Request, response: Response) { function applyPusherOnyxUpdates(updates: OnyxUpdateEvent[]) { console.debug('[OnyxUpdateManager] Applying pusher update'); - for (const update of updates) { - pusherEventsQueuePromise = pusherEventsQueuePromise.then(() => PusherUtils.triggerMultiEventHandler(update.eventType, update.data)); - } - - pusherEventsQueuePromise = pusherEventsQueuePromise.then(() => { - console.debug('[OnyxUpdateManager] Done applying Pusher update'); - }); + pusherEventsPromise = updates + .reduce((promise, update) => { + return promise.then(() => PusherUtils.triggerMultiEventHandler(update.eventType, update.data)); + }, pusherEventsPromise) + .then(() => { + console.debug('[OnyxUpdateManager2] Done applying Pusher update'); + }); - return pusherEventsQueuePromise; + return pusherEventsPromise; } /** From b79db02d7fd1e0815d3f81f49f561469a6b8e376 Mon Sep 17 00:00:00 2001 From: Carlos Barros <765936+barros001@users.noreply.github.com> Date: Thu, 9 Nov 2023 10:48:49 -0500 Subject: [PATCH 6/9] add comment --- src/libs/actions/OnyxUpdates.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/OnyxUpdates.ts b/src/libs/actions/OnyxUpdates.ts index 8a855698a5b7..213a47b64b17 100644 --- a/src/libs/actions/OnyxUpdates.ts +++ b/src/libs/actions/OnyxUpdates.ts @@ -15,6 +15,8 @@ Onyx.connect({ callback: (val) => (lastUpdateIDAppliedToClient = val), }); +// This promise is used to ensure pusher events are always processed in the order they are received, +// even when such events are received over multiple separate pusher updates. let pusherEventsPromise = Promise.resolve(); function applyHTTPSOnyxUpdates(request: Request, response: Response) { @@ -53,7 +55,7 @@ function applyPusherOnyxUpdates(updates: OnyxUpdateEvent[]) { return promise.then(() => PusherUtils.triggerMultiEventHandler(update.eventType, update.data)); }, pusherEventsPromise) .then(() => { - console.debug('[OnyxUpdateManager2] Done applying Pusher update'); + console.debug('[OnyxUpdateManager] Done applying Pusher update'); }); return pusherEventsPromise; From 39508d3072c517e6095feb5e1572d4cfd874073c Mon Sep 17 00:00:00 2001 From: Carlos Barros <765936+barros001@users.noreply.github.com> Date: Thu, 9 Nov 2023 11:27:35 -0500 Subject: [PATCH 7/9] fixed lint issues --- src/libs/actions/OnyxUpdates.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/OnyxUpdates.ts b/src/libs/actions/OnyxUpdates.ts index 213a47b64b17..ce673fa6aaaf 100644 --- a/src/libs/actions/OnyxUpdates.ts +++ b/src/libs/actions/OnyxUpdates.ts @@ -48,12 +48,12 @@ function applyHTTPSOnyxUpdates(request: Request, response: Response) { } function applyPusherOnyxUpdates(updates: OnyxUpdateEvent[]) { - console.debug('[OnyxUpdateManager] Applying pusher update'); + pusherEventsPromise = pusherEventsPromise.then(() => { + console.debug('[OnyxUpdateManager] Applying pusher update'); + }); pusherEventsPromise = updates - .reduce((promise, update) => { - return promise.then(() => PusherUtils.triggerMultiEventHandler(update.eventType, update.data)); - }, pusherEventsPromise) + .reduce((promise, update) => promise.then(() => PusherUtils.triggerMultiEventHandler(update.eventType, update.data)), pusherEventsPromise) .then(() => { console.debug('[OnyxUpdateManager] Done applying Pusher update'); }); From 82fb44bdfd00d84ddc4abcb1e19ba686c7ed9fed Mon Sep 17 00:00:00 2001 From: Carlos Barros <765936+barros001@users.noreply.github.com> Date: Tue, 14 Nov 2023 14:04:34 -0500 Subject: [PATCH 8/9] serialize execution of entire apply method instead of only applyPusherOnyxUpdates --- src/libs/actions/OnyxUpdates.ts | 48 ++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/src/libs/actions/OnyxUpdates.ts b/src/libs/actions/OnyxUpdates.ts index ce673fa6aaaf..852fc907a920 100644 --- a/src/libs/actions/OnyxUpdates.ts +++ b/src/libs/actions/OnyxUpdates.ts @@ -15,10 +15,6 @@ Onyx.connect({ callback: (val) => (lastUpdateIDAppliedToClient = val), }); -// This promise is used to ensure pusher events are always processed in the order they are received, -// even when such events are received over multiple separate pusher updates. -let pusherEventsPromise = Promise.resolve(); - function applyHTTPSOnyxUpdates(request: Request, response: Response) { console.debug('[OnyxUpdateManager] Applying https update'); // For most requests we can immediately update Onyx. For write requests we queue the updates and apply them after the sequential queue has flushed to prevent a replay effect in @@ -48,19 +44,22 @@ function applyHTTPSOnyxUpdates(request: Request, response: Response) { } function applyPusherOnyxUpdates(updates: OnyxUpdateEvent[]) { - pusherEventsPromise = pusherEventsPromise.then(() => { - console.debug('[OnyxUpdateManager] Applying pusher update'); - }); - - pusherEventsPromise = updates - .reduce((promise, update) => promise.then(() => PusherUtils.triggerMultiEventHandler(update.eventType, update.data)), pusherEventsPromise) + return updates + .reduce( + (promise, update) => promise.then(() => PusherUtils.triggerMultiEventHandler(update.eventType, update.data)), + Promise.resolve().then(() => { + console.debug('[OnyxUpdateManager] Applying pusher update'); + }), + ) .then(() => { console.debug('[OnyxUpdateManager] Done applying Pusher update'); }); - - return pusherEventsPromise; } +// This promise is used to ensure pusher events are always processed in the order they are received, +// even when such events are received over multiple separate pusher updates. +let applyPromise: Promise = Promise.resolve(); + /** * @param [updateParams.request] Exists if updateParams.type === 'https' * @param [updateParams.response] Exists if updateParams.type === 'https' @@ -69,21 +68,28 @@ function applyPusherOnyxUpdates(updates: OnyxUpdateEvent[]) { function apply({lastUpdateID, type, request, response, updates}: Merge): Promise; function apply({lastUpdateID, type, request, response, updates}: Merge): Promise; function apply({lastUpdateID, type, request, response, updates}: OnyxUpdatesFromServer): Promise | undefined { - console.debug(`[OnyxUpdateManager] Applying update type: ${type} with lastUpdateID: ${lastUpdateID}`, {request, response, updates}); + applyPromise = applyPromise.then(() => { + console.debug(`[OnyxUpdateManager] Applying update type: ${type} with lastUpdateID: ${lastUpdateID}`, {request, response, updates}); + + if (lastUpdateID && lastUpdateIDAppliedToClient && Number(lastUpdateID) < lastUpdateIDAppliedToClient) { + console.debug('[OnyxUpdateManager] Update received was older than current state, returning without applying the updates'); + return Promise.resolve(); + } + if (lastUpdateID && (lastUpdateIDAppliedToClient === null || Number(lastUpdateID) > lastUpdateIDAppliedToClient)) { + return Onyx.merge(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, Number(lastUpdateID)); + } - if (lastUpdateID && lastUpdateIDAppliedToClient && Number(lastUpdateID) < lastUpdateIDAppliedToClient) { - console.debug('[OnyxUpdateManager] Update received was older than current state, returning without applying the updates'); return Promise.resolve(); - } - if (lastUpdateID && (lastUpdateIDAppliedToClient === null || Number(lastUpdateID) > lastUpdateIDAppliedToClient)) { - Onyx.merge(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, Number(lastUpdateID)); - } + }); + if (type === CONST.ONYX_UPDATE_TYPES.HTTPS && request && response) { - return applyHTTPSOnyxUpdates(request, response); + applyPromise = applyPromise.then(() => applyHTTPSOnyxUpdates(request, response)); } if (type === CONST.ONYX_UPDATE_TYPES.PUSHER && updates) { - return applyPusherOnyxUpdates(updates); + applyPromise = applyPromise.then(() => applyPusherOnyxUpdates(updates)); } + + return applyPromise; } /** From cb7356e22acf5e399b18a95fe179c7fb4da47b23 Mon Sep 17 00:00:00 2001 From: Carlos Barros <765936+barros001@users.noreply.github.com> Date: Tue, 14 Nov 2023 14:14:28 -0500 Subject: [PATCH 9/9] Revert "serialize execution of entire apply method instead of only applyPusherOnyxUpdates" This reverts commit 82fb44bdfd00d84ddc4abcb1e19ba686c7ed9fed. --- src/libs/actions/OnyxUpdates.ts | 48 +++++++++++++++------------------ 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/src/libs/actions/OnyxUpdates.ts b/src/libs/actions/OnyxUpdates.ts index 852fc907a920..ce673fa6aaaf 100644 --- a/src/libs/actions/OnyxUpdates.ts +++ b/src/libs/actions/OnyxUpdates.ts @@ -15,6 +15,10 @@ Onyx.connect({ callback: (val) => (lastUpdateIDAppliedToClient = val), }); +// This promise is used to ensure pusher events are always processed in the order they are received, +// even when such events are received over multiple separate pusher updates. +let pusherEventsPromise = Promise.resolve(); + function applyHTTPSOnyxUpdates(request: Request, response: Response) { console.debug('[OnyxUpdateManager] Applying https update'); // For most requests we can immediately update Onyx. For write requests we queue the updates and apply them after the sequential queue has flushed to prevent a replay effect in @@ -44,21 +48,18 @@ function applyHTTPSOnyxUpdates(request: Request, response: Response) { } function applyPusherOnyxUpdates(updates: OnyxUpdateEvent[]) { - return updates - .reduce( - (promise, update) => promise.then(() => PusherUtils.triggerMultiEventHandler(update.eventType, update.data)), - Promise.resolve().then(() => { - console.debug('[OnyxUpdateManager] Applying pusher update'); - }), - ) + pusherEventsPromise = pusherEventsPromise.then(() => { + console.debug('[OnyxUpdateManager] Applying pusher update'); + }); + + pusherEventsPromise = updates + .reduce((promise, update) => promise.then(() => PusherUtils.triggerMultiEventHandler(update.eventType, update.data)), pusherEventsPromise) .then(() => { console.debug('[OnyxUpdateManager] Done applying Pusher update'); }); -} -// This promise is used to ensure pusher events are always processed in the order they are received, -// even when such events are received over multiple separate pusher updates. -let applyPromise: Promise = Promise.resolve(); + return pusherEventsPromise; +} /** * @param [updateParams.request] Exists if updateParams.type === 'https' @@ -68,28 +69,21 @@ let applyPromise: Promise = Promise.resolve(); function apply({lastUpdateID, type, request, response, updates}: Merge): Promise; function apply({lastUpdateID, type, request, response, updates}: Merge): Promise; function apply({lastUpdateID, type, request, response, updates}: OnyxUpdatesFromServer): Promise | undefined { - applyPromise = applyPromise.then(() => { - console.debug(`[OnyxUpdateManager] Applying update type: ${type} with lastUpdateID: ${lastUpdateID}`, {request, response, updates}); - - if (lastUpdateID && lastUpdateIDAppliedToClient && Number(lastUpdateID) < lastUpdateIDAppliedToClient) { - console.debug('[OnyxUpdateManager] Update received was older than current state, returning without applying the updates'); - return Promise.resolve(); - } - if (lastUpdateID && (lastUpdateIDAppliedToClient === null || Number(lastUpdateID) > lastUpdateIDAppliedToClient)) { - return Onyx.merge(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, Number(lastUpdateID)); - } + console.debug(`[OnyxUpdateManager] Applying update type: ${type} with lastUpdateID: ${lastUpdateID}`, {request, response, updates}); + if (lastUpdateID && lastUpdateIDAppliedToClient && Number(lastUpdateID) < lastUpdateIDAppliedToClient) { + console.debug('[OnyxUpdateManager] Update received was older than current state, returning without applying the updates'); return Promise.resolve(); - }); - + } + if (lastUpdateID && (lastUpdateIDAppliedToClient === null || Number(lastUpdateID) > lastUpdateIDAppliedToClient)) { + Onyx.merge(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, Number(lastUpdateID)); + } if (type === CONST.ONYX_UPDATE_TYPES.HTTPS && request && response) { - applyPromise = applyPromise.then(() => applyHTTPSOnyxUpdates(request, response)); + return applyHTTPSOnyxUpdates(request, response); } if (type === CONST.ONYX_UPDATE_TYPES.PUSHER && updates) { - applyPromise = applyPromise.then(() => applyPusherOnyxUpdates(updates)); + return applyPusherOnyxUpdates(updates); } - - return applyPromise; } /**