fixing notifications not dismissing when the in memory state becomes out of sync

- lazily loads the initial notification state as we rely on a current session to exist
- extracts all the notification state manipulation to a separate state class

vector/src/main/java/im/vector/app/VectorApplication.kt

@@ -175,8 +175,7 @@ class VectorApplication :
             }
 
             override fun onPause(owner: LifecycleOwner) {
-                Timber.i("App entered background") // call persistInfo
+                Timber.i("App entered background")
-                notificationDrawerManager.persistInfo()
                 FcmHelper.onEnterBackground(appContext, vectorPreferences, activeSessionHolder)
             }
         })

vector/src/main/java/im/vector/app/features/MainActivity.kt

@@ -116,7 +116,6 @@ class MainActivity : VectorBaseActivity<ActivityMainBinding>(), UnlockedActivity
     private fun clearNotifications() {
         // Dismiss all notifications
         notificationDrawerManager.clearAllEvents()
-        notificationDrawerManager.persistInfo()
 
         // Also clear the dynamic shortcuts
         shortcutsHandler.clearShortcuts()

vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt

@@ -29,8 +29,6 @@ import org.matrix.android.sdk.api.session.Session
 import org.matrix.android.sdk.api.session.content.ContentUrlResolver
 import org.matrix.android.sdk.api.util.toMatrixItem
 import timber.log.Timber
-import java.io.File
-import java.io.FileOutputStream
 import javax.inject.Inject
 import javax.inject.Singleton
 
@@ -50,42 +48,24 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
     private val handlerThread: HandlerThread = HandlerThread("NotificationDrawerManager", Thread.MIN_PRIORITY)
     private var backgroundHandler: Handler
 
-    init {
-        handlerThread.start()
-        backgroundHandler = Handler(handlerThread.looper)
-    }
-
-    /**
-     * The notifiable events to render
-     * this is our source of truth for notifications, any changes to this list will be rendered as notifications
-     * when events are removed the previously rendered notifications will be cancelled
-     * when adding or updating, the notifications will be notified
-     *
-     * Events are unique by their properties, we should be careful not to insert multiple events with the same event-id
-     */
-    private val queuedEvents = loadEventInfo()
-
-    /**
-     * The last known rendered notifiable events
-     * we keep track of them in order to know which events have been removed from the eventList
-     * allowing us to cancel any notifications previous displayed by now removed events
-     */
-    private var renderedEvents = emptyList<ProcessedEvent<NotifiableEvent>>()
-    private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size)
-    private var currentRoomId: String? = null
-
     // TODO Multi-session: this will have to be improved
     private val currentSession: Session?
         get() = activeSessionDataSource.currentValue?.orNull()
 
+    /**
+     * Lazily initializes the NotificationState as we rely on having a current session in order to fetch the persisted queue of events
+     */
+    private val notificationState by lazy { NotificationState.createInitialNotificationState(context, currentSession) }
+    private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size)
+    private var currentRoomId: String? = null
+    private val firstThrottler = FirstThrottler(200)
+
     private var useCompleteNotificationFormat = vectorPreferences.useCompleteNotificationFormat()
 
-    /**
+    init {
-     * An in memory FIFO cache of the seen events.
+        handlerThread.start()
-     * Acts as a notification debouncer to stop already dismissed push notifications from
+        backgroundHandler = Handler(handlerThread.looper)
-     * displaying again when the /sync response is delayed.
+    }
-     */
-    private val seenEventIds = CircularCache.create<String>(cacheSize = 25)
 
     /**
     Should be called as soon as a new event is ready to be displayed.
@@ -106,7 +86,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
             Timber.d("onNotifiableEventReceived(): is push: ${notifiableEvent.canBeReplaced}")
         }
 
-        add(notifiableEvent, seenEventIds)
+        add(notifiableEvent, notificationState.seenEventIds)
     }
 
     /**
@@ -142,14 +122,12 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
     }
 
     fun updateEvents(action: NotificationDrawerManager.(NotificationEventQueue) -> Unit) {
-        synchronized(queuedEvents) {
+        notificationState.updateQueuedEvents(this) { queuedEvents, _ ->
-            action(this, queuedEvents)
+            action(queuedEvents)
         }
         refreshNotificationDrawer()
     }
 
-    private var firstThrottler = FirstThrottler(200)
-
     private fun refreshNotificationDrawer() {
         // Implement last throttler
         val canHandle = firstThrottler.canHandle()
@@ -171,85 +149,43 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
     @WorkerThread
     private fun refreshNotificationDrawerBg() {
         Timber.v("refreshNotificationDrawerBg()")
-        val eventsToRender = synchronized(queuedEvents) {
+        val eventsToRender = notificationState.updateQueuedEvents(this) { queuedEvents, renderedEvents ->
             notifiableEventProcessor.process(queuedEvents.rawEvents(), currentRoomId, renderedEvents).also {
                 queuedEvents.clearAndAdd(it.onlyKeptEvents())
             }
         }
 
-        if (renderedEvents == eventsToRender) {
+        if (notificationState.hasAlreadyRendered(eventsToRender)) {
             Timber.d("Skipping notification update due to event list not changing")
         } else {
-            renderedEvents = eventsToRender
+            notificationState.clearAndAddRenderedEvents(eventsToRender)
             val session = currentSession ?: return
-            val user = session.getUser(session.myUserId)
+            renderEvents(session, eventsToRender)
-            // myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash
+            notificationState.persistState(context, session)
-            val myUserDisplayName = user?.toMatrixItem()?.getBestName() ?: session.myUserId
-            val myUserAvatarUrl = session.contentUrlResolver().resolveThumbnail(
-                    contentUrl = user?.avatarUrl,
-                    width = avatarSize,
-                    height = avatarSize,
-                    method = ContentUrlResolver.ThumbnailMethod.SCALE
-            )
-            notificationRenderer.render(session.myUserId, myUserDisplayName, myUserAvatarUrl, useCompleteNotificationFormat, eventsToRender)
         }
     }
 
+    private fun renderEvents(session: Session, eventsToRender: List<ProcessedEvent<NotifiableEvent>>) {
+        val user = session.getUser(session.myUserId)
+        // myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash
+        val myUserDisplayName = user?.toMatrixItem()?.getBestName() ?: session.myUserId
+        val myUserAvatarUrl = session.contentUrlResolver().resolveThumbnail(
+                contentUrl = user?.avatarUrl,
+                width = avatarSize,
+                height = avatarSize,
+                method = ContentUrlResolver.ThumbnailMethod.SCALE
+        )
+        notificationRenderer.render(session.myUserId, myUserDisplayName, myUserAvatarUrl, useCompleteNotificationFormat, eventsToRender)
+    }
+
     fun shouldIgnoreMessageEventInRoom(roomId: String?): Boolean {
         return currentRoomId != null && roomId == currentRoomId
     }
 
-    fun persistInfo() {
-        synchronized(queuedEvents) {
-            if (queuedEvents.isEmpty()) {
-                deleteCachedRoomNotifications()
-                return
-            }
-            try {
-                val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME)
-                if (!file.exists()) file.createNewFile()
-                FileOutputStream(file).use {
-                    currentSession?.securelyStoreObject(queuedEvents.rawEvents(), KEY_ALIAS_SECRET_STORAGE, it)
-                }
-            } catch (e: Throwable) {
-                Timber.e(e, "## Failed to save cached notification info")
-            }
-        }
-    }
-
-    private fun loadEventInfo(): NotificationEventQueue {
-        try {
-            val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME)
-            if (file.exists()) {
-                file.inputStream().use {
-                    val events: ArrayList<NotifiableEvent>? = currentSession?.loadSecureSecret(it, KEY_ALIAS_SECRET_STORAGE)
-                    if (events != null) {
-                        return NotificationEventQueue(events.toMutableList())
-                    }
-                }
-            }
-        } catch (e: Throwable) {
-            Timber.e(e, "## Failed to load cached notification info")
-        }
-        return NotificationEventQueue()
-    }
-
-    private fun deleteCachedRoomNotifications() {
-        val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME)
-        if (file.exists()) {
-            file.delete()
-        }
-    }
-
     companion object {
         const val SUMMARY_NOTIFICATION_ID = 0
         const val ROOM_MESSAGES_NOTIFICATION_ID = 1
         const val ROOM_EVENT_NOTIFICATION_ID = 2
         const val ROOM_INVITATION_NOTIFICATION_ID = 3
-
-        // TODO Mutliaccount
-        private const val ROOMS_NOTIFICATIONS_FILE_NAME = "im.vector.notifications.cache"
-
-        private const val KEY_ALIAS_SECRET_STORAGE = "notificationMgr"
     }
 }

vector/src/main/java/im/vector/app/features/notifications/NotificationState.kt

@@ -0,0 +1,121 @@
+/*
+ * Copyright (c) 2021 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.features.notifications
+
+import android.content.Context
+import org.matrix.android.sdk.api.session.Session
+import timber.log.Timber
+import java.io.File
+import java.io.FileOutputStream
+
+// TODO Mutliaccount
+private const val ROOMS_NOTIFICATIONS_FILE_NAME = "im.vector.notifications.cache"
+private const val KEY_ALIAS_SECRET_STORAGE = "notificationMgr"
+
+class NotificationState(
+        /**
+         * The notifiable events to render
+         * this is our source of truth for notifications, any changes to this list will be rendered as notifications
+         * when events are removed the previously rendered notifications will be cancelled
+         * when adding or updating, the notifications will be notified
+         *
+         * Events are unique by their properties, we should be careful not to insert multiple events with the same event-id
+         */
+        private val queuedEvents: NotificationEventQueue,
+
+        /**
+         * The last known rendered notifiable events
+         * we keep track of them in order to know which events have been removed from the eventList
+         * allowing us to cancel any notifications previous displayed by now removed events
+         */
+        private val renderedEvents: MutableList<ProcessedEvent<NotifiableEvent>>,
+
+        /**
+         * An in memory FIFO cache of the seen events.
+         * Acts as a notification debouncer to stop already dismissed push notifications from
+         * displaying again when the /sync response is delayed.
+         */
+        val seenEventIds: CircularCache<String>
+) {
+
+    fun <T> updateQueuedEvents(drawerManager: NotificationDrawerManager, action: NotificationDrawerManager.(NotificationEventQueue, List<ProcessedEvent<NotifiableEvent>>) -> T): T {
+        return synchronized(queuedEvents) {
+            action(drawerManager, queuedEvents, renderedEvents)
+        }
+    }
+
+    fun clearAndAddRenderedEvents(eventsToRender: List<ProcessedEvent<NotifiableEvent>>) {
+        renderedEvents.clear()
+        renderedEvents.addAll(eventsToRender)
+    }
+
+    fun hasAlreadyRendered(eventsToRender: List<ProcessedEvent<NotifiableEvent>>) = renderedEvents == eventsToRender
+
+    fun persistState(context: Context, currentSession: Session) {
+        synchronized(queuedEvents) {
+            if (queuedEvents.isEmpty()) {
+                deleteCachedRoomNotifications(context)
+                return
+            }
+            try {
+                val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME)
+                if (!file.exists()) file.createNewFile()
+                FileOutputStream(file).use {
+                    currentSession.securelyStoreObject(queuedEvents.rawEvents(), KEY_ALIAS_SECRET_STORAGE, it)
+                }
+            } catch (e: Throwable) {
+                Timber.e(e, "## Failed to save cached notification info")
+            }
+        }
+    }
+
+    private fun deleteCachedRoomNotifications(context: Context) {
+        val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME)
+        if (file.exists()) {
+            file.delete()
+        }
+    }
+
+    companion object {
+
+        fun createInitialNotificationState(context: Context, currentSession: Session?): NotificationState {
+            val queuedEvents = loadEventInfo(context, currentSession)
+            return NotificationState(
+                    queuedEvents = queuedEvents,
+                    renderedEvents = queuedEvents.rawEvents().map { ProcessedEvent(ProcessedEvent.Type.KEEP, it) }.toMutableList(),
+                    seenEventIds = CircularCache.create(cacheSize = 25)
+            )
+        }
+
+        private fun loadEventInfo(context: Context, currentSession: Session?): NotificationEventQueue {
+            try {
+                val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME)
+                if (file.exists()) {
+                    file.inputStream().use {
+                        val events: ArrayList<NotifiableEvent>? = currentSession?.loadSecureSecret(it, KEY_ALIAS_SECRET_STORAGE)
+                        if (events != null) {
+                            return NotificationEventQueue(events.toMutableList())
+                        }
+                    }
+                }
+            } catch (e: Throwable) {
+                Timber.e(e, "## Failed to load cached notification info")
+            }
+            return NotificationEventQueue()
+        }
+    }
+}