extracting the notification event logic to its own class and provide a single update point of entry for mutating the events

- this avoids multiple synchronisation locks by batching updates and ensures a single notification render pass

vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailFragment.kt

@@ -2102,12 +2102,12 @@ class RoomDetailFragment @Inject constructor(
 // VectorInviteView.Callback
 
     override fun onAcceptInvite() {
-        notificationDrawerManager.clearMemberShipNotificationForRoom(roomDetailArgs.roomId)
+        notificationDrawerManager.updateEvents { it.clearMemberShipNotificationForRoom(roomDetailArgs.roomId) }
         roomDetailViewModel.handle(RoomDetailAction.AcceptInvite)
     }
 
     override fun onRejectInvite() {
-        notificationDrawerManager.clearMemberShipNotificationForRoom(roomDetailArgs.roomId)
+        notificationDrawerManager.updateEvents { it.clearMemberShipNotificationForRoom(roomDetailArgs.roomId) }
         roomDetailViewModel.handle(RoomDetailAction.RejectInvite)
     }
 

vector/src/main/java/im/vector/app/features/home/room/list/RoomListFragment.kt

@@ -482,7 +482,7 @@ class RoomListFragment @Inject constructor(
     }
 
     override fun onAcceptRoomInvitation(room: RoomSummary) {
-        notificationDrawerManager.clearMemberShipNotificationForRoom(room.roomId)
+        notificationDrawerManager.updateEvents { it.clearMemberShipNotificationForRoom(room.roomId) }
         roomListViewModel.handle(RoomListAction.AcceptInvitation(room))
     }
 
@@ -495,7 +495,7 @@ class RoomListFragment @Inject constructor(
     }
 
     override fun onRejectRoomInvitation(room: RoomSummary) {
-        notificationDrawerManager.clearMemberShipNotificationForRoom(room.roomId)
+        notificationDrawerManager.updateEvents { it.clearMemberShipNotificationForRoom(room.roomId) }
         roomListViewModel.handle(RoomListAction.RejectInvitation(room))
     }
 }

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

@@ -49,26 +49,26 @@ class NotificationBroadcastReceiver : BroadcastReceiver() {
             NotificationUtils.SMART_REPLY_ACTION        ->
                 handleSmartReply(intent, context)
             NotificationUtils.DISMISS_ROOM_NOTIF_ACTION ->
-                intent.getStringExtra(KEY_ROOM_ID)?.let {
-                    notificationDrawerManager.clearMessageEventOfRoom(it)
+                intent.getStringExtra(KEY_ROOM_ID)?.let { roomId ->
+                    notificationDrawerManager.updateEvents { it.clearMessagesForRoom(roomId) }
                 }
             NotificationUtils.DISMISS_SUMMARY_ACTION    ->
                 notificationDrawerManager.clearAllEvents()
             NotificationUtils.MARK_ROOM_READ_ACTION     ->
-                intent.getStringExtra(KEY_ROOM_ID)?.let {
-                    notificationDrawerManager.clearMessageEventOfRoom(it)
-                    handleMarkAsRead(it)
+                intent.getStringExtra(KEY_ROOM_ID)?.let { roomId ->
+                    notificationDrawerManager.updateEvents { it.clearMessagesForRoom(roomId) }
+                    handleMarkAsRead(roomId)
                 }
             NotificationUtils.JOIN_ACTION               -> {
-                intent.getStringExtra(KEY_ROOM_ID)?.let {
-                    notificationDrawerManager.clearMemberShipNotificationForRoom(it)
-                    handleJoinRoom(it)
+                intent.getStringExtra(KEY_ROOM_ID)?.let { roomId ->
+                    notificationDrawerManager.updateEvents { it.clearMemberShipNotificationForRoom(roomId) }
+                    handleJoinRoom(roomId)
                 }
             }
             NotificationUtils.REJECT_ACTION             -> {
-                intent.getStringExtra(KEY_ROOM_ID)?.let {
-                    notificationDrawerManager.clearMemberShipNotificationForRoom(it)
-                    handleRejectRoom(it)
+                intent.getStringExtra(KEY_ROOM_ID)?.let { roomId ->
+                    notificationDrawerManager.updateEvents { it.clearMemberShipNotificationForRoom(roomId) }
+                    handleRejectRoom(roomId)
                 }
             }
         }

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

@@ -106,7 +106,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
             Timber.d("onNotifiableEventReceived(): is push: ${notifiableEvent.canBeReplaced}")
         }
         synchronized(queuedEvents) {
-            val existing = queuedEvents.firstOrNull { it.eventId == notifiableEvent.eventId }
+            val existing = queuedEvents.findExistingById(notifiableEvent)
             if (existing != null) {
                 if (existing.canBeReplaced) {
                     // Use the event coming from the event stream as it may contains more info than
@@ -117,8 +117,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
                     // Use setOnlyAlertOnce to ensure update notification does not interfere with sound
                     // from first notify invocation as outlined in:
                     // https://developer.android.com/training/notify-user/build-notification#Updating
-                    queuedEvents.remove(existing)
-                    queuedEvents.add(notifiableEvent)
+                    queuedEvents.replace(replace = existing, with = notifiableEvent)
                 } else {
                     // keep the existing one, do not replace
                 }
@@ -126,18 +125,10 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
                 // Check if this is an edit
                 if (notifiableEvent.editedEventId != null) {
                     // This is an edition
-                    val eventBeforeEdition = queuedEvents.firstOrNull {
-                        // Edition of an event
-                        it.eventId == notifiableEvent.editedEventId ||
-                                // or edition of an edition
-                                it.editedEventId == notifiableEvent.editedEventId
-                    }
-
+                    val eventBeforeEdition = queuedEvents.findEdited(notifiableEvent)
                     if (eventBeforeEdition != null) {
                         // Replace the existing notification with the new content
-                        queuedEvents.remove(eventBeforeEdition)
-
-                        queuedEvents.add(notifiableEvent)
+                        queuedEvents.replace(replace = eventBeforeEdition, with = notifiableEvent)
                     } else {
                         // Ignore an edit of a not displayed event in the notification drawer
                     }
@@ -155,37 +146,18 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         }
     }
 
-    fun onEventRedacted(eventId: String) {
+    fun updateEvents(action: (NotificationEventQueue) -> Unit) {
         synchronized(queuedEvents) {
-            queuedEvents.replace(eventId) {
-                when (it) {
-                    is InviteNotifiableEvent  -> it.copy(isRedacted = true)
-                    is NotifiableMessageEvent -> it.copy(isRedacted = true)
-                    is SimpleNotifiableEvent  -> it.copy(isRedacted = true)
-                }
-            }
+            action(queuedEvents)
         }
+        refreshNotificationDrawer()
     }
 
     /**
      * Clear all known events and refresh the notification drawer
      */
     fun clearAllEvents() {
-        synchronized(queuedEvents) {
-            queuedEvents.clear()
-        }
-        refreshNotificationDrawer()
-    }
-
-    /** Clear all known message events for this room */
-    fun clearMessageEventOfRoom(roomId: String?) {
-        Timber.v("clearMessageEventOfRoom $roomId")
-        if (roomId != null) {
-            val shouldUpdate = removeAll { it is NotifiableMessageEvent && it.roomId == roomId }
-            if (shouldUpdate) {
-                refreshNotificationDrawer()
-            }
-        }
+        updateEvents { it.clear() }
     }
 
     /**
@@ -193,26 +165,12 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
      * Used to ignore events related to that room (no need to display notification) and clean any existing notification on this room.
      */
     fun setCurrentRoom(roomId: String?) {
-        var hasChanged: Boolean
-        synchronized(queuedEvents) {
-            hasChanged = roomId != currentRoomId
+        updateEvents {
+            val hasChanged = roomId != currentRoomId
             currentRoomId = roomId
-        }
-        if (hasChanged) {
-            clearMessageEventOfRoom(roomId)
-        }
-    }
-
-    fun clearMemberShipNotificationForRoom(roomId: String) {
-        val shouldUpdate = removeAll { it is InviteNotifiableEvent && it.roomId == roomId }
-        if (shouldUpdate) {
-            refreshNotificationDrawerBg()
-        }
-    }
-
-    private fun removeAll(predicate: (NotifiableEvent) -> Boolean): Boolean {
-        return synchronized(queuedEvents) {
-            queuedEvents.removeAll(predicate)
+            if (hasChanged && roomId != null) {
+                it.clearMessagesForRoom(roomId)
+            }
         }
     }
 
@@ -248,9 +206,8 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         }
 
         val eventsToRender = synchronized(queuedEvents) {
-            notifiableEventProcessor.process(queuedEvents, currentRoomId, renderedEvents).also {
-                queuedEvents.clear()
-                queuedEvents.addAll(it.onlyKeptEvents())
+            notifiableEventProcessor.process(queuedEvents.rawEvents(), currentRoomId, renderedEvents).also {
+                queuedEvents.clearAndAdd(it.onlyKeptEvents())
             }
         }
 
@@ -286,7 +243,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
                 val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME)
                 if (!file.exists()) file.createNewFile()
                 FileOutputStream(file).use {
-                    currentSession?.securelyStoreObject(queuedEvents, KEY_ALIAS_SECRET_STORAGE, it)
+                    currentSession?.securelyStoreObject(queuedEvents.rawEvents(), KEY_ALIAS_SECRET_STORAGE, it)
                 }
             } catch (e: Throwable) {
                 Timber.e(e, "## Failed to save cached notification info")
@@ -294,21 +251,21 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         }
     }
 
-    private fun loadEventInfo(): MutableList<NotifiableEvent> {
+    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 events.toMutableList()
+                        return NotificationEventQueue(events.toMutableList())
                     }
                 }
             }
         } catch (e: Throwable) {
             Timber.e(e, "## Failed to load cached notification info")
         }
-        return ArrayList()
+        return NotificationEventQueue()
     }
 
     private fun deleteCachedRoomNotifications() {
@@ -330,11 +287,3 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         private const val KEY_ALIAS_SECRET_STORAGE = "notificationMgr"
     }
 }
-
-private fun MutableList<NotifiableEvent>.replace(eventId: String, block: (NotifiableEvent) -> NotifiableEvent) {
-    val indexToReplace = indexOfFirst { it.eventId == eventId }
-    if (indexToReplace == -1) {
-        return
-    }
-    set(indexToReplace, block(get(indexToReplace)))
-}

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

@@ -0,0 +1,99 @@
+/*
+ * 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 timber.log.Timber
+
+class NotificationEventQueue(
+        private val queue: MutableList<NotifiableEvent> = mutableListOf()
+) {
+
+    fun markRedacted(eventIds: List<String>) {
+        eventIds.forEach { redactedId ->
+            queue.replace(redactedId) {
+                when (it) {
+                    is InviteNotifiableEvent  -> it.copy(isRedacted = true)
+                    is NotifiableMessageEvent -> it.copy(isRedacted = true)
+                    is SimpleNotifiableEvent  -> it.copy(isRedacted = true)
+                }
+            }
+        }
+    }
+
+    fun syncRoomEvents(roomsLeft: Collection<String>, roomsJoined: Collection<String>) {
+        if (roomsLeft.isNotEmpty() || roomsJoined.isNotEmpty()) {
+            queue.removeAll {
+                when (it) {
+                    is NotifiableMessageEvent -> roomsLeft.contains(it.roomId)
+                    is InviteNotifiableEvent  -> roomsLeft.contains(it.roomId) || roomsJoined.contains(it.roomId)
+                    else                      -> false
+                }
+            }
+        }
+    }
+
+    private fun MutableList<NotifiableEvent>.replace(eventId: String, block: (NotifiableEvent) -> NotifiableEvent) {
+        val indexToReplace = indexOfFirst { it.eventId == eventId }
+        if (indexToReplace == -1) {
+            return
+        }
+        set(indexToReplace, block(get(indexToReplace)))
+    }
+
+    fun isEmpty() = queue.isEmpty()
+
+    fun clearAndAdd(events: List<NotifiableEvent>) {
+        queue.clear()
+        queue.addAll(events)
+    }
+
+    fun findExistingById(notifiableEvent: NotifiableEvent): NotifiableEvent? {
+        return queue.firstOrNull { it.eventId == notifiableEvent.eventId }
+    }
+
+    fun findEdited(notifiableEvent: NotifiableEvent): NotifiableEvent? {
+        return queue.firstOrNull {
+            it.eventId == notifiableEvent.editedEventId ||
+                    it.editedEventId == notifiableEvent.editedEventId // or edition of an edition
+        }
+    }
+
+    fun replace(replace: NotifiableEvent, with: NotifiableEvent) {
+        queue.remove(replace)
+        queue.add(with)
+    }
+
+    fun clear() {
+        queue.clear()
+    }
+
+    fun add(notifiableEvent: NotifiableEvent) {
+        queue.add(notifiableEvent)
+    }
+
+    fun clearMemberShipNotificationForRoom(roomId: String) {
+        Timber.v("clearMemberShipOfRoom $roomId")
+        queue.removeAll { it is InviteNotifiableEvent && it.roomId == roomId }
+    }
+
+    fun clearMessagesForRoom(roomId: String) {
+        Timber.v("clearMessageEventOfRoom $roomId")
+        queue.removeAll { it is NotifiableMessageEvent && it.roomId == roomId }
+    }
+
+    fun rawEvents(): List<NotifiableEvent> = queue
+}

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

@@ -44,16 +44,12 @@ class PushRuleTriggerListener @Inject constructor(
                     null
                 }
             }.forEach { notificationDrawerManager.onNotifiableEventReceived(it) }
-            pushEvents.roomsLeft.forEach { roomId ->
-                notificationDrawerManager.clearMessageEventOfRoom(roomId)
-                notificationDrawerManager.clearMemberShipNotificationForRoom(roomId)
-            }
-            pushEvents.roomsJoined.forEach { roomId ->
-                notificationDrawerManager.clearMemberShipNotificationForRoom(roomId)
-            }
-            pushEvents.redactedEventIds.forEach {
-                notificationDrawerManager.onEventRedacted(it)
+
+            notificationDrawerManager.updateEvents { queuedEvents ->
+                queuedEvents.syncRoomEvents(roomsLeft = pushEvents.roomsLeft, roomsJoined = pushEvents.roomsJoined)
+                queuedEvents.markRedacted(pushEvents.redactedEventIds)
             }
+
             notificationDrawerManager.refreshNotificationDrawer()
         } ?: Timber.e("Called without active session")
     }