Invitation notifications are not dismissed automatically if room is joined from another client (#347)

CHANGES.md

@@ -18,6 +18,7 @@ Bugfix:
  - Fix opening a permalink paginates all the history up to the last event (#282)
  - after login, the icon in the top left is a green 'A' for (all communities) rather than my avatar (#267)
  - Picture uploads are unreliable, pictures are shown in wrong aspect ratio on desktop client (#517)
+ - Invitation notifications are not dismissed automatically if room is joined from another client (#347)
 
 Translations:
  -

matrix-sdk-android/src/main/java/im/vector/matrix/android/api/pushrules/PushRuleService.kt

@@ -42,6 +42,7 @@ interface PushRuleService {
 
     interface PushRuleListener {
         fun onMatchRule(event: Event, actions: List<Action>)
+        fun onRoomJoined(roomId: String)
         fun onRoomLeft(roomId: String)
         fun onEventRedacted(redactedEventId: String)
         fun batchFinish()

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/notification/DefaultPushRuleService.kt

@@ -42,7 +42,7 @@ internal class DefaultPushRuleService @Inject constructor(private val getPushRul
                                                           private val monarchy: Monarchy
 ) : PushRuleService {
 
-    private var listeners = ArrayList<PushRuleService.PushRuleListener>()
+    private var listeners = mutableSetOf<PushRuleService.PushRuleListener>()
 
     override fun fetchPushRules(scope: String) {
         getPushRulesTask
@@ -90,22 +90,25 @@ internal class DefaultPushRuleService @Inject constructor(private val getPushRul
     }
 
     override fun updatePushRuleEnableStatus(kind: RuleKind, pushRule: PushRule, enabled: Boolean, callback: MatrixCallback<Unit>): Cancelable {
+        // The rules will be updated, and will come back from the next sync response
         return updatePushRuleEnableStatusTask
                 .configureWith(UpdatePushRuleEnableStatusTask.Params(kind, pushRule, enabled)) {
                     this.callback = callback
                 }
-                // TODO Fetch the rules
                 .executeBy(taskExecutor)
     }
 
     override fun removePushRuleListener(listener: PushRuleService.PushRuleListener) {
-        listeners.remove(listener)
+        synchronized(listeners) {
+            listeners.remove(listener)
+        }
     }
 
 
     override fun addPushRuleListener(listener: PushRuleService.PushRuleListener) {
-        if (!listeners.contains(listener))
+        synchronized(listeners) {
             listeners.add(listener)
+        }
     }
 
 //    fun processEvents(events: List<Event>) {
@@ -121,43 +124,63 @@ internal class DefaultPushRuleService @Inject constructor(private val getPushRul
 //    }
 
     fun dispatchBing(event: Event, rule: PushRule) {
-        try {
+        synchronized(listeners) {
             val actionsList = rule.getActions()
             listeners.forEach {
-                it.onMatchRule(event, actionsList)
+                try {
+                    it.onMatchRule(event, actionsList)
+                } catch (e: Throwable) {
+                    Timber.e(e, "Error while dispatching bing")
+                }
+            }
+        }
+    }
+
+    fun dispatchRoomJoined(roomId: String) {
+        synchronized(listeners) {
+            listeners.forEach {
+                try {
+                    it.onRoomJoined(roomId)
+                } catch (e: Throwable) {
+                    Timber.e(e, "Error while dispatching room joined")
+                }
             }
-        } catch (e: Throwable) {
-            Timber.e(e, "Error while dispatching bing")
         }
     }
 
-    fun dispatchRoomLeft(roomid: String) {
-        try {
+    fun dispatchRoomLeft(roomId: String) {
+        synchronized(listeners) {
             listeners.forEach {
-                it.onRoomLeft(roomid)
+                try {
+                    it.onRoomLeft(roomId)
+                } catch (e: Throwable) {
+                    Timber.e(e, "Error while dispatching room left")
+                }
             }
-        } catch (e: Throwable) {
-            Timber.e(e, "Error while dispatching room left")
         }
     }
 
     fun dispatchRedactedEventId(redactedEventId: String) {
-        try {
+        synchronized(listeners) {
             listeners.forEach {
-                it.onEventRedacted(redactedEventId)
+                try {
+                    it.onEventRedacted(redactedEventId)
+                } catch (e: Throwable) {
+                    Timber.e(e, "Error while dispatching redacted event")
+                }
             }
-        } catch (e: Throwable) {
-            Timber.e(e, "Error while dispatching room left")
         }
     }
 
     fun dispatchFinish() {
-        try {
+        synchronized(listeners) {
             listeners.forEach {
-                it.batchFinish()
+                try {
+                    it.batchFinish()
+                } catch (e: Throwable) {
+                    Timber.e(e, "Error while dispatching finish")
+                }
             }
-        } catch (e: Throwable) {
-            Timber.e(e, "Error while dispatching finish")
         }
     }
 }

matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/notification/ProcessEventForPushTask.kt

@@ -45,6 +45,10 @@ internal class DefaultProcessEventForPushTask @Inject constructor(
         params.syncResponse.leave.keys.forEach {
             defaultPushRuleService.dispatchRoomLeft(it)
         }
+        // Handle joined rooms
+        params.syncResponse.join.keys.forEach {
+            defaultPushRuleService.dispatchRoomJoined(it)
+        }
         val newJoinEvents = params.syncResponse.join
                 .map { entries ->
                     entries.value.timeline?.events?.map { it.copy(roomId = entries.key) }

vector/src/main/java/im/vector/riotx/features/home/room/list/RoomListViewModel.kt

@@ -173,8 +173,10 @@ class RoomListViewModel @AssistedInject constructor(@Assisted initialState: Room
 
         session.getRoom(roomId)?.leave(object : MatrixCallback<Unit> {
             override fun onSuccess(data: Unit) {
-                // We do not update the joiningRoomsIds here, because, the room is not joined yet regarding the sync data.
-                // Instead, we wait for the room to be joined
+                // We do not update the rejectingRoomsIds here, because, the room is not rejected yet regarding the sync data.
+                // Instead, we wait for the room to be rejected
+                // Known bug: if the user is invited again (after rejecting the first invitation), the loading will be displayed instead of the buttons.
+                // If we update the state, the button will be displayed again, so it's not ideal...
             }
 
             override fun onFailure(failure: Throwable) {

vector/src/main/java/im/vector/riotx/features/notifications/NotifiableEvent.kt

@@ -17,6 +17,9 @@ package im.vector.riotx.features.notifications
 
 import java.io.Serializable
 
+/**
+ * Parent interface for all events which can be displayed as a Notification
+ */
 interface NotifiableEvent : Serializable {
     var matrixID: String?
     val eventId: String

vector/src/main/java/im/vector/riotx/features/notifications/NotifiableEventResolver.kt

@@ -150,7 +150,6 @@ class NotifiableEventResolver @Inject constructor(private val stringProvider: St
             notifiableEvent.soundName = null
 
             // Get the avatars URL
-            // TODO They will be not displayed the first time (known limitation)
             notifiableEvent.roomAvatarPath = session.contentUrlResolver()
                     .resolveThumbnail(room.roomSummary()?.avatarUrl,
                             250,

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

@@ -138,7 +138,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
     }
 
     /**
-    Clear all known events and refresh the notification drawer
+     * Clear all known events and refresh the notification drawer
      */
     fun clearAllEvents() {
         synchronized(eventList) {
@@ -147,7 +147,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         refreshNotificationDrawer()
     }
 
-    /** Clear all known message events for this room and refresh the notification drawer */
+    /** Clear all known message events for this room */
     fun clearMessageEventOfRoom(roomId: String?) {
         Timber.v("clearMessageEventOfRoom $roomId")
 
@@ -159,7 +159,6 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
             }
             notificationUtils.cancelNotificationMessage(roomId, ROOM_MESSAGES_NOTIFICATION_ID)
         }
-        refreshNotificationDrawer()
     }
 
     /**
@@ -177,21 +176,14 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         }
     }
 
-    fun homeActivityDidResume(matrixID: String?) {
-        synchronized(eventList) {
-            eventList.removeAll { e ->
-                // messages are cleared when entering room
-                e !is NotifiableMessageEvent
-            }
-        }
-    }
-
     fun clearMemberShipNotificationForRoom(roomId: String) {
         synchronized(eventList) {
             eventList.removeAll { e ->
                 e is InviteNotifiableEvent && e.roomId == roomId
             }
         }
+
+        notificationUtils.cancelNotificationMessage(roomId, ROOM_INVITATION_NOTIFICATION_ID)
     }
 
 
@@ -226,23 +218,26 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
 
             //group events by room to create a single MessagingStyle notif
             val roomIdToEventMap: MutableMap<String, MutableList<NotifiableMessageEvent>> = LinkedHashMap()
-            val simpleEvents: MutableList<NotifiableEvent> = ArrayList()
+            val simpleEvents: MutableList<SimpleNotifiableEvent> = ArrayList()
+            val invitationEvents: MutableList<InviteNotifiableEvent> = ArrayList()
 
             val eventIterator = eventList.listIterator()
             while (eventIterator.hasNext()) {
-                val event = eventIterator.next()
-                if (event is NotifiableMessageEvent) {
-                    val roomId = event.roomId
-                    val roomEvents = roomIdToEventMap.getOrPut(roomId) { ArrayList() }
-
-                    if (shouldIgnoreMessageEventInRoom(roomId) || outdatedDetector?.isMessageOutdated(event) == true) {
-                        //forget this event
-                        eventIterator.remove()
-                    } else {
-                        roomEvents.add(event)
+                when (val event = eventIterator.next()) {
+                    is NotifiableMessageEvent -> {
+                        val roomId = event.roomId
+                        val roomEvents = roomIdToEventMap.getOrPut(roomId) { ArrayList() }
+
+                        if (shouldIgnoreMessageEventInRoom(roomId) || outdatedDetector?.isMessageOutdated(event) == true) {
+                            //forget this event
+                            eventIterator.remove()
+                        } else {
+                            roomEvents.add(event)
+                        }
                     }
-                } else {
-                    simpleEvents.add(event)
+                    is InviteNotifiableEvent  -> invitationEvents.add(event)
+                    is SimpleNotifiableEvent  -> simpleEvents.add(event)
+                    else                      -> Timber.w("Type not handled")
                 }
             }
 
@@ -251,7 +246,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
 
             var globalLastMessageTimestamp = 0L
 
-            //events have been grouped by roomId
+            // events have been grouped by roomId
             for ((roomId, events) in roomIdToEventMap) {
                 // Build the notification for the room
                 if (events.isEmpty() || events.all { it.isRedacted }) {
@@ -372,11 +367,24 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
             }
 
 
-            //Handle simple events
+            // Handle invitation events
+            for (event in invitationEvents) {
+                //We build a invitation notification
+                if (firstTime || !event.hasBeenDisplayed) {
+                    val notification = notificationUtils.buildRoomInvitationNotification(event, session.myUserId)
+                    notificationUtils.showNotificationMessage(event.roomId, ROOM_INVITATION_NOTIFICATION_ID, notification)
+                    event.hasBeenDisplayed = true //we can consider it as displayed
+                    hasNewEvent = true
+                    summaryIsNoisy = summaryIsNoisy || event.noisy
+                    summaryInboxStyle.addLine(event.description)
+                }
+            }
+
+            // Handle simple events
             for (event in simpleEvents) {
-                //We build a simple event
+                //We build a simple notification
                 if (firstTime || !event.hasBeenDisplayed) {
-                    val notification = notificationUtils.buildSimpleEventNotification(event, null, session.myUserId)
+                    val notification = notificationUtils.buildSimpleEventNotification(event, session.myUserId)
                     notificationUtils.showNotificationMessage(event.eventId, ROOM_EVENT_NOTIFICATION_ID, notification)
                     event.hasBeenDisplayed = true //we can consider it as displayed
                     hasNewEvent = true
@@ -500,6 +508,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
         private const val SUMMARY_NOTIFICATION_ID = 0
         private const val ROOM_MESSAGES_NOTIFICATION_ID = 1
         private const val ROOM_EVENT_NOTIFICATION_ID = 2
+        private const val ROOM_INVITATION_NOTIFICATION_ID = 3
 
         // TODO Mutliaccount
         private const val ROOMS_NOTIFICATIONS_FILE_NAME = "im.vector.notifications.cache"