From 5190ef42807aaa7f848f8f12991c0b5be9479b6b Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 11:12:48 +0000 Subject: [PATCH 01/17] replacing separated push listener callbacks with a single onEvents callback - simplifies the handling of notifications, will allow us to reduce redundant synchronisations and suspend the entire notification update (will be needed for supporting images) --- .../android/sdk/api/pushrules/PushEvents.kt | 27 ++++++ .../sdk/api/pushrules/PushRuleService.kt | 6 +- .../notification/DefaultPushRuleService.kt | 83 ++----------------- .../notification/ProcessEventForPushTask.kt | 26 +++--- .../notifications/PushRuleTriggerListener.kt | 62 ++++++-------- 5 files changed, 73 insertions(+), 131 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushEvents.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushEvents.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushEvents.kt new file mode 100644 index 0000000000..38a793b60c --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushEvents.kt @@ -0,0 +1,27 @@ +/* + * 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 org.matrix.android.sdk.api.pushrules + +import org.matrix.android.sdk.api.pushrules.rest.PushRule +import org.matrix.android.sdk.api.session.events.model.Event + +data class PushEvents( + val matchedEvents: List>, + val roomsJoined: Collection, + val roomsLeft: Collection, + val redactedEventIds: List +) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushRuleService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushRuleService.kt index 1d0acf38fa..88268f0f86 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushRuleService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushRuleService.kt @@ -51,11 +51,7 @@ interface PushRuleService { // fun fulfilledBingRule(event: Event, rules: List): PushRule? interface PushRuleListener { - fun onMatchRule(event: Event, actions: List) - fun onRoomJoined(roomId: String) - fun onRoomLeft(roomId: String) - fun onEventRedacted(redactedEventId: String) - fun batchFinish() + fun onEvents(pushEvents: PushEvents) } fun getKeywords(): LiveData> diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/DefaultPushRuleService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/DefaultPushRuleService.kt index 65974151c8..aaaeb4874e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/DefaultPushRuleService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/DefaultPushRuleService.kt @@ -19,6 +19,7 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.Transformations import com.zhuinden.monarchy.Monarchy import org.matrix.android.sdk.api.pushrules.Action +import org.matrix.android.sdk.api.pushrules.PushEvents import org.matrix.android.sdk.api.pushrules.PushRuleService import org.matrix.android.sdk.api.pushrules.RuleKind import org.matrix.android.sdk.api.pushrules.RuleScope @@ -40,7 +41,6 @@ import org.matrix.android.sdk.internal.session.pushers.UpdatePushRuleActionsTask import org.matrix.android.sdk.internal.session.pushers.UpdatePushRuleEnableStatusTask import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.task.configureWith -import timber.log.Timber import javax.inject.Inject @SessionScope @@ -142,79 +142,6 @@ internal class DefaultPushRuleService @Inject constructor( return pushRuleFinder.fulfilledBingRule(event, rules)?.getActions().orEmpty() } -// fun processEvents(events: List) { -// var hasDoneSomething = false -// events.forEach { event -> -// fulfilledBingRule(event)?.let { -// hasDoneSomething = true -// dispatchBing(event, it) -// } -// } -// if (hasDoneSomething) -// dispatchFinish() -// } - - fun dispatchBing(event: Event, rule: PushRule) { - synchronized(listeners) { - val actionsList = rule.getActions() - listeners.forEach { - 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") - } - } - } - } - - fun dispatchRoomLeft(roomId: String) { - synchronized(listeners) { - listeners.forEach { - try { - it.onRoomLeft(roomId) - } catch (e: Throwable) { - Timber.e(e, "Error while dispatching room left") - } - } - } - } - - fun dispatchRedactedEventId(redactedEventId: String) { - synchronized(listeners) { - listeners.forEach { - try { - it.onEventRedacted(redactedEventId) - } catch (e: Throwable) { - Timber.e(e, "Error while dispatching redacted event") - } - } - } - } - - fun dispatchFinish() { - synchronized(listeners) { - listeners.forEach { - try { - it.batchFinish() - } catch (e: Throwable) { - Timber.e(e, "Error while dispatching finish") - } - } - } - } - override fun getKeywords(): LiveData> { // Keywords are all content rules that don't start with '.' val liveData = monarchy.findAllMappedWithChanges( @@ -229,4 +156,12 @@ internal class DefaultPushRuleService @Inject constructor( results.firstOrNull().orEmpty().toSet() } } + + fun dispatchEvents(pushEvents: PushEvents) { + synchronized(listeners) { + listeners.forEach { + it.onEvents(pushEvents) + } + } + } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/ProcessEventForPushTask.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/ProcessEventForPushTask.kt index 3c74888eda..0ac21b555e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/ProcessEventForPushTask.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/ProcessEventForPushTask.kt @@ -16,6 +16,7 @@ package org.matrix.android.sdk.internal.session.notification +import org.matrix.android.sdk.api.pushrules.PushEvents import org.matrix.android.sdk.api.pushrules.rest.PushRule import org.matrix.android.sdk.api.session.events.model.EventType import org.matrix.android.sdk.api.session.events.model.isInvitation @@ -39,14 +40,6 @@ internal class DefaultProcessEventForPushTask @Inject constructor( ) : ProcessEventForPushTask { override suspend fun execute(params: ProcessEventForPushTask.Params) { - // Handle left rooms - params.syncResponse.leave.keys.forEach { - defaultPushRuleService.dispatchRoomLeft(it) - } - // Handle joined rooms - params.syncResponse.join.keys.forEach { - defaultPushRuleService.dispatchRoomJoined(it) - } val newJoinEvents = params.syncResponse.join .mapNotNull { (key, value) -> value.timeline?.events?.mapNotNull { @@ -74,10 +67,10 @@ internal class DefaultProcessEventForPushTask @Inject constructor( } Timber.v("[PushRules] Found ${allEvents.size} out of ${(newJoinEvents + inviteEvents).size}" + " to check for push rules with ${params.rules.size} rules") - allEvents.forEach { event -> + val matchedEvents = allEvents.mapNotNull { event -> pushRuleFinder.fulfilledBingRule(event, params.rules)?.let { Timber.v("[PushRules] Rule $it match for event ${event.eventId}") - defaultPushRuleService.dispatchBing(event, it) + event to it } } @@ -91,10 +84,13 @@ internal class DefaultProcessEventForPushTask @Inject constructor( Timber.v("[PushRules] Found ${allRedactedEvents.size} redacted events") - allRedactedEvents.forEach { redactedEventId -> - defaultPushRuleService.dispatchRedactedEventId(redactedEventId) - } - - defaultPushRuleService.dispatchFinish() + defaultPushRuleService.dispatchEvents( + PushEvents( + matchedEvents = matchedEvents, + roomsJoined = params.syncResponse.join.keys, + roomsLeft = params.syncResponse.leave.keys, + redactedEventIds = allRedactedEvents + ) + ) } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt b/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt index abbbd47f95..2620e05dde 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt @@ -16,10 +16,10 @@ package im.vector.app.features.notifications -import org.matrix.android.sdk.api.pushrules.Action +import org.matrix.android.sdk.api.pushrules.PushEvents import org.matrix.android.sdk.api.pushrules.PushRuleService +import org.matrix.android.sdk.api.pushrules.getActions import org.matrix.android.sdk.api.session.Session -import org.matrix.android.sdk.api.session.events.model.Event import timber.log.Timber import javax.inject.Inject import javax.inject.Singleton @@ -32,42 +32,30 @@ class PushRuleTriggerListener @Inject constructor( private var session: Session? = null - override fun onMatchRule(event: Event, actions: List) { - Timber.v("Push rule match for event ${event.eventId}") - val safeSession = session ?: return Unit.also { - Timber.e("Called without active session") - } - - val notificationAction = actions.toNotificationAction() - if (notificationAction.shouldNotify) { - val notifiableEvent = resolver.resolveEvent(event, safeSession, isNoisy = !notificationAction.soundName.isNullOrBlank()) - if (notifiableEvent == null) { - Timber.v("## Failed to resolve event") - // TODO - } else { - Timber.v("New event to notify") - notificationDrawerManager.onNotifiableEventReceived(notifiableEvent) + override fun onEvents(pushEvents: PushEvents) { + session?.let { session -> + pushEvents.matchedEvents.mapNotNull { (event, pushRule) -> + Timber.v("Push rule match for event ${event.eventId}") + val action = pushRule.getActions().toNotificationAction() + if (action.shouldNotify) { + resolver.resolveEvent(event, session, isNoisy = !action.soundName.isNullOrBlank()) + } else { + Timber.v("Matched push rule is set to not notify") + null + } + }.forEach { notificationDrawerManager.onNotifiableEventReceived(it) } + pushEvents.roomsLeft.forEach { roomId -> + notificationDrawerManager.clearMessageEventOfRoom(roomId) + notificationDrawerManager.clearMemberShipNotificationForRoom(roomId) } - } else { - Timber.v("Matched push rule is set to not notify") - } - } - - override fun onRoomLeft(roomId: String) { - notificationDrawerManager.clearMessageEventOfRoom(roomId) - notificationDrawerManager.clearMemberShipNotificationForRoom(roomId) - } - - override fun onRoomJoined(roomId: String) { - notificationDrawerManager.clearMemberShipNotificationForRoom(roomId) - } - - override fun onEventRedacted(redactedEventId: String) { - notificationDrawerManager.onEventRedacted(redactedEventId) - } - - override fun batchFinish() { - notificationDrawerManager.refreshNotificationDrawer() + pushEvents.roomsJoined.forEach { roomId -> + notificationDrawerManager.clearMemberShipNotificationForRoom(roomId) + } + pushEvents.redactedEventIds.forEach { + notificationDrawerManager.onEventRedacted(it) + } + notificationDrawerManager.refreshNotificationDrawer() + } ?: Timber.e("Called without active session") } fun startWithSession(session: Session) { From ef348c24a0cecd663000061ca8b85a3d4694e7e9 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 12:06:46 +0000 Subject: [PATCH 02/17] 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 --- .../home/room/detail/RoomDetailFragment.kt | 4 +- .../home/room/list/RoomListFragment.kt | 4 +- .../NotificationBroadcastReceiver.kt | 22 ++--- .../NotificationDrawerManager.kt | 89 ++++------------- .../notifications/NotificationEventQueue.kt | 99 +++++++++++++++++++ .../notifications/PushRuleTriggerListener.kt | 14 +-- 6 files changed, 138 insertions(+), 94 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailFragment.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailFragment.kt index d3af9c4196..dbd9c4466b 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailFragment.kt +++ b/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) } diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/RoomListFragment.kt b/vector/src/main/java/im/vector/app/features/home/room/list/RoomListFragment.kt index 0e049e22b1..6543cc8795 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/RoomListFragment.kt +++ b/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)) } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt index 33e43cd7e4..fe1e367fe2 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt +++ b/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) } } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt index c052de650e..649d733914 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt +++ b/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 { + private fun loadEventInfo(): NotificationEventQueue { try { val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME) if (file.exists()) { file.inputStream().use { val events: ArrayList? = 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.replace(eventId: String, block: (NotifiableEvent) -> NotifiableEvent) { - val indexToReplace = indexOfFirst { it.eventId == eventId } - if (indexToReplace == -1) { - return - } - set(indexToReplace, block(get(indexToReplace))) -} diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt new file mode 100644 index 0000000000..29ec91c976 --- /dev/null +++ b/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 = mutableListOf() +) { + + fun markRedacted(eventIds: List) { + 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, roomsJoined: Collection) { + 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.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) { + 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 = queue +} diff --git a/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt b/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt index 2620e05dde..59d8840c16 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt +++ b/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") } From 9009606e86fd1584513324f203c8051af19d68ea Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 12:24:16 +0000 Subject: [PATCH 03/17] flattening some of the onNotifiableEventReceived branches to simplify the chain --- .../NotificationDrawerManager.kt | 60 +++++++++---------- .../notifications/NotificationEventQueue.kt | 7 ++- 2 files changed, 31 insertions(+), 36 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt index 649d733914..0e43e41819 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt @@ -107,40 +107,34 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } synchronized(queuedEvents) { 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 - // the fcm one (like type/content/clear text) (e.g when an encrypted message from - // FCM should be update with clear text after a sync) - // In this case the message has already been notified, and might have done some noise - // So we want the notification to be updated even if it has already been displayed - // 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.replace(replace = existing, with = notifiableEvent) - } else { - // keep the existing one, do not replace + val edited = queuedEvents.findEdited(notifiableEvent) + when { + existing != null -> { + if (existing.canBeReplaced) { + // Use the event coming from the event stream as it may contains more info than + // the fcm one (like type/content/clear text) (e.g when an encrypted message from + // FCM should be update with clear text after a sync) + // In this case the message has already been notified, and might have done some noise + // So we want the notification to be updated even if it has already been displayed + // 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.replace(replace = existing, with = notifiableEvent) + } else { + // keep the existing one, do not replace + } } - } else { - // Check if this is an edit - if (notifiableEvent.editedEventId != null) { - // This is an edition - val eventBeforeEdition = queuedEvents.findEdited(notifiableEvent) - if (eventBeforeEdition != null) { - // Replace the existing notification with the new content - queuedEvents.replace(replace = eventBeforeEdition, with = notifiableEvent) - } else { - // Ignore an edit of a not displayed event in the notification drawer - } - } else { - // Not an edit - if (seenEventIds.contains(notifiableEvent.eventId)) { - // we've already seen the event, lets skip - Timber.d("onNotifiableEventReceived(): skipping event, already seen") - } else { - seenEventIds.put(notifiableEvent.eventId) - queuedEvents.add(notifiableEvent) - } + edited != null -> { + // Replace the existing notification with the new content + queuedEvents.replace(replace = edited, with = notifiableEvent) + } + seenEventIds.contains(notifiableEvent.eventId) -> { + // we've already seen the event, lets skip + Timber.d("onNotifiableEventReceived(): skipping event, already seen") + } + else -> { + seenEventIds.put(notifiableEvent.eventId) + queuedEvents.add(notifiableEvent) } } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt index 29ec91c976..7488b98664 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt @@ -66,9 +66,10 @@ class NotificationEventQueue( } fun findEdited(notifiableEvent: NotifiableEvent): NotifiableEvent? { - return queue.firstOrNull { - it.eventId == notifiableEvent.editedEventId || - it.editedEventId == notifiableEvent.editedEventId // or edition of an edition + return notifiableEvent.editedEventId?.let { editedId -> + queue.firstOrNull { + it.eventId == editedId || it.editedEventId == editedId + } } } From 588958c807124d05dde971646ee655fe117aa8f3 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 12:54:58 +0000 Subject: [PATCH 04/17] moving the notifable queue adding to the queue itself and making onNotifiableEventReceived not synchronised for use within the synchronized batching - makes the refresh function private as all interactions now come through via update --- .../fcm/VectorFirebaseMessagingService.kt | 3 +- .../NotificationBroadcastReceiver.kt | 3 +- .../NotificationDrawerManager.kt | 70 ++++++------------- .../notifications/NotificationEventQueue.kt | 35 +++++++++- .../notifications/PushRuleTriggerListener.kt | 9 +-- .../settings/VectorSettingsPinFragment.kt | 2 +- 6 files changed, 62 insertions(+), 60 deletions(-) diff --git a/vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt b/vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt index 63d50d4f97..e323506e9f 100755 --- a/vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt +++ b/vector/src/gplay/java/im/vector/app/gplay/push/fcm/VectorFirebaseMessagingService.kt @@ -201,8 +201,7 @@ class VectorFirebaseMessagingService : FirebaseMessagingService() { resolvedEvent ?.also { Timber.tag(loggerTag.value).d("Fast lane: notify drawer") } ?.let { - notificationDrawerManager.onNotifiableEventReceived(it) - notificationDrawerManager.refreshNotificationDrawer() + notificationDrawerManager.updateEvents { it.onNotifiableEventReceived(resolvedEvent) } } } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt index fe1e367fe2..06ef3f4aeb 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt @@ -145,8 +145,7 @@ class NotificationBroadcastReceiver : BroadcastReceiver() { canBeReplaced = false ) - notificationDrawerManager.onNotifiableEventReceived(notifiableMessageEvent) - notificationDrawerManager.refreshNotificationDrawer() + notificationDrawerManager.updateEvents { it.onNotifiableEventReceived(notifiableMessageEvent) } /* // TODO Error cannot be managed the same way than in Riot diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt index 0e43e41819..e86a65c3fe 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt @@ -93,7 +93,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context #refreshNotificationDrawer() is called. Events might be grouped and there might not be one notification per event! */ - fun onNotifiableEventReceived(notifiableEvent: NotifiableEvent) { + fun NotificationEventQueue.onNotifiableEventReceived(notifiableEvent: NotifiableEvent) { if (!vectorPreferences.areNotificationEnabledForDevice()) { Timber.i("Notification are disabled for this device") return @@ -105,46 +105,8 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } else { Timber.d("onNotifiableEventReceived(): is push: ${notifiableEvent.canBeReplaced}") } - synchronized(queuedEvents) { - val existing = queuedEvents.findExistingById(notifiableEvent) - val edited = queuedEvents.findEdited(notifiableEvent) - when { - existing != null -> { - if (existing.canBeReplaced) { - // Use the event coming from the event stream as it may contains more info than - // the fcm one (like type/content/clear text) (e.g when an encrypted message from - // FCM should be update with clear text after a sync) - // In this case the message has already been notified, and might have done some noise - // So we want the notification to be updated even if it has already been displayed - // 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.replace(replace = existing, with = notifiableEvent) - } else { - // keep the existing one, do not replace - } - } - edited != null -> { - // Replace the existing notification with the new content - queuedEvents.replace(replace = edited, with = notifiableEvent) - } - seenEventIds.contains(notifiableEvent.eventId) -> { - // we've already seen the event, lets skip - Timber.d("onNotifiableEventReceived(): skipping event, already seen") - } - else -> { - seenEventIds.put(notifiableEvent.eventId) - queuedEvents.add(notifiableEvent) - } - } - } - } - fun updateEvents(action: (NotificationEventQueue) -> Unit) { - synchronized(queuedEvents) { - action(queuedEvents) - } - refreshNotificationDrawer() + add(notifiableEvent, seenEventIds) } /** @@ -168,9 +130,27 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } } + fun notificationStyleChanged() { + updateEvents { + val newSettings = vectorPreferences.useCompleteNotificationFormat() + if (newSettings != useCompleteNotificationFormat) { + // Settings has changed, remove all current notifications + notificationDisplayer.cancelAllNotifications() + useCompleteNotificationFormat = newSettings + } + } + } + + fun updateEvents(action: NotificationDrawerManager.(NotificationEventQueue) -> Unit) { + synchronized(queuedEvents) { + action(this, queuedEvents) + } + refreshNotificationDrawer() + } + private var firstThrottler = FirstThrottler(200) - fun refreshNotificationDrawer() { + private fun refreshNotificationDrawer() { // Implement last throttler val canHandle = firstThrottler.canHandle() Timber.v("refreshNotificationDrawer(), delay: ${canHandle.waitMillis()} ms") @@ -191,14 +171,6 @@ class NotificationDrawerManager @Inject constructor(private val context: Context @WorkerThread private fun refreshNotificationDrawerBg() { Timber.v("refreshNotificationDrawerBg()") - - val newSettings = vectorPreferences.useCompleteNotificationFormat() - if (newSettings != useCompleteNotificationFormat) { - // Settings has changed, remove all current notifications - notificationDisplayer.cancelAllNotifications() - useCompleteNotificationFormat = newSettings - } - val eventsToRender = synchronized(queuedEvents) { notifiableEventProcessor.process(queuedEvents.rawEvents(), currentRoomId, renderedEvents).also { queuedEvents.clearAndAdd(it.onlyKeptEvents()) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt index 7488b98664..966c9fabad 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt @@ -82,10 +82,41 @@ class NotificationEventQueue( queue.clear() } - fun add(notifiableEvent: NotifiableEvent) { - queue.add(notifiableEvent) + fun add(notifiableEvent: NotifiableEvent, seenEventIds: CircularCache) { + val existing = findExistingById(notifiableEvent) + val edited = findEdited(notifiableEvent) + when { + existing != null -> { + if (existing.canBeReplaced) { + // Use the event coming from the event stream as it may contains more info than + // the fcm one (like type/content/clear text) (e.g when an encrypted message from + // FCM should be update with clear text after a sync) + // In this case the message has already been notified, and might have done some noise + // So we want the notification to be updated even if it has already been displayed + // 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 + replace(replace = existing, with = notifiableEvent) + } else { + // keep the existing one, do not replace + } + } + edited != null -> { + // Replace the existing notification with the new content + replace(replace = edited, with = notifiableEvent) + } + seenEventIds.contains(notifiableEvent.eventId) -> { + // we've already seen the event, lets skip + Timber.d("onNotifiableEventReceived(): skipping event, already seen") + } + else -> { + seenEventIds.put(notifiableEvent.eventId) + queue.add(notifiableEvent) + } + } } + fun clearMemberShipNotificationForRoom(roomId: String) { Timber.v("clearMemberShipOfRoom $roomId") queue.removeAll { it is InviteNotifiableEvent && it.roomId == roomId } diff --git a/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt b/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt index 59d8840c16..c5d4454bb7 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt @@ -34,7 +34,7 @@ class PushRuleTriggerListener @Inject constructor( override fun onEvents(pushEvents: PushEvents) { session?.let { session -> - pushEvents.matchedEvents.mapNotNull { (event, pushRule) -> + val notifiableEvents = pushEvents.matchedEvents.mapNotNull { (event, pushRule) -> Timber.v("Push rule match for event ${event.eventId}") val action = pushRule.getActions().toNotificationAction() if (action.shouldNotify) { @@ -43,14 +43,15 @@ class PushRuleTriggerListener @Inject constructor( Timber.v("Matched push rule is set to not notify") null } - }.forEach { notificationDrawerManager.onNotifiableEventReceived(it) } + } notificationDrawerManager.updateEvents { queuedEvents -> + notifiableEvents.forEach { notifiableEvent -> + queuedEvents.onNotifiableEventReceived(notifiableEvent) + } queuedEvents.syncRoomEvents(roomsLeft = pushEvents.roomsLeft, roomsJoined = pushEvents.roomsJoined) queuedEvents.markRedacted(pushEvents.redactedEventIds) } - - notificationDrawerManager.refreshNotificationDrawer() } ?: Timber.e("Called without active session") } diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorSettingsPinFragment.kt b/vector/src/main/java/im/vector/app/features/settings/VectorSettingsPinFragment.kt index 1a04dab950..254c82e285 100644 --- a/vector/src/main/java/im/vector/app/features/settings/VectorSettingsPinFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorSettingsPinFragment.kt @@ -55,7 +55,7 @@ class VectorSettingsPinFragment @Inject constructor( useCompleteNotificationPref.setOnPreferenceChangeListener { _, _ -> // Refresh the drawer for an immediate effect of this change - notificationDrawerManager.refreshNotificationDrawer() + notificationDrawerManager.notificationStyleChanged() true } } From 77e0b22982a9de8a409abe9385bbfc2081b3add9 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 12:59:37 +0000 Subject: [PATCH 05/17] extracting notifiable event creation to its own function --- .../notifications/PushRuleTriggerListener.kt | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt b/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt index c5d4454bb7..f0633b24de 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt @@ -34,17 +34,7 @@ class PushRuleTriggerListener @Inject constructor( override fun onEvents(pushEvents: PushEvents) { session?.let { session -> - val notifiableEvents = pushEvents.matchedEvents.mapNotNull { (event, pushRule) -> - Timber.v("Push rule match for event ${event.eventId}") - val action = pushRule.getActions().toNotificationAction() - if (action.shouldNotify) { - resolver.resolveEvent(event, session, isNoisy = !action.soundName.isNullOrBlank()) - } else { - Timber.v("Matched push rule is set to not notify") - null - } - } - + val notifiableEvents = createNotifiableEvents(pushEvents, session) notificationDrawerManager.updateEvents { queuedEvents -> notifiableEvents.forEach { notifiableEvent -> queuedEvents.onNotifiableEventReceived(notifiableEvent) @@ -55,6 +45,19 @@ class PushRuleTriggerListener @Inject constructor( } ?: Timber.e("Called without active session") } + private fun createNotifiableEvents(pushEvents: PushEvents, session: Session): List { + return pushEvents.matchedEvents.mapNotNull { (event, pushRule) -> + Timber.v("Push rule match for event ${event.eventId}") + val action = pushRule.getActions().toNotificationAction() + if (action.shouldNotify) { + resolver.resolveEvent(event, session, isNoisy = !action.soundName.isNullOrBlank()) + } else { + Timber.v("Matched push rule is set to not notify") + null + } + } + } + fun startWithSession(session: Session) { if (this.session != null) { stop() From c0ef25756d7a770fe21e6606f09f92f0259bd51b Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 13:06:56 +0000 Subject: [PATCH 06/17] cleaning up method ordering and visibility --- .../notifications/NotificationEventQueue.kt | 49 +++++++++---------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt index 966c9fabad..83beb4fa02 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt @@ -46,14 +46,6 @@ class NotificationEventQueue( } } - private fun MutableList.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) { @@ -61,23 +53,6 @@ class NotificationEventQueue( queue.addAll(events) } - fun findExistingById(notifiableEvent: NotifiableEvent): NotifiableEvent? { - return queue.firstOrNull { it.eventId == notifiableEvent.eventId } - } - - fun findEdited(notifiableEvent: NotifiableEvent): NotifiableEvent? { - return notifiableEvent.editedEventId?.let { editedId -> - queue.firstOrNull { - it.eventId == editedId || it.editedEventId == editedId - } - } - } - - fun replace(replace: NotifiableEvent, with: NotifiableEvent) { - queue.remove(replace) - queue.add(with) - } - fun clear() { queue.clear() } @@ -116,6 +91,22 @@ class NotificationEventQueue( } } + private fun findExistingById(notifiableEvent: NotifiableEvent): NotifiableEvent? { + return queue.firstOrNull { it.eventId == notifiableEvent.eventId } + } + + private fun findEdited(notifiableEvent: NotifiableEvent): NotifiableEvent? { + return notifiableEvent.editedEventId?.let { editedId -> + queue.firstOrNull { + it.eventId == editedId || it.editedEventId == editedId + } + } + } + + private fun replace(replace: NotifiableEvent, with: NotifiableEvent) { + queue.remove(replace) + queue.add(with) + } fun clearMemberShipNotificationForRoom(roomId: String) { Timber.v("clearMemberShipOfRoom $roomId") @@ -129,3 +120,11 @@ class NotificationEventQueue( fun rawEvents(): List = queue } + +private fun MutableList.replace(eventId: String, block: (NotifiableEvent) -> NotifiableEvent) { + val indexToReplace = indexOfFirst { it.eventId == eventId } + if (indexToReplace == -1) { + return + } + set(indexToReplace, block(get(indexToReplace))) +} From 6bc121ad4a379b5e69f4c4b3446c19fc992df857 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 13:12:49 +0000 Subject: [PATCH 07/17] extracting notifiable event fixtures to their own file --- .../NotifiableEventProcessorTest.kt | 48 +------------- .../notifications/NotificationFactoryTest.kt | 3 + .../test/fixtures/NotifiableEventFixture.kt | 66 +++++++++++++++++++ 3 files changed, 72 insertions(+), 45 deletions(-) create mode 100644 vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotifiableEventProcessorTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotifiableEventProcessorTest.kt index 229ab39d1d..131a423316 100644 --- a/vector/src/test/java/im/vector/app/features/notifications/NotifiableEventProcessorTest.kt +++ b/vector/src/test/java/im/vector/app/features/notifications/NotifiableEventProcessorTest.kt @@ -19,6 +19,9 @@ package im.vector.app.features.notifications import im.vector.app.features.notifications.ProcessedEvent.Type import im.vector.app.test.fakes.FakeAutoAcceptInvites import im.vector.app.test.fakes.FakeOutdatedEventDetector +import im.vector.app.test.fixtures.aNotifiableMessageEvent +import im.vector.app.test.fixtures.aSimpleNotifiableEvent +import im.vector.app.test.fixtures.anInviteNotifiableEvent import org.amshove.kluent.shouldBeEqualTo import org.junit.Test import org.matrix.android.sdk.api.session.events.model.EventType @@ -145,48 +148,3 @@ class NotifiableEventProcessorTest { ProcessedEvent(it.first, it.second) } } - -fun aSimpleNotifiableEvent(eventId: String, type: String? = null) = SimpleNotifiableEvent( - matrixID = null, - eventId = eventId, - editedEventId = null, - noisy = false, - title = "title", - description = "description", - type = type, - timestamp = 0, - soundName = null, - canBeReplaced = false, - isRedacted = false -) - -fun anInviteNotifiableEvent(roomId: String) = InviteNotifiableEvent( - matrixID = null, - eventId = "event-id", - roomId = roomId, - roomName = "a room name", - editedEventId = null, - noisy = false, - title = "title", - description = "description", - type = null, - timestamp = 0, - soundName = null, - canBeReplaced = false, - isRedacted = false -) - -fun aNotifiableMessageEvent(eventId: String, roomId: String) = NotifiableMessageEvent( - eventId = eventId, - editedEventId = null, - noisy = false, - timestamp = 0, - senderName = "sender-name", - senderId = "sending-id", - body = "message-body", - roomId = roomId, - roomName = "room-name", - roomIsDirect = false, - canBeReplaced = false, - isRedacted = false -) diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationFactoryTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationFactoryTest.kt index d720881bac..f0f9a4dbc7 100644 --- a/vector/src/test/java/im/vector/app/features/notifications/NotificationFactoryTest.kt +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationFactoryTest.kt @@ -20,6 +20,9 @@ import im.vector.app.features.notifications.ProcessedEvent.Type import im.vector.app.test.fakes.FakeNotificationUtils import im.vector.app.test.fakes.FakeRoomGroupMessageCreator import im.vector.app.test.fakes.FakeSummaryGroupMessageCreator +import im.vector.app.test.fixtures.aNotifiableMessageEvent +import im.vector.app.test.fixtures.aSimpleNotifiableEvent +import im.vector.app.test.fixtures.anInviteNotifiableEvent import org.amshove.kluent.shouldBeEqualTo import org.junit.Test diff --git a/vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt b/vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt new file mode 100644 index 0000000000..f86953b338 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt @@ -0,0 +1,66 @@ +/* + * 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.test.fixtures + +import im.vector.app.features.notifications.InviteNotifiableEvent +import im.vector.app.features.notifications.NotifiableMessageEvent +import im.vector.app.features.notifications.SimpleNotifiableEvent + +fun aSimpleNotifiableEvent(eventId: String, type: String? = null) = SimpleNotifiableEvent( + matrixID = null, + eventId = eventId, + editedEventId = null, + noisy = false, + title = "title", + description = "description", + type = type, + timestamp = 0, + soundName = null, + canBeReplaced = false, + isRedacted = false +) + +fun anInviteNotifiableEvent(roomId: String) = InviteNotifiableEvent( + matrixID = null, + eventId = "event-id", + roomId = roomId, + roomName = "a room name", + editedEventId = null, + noisy = false, + title = "title", + description = "description", + type = null, + timestamp = 0, + soundName = null, + canBeReplaced = false, + isRedacted = false +) + +fun aNotifiableMessageEvent(eventId: String, roomId: String) = NotifiableMessageEvent( + eventId = eventId, + editedEventId = null, + noisy = false, + timestamp = 0, + senderName = "sender-name", + senderId = "sending-id", + body = "message-body", + roomId = roomId, + roomName = "room-name", + roomIsDirect = false, + canBeReplaced = false, + isRedacted = false +) From 57037c9ac62d43561980f5a48ff061a060da42fe Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 15:00:57 +0000 Subject: [PATCH 08/17] adding test cases around the mutating of the notification event queue --- .../NotificationEventQueueTest.kt | 216 ++++++++++++++++++ .../test/fixtures/NotifiableEventFixture.kt | 32 ++- 2 files changed, 239 insertions(+), 9 deletions(-) create mode 100644 vector/src/test/java/im/vector/app/features/notifications/NotificationEventQueueTest.kt diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationEventQueueTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationEventQueueTest.kt new file mode 100644 index 0000000000..26c0030d26 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationEventQueueTest.kt @@ -0,0 +1,216 @@ +/* + * 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 im.vector.app.test.fixtures.aNotifiableMessageEvent +import im.vector.app.test.fixtures.aSimpleNotifiableEvent +import im.vector.app.test.fixtures.anInviteNotifiableEvent +import org.amshove.kluent.shouldBeEqualTo +import org.junit.Test + +class NotificationEventQueueTest { + + private val seenIdsCache = CircularCache.create(5) + + @Test + fun `given events when redacting some then marks matching event ids as redacted`() { + val queue = givenQueue(listOf( + aSimpleNotifiableEvent(eventId = "redacted-id-1"), + aNotifiableMessageEvent(eventId = "redacted-id-2"), + anInviteNotifiableEvent(eventId = "redacted-id-3"), + aSimpleNotifiableEvent(eventId = "kept-id"), + )) + + queue.markRedacted(listOf("redacted-id-1", "redacted-id-2", "redacted-id-3")) + + queue.rawEvents() shouldBeEqualTo listOf( + aSimpleNotifiableEvent(eventId = "redacted-id-1", isRedacted = true), + aNotifiableMessageEvent(eventId = "redacted-id-2", isRedacted = true), + anInviteNotifiableEvent(eventId = "redacted-id-3", isRedacted = true), + aSimpleNotifiableEvent(eventId = "kept-id", isRedacted = false), + ) + } + + @Test + fun `given invite event when leaving invited room and syncing then removes event`() { + val queue = givenQueue(listOf(anInviteNotifiableEvent(roomId = "a-room-id"))) + val roomsLeft = listOf("a-room-id") + + queue.syncRoomEvents(roomsLeft = roomsLeft, roomsJoined = emptyList()) + + queue.rawEvents() shouldBeEqualTo emptyList() + } + + @Test + fun `given invite event when joining invited room and syncing then removes event`() { + val queue = givenQueue(listOf(anInviteNotifiableEvent(roomId = "a-room-id"))) + val joinedRooms = listOf("a-room-id") + + queue.syncRoomEvents(roomsLeft = emptyList(), roomsJoined = joinedRooms) + + queue.rawEvents() shouldBeEqualTo emptyList() + } + + @Test + fun `given message event when leaving message room and syncing then removes event`() { + val queue = givenQueue(listOf(aNotifiableMessageEvent(roomId = "a-room-id"))) + val roomsLeft = listOf("a-room-id") + + queue.syncRoomEvents(roomsLeft = roomsLeft, roomsJoined = emptyList()) + + queue.rawEvents() shouldBeEqualTo emptyList() + } + + @Test + fun `given events when syncing without rooms left or joined ids then does not change the events`() { + val queue = givenQueue(listOf( + aNotifiableMessageEvent(roomId = "a-room-id"), + anInviteNotifiableEvent(roomId = "a-room-id") + )) + + queue.syncRoomEvents(roomsLeft = emptyList(), roomsJoined = emptyList()) + + queue.rawEvents() shouldBeEqualTo listOf( + aNotifiableMessageEvent(roomId = "a-room-id"), + anInviteNotifiableEvent(roomId = "a-room-id") + ) + } + + @Test + fun `given events then is not empty`() { + val queue = givenQueue(listOf(aSimpleNotifiableEvent())) + + queue.isEmpty() shouldBeEqualTo false + } + + @Test + fun `given no events then is empty`() { + val queue = givenQueue(emptyList()) + + queue.isEmpty() shouldBeEqualTo true + } + + @Test + fun `given events when clearing and adding then removes previous events and adds only new events`() { + val queue = givenQueue(listOf(aSimpleNotifiableEvent())) + + queue.clearAndAdd(listOf(anInviteNotifiableEvent())) + + queue.rawEvents() shouldBeEqualTo listOf(anInviteNotifiableEvent()) + } + + @Test + fun `when clearing then is empty`() { + val queue = givenQueue(listOf(aSimpleNotifiableEvent())) + + queue.clear() + + queue.rawEvents() shouldBeEqualTo emptyList() + } + + @Test + fun `given no events when adding then adds event`() { + val queue = givenQueue(listOf()) + + queue.add(aSimpleNotifiableEvent(), seenEventIds = seenIdsCache) + + queue.rawEvents() shouldBeEqualTo listOf(aSimpleNotifiableEvent()) + } + + @Test + fun `given no events when adding already seen event then ignores event`() { + val queue = givenQueue(listOf()) + val notifiableEvent = aSimpleNotifiableEvent() + seenIdsCache.put(notifiableEvent.eventId) + + queue.add(notifiableEvent, seenEventIds = seenIdsCache) + + queue.rawEvents() shouldBeEqualTo emptyList() + } + + @Test + fun `given replaceable event when adding event with same id then updates existing event`() { + val replaceableEvent = aSimpleNotifiableEvent(canBeReplaced = true) + val updatedEvent = replaceableEvent.copy(title = "updated title") + val queue = givenQueue(listOf(replaceableEvent)) + + queue.add(updatedEvent, seenEventIds = seenIdsCache) + + queue.rawEvents() shouldBeEqualTo listOf(updatedEvent) + } + + @Test + fun `given non replaceable event when adding event with same id then ignores event`() { + val nonReplaceableEvent = aSimpleNotifiableEvent(canBeReplaced = false) + val updatedEvent = nonReplaceableEvent.copy(title = "updated title") + val queue = givenQueue(listOf(nonReplaceableEvent)) + + queue.add(updatedEvent, seenEventIds = seenIdsCache) + + queue.rawEvents() shouldBeEqualTo listOf(nonReplaceableEvent) + } + + @Test + fun `given event when adding new event with edited event id matching the existing event id then updates existing event`() { + val editedEvent = aSimpleNotifiableEvent(eventId = "id-to-edit") + val updatedEvent = editedEvent.copy(eventId = "1", editedEventId = "id-to-edit", title = "updated title") + val queue = givenQueue(listOf(editedEvent)) + + queue.add(updatedEvent, seenEventIds = seenIdsCache) + + queue.rawEvents() shouldBeEqualTo listOf(updatedEvent) + } + + @Test + fun `given event when adding new event with edited event id matching the existing event edited id then updates existing event`() { + val editedEvent = aSimpleNotifiableEvent(eventId = "0", editedEventId = "id-to-edit") + val updatedEvent = editedEvent.copy(eventId = "1", editedEventId = "id-to-edit", title = "updated title") + val queue = givenQueue(listOf(editedEvent)) + + queue.add(updatedEvent, seenEventIds = seenIdsCache) + + queue.rawEvents() shouldBeEqualTo listOf(updatedEvent) + } + + @Test + fun `when clearing membership notification then removes invite events with matching room id`() { + val roomId = "a-room-id" + val queue = givenQueue(listOf( + anInviteNotifiableEvent(roomId = roomId), + aNotifiableMessageEvent(roomId = roomId) + )) + + queue.clearMemberShipNotificationForRoom(roomId) + + queue.rawEvents() shouldBeEqualTo listOf(aNotifiableMessageEvent(roomId = roomId)) + } + + @Test + fun `when clearing messages for room then removes message events with matching room id`() { + val roomId = "a-room-id" + val queue = givenQueue(listOf( + anInviteNotifiableEvent(roomId = roomId), + aNotifiableMessageEvent(roomId = roomId) + )) + + queue.clearMessagesForRoom(roomId) + + queue.rawEvents() shouldBeEqualTo listOf(anInviteNotifiableEvent(roomId = roomId)) + } + + private fun givenQueue(events: List) = NotificationEventQueue(events.toMutableList()) +} diff --git a/vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt b/vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt index f86953b338..a3dab7e069 100644 --- a/vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt +++ b/vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt @@ -20,23 +20,33 @@ import im.vector.app.features.notifications.InviteNotifiableEvent import im.vector.app.features.notifications.NotifiableMessageEvent import im.vector.app.features.notifications.SimpleNotifiableEvent -fun aSimpleNotifiableEvent(eventId: String, type: String? = null) = SimpleNotifiableEvent( +fun aSimpleNotifiableEvent( + eventId: String = "simple-event-id", + type: String? = null, + isRedacted: Boolean = false, + canBeReplaced: Boolean = false, + editedEventId: String? = null +) = SimpleNotifiableEvent( matrixID = null, eventId = eventId, - editedEventId = null, + editedEventId = editedEventId, noisy = false, title = "title", description = "description", type = type, timestamp = 0, soundName = null, - canBeReplaced = false, - isRedacted = false + canBeReplaced = canBeReplaced, + isRedacted = isRedacted ) -fun anInviteNotifiableEvent(roomId: String) = InviteNotifiableEvent( +fun anInviteNotifiableEvent( + roomId: String = "an-invite-room-id", + eventId: String = "invite-event-id", + isRedacted: Boolean = false +) = InviteNotifiableEvent( matrixID = null, - eventId = "event-id", + eventId = eventId, roomId = roomId, roomName = "a room name", editedEventId = null, @@ -47,10 +57,14 @@ fun anInviteNotifiableEvent(roomId: String) = InviteNotifiableEvent( timestamp = 0, soundName = null, canBeReplaced = false, - isRedacted = false + isRedacted = isRedacted ) -fun aNotifiableMessageEvent(eventId: String, roomId: String) = NotifiableMessageEvent( +fun aNotifiableMessageEvent( + eventId: String = "a-message-event-id", + roomId: String = "a-message-room-id", + isRedacted: Boolean = false +) = NotifiableMessageEvent( eventId = eventId, editedEventId = null, noisy = false, @@ -62,5 +76,5 @@ fun aNotifiableMessageEvent(eventId: String, roomId: String) = NotifiableMessage roomName = "room-name", roomIsDirect = false, canBeReplaced = false, - isRedacted = false + isRedacted = isRedacted ) From 037d1fcf521c63ab8b9cf2fb9040635d05c223ce Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 15:07:44 +0000 Subject: [PATCH 09/17] adding catch around the push event dispatching to match previous behaviour --- .../session/notification/DefaultPushRuleService.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/DefaultPushRuleService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/DefaultPushRuleService.kt index aaaeb4874e..3e821b8956 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/DefaultPushRuleService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/notification/DefaultPushRuleService.kt @@ -41,6 +41,7 @@ import org.matrix.android.sdk.internal.session.pushers.UpdatePushRuleActionsTask import org.matrix.android.sdk.internal.session.pushers.UpdatePushRuleEnableStatusTask import org.matrix.android.sdk.internal.task.TaskExecutor import org.matrix.android.sdk.internal.task.configureWith +import timber.log.Timber import javax.inject.Inject @SessionScope @@ -160,7 +161,11 @@ internal class DefaultPushRuleService @Inject constructor( fun dispatchEvents(pushEvents: PushEvents) { synchronized(listeners) { listeners.forEach { - it.onEvents(pushEvents) + try { + it.onEvents(pushEvents) + } catch (e: Throwable) { + Timber.e(e, "Error while dispatching push events") + } } } } From 7646f7ce32a5e4266bc8b9052bd876b4afd96a6a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 15:18:08 +0000 Subject: [PATCH 10/17] updating copyright header --- .../java/org/matrix/android/sdk/api/pushrules/PushEvents.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushEvents.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushEvents.kt index 38a793b60c..466e345cad 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushEvents.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/pushrules/PushEvents.kt @@ -1,11 +1,11 @@ /* - * Copyright (c) 2021 New Vector Ltd + * Copyright 2021 The Matrix.org Foundation C.I.C. * * 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 + * 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, @@ -13,7 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.matrix.android.sdk.api.pushrules import org.matrix.android.sdk.api.pushrules.rest.PushRule From 4597cb3816e9919cbe460c4323e7f57bf614fad7 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 18:18:06 +0000 Subject: [PATCH 11/17] add changelog entry for breaking API change --- changelog.d/4401.removal | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4401.removal diff --git a/changelog.d/4401.removal b/changelog.d/4401.removal new file mode 100644 index 0000000000..ce58372a18 --- /dev/null +++ b/changelog.d/4401.removal @@ -0,0 +1 @@ +Breaking SDK API change to PushRuleListener, the separated callbacks have been merged into one with a data class which includes all the previously separated push information From 8cc68e16d22f697b614cc9a4a3b4b75357faeaee Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 15:42:35 +0000 Subject: [PATCH 12/17] supporting images in the room notifications 15:40:32 - downloads and exports any images whilst resolving the notification event --- .../app/core/extensions/BasicExtensions.kt | 4 +++ .../notifications/NotifiableEventResolver.kt | 32 ++++++++++++++++--- .../notifications/NotifiableMessageEvent.kt | 2 ++ .../NotificationBroadcastReceiver.kt | 1 + .../notifications/PushRuleTriggerListener.kt | 29 +++++++++++------ .../notifications/RoomGroupMessageCreator.kt | 11 ++++++- 6 files changed, 64 insertions(+), 15 deletions(-) diff --git a/vector/src/main/java/im/vector/app/core/extensions/BasicExtensions.kt b/vector/src/main/java/im/vector/app/core/extensions/BasicExtensions.kt index ee3d79d846..dbe90dfdc1 100644 --- a/vector/src/main/java/im/vector/app/core/extensions/BasicExtensions.kt +++ b/vector/src/main/java/im/vector/app/core/extensions/BasicExtensions.kt @@ -66,3 +66,7 @@ fun String?.insertBeforeLast(insert: String, delimiter: String = "."): String { replaceRange(idx, idx, insert) } } + +inline fun Any?.takeAs(): R? { + return takeIf { it is R } as R? +} diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt index d2db73af3d..951b593358 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt @@ -15,8 +15,10 @@ */ package im.vector.app.features.notifications +import android.net.Uri import im.vector.app.BuildConfig import im.vector.app.R +import im.vector.app.core.extensions.takeAs import im.vector.app.core.resources.StringProvider import im.vector.app.features.displayname.getBestName import im.vector.app.features.home.room.detail.timeline.format.DisplayableEventFormatter @@ -31,9 +33,11 @@ import org.matrix.android.sdk.api.session.events.model.isEdition import org.matrix.android.sdk.api.session.events.model.toModel import org.matrix.android.sdk.api.session.room.model.Membership import org.matrix.android.sdk.api.session.room.model.RoomMemberContent +import org.matrix.android.sdk.api.session.room.model.message.MessageWithAttachmentContent import org.matrix.android.sdk.api.session.room.sender.SenderInfo import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent import org.matrix.android.sdk.api.session.room.timeline.getEditedEventId +import org.matrix.android.sdk.api.session.room.timeline.getLastMessageContent import org.matrix.android.sdk.api.util.toMatrixItem import org.matrix.android.sdk.internal.crypto.algorithms.olm.OlmDecryptionResult import timber.log.Timber @@ -49,11 +53,12 @@ import javax.inject.Inject class NotifiableEventResolver @Inject constructor( private val stringProvider: StringProvider, private val noticeEventFormatter: NoticeEventFormatter, - private val displayableEventFormatter: DisplayableEventFormatter) { + private val displayableEventFormatter: DisplayableEventFormatter +) { // private val eventDisplay = RiotEventDisplay(context) - fun resolveEvent(event: Event/*, roomState: RoomState?, bingRule: PushRule?*/, session: Session, isNoisy: Boolean): NotifiableEvent? { + suspend fun resolveEvent(event: Event/*, roomState: RoomState?, bingRule: PushRule?*/, session: Session, isNoisy: Boolean): NotifiableEvent? { val roomID = event.roomId ?: return null val eventId = event.eventId ?: return null if (event.getClearType() == EventType.STATE_ROOM_MEMBER) { @@ -89,7 +94,7 @@ class NotifiableEventResolver @Inject constructor( } } - fun resolveInMemoryEvent(session: Session, event: Event, canBeReplaced: Boolean): NotifiableEvent? { + suspend fun resolveInMemoryEvent(session: Session, event: Event, canBeReplaced: Boolean): NotifiableEvent? { if (event.getClearType() != EventType.MESSAGE) return null // Ignore message edition @@ -120,7 +125,7 @@ class NotifiableEventResolver @Inject constructor( } } - private fun resolveMessageEvent(event: TimelineEvent, session: Session, canBeReplaced: Boolean, isNoisy: Boolean): NotifiableEvent { + private suspend fun resolveMessageEvent(event: TimelineEvent, session: Session, canBeReplaced: Boolean, isNoisy: Boolean): NotifiableEvent { // The event only contains an eventId, and roomId (type is m.room.*) , we need to get the displayable content (names, avatar, text, etc...) val room = session.getRoom(event.root.roomId!! /*roomID cannot be null*/) @@ -140,6 +145,7 @@ class NotifiableEventResolver @Inject constructor( senderName = senderDisplayName, senderId = event.root.senderId, body = body.toString(), + imageUri = event.fetchImageIfPresent(session), roomId = event.root.roomId!!, roomName = roomName, matrixID = session.myUserId @@ -173,6 +179,7 @@ class NotifiableEventResolver @Inject constructor( senderName = senderDisplayName, senderId = event.root.senderId, body = body, + imageUri = event.fetchImageIfPresent(session), roomId = event.root.roomId!!, roomName = roomName, roomIsDirect = room.roomSummary()?.isDirect ?: false, @@ -192,6 +199,22 @@ class NotifiableEventResolver @Inject constructor( } } + private suspend fun TimelineEvent.fetchImageIfPresent(session: Session): Uri? { + return when { + root.isEncrypted() && root.mxDecryptionResult == null -> null + root.getClearType() == EventType.MESSAGE -> downloadAndExportImage(session) + else -> null + } + } + + private suspend fun TimelineEvent.downloadAndExportImage(session: Session): Uri? { + return getLastMessageContent()?.takeAs()?.let { imageMessage -> + val fileService = session.fileService() + fileService.downloadFile(imageMessage) + fileService.getTemporarySharableURI(imageMessage) + } + } + private fun resolveStateRoomEvent(event: Event, session: Session, canBeReplaced: Boolean, isNoisy: Boolean): NotifiableEvent? { val content = event.content?.toModel() ?: return null val roomId = event.roomId ?: return null @@ -224,3 +247,4 @@ class NotifiableEventResolver @Inject constructor( return null } } + diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableMessageEvent.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableMessageEvent.kt index 161c9f74a6..35718666b0 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableMessageEvent.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableMessageEvent.kt @@ -15,6 +15,7 @@ */ package im.vector.app.features.notifications +import android.net.Uri import org.matrix.android.sdk.api.session.events.model.EventType data class NotifiableMessageEvent( @@ -26,6 +27,7 @@ data class NotifiableMessageEvent( val senderName: String?, val senderId: String?, val body: String?, + val imageUri: Uri?, val roomId: String, val roomName: String?, val roomIsDirect: Boolean = false, diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt index 06ef3f4aeb..b1905059a1 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt @@ -138,6 +138,7 @@ class NotificationBroadcastReceiver : BroadcastReceiver() { ?: context?.getString(R.string.notification_sender_me), senderId = session.myUserId, body = message, + imageUri = null, roomId = room.roomId, roomName = room.roomSummary()?.displayName ?: room.roomId, roomIsDirect = room.roomSummary()?.isDirect == true, diff --git a/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt b/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt index f0633b24de..ff817520db 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/PushRuleTriggerListener.kt @@ -16,6 +16,11 @@ package im.vector.app.features.notifications +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancelChildren +import kotlinx.coroutines.launch import org.matrix.android.sdk.api.pushrules.PushEvents import org.matrix.android.sdk.api.pushrules.PushRuleService import org.matrix.android.sdk.api.pushrules.getActions @@ -31,21 +36,24 @@ class PushRuleTriggerListener @Inject constructor( ) : PushRuleService.PushRuleListener { private var session: Session? = null + private val scope: CoroutineScope = CoroutineScope(SupervisorJob()) override fun onEvents(pushEvents: PushEvents) { - session?.let { session -> - val notifiableEvents = createNotifiableEvents(pushEvents, session) - notificationDrawerManager.updateEvents { queuedEvents -> - notifiableEvents.forEach { notifiableEvent -> - queuedEvents.onNotifiableEventReceived(notifiableEvent) + scope.launch { + session?.let { session -> + val notifiableEvents = createNotifiableEvents(pushEvents, session) + notificationDrawerManager.updateEvents { queuedEvents -> + notifiableEvents.forEach { notifiableEvent -> + queuedEvents.onNotifiableEventReceived(notifiableEvent) + } + queuedEvents.syncRoomEvents(roomsLeft = pushEvents.roomsLeft, roomsJoined = pushEvents.roomsJoined) + queuedEvents.markRedacted(pushEvents.redactedEventIds) } - queuedEvents.syncRoomEvents(roomsLeft = pushEvents.roomsLeft, roomsJoined = pushEvents.roomsJoined) - queuedEvents.markRedacted(pushEvents.redactedEventIds) - } - } ?: Timber.e("Called without active session") + } ?: Timber.e("Called without active session") + } } - private fun createNotifiableEvents(pushEvents: PushEvents, session: Session): List { + private suspend fun createNotifiableEvents(pushEvents: PushEvents, session: Session): List { return pushEvents.matchedEvents.mapNotNull { (event, pushRule) -> Timber.v("Push rule match for event ${event.eventId}") val action = pushRule.getActions().toNotificationAction() @@ -67,6 +75,7 @@ class PushRuleTriggerListener @Inject constructor( } fun stop() { + scope.coroutineContext.cancelChildren(CancellationException("PushRuleTriggerListener stopping")) session?.removePushRuleListener(this) session = null notificationDrawerManager.clearAllEvents() diff --git a/vector/src/main/java/im/vector/app/features/notifications/RoomGroupMessageCreator.kt b/vector/src/main/java/im/vector/app/features/notifications/RoomGroupMessageCreator.kt index bdd7d026f9..2178e200e5 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/RoomGroupMessageCreator.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/RoomGroupMessageCreator.kt @@ -19,6 +19,7 @@ package im.vector.app.features.notifications import android.content.Context import android.graphics.Bitmap import android.os.Build +import android.util.Log import androidx.core.app.NotificationCompat import androidx.core.app.Person import androidx.core.content.pm.ShortcutInfoCompat @@ -103,6 +104,7 @@ class RoomGroupMessageCreator @Inject constructor( private fun NotificationCompat.MessagingStyle.addMessagesFromEvents(events: List) { events.forEach { event -> + Log.e("!!!", "event: $event") val senderPerson = if (event.outGoingMessage) { null } else { @@ -114,7 +116,14 @@ class RoomGroupMessageCreator @Inject constructor( } when { event.isSmartReplyError() -> addMessage(stringProvider.getString(R.string.notification_inline_reply_failed), event.timestamp, senderPerson) - else -> addMessage(event.body, event.timestamp, senderPerson) + else -> { + val message = NotificationCompat.MessagingStyle.Message(event.body, event.timestamp, senderPerson).also { message -> + event.imageUri?.let { + message.setData("image/", it) + } + } + addMessage(message) + } } } } From a37ff83fdcd69832bec4da321b249c8b2f41b667 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 15:44:58 +0000 Subject: [PATCH 13/17] catching any potential errors whilst download/exporting the notification image, will allow us to continue to show the notifications --- .../notifications/NotifiableEventResolver.kt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt index 951b593358..23f9fc307b 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt @@ -208,11 +208,15 @@ class NotifiableEventResolver @Inject constructor( } private suspend fun TimelineEvent.downloadAndExportImage(session: Session): Uri? { - return getLastMessageContent()?.takeAs()?.let { imageMessage -> - val fileService = session.fileService() - fileService.downloadFile(imageMessage) - fileService.getTemporarySharableURI(imageMessage) - } + return kotlin.runCatching { + getLastMessageContent()?.takeAs()?.let { imageMessage -> + val fileService = session.fileService() + fileService.downloadFile(imageMessage) + fileService.getTemporarySharableURI(imageMessage) + } + }.onFailure { + Timber.e(it, "Failed to download and export image for notification") + }.getOrNull() } private fun resolveStateRoomEvent(event: Event, session: Session, canBeReplaced: Boolean, isNoisy: Boolean): NotifiableEvent? { From 22f73c80f0699e2665cbdb66acd98111336c5d23 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 3 Nov 2021 15:51:52 +0000 Subject: [PATCH 14/17] removing debug log --- .../app/features/notifications/RoomGroupMessageCreator.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/RoomGroupMessageCreator.kt b/vector/src/main/java/im/vector/app/features/notifications/RoomGroupMessageCreator.kt index 2178e200e5..c1df6fe641 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/RoomGroupMessageCreator.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/RoomGroupMessageCreator.kt @@ -19,7 +19,6 @@ package im.vector.app.features.notifications import android.content.Context import android.graphics.Bitmap import android.os.Build -import android.util.Log import androidx.core.app.NotificationCompat import androidx.core.app.Person import androidx.core.content.pm.ShortcutInfoCompat @@ -104,7 +103,6 @@ class RoomGroupMessageCreator @Inject constructor( private fun NotificationCompat.MessagingStyle.addMessagesFromEvents(events: List) { events.forEach { event -> - Log.e("!!!", "event: $event") val senderPerson = if (event.outGoingMessage) { null } else { From 431f06020947b84c09ad5753649deef0ab276c7e Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 4 Nov 2021 09:55:05 +0000 Subject: [PATCH 15/17] add changelog entry --- changelog.d/4402.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4402.feature diff --git a/changelog.d/4402.feature b/changelog.d/4402.feature new file mode 100644 index 0000000000..29b9f9a337 --- /dev/null +++ b/changelog.d/4402.feature @@ -0,0 +1 @@ +Adds support for images inside message notifications \ No newline at end of file From 9e6bd2ee9c6abf2037810dface717e1f8b0800d5 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 4 Nov 2021 10:10:36 +0000 Subject: [PATCH 16/17] checking if the event is an image message to avoid attepting to render non image based attachments --- .../app/features/notifications/NotifiableEventResolver.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt index 23f9fc307b..fa66924497 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt @@ -30,6 +30,7 @@ import org.matrix.android.sdk.api.session.crypto.MXCryptoError import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.api.session.events.model.EventType import org.matrix.android.sdk.api.session.events.model.isEdition +import org.matrix.android.sdk.api.session.events.model.isImageMessage import org.matrix.android.sdk.api.session.events.model.toModel import org.matrix.android.sdk.api.session.room.model.Membership import org.matrix.android.sdk.api.session.room.model.RoomMemberContent @@ -202,7 +203,7 @@ class NotifiableEventResolver @Inject constructor( private suspend fun TimelineEvent.fetchImageIfPresent(session: Session): Uri? { return when { root.isEncrypted() && root.mxDecryptionResult == null -> null - root.getClearType() == EventType.MESSAGE -> downloadAndExportImage(session) + root.isImageMessage() -> downloadAndExportImage(session) else -> null } } From 145ceacf78a7c677a1f0a710d33025d93bc5b797 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 4 Nov 2021 10:43:51 +0000 Subject: [PATCH 17/17] fixing auto merge issues --- .../app/features/notifications/NotifiableEventResolver.kt | 1 - .../java/im/vector/app/test/fixtures/NotifiableEventFixture.kt | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt index fa66924497..87b31fa92a 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt @@ -252,4 +252,3 @@ class NotifiableEventResolver @Inject constructor( return null } } - diff --git a/vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt b/vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt index a3dab7e069..53d38aa228 100644 --- a/vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt +++ b/vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt @@ -76,5 +76,6 @@ fun aNotifiableMessageEvent( roomName = "room-name", roomIsDirect = false, canBeReplaced = false, - isRedacted = isRedacted + isRedacted = isRedacted, + imageUri = null )