Merge pull request #203 from Unity-Technologies/ensure-unique-generated-id-remake

Scheduling tracking refactoring and unique ids (remake)

Author: aurimasc

com.unity.mobile.notifications/Runtime/Android/AndroidNotificationCenter.cs

@@ -114,7 +114,7 @@ void CollectMethods(AndroidJavaClass clazz)
             setNotificationGroupAlertBehavior = JniApi.FindMethod(clazz, "setNotificationGroupAlertBehavior", "(Landroid/app/Notification$Builder;I)V", true);
             getNotificationGroupAlertBehavior = JniApi.FindMethod(clazz, "getNotificationGroupAlertBehavior", "(Landroid/app/Notification;)I", true);
             getNotificationChannelId = JniApi.FindMethod(clazz, "getNotificationChannelId", "(Landroid/app/Notification;)Ljava/lang/String;", true);
-            scheduleNotification = JniApi.FindMethod(clazz, "scheduleNotification", "(Landroid/app/Notification$Builder;)V", false);
+            scheduleNotification = JniApi.FindMethod(clazz, "scheduleNotification", "(Landroid/app/Notification$Builder;)I", false);
             createNotificationBuilder = JniApi.FindMethod(clazz, "createNotificationBuilder", "(Ljava/lang/String;)Landroid/app/Notification$Builder;", false);
         }
 
@@ -195,9 +195,9 @@ public void DeleteNotificationChannel(string channelId)
             self.Call("deleteNotificationChannel", channelId);
         }
 
-        public void ScheduleNotification(AndroidJavaObject notificationBuilder)
+        public int ScheduleNotification(AndroidJavaObject notificationBuilder)
         {
-            self.Call(scheduleNotification, notificationBuilder);
+            return self.Call<int>(scheduleNotification, notificationBuilder);
         }
 
         public bool CheckIfPendingNotificationIsRegistered(int id)
@@ -680,12 +680,9 @@ public static int SendNotification(AndroidNotification notification, string chan
             if (!Initialize())
                 return -1;
 
-            // Now.ToString("yyMMddHHmmssffffff"), but avoiding any culture-related formatting or dependencies
-            var now = DateTime.UtcNow;
-            var nowFormatted = $"{now.Year}{now.Month}{now.Day}{now.Hour}{now.Minute}{now.Second}{now.Millisecond}";
-            int id = Math.Abs(nowFormatted.GetHashCode()) + (new System.Random().Next(10000));
-            using (var builder = CreateNotificationBuilder(id, notification, channelId))
-                SendNotification(builder);
+            int id;
+            using (var builder = CreateNotificationBuilder(notification, channelId))
+                SendNotification(builder, out id);
 
             return id;
         }
@@ -716,6 +713,18 @@ public static void SendNotification(AndroidJavaObject notificationBuilder)
                 s_Jni.NotificationManager.ScheduleNotification(notificationBuilder);
         }
 
+        /// <summary>
+        /// Schedule a notification created using the provided Notification.Builder object.
+        /// Notification builder should be created by calling CreateNotificationBuilder.
+        /// Stores the notification id to the second argument
+        /// </summary>
+        public static void SendNotification(AndroidJavaObject notificationBuilder, out int id)
+        {
+            id = -1;
+            if (Initialize())
+                id = s_Jni.NotificationManager.ScheduleNotification(notificationBuilder);
+        }
+
         /// <summary>
         /// Update an already scheduled notification.
         /// If a notification with the specified id was already scheduled it will be overridden with the information from the passed notification struct.
@@ -860,8 +869,11 @@ public static void OpenNotificationSettings(string channelId = null)
         /// </summary>
         public static AndroidJavaObject CreateNotificationBuilder(AndroidNotification notification, string channelId)
         {
-            int id = Math.Abs(DateTime.Now.ToString("yyMMddHHmmssffffff").GetHashCode()) + (new System.Random().Next(10000));
-            return CreateNotificationBuilder(id, notification, channelId);
+            AndroidJavaObject builder, extras;
+            CreateNotificationBuilder(notification, channelId, out builder, out extras);
+            if (extras != null)
+                extras.Dispose();
+            return builder;
         }
 
         /// <summary>
@@ -873,6 +885,24 @@ public static AndroidJavaObject CreateNotificationBuilder(AndroidNotification no
         /// <returns>A proxy object for created Notification.Builder</returns>
         public static AndroidJavaObject CreateNotificationBuilder(int id, AndroidNotification notification, string channelId)
         {
+            AndroidJavaObject builder, extras;
+            CreateNotificationBuilder(notification, channelId, out builder, out extras);
+            if (extras != null)
+            {
+                s_Jni.Bundle.PutInt(extras, s_Jni.NotificationManager.KEY_ID, id);
+                extras.Dispose();
+            }
+            return builder;
+        }
+
+        static void CreateNotificationBuilder(AndroidNotification notification, string channelId, out AndroidJavaObject notificationBuilder, out AndroidJavaObject extras)
+        {
+            if (!Initialize())
+            {
+                notificationBuilder = extras = null;
+                return;
+            }
+
             long fireTime = notification.FireTime.ToLong();
             if (fireTime < 0L)
             {
@@ -881,7 +911,7 @@ public static AndroidJavaObject CreateNotificationBuilder(int id, AndroidNotific
 
             // NOTE: JNI calls are expensive, so we avoid calls that set something that is also a default
 
-            var notificationBuilder = s_Jni.NotificationManager.CreateNotificationBuilder(channelId);
+            notificationBuilder = s_Jni.NotificationManager.CreateNotificationBuilder(channelId);
             s_Jni.NotificationManager.SetNotificationIcon(notificationBuilder, s_Jni.NotificationManager.KEY_SMALL_ICON, notification.SmallIcon);
             if (!string.IsNullOrEmpty(notification.LargeIcon))
                 s_Jni.NotificationManager.SetNotificationIcon(notificationBuilder, s_Jni.NotificationManager.KEY_LARGE_ICON, notification.LargeIcon);
@@ -919,17 +949,12 @@ public static AndroidJavaObject CreateNotificationBuilder(int id, AndroidNotific
             if (notification.GroupAlertBehaviour != GroupAlertBehaviours.GroupAlertAll)  // All is default value
                 s_Jni.NotificationManager.SetNotificationGroupAlertBehavior(notificationBuilder, (int)notification.GroupAlertBehaviour);
 
-            using (var extras = s_Jni.NotificationBuilder.GetExtras(notificationBuilder))
-            {
-                s_Jni.Bundle.PutInt(extras, s_Jni.NotificationManager.KEY_ID, id);
-                s_Jni.Bundle.PutLong(extras, s_Jni.NotificationManager.KEY_REPEAT_INTERVAL, notification.RepeatInterval.ToLong());
-                s_Jni.Bundle.PutLong(extras, s_Jni.NotificationManager.KEY_FIRE_TIME, fireTime);
-                s_Jni.Bundle.PutBoolean(extras, s_Jni.NotificationManager.KEY_SHOW_IN_FOREGROUND, notification.ShowInForeground);
-                if (!string.IsNullOrEmpty(notification.IntentData))
-                    s_Jni.Bundle.PutString(extras, s_Jni.NotificationManager.KEY_INTENT_DATA, notification.IntentData);
-            }
-
-            return notificationBuilder;
+            extras = s_Jni.NotificationBuilder.GetExtras(notificationBuilder);
+            s_Jni.Bundle.PutLong(extras, s_Jni.NotificationManager.KEY_REPEAT_INTERVAL, notification.RepeatInterval.ToLong());
+            s_Jni.Bundle.PutLong(extras, s_Jni.NotificationManager.KEY_FIRE_TIME, fireTime);
+            s_Jni.Bundle.PutBoolean(extras, s_Jni.NotificationManager.KEY_SHOW_IN_FOREGROUND, notification.ShowInForeground);
+            if (!string.IsNullOrEmpty(notification.IntentData))
+                s_Jni.Bundle.PutString(extras, s_Jni.NotificationManager.KEY_INTENT_DATA, notification.IntentData);
         }
 
         internal static AndroidNotificationIntentData GetNotificationData(AndroidJavaObject notificationObj)

com.unity.mobile.notifications/Runtime/Android/Plugins/com/unity/androidnotifications/UnityNotificationBackgroundThread.java

@@ -1,43 +1,54 @@
 package com.unity.androidnotifications;
 
+import static com.unity.androidnotifications.UnityNotificationManager.KEY_ID;
 import static com.unity.androidnotifications.UnityNotificationManager.TAG_UNITY;
 
 import android.app.Notification;
 import android.content.Context;
 import android.util.Log;
 
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.LinkedTransferQueue;
+import java.util.Enumeration;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
 public class UnityNotificationBackgroundThread extends Thread {
     private static abstract class Task {
         // returns true if notificationIds was modified (needs to be saved)
-        public abstract boolean run(Context context, Set<String> notificationIds);
+        public abstract boolean run(Context context, ConcurrentHashMap<Integer, Notification.Builder> notifications);
     }
 
     private static class ScheduleNotificationTask extends Task {
         private int notificationId;
         private Notification.Builder notificationBuilder;
+        private boolean isNew;
 
-        public ScheduleNotificationTask(int id, Notification.Builder builder) {
+        public ScheduleNotificationTask(int id, Notification.Builder builder, boolean addedNew) {
             notificationId = id;
             notificationBuilder = builder;
+            isNew = addedNew;
         }
 
         @Override
-        public boolean run(Context context, Set<String> notificationIds) {
+        public boolean run(Context context, ConcurrentHashMap<Integer, Notification.Builder> notifications) {
             String id = String.valueOf(notificationId);
+            Integer ID = Integer.valueOf(notificationId);
+            boolean didSchedule = false;
             try {
                 UnityNotificationManager.mUnityNotificationManager.performNotificationScheduling(notificationId, notificationBuilder);
-                return notificationIds.add(id);
+                didSchedule = true;
             } finally {
-                // if failed to schedule or replace, remove from settings and cache, so the status is correctly reported
-                if (!notificationIds.contains(id)) {
+                // if failed to schedule or replace, remove
+                if (!didSchedule) {
+                    notifications.remove(notificationId);
+                    UnityNotificationManager.cancelPendingNotificationIntent(context, notificationId);
                     UnityNotificationManager.deleteExpiredNotificationIntent(context, id);
-                    UnityNotificationManager.removeScheduledNotification(Integer.valueOf(notificationId));
                 }
             }
+
+            return isNew;
         }
     }
 
@@ -49,11 +60,10 @@ public CancelNotificationTask(int id) {
         }
 
         @Override
-        public boolean run(Context context, Set<String> notificationIds) {
+        public boolean run(Context context, ConcurrentHashMap<Integer, Notification.Builder> notifications) {
             UnityNotificationManager.cancelPendingNotificationIntent(context, notificationId);
-            String id = String.valueOf(notificationId);
-            if (notificationIds.remove(id)) {
-                UnityNotificationManager.deleteExpiredNotificationIntent(context, id);
+            if (notifications.remove(notificationId) != null) {
+                UnityNotificationManager.deleteExpiredNotificationIntent(context, String.valueOf(notificationId));
                 return true;
             }
 
@@ -63,16 +73,18 @@ public boolean run(Context context, Set<String> notificationIds) {
 
     private static class CancelAllNotificationsTask extends Task {
         @Override
-        public boolean run(Context context, Set<String> notificationIds) {
-            if (notificationIds.isEmpty())
+        public boolean run(Context context, ConcurrentHashMap<Integer, Notification.Builder> notifications) {
+            if (notifications.isEmpty())
                 return false;
 
-            for (String id : notificationIds) {
-                UnityNotificationManager.cancelPendingNotificationIntent(context, Integer.valueOf(id));
-                UnityNotificationManager.deleteExpiredNotificationIntent(context, id);
+            Enumeration<Integer> ids = notifications.keys();
+            while (ids.hasMoreElements()) {
+                Integer notificationId = ids.nextElement();
+                UnityNotificationManager.cancelPendingNotificationIntent(context, notificationId);
+                UnityNotificationManager.deleteExpiredNotificationIntent(context, String.valueOf(notificationId));
             }
 
-            notificationIds.clear();
+            notifications.clear();
             return true;
         }
     }
@@ -85,22 +97,33 @@ public HousekeepingTask(UnityNotificationBackgroundThread th) {
         }
 
         @Override
-        public boolean run(Context context, Set<String> notificationIds) {
+        public boolean run(Context context, ConcurrentHashMap<Integer, Notification.Builder> notifications) {
+            HashSet<String> notificationIds = new HashSet<>();
+            Enumeration<Integer> ids = notifications.keys();
+            while (ids.hasMoreElements()) {
+                notificationIds.add(String.valueOf(ids.nextElement()));
+            }
             thread.performHousekeeping(context, notificationIds);
             return false;
         }
     }
 
     private static final int TASKS_FOR_HOUSEKEEPING = 50;
     private LinkedTransferQueue<Task> mTasks = new LinkedTransferQueue();
+    private ConcurrentHashMap<Integer, Notification.Builder> mScheduledNotifications;
+    private static Context mContext;
     private int mTasksSinceHousekeeping = TASKS_FOR_HOUSEKEEPING;  // we want hoursekeeping at the start
 
-    public UnityNotificationBackgroundThread() {
-        enqueueHousekeeping();
+    public UnityNotificationBackgroundThread(Context context, ConcurrentHashMap<Integer, Notification.Builder> scheduledNotifications) {
+        mContext = context;
+        mScheduledNotifications = scheduledNotifications;
+        // rescheduling after reboot may have loaded, otherwise load here
+        if (mScheduledNotifications.size() == 0)
+            loadNotifications();
     }
 
-    public void enqueueNotification(int id, Notification.Builder notificationBuilder) {
-        mTasks.add(new UnityNotificationBackgroundThread.ScheduleNotificationTask(id, notificationBuilder));
+    public void enqueueNotification(int id, Notification.Builder notificationBuilder, boolean addedNew) {
+        mTasks.add(new UnityNotificationBackgroundThread.ScheduleNotificationTask(id, notificationBuilder, addedNew));
     }
 
     public void enqueueCancelNotification(int id) {
@@ -117,13 +140,11 @@ private void enqueueHousekeeping() {
 
     @Override
     public void run() {
-        Context context = UnityNotificationManager.mUnityNotificationManager.mContext;
-        HashSet<String> notificationIds = new HashSet(UnityNotificationManager.getScheduledNotificationIDs(context));
         boolean haveChanges = false;
         while (true) {
             try {
                 Task task = mTasks.take();
-                haveChanges |= executeTask(context, task, notificationIds);
+                haveChanges |= executeTask(mContext, task, mScheduledNotifications);
                 if (!(task instanceof HousekeepingTask))
                     ++mTasksSinceHousekeeping;
                 if (mTasks.size() == 0 && haveChanges) {
@@ -137,9 +158,9 @@ public void run() {
         }
     }
 
-    private boolean executeTask(Context context, Task task, Set<String> notificationIds) {
+    private boolean executeTask(Context context, Task task, ConcurrentHashMap<Integer, Notification.Builder> notifications) {
         try {
-            return task.run(context, notificationIds);
+            return task.run(context, notifications);
         } catch (Exception e) {
             Log.e(TAG_UNITY, "Exception executing notification task", e);
             return false;
@@ -154,4 +175,12 @@ private void performHousekeeping(Context context, Set<String> notificationIds) {
             UnityNotificationManager.performNotificationHousekeeping(context, notificationIds);
         UnityNotificationManager.saveScheduledNotificationIDs(context, notificationIds);
     }
+
+    private void loadNotifications() {
+        List<Notification.Builder> notifications = UnityNotificationManager.loadSavedNotifications(mContext);
+        for (Notification.Builder builder : notifications) {
+            int id = builder.getExtras().getInt(KEY_ID, -1);
+            mScheduledNotifications.put(id, builder);
+        }
+    }
 }