Android: fix GetMissingOnyxUpdates in the background

src/libs/Notification/PushNotification/index.native.ts

@@ -10,7 +10,7 @@ import NotificationType from './NotificationType';
 import type {ClearNotifications, Deregister, Init, OnReceived, OnSelected, Register} from './types';
 import type PushNotificationType from './types';
 
-type NotificationEventActionCallback = (data: NotificationData) => void;
+type NotificationEventActionCallback = (data: NotificationData) => Promise<void>;
 
 type NotificationEventActionMap = Partial<Record<EventType, Record<string, NotificationEventActionCallback>>>;
 
@@ -56,7 +56,13 @@ function pushNotificationEventCallback(eventType: EventType, notification: PushP
         });
         return;
     }
-    action(data);
+
+    /**
+     * The action callback should return a promise. It's very important we return that promise so that
+     * when these callbacks are run in Android's background process (via Headless JS), the process waits
+     * for the promise to resolve before quitting
+     */
+    return action(data);
 }
 
 /**
@@ -83,15 +89,11 @@ function refreshNotificationOptInStatus() {
  */
 const init: Init = () => {
     // Setup event listeners
-    Airship.addListener(EventType.PushReceived, (notification) => {
-        pushNotificationEventCallback(EventType.PushReceived, notification.pushPayload);
-    });
+    Airship.addListener(EventType.PushReceived, (notification) => pushNotificationEventCallback(EventType.PushReceived, notification.pushPayload));
 
     // Note: the NotificationResponse event has a nested PushReceived event,
     // so event.notification refers to the same thing as notification above ^
-    Airship.addListener(EventType.NotificationResponse, (event) => {
-        pushNotificationEventCallback(EventType.NotificationResponse, event.pushPayload);
-    });
+    Airship.addListener(EventType.NotificationResponse, (event) => pushNotificationEventCallback(EventType.NotificationResponse, event.pushPayload));
 
     // Keep track of which users have enabled push notifications via an NVP.
     Airship.addListener(EventType.PushNotificationStatusChangedStatus, refreshNotificationOptInStatus);

src/libs/Notification/PushNotification/subscribeToReportCommentPushNotifications.ts

@@ -1,5 +1,5 @@
 import Onyx from 'react-native-onyx';
-import * as OnyxUpdates from '@libs/actions/OnyxUpdates';
+import applyOnyxUpdatesReliably from '@libs/actions/applyOnyxUpdatesReliably';
 import * as ActiveClientManager from '@libs/ActiveClientManager';
 import Log from '@libs/Log';
 import Navigation from '@libs/Navigation/Navigation';
@@ -26,35 +26,51 @@ Onyx.connect({
     },
 });
 
+function getLastUpdateIDAppliedToClient(): Promise<number> {
+    return new Promise((resolve) => {
+        Onyx.connect({
+            key: ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT,
+            callback: (value) => resolve(value ?? 0),
+        });
+    });
+}
+
 /**
  * Setup reportComment push notification callbacks.
  */
 export default function subscribeToReportCommentPushNotifications() {
     PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, reportActionID, onyxData, lastUpdateID, previousUpdateID}) => {
+        Log.info(`[PushNotification] received report comment notification in the ${Visibility.isVisible() ? 'foreground' : 'background'}`, false, {reportID, reportActionID});
+
         if (!ActiveClientManager.isClientTheLeader()) {
             Log.info('[PushNotification] received report comment notification, but ignoring it since this is not the active client');
-            return;
+            return Promise.resolve();
         }
-        Log.info(`[PushNotification] received report comment notification in the ${Visibility.isVisible() ? 'foreground' : 'background'}`, false, {reportID, reportActionID});
-
-        if (onyxData && lastUpdateID && previousUpdateID) {
-            Log.info('[PushNotification] reliable onyx update received', false, {lastUpdateID, previousUpdateID, onyxDataCount: onyxData?.length ?? 0});
 
-            const updates: OnyxUpdatesFromServer = {
-                type: CONST.ONYX_UPDATE_TYPES.AIRSHIP,
-                lastUpdateID,
-                previousUpdateID,
-                updates: [
-                    {
-                        eventType: 'eventType',
-                        data: onyxData,
-                    },
-                ],
-            };
-            OnyxUpdates.applyOnyxUpdatesReliably(updates);
-        } else {
-            Log.hmmm("[PushNotification] Didn't apply onyx updates because some data is missing", {lastUpdateID, previousUpdateID, onyxDataCount: onyxData?.length ?? 0});
+        if (!onyxData || !lastUpdateID || !previousUpdateID) {
+            Log.hmmm("[PushNotification] didn't apply onyx updates because some data is missing", {lastUpdateID, previousUpdateID, onyxDataCount: onyxData?.length ?? 0});
+            return Promise.resolve();
         }
+
+        Log.info('[PushNotification] reliable onyx update received', false, {lastUpdateID, previousUpdateID, onyxDataCount: onyxData?.length ?? 0});
+        const updates: OnyxUpdatesFromServer = {
+            type: CONST.ONYX_UPDATE_TYPES.AIRSHIP,
+            lastUpdateID,
+            previousUpdateID,
+            updates: [
+                {
+                    eventType: 'eventType',
+                    data: onyxData,
+                },
+            ],
+        };
+
+        /**
+         * When this callback runs in the background on Android (via Headless JS), no other Onyx.connect callbacks will run. This means that
+         * lastUpdateIDAppliedToClient will NOT be populated in other libs. To workaround this, we manually read the value here
+         * and pass it as a param
+         */
+        return getLastUpdateIDAppliedToClient().then((lastUpdateIDAppliedToClient) => applyOnyxUpdatesReliably(updates, true, lastUpdateIDAppliedToClient));
     });
 
     // Open correct report when push notification is clicked
@@ -96,5 +112,7 @@ export default function subscribeToReportCommentPushNotifications() {
                     }
                 });
             });
+
+        return Promise.resolve();
     });
 }

src/libs/Notification/PushNotification/types.ts

@@ -5,8 +5,8 @@ import type NotificationType from './NotificationType';
 type Init = () => void;
 type Register = (notificationID: string | number) => void;
 type Deregister = () => void;
-type OnReceived = <T extends ValueOf<typeof NotificationType>>(notificationType: T, callback: (data: NotificationDataMap[T]) => void) => void;
-type OnSelected = <T extends ValueOf<typeof NotificationType>>(notificationType: T, callback: (data: NotificationDataMap[T]) => void) => void;
+type OnReceived = <T extends ValueOf<typeof NotificationType>>(notificationType: T, callback: (data: NotificationDataMap[T]) => Promise<void>) => void;
+type OnSelected = <T extends ValueOf<typeof NotificationType>>(notificationType: T, callback: (data: NotificationDataMap[T]) => Promise<void>) => void;
 type ClearNotifications = () => void;
 
 type PushNotification = {

src/libs/actions/OnyxUpdateManager.ts

@@ -1,9 +1,11 @@
+import type {OnyxEntry} from 'react-native-onyx';
 import Onyx from 'react-native-onyx';
 import * as ActiveClientManager from '@libs/ActiveClientManager';
 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';
 
@@ -36,83 +38,94 @@ Onyx.connect({
     },
 });
 
-export default () => {
-    console.debug('[OnyxUpdateManager] Listening for updates from the server');
-    Onyx.connect({
-        key: ONYXKEYS.ONYX_UPDATES_FROM_SERVER,
-        callback: (value) => {
-            // When there's no value, there's nothing to process, so let's return early.
-            if (!value) {
-                return;
-            }
-            // If isLoadingApp is positive it means that OpenApp command hasn't finished yet, and in that case
-            // we don't have base state of the app (reports, policies, etc) setup. If we apply this update,
-            // we'll only have them overriten by the openApp response. So let's skip it and return.
-            if (isLoadingApp) {
-                // When ONYX_UPDATES_FROM_SERVER is set, we pause the queue. Let's unpause
-                // it so the app is not stuck forever without processing requests.
-                SequentialQueue.unpause();
-                console.debug(`[OnyxUpdateManager] Ignoring Onyx updates while OpenApp hans't finished yet.`);
-                return;
-            }
-            // This key is shared across clients, thus every client/tab will have a copy and try to execute this method.
-            // It is very important to only process the missing onyx updates from leader client otherwise requests we'll execute
-            // several duplicated requests that are not controlled by the SequentialQueue.
-            if (!ActiveClientManager.isClientTheLeader()) {
-                return;
-            }
+/**
+ *
+ * @param onyxUpdatesFromServer
+ * @param clientLastUpdateID an optional override for the lastUpdateIDAppliedToClient
+ * @returns
+ */
+function handleOnyxUpdateGap(onyxUpdatesFromServer: OnyxEntry<OnyxUpdatesFromServer>, clientLastUpdateID = 0) {
+    // When there's no value, there's nothing to process, so let's return early.
+    if (!onyxUpdatesFromServer) {
+        return;
+    }
+    // If isLoadingApp is positive it means that OpenApp command hasn't finished yet, and in that case
+    // we don't have base state of the app (reports, policies, etc) setup. If we apply this update,
+    // we'll only have them overriten by the openApp response. So let's skip it and return.
+    if (isLoadingApp) {
+        // When ONYX_UPDATES_FROM_SERVER is set, we pause the queue. Let's unpause
+        // it so the app is not stuck forever without processing requests.
+        SequentialQueue.unpause();
+        console.debug(`[OnyxUpdateManager] Ignoring Onyx updates while OpenApp hans't finished yet.`);
+        return;
+    }
+    // This key is shared across clients, thus every client/tab will have a copy and try to execute this method.
+    // It is very important to only process the missing onyx updates from leader client otherwise requests we'll execute
+    // several duplicated requests that are not controlled by the SequentialQueue.
+    if (!ActiveClientManager.isClientTheLeader()) {
+        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 (
-                !(typeof value === 'object' && !!value) ||
-                !('type' in value) ||
-                (!(value.type === CONST.ONYX_UPDATE_TYPES.HTTPS && value.request && value.response) &&
-                    !((value.type === CONST.ONYX_UPDATE_TYPES.PUSHER || value.type === CONST.ONYX_UPDATE_TYPES.AIRSHIP) && value.updates))
-            ) {
-                console.debug('[OnyxUpdateManager] Invalid format found for updates, cleaning and unpausing the queue');
-                Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null);
-                SequentialQueue.unpause();
-                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 (
+        !(typeof onyxUpdatesFromServer === 'object' && !!onyxUpdatesFromServer) ||
+        !('type' in onyxUpdatesFromServer) ||
+        (!(onyxUpdatesFromServer.type === CONST.ONYX_UPDATE_TYPES.HTTPS && onyxUpdatesFromServer.request && onyxUpdatesFromServer.response) &&
+            !((onyxUpdatesFromServer.type === CONST.ONYX_UPDATE_TYPES.PUSHER || onyxUpdatesFromServer.type === CONST.ONYX_UPDATE_TYPES.AIRSHIP) && onyxUpdatesFromServer.updates))
+    ) {
+        console.debug('[OnyxUpdateManager] Invalid format found for updates, cleaning and unpausing the queue');
+        Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null);
+        SequentialQueue.unpause();
+        return;
+    }
 
-            const updateParams = value;
-            const lastUpdateIDFromServer = value.lastUpdateID;
-            const previousUpdateIDFromServer = value.previousUpdateID;
+    const updateParams = onyxUpdatesFromServer;
+    const lastUpdateIDFromServer = onyxUpdatesFromServer.lastUpdateID;
+    const previousUpdateIDFromServer = onyxUpdatesFromServer.previousUpdateID;
+    const lastUpdateIDFromClient = clientLastUpdateID || lastUpdateIDAppliedToClient;
 
-            // In cases where we received a previousUpdateID and it doesn't match our lastUpdateIDAppliedToClient
-            // we need to perform one of the 2 possible cases:
-            //
-            // 1. This is the first time we're receiving an lastUpdateID, so we need to do a final reconnectApp before
-            // fully migrating to the reliable updates mode.
-            // 2. This client already has the reliable updates mode enabled, but it's missing some updates and it
-            // needs to fetch those.
-            let canUnpauseQueuePromise;
+    // In cases where we received a previousUpdateID and it doesn't match our lastUpdateIDAppliedToClient
+    // we need to perform one of the 2 possible cases:
+    //
+    // 1. This is the first time we're receiving an lastUpdateID, so we need to do a final reconnectApp before
+    // fully migrating to the reliable updates mode.
+    // 2. This client already has the reliable updates mode enabled, but it's missing some updates and it
+    // needs to fetch those.
+    let canUnpauseQueuePromise;
 
-            // 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');
+    // The flow below is setting the promise to a reconnect app to address flow (1) explained above.
+    if (!lastUpdateIDFromClient) {
+        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.
-                canUnpauseQueuePromise = App.finalReconnectAppAfterActivatingReliableUpdates();
-            } else {
-                // The flow below is setting the promise to a getMissingOnyxUpdates to address flow (2) explained above.
-                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,
-                });
-                canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDAppliedToClient, previousUpdateIDFromServer);
-            }
+        // 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.
+        console.debug(`[OnyxUpdateManager] Client is behind the server by ${Number(previousUpdateIDFromServer) - lastUpdateIDFromClient} so fetching incremental updates`);
+        Log.info('Gap detected in update IDs from server so fetching incremental updates', true, {
+            lastUpdateIDFromServer,
+            previousUpdateIDFromServer,
+            lastUpdateIDFromClient,
+        });
+        canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDFromClient, previousUpdateIDFromServer);
+    }
 
-            canUnpauseQueuePromise.finally(() => {
-                OnyxUpdates.apply(updateParams).finally(() => {
-                    console.debug('[OnyxUpdateManager] Done applying all updates');
-                    Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null);
-                    SequentialQueue.unpause();
-                });
-            });
-        },
+    canUnpauseQueuePromise.finally(() => {
+        OnyxUpdates.apply(updateParams).finally(() => {
+            console.debug('[OnyxUpdateManager] Done applying all updates');
+            Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null);
+            SequentialQueue.unpause();
+        });
+    });
+}
+
+export default () => {
+    console.debug('[OnyxUpdateManager] Listening for updates from the server');
+    Onyx.connect({
+        key: ONYXKEYS.ONYX_UPDATES_FROM_SERVER,
+        callback: (value) => handleOnyxUpdateGap(value),
     });
 };
+
+export {handleOnyxUpdateGap};

src/libs/actions/OnyxUpdates.ts

@@ -132,29 +132,23 @@ function saveUpdateInformation(updateParams: OnyxUpdatesFromServer) {
  * This function will receive the previousUpdateID from any request/pusher update that has it, compare to our current app state
  * and return if an update is needed
  * @param previousUpdateID The previousUpdateID contained in the response object
+ * @param clientLastUpdateID an optional override for the lastUpdateIDAppliedToClient
  */
-function doesClientNeedToBeUpdated(previousUpdateID = 0): boolean {
+function doesClientNeedToBeUpdated(previousUpdateID = 0, clientLastUpdateID = 0): boolean {
     // If no previousUpdateID is sent, this is not a WRITE request so we don't need to update our current state
     if (!previousUpdateID) {
         return false;
     }
 
-    // If we don't have any value in lastUpdateIDAppliedToClient, this is the first time we're receiving anything, so we need to do a last reconnectApp
-    if (!lastUpdateIDAppliedToClient) {
+    const lastUpdateIDFromClient = clientLastUpdateID || lastUpdateIDAppliedToClient;
+
+    // If we don't have any value in lastUpdateIDFromClient, this is the first time we're receiving anything, so we need to do a last reconnectApp
+    if (!lastUpdateIDFromClient) {
         return true;
     }
 
-    return lastUpdateIDAppliedToClient < previousUpdateID;
-}
-
-function applyOnyxUpdatesReliably(updates: OnyxUpdatesFromServer) {
-    const previousUpdateID = Number(updates.previousUpdateID) || 0;
-    if (!doesClientNeedToBeUpdated(previousUpdateID)) {
-        apply(updates);
-        return;
-    }
-    saveUpdateInformation(updates);
+    return lastUpdateIDFromClient < previousUpdateID;
 }
 
 // eslint-disable-next-line import/prefer-default-export
-export {saveUpdateInformation, doesClientNeedToBeUpdated, apply, applyOnyxUpdatesReliably};
+export {saveUpdateInformation, doesClientNeedToBeUpdated, apply};

src/libs/actions/User.ts

@@ -44,8 +44,8 @@ import type ReportAction from '@src/types/onyx/ReportAction';
 import type {OriginalMessage} from '@src/types/onyx/ReportAction';
 import {isEmptyObject} from '@src/types/utils/EmptyObject';
 import type {EmptyObject} from '@src/types/utils/EmptyObject';
+import applyOnyxUpdatesReliably from './applyOnyxUpdatesReliably';
 import * as Link from './Link';
-import * as OnyxUpdates from './OnyxUpdates';
 import * as Report from './Report';
 import * as Session from './Session';
 
@@ -597,7 +597,7 @@ function subscribeToUserEvents() {
             updates: pushJSON.updates ?? [],
             previousUpdateID: Number(pushJSON.previousUpdateID || 0),
         };
-        OnyxUpdates.applyOnyxUpdatesReliably(updates);
+        applyOnyxUpdatesReliably(updates);
     });
 
     // Handles Onyx updates coming from Pusher through the mega multipleEvents.