Merge pull request #1484 from OneSignal/fix/notification-processed-twice

Use two `Workers` to display notification and send receive receipt

Author: nan-li

OneSignalSDK/build.gradle

@@ -8,7 +8,6 @@ buildscript {
                 targetSdkVersion: 30
         ]
         androidGradlePluginVersion = '3.6.2'
-        androidConcurrentFutures = '1.1.0'
         googleServicesGradlePluginVersion = '4.3.2'
         huaweiAgconnectVersion = '1.2.1.301'
         huaweiHMSPushVersion = '5.3.0.304'

OneSignalSDK/onesignal/build.gradle

@@ -48,8 +48,6 @@ dependencies {
     // Required for GoogleApiAvailability
     implementation 'com.google.android.gms:play-services-base:[17.0.0, 17.6.99]'
 
-    implementation("androidx.concurrent:concurrent-futures:$androidConcurrentFutures")
-
     // firebase-messaging:18.0.0 is the last version before going to AndroidX
     // firebase-messaging:19.0.0 is the first version using AndroidX
     api 'com.google.firebase:firebase-messaging:[19.0.0, 22.0.99]'

OneSignalSDK/onesignal/src/main/java/com/onesignal/NotificationBundleProcessor.java

@@ -38,8 +38,6 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.WorkerThread;
-import androidx.concurrent.futures.CallbackToFutureAdapter;
-import androidx.work.ListenableWorker;
 
 import com.onesignal.OneSignalDbContract.NotificationTable;
 
@@ -100,8 +98,7 @@ public void onResult(boolean result) {
                             jsonStrPayload,
                             shownTimeStamp,
                             isRestoring,
-                            false,
-                            true);
+                            false);
 
                     // Delay to prevent CPU spikes.
                     // Normally more than one notification is restored at a time.
@@ -164,11 +161,6 @@ private static int processJobForDisplay(OSNotificationController notificationCon
             String osNotificationId = OSNotificationFormatHelper.getOSNotificationIdFromJson(notificationController.getNotificationJob().getJsonPayload());
             OSNotificationWorkManager.removeNotificationIdProcessed(osNotificationId);
             OneSignal.handleNotificationReceived(notificationJob);
-        } else {
-            CallbackToFutureAdapter.Completer<ListenableWorker.Result>  callbackCompleter = notificationJob.getCallbackCompleter();
-            OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "Process notification restored or IAM with callback completer: " + callbackCompleter);
-            if (callbackCompleter != null)
-                callbackCompleter.set(ListenableWorker.Result.success());
         }
 
         return androidNotificationId;
@@ -183,22 +175,18 @@ private static boolean shouldDisplayNotification(OSNotificationGenerationJob not
      */
     static void processNotification(OSNotificationGenerationJob notificationJob, boolean opened, boolean notificationDisplayed) {
         saveNotification(notificationJob, opened);
-        CallbackToFutureAdapter.Completer<ListenableWorker.Result>  callbackCompleter = notificationJob.getCallbackCompleter();
 
         if (!notificationDisplayed) {
             // Notification channel disable or not displayed
             // save notification as dismissed to avoid user re-enabling channel and notification being displayed due to restore
             markNotificationAsDismissed(notificationJob);
-
-            OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "Process notification not displayed with callback completer: " + callbackCompleter);
-            if (callbackCompleter != null)
-                callbackCompleter.set(ListenableWorker.Result.success());
             return;
         }
 
         // Logic for when the notification is displayed
         String notificationId = notificationJob.getApiNotificationId();
-        OSReceiveReceiptController.getInstance().sendReceiveReceipt(callbackCompleter, notificationId);
+        Context context = notificationJob.getContext();
+        OSReceiveReceiptController.getInstance().beginEnqueueingWork(context, notificationId);
         OneSignal.getSessionManager().onNotificationReceived(notificationId);
     }
 
@@ -439,8 +427,7 @@ public void onResult(boolean result) {
                         jsonPayload.toString(),
                         timestamp,
                         isRestoring,
-                        isHighPriority,
-                        true);
+                        isHighPriority);
 
                 bundleResult.setWorkManagerProcessing(true);
                 notificationProcessingCallback.onResult(true);

OneSignalSDK/onesignal/src/main/java/com/onesignal/NotificationSummaryManager.java

@@ -146,7 +146,7 @@ private static void restoreSummary(Context context, String group) {
              null
          );
 
-         OSNotificationRestoreWorkManager.showNotificationsFromCursor(context, cursor, 0, true);
+         OSNotificationRestoreWorkManager.showNotificationsFromCursor(context, cursor, 0);
       } catch (Throwable t) {
          OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error restoring notification records! ", t);
       } finally {

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSDelayTaskController.kt

@@ -34,8 +34,6 @@
 import android.content.Context;
 
 import androidx.annotation.Nullable;
-import androidx.concurrent.futures.CallbackToFutureAdapter;
-import androidx.work.ListenableWorker;
 
 import org.json.JSONObject;
 
@@ -46,7 +44,6 @@ public class OSNotificationController {
    static final String GOOGLE_SENT_TIME_KEY = "google.sent_time";
    static final String GOOGLE_TTL_KEY = "google.ttl";
 
-   private final CallbackToFutureAdapter.Completer<ListenableWorker.Result> callbackCompleter;
    private final OSNotificationGenerationJob notificationJob;
    private boolean restoring;
    private boolean fromBackgroundLogic;
@@ -55,12 +52,9 @@ public class OSNotificationController {
       this.restoring = restoring;
       this.fromBackgroundLogic = fromBackgroundLogic;
       this.notificationJob = notificationJob;
-      this.callbackCompleter = notificationJob.getCallbackCompleter();
    }
 
-   OSNotificationController(CallbackToFutureAdapter.Completer<ListenableWorker.Result> callbackCompleter,
-                            Context context, OSNotification notification, JSONObject jsonPayload, boolean restoring, boolean fromBackgroundLogic, Long timestamp) {
-      this.callbackCompleter = callbackCompleter;
+   OSNotificationController(Context context, OSNotification notification, JSONObject jsonPayload, boolean restoring, boolean fromBackgroundLogic, Long timestamp) {
       this.restoring = restoring;
       this.fromBackgroundLogic = fromBackgroundLogic;
 
@@ -74,7 +68,7 @@ public class OSNotificationController {
     * @see OSNotificationGenerationJob
     */
    private OSNotificationGenerationJob createNotificationJobFromCurrent(Context context, OSNotification notification, JSONObject jsonPayload, Long timestamp) {
-      OSNotificationGenerationJob notificationJob = new OSNotificationGenerationJob(callbackCompleter, context);
+      OSNotificationGenerationJob notificationJob = new OSNotificationGenerationJob(context);
       notificationJob.setJsonPayload(jsonPayload);
       notificationJob.setShownTimeStamp(timestamp);
       notificationJob.setRestoring(restoring);

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationController.java

@@ -30,17 +30,12 @@
 import android.content.Context;
 import android.net.Uri;
 
-import androidx.annotation.Nullable;
-import androidx.concurrent.futures.CallbackToFutureAdapter;
-import androidx.work.ListenableWorker;
-
 import org.json.JSONObject;
 
 import java.security.SecureRandom;
 
 public class OSNotificationGenerationJob {
 
-    private CallbackToFutureAdapter.Completer<ListenableWorker.Result> callbackCompleter;
     private OSNotification notification;
     private Context context;
     private JSONObject jsonPayload;
@@ -56,11 +51,6 @@ public class OSNotificationGenerationJob {
     private Uri orgSound;
 
     OSNotificationGenerationJob(Context context) {
-        this(null, context);
-    }
-
-    OSNotificationGenerationJob(CallbackToFutureAdapter.Completer<ListenableWorker.Result> callbackCompleter, Context context) {
-        this.callbackCompleter = callbackCompleter;
         this.context = context;
     }
 
@@ -74,11 +64,6 @@ public class OSNotificationGenerationJob {
         this.notification = notification;
     }
 
-    @Nullable
-    public CallbackToFutureAdapter.Completer<ListenableWorker.Result> getCallbackCompleter() {
-        return callbackCompleter;
-    }
-
     /**
      * Get the notification title from the payload
      */

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationGenerationJob.java

@@ -104,7 +104,7 @@ private static void queryAndRestoreNotificationsAndBadgeCount(
                     OneSignalDbContract.NotificationTable._ID + " DESC", // sort order, new to old
                     NotificationLimitManager.MAX_NUMBER_OF_NOTIFICATIONS_STR // limit
             );
-            showNotificationsFromCursor(context, cursor, DELAY_BETWEEN_NOTIFICATION_RESTORES_MS, false);
+            showNotificationsFromCursor(context, cursor, DELAY_BETWEEN_NOTIFICATION_RESTORES_MS);
             BadgeCountUpdater.update(dbHelper, context);
         } catch (Throwable t) {
             OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "Error restoring notification records! ", t);
@@ -144,7 +144,7 @@ private static void skipVisibleNotifications(Context context, StringBuilder dbQu
      * @param cursor - Source cursor to generate notifications from
      * @param delay - Delay to slow down process to ensure we don't spike CPU and I/O on the device
      */
-    static void showNotificationsFromCursor(Context context, Cursor cursor, int delay, boolean needsWorkerThread) {
+    static void showNotificationsFromCursor(Context context, Cursor cursor, int delay) {
         if (!cursor.moveToFirst())
             return;
 
@@ -161,8 +161,7 @@ static void showNotificationsFromCursor(Context context, Cursor cursor, int dela
                     fullData,
                     dateTime,
                     true,
-                    false,
-                    needsWorkerThread
+                    false
             );
 
             if (delay > 0)

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationRestoreWorkManager.java

@@ -3,17 +3,13 @@
 import android.content.Context;
 
 import androidx.annotation.NonNull;
-import androidx.annotation.Nullable;
-import androidx.concurrent.futures.CallbackToFutureAdapter;
 import androidx.work.Data;
 import androidx.work.ExistingWorkPolicy;
-import androidx.work.ListenableWorker;
 import androidx.work.OneTimeWorkRequest;
 import androidx.work.WorkManager;
+import androidx.work.Worker;
 import androidx.work.WorkerParameters;
 
-import com.google.common.util.concurrent.ListenableFuture;
-
 import org.json.JSONException;
 import org.json.JSONObject;
 
@@ -23,7 +19,6 @@
 
 class OSNotificationWorkManager {
 
-    private static final String OS_NOTIFICATION_ID = "os_bnotification_id";
     private static final String ANDROID_NOTIF_ID_WORKER_DATA_PARAM = "android_notif_id";
     private static final String JSON_PAYLOAD_WORKER_DATA_PARAM = "json_payload";
     private static final String TIMESTAMP_WORKER_DATA_PARAM = "timestamp";
@@ -54,20 +49,9 @@ static void removeNotificationIdProcessed(String osNotificationId) {
     }
 
     static void beginEnqueueingWork(Context context, String osNotificationId, int androidNotificationId, String jsonPayload, long timestamp,
-                                    boolean isRestoring, boolean isHighPriority, boolean needsWorkerThread) {
-        if (!needsWorkerThread) {
-            try {
-                JSONObject jsonPayloadObject = new JSONObject(jsonPayload);
-                processNotificationData(context, androidNotificationId, jsonPayloadObject, isRestoring, timestamp);
-            } catch (JSONException e) {
-                OneSignal.onesignalLog(OneSignal.LOG_LEVEL.ERROR, "Error occurred parsing jsonPayload to JSONObject in beginEnqueueingWork e: " + e.getMessage());
-                e.printStackTrace();
-            }
-            return;
-        }
+                                    boolean isRestoring, boolean isHighPriority) {
         // TODO: Need to figure out how to implement the isHighPriority param
         Data inputData = new Data.Builder()
-                .putString(OS_NOTIFICATION_ID, osNotificationId)
                 .putInt(ANDROID_NOTIF_ID_WORKER_DATA_PARAM, androidNotificationId)
                 .putString(JSON_PAYLOAD_WORKER_DATA_PARAM, jsonPayload)
                 .putLong(TIMESTAMP_WORKER_DATA_PARAM, timestamp)
@@ -83,29 +67,15 @@ static void beginEnqueueingWork(Context context, String osNotificationId, int an
                 .enqueueUniqueWork(osNotificationId, ExistingWorkPolicy.KEEP, workRequest);
     }
 
-    public static class NotificationWorker extends ListenableWorker {
+    public static class NotificationWorker extends Worker {
 
         public NotificationWorker(@NonNull Context context, @NonNull WorkerParameters workerParams) {
             super(context, workerParams);
         }
 
         @NonNull
         @Override
-        public ListenableFuture<Result> startWork() {
-            return CallbackToFutureAdapter.getFuture(new CallbackToFutureAdapter.Resolver<Result>() {
-                @Nullable
-                @Override
-                public Object attachCompleter(@NonNull CallbackToFutureAdapter.Completer<Result> completer) throws Exception {
-                    String notificationId = NotificationWorker.this.doWork(completer);
-
-                    // This value is used only for debug purposes: it will be used
-                    // in toString() of returned future or error cases.
-                    return "NotificationWorkerFutureCallback_" + notificationId;
-                }
-            });
-        }
-
-        private String doWork(@NonNull CallbackToFutureAdapter.Completer<Result> completer) {
+        public Result doWork() {
             Data inputData = getInputData();
             try {
                 OneSignal.onesignalLog(OneSignal.LOG_LEVEL.DEBUG, "NotificationWorker running doWork with data: " + inputData);
@@ -116,7 +86,6 @@ private String doWork(@NonNull CallbackToFutureAdapter.Completer<Result> complet
                 boolean isRestoring = inputData.getBoolean(IS_RESTORING_WORKER_DATA_PARAM, false);
 
                 processNotificationData(
-                        completer,
                         getApplicationContext(),
                         androidNotificationId,
                         jsonPayload,
@@ -125,36 +94,30 @@ private String doWork(@NonNull CallbackToFutureAdapter.Completer<Result> complet
             } catch (JSONException e) {
                 OneSignal.onesignalLog(OneSignal.LOG_LEVEL.ERROR, "Error occurred doing work for job with id: " + getId().toString());
                 e.printStackTrace();
-                completer.setException(e);
+                return Result.failure();
             }
-            return inputData.getString(OS_NOTIFICATION_ID);
+            return Result.success();
         }
-    }
 
-    static void processNotificationData(Context context, int androidNotificationId, JSONObject jsonPayload,
-                                        boolean isRestoring, Long timestamp) {
-        processNotificationData(null, context, androidNotificationId, jsonPayload, isRestoring, timestamp);
-    }
+        private void processNotificationData(Context context, int androidNotificationId, JSONObject jsonPayload,
+                                             boolean isRestoring, Long timestamp) {
+            OSNotification notification = new OSNotification(null, jsonPayload, androidNotificationId);
+            OSNotificationController controller = new OSNotificationController(context, notification, jsonPayload, isRestoring, true, timestamp);
+            OSNotificationReceivedEvent notificationReceived = new OSNotificationReceivedEvent(controller, notification);
 
-    static void processNotificationData(CallbackToFutureAdapter.Completer<ListenableWorker.Result> completer,
-                                 Context context, int androidNotificationId, JSONObject jsonPayload,
-                                 boolean isRestoring, Long timestamp) {
-        OSNotification notification = new OSNotification(null, jsonPayload, androidNotificationId);
-        OSNotificationController controller = new OSNotificationController(completer, context, notification, jsonPayload, isRestoring, true, timestamp);
-        OSNotificationReceivedEvent notificationReceived = new OSNotificationReceivedEvent(controller, notification);
+            if (OneSignal.remoteNotificationReceivedHandler != null)
+                try {
+                    OneSignal.remoteNotificationReceivedHandler.remoteNotificationReceived(context, notificationReceived);
+                } catch (Throwable t) {
+                    OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "remoteNotificationReceived throw an exception. Displaying normal OneSignal notification.", t);
+                    notificationReceived.complete(notification);
 
-        if (OneSignal.remoteNotificationReceivedHandler != null)
-            try {
-                OneSignal.remoteNotificationReceivedHandler.remoteNotificationReceived(context, notificationReceived);
-            } catch (Throwable t) {
-                OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "remoteNotificationReceived throw an exception. Displaying normal OneSignal notification.", t);
+                    throw t;
+                }
+            else {
+                OneSignal.Log(OneSignal.LOG_LEVEL.WARN, "remoteNotificationReceivedHandler not setup, displaying normal OneSignal notification");
                 notificationReceived.complete(notification);
-
-                throw t;
             }
-        else {
-            OneSignal.Log(OneSignal.LOG_LEVEL.WARN, "remoteNotificationReceivedHandler not setup, displaying normal OneSignal notification");
-            notificationReceived.complete(notification);
         }
     }
 }