Merge pull request #1597 from OneSignal/generate-android-id

Fix issue where a notification's android ID is not available to OSRemoteNotificationReceivedHandler

Author: brismithers

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

@@ -525,7 +525,7 @@ private static void createSummaryNotification(OSNotificationGenerationJob notifi
          String[] whereArgs = { group };
 
          // Make sure to omit any old existing matching android ids in-case we are replacing it.
-         if (!updateSummary && notificationJob.getAndroidId() != -1)
+         if (!updateSummary)
             whereStr += " AND " + NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID + " <> " + notificationJob.getAndroidId();
 
          cursor = dbHelper.query(

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

@@ -136,12 +136,12 @@ private static int processJobForDisplay(OSNotificationController notificationCon
 
         processCollapseKey(notificationJob);
 
-        int androidNotificationId = notificationJob.getAndroidIdWithoutCreate();
+        int androidNotificationId = notificationJob.getAndroidId();
         boolean doDisplay = shouldDisplayNotification(notificationJob);
         boolean notificationDisplayed = false;
 
         if (doDisplay) {
-            androidNotificationId = notificationJob.getAndroidId();
+            notificationJob.setIsNotificationToDisplay(true);
             if (fromBackgroundLogic && OneSignal.shouldFireForegroundHandlers(notificationJob)) {
                 notificationController.setFromBackgroundLogic(false);
                 OneSignal.fireForegroundHandlers(notificationController);
@@ -206,10 +206,10 @@ private static void saveNotification(OSNotificationGenerationJob notificationJob
 
          OneSignalDbHelper dbHelper = OneSignalDbHelper.getInstance(notificationJob.getContext());
 
-         // Count any notifications with duplicated android notification ids as dismissed.
-         // -1 is used to note never displayed
+         // When notification was displayed, count any notifications with duplicated android
+         // notification ids as dismissed.
          if (notificationJob.isNotificationToDisplay()) {
-            String whereStr = NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID + " = " + notificationJob.getAndroidIdWithoutCreate();
+            String whereStr = NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID + " = " + notificationJob.getAndroidId();
 
             ContentValues values = new ContentValues();
             values.put(NotificationTable.COLUMN_NAME_DISMISSED, 1);
@@ -228,7 +228,7 @@ private static void saveNotification(OSNotificationGenerationJob notificationJob
 
          values.put(NotificationTable.COLUMN_NAME_OPENED, opened ? 1 : 0);
          if (!opened)
-            values.put(NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID, notificationJob.getAndroidIdWithoutCreate());
+            values.put(NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID, notificationJob.getAndroidId());
 
          if (notificationJob.getTitle() != null)
             values.put(NotificationTable.COLUMN_NAME_TITLE, notificationJob.getTitle().toString());
@@ -253,11 +253,11 @@ private static void saveNotification(OSNotificationGenerationJob notificationJob
    }
 
     static void markNotificationAsDismissed(OSNotificationGenerationJob notifiJob) {
-        if (notifiJob.getAndroidIdWithoutCreate() == -1)
+        if (!notifiJob.isNotificationToDisplay())
             return;
 
         OneSignal.Log(OneSignal.LOG_LEVEL.DEBUG, "Marking restored or disabled notifications as dismissed: " + notifiJob.toString());
-        String whereStr = NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID + " = " + notifiJob.getAndroidIdWithoutCreate();
+        String whereStr = NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID + " = " + notifiJob.getAndroidId();
 
         OneSignalDbHelper dbHelper = OneSignalDbHelper.getInstance(notifiJob.getContext());
 
@@ -351,7 +351,7 @@ private static void processCollapseKey(OSNotificationGenerationJob notificationJ
 
         if (cursor.moveToFirst()) {
             int androidNotificationId = cursor.getInt(cursor.getColumnIndex(NotificationTable.COLUMN_NAME_ANDROID_NOTIFICATION_ID));
-            notificationJob.setAndroidIdWithoutOverriding(androidNotificationId);
+            notificationJob.getNotification().setAndroidNotificationId(androidNotificationId);
         }
 
         cursor.close();

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

@@ -113,8 +113,8 @@ private void notDisplayNotificationLogic(OSNotification originalNotification) {
          // This will prevent it from being restored again
          NotificationBundleProcessor.markNotificationAsDismissed(notificationJob);
       } else {
-         // -1 is used to note never displayed
-         notificationJob.getNotification().setAndroidNotificationId(-1);
+         // indicate the notification job did not display
+         notificationJob.setIsNotificationToDisplay(false);
          NotificationBundleProcessor.processNotification(notificationJob, true, false);
          OneSignal.handleNotificationReceived(notificationJob);
       }

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

@@ -40,6 +40,7 @@ public class OSNotificationGenerationJob {
     private Context context;
     private JSONObject jsonPayload;
     private boolean restoring;
+    private boolean isNotificationToDisplay;
 
     private Long shownTimeStamp;
 
@@ -61,7 +62,7 @@ public class OSNotificationGenerationJob {
     OSNotificationGenerationJob(Context context, OSNotification notification, JSONObject jsonPayload) {
         this.context = context;
         this.jsonPayload = jsonPayload;
-        this.notification = notification;
+        this.setNotification(notification);
     }
 
     /**
@@ -90,10 +91,17 @@ JSONObject getAdditionalData() {
     }
 
     /**
-     * If androidNotificationId is -1 then the notification is a silent one
+     * Determine whether this notification has been displayed.
      */
     boolean isNotificationToDisplay() {
-        return getAndroidIdWithoutCreate() != -1;
+        return isNotificationToDisplay;
+    }
+
+    /**
+     * Indicate whether notification has been displayed.
+     */
+    void setIsNotificationToDisplay(boolean isNotificationToDisplay) {
+        this.isNotificationToDisplay = isNotificationToDisplay;
     }
 
     boolean hasExtender() {
@@ -104,35 +112,24 @@ String getApiNotificationId() {
         return OneSignal.getNotificationIdFromFCMJson(jsonPayload);
     }
 
-    int getAndroidIdWithoutCreate() {
-        if (!notification.hasNotificationId())
-            return -1;
-
-        return notification.getAndroidNotificationId();
-    }
-
     Integer getAndroidId() {
-        if (!notification.hasNotificationId())
-            notification.setAndroidNotificationId(new SecureRandom().nextInt());
-
         return notification.getAndroidNotificationId() ;
     }
 
-    void setAndroidIdWithoutOverriding(Integer id) {
-        if (id == null)
-            return;
-
-        if (notification.hasNotificationId())
-            return;
-
-        notification.setAndroidNotificationId(id);
-    }
-
     public OSNotification getNotification() {
         return notification;
     }
 
     public void setNotification(OSNotification notification) {
+        // If there is no android ID on the notification coming in, create one either
+        // copying from the previous one or generating a new one.
+        if (notification != null && !notification.hasNotificationId()) {
+            if (this.notification != null && this.notification.hasNotificationId())
+                notification.setAndroidNotificationId(this.notification.getAndroidNotificationId());
+            else
+                notification.setAndroidNotificationId(new SecureRandom().nextInt());
+        }
+
         this.notification = notification;
     }
 
@@ -221,6 +218,7 @@ public String toString() {
         return "OSNotificationGenerationJob{" +
                 "jsonPayload=" + jsonPayload +
                 ", isRestoring=" + restoring +
+                ", isNotificationToDisplay=" + isNotificationToDisplay +
                 ", shownTimeStamp=" + shownTimeStamp +
                 ", overriddenBodyFromExtender=" + overriddenBodyFromExtender +
                 ", overriddenTitleFromExtender=" + overriddenTitleFromExtender +