Merge pull request #4506 from vector-im/feature/adm/non-dismissing-notifications

Non dismissing notifications

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
 
@@ -40,52 +38,45 @@ import javax.inject.Singleton
  * Events can be grouped into the same notification, old (already read) events can be removed to do some cleaning.
  */
 @Singleton
-class NotificationDrawerManager @Inject constructor(private val context: Context,
+class NotificationDrawerManager @Inject constructor(
+        private val context: Context,
         private val notificationDisplayer: NotificationDisplayer,
         private val vectorPreferences: VectorPreferences,
         private val activeSessionDataSource: ActiveSessionDataSource,
         private val notifiableEventProcessor: NotifiableEventProcessor,
-                                                    private val notificationRenderer: NotificationRenderer) {
+        private val notificationRenderer: NotificationRenderer,
+        private val notificationEventPersistence: NotificationEventPersistence
+) {
 
     private val handlerThread: HandlerThread = HandlerThread("NotificationDrawerManager", Thread.MIN_PRIORITY)
     private var backgroundHandler: Handler
 
+    // 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 { createInitialNotificationState() }
+    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 {
         handlerThread.start()
         backgroundHandler = Handler(handlerThread.looper)
     }
 
-    /**
+    private fun createInitialNotificationState(): NotificationState {
-     * The notifiable events to render
+        val queuedEvents = notificationEventPersistence.loadEvents(currentSession, factory = { rawEvents ->
-     * this is our source of truth for notifications, any changes to this list will be rendered as notifications
+            NotificationEventQueue(rawEvents.toMutableList(), seenEventIds = CircularCache.create(cacheSize = 25))
-     * when events are removed the previously rendered notifications will be cancelled
+        })
-     * when adding or updating, the notifications will be notified
+        val renderedEvents = queuedEvents.rawEvents().map { ProcessedEvent(ProcessedEvent.Type.KEEP, it) }.toMutableList()
-     *
+        return NotificationState(queuedEvents, renderedEvents)
-     * 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()
-
-    private var useCompleteNotificationFormat = vectorPreferences.useCompleteNotificationFormat()
-
-    /**
-     * 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.
-     */
-    private val seenEventIds = CircularCache.create<String>(cacheSize = 25)
 
     /**
     Should be called as soon as a new event is ready to be displayed.
@@ -106,7 +97,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
             Timber.d("onNotifiableEventReceived(): is push: ${notifiableEvent.canBeReplaced}")
         }
 
-        add(notifiableEvent, seenEventIds)
+        add(notifiableEvent)
     }
 
     /**
@@ -142,14 +133,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,17 +160,29 @@ 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
+            renderEvents(session, eventsToRender)
+            persistEvents(session)
+        }
+    }
+
+    private fun persistEvents(session: Session) {
+        notificationState.queuedEvents { queuedEvents ->
+            notificationEventPersistence.persistEvents(queuedEvents, session)
+        }
+    }
+
+    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
@@ -193,63 +194,15 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         )
         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/NotificationEventPersistence.kt

@@ -0,0 +1,71 @@
+/*
+ * 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
+import javax.inject.Inject
+
+// TODO Multi-account
+private const val ROOMS_NOTIFICATIONS_FILE_NAME = "im.vector.notifications.cache"
+private const val KEY_ALIAS_SECRET_STORAGE = "notificationMgr"
+
+class NotificationEventPersistence @Inject constructor(private val context: Context) {
+
+    fun loadEvents(currentSession: Session?, factory: (List<NotifiableEvent>) -> NotificationEventQueue): 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 factory(events)
+                    }
+                }
+            }
+        } catch (e: Throwable) {
+            Timber.e(e, "## Failed to load cached notification info")
+        }
+        return factory(emptyList())
+    }
+
+    fun persistEvents(queuedEvents: NotificationEventQueue, currentSession: Session) {
+        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()
+        }
+    }
+}

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

@@ -18,8 +18,15 @@ package im.vector.app.features.notifications
 
 import timber.log.Timber
 
-class NotificationEventQueue(
+data class NotificationEventQueue(
-        private val queue: MutableList<NotifiableEvent> = mutableListOf()
+        private val queue: MutableList<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.
+         */
+        private val seenEventIds: CircularCache<String>
 ) {
 
     fun markRedacted(eventIds: List<String>) {
@@ -57,7 +64,7 @@ class NotificationEventQueue(
         queue.clear()
     }
 
-    fun add(notifiableEvent: NotifiableEvent, seenEventIds: CircularCache<String>) {
+    fun add(notifiableEvent: NotifiableEvent) {
         val existing = findExistingById(notifiableEvent)
         val edited = findEdited(notifiableEvent)
         when {

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

@@ -0,0 +1,58 @@
+/*
+ * 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
+
+class NotificationState(
+        /**
+         * The notifiable events queued for rendering or currently rendered
+         *
+         * 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>>,
+) {
+
+    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 queuedEvents(block: (NotificationEventQueue) -> Unit) {
+        synchronized(queuedEvents) {
+            block(queuedEvents)
+        }
+    }
+}

vector/src/test/java/im/vector/app/features/notifications/NotificationEventQueueTest.kt

@@ -126,7 +126,7 @@ class NotificationEventQueueTest {
     fun `given no events when adding then adds event`() {
         val queue = givenQueue(listOf())
 
-        queue.add(aSimpleNotifiableEvent(), seenEventIds = seenIdsCache)
+        queue.add(aSimpleNotifiableEvent())
 
         queue.rawEvents() shouldBeEqualTo listOf(aSimpleNotifiableEvent())
     }
@@ -137,7 +137,7 @@ class NotificationEventQueueTest {
         val notifiableEvent = aSimpleNotifiableEvent()
         seenIdsCache.put(notifiableEvent.eventId)
 
-        queue.add(notifiableEvent, seenEventIds = seenIdsCache)
+        queue.add(notifiableEvent)
 
         queue.rawEvents() shouldBeEqualTo emptyList()
     }
@@ -148,7 +148,7 @@ class NotificationEventQueueTest {
         val updatedEvent = replaceableEvent.copy(title = "updated title")
         val queue = givenQueue(listOf(replaceableEvent))
 
-        queue.add(updatedEvent, seenEventIds = seenIdsCache)
+        queue.add(updatedEvent)
 
         queue.rawEvents() shouldBeEqualTo listOf(updatedEvent)
     }
@@ -159,7 +159,7 @@ class NotificationEventQueueTest {
         val updatedEvent = nonReplaceableEvent.copy(title = "updated title")
         val queue = givenQueue(listOf(nonReplaceableEvent))
 
-        queue.add(updatedEvent, seenEventIds = seenIdsCache)
+        queue.add(updatedEvent)
 
         queue.rawEvents() shouldBeEqualTo listOf(nonReplaceableEvent)
     }
@@ -170,7 +170,7 @@ class NotificationEventQueueTest {
         val updatedEvent = editedEvent.copy(eventId = "1", editedEventId = "id-to-edit", title = "updated title")
         val queue = givenQueue(listOf(editedEvent))
 
-        queue.add(updatedEvent, seenEventIds = seenIdsCache)
+        queue.add(updatedEvent)
 
         queue.rawEvents() shouldBeEqualTo listOf(updatedEvent)
     }
@@ -181,7 +181,7 @@ class NotificationEventQueueTest {
         val updatedEvent = editedEvent.copy(eventId = "1", editedEventId = "id-to-edit", title = "updated title")
         val queue = givenQueue(listOf(editedEvent))
 
-        queue.add(updatedEvent, seenEventIds = seenIdsCache)
+        queue.add(updatedEvent)
 
         queue.rawEvents() shouldBeEqualTo listOf(updatedEvent)
     }
@@ -212,5 +212,5 @@ class NotificationEventQueueTest {
         queue.rawEvents() shouldBeEqualTo listOf(anInviteNotifiableEvent(roomId = roomId))
     }
 
-    private fun givenQueue(events: List<NotifiableEvent>) = NotificationEventQueue(events.toMutableList())
+    private fun givenQueue(events: List<NotifiableEvent>) = NotificationEventQueue(events.toMutableList(), seenEventIds = seenIdsCache)
 }