From 098d4ab9e9275fe375b056eb25a11549c7d35953 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 5 Oct 2021 16:55:28 +0100 Subject: [PATCH 01/26] making the event body non null and immutable to allow less cases to be handled - also puts in the basis for a separate notification refreshing implementation --- .../NotificationDrawerManager.kt | 485 +++++++++--------- 1 file changed, 245 insertions(+), 240 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 8c0e5fee5f7..843b7208fde 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 @@ -250,31 +250,35 @@ class NotificationDrawerManager @Inject constructor(private val context: Context @WorkerThread private fun refreshNotificationDrawerBg() { Timber.v("refreshNotificationDrawerBg()") - val session = currentSession ?: return val user = session.getUser(session.myUserId) // myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash val myUserDisplayName = user?.toMatrixItem()?.getBestName() ?: session.myUserId val myUserAvatarUrl = session.contentUrlResolver().resolveThumbnail(user?.avatarUrl, avatarSize, avatarSize, ContentUrlResolver.ThumbnailMethod.SCALE) + synchronized(eventList) { - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER ") - // TMP code - var hasNewEvent = false - var summaryIsNoisy = false - val summaryInboxStyle = NotificationCompat.InboxStyle() - - // group events by room to create a single MessagingStyle notif - val roomIdToEventMap: MutableMap> = LinkedHashMap() - val simpleEvents: MutableList = ArrayList() - val invitationEvents: MutableList = ArrayList() - - val eventIterator = eventList.listIterator() - while (eventIterator.hasNext()) { - when (val event = eventIterator.next()) { - is NotifiableMessageEvent -> { - val roomId = event.roomId - val roomEvents = roomIdToEventMap.getOrPut(roomId) { ArrayList() } + val useSplitNotifications = false + if (useSplitNotifications) { + // TODO + } else { + Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER ") + // TMP code + var hasNewEvent = false + var summaryIsNoisy = false + val summaryInboxStyle = NotificationCompat.InboxStyle() + + // group events by room to create a single MessagingStyle notif + val roomIdToEventMap: MutableMap> = LinkedHashMap() + val simpleEvents: MutableList = ArrayList() + val invitationEvents: MutableList = ArrayList() + + val eventIterator = eventList.listIterator() + while (eventIterator.hasNext()) { + 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 @@ -296,59 +300,59 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } } - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER ${roomIdToEventMap.size} room groups") - - var globalLastMessageTimestamp = 0L + Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER ${roomIdToEventMap.size} room groups") - val newSettings = vectorPreferences.useCompleteNotificationFormat() - if (newSettings != useCompleteNotificationFormat) { - // Settings has changed, remove all current notifications - notificationUtils.cancelAllNotifications() - useCompleteNotificationFormat = newSettings - } + var globalLastMessageTimestamp = 0L - var simpleNotificationRoomCounter = 0 - var simpleNotificationMessageCounter = 0 - - // events have been grouped by roomId - for ((roomId, events) in roomIdToEventMap) { - // Build the notification for the room - if (events.isEmpty() || events.all { it.isRedacted }) { - // Just clear this notification - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER $roomId has no more events") - notificationUtils.cancelNotificationMessage(roomId, ROOM_MESSAGES_NOTIFICATION_ID) - continue + val newSettings = vectorPreferences.useCompleteNotificationFormat() + if (newSettings != useCompleteNotificationFormat) { + // Settings has changed, remove all current notifications + notificationUtils.cancelAllNotifications() + useCompleteNotificationFormat = newSettings } - simpleNotificationRoomCounter++ - val roomName = events[0].roomName ?: events[0].senderName ?: "" - - val roomEventGroupInfo = RoomEventGroupInfo( - roomId = roomId, - isDirect = events[0].roomIsDirect, - roomDisplayName = roomName) + var simpleNotificationRoomCounter = 0 + var simpleNotificationMessageCounter = 0 + + // events have been grouped by roomId + for ((roomId, events) in roomIdToEventMap) { + // Build the notification for the room + if (events.isEmpty() || events.all { it.isRedacted }) { + // Just clear this notification + Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER $roomId has no more events") + notificationUtils.cancelNotificationMessage(roomId, ROOM_MESSAGES_NOTIFICATION_ID) + continue + } - val style = NotificationCompat.MessagingStyle(Person.Builder() - .setName(myUserDisplayName) - .setIcon(iconLoader.getUserIcon(myUserAvatarUrl)) - .setKey(events[0].matrixID) - .build()) + simpleNotificationRoomCounter++ + val roomName = events[0].roomName ?: events[0].senderName ?: "" - style.isGroupConversation = !roomEventGroupInfo.isDirect + val roomEventGroupInfo = RoomEventGroupInfo( + roomId = roomId, + isDirect = events[0].roomIsDirect, + roomDisplayName = roomName) - if (!roomEventGroupInfo.isDirect) { - style.conversationTitle = roomEventGroupInfo.roomDisplayName - } + val style = NotificationCompat.MessagingStyle(Person.Builder() + .setName(myUserDisplayName) + .setIcon(iconLoader.getUserIcon(myUserAvatarUrl)) + .setKey(events[0].matrixID) + .build()) - val largeBitmap = getRoomBitmap(events) + style.isGroupConversation = !roomEventGroupInfo.isDirect - for (event in events) { - // if all events in this room have already been displayed there is no need to update it - if (!event.hasBeenDisplayed && !event.isRedacted) { - roomEventGroupInfo.shouldBing = roomEventGroupInfo.shouldBing || event.noisy - roomEventGroupInfo.customSound = event.soundName + if (!roomEventGroupInfo.isDirect) { + style.conversationTitle = roomEventGroupInfo.roomDisplayName } - roomEventGroupInfo.hasNewEvent = roomEventGroupInfo.hasNewEvent || !event.hasBeenDisplayed + + val largeBitmap = getRoomBitmap(events) + + for (event in events) { + // if all events in this room have already been displayed there is no need to update it + if (!event.hasBeenDisplayed && !event.isRedacted) { + roomEventGroupInfo.shouldBing = roomEventGroupInfo.shouldBing || event.noisy + roomEventGroupInfo.customSound = event.soundName + } + roomEventGroupInfo.hasNewEvent = roomEventGroupInfo.hasNewEvent || !event.hasBeenDisplayed val senderPerson = if (event.outGoingMessage) { null @@ -373,211 +377,211 @@ class NotificationDrawerManager @Inject constructor(private val context: Context ShortcutManagerCompat.pushDynamicShortcut(context, shortcut) } - if (event.outGoingMessage && event.outGoingMessageFailed) { - style.addMessage(stringProvider.getString(R.string.notification_inline_reply_failed), event.timestamp, senderPerson) - roomEventGroupInfo.hasSmartReplyError = true - } else { - if (!event.isRedacted) { - simpleNotificationMessageCounter++ - style.addMessage(event.body, event.timestamp, senderPerson) + if (event.outGoingMessage && event.outGoingMessageFailed) { + style.addMessage(stringProvider.getString(R.string.notification_inline_reply_failed), event.timestamp, senderPerson) + roomEventGroupInfo.hasSmartReplyError = true + } else { + if (!event.isRedacted) { + simpleNotificationMessageCounter++ + style.addMessage(event.body, event.timestamp, senderPerson) + } } - } - event.hasBeenDisplayed = true // we can consider it as displayed + event.hasBeenDisplayed = true // we can consider it as displayed - // It is possible that this event was previously shown as an 'anonymous' simple notif. - // And now it will be merged in a single MessageStyle notif, so we can clean to be sure - notificationUtils.cancelNotificationMessage(event.eventId, ROOM_EVENT_NOTIFICATION_ID) - } + // It is possible that this event was previously shown as an 'anonymous' simple notif. + // And now it will be merged in a single MessageStyle notif, so we can clean to be sure + notificationUtils.cancelNotificationMessage(event.eventId, ROOM_EVENT_NOTIFICATION_ID) + } - try { - if (events.size == 1) { - val event = events[0] - if (roomEventGroupInfo.isDirect) { - val line = span { - span { - textStyle = "bold" - +String.format("%s: ", event.senderName) + try { + if (events.size == 1) { + val event = events[0] + if (roomEventGroupInfo.isDirect) { + val line = span { + span { + textStyle = "bold" + +String.format("%s: ", event.senderName) + } + +(event.description) } - +(event.description) - } - summaryInboxStyle.addLine(line) - } else { - val line = span { - span { - textStyle = "bold" - +String.format("%s: %s ", roomName, event.senderName) + summaryInboxStyle.addLine(line) + } else { + val line = span { + span { + textStyle = "bold" + +String.format("%s: %s ", roomName, event.senderName) + } + +(event.description) } - +(event.description) + summaryInboxStyle.addLine(line) } - summaryInboxStyle.addLine(line) + } else { + val summaryLine = stringProvider.getQuantityString( + R.plurals.notification_compat_summary_line_for_room, events.size, roomName, events.size) + summaryInboxStyle.addLine(summaryLine) } - } else { - val summaryLine = stringProvider.getQuantityString( - R.plurals.notification_compat_summary_line_for_room, events.size, roomName, events.size) - summaryInboxStyle.addLine(summaryLine) + } catch (e: Throwable) { + // String not found or bad format + Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER failed to resolve string") + summaryInboxStyle.addLine(roomName) } - } catch (e: Throwable) { - // String not found or bad format - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER failed to resolve string") - summaryInboxStyle.addLine(roomName) - } - if (firstTime || roomEventGroupInfo.hasNewEvent) { - // Should update displayed notification - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER $roomId need refresh") - val lastMessageTimestamp = events.last().timestamp + if (firstTime || roomEventGroupInfo.hasNewEvent) { + // Should update displayed notification + Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER $roomId need refresh") + val lastMessageTimestamp = events.last().timestamp - if (globalLastMessageTimestamp < lastMessageTimestamp) { - globalLastMessageTimestamp = lastMessageTimestamp - } + if (globalLastMessageTimestamp < lastMessageTimestamp) { + globalLastMessageTimestamp = lastMessageTimestamp + } - val tickerText = if (roomEventGroupInfo.isDirect) { - stringProvider.getString(R.string.notification_ticker_text_dm, events.last().senderName, events.last().description) + val tickerText = if (roomEventGroupInfo.isDirect) { + stringProvider.getString(R.string.notification_ticker_text_dm, events.last().senderName, events.last().description) + } else { + stringProvider.getString(R.string.notification_ticker_text_group, roomName, events.last().senderName, events.last().description) + } + + if (useCompleteNotificationFormat) { + val notification = notificationUtils.buildMessagesListNotification( + style, + roomEventGroupInfo, + largeBitmap, + lastMessageTimestamp, + myUserDisplayName, + tickerText) + + // is there an id for this room? + notificationUtils.showNotificationMessage(roomId, ROOM_MESSAGES_NOTIFICATION_ID, notification) + } + + hasNewEvent = true + summaryIsNoisy = summaryIsNoisy || roomEventGroupInfo.shouldBing } else { - stringProvider.getString(R.string.notification_ticker_text_group, roomName, events.last().senderName, events.last().description) + Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER $roomId is up to date") } + } - if (useCompleteNotificationFormat) { - val notification = notificationUtils.buildMessagesListNotification( - style, - roomEventGroupInfo, - largeBitmap, - lastMessageTimestamp, - myUserDisplayName, - tickerText) - - // is there an id for this room? - notificationUtils.showNotificationMessage(roomId, ROOM_MESSAGES_NOTIFICATION_ID, notification) + // Handle invitation events + for (event in invitationEvents) { + // We build a invitation notification + if (firstTime || !event.hasBeenDisplayed) { + if (useCompleteNotificationFormat) { + 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) } - - hasNewEvent = true - summaryIsNoisy = summaryIsNoisy || roomEventGroupInfo.shouldBing - } else { - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER $roomId is up to date") } - } - // Handle invitation events - for (event in invitationEvents) { - // We build a invitation notification - if (firstTime || !event.hasBeenDisplayed) { - if (useCompleteNotificationFormat) { - val notification = notificationUtils.buildRoomInvitationNotification(event, session.myUserId) - notificationUtils.showNotificationMessage(event.roomId, ROOM_INVITATION_NOTIFICATION_ID, notification) + // Handle simple events + for (event in simpleEvents) { + // We build a simple notification + if (firstTime || !event.hasBeenDisplayed) { + if (useCompleteNotificationFormat) { + 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 + summaryIsNoisy = summaryIsNoisy || event.noisy + summaryInboxStyle.addLine(event.description) } - 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 notification - if (firstTime || !event.hasBeenDisplayed) { + // ======== Build summary notification ========= + // On Android 7.0 (API level 24) and higher, the system automatically builds a summary for + // your group using snippets of text from each notification. The user can expand this + // notification to see each separate notification. + // To support older versions, which cannot show a nested group of notifications, + // you must create an extra notification that acts as the summary. + // This appears as the only notification and the system hides all the others. + // So this summary should include a snippet from all the other notifications, + // which the user can tap to open your app. + // The behavior of the group summary may vary on some device types such as wearables. + // To ensure the best experience on all devices and versions, always include a group summary when you create a group + // https://developer.android.com/training/notify-user/group + + if (eventList.isEmpty() || eventList.all { it.isRedacted }) { + notificationUtils.cancelNotificationMessage(null, SUMMARY_NOTIFICATION_ID) + } else if (hasNewEvent) { + // FIXME roomIdToEventMap.size is not correct, this is the number of rooms + val nbEvents = roomIdToEventMap.size + simpleEvents.size + val sumTitle = stringProvider.getQuantityString(R.plurals.notification_compat_summary_title, nbEvents, nbEvents) + summaryInboxStyle.setBigContentTitle(sumTitle) + // TODO get latest event? + .setSummaryText(stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages, nbEvents, nbEvents)) + if (useCompleteNotificationFormat) { - 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 - summaryIsNoisy = summaryIsNoisy || event.noisy - summaryInboxStyle.addLine(event.description) - } - } + val notification = notificationUtils.buildSummaryListNotification( + summaryInboxStyle, + sumTitle, + noisy = hasNewEvent && summaryIsNoisy, + lastMessageTimestamp = globalLastMessageTimestamp) - // ======== Build summary notification ========= - // On Android 7.0 (API level 24) and higher, the system automatically builds a summary for - // your group using snippets of text from each notification. The user can expand this - // notification to see each separate notification. - // To support older versions, which cannot show a nested group of notifications, - // you must create an extra notification that acts as the summary. - // This appears as the only notification and the system hides all the others. - // So this summary should include a snippet from all the other notifications, - // which the user can tap to open your app. - // The behavior of the group summary may vary on some device types such as wearables. - // To ensure the best experience on all devices and versions, always include a group summary when you create a group - // https://developer.android.com/training/notify-user/group - - if (eventList.isEmpty() || eventList.all { it.isRedacted }) { - notificationUtils.cancelNotificationMessage(null, SUMMARY_NOTIFICATION_ID) - } else if (hasNewEvent) { - // FIXME roomIdToEventMap.size is not correct, this is the number of rooms - val nbEvents = roomIdToEventMap.size + simpleEvents.size - val sumTitle = stringProvider.getQuantityString(R.plurals.notification_compat_summary_title, nbEvents, nbEvents) - summaryInboxStyle.setBigContentTitle(sumTitle) - // TODO get latest event? - .setSummaryText(stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages, nbEvents, nbEvents)) - - if (useCompleteNotificationFormat) { - val notification = notificationUtils.buildSummaryListNotification( - summaryInboxStyle, - sumTitle, - noisy = hasNewEvent && summaryIsNoisy, - lastMessageTimestamp = globalLastMessageTimestamp) - - notificationUtils.showNotificationMessage(null, SUMMARY_NOTIFICATION_ID, notification) - } else { - // Add the simple events as message (?) - simpleNotificationMessageCounter += simpleEvents.size - val numberOfInvitations = invitationEvents.size - - val privacyTitle = if (numberOfInvitations > 0) { - val invitationsStr = stringProvider.getQuantityString(R.plurals.notification_invitations, numberOfInvitations, numberOfInvitations) - if (simpleNotificationMessageCounter > 0) { - // Invitation and message + notificationUtils.showNotificationMessage(null, SUMMARY_NOTIFICATION_ID, notification) + } else { + // Add the simple events as message (?) + simpleNotificationMessageCounter += simpleEvents.size + val numberOfInvitations = invitationEvents.size + + val privacyTitle = if (numberOfInvitations > 0) { + val invitationsStr = stringProvider.getQuantityString(R.plurals.notification_invitations, numberOfInvitations, numberOfInvitations) + if (simpleNotificationMessageCounter > 0) { + // Invitation and message + val messageStr = stringProvider.getQuantityString(R.plurals.room_new_messages_notification, + simpleNotificationMessageCounter, simpleNotificationMessageCounter) + if (simpleNotificationRoomCounter > 1) { + // In several rooms + val roomStr = stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages_in_room_rooms, + simpleNotificationRoomCounter, simpleNotificationRoomCounter) + stringProvider.getString( + R.string.notification_unread_notified_messages_in_room_and_invitation, + messageStr, + roomStr, + invitationsStr + ) + } else { + // In one room + stringProvider.getString( + R.string.notification_unread_notified_messages_and_invitation, + messageStr, + invitationsStr + ) + } + } else { + // Only invitation + invitationsStr + } + } else { + // No invitation, only messages val messageStr = stringProvider.getQuantityString(R.plurals.room_new_messages_notification, simpleNotificationMessageCounter, simpleNotificationMessageCounter) if (simpleNotificationRoomCounter > 1) { // In several rooms val roomStr = stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages_in_room_rooms, simpleNotificationRoomCounter, simpleNotificationRoomCounter) - stringProvider.getString( - R.string.notification_unread_notified_messages_in_room_and_invitation, - messageStr, - roomStr, - invitationsStr - ) + stringProvider.getString(R.string.notification_unread_notified_messages_in_room, messageStr, roomStr) } else { // In one room - stringProvider.getString( - R.string.notification_unread_notified_messages_and_invitation, - messageStr, - invitationsStr - ) + messageStr } - } else { - // Only invitation - invitationsStr } - } else { - // No invitation, only messages - val messageStr = stringProvider.getQuantityString(R.plurals.room_new_messages_notification, - simpleNotificationMessageCounter, simpleNotificationMessageCounter) - if (simpleNotificationRoomCounter > 1) { - // In several rooms - val roomStr = stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages_in_room_rooms, - simpleNotificationRoomCounter, simpleNotificationRoomCounter) - stringProvider.getString(R.string.notification_unread_notified_messages_in_room, messageStr, roomStr) - } else { - // In one room - messageStr - } - } - val notification = notificationUtils.buildSummaryListNotification( - style = null, - compatSummary = privacyTitle, - noisy = hasNewEvent && summaryIsNoisy, - lastMessageTimestamp = globalLastMessageTimestamp) + val notification = notificationUtils.buildSummaryListNotification( + style = null, + compatSummary = privacyTitle, + noisy = hasNewEvent && summaryIsNoisy, + lastMessageTimestamp = globalLastMessageTimestamp) - notificationUtils.showNotificationMessage(null, SUMMARY_NOTIFICATION_ID, notification) - } + notificationUtils.showNotificationMessage(null, SUMMARY_NOTIFICATION_ID, notification) + } - if (hasNewEvent && summaryIsNoisy) { - try { - // turn the screen on for 3 seconds - /* + if (hasNewEvent && summaryIsNoisy) { + try { + // turn the screen on for 3 seconds + /* TODO if (Matrix.getInstance(VectorApp.getInstance())!!.pushManager.isScreenTurnedOn) { val pm = VectorApp.getInstance().getSystemService()!! @@ -587,13 +591,14 @@ class NotificationDrawerManager @Inject constructor(private val context: Context wl.release() } */ - } catch (e: Throwable) { - Timber.e(e, "## Failed to turn screen on") + } catch (e: Throwable) { + Timber.e(e, "## Failed to turn screen on") + } } } + // notice that we can get bit out of sync with actual display but not a big issue + firstTime = false } - // notice that we can get bit out of sync with actual display but not a big issue - firstTime = false } } @@ -657,10 +662,10 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } companion object { - 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 + const val SUMMARY_NOTIFICATION_ID = 0 + const val ROOM_MESSAGES_NOTIFICATION_ID = 1 + const val ROOM_EVENT_NOTIFICATION_ID = 2 + const val ROOM_INVITATION_NOTIFICATION_ID = 3 // TODO Mutliaccount private const val ROOMS_NOTIFICATIONS_FILE_NAME = "im.vector.notifications.cache" From 51d3cc507de1bd7bb0a723a98e83f6a3eaf4f52d Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 6 Oct 2021 13:37:10 +0100 Subject: [PATCH 02/26] creating dedicated class for the processing the serialized events - updates the logic to track when events are removed as a way for the notifications to remove themselves, null events mean they've been removed --- .../notifications/NotifiableEventProcessor.kt | 73 +++++++ .../NotifiableEventProcessorTest.kt | 187 ++++++++++++++++++ .../app/test/fakes/FakeAutoAcceptInvites.kt | 27 +++ .../test/fakes/FakeOutdatedEventDetector.kt | 34 ++++ 4 files changed, 321 insertions(+) create mode 100644 vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt create mode 100644 vector/src/test/java/im/vector/app/features/notifications/NotifiableEventProcessorTest.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeAutoAcceptInvites.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeOutdatedEventDetector.kt diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt new file mode 100644 index 00000000000..3f77ce54ca5 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt @@ -0,0 +1,73 @@ +/* + * 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.features.invite.AutoAcceptInvites +import timber.log.Timber +import javax.inject.Inject + +class NotifiableEventProcessor @Inject constructor( + private val outdatedDetector: OutdatedEventDetector, + private val autoAcceptInvites: AutoAcceptInvites +) { + + fun modifyAndProcess(eventList: MutableList, currentRoomId: String?): ProcessedNotificationEvents { + val roomIdToEventMap: MutableMap> = LinkedHashMap() + val simpleEvents: MutableMap = LinkedHashMap() + val invitationEvents: MutableMap = LinkedHashMap() + + val eventIterator = eventList.listIterator() + while (eventIterator.hasNext()) { + when (val event = eventIterator.next()) { + is NotifiableMessageEvent -> { + val roomId = event.roomId + val roomEvents = roomIdToEventMap.getOrPut(roomId) { ArrayList() } + + // should we limit to last 7 messages per room? + if (shouldIgnoreMessageEventInRoom(currentRoomId, roomId) || outdatedDetector.isMessageOutdated(event)) { + // forget this event + eventIterator.remove() + } else { + roomEvents.add(event) + } + } + is InviteNotifiableEvent -> { + if (autoAcceptInvites.hideInvites) { + // Forget this event + eventIterator.remove() + invitationEvents[event.roomId] = null + } else { + invitationEvents[event.roomId] = event + } + } + is SimpleNotifiableEvent -> simpleEvents[event.eventId] = event + else -> Timber.w("Type not handled") + } + } + return ProcessedNotificationEvents(roomIdToEventMap, simpleEvents, invitationEvents) + } + + private fun shouldIgnoreMessageEventInRoom(currentRoomId: String?, roomId: String?): Boolean { + return currentRoomId != null && roomId == currentRoomId + } +} + +data class ProcessedNotificationEvents( + val roomEvents: Map>, + val simpleEvents: Map, + val invitationEvents: Map +) 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 new file mode 100644 index 00000000000..a5bb9978dd9 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/notifications/NotifiableEventProcessorTest.kt @@ -0,0 +1,187 @@ +/* + * 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.fakes.FakeAutoAcceptInvites +import im.vector.app.test.fakes.FakeOutdatedEventDetector +import org.amshove.kluent.shouldBeEqualTo +import org.junit.Test + +private val NOT_VIEWING_A_ROOM: String? = null + +class NotifiableEventProcessorTest { + + private val outdatedDetector = FakeOutdatedEventDetector() + private val autoAcceptInvites = FakeAutoAcceptInvites() + + private val eventProcessor = NotifiableEventProcessor(outdatedDetector.instance, autoAcceptInvites) + + @Test + fun `given simple events when processing then return without mutating`() { + val (events, originalEvents) = createEventsList( + aSimpleNotifiableEvent(eventId = "event-1"), + aSimpleNotifiableEvent(eventId = "event-2") + ) + + val result = eventProcessor.modifyAndProcess(events, currentRoomId = NOT_VIEWING_A_ROOM) + + result shouldBeEqualTo aProcessedNotificationEvents( + simpleEvents = mapOf( + "event-1" to events[0] as SimpleNotifiableEvent, + "event-2" to events[1] as SimpleNotifiableEvent + ) + ) + events shouldBeEqualTo originalEvents + } + + @Test + fun `given invites are auto accepted when processing then remove invitations`() { + autoAcceptInvites._isEnabled = true + val events = mutableListOf( + anInviteNotifiableEvent(roomId = "room-1"), + anInviteNotifiableEvent(roomId = "room-2") + ) + + val result = eventProcessor.modifyAndProcess(events, currentRoomId = NOT_VIEWING_A_ROOM) + + result shouldBeEqualTo aProcessedNotificationEvents( + invitationEvents = mapOf( + "room-1" to null, + "room-2" to null + ) + ) + events shouldBeEqualTo emptyList() + } + + @Test + fun `given invites are not auto accepted when processing then return without mutating`() { + autoAcceptInvites._isEnabled = false + val (events, originalEvents) = createEventsList( + anInviteNotifiableEvent(roomId = "room-1"), + anInviteNotifiableEvent(roomId = "room-2") + ) + + val result = eventProcessor.modifyAndProcess(events, currentRoomId = NOT_VIEWING_A_ROOM) + + result shouldBeEqualTo aProcessedNotificationEvents( + invitationEvents = mapOf( + "room-1" to originalEvents[0] as InviteNotifiableEvent, + "room-2" to originalEvents[1] as InviteNotifiableEvent + ) + ) + events shouldBeEqualTo originalEvents + } + + @Test + fun `given out of date message event when processing then removes message`() { + val (events) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) + outdatedDetector.givenEventIsOutOfDate(events[0]) + + val result = eventProcessor.modifyAndProcess(events, currentRoomId = NOT_VIEWING_A_ROOM) + + result shouldBeEqualTo aProcessedNotificationEvents( + roomEvents = mapOf( + "room-1" to emptyList() + ) + ) + events shouldBeEqualTo emptyList() + } + + @Test + fun `given in date message event when processing then without mutating`() { + val (events, originalEvents) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) + outdatedDetector.givenEventIsInDate(events[0]) + + val result = eventProcessor.modifyAndProcess(events, currentRoomId = NOT_VIEWING_A_ROOM) + + result shouldBeEqualTo aProcessedNotificationEvents( + roomEvents = mapOf( + "room-1" to listOf(events[0] as NotifiableMessageEvent) + ) + ) + events shouldBeEqualTo originalEvents + } + + @Test + fun `given viewing the same room as message event when processing then removes message`() { + val (events) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) + + val result = eventProcessor.modifyAndProcess(events, currentRoomId = "room-1") + + result shouldBeEqualTo aProcessedNotificationEvents( + roomEvents = mapOf( + "room-1" to emptyList() + ) + ) + events shouldBeEqualTo emptyList() + } +} + +fun createEventsList(vararg event: NotifiableEvent): Pair, List> { + val mutableEvents = mutableListOf(*event) + val immutableEvents = mutableEvents.toList() + return mutableEvents to immutableEvents +} + +fun aProcessedNotificationEvents(simpleEvents: Map = emptyMap(), + invitationEvents: Map = emptyMap(), + roomEvents: Map> = emptyMap() +) = ProcessedNotificationEvents( + roomEvents = roomEvents, + simpleEvents = simpleEvents, + invitationEvents = invitationEvents, +) + +fun aSimpleNotifiableEvent(eventId: String) = SimpleNotifiableEvent( + matrixID = null, + eventId = eventId, + editedEventId = null, + noisy = false, + title = "title", + description = "description", + type = null, + timestamp = 0, + soundName = null, + isPushGatewayEvent = false +) + +fun anInviteNotifiableEvent(roomId: String) = InviteNotifiableEvent( + matrixID = null, + eventId = "event-id", + roomId = roomId, + editedEventId = null, + noisy = false, + title = "title", + description = "description", + type = null, + timestamp = 0, + soundName = null, + isPushGatewayEvent = 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 +) diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeAutoAcceptInvites.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeAutoAcceptInvites.kt new file mode 100644 index 00000000000..778c2f113d8 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeAutoAcceptInvites.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 im.vector.app.test.fakes + +import im.vector.app.features.invite.AutoAcceptInvites + +class FakeAutoAcceptInvites : AutoAcceptInvites { + + var _isEnabled: Boolean = false + + override val isEnabled: Boolean + get() = _isEnabled +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeOutdatedEventDetector.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeOutdatedEventDetector.kt new file mode 100644 index 00000000000..0e1d617ca2a --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeOutdatedEventDetector.kt @@ -0,0 +1,34 @@ +/* + * 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.fakes + +import im.vector.app.features.notifications.NotifiableEvent +import im.vector.app.features.notifications.OutdatedEventDetector +import io.mockk.every +import io.mockk.mockk + +class FakeOutdatedEventDetector { + val instance = mockk() + + fun givenEventIsOutOfDate(notifiableEvent: NotifiableEvent) { + every { instance.isMessageOutdated(notifiableEvent) } returns true + } + + fun givenEventIsInDate(notifiableEvent: NotifiableEvent) { + every { instance.isMessageOutdated(notifiableEvent) } returns false + } +} From dded4000dc81a22c098d9350584f39de14ed2483 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 6 Oct 2021 19:03:22 +0100 Subject: [PATCH 03/26] creating the notifications separate to where they're displayed - also handles when the event diff means the notifications should be removed --- .../notifications/NotificationFactory.kt | 106 +++++++++++++ .../notifications/RoomGroupMessageCreator.kt | 148 ++++++++++++++++++ .../SummaryGroupMessageCreator.kt | 140 +++++++++++++++++ .../notifications/NotificationFactoryTest.kt | 138 ++++++++++++++++ .../app/test/fakes/FakeNotificationUtils.kt | 41 +++++ .../test/fakes/FakeRoomGroupMessageCreator.kt | 37 +++++ .../fakes/FakeSummaryGroupMessageCreator.kt | 25 +++ 7 files changed, 635 insertions(+) create mode 100644 vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt create mode 100644 vector/src/main/java/im/vector/app/features/notifications/RoomGroupMessageCreator.kt create mode 100644 vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt create mode 100644 vector/src/test/java/im/vector/app/features/notifications/NotificationFactoryTest.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeNotificationUtils.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeRoomGroupMessageCreator.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeSummaryGroupMessageCreator.kt diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt new file mode 100644 index 00000000000..174a457334d --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -0,0 +1,106 @@ +/* + * Copyright (c) 2021 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.notifications + +import android.app.Notification +import javax.inject.Inject + +class NotificationFactory @Inject constructor( + private val notificationUtils: NotificationUtils, + private val roomGroupMessageCreator: RoomGroupMessageCreator, + private val summaryGroupMessageCreator: SummaryGroupMessageCreator +) { + + fun Map>.toNotifications(myUserDisplayName: String, myUserAvatarUrl: String?): List { + return this.map { (roomId, events) -> + when { + events.hasNoEventsToDisplay() -> RoomNotification.EmptyRoom(roomId) + else -> roomGroupMessageCreator.createRoomMessage(events, roomId, myUserDisplayName, myUserAvatarUrl) + } + } + } + + private fun List.hasNoEventsToDisplay() = isEmpty() || all { it.canNotBeDisplayed() } + + private fun NotifiableMessageEvent.canNotBeDisplayed() = hasBeenDisplayed || isRedacted + + fun Map.toNotifications(myUserId: String): List { + return this.map { (roomId, event) -> + when (event) { + null -> OneShotNotification.Removed(key = roomId) + else -> OneShotNotification.Append( + notificationUtils.buildRoomInvitationNotification(event, myUserId), + OneShotNotification.Append.Meta(key = roomId, summaryLine = event.description, isNoisy = event.noisy) + ) + } + } + } + + @JvmName("toNotificationsSimpleNotifiableEvent") + fun Map.toNotifications(myUserId: String): List { + return this.map { (eventId, event) -> + when (event) { + null -> OneShotNotification.Removed(key = eventId) + else -> OneShotNotification.Append( + notificationUtils.buildSimpleEventNotification(event, myUserId), + OneShotNotification.Append.Meta(key = eventId, summaryLine = event.description, isNoisy = event.noisy) + ) + } + } + } + + fun createSummaryNotification(roomNotifications: List, + invitationNotifications: List, + simpleNotifications: List, + useCompleteNotificationFormat: Boolean): Notification { + return summaryGroupMessageCreator.createSummaryNotification( + roomNotifications = roomNotifications.mapToMeta(), + invitationNotifications = invitationNotifications.mapToMeta(), + simpleNotifications = simpleNotifications.mapToMeta(), + useCompleteNotificationFormat = useCompleteNotificationFormat + ) + } +} + +private fun List.mapToMeta() = filterIsInstance().map { it.meta } + +@JvmName("mapToMetaOneShotNotification") +private fun List.mapToMeta() = filterIsInstance().map { it.meta } + +sealed interface RoomNotification { + data class EmptyRoom(val roomId: String) : RoomNotification + data class Message(val notification: Notification, val meta: Meta) : RoomNotification { + data class Meta( + val summaryLine: CharSequence, + val messageCount: Int, + val latestTimestamp: Long, + val roomId: String, + val shouldBing: Boolean + ) + } +} + +sealed interface OneShotNotification { + data class Removed(val key: String) : OneShotNotification + data class Append(val notification: Notification, val meta: Meta) : OneShotNotification { + data class Meta( + val key: String, + val summaryLine: CharSequence, + val isNoisy: Boolean + ) + } +} 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 new file mode 100644 index 00000000000..786ce40046b --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/notifications/RoomGroupMessageCreator.kt @@ -0,0 +1,148 @@ +/* + * Copyright (c) 2021 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.notifications + +import android.graphics.Bitmap +import androidx.core.app.NotificationCompat +import androidx.core.app.Person +import im.vector.app.R +import im.vector.app.core.resources.StringProvider +import me.gujun.android.span.Span +import me.gujun.android.span.span +import timber.log.Timber +import javax.inject.Inject + +class RoomGroupMessageCreator @Inject constructor( + private val iconLoader: IconLoader, + private val bitmapLoader: BitmapLoader, + private val stringProvider: StringProvider, + private val notificationUtils: NotificationUtils +) { + + fun createRoomMessage(events: List, roomId: String, userDisplayName: String, userAvatarUrl: String?): RoomNotification.Message { + val firstKnownRoomEvent = events[0] + val roomName = firstKnownRoomEvent.roomName ?: firstKnownRoomEvent.senderName ?: "" + val roomIsGroup = !firstKnownRoomEvent.roomIsDirect + val style = NotificationCompat.MessagingStyle(Person.Builder() + .setName(userDisplayName) + .setIcon(iconLoader.getUserIcon(userAvatarUrl)) + .setKey(firstKnownRoomEvent.matrixID) + .build() + ).also { + it.conversationTitle = roomName.takeIf { roomIsGroup } + it.isGroupConversation = roomIsGroup + it.addMessagesFromEvents(events) + } + + val tickerText = if (roomIsGroup) { + stringProvider.getString(R.string.notification_ticker_text_group, roomName, events.last().senderName, events.last().description) + } else { + stringProvider.getString(R.string.notification_ticker_text_dm, events.last().senderName, events.last().description) + } + + val lastMessageTimestamp = events.last().timestamp + val smartReplyErrors = events.filter { it.isSmartReplyError() } + val messageCount = (events.size - smartReplyErrors.size) + val meta = RoomNotification.Message.Meta( + summaryLine = createRoomMessagesGroupSummaryLine(events, roomName, roomIsDirect = !roomIsGroup), + messageCount = messageCount, + latestTimestamp = lastMessageTimestamp, + roomId = roomId, + shouldBing = events.any { it.noisy } + ) + return RoomNotification.Message( + notificationUtils.buildMessagesListNotification( + style, + RoomEventGroupInfo(roomId, roomName, isDirect = !roomIsGroup).also { + it.hasSmartReplyError = smartReplyErrors.isNotEmpty() + it.shouldBing = meta.shouldBing + it.customSound = events.last().soundName + }, + largeIcon = getRoomBitmap(events), + lastMessageTimestamp, + userDisplayName, + tickerText + ), + meta + ) + } + + private fun NotificationCompat.MessagingStyle.addMessagesFromEvents(events: List) { + events.forEach { event -> + val senderPerson = Person.Builder() + .setName(event.senderName) + .setIcon(iconLoader.getUserIcon(event.senderAvatarPath)) + .setKey(event.senderId) + .build() + when { + event.isSmartReplyError() -> addMessage(stringProvider.getString(R.string.notification_inline_reply_failed), event.timestamp, senderPerson) + else -> addMessage(event.body, event.timestamp, senderPerson) + } + } + } + + private fun createRoomMessagesGroupSummaryLine(events: List, roomName: String, roomIsDirect: Boolean): CharSequence { + return try { + when (events.size) { + 1 -> createFirstMessageSummaryLine(events.first(), roomName, roomIsDirect) + else -> { + stringProvider.getQuantityString( + R.plurals.notification_compat_summary_line_for_room, + events.size, + roomName, + events.size + ) + } + } + } catch (e: Throwable) { + // String not found or bad format + Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER failed to resolve string") + roomName + } + } + + private fun createFirstMessageSummaryLine(event: NotifiableMessageEvent, roomName: String, roomIsDirect: Boolean): Span { + return if (roomIsDirect) { + span { + span { + textStyle = "bold" + +String.format("%s: ", event.senderName) + } + +(event.description) + } + } else { + span { + span { + textStyle = "bold" + +String.format("%s: %s ", roomName, event.senderName) + } + +(event.description) + } + } + } + + private fun getRoomBitmap(events: List): Bitmap? { + if (events.isEmpty()) return null + + // Use the last event (most recent?) + val roomAvatarPath = events.last().roomAvatarPath ?: events.last().senderAvatarPath + + return bitmapLoader.getRoomBitmap(roomAvatarPath) + } +} + +private fun NotifiableMessageEvent.isSmartReplyError() = this.outGoingMessage && this.outGoingMessageFailed diff --git a/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt b/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt new file mode 100644 index 00000000000..38eac8b5656 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt @@ -0,0 +1,140 @@ +/* + * Copyright (c) 2021 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.notifications + +import android.app.Notification +import androidx.core.app.NotificationCompat +import im.vector.app.R +import im.vector.app.core.resources.StringProvider +import javax.inject.Inject + +class SummaryGroupMessageCreator @Inject constructor( + private val stringProvider: StringProvider, + private val notificationUtils: NotificationUtils +) { + + /** + * ======== Build summary notification ========= + * On Android 7.0 (API level 24) and higher, the system automatically builds a summary for + * your group using snippets of text from each notification. The user can expand this + * notification to see each separate notification. + * To support older versions, which cannot show a nested group of notifications, + * you must create an extra notification that acts as the summary. + * This appears as the only notification and the system hides all the others. + * So this summary should include a snippet from all the other notifications, + * which the user can tap to open your app. + * The behavior of the group summary may vary on some device types such as wearables. + * To ensure the best experience on all devices and versions, always include a group summary when you create a group + * https://developer.android.com/training/notify-user/group + */ + fun createSummaryNotification(roomNotifications: List, + invitationNotifications: List, + simpleNotifications: List, + useCompleteNotificationFormat: Boolean): Notification { + val summaryInboxStyle = NotificationCompat.InboxStyle().also { style -> + roomNotifications.forEach { style.addLine(it.summaryLine) } + invitationNotifications.forEach { style.addLine(it.summaryLine) } + simpleNotifications.forEach { style.addLine(it.summaryLine) } + } + + val summaryIsNoisy = roomNotifications.any { it.shouldBing } + || invitationNotifications.any { it.isNoisy } + || simpleNotifications.any { it.isNoisy } + + val messageCount = roomNotifications.fold(initial = 0) { acc, current -> acc + current.messageCount } + + val lastMessageTimestamp1 = roomNotifications.last().latestTimestamp + + // FIXME roomIdToEventMap.size is not correct, this is the number of rooms + val nbEvents = roomNotifications.size + simpleNotifications.size + val sumTitle = stringProvider.getQuantityString(R.plurals.notification_compat_summary_title, nbEvents, nbEvents) + summaryInboxStyle.setBigContentTitle(sumTitle) + // TODO get latest event? + .setSummaryText(stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages, nbEvents, nbEvents)) + return if (useCompleteNotificationFormat + ) { + notificationUtils.buildSummaryListNotification( + summaryInboxStyle, + sumTitle, + noisy = summaryIsNoisy, + lastMessageTimestamp = lastMessageTimestamp1 + ) + } else { + processSimpleGroupSummary(summaryIsNoisy, messageCount, + simpleNotifications.size, invitationNotifications.size, + roomNotifications.size, lastMessageTimestamp1) + } + } + + private fun processSimpleGroupSummary(summaryIsNoisy: Boolean, + messageEventsCount: Int, + simpleEventsCount: Int, + invitationEventsCount: Int, + roomCount: Int, + lastMessageTimestamp: Long): Notification { + // Add the simple events as message (?) + val messageNotificationCount = messageEventsCount + simpleEventsCount + + val privacyTitle = if (invitationEventsCount > 0) { + val invitationsStr = stringProvider.getQuantityString(R.plurals.notification_invitations, invitationEventsCount, invitationEventsCount) + if (messageNotificationCount > 0) { + // Invitation and message + val messageStr = stringProvider.getQuantityString(R.plurals.room_new_messages_notification, + messageNotificationCount, messageNotificationCount) + if (roomCount > 1) { + // In several rooms + val roomStr = stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages_in_room_rooms, + roomCount, roomCount) + stringProvider.getString( + R.string.notification_unread_notified_messages_in_room_and_invitation, + messageStr, + roomStr, + invitationsStr + ) + } else { + // In one room + stringProvider.getString( + R.string.notification_unread_notified_messages_and_invitation, + messageStr, + invitationsStr + ) + } + } else { + // Only invitation + invitationsStr + } + } else { + // No invitation, only messages + val messageStr = stringProvider.getQuantityString(R.plurals.room_new_messages_notification, + messageNotificationCount, messageNotificationCount) + if (roomCount > 1) { + // In several rooms + val roomStr = stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages_in_room_rooms, roomCount, roomCount) + stringProvider.getString(R.string.notification_unread_notified_messages_in_room, messageStr, roomStr) + } else { + // In one room + messageStr + } + } + return notificationUtils.buildSummaryListNotification( + style = null, + compatSummary = privacyTitle, + noisy = summaryIsNoisy, + lastMessageTimestamp = lastMessageTimestamp + ) + } +} 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 new file mode 100644 index 00000000000..c42e0b21c17 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationFactoryTest.kt @@ -0,0 +1,138 @@ +/* + * 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.fakes.FakeNotificationUtils +import im.vector.app.test.fakes.FakeRoomGroupMessageCreator +import im.vector.app.test.fakes.FakeSummaryGroupMessageCreator +import org.amshove.kluent.shouldBeEqualTo +import org.junit.Test + +private const val MY_USER_ID = "user-id" +private const val A_ROOM_ID = "room-id" +private const val AN_EVENT_ID = "event-id" + +private val MY_AVATAR_URL: String? = null +private val AN_INVITATION_EVENT = anInviteNotifiableEvent(roomId = A_ROOM_ID) +private val A_SIMPLE_EVENT = aSimpleNotifiableEvent(eventId = AN_EVENT_ID) +private val A_MESSAGE_EVENT = aNotifiableMessageEvent(eventId = AN_EVENT_ID, roomId = A_ROOM_ID) + +class NotificationFactoryTest { + + private val notificationUtils = FakeNotificationUtils() + private val roomGroupMessageCreator = FakeRoomGroupMessageCreator() + private val summaryGroupMessageCreator = FakeSummaryGroupMessageCreator() + + private val notificationFactory = NotificationFactory( + notificationUtils.instance, + roomGroupMessageCreator.instance, + summaryGroupMessageCreator.instance + ) + + @Test + fun `given a room invitation when mapping to notification then is Append`() = testWith(notificationFactory) { + val expectedNotification = notificationUtils.givenBuildRoomInvitationNotificationFor(AN_INVITATION_EVENT, MY_USER_ID) + val roomInvitation = mapOf(A_ROOM_ID to AN_INVITATION_EVENT) + + val result = roomInvitation.toNotifications(MY_USER_ID) + + result shouldBeEqualTo listOf(OneShotNotification.Append( + notification = expectedNotification, + meta = OneShotNotification.Append.Meta( + key = A_ROOM_ID, + summaryLine = AN_INVITATION_EVENT.description, + isNoisy = AN_INVITATION_EVENT.noisy + )) + ) + } + + @Test + fun `given a missing event in room invitation when mapping to notification then is Removed`() = testWith(notificationFactory) { + val missingEventRoomInvitation: Map = mapOf(A_ROOM_ID to null) + + val result = missingEventRoomInvitation.toNotifications(MY_USER_ID) + + result shouldBeEqualTo listOf(OneShotNotification.Removed( + key = A_ROOM_ID + )) + } + + @Test + fun `given a simple event when mapping to notification then is Append`() = testWith(notificationFactory) { + val expectedNotification = notificationUtils.givenBuildSimpleInvitationNotificationFor(A_SIMPLE_EVENT, MY_USER_ID) + val roomInvitation = mapOf(AN_EVENT_ID to A_SIMPLE_EVENT) + + val result = roomInvitation.toNotifications(MY_USER_ID) + + result shouldBeEqualTo listOf(OneShotNotification.Append( + notification = expectedNotification, + meta = OneShotNotification.Append.Meta( + key = AN_EVENT_ID, + summaryLine = A_SIMPLE_EVENT.description, + isNoisy = A_SIMPLE_EVENT.noisy + )) + ) + } + + @Test + fun `given a missing simple event when mapping to notification then is Removed`() = testWith(notificationFactory) { + val missingEventRoomInvitation: Map = mapOf(AN_EVENT_ID to null) + + val result = missingEventRoomInvitation.toNotifications(MY_USER_ID) + + result shouldBeEqualTo listOf(OneShotNotification.Removed( + key = AN_EVENT_ID + )) + } + + @Test + fun `given room with message when mapping to notification then delegates to room group message creator`() = testWith(notificationFactory) { + val events = listOf(A_MESSAGE_EVENT) + val expectedNotification = roomGroupMessageCreator.givenCreatesRoomMessageFor(events, A_ROOM_ID, MY_USER_ID, MY_AVATAR_URL) + val roomWithMessage = mapOf(A_ROOM_ID to events) + + val result = roomWithMessage.toNotifications(MY_USER_ID, MY_AVATAR_URL) + + result shouldBeEqualTo listOf(expectedNotification) + } + + @Test + fun `given a room with no events to display when mapping to notification then is Empty`() = testWith(notificationFactory) { + val emptyRoom: Map> = mapOf(A_ROOM_ID to emptyList()) + + val result = emptyRoom.toNotifications(MY_USER_ID, MY_AVATAR_URL) + + result shouldBeEqualTo listOf(RoomNotification.EmptyRoom( + roomId = A_ROOM_ID + )) + } + + @Test + fun `given a room with only redacted events when mapping to notification then is Empty`() = testWith(notificationFactory) { + val redactedRoom = mapOf(A_ROOM_ID to listOf(A_MESSAGE_EVENT.copy().apply { isRedacted = true })) + + val result = redactedRoom.toNotifications(MY_USER_ID, MY_AVATAR_URL) + + result shouldBeEqualTo listOf(RoomNotification.EmptyRoom( + roomId = A_ROOM_ID + )) + } +} + +fun testWith(receiver: T, block: T.() -> Unit) { + receiver.block() +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationUtils.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationUtils.kt new file mode 100644 index 00000000000..39f2ad59ff4 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationUtils.kt @@ -0,0 +1,41 @@ +/* + * 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.fakes + +import android.app.Notification +import im.vector.app.features.notifications.InviteNotifiableEvent +import im.vector.app.features.notifications.NotificationUtils +import im.vector.app.features.notifications.SimpleNotifiableEvent +import io.mockk.every +import io.mockk.mockk + +class FakeNotificationUtils { + + val instance = mockk() + + fun givenBuildRoomInvitationNotificationFor(event: InviteNotifiableEvent, myUserId: String): Notification { + val mockNotification = mockk() + every { instance.buildRoomInvitationNotification(event, myUserId) } returns mockNotification + return mockNotification + } + + fun givenBuildSimpleInvitationNotificationFor(event: SimpleNotifiableEvent, myUserId: String): Notification { + val mockNotification = mockk() + every { instance.buildSimpleEventNotification(event, myUserId) } returns mockNotification + return mockNotification + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeRoomGroupMessageCreator.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeRoomGroupMessageCreator.kt new file mode 100644 index 00000000000..c164b9a661f --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeRoomGroupMessageCreator.kt @@ -0,0 +1,37 @@ +/* + * 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.fakes + +import im.vector.app.features.notifications.NotifiableMessageEvent +import im.vector.app.features.notifications.RoomGroupMessageCreator +import im.vector.app.features.notifications.RoomNotification +import io.mockk.every +import io.mockk.mockk + +class FakeRoomGroupMessageCreator { + + val instance = mockk() + + fun givenCreatesRoomMessageFor(events: List, + roomId: String, + userDisplayName: String, + userAvatarUrl: String?): RoomNotification.Message { + val mockMessage = mockk() + every { instance.createRoomMessage(events, roomId, userDisplayName, userAvatarUrl) } returns mockMessage + return mockMessage + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeSummaryGroupMessageCreator.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeSummaryGroupMessageCreator.kt new file mode 100644 index 00000000000..eef77298a09 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeSummaryGroupMessageCreator.kt @@ -0,0 +1,25 @@ +/* + * 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.fakes + +import im.vector.app.features.notifications.SummaryGroupMessageCreator +import io.mockk.mockk + +class FakeSummaryGroupMessageCreator { + + val instance = mockk() +} From 5c4ea4a425c1b085f88d1242de004165d8a5329a Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 6 Oct 2021 20:28:29 +0100 Subject: [PATCH 04/26] chaining the event process, notification creation and display logic into a NotificationRender - extract the displaying into its own class to avoid leaking the entire notificationutils - cancel/display notification actions are completely driven by the event or abscense of event from the eventList - attempts to avoid redundant render passes by checking if the eventList has changed since the last render --- .../notifications/NotificationDisplayer.kt | 45 +++++ .../notifications/NotificationFactory.kt | 4 +- .../notifications/NotificationRenderer.kt | 101 ++++++++++ .../SummaryGroupMessageCreator.kt | 28 +-- .../notifications/NotificationFactoryTest.kt | 4 +- .../notifications/NotificationRendererTest.kt | 183 ++++++++++++++++++ .../fakes/FakeNotifiableEventProcessor.kt | 32 +++ .../test/fakes/FakeNotificationDisplayer.kt | 42 ++++ .../app/test/fakes/FakeNotificationFactory.kt | 56 ++++++ .../app/test/fakes/FakeVectorPreferences.kt | 30 +++ 10 files changed, 507 insertions(+), 18 deletions(-) create mode 100644 vector/src/main/java/im/vector/app/features/notifications/NotificationDisplayer.kt create mode 100644 vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt create mode 100644 vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeNotificationDisplayer.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt create mode 100644 vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationDisplayer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationDisplayer.kt new file mode 100644 index 00000000000..680ff32a52e --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationDisplayer.kt @@ -0,0 +1,45 @@ +/* + * Copyright (c) 2021 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.notifications + +import android.app.Notification +import android.content.Context +import androidx.core.app.NotificationManagerCompat +import timber.log.Timber +import javax.inject.Inject + +class NotificationDisplayer @Inject constructor(context: Context) { + + private val notificationManager = NotificationManagerCompat.from(context) + + fun showNotificationMessage(tag: String?, id: Int, notification: Notification) { + notificationManager.notify(tag, id, notification) + } + + fun cancelNotificationMessage(tag: String?, id: Int) { + notificationManager.cancel(tag, id) + } + + fun cancelAllNotifications() { + // Keep this try catch (reported by GA) + try { + notificationManager.cancelAll() + } catch (e: Exception) { + Timber.e(e, "## cancelAllNotifications() failed") + } + } +} diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index 174a457334d..55e9f7352d3 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -28,7 +28,7 @@ class NotificationFactory @Inject constructor( fun Map>.toNotifications(myUserDisplayName: String, myUserAvatarUrl: String?): List { return this.map { (roomId, events) -> when { - events.hasNoEventsToDisplay() -> RoomNotification.EmptyRoom(roomId) + events.hasNoEventsToDisplay() -> RoomNotification.Removed(roomId) else -> roomGroupMessageCreator.createRoomMessage(events, roomId, myUserDisplayName, myUserAvatarUrl) } } @@ -82,7 +82,7 @@ private fun List.mapToMeta() = filterIsInstance.mapToMeta() = filterIsInstance().map { it.meta } sealed interface RoomNotification { - data class EmptyRoom(val roomId: String) : RoomNotification + data class Removed(val roomId: String) : RoomNotification data class Message(val notification: Notification, val meta: Meta) : RoomNotification { data class Meta( val summaryLine: CharSequence, diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt new file mode 100644 index 00000000000..257f9987745 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt @@ -0,0 +1,101 @@ +/* + * Copyright 2019 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 androidx.annotation.WorkerThread +import im.vector.app.features.settings.VectorPreferences +import timber.log.Timber +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class NotificationRenderer @Inject constructor(private val notifiableEventProcessor: NotifiableEventProcessor, + private val notificationDisplayer: NotificationDisplayer, + private val vectorPreferences: VectorPreferences, + private val notificationFactory: NotificationFactory) { + + private var lastKnownEventList = -1 + private var useCompleteNotificationFormat = vectorPreferences.useCompleteNotificationFormat() + + @WorkerThread + fun render(currentRoomId: String?, myUserId: String, myUserDisplayName: String, myUserAvatarUrl: String?, eventList: MutableList) { + Timber.v("refreshNotificationDrawerBg()") + val newSettings = vectorPreferences.useCompleteNotificationFormat() + if (newSettings != useCompleteNotificationFormat) { + // Settings has changed, remove all current notifications + notificationDisplayer.cancelAllNotifications() + useCompleteNotificationFormat = newSettings + } + + val notificationEvents = notifiableEventProcessor.modifyAndProcess(eventList, currentRoomId) + if (lastKnownEventList == notificationEvents.hashCode()) { + Timber.d("Skipping notification update due to event list not changing") + } else { + processEvents(notificationEvents, myUserId, myUserDisplayName, myUserAvatarUrl) + lastKnownEventList = notificationEvents.hashCode() + } + } + + private fun processEvents(notificationEvents: ProcessedNotificationEvents, myUserId: String, myUserDisplayName: String, myUserAvatarUrl: String?) { + val (roomEvents, simpleEvents, invitationEvents) = notificationEvents + with(notificationFactory) { + val roomNotifications = roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) + val invitationNotifications = invitationEvents.toNotifications(myUserId) + val simpleNotifications = simpleEvents.toNotifications(myUserId) + + if (roomNotifications.isEmpty() && invitationNotifications.isEmpty() && simpleNotifications.isEmpty()) { + notificationDisplayer.cancelNotificationMessage(null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID) + } else { + val summaryNotification = createSummaryNotification( + roomNotifications = roomNotifications, + invitationNotifications = invitationNotifications, + simpleNotifications = simpleNotifications, + useCompleteNotificationFormat = useCompleteNotificationFormat + ) + roomNotifications.forEach { wrapper -> + when (wrapper) { + is RoomNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.roomId, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID) + is RoomNotification.Message -> if (useCompleteNotificationFormat) { + Timber.d("Updating room messages notification ${wrapper.meta.roomId}") + notificationDisplayer.showNotificationMessage(wrapper.meta.roomId, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID, wrapper.notification) + } + } + } + + invitationNotifications.forEach { wrapper -> + when (wrapper) { + is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID) + is OneShotNotification.Append -> if (useCompleteNotificationFormat) { + Timber.d("Updating invitation notification ${wrapper.meta.key}") + notificationDisplayer.showNotificationMessage(wrapper.meta.key, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID, wrapper.notification) + } + } + } + + simpleNotifications.forEach { wrapper -> + when (wrapper) { + is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID) + is OneShotNotification.Append -> if (useCompleteNotificationFormat) { + Timber.d("Updating simple notification ${wrapper.meta.key}") + notificationDisplayer.showNotificationMessage(wrapper.meta.key, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID, wrapper.notification) + } + } + } + notificationDisplayer.showNotificationMessage(null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, summaryNotification) + } + } + } +} diff --git a/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt b/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt index 38eac8b5656..dc9ff92aa69 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt @@ -22,25 +22,25 @@ import im.vector.app.R import im.vector.app.core.resources.StringProvider import javax.inject.Inject +/** + * ======== Build summary notification ========= + * On Android 7.0 (API level 24) and higher, the system automatically builds a summary for + * your group using snippets of text from each notification. The user can expand this + * notification to see each separate notification. + * To support older versions, which cannot show a nested group of notifications, + * you must create an extra notification that acts as the summary. + * This appears as the only notification and the system hides all the others. + * So this summary should include a snippet from all the other notifications, + * which the user can tap to open your app. + * The behavior of the group summary may vary on some device types such as wearables. + * To ensure the best experience on all devices and versions, always include a group summary when you create a group + * https://developer.android.com/training/notify-user/group + */ class SummaryGroupMessageCreator @Inject constructor( private val stringProvider: StringProvider, private val notificationUtils: NotificationUtils ) { - /** - * ======== Build summary notification ========= - * On Android 7.0 (API level 24) and higher, the system automatically builds a summary for - * your group using snippets of text from each notification. The user can expand this - * notification to see each separate notification. - * To support older versions, which cannot show a nested group of notifications, - * you must create an extra notification that acts as the summary. - * This appears as the only notification and the system hides all the others. - * So this summary should include a snippet from all the other notifications, - * which the user can tap to open your app. - * The behavior of the group summary may vary on some device types such as wearables. - * To ensure the best experience on all devices and versions, always include a group summary when you create a group - * https://developer.android.com/training/notify-user/group - */ fun createSummaryNotification(roomNotifications: List, invitationNotifications: List, simpleNotifications: List, 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 c42e0b21c17..c08be9e8c7d 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 @@ -116,7 +116,7 @@ class NotificationFactoryTest { val result = emptyRoom.toNotifications(MY_USER_ID, MY_AVATAR_URL) - result shouldBeEqualTo listOf(RoomNotification.EmptyRoom( + result shouldBeEqualTo listOf(RoomNotification.Removed( roomId = A_ROOM_ID )) } @@ -127,7 +127,7 @@ class NotificationFactoryTest { val result = redactedRoom.toNotifications(MY_USER_ID, MY_AVATAR_URL) - result shouldBeEqualTo listOf(RoomNotification.EmptyRoom( + result shouldBeEqualTo listOf(RoomNotification.Removed( roomId = A_ROOM_ID )) } diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt new file mode 100644 index 00000000000..a07dd61368f --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt @@ -0,0 +1,183 @@ +/* + * Copyright (c) 2021 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.notifications + +import android.app.Notification +import im.vector.app.test.fakes.FakeNotifiableEventProcessor +import im.vector.app.test.fakes.FakeNotificationDisplayer +import im.vector.app.test.fakes.FakeNotificationFactory +import im.vector.app.test.fakes.FakeVectorPreferences +import io.mockk.mockk +import org.junit.Test + +private const val A_CURRENT_ROOM_ID = "current-room-id" +private const val MY_USER_ID = "my-user-id" +private const val MY_USER_DISPLAY_NAME = "display-name" +private const val MY_USER_AVATAR_URL = "avatar-url" +private const val AN_EVENT_ID = "event-id" +private const val A_ROOM_ID = "room-id" +private const val USE_COMPLETE_NOTIFICATION_FORMAT = true + +private val AN_EVENT_LIST = mutableListOf() +private val A_PROCESSED_EVENTS = ProcessedNotificationEvents(emptyMap(), emptyMap(), emptyMap()) +private val A_SUMMARY_NOTIFICATION = mockk() +private val A_NOTIFICATION = mockk() +private val MESSAGE_META = RoomNotification.Message.Meta( + summaryLine = "ignored", messageCount = 1, latestTimestamp = -1, roomId = A_ROOM_ID, shouldBing = false +) +private val ONE_SHOT_META = OneShotNotification.Append.Meta(key = "ignored", summaryLine = "ignored", isNoisy = false) + +class NotificationRendererTest { + + private val notifiableEventProcessor = FakeNotifiableEventProcessor() + private val notificationDisplayer = FakeNotificationDisplayer() + private val preferences = FakeVectorPreferences().also { + it.givenUseCompleteNotificationFormat(USE_COMPLETE_NOTIFICATION_FORMAT) + } + private val notificationFactory = FakeNotificationFactory() + + private val notificationRenderer = NotificationRenderer( + notifiableEventProcessor = notifiableEventProcessor.instance, + notificationDisplayer = notificationDisplayer.instance, + vectorPreferences = preferences.instance, + notificationFactory = notificationFactory.instance + ) + + @Test + fun `given no notifications when rendering then cancels summary notification`() { + givenNoNotifications() + + renderEventsAsNotifications() + + notificationDisplayer.verifySummaryCancelled() + notificationDisplayer.verifyNoOtherInteractions() + } + + @Test + fun `given a room message group notification is removed when rendering then remove the message notification and update summary`() { + givenNotifications(roomNotifications = listOf(RoomNotification.Removed(A_ROOM_ID))) + + renderEventsAsNotifications() + + notificationDisplayer.verifyInOrder { + cancelNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + } + } + + @Test + fun `given a room message group notification is added when rendering then show the message notification and update summary`() { + givenNotifications(roomNotifications = listOf(RoomNotification.Message( + A_NOTIFICATION, + MESSAGE_META + ))) + + renderEventsAsNotifications() + + notificationDisplayer.verifyInOrder { + showNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID, A_NOTIFICATION) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + } + } + + @Test + fun `given a simple notification is removed when rendering then remove the simple notification and update summary`() { + givenNotifications(simpleNotifications = listOf(OneShotNotification.Removed(AN_EVENT_ID))) + + renderEventsAsNotifications() + + notificationDisplayer.verifyInOrder { + cancelNotificationMessage(tag = AN_EVENT_ID, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + } + } + + @Test + fun `given a simple notification is added when rendering then show the simple notification and update summary`() { + givenNotifications(simpleNotifications = listOf(OneShotNotification.Append( + A_NOTIFICATION, + ONE_SHOT_META.copy(key = AN_EVENT_ID) + ))) + + renderEventsAsNotifications() + + notificationDisplayer.verifyInOrder { + showNotificationMessage(tag = AN_EVENT_ID, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID, A_NOTIFICATION) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + } + } + + @Test + fun `given an invitation notification is removed when rendering then remove the invitation notification and update summary`() { + givenNotifications(invitationNotifications = listOf(OneShotNotification.Removed(A_ROOM_ID))) + + renderEventsAsNotifications() + + notificationDisplayer.verifyInOrder { + cancelNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + } + } + + @Test + fun `given an invitation notification is added when rendering then show the invitation notification and update summary`() { + givenNotifications(simpleNotifications = listOf(OneShotNotification.Append( + A_NOTIFICATION, + ONE_SHOT_META.copy(key = A_ROOM_ID) + ))) + + renderEventsAsNotifications() + + notificationDisplayer.verifyInOrder { + showNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID, A_NOTIFICATION) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + } + } + + private fun renderEventsAsNotifications() { + notificationRenderer.render( + currentRoomId = A_CURRENT_ROOM_ID, + myUserId = MY_USER_ID, + myUserDisplayName = MY_USER_DISPLAY_NAME, + myUserAvatarUrl = MY_USER_AVATAR_URL, + eventList = AN_EVENT_LIST + ) + } + + private fun givenNoNotifications() { + givenNotifications(emptyList(), emptyList(), emptyList(), USE_COMPLETE_NOTIFICATION_FORMAT, A_SUMMARY_NOTIFICATION) + } + + private fun givenNotifications(roomNotifications: List = emptyList(), + invitationNotifications: List = emptyList(), + simpleNotifications: List = emptyList(), + useCompleteNotificationFormat: Boolean = USE_COMPLETE_NOTIFICATION_FORMAT, + summaryNotification: Notification = A_SUMMARY_NOTIFICATION) { + notifiableEventProcessor.givenProcessedEventsFor(AN_EVENT_LIST, A_CURRENT_ROOM_ID, A_PROCESSED_EVENTS) + notificationFactory.givenNotificationsFor( + processedEvents = A_PROCESSED_EVENTS, + myUserId = MY_USER_ID, + myUserDisplayName = MY_USER_DISPLAY_NAME, + myUserAvatarUrl = MY_USER_AVATAR_URL, + useCompleteNotificationFormat = useCompleteNotificationFormat, + roomNotifications = roomNotifications, + invitationNotifications = invitationNotifications, + simpleNotifications = simpleNotifications, + summaryNotification = summaryNotification + ) + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt new file mode 100644 index 00000000000..93f5e405248 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt @@ -0,0 +1,32 @@ +/* + * 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.fakes + +import im.vector.app.features.notifications.NotifiableEvent +import im.vector.app.features.notifications.NotifiableEventProcessor +import im.vector.app.features.notifications.ProcessedNotificationEvents +import io.mockk.every +import io.mockk.mockk + +class FakeNotifiableEventProcessor { + + val instance = mockk() + + fun givenProcessedEventsFor(events: MutableList, currentRoomId: String?, processedEvents: ProcessedNotificationEvents) { + every { instance.modifyAndProcess(events, currentRoomId) } returns processedEvents + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationDisplayer.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationDisplayer.kt new file mode 100644 index 00000000000..2856b0f49c9 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationDisplayer.kt @@ -0,0 +1,42 @@ +/* + * 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.fakes + +import im.vector.app.features.notifications.NotificationDisplayer +import im.vector.app.features.notifications.NotificationDrawerManager +import io.mockk.confirmVerified +import io.mockk.mockk +import io.mockk.verify +import io.mockk.verifyOrder + +class FakeNotificationDisplayer { + + val instance = mockk(relaxed = true) + + fun verifySummaryCancelled() { + verify { instance.cancelNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID) } + } + + fun verifyNoOtherInteractions() { + confirmVerified(instance) + } + + fun verifyInOrder(verifyBlock: NotificationDisplayer.() -> Unit) { + verifyOrder { verifyBlock(instance) } + verifyNoOtherInteractions() + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt new file mode 100644 index 00000000000..921999bd932 --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt @@ -0,0 +1,56 @@ +/* + * 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.fakes + +import android.app.Notification +import im.vector.app.features.notifications.NotificationFactory +import im.vector.app.features.notifications.OneShotNotification +import im.vector.app.features.notifications.ProcessedNotificationEvents +import im.vector.app.features.notifications.RoomNotification +import io.mockk.every +import io.mockk.mockk + +class FakeNotificationFactory { + + val instance = mockk() + + fun givenNotificationsFor(processedEvents: ProcessedNotificationEvents, + myUserId: String, + myUserDisplayName: String, + myUserAvatarUrl: String?, + useCompleteNotificationFormat: Boolean, + roomNotifications: List, + invitationNotifications: List, + simpleNotifications: List, + summaryNotification: Notification) { + with(instance) { + every { processedEvents.roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) } returns roomNotifications + every { processedEvents.invitationEvents.toNotifications(myUserId) } returns invitationNotifications + every { processedEvents.simpleEvents.toNotifications(myUserId) } returns simpleNotifications + + every { + createSummaryNotification( + roomNotifications, + invitationNotifications, + simpleNotifications, + useCompleteNotificationFormat + ) + } returns summaryNotification + + } + } +} diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt new file mode 100644 index 00000000000..eb8f9ac413b --- /dev/null +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeVectorPreferences.kt @@ -0,0 +1,30 @@ +/* + * 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.fakes + +import im.vector.app.features.settings.VectorPreferences +import io.mockk.every +import io.mockk.mockk + +class FakeVectorPreferences { + + val instance = mockk() + + fun givenUseCompleteNotificationFormat(value: Boolean) { + every { instance.useCompleteNotificationFormat() } returns value + } +} From b3bb42f0a0e4ebc883e0eb24cb64e1d5bdf9a0ee Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 7 Oct 2021 08:22:45 +0100 Subject: [PATCH 05/26] lifting settings change to cancel all notifications out of the renderer - the renderer's responsibility it handling events --- .../NotificationDrawerManager.kt | 376 +----------------- .../notifications/NotificationFactory.kt | 4 +- .../notifications/NotificationRenderer.kt | 33 +- .../notifications/RoomGroupMessageCreator.kt | 39 +- .../notifications/NotificationRendererTest.kt | 5 +- 5 files changed, 62 insertions(+), 395 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 843b7208fde..a3a3e898bac 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 @@ -16,26 +16,15 @@ package im.vector.app.features.notifications import android.content.Context -import android.graphics.Bitmap -import android.os.Build import android.os.Handler import android.os.HandlerThread import androidx.annotation.WorkerThread -import androidx.core.app.NotificationCompat -import androidx.core.app.Person -import androidx.core.content.pm.ShortcutInfoCompat -import androidx.core.content.pm.ShortcutManagerCompat -import androidx.core.graphics.drawable.IconCompat import im.vector.app.ActiveSessionDataSource import im.vector.app.BuildConfig import im.vector.app.R -import im.vector.app.core.resources.StringProvider import im.vector.app.core.utils.FirstThrottler import im.vector.app.features.displayname.getBestName -import im.vector.app.features.home.room.detail.RoomDetailActivity -import im.vector.app.features.invite.AutoAcceptInvites import im.vector.app.features.settings.VectorPreferences -import me.gujun.android.span.span import org.matrix.android.sdk.api.session.Session import org.matrix.android.sdk.api.session.content.ContentUrlResolver import org.matrix.android.sdk.api.util.toMatrixItem @@ -54,12 +43,8 @@ import javax.inject.Singleton class NotificationDrawerManager @Inject constructor(private val context: Context, private val notificationUtils: NotificationUtils, private val vectorPreferences: VectorPreferences, - private val stringProvider: StringProvider, private val activeSessionDataSource: ActiveSessionDataSource, - private val iconLoader: IconLoader, - private val bitmapLoader: BitmapLoader, - private val outdatedDetector: OutdatedEventDetector?, - private val autoAcceptInvites: AutoAcceptInvites) { + private val notificationRenderer: NotificationRenderer) { private val handlerThread: HandlerThread = HandlerThread("NotificationDrawerManager", Thread.MIN_PRIORITY) private var backgroundHandler: Handler @@ -69,13 +54,8 @@ class NotificationDrawerManager @Inject constructor(private val context: Context backgroundHandler = Handler(handlerThread.looper) } - // The first time the notification drawer is refreshed, we force re-render of all notifications - private var firstTime = true - private val eventList = loadEventInfo() - private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size) - private var currentRoomId: String? = null // TODO Multi-session: this will have to be improved @@ -258,359 +238,17 @@ class NotificationDrawerManager @Inject constructor(private val context: Context val myUserAvatarUrl = session.contentUrlResolver().resolveThumbnail(user?.avatarUrl, avatarSize, avatarSize, ContentUrlResolver.ThumbnailMethod.SCALE) synchronized(eventList) { - val useSplitNotifications = false - if (useSplitNotifications) { - // TODO - } else { - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER ") - // TMP code - var hasNewEvent = false - var summaryIsNoisy = false - val summaryInboxStyle = NotificationCompat.InboxStyle() - - // group events by room to create a single MessagingStyle notif - val roomIdToEventMap: MutableMap> = LinkedHashMap() - val simpleEvents: MutableList = ArrayList() - val invitationEvents: MutableList = ArrayList() - - val eventIterator = eventList.listIterator() - while (eventIterator.hasNext()) { - 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) - } - } - is InviteNotifiableEvent -> { - if (autoAcceptInvites.hideInvites) { - // Forget this event - eventIterator.remove() - } else { - invitationEvents.add(event) - } - } - is SimpleNotifiableEvent -> simpleEvents.add(event) - else -> Timber.w("Type not handled") - } + val newSettings = vectorPreferences.useCompleteNotificationFormat() + if (newSettings != useCompleteNotificationFormat) { + // Settings has changed, remove all current notifications + notificationUtils.cancelAllNotifications() + useCompleteNotificationFormat = newSettings } - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER ${roomIdToEventMap.size} room groups") - - var globalLastMessageTimestamp = 0L - - val newSettings = vectorPreferences.useCompleteNotificationFormat() - if (newSettings != useCompleteNotificationFormat) { - // Settings has changed, remove all current notifications - notificationUtils.cancelAllNotifications() - useCompleteNotificationFormat = newSettings - } - - var simpleNotificationRoomCounter = 0 - var simpleNotificationMessageCounter = 0 - - // events have been grouped by roomId - for ((roomId, events) in roomIdToEventMap) { - // Build the notification for the room - if (events.isEmpty() || events.all { it.isRedacted }) { - // Just clear this notification - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER $roomId has no more events") - notificationUtils.cancelNotificationMessage(roomId, ROOM_MESSAGES_NOTIFICATION_ID) - continue - } - - simpleNotificationRoomCounter++ - val roomName = events[0].roomName ?: events[0].senderName ?: "" - - val roomEventGroupInfo = RoomEventGroupInfo( - roomId = roomId, - isDirect = events[0].roomIsDirect, - roomDisplayName = roomName) - - val style = NotificationCompat.MessagingStyle(Person.Builder() - .setName(myUserDisplayName) - .setIcon(iconLoader.getUserIcon(myUserAvatarUrl)) - .setKey(events[0].matrixID) - .build()) - - style.isGroupConversation = !roomEventGroupInfo.isDirect - - if (!roomEventGroupInfo.isDirect) { - style.conversationTitle = roomEventGroupInfo.roomDisplayName - } - - val largeBitmap = getRoomBitmap(events) - - for (event in events) { - // if all events in this room have already been displayed there is no need to update it - if (!event.hasBeenDisplayed && !event.isRedacted) { - roomEventGroupInfo.shouldBing = roomEventGroupInfo.shouldBing || event.noisy - roomEventGroupInfo.customSound = event.soundName - } - roomEventGroupInfo.hasNewEvent = roomEventGroupInfo.hasNewEvent || !event.hasBeenDisplayed - - val senderPerson = if (event.outGoingMessage) { - null - } else { - Person.Builder() - .setName(event.senderName) - .setIcon(iconLoader.getUserIcon(event.senderAvatarPath)) - .setKey(event.senderId) - .build() - } - - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { - val openRoomIntent = RoomDetailActivity.shortcutIntent(context, roomId) - - val shortcut = ShortcutInfoCompat.Builder(context, roomId) - .setLongLived(true) - .setIntent(openRoomIntent) - .setShortLabel(roomName) - .setIcon(largeBitmap?.let { IconCompat.createWithAdaptiveBitmap(it) } ?: iconLoader.getUserIcon(event.senderAvatarPath)) - .build() - - ShortcutManagerCompat.pushDynamicShortcut(context, shortcut) - } - - if (event.outGoingMessage && event.outGoingMessageFailed) { - style.addMessage(stringProvider.getString(R.string.notification_inline_reply_failed), event.timestamp, senderPerson) - roomEventGroupInfo.hasSmartReplyError = true - } else { - if (!event.isRedacted) { - simpleNotificationMessageCounter++ - style.addMessage(event.body, event.timestamp, senderPerson) - } - } - event.hasBeenDisplayed = true // we can consider it as displayed - - // It is possible that this event was previously shown as an 'anonymous' simple notif. - // And now it will be merged in a single MessageStyle notif, so we can clean to be sure - notificationUtils.cancelNotificationMessage(event.eventId, ROOM_EVENT_NOTIFICATION_ID) - } - - try { - if (events.size == 1) { - val event = events[0] - if (roomEventGroupInfo.isDirect) { - val line = span { - span { - textStyle = "bold" - +String.format("%s: ", event.senderName) - } - +(event.description) - } - summaryInboxStyle.addLine(line) - } else { - val line = span { - span { - textStyle = "bold" - +String.format("%s: %s ", roomName, event.senderName) - } - +(event.description) - } - summaryInboxStyle.addLine(line) - } - } else { - val summaryLine = stringProvider.getQuantityString( - R.plurals.notification_compat_summary_line_for_room, events.size, roomName, events.size) - summaryInboxStyle.addLine(summaryLine) - } - } catch (e: Throwable) { - // String not found or bad format - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER failed to resolve string") - summaryInboxStyle.addLine(roomName) - } - - if (firstTime || roomEventGroupInfo.hasNewEvent) { - // Should update displayed notification - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER $roomId need refresh") - val lastMessageTimestamp = events.last().timestamp - - if (globalLastMessageTimestamp < lastMessageTimestamp) { - globalLastMessageTimestamp = lastMessageTimestamp - } - - val tickerText = if (roomEventGroupInfo.isDirect) { - stringProvider.getString(R.string.notification_ticker_text_dm, events.last().senderName, events.last().description) - } else { - stringProvider.getString(R.string.notification_ticker_text_group, roomName, events.last().senderName, events.last().description) - } - - if (useCompleteNotificationFormat) { - val notification = notificationUtils.buildMessagesListNotification( - style, - roomEventGroupInfo, - largeBitmap, - lastMessageTimestamp, - myUserDisplayName, - tickerText) - - // is there an id for this room? - notificationUtils.showNotificationMessage(roomId, ROOM_MESSAGES_NOTIFICATION_ID, notification) - } - - hasNewEvent = true - summaryIsNoisy = summaryIsNoisy || roomEventGroupInfo.shouldBing - } else { - Timber.v("%%%%%%%% REFRESH NOTIFICATION DRAWER $roomId is up to date") - } - } - - // Handle invitation events - for (event in invitationEvents) { - // We build a invitation notification - if (firstTime || !event.hasBeenDisplayed) { - if (useCompleteNotificationFormat) { - 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 notification - if (firstTime || !event.hasBeenDisplayed) { - if (useCompleteNotificationFormat) { - 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 - summaryIsNoisy = summaryIsNoisy || event.noisy - summaryInboxStyle.addLine(event.description) - } - } - - // ======== Build summary notification ========= - // On Android 7.0 (API level 24) and higher, the system automatically builds a summary for - // your group using snippets of text from each notification. The user can expand this - // notification to see each separate notification. - // To support older versions, which cannot show a nested group of notifications, - // you must create an extra notification that acts as the summary. - // This appears as the only notification and the system hides all the others. - // So this summary should include a snippet from all the other notifications, - // which the user can tap to open your app. - // The behavior of the group summary may vary on some device types such as wearables. - // To ensure the best experience on all devices and versions, always include a group summary when you create a group - // https://developer.android.com/training/notify-user/group - - if (eventList.isEmpty() || eventList.all { it.isRedacted }) { - notificationUtils.cancelNotificationMessage(null, SUMMARY_NOTIFICATION_ID) - } else if (hasNewEvent) { - // FIXME roomIdToEventMap.size is not correct, this is the number of rooms - val nbEvents = roomIdToEventMap.size + simpleEvents.size - val sumTitle = stringProvider.getQuantityString(R.plurals.notification_compat_summary_title, nbEvents, nbEvents) - summaryInboxStyle.setBigContentTitle(sumTitle) - // TODO get latest event? - .setSummaryText(stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages, nbEvents, nbEvents)) - - if (useCompleteNotificationFormat) { - val notification = notificationUtils.buildSummaryListNotification( - summaryInboxStyle, - sumTitle, - noisy = hasNewEvent && summaryIsNoisy, - lastMessageTimestamp = globalLastMessageTimestamp) - - notificationUtils.showNotificationMessage(null, SUMMARY_NOTIFICATION_ID, notification) - } else { - // Add the simple events as message (?) - simpleNotificationMessageCounter += simpleEvents.size - val numberOfInvitations = invitationEvents.size - - val privacyTitle = if (numberOfInvitations > 0) { - val invitationsStr = stringProvider.getQuantityString(R.plurals.notification_invitations, numberOfInvitations, numberOfInvitations) - if (simpleNotificationMessageCounter > 0) { - // Invitation and message - val messageStr = stringProvider.getQuantityString(R.plurals.room_new_messages_notification, - simpleNotificationMessageCounter, simpleNotificationMessageCounter) - if (simpleNotificationRoomCounter > 1) { - // In several rooms - val roomStr = stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages_in_room_rooms, - simpleNotificationRoomCounter, simpleNotificationRoomCounter) - stringProvider.getString( - R.string.notification_unread_notified_messages_in_room_and_invitation, - messageStr, - roomStr, - invitationsStr - ) - } else { - // In one room - stringProvider.getString( - R.string.notification_unread_notified_messages_and_invitation, - messageStr, - invitationsStr - ) - } - } else { - // Only invitation - invitationsStr - } - } else { - // No invitation, only messages - val messageStr = stringProvider.getQuantityString(R.plurals.room_new_messages_notification, - simpleNotificationMessageCounter, simpleNotificationMessageCounter) - if (simpleNotificationRoomCounter > 1) { - // In several rooms - val roomStr = stringProvider.getQuantityString(R.plurals.notification_unread_notified_messages_in_room_rooms, - simpleNotificationRoomCounter, simpleNotificationRoomCounter) - stringProvider.getString(R.string.notification_unread_notified_messages_in_room, messageStr, roomStr) - } else { - // In one room - messageStr - } - } - val notification = notificationUtils.buildSummaryListNotification( - style = null, - compatSummary = privacyTitle, - noisy = hasNewEvent && summaryIsNoisy, - lastMessageTimestamp = globalLastMessageTimestamp) - - notificationUtils.showNotificationMessage(null, SUMMARY_NOTIFICATION_ID, notification) - } - - if (hasNewEvent && summaryIsNoisy) { - try { - // turn the screen on for 3 seconds - /* - TODO - if (Matrix.getInstance(VectorApp.getInstance())!!.pushManager.isScreenTurnedOn) { - val pm = VectorApp.getInstance().getSystemService()!! - val wl = pm.newWakeLock(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON or PowerManager.ACQUIRE_CAUSES_WAKEUP, - NotificationDrawerManager::class.java.name) - wl.acquire(3000) - wl.release() - } - */ - } catch (e: Throwable) { - Timber.e(e, "## Failed to turn screen on") - } - } - } - // notice that we can get bit out of sync with actual display but not a big issue - firstTime = false - } + notificationRenderer.render(currentRoomId, session.myUserId, myUserDisplayName, myUserAvatarUrl, useCompleteNotificationFormat, eventList) } } - private fun getRoomBitmap(events: List): Bitmap? { - if (events.isEmpty()) return null - - // Use the last event (most recent?) - val roomAvatarPath = events.last().roomAvatarPath ?: events.last().senderAvatarPath - - return bitmapLoader.getRoomBitmap(roomAvatarPath) - } - fun shouldIgnoreMessageEventInRoom(roomId: String?): Boolean { return currentRoomId != null && roomId == currentRoomId } diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index 55e9f7352d3..ec8e372c4e8 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -17,6 +17,8 @@ package im.vector.app.features.notifications import android.app.Notification +import androidx.core.content.pm.ShortcutInfoCompat +import androidx.core.content.pm.ShortcutManagerCompat import javax.inject.Inject class NotificationFactory @Inject constructor( @@ -83,7 +85,7 @@ private fun List.mapToMeta() = filterIsInstance) { + fun render(currentRoomId: String?, + myUserId: String, + myUserDisplayName: String, + myUserAvatarUrl: String?, + useCompleteNotificationFormat: Boolean, + eventList: MutableList) { Timber.v("refreshNotificationDrawerBg()") - val newSettings = vectorPreferences.useCompleteNotificationFormat() - if (newSettings != useCompleteNotificationFormat) { - // Settings has changed, remove all current notifications - notificationDisplayer.cancelAllNotifications() - useCompleteNotificationFormat = newSettings - } - val notificationEvents = notifiableEventProcessor.modifyAndProcess(eventList, currentRoomId) if (lastKnownEventList == notificationEvents.hashCode()) { Timber.d("Skipping notification update due to event list not changing") } else { - processEvents(notificationEvents, myUserId, myUserDisplayName, myUserAvatarUrl) + processEvents(notificationEvents, myUserId, myUserDisplayName, myUserAvatarUrl, useCompleteNotificationFormat) lastKnownEventList = notificationEvents.hashCode() } } - private fun processEvents(notificationEvents: ProcessedNotificationEvents, myUserId: String, myUserDisplayName: String, myUserAvatarUrl: String?) { + private fun processEvents(notificationEvents: ProcessedNotificationEvents, myUserId: String, myUserDisplayName: String, myUserAvatarUrl: String?, useCompleteNotificationFormat: Boolean) { val (roomEvents, simpleEvents, invitationEvents) = notificationEvents with(notificationFactory) { val roomNotifications = roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) @@ -70,6 +72,9 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces is RoomNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.roomId, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID) is RoomNotification.Message -> if (useCompleteNotificationFormat) { Timber.d("Updating room messages notification ${wrapper.meta.roomId}") + wrapper.shortcutInfo?.let { + ShortcutManagerCompat.pushDynamicShortcut(appContext, it) + } notificationDisplayer.showNotificationMessage(wrapper.meta.roomId, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID, wrapper.notification) } } 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 786ce40046b..56e3274515d 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 @@ -16,11 +16,17 @@ package im.vector.app.features.notifications +import android.content.Context import android.graphics.Bitmap +import android.os.Build import androidx.core.app.NotificationCompat import androidx.core.app.Person +import androidx.core.content.pm.ShortcutInfoCompat +import androidx.core.content.pm.ShortcutManagerCompat +import androidx.core.graphics.drawable.IconCompat import im.vector.app.R import im.vector.app.core.resources.StringProvider +import im.vector.app.features.home.room.detail.RoomDetailActivity import me.gujun.android.span.Span import me.gujun.android.span.span import timber.log.Timber @@ -30,7 +36,8 @@ class RoomGroupMessageCreator @Inject constructor( private val iconLoader: IconLoader, private val bitmapLoader: BitmapLoader, private val stringProvider: StringProvider, - private val notificationUtils: NotificationUtils + private val notificationUtils: NotificationUtils, + private val appContext: Context ) { fun createRoomMessage(events: List, roomId: String, userDisplayName: String, userAvatarUrl: String?): RoomNotification.Message { @@ -54,6 +61,19 @@ class RoomGroupMessageCreator @Inject constructor( stringProvider.getString(R.string.notification_ticker_text_dm, events.last().senderName, events.last().description) } + val largeBitmap = getRoomBitmap(events) + val shortcutInfo = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { + val openRoomIntent = RoomDetailActivity.shortcutIntent(appContext, roomId) + ShortcutInfoCompat.Builder(appContext, roomId) + .setLongLived(true) + .setIntent(openRoomIntent) + .setShortLabel(roomName) + .setIcon(largeBitmap?.let { IconCompat.createWithAdaptiveBitmap(it) } ?: iconLoader.getUserIcon(events.last().senderAvatarPath)) + .build() + } else { + null + } + val lastMessageTimestamp = events.last().timestamp val smartReplyErrors = events.filter { it.isSmartReplyError() } val messageCount = (events.size - smartReplyErrors.size) @@ -72,22 +92,27 @@ class RoomGroupMessageCreator @Inject constructor( it.shouldBing = meta.shouldBing it.customSound = events.last().soundName }, - largeIcon = getRoomBitmap(events), + largeIcon = largeBitmap, lastMessageTimestamp, userDisplayName, tickerText ), + shortcutInfo, meta ) } private fun NotificationCompat.MessagingStyle.addMessagesFromEvents(events: List) { events.forEach { event -> - val senderPerson = Person.Builder() - .setName(event.senderName) - .setIcon(iconLoader.getUserIcon(event.senderAvatarPath)) - .setKey(event.senderId) - .build() + val senderPerson = if (event.outGoingMessage) { + null + } else { + Person.Builder() + .setName(event.senderName) + .setIcon(iconLoader.getUserIcon(event.senderAvatarPath)) + .setKey(event.senderId) + .build() + } when { event.isSmartReplyError() -> addMessage(stringProvider.getString(R.string.notification_inline_reply_failed), event.timestamp, senderPerson) else -> addMessage(event.body, event.timestamp, senderPerson) diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt index a07dd61368f..74a11b6d0df 100644 --- a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt @@ -45,15 +45,11 @@ class NotificationRendererTest { private val notifiableEventProcessor = FakeNotifiableEventProcessor() private val notificationDisplayer = FakeNotificationDisplayer() - private val preferences = FakeVectorPreferences().also { - it.givenUseCompleteNotificationFormat(USE_COMPLETE_NOTIFICATION_FORMAT) - } private val notificationFactory = FakeNotificationFactory() private val notificationRenderer = NotificationRenderer( notifiableEventProcessor = notifiableEventProcessor.instance, notificationDisplayer = notificationDisplayer.instance, - vectorPreferences = preferences.instance, notificationFactory = notificationFactory.instance ) @@ -154,6 +150,7 @@ class NotificationRendererTest { myUserId = MY_USER_ID, myUserDisplayName = MY_USER_DISPLAY_NAME, myUserAvatarUrl = MY_USER_AVATAR_URL, + useCompleteNotificationFormat = USE_COMPLETE_NOTIFICATION_FORMAT, eventList = AN_EVENT_LIST ) } From 55480e09b66704191ceec3cd12ce09054d7580d3 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 7 Oct 2021 14:18:27 +0100 Subject: [PATCH 06/26] removing no longer needed hasBeenDisplayed state, the eventList is our source of truth - when events have finished being displayed they should be removed from the eventList via notification delete actions --- .../features/notifications/InviteNotifiableEvent.kt | 5 +---- .../app/features/notifications/NotifiableEvent.kt | 1 - .../features/notifications/NotifiableMessageEvent.kt | 2 -- .../notifications/NotificationDrawerManager.kt | 7 +++---- .../app/features/notifications/NotificationFactory.kt | 2 +- .../features/notifications/SimpleNotifiableEvent.kt | 5 +---- .../notifications/NotifiableEventProcessorTest.kt | 10 +++++++--- .../features/notifications/NotificationFactoryTest.kt | 2 +- 8 files changed, 14 insertions(+), 20 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/InviteNotifiableEvent.kt b/vector/src/main/java/im/vector/app/features/notifications/InviteNotifiableEvent.kt index 743b3587a8a..832f97bc4e9 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/InviteNotifiableEvent.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/InviteNotifiableEvent.kt @@ -29,7 +29,4 @@ data class InviteNotifiableEvent( val timestamp: Long, val soundName: String?, override val isRedacted: Boolean = false -) : NotifiableEvent { - - override var hasBeenDisplayed = false -} +) : NotifiableEvent diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEvent.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEvent.kt index 2f79da67955..52d8119cbbd 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEvent.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEvent.kt @@ -23,7 +23,6 @@ import java.io.Serializable sealed interface NotifiableEvent : Serializable { val eventId: String val editedEventId: String? - var hasBeenDisplayed: Boolean // Used to know if event should be replaced with the one coming from eventstream val canBeReplaced: Boolean 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 4a2152c4179..161c9f74a69 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 @@ -39,8 +39,6 @@ data class NotifiableMessageEvent( override val isRedacted: Boolean = false ) : NotifiableEvent { - override var hasBeenDisplayed: Boolean = false - val type: String = EventType.MESSAGE val description: String = body ?: "" val title: String = senderName ?: "" 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 a3a3e898bac..4be1f6ee6e0 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 @@ -101,7 +101,6 @@ 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 - notifiableEvent.hasBeenDisplayed = false eventList.remove(existing) eventList.add(notifiableEvent) } else { @@ -144,9 +143,9 @@ class NotificationDrawerManager @Inject constructor(private val context: Context synchronized(eventList) { eventList.replace(eventId) { when (it) { - is InviteNotifiableEvent -> it.copy(isRedacted = true).apply { hasBeenDisplayed = false } - is NotifiableMessageEvent -> it.copy(isRedacted = true).apply { hasBeenDisplayed = false } - is SimpleNotifiableEvent -> it.copy(isRedacted = true).apply { hasBeenDisplayed = false } + is InviteNotifiableEvent -> it.copy(isRedacted = true) + is NotifiableMessageEvent -> it.copy(isRedacted = true) + is SimpleNotifiableEvent -> it.copy(isRedacted = true) } } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index ec8e372c4e8..f47e82e845f 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -38,7 +38,7 @@ class NotificationFactory @Inject constructor( private fun List.hasNoEventsToDisplay() = isEmpty() || all { it.canNotBeDisplayed() } - private fun NotifiableMessageEvent.canNotBeDisplayed() = hasBeenDisplayed || isRedacted + private fun NotifiableMessageEvent.canNotBeDisplayed() = isRedacted fun Map.toNotifications(myUserId: String): List { return this.map { (roomId, event) -> diff --git a/vector/src/main/java/im/vector/app/features/notifications/SimpleNotifiableEvent.kt b/vector/src/main/java/im/vector/app/features/notifications/SimpleNotifiableEvent.kt index 940d8a37703..8c723722041 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/SimpleNotifiableEvent.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/SimpleNotifiableEvent.kt @@ -27,7 +27,4 @@ data class SimpleNotifiableEvent( val soundName: String?, override var canBeReplaced: Boolean, override val isRedacted: Boolean = false -) : NotifiableEvent { - - override var hasBeenDisplayed: Boolean = false -} +) : NotifiableEvent 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 a5bb9978dd9..6f47e71500b 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 @@ -156,7 +156,8 @@ fun aSimpleNotifiableEvent(eventId: String) = SimpleNotifiableEvent( type = null, timestamp = 0, soundName = null, - isPushGatewayEvent = false + canBeReplaced = false, + isRedacted = false ) fun anInviteNotifiableEvent(roomId: String) = InviteNotifiableEvent( @@ -170,7 +171,8 @@ fun anInviteNotifiableEvent(roomId: String) = InviteNotifiableEvent( type = null, timestamp = 0, soundName = null, - isPushGatewayEvent = false + canBeReplaced = false, + isRedacted = false ) fun aNotifiableMessageEvent(eventId: String, roomId: String) = NotifiableMessageEvent( @@ -183,5 +185,7 @@ fun aNotifiableMessageEvent(eventId: String, roomId: String) = NotifiableMessage body = "message-body", roomId = roomId, roomName = "room-name", - roomIsDirect = false + 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 c08be9e8c7d..f8e6813d9b1 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 @@ -123,7 +123,7 @@ class NotificationFactoryTest { @Test fun `given a room with only redacted events when mapping to notification then is Empty`() = testWith(notificationFactory) { - val redactedRoom = mapOf(A_ROOM_ID to listOf(A_MESSAGE_EVENT.copy().apply { isRedacted = true })) + val redactedRoom = mapOf(A_ROOM_ID to listOf(A_MESSAGE_EVENT.copy(isRedacted = true))) val result = redactedRoom.toNotifications(MY_USER_ID, MY_AVATAR_URL) From 066fd49c5a78b65dc7b2be04a3a3f5a96c7009f3 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 7 Oct 2021 16:03:52 +0100 Subject: [PATCH 07/26] handling creating the summary when notification events are filtered to empty due to only containing removals --- .../notifications/NotificationFactory.kt | 42 +++++++--- .../notifications/NotificationRenderer.kt | 78 ++++++++++--------- .../SummaryGroupMessageCreator.kt | 8 +- .../notifications/NotificationFactoryTest.kt | 6 +- .../notifications/NotificationRendererTest.kt | 21 ++--- .../app/test/fakes/FakeNotificationFactory.kt | 4 +- 6 files changed, 94 insertions(+), 65 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index f47e82e845f..8189977ba81 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -46,7 +46,12 @@ class NotificationFactory @Inject constructor( null -> OneShotNotification.Removed(key = roomId) else -> OneShotNotification.Append( notificationUtils.buildRoomInvitationNotification(event, myUserId), - OneShotNotification.Append.Meta(key = roomId, summaryLine = event.description, isNoisy = event.noisy) + OneShotNotification.Append.Meta( + key = roomId, + summaryLine = event.description, + isNoisy = event.noisy, + timestamp = event.timestamp + ) ) } } @@ -59,7 +64,12 @@ class NotificationFactory @Inject constructor( null -> OneShotNotification.Removed(key = eventId) else -> OneShotNotification.Append( notificationUtils.buildSimpleEventNotification(event, myUserId), - OneShotNotification.Append.Meta(key = eventId, summaryLine = event.description, isNoisy = event.noisy) + OneShotNotification.Append.Meta( + key = eventId, + summaryLine = event.description, + isNoisy = event.noisy, + timestamp = event.timestamp + ) ) } } @@ -68,13 +78,19 @@ class NotificationFactory @Inject constructor( fun createSummaryNotification(roomNotifications: List, invitationNotifications: List, simpleNotifications: List, - useCompleteNotificationFormat: Boolean): Notification { - return summaryGroupMessageCreator.createSummaryNotification( - roomNotifications = roomNotifications.mapToMeta(), - invitationNotifications = invitationNotifications.mapToMeta(), - simpleNotifications = simpleNotifications.mapToMeta(), - useCompleteNotificationFormat = useCompleteNotificationFormat - ) + useCompleteNotificationFormat: Boolean): SummaryNotification { + val roomMeta = roomNotifications.mapToMeta() + val invitationMeta = invitationNotifications.mapToMeta() + val simpleMeta = simpleNotifications.mapToMeta() + return when { + roomMeta.isEmpty() && invitationMeta.isEmpty() && simpleMeta.isEmpty() -> SummaryNotification.Removed + else -> SummaryNotification.Update(summaryGroupMessageCreator.createSummaryNotification( + roomNotifications = roomMeta, + invitationNotifications = invitationMeta, + simpleNotifications = simpleMeta, + useCompleteNotificationFormat = useCompleteNotificationFormat + )) + } } } @@ -102,7 +118,13 @@ sealed interface OneShotNotification { data class Meta( val key: String, val summaryLine: CharSequence, - val isNoisy: Boolean + val isNoisy: Boolean, + val timestamp: Long, ) } } + +sealed interface SummaryNotification { + object Removed : SummaryNotification + data class Update(val notification: Notification) : SummaryNotification +} diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt index 005cf04f070..749e3c61b6b 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt @@ -16,12 +16,8 @@ package im.vector.app.features.notifications import android.content.Context -import android.os.Build import androidx.annotation.WorkerThread -import androidx.core.content.pm.ShortcutInfoCompat import androidx.core.content.pm.ShortcutManagerCompat -import androidx.core.graphics.drawable.IconCompat -import im.vector.app.features.home.room.detail.RoomDetailActivity import timber.log.Timber import javax.inject.Inject import javax.inject.Singleton @@ -41,7 +37,7 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces myUserAvatarUrl: String?, useCompleteNotificationFormat: Boolean, eventList: MutableList) { - Timber.v("refreshNotificationDrawerBg()") + Timber.v("Render notification events - count: ${eventList.size}") val notificationEvents = notifiableEventProcessor.modifyAndProcess(eventList, currentRoomId) if (lastKnownEventList == notificationEvents.hashCode()) { Timber.d("Skipping notification update due to event list not changing") @@ -57,49 +53,55 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces val roomNotifications = roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) val invitationNotifications = invitationEvents.toNotifications(myUserId) val simpleNotifications = simpleEvents.toNotifications(myUserId) + val summaryNotification = createSummaryNotification( + roomNotifications = roomNotifications, + invitationNotifications = invitationNotifications, + simpleNotifications = simpleNotifications, + useCompleteNotificationFormat = useCompleteNotificationFormat + ) - if (roomNotifications.isEmpty() && invitationNotifications.isEmpty() && simpleNotifications.isEmpty()) { - notificationDisplayer.cancelNotificationMessage(null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID) - } else { - val summaryNotification = createSummaryNotification( - roomNotifications = roomNotifications, - invitationNotifications = invitationNotifications, - simpleNotifications = simpleNotifications, - useCompleteNotificationFormat = useCompleteNotificationFormat - ) - roomNotifications.forEach { wrapper -> - when (wrapper) { - is RoomNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.roomId, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID) - is RoomNotification.Message -> if (useCompleteNotificationFormat) { - Timber.d("Updating room messages notification ${wrapper.meta.roomId}") - wrapper.shortcutInfo?.let { - ShortcutManagerCompat.pushDynamicShortcut(appContext, it) - } - notificationDisplayer.showNotificationMessage(wrapper.meta.roomId, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID, wrapper.notification) + roomNotifications.forEach { wrapper -> + when (wrapper) { + is RoomNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.roomId, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID) + is RoomNotification.Message -> if (useCompleteNotificationFormat) { + Timber.d("Updating room messages notification ${wrapper.meta.roomId}") + wrapper.shortcutInfo?.let { + ShortcutManagerCompat.pushDynamicShortcut(appContext, it) } + notificationDisplayer.showNotificationMessage(wrapper.meta.roomId, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID, wrapper.notification) } } + } - invitationNotifications.forEach { wrapper -> - when (wrapper) { - is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID) - is OneShotNotification.Append -> if (useCompleteNotificationFormat) { - Timber.d("Updating invitation notification ${wrapper.meta.key}") - notificationDisplayer.showNotificationMessage(wrapper.meta.key, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID, wrapper.notification) - } + invitationNotifications.forEach { wrapper -> + when (wrapper) { + is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID) + is OneShotNotification.Append -> if (useCompleteNotificationFormat) { + Timber.d("Updating invitation notification ${wrapper.meta.key}") + notificationDisplayer.showNotificationMessage(wrapper.meta.key, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID, wrapper.notification) } } + } - simpleNotifications.forEach { wrapper -> - when (wrapper) { - is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID) - is OneShotNotification.Append -> if (useCompleteNotificationFormat) { - Timber.d("Updating simple notification ${wrapper.meta.key}") - notificationDisplayer.showNotificationMessage(wrapper.meta.key, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID, wrapper.notification) - } + simpleNotifications.forEach { wrapper -> + when (wrapper) { + is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID) + is OneShotNotification.Append -> if (useCompleteNotificationFormat) { + Timber.d("Updating simple notification ${wrapper.meta.key}") + notificationDisplayer.showNotificationMessage(wrapper.meta.key, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID, wrapper.notification) } } - notificationDisplayer.showNotificationMessage(null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, summaryNotification) + } + + when (summaryNotification) { + SummaryNotification.Removed -> { + Timber.d("Removing summary notification") + notificationDisplayer.cancelNotificationMessage(null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID) + } + is SummaryNotification.Update -> { + Timber.d("Updating summary notification") + notificationDisplayer.showNotificationMessage(null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, summaryNotification.notification) + } } } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt b/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt index dc9ff92aa69..821f24b4363 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt @@ -57,7 +57,9 @@ class SummaryGroupMessageCreator @Inject constructor( val messageCount = roomNotifications.fold(initial = 0) { acc, current -> acc + current.messageCount } - val lastMessageTimestamp1 = roomNotifications.last().latestTimestamp + val lastMessageTimestamp = roomNotifications.lastOrNull()?.latestTimestamp + ?: invitationNotifications.lastOrNull()?.timestamp + ?: simpleNotifications.last().timestamp // FIXME roomIdToEventMap.size is not correct, this is the number of rooms val nbEvents = roomNotifications.size + simpleNotifications.size @@ -71,12 +73,12 @@ class SummaryGroupMessageCreator @Inject constructor( summaryInboxStyle, sumTitle, noisy = summaryIsNoisy, - lastMessageTimestamp = lastMessageTimestamp1 + lastMessageTimestamp = lastMessageTimestamp ) } else { processSimpleGroupSummary(summaryIsNoisy, messageCount, simpleNotifications.size, invitationNotifications.size, - roomNotifications.size, lastMessageTimestamp1) + roomNotifications.size, lastMessageTimestamp) } } 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 f8e6813d9b1..fc20f098111 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 @@ -55,7 +55,8 @@ class NotificationFactoryTest { meta = OneShotNotification.Append.Meta( key = A_ROOM_ID, summaryLine = AN_INVITATION_EVENT.description, - isNoisy = AN_INVITATION_EVENT.noisy + isNoisy = AN_INVITATION_EVENT.noisy, + timestamp = AN_INVITATION_EVENT.timestamp )) ) } @@ -83,7 +84,8 @@ class NotificationFactoryTest { meta = OneShotNotification.Append.Meta( key = AN_EVENT_ID, summaryLine = A_SIMPLE_EVENT.description, - isNoisy = A_SIMPLE_EVENT.noisy + isNoisy = A_SIMPLE_EVENT.noisy, + timestamp = AN_INVITATION_EVENT.timestamp )) ) } diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt index 74a11b6d0df..e3c97ad3cf7 100644 --- a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt @@ -34,12 +34,13 @@ private const val USE_COMPLETE_NOTIFICATION_FORMAT = true private val AN_EVENT_LIST = mutableListOf() private val A_PROCESSED_EVENTS = ProcessedNotificationEvents(emptyMap(), emptyMap(), emptyMap()) -private val A_SUMMARY_NOTIFICATION = mockk() +private val A_SUMMARY_NOTIFICATION = SummaryNotification.Update(mockk()) +private val A_REMOVE_SUMMARY_NOTIFICATION = SummaryNotification.Removed private val A_NOTIFICATION = mockk() private val MESSAGE_META = RoomNotification.Message.Meta( summaryLine = "ignored", messageCount = 1, latestTimestamp = -1, roomId = A_ROOM_ID, shouldBing = false ) -private val ONE_SHOT_META = OneShotNotification.Append.Meta(key = "ignored", summaryLine = "ignored", isNoisy = false) +private val ONE_SHOT_META = OneShotNotification.Append.Meta(key = "ignored", summaryLine = "ignored", isNoisy = false, timestamp = -1) class NotificationRendererTest { @@ -71,7 +72,7 @@ class NotificationRendererTest { notificationDisplayer.verifyInOrder { cancelNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID) - showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION.notification) } } @@ -86,7 +87,7 @@ class NotificationRendererTest { notificationDisplayer.verifyInOrder { showNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID, A_NOTIFICATION) - showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION.notification) } } @@ -98,7 +99,7 @@ class NotificationRendererTest { notificationDisplayer.verifyInOrder { cancelNotificationMessage(tag = AN_EVENT_ID, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID) - showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION.notification) } } @@ -113,7 +114,7 @@ class NotificationRendererTest { notificationDisplayer.verifyInOrder { showNotificationMessage(tag = AN_EVENT_ID, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID, A_NOTIFICATION) - showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION.notification) } } @@ -125,7 +126,7 @@ class NotificationRendererTest { notificationDisplayer.verifyInOrder { cancelNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID) - showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION.notification) } } @@ -140,7 +141,7 @@ class NotificationRendererTest { notificationDisplayer.verifyInOrder { showNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID, A_NOTIFICATION) - showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION) + showNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, A_SUMMARY_NOTIFICATION.notification) } } @@ -156,14 +157,14 @@ class NotificationRendererTest { } private fun givenNoNotifications() { - givenNotifications(emptyList(), emptyList(), emptyList(), USE_COMPLETE_NOTIFICATION_FORMAT, A_SUMMARY_NOTIFICATION) + givenNotifications(emptyList(), emptyList(), emptyList(), USE_COMPLETE_NOTIFICATION_FORMAT, A_REMOVE_SUMMARY_NOTIFICATION) } private fun givenNotifications(roomNotifications: List = emptyList(), invitationNotifications: List = emptyList(), simpleNotifications: List = emptyList(), useCompleteNotificationFormat: Boolean = USE_COMPLETE_NOTIFICATION_FORMAT, - summaryNotification: Notification = A_SUMMARY_NOTIFICATION) { + summaryNotification: SummaryNotification = A_SUMMARY_NOTIFICATION) { notifiableEventProcessor.givenProcessedEventsFor(AN_EVENT_LIST, A_CURRENT_ROOM_ID, A_PROCESSED_EVENTS) notificationFactory.givenNotificationsFor( processedEvents = A_PROCESSED_EVENTS, diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt index 921999bd932..da2dbc27da5 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt @@ -16,11 +16,11 @@ package im.vector.app.test.fakes -import android.app.Notification import im.vector.app.features.notifications.NotificationFactory import im.vector.app.features.notifications.OneShotNotification import im.vector.app.features.notifications.ProcessedNotificationEvents import im.vector.app.features.notifications.RoomNotification +import im.vector.app.features.notifications.SummaryNotification import io.mockk.every import io.mockk.mockk @@ -36,7 +36,7 @@ class FakeNotificationFactory { roomNotifications: List, invitationNotifications: List, simpleNotifications: List, - summaryNotification: Notification) { + summaryNotification: SummaryNotification) { with(instance) { every { processedEvents.roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) } returns roomNotifications every { processedEvents.invitationEvents.toNotifications(myUserId) } returns invitationNotifications From d92504458d8262ce3771cee2d59f5b294fd49f6f Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 7 Oct 2021 16:29:20 +0100 Subject: [PATCH 08/26] removing this usages for project convention --- .../app/features/notifications/NotificationFactory.kt | 6 +++--- .../app/features/notifications/RoomGroupMessageCreator.kt | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index 8189977ba81..0c3aab7bd39 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -28,7 +28,7 @@ class NotificationFactory @Inject constructor( ) { fun Map>.toNotifications(myUserDisplayName: String, myUserAvatarUrl: String?): List { - return this.map { (roomId, events) -> + return map { (roomId, events) -> when { events.hasNoEventsToDisplay() -> RoomNotification.Removed(roomId) else -> roomGroupMessageCreator.createRoomMessage(events, roomId, myUserDisplayName, myUserAvatarUrl) @@ -41,7 +41,7 @@ class NotificationFactory @Inject constructor( private fun NotifiableMessageEvent.canNotBeDisplayed() = isRedacted fun Map.toNotifications(myUserId: String): List { - return this.map { (roomId, event) -> + return map { (roomId, event) -> when (event) { null -> OneShotNotification.Removed(key = roomId) else -> OneShotNotification.Append( @@ -59,7 +59,7 @@ class NotificationFactory @Inject constructor( @JvmName("toNotificationsSimpleNotifiableEvent") fun Map.toNotifications(myUserId: String): List { - return this.map { (eventId, event) -> + return map { (eventId, event) -> when (event) { null -> OneShotNotification.Removed(key = eventId) else -> OneShotNotification.Append( 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 56e3274515d..113dc21ebd4 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 @@ -170,4 +170,4 @@ class RoomGroupMessageCreator @Inject constructor( } } -private fun NotifiableMessageEvent.isSmartReplyError() = this.outGoingMessage && this.outGoingMessageFailed +private fun NotifiableMessageEvent.isSmartReplyError() = outGoingMessage && outGoingMessageFailed From 1be096bee05bd45d9142068cf691f061f881bc79 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 7 Oct 2021 16:32:00 +0100 Subject: [PATCH 09/26] formatting --- .../notifications/NotificationFactory.kt | 13 +++++----- .../notifications/NotificationRenderer.kt | 26 ++++++++++++------- .../SummaryGroupMessageCreator.kt | 6 ++--- .../notifications/NotificationRendererTest.kt | 1 - 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index 0c3aab7bd39..d5de0221f6f 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -84,12 +84,13 @@ class NotificationFactory @Inject constructor( val simpleMeta = simpleNotifications.mapToMeta() return when { roomMeta.isEmpty() && invitationMeta.isEmpty() && simpleMeta.isEmpty() -> SummaryNotification.Removed - else -> SummaryNotification.Update(summaryGroupMessageCreator.createSummaryNotification( - roomNotifications = roomMeta, - invitationNotifications = invitationMeta, - simpleNotifications = simpleMeta, - useCompleteNotificationFormat = useCompleteNotificationFormat - )) + else -> SummaryNotification.Update( + summaryGroupMessageCreator.createSummaryNotification( + roomNotifications = roomMeta, + invitationNotifications = invitationMeta, + simpleNotifications = simpleMeta, + useCompleteNotificationFormat = useCompleteNotificationFormat + )) } } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt index 749e3c61b6b..844ea46f93b 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt @@ -17,6 +17,10 @@ package im.vector.app.features.notifications import android.content.Context import androidx.annotation.WorkerThread +import im.vector.app.features.notifications.NotificationDrawerManager.Companion.ROOM_EVENT_NOTIFICATION_ID +import im.vector.app.features.notifications.NotificationDrawerManager.Companion.ROOM_INVITATION_NOTIFICATION_ID +import im.vector.app.features.notifications.NotificationDrawerManager.Companion.ROOM_MESSAGES_NOTIFICATION_ID +import im.vector.app.features.notifications.NotificationDrawerManager.Companion.SUMMARY_NOTIFICATION_ID import androidx.core.content.pm.ShortcutManagerCompat import timber.log.Timber import javax.inject.Inject @@ -47,7 +51,11 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces } } - private fun processEvents(notificationEvents: ProcessedNotificationEvents, myUserId: String, myUserDisplayName: String, myUserAvatarUrl: String?, useCompleteNotificationFormat: Boolean) { + private fun processEvents(notificationEvents: ProcessedNotificationEvents, + myUserId: String, + myUserDisplayName: String, + myUserAvatarUrl: String?, + useCompleteNotificationFormat: Boolean) { val (roomEvents, simpleEvents, invitationEvents) = notificationEvents with(notificationFactory) { val roomNotifications = roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) @@ -62,33 +70,33 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces roomNotifications.forEach { wrapper -> when (wrapper) { - is RoomNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.roomId, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID) + is RoomNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.roomId, ROOM_MESSAGES_NOTIFICATION_ID) is RoomNotification.Message -> if (useCompleteNotificationFormat) { Timber.d("Updating room messages notification ${wrapper.meta.roomId}") wrapper.shortcutInfo?.let { ShortcutManagerCompat.pushDynamicShortcut(appContext, it) } - notificationDisplayer.showNotificationMessage(wrapper.meta.roomId, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID, wrapper.notification) + notificationDisplayer.showNotificationMessage(wrapper.meta.roomId, ROOM_MESSAGES_NOTIFICATION_ID, wrapper.notification) } } } invitationNotifications.forEach { wrapper -> when (wrapper) { - is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID) + is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, ROOM_INVITATION_NOTIFICATION_ID) is OneShotNotification.Append -> if (useCompleteNotificationFormat) { Timber.d("Updating invitation notification ${wrapper.meta.key}") - notificationDisplayer.showNotificationMessage(wrapper.meta.key, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID, wrapper.notification) + notificationDisplayer.showNotificationMessage(wrapper.meta.key, ROOM_INVITATION_NOTIFICATION_ID, wrapper.notification) } } } simpleNotifications.forEach { wrapper -> when (wrapper) { - is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID) + is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, ROOM_EVENT_NOTIFICATION_ID) is OneShotNotification.Append -> if (useCompleteNotificationFormat) { Timber.d("Updating simple notification ${wrapper.meta.key}") - notificationDisplayer.showNotificationMessage(wrapper.meta.key, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID, wrapper.notification) + notificationDisplayer.showNotificationMessage(wrapper.meta.key, ROOM_EVENT_NOTIFICATION_ID, wrapper.notification) } } } @@ -96,11 +104,11 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces when (summaryNotification) { SummaryNotification.Removed -> { Timber.d("Removing summary notification") - notificationDisplayer.cancelNotificationMessage(null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID) + notificationDisplayer.cancelNotificationMessage(null, SUMMARY_NOTIFICATION_ID) } is SummaryNotification.Update -> { Timber.d("Updating summary notification") - notificationDisplayer.showNotificationMessage(null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID, summaryNotification.notification) + notificationDisplayer.showNotificationMessage(null, SUMMARY_NOTIFICATION_ID, summaryNotification.notification) } } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt b/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt index 821f24b4363..ddef31a0f50 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/SummaryGroupMessageCreator.kt @@ -51,9 +51,9 @@ class SummaryGroupMessageCreator @Inject constructor( simpleNotifications.forEach { style.addLine(it.summaryLine) } } - val summaryIsNoisy = roomNotifications.any { it.shouldBing } - || invitationNotifications.any { it.isNoisy } - || simpleNotifications.any { it.isNoisy } + val summaryIsNoisy = roomNotifications.any { it.shouldBing } || + invitationNotifications.any { it.isNoisy } || + simpleNotifications.any { it.isNoisy } val messageCount = roomNotifications.fold(initial = 0) { acc, current -> acc + current.messageCount } diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt index e3c97ad3cf7..b0f1772fc70 100644 --- a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt @@ -20,7 +20,6 @@ import android.app.Notification import im.vector.app.test.fakes.FakeNotifiableEventProcessor import im.vector.app.test.fakes.FakeNotificationDisplayer import im.vector.app.test.fakes.FakeNotificationFactory -import im.vector.app.test.fakes.FakeVectorPreferences import io.mockk.mockk import org.junit.Test From c7054962c0db78051ed0d8239ea1ba9de8403568 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Fri, 8 Oct 2021 14:50:53 +0100 Subject: [PATCH 10/26] ensuring that we removing the summary group before removing individual notifications - adds some comments to explain the positioning --- .../notifications/NotificationRenderer.kt | 28 +++++++++++---- .../notifications/NotificationRendererTest.kt | 36 +++++++++++++++++++ 2 files changed, 57 insertions(+), 7 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt index 844ea46f93b..73ea65debca 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt @@ -68,9 +68,20 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces useCompleteNotificationFormat = useCompleteNotificationFormat ) + // Remove summary first to avoid briefly displaying it after dismissing the last notification + when (summaryNotification) { + SummaryNotification.Removed -> { + Timber.d("Removing summary notification") + notificationDisplayer.cancelNotificationMessage(null, SUMMARY_NOTIFICATION_ID) + } + } + roomNotifications.forEach { wrapper -> when (wrapper) { - is RoomNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.roomId, ROOM_MESSAGES_NOTIFICATION_ID) + is RoomNotification.Removed -> { + Timber.d("Removing room messages notification ${wrapper.roomId}") + notificationDisplayer.cancelNotificationMessage(wrapper.roomId, ROOM_MESSAGES_NOTIFICATION_ID) + } is RoomNotification.Message -> if (useCompleteNotificationFormat) { Timber.d("Updating room messages notification ${wrapper.meta.roomId}") wrapper.shortcutInfo?.let { @@ -83,7 +94,10 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces invitationNotifications.forEach { wrapper -> when (wrapper) { - is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, ROOM_INVITATION_NOTIFICATION_ID) + is OneShotNotification.Removed -> { + Timber.d("Removing invitation notification ${wrapper.key}") + notificationDisplayer.cancelNotificationMessage(wrapper.key, ROOM_INVITATION_NOTIFICATION_ID) + } is OneShotNotification.Append -> if (useCompleteNotificationFormat) { Timber.d("Updating invitation notification ${wrapper.meta.key}") notificationDisplayer.showNotificationMessage(wrapper.meta.key, ROOM_INVITATION_NOTIFICATION_ID, wrapper.notification) @@ -93,7 +107,10 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces simpleNotifications.forEach { wrapper -> when (wrapper) { - is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, ROOM_EVENT_NOTIFICATION_ID) + is OneShotNotification.Removed -> { + Timber.d("Removing simple notification ${wrapper.key}") + notificationDisplayer.cancelNotificationMessage(wrapper.key, ROOM_EVENT_NOTIFICATION_ID) + } is OneShotNotification.Append -> if (useCompleteNotificationFormat) { Timber.d("Updating simple notification ${wrapper.meta.key}") notificationDisplayer.showNotificationMessage(wrapper.meta.key, ROOM_EVENT_NOTIFICATION_ID, wrapper.notification) @@ -101,11 +118,8 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces } } + // Update summary last to avoid briefly displaying it before other notifications when (summaryNotification) { - SummaryNotification.Removed -> { - Timber.d("Removing summary notification") - notificationDisplayer.cancelNotificationMessage(null, SUMMARY_NOTIFICATION_ID) - } is SummaryNotification.Update -> { Timber.d("Updating summary notification") notificationDisplayer.showNotificationMessage(null, SUMMARY_NOTIFICATION_ID, summaryNotification.notification) diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt index b0f1772fc70..1c68dc4f688 100644 --- a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt @@ -63,6 +63,18 @@ class NotificationRendererTest { notificationDisplayer.verifyNoOtherInteractions() } + @Test + fun `given last room message group notification is removed when rendering then remove the summary and then remove message notification`() { + givenNotifications(roomNotifications = listOf(RoomNotification.Removed(A_ROOM_ID)), summaryNotification = A_REMOVE_SUMMARY_NOTIFICATION) + + renderEventsAsNotifications() + + notificationDisplayer.verifyInOrder { + cancelNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID) + cancelNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID) + } + } + @Test fun `given a room message group notification is removed when rendering then remove the message notification and update summary`() { givenNotifications(roomNotifications = listOf(RoomNotification.Removed(A_ROOM_ID))) @@ -90,6 +102,18 @@ class NotificationRendererTest { } } + @Test + fun `given last simple notification is removed when rendering then remove the summary and then remove simple notification`() { + givenNotifications(simpleNotifications = listOf(OneShotNotification.Removed(AN_EVENT_ID)), summaryNotification = A_REMOVE_SUMMARY_NOTIFICATION) + + renderEventsAsNotifications() + + notificationDisplayer.verifyInOrder { + cancelNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID) + cancelNotificationMessage(tag = AN_EVENT_ID, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID) + } + } + @Test fun `given a simple notification is removed when rendering then remove the simple notification and update summary`() { givenNotifications(simpleNotifications = listOf(OneShotNotification.Removed(AN_EVENT_ID))) @@ -117,6 +141,18 @@ class NotificationRendererTest { } } + @Test + fun `given last invitation notification is removed when rendering then remove the summary and then remove invitation notification`() { + givenNotifications(invitationNotifications = listOf(OneShotNotification.Removed(A_ROOM_ID)), summaryNotification = A_REMOVE_SUMMARY_NOTIFICATION) + + renderEventsAsNotifications() + + notificationDisplayer.verifyInOrder { + cancelNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID) + cancelNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID) + } + } + @Test fun `given an invitation notification is removed when rendering then remove the invitation notification and update summary`() { givenNotifications(invitationNotifications = listOf(OneShotNotification.Removed(A_ROOM_ID))) From bf9a3ef5646ddee3709a0afecf2f80db10f6cdc8 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 11 Oct 2021 09:55:26 +0100 Subject: [PATCH 11/26] relying on the notification refreshing to cancel/update the notifications --- .../features/notifications/NotificationDrawerManager.kt | 8 +------- 1 file changed, 1 insertion(+), 7 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 4be1f6ee6e0..042d2f1f882 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 @@ -165,14 +165,8 @@ class NotificationDrawerManager @Inject constructor(private val context: Context fun clearMessageEventOfRoom(roomId: String?) { Timber.v("clearMessageEventOfRoom $roomId") if (roomId != null) { - var shouldUpdate = false - synchronized(eventList) { - shouldUpdate = eventList.removeAll { e -> - e is NotifiableMessageEvent && e.roomId == roomId - } - } + val shouldUpdate = removeAll { it is NotifiableMessageEvent && it.roomId == roomId } if (shouldUpdate) { - notificationUtils.cancelNotificationMessage(roomId, ROOM_MESSAGES_NOTIFICATION_ID) refreshNotificationDrawer() } } From 41b3deb4fb0b5f77822a07457b50067dbd0da987 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 11 Oct 2021 14:45:41 +0100 Subject: [PATCH 12/26] splitting the event processing from the rendering - this allows us to only synchronise of the event list modifications rather than the entire notification creation/rendering which should in turn reduce some of our ANRs https://github.com/vector-im/element-android/issues/4214 --- .../notifications/NotifiableEventProcessor.kt | 48 ++++------------- .../NotificationDrawerManager.kt | 36 ++++++++----- .../notifications/NotificationRenderer.kt | 54 ++++++++++--------- .../NotifiableEventProcessorTest.kt | 14 ++--- .../notifications/NotificationRendererTest.kt | 14 ++--- .../fakes/FakeNotifiableEventProcessor.kt | 6 --- .../app/test/fakes/FakeNotificationFactory.kt | 10 ++-- 7 files changed, 77 insertions(+), 105 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt index 3f77ce54ca5..bf9e805fc8c 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt @@ -17,7 +17,6 @@ package im.vector.app.features.notifications import im.vector.app.features.invite.AutoAcceptInvites -import timber.log.Timber import javax.inject.Inject class NotifiableEventProcessor @Inject constructor( @@ -25,49 +24,20 @@ class NotifiableEventProcessor @Inject constructor( private val autoAcceptInvites: AutoAcceptInvites ) { - fun modifyAndProcess(eventList: MutableList, currentRoomId: String?): ProcessedNotificationEvents { - val roomIdToEventMap: MutableMap> = LinkedHashMap() - val simpleEvents: MutableMap = LinkedHashMap() - val invitationEvents: MutableMap = LinkedHashMap() - - val eventIterator = eventList.listIterator() - while (eventIterator.hasNext()) { - when (val event = eventIterator.next()) { - is NotifiableMessageEvent -> { - val roomId = event.roomId - val roomEvents = roomIdToEventMap.getOrPut(roomId) { ArrayList() } - - // should we limit to last 7 messages per room? - if (shouldIgnoreMessageEventInRoom(currentRoomId, roomId) || outdatedDetector.isMessageOutdated(event)) { - // forget this event - eventIterator.remove() - } else { - roomEvents.add(event) + fun process(eventList: List, currentRoomId: String?): Map { + return eventList.associateBy { it.eventId } + .mapValues { (_, value) -> + when (value) { + is InviteNotifiableEvent -> if (autoAcceptInvites.hideInvites) null else value + is NotifiableMessageEvent -> if (shouldIgnoreMessageEventInRoom(currentRoomId, value.roomId) || outdatedDetector.isMessageOutdated(value)) { + null + } else value + is SimpleNotifiableEvent -> value } } - is InviteNotifiableEvent -> { - if (autoAcceptInvites.hideInvites) { - // Forget this event - eventIterator.remove() - invitationEvents[event.roomId] = null - } else { - invitationEvents[event.roomId] = event - } - } - is SimpleNotifiableEvent -> simpleEvents[event.eventId] = event - else -> Timber.w("Type not handled") - } - } - return ProcessedNotificationEvents(roomIdToEventMap, simpleEvents, invitationEvents) } private fun shouldIgnoreMessageEventInRoom(currentRoomId: String?, roomId: String?): Boolean { return currentRoomId != null && roomId == currentRoomId } } - -data class ProcessedNotificationEvents( - val roomEvents: Map>, - val simpleEvents: Map, - val invitationEvents: Map -) 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 042d2f1f882..43d9eff1855 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 @@ -44,6 +44,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context private val notificationUtils: NotificationUtils, private val vectorPreferences: VectorPreferences, private val activeSessionDataSource: ActiveSessionDataSource, + private val notifiableEventProcessor: NotifiableEventProcessor, private val notificationRenderer: NotificationRenderer) { private val handlerThread: HandlerThread = HandlerThread("NotificationDrawerManager", Thread.MIN_PRIORITY) @@ -55,6 +56,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } private val eventList = loadEventInfo() + private var renderedEventsList = emptyMap() private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size) private var currentRoomId: String? = null @@ -223,22 +225,32 @@ class NotificationDrawerManager @Inject constructor(private val context: Context @WorkerThread private fun refreshNotificationDrawerBg() { Timber.v("refreshNotificationDrawerBg()") - val session = currentSession ?: return - val user = session.getUser(session.myUserId) - // myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash - val myUserDisplayName = user?.toMatrixItem()?.getBestName() ?: session.myUserId - val myUserAvatarUrl = session.contentUrlResolver().resolveThumbnail(user?.avatarUrl, avatarSize, avatarSize, ContentUrlResolver.ThumbnailMethod.SCALE) + val newSettings = vectorPreferences.useCompleteNotificationFormat() + if (newSettings != useCompleteNotificationFormat) { + // Settings has changed, remove all current notifications + notificationUtils.cancelAllNotifications() + useCompleteNotificationFormat = newSettings + } - synchronized(eventList) { - val newSettings = vectorPreferences.useCompleteNotificationFormat() - if (newSettings != useCompleteNotificationFormat) { - // Settings has changed, remove all current notifications - notificationUtils.cancelAllNotifications() - useCompleteNotificationFormat = newSettings + val eventsToRender = synchronized(eventList) { + notifiableEventProcessor.process(eventList, currentRoomId).also { + eventList.clear() + eventList.addAll(it.values.filterNotNull()) } + } - notificationRenderer.render(currentRoomId, session.myUserId, myUserDisplayName, myUserAvatarUrl, useCompleteNotificationFormat, eventList) + if (renderedEventsList == eventsToRender) { + Timber.d("Skipping notification update due to event list not changing") + } else { + renderedEventsList = eventsToRender + val session = currentSession ?: return + val user = session.getUser(session.myUserId) + // myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash + val myUserDisplayName = user?.toMatrixItem()?.getBestName() ?: session.myUserId + val myUserAvatarUrl = session.contentUrlResolver().resolveThumbnail(user?.avatarUrl, avatarSize, avatarSize, ContentUrlResolver.ThumbnailMethod.SCALE) + + notificationRenderer.render(session.myUserId, myUserDisplayName, myUserAvatarUrl, useCompleteNotificationFormat, eventsToRender) } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt index 73ea65debca..80391b1e063 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt @@ -15,7 +15,6 @@ */ package im.vector.app.features.notifications -import android.content.Context import androidx.annotation.WorkerThread import im.vector.app.features.notifications.NotificationDrawerManager.Companion.ROOM_EVENT_NOTIFICATION_ID import im.vector.app.features.notifications.NotificationDrawerManager.Companion.ROOM_INVITATION_NOTIFICATION_ID @@ -27,36 +26,16 @@ import javax.inject.Inject import javax.inject.Singleton @Singleton -class NotificationRenderer @Inject constructor(private val notifiableEventProcessor: NotifiableEventProcessor, - private val notificationDisplayer: NotificationDisplayer, - private val notificationFactory: NotificationFactory, - private val appContext: Context) { - - private var lastKnownEventList = -1 +class NotificationRenderer @Inject constructor(private val notificationDisplayer: NotificationDisplayer, + private val notificationFactory: NotificationFactory) { @WorkerThread - fun render(currentRoomId: String?, - myUserId: String, + fun render(myUserId: String, myUserDisplayName: String, myUserAvatarUrl: String?, useCompleteNotificationFormat: Boolean, - eventList: MutableList) { - Timber.v("Render notification events - count: ${eventList.size}") - val notificationEvents = notifiableEventProcessor.modifyAndProcess(eventList, currentRoomId) - if (lastKnownEventList == notificationEvents.hashCode()) { - Timber.d("Skipping notification update due to event list not changing") - } else { - processEvents(notificationEvents, myUserId, myUserDisplayName, myUserAvatarUrl, useCompleteNotificationFormat) - lastKnownEventList = notificationEvents.hashCode() - } - } - - private fun processEvents(notificationEvents: ProcessedNotificationEvents, - myUserId: String, - myUserDisplayName: String, - myUserAvatarUrl: String?, - useCompleteNotificationFormat: Boolean) { - val (roomEvents, simpleEvents, invitationEvents) = notificationEvents + eventsToProcess: Map) { + val (roomEvents, simpleEvents, invitationEvents) = eventsToProcess.groupByType() with(notificationFactory) { val roomNotifications = roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) val invitationNotifications = invitationEvents.toNotifications(myUserId) @@ -128,3 +107,26 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces } } } + +private fun Map.groupByType(): GroupedNotificationEvents { + val roomIdToEventMap: MutableMap> = LinkedHashMap() + val simpleEvents: MutableMap = LinkedHashMap() + val invitationEvents: MutableMap = LinkedHashMap() + forEach { (_, value) -> + when (value) { + is InviteNotifiableEvent -> invitationEvents[value.roomId] + is NotifiableMessageEvent -> { + val roomEvents = roomIdToEventMap.getOrPut(value.roomId) { ArrayList() } + roomEvents.add(value) + } + is SimpleNotifiableEvent -> simpleEvents[value.eventId] = value + } + } + return GroupedNotificationEvents(roomIdToEventMap, simpleEvents, invitationEvents) +} + +data class GroupedNotificationEvents( + val roomEvents: Map>, + val simpleEvents: Map, + val invitationEvents: Map +) 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 6f47e71500b..3e66f82bc38 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 @@ -37,7 +37,7 @@ class NotifiableEventProcessorTest { aSimpleNotifiableEvent(eventId = "event-2") ) - val result = eventProcessor.modifyAndProcess(events, currentRoomId = NOT_VIEWING_A_ROOM) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM) result shouldBeEqualTo aProcessedNotificationEvents( simpleEvents = mapOf( @@ -56,7 +56,7 @@ class NotifiableEventProcessorTest { anInviteNotifiableEvent(roomId = "room-2") ) - val result = eventProcessor.modifyAndProcess(events, currentRoomId = NOT_VIEWING_A_ROOM) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM) result shouldBeEqualTo aProcessedNotificationEvents( invitationEvents = mapOf( @@ -75,7 +75,7 @@ class NotifiableEventProcessorTest { anInviteNotifiableEvent(roomId = "room-2") ) - val result = eventProcessor.modifyAndProcess(events, currentRoomId = NOT_VIEWING_A_ROOM) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM) result shouldBeEqualTo aProcessedNotificationEvents( invitationEvents = mapOf( @@ -91,7 +91,7 @@ class NotifiableEventProcessorTest { val (events) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) outdatedDetector.givenEventIsOutOfDate(events[0]) - val result = eventProcessor.modifyAndProcess(events, currentRoomId = NOT_VIEWING_A_ROOM) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM) result shouldBeEqualTo aProcessedNotificationEvents( roomEvents = mapOf( @@ -106,7 +106,7 @@ class NotifiableEventProcessorTest { val (events, originalEvents) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) outdatedDetector.givenEventIsInDate(events[0]) - val result = eventProcessor.modifyAndProcess(events, currentRoomId = NOT_VIEWING_A_ROOM) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM) result shouldBeEqualTo aProcessedNotificationEvents( roomEvents = mapOf( @@ -120,7 +120,7 @@ class NotifiableEventProcessorTest { fun `given viewing the same room as message event when processing then removes message`() { val (events) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) - val result = eventProcessor.modifyAndProcess(events, currentRoomId = "room-1") + val result = eventProcessor.process(events, currentRoomId = "room-1") result shouldBeEqualTo aProcessedNotificationEvents( roomEvents = mapOf( @@ -140,7 +140,7 @@ fun createEventsList(vararg event: NotifiableEvent): Pair = emptyMap(), invitationEvents: Map = emptyMap(), roomEvents: Map> = emptyMap() -) = ProcessedNotificationEvents( +) = GroupedNotificationEvents( roomEvents = roomEvents, simpleEvents = simpleEvents, invitationEvents = invitationEvents, diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt index 1c68dc4f688..bd0d1e8d3f4 100644 --- a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt @@ -17,13 +17,11 @@ package im.vector.app.features.notifications import android.app.Notification -import im.vector.app.test.fakes.FakeNotifiableEventProcessor import im.vector.app.test.fakes.FakeNotificationDisplayer import im.vector.app.test.fakes.FakeNotificationFactory import io.mockk.mockk import org.junit.Test -private const val A_CURRENT_ROOM_ID = "current-room-id" private const val MY_USER_ID = "my-user-id" private const val MY_USER_DISPLAY_NAME = "display-name" private const val MY_USER_AVATAR_URL = "avatar-url" @@ -31,8 +29,8 @@ private const val AN_EVENT_ID = "event-id" private const val A_ROOM_ID = "room-id" private const val USE_COMPLETE_NOTIFICATION_FORMAT = true -private val AN_EVENT_LIST = mutableListOf() -private val A_PROCESSED_EVENTS = ProcessedNotificationEvents(emptyMap(), emptyMap(), emptyMap()) +private val AN_EVENT_LIST = mapOf() +private val A_PROCESSED_EVENTS = GroupedNotificationEvents(emptyMap(), emptyMap(), emptyMap()) private val A_SUMMARY_NOTIFICATION = SummaryNotification.Update(mockk()) private val A_REMOVE_SUMMARY_NOTIFICATION = SummaryNotification.Removed private val A_NOTIFICATION = mockk() @@ -43,12 +41,10 @@ private val ONE_SHOT_META = OneShotNotification.Append.Meta(key = "ignored", sum class NotificationRendererTest { - private val notifiableEventProcessor = FakeNotifiableEventProcessor() private val notificationDisplayer = FakeNotificationDisplayer() private val notificationFactory = FakeNotificationFactory() private val notificationRenderer = NotificationRenderer( - notifiableEventProcessor = notifiableEventProcessor.instance, notificationDisplayer = notificationDisplayer.instance, notificationFactory = notificationFactory.instance ) @@ -182,12 +178,11 @@ class NotificationRendererTest { private fun renderEventsAsNotifications() { notificationRenderer.render( - currentRoomId = A_CURRENT_ROOM_ID, myUserId = MY_USER_ID, myUserDisplayName = MY_USER_DISPLAY_NAME, myUserAvatarUrl = MY_USER_AVATAR_URL, useCompleteNotificationFormat = USE_COMPLETE_NOTIFICATION_FORMAT, - eventList = AN_EVENT_LIST + eventsToProcess = AN_EVENT_LIST ) } @@ -200,9 +195,8 @@ class NotificationRendererTest { simpleNotifications: List = emptyList(), useCompleteNotificationFormat: Boolean = USE_COMPLETE_NOTIFICATION_FORMAT, summaryNotification: SummaryNotification = A_SUMMARY_NOTIFICATION) { - notifiableEventProcessor.givenProcessedEventsFor(AN_EVENT_LIST, A_CURRENT_ROOM_ID, A_PROCESSED_EVENTS) notificationFactory.givenNotificationsFor( - processedEvents = A_PROCESSED_EVENTS, + groupedEvents = A_PROCESSED_EVENTS, myUserId = MY_USER_ID, myUserDisplayName = MY_USER_DISPLAY_NAME, myUserAvatarUrl = MY_USER_AVATAR_URL, diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt index 93f5e405248..6143c7a9072 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt @@ -16,17 +16,11 @@ package im.vector.app.test.fakes -import im.vector.app.features.notifications.NotifiableEvent import im.vector.app.features.notifications.NotifiableEventProcessor -import im.vector.app.features.notifications.ProcessedNotificationEvents -import io.mockk.every import io.mockk.mockk class FakeNotifiableEventProcessor { val instance = mockk() - fun givenProcessedEventsFor(events: MutableList, currentRoomId: String?, processedEvents: ProcessedNotificationEvents) { - every { instance.modifyAndProcess(events, currentRoomId) } returns processedEvents - } } diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt index da2dbc27da5..cc6f84f813d 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt @@ -18,7 +18,7 @@ package im.vector.app.test.fakes import im.vector.app.features.notifications.NotificationFactory import im.vector.app.features.notifications.OneShotNotification -import im.vector.app.features.notifications.ProcessedNotificationEvents +import im.vector.app.features.notifications.GroupedNotificationEvents import im.vector.app.features.notifications.RoomNotification import im.vector.app.features.notifications.SummaryNotification import io.mockk.every @@ -28,7 +28,7 @@ class FakeNotificationFactory { val instance = mockk() - fun givenNotificationsFor(processedEvents: ProcessedNotificationEvents, + fun givenNotificationsFor(groupedEvents: GroupedNotificationEvents, myUserId: String, myUserDisplayName: String, myUserAvatarUrl: String?, @@ -38,9 +38,9 @@ class FakeNotificationFactory { simpleNotifications: List, summaryNotification: SummaryNotification) { with(instance) { - every { processedEvents.roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) } returns roomNotifications - every { processedEvents.invitationEvents.toNotifications(myUserId) } returns invitationNotifications - every { processedEvents.simpleEvents.toNotifications(myUserId) } returns simpleNotifications + every { groupedEvents.roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) } returns roomNotifications + every { groupedEvents.invitationEvents.toNotifications(myUserId) } returns invitationNotifications + every { groupedEvents.simpleEvents.toNotifications(myUserId) } returns simpleNotifications every { createSummaryNotification( From b0306ab9d153fe32e9451973927cd0c233689959 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 11 Oct 2021 15:32:12 +0100 Subject: [PATCH 13/26] using a process state of keep/removed rather than mapping to an ignored event id - this state will be used to diff the currently rendered events against the new ones --- .../notifications/NotifiableEventProcessor.kt | 30 ++++++++++++------- .../NotificationDrawerManager.kt | 6 ++-- .../notifications/NotificationFactory.kt | 24 +++++++-------- .../notifications/NotificationRenderer.kt | 27 +++++++++-------- .../NotifiableEventProcessorTest.kt | 12 ++++---- 5 files changed, 56 insertions(+), 43 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt index bf9e805fc8c..782f70645b0 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt @@ -17,6 +17,8 @@ package im.vector.app.features.notifications import im.vector.app.features.invite.AutoAcceptInvites +import im.vector.app.features.notifications.Processed.KEEP +import im.vector.app.features.notifications.Processed.REMOVE import javax.inject.Inject class NotifiableEventProcessor @Inject constructor( @@ -24,20 +26,26 @@ class NotifiableEventProcessor @Inject constructor( private val autoAcceptInvites: AutoAcceptInvites ) { - fun process(eventList: List, currentRoomId: String?): Map { - return eventList.associateBy { it.eventId } - .mapValues { (_, value) -> - when (value) { - is InviteNotifiableEvent -> if (autoAcceptInvites.hideInvites) null else value - is NotifiableMessageEvent -> if (shouldIgnoreMessageEventInRoom(currentRoomId, value.roomId) || outdatedDetector.isMessageOutdated(value)) { - null - } else value - is SimpleNotifiableEvent -> value - } - } + fun process(eventList: List, currentRoomId: String?): List> { + return eventList.map { + when (it) { + is InviteNotifiableEvent -> if (autoAcceptInvites.hideInvites) REMOVE else KEEP + is NotifiableMessageEvent -> if (shouldIgnoreMessageEventInRoom(currentRoomId, it.roomId) || outdatedDetector.isMessageOutdated(it)) { + REMOVE + } else KEEP + is SimpleNotifiableEvent -> KEEP + } to it + } } private fun shouldIgnoreMessageEventInRoom(currentRoomId: String?, roomId: String?): Boolean { return currentRoomId != null && roomId == currentRoomId } } + +enum class Processed { + KEEP, + REMOVE +} + +fun List>.onlyKeptEvents() = filter { it.first == KEEP }.map { it.second } 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 43d9eff1855..c73b4b2a9c0 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 @@ -56,7 +56,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } private val eventList = loadEventInfo() - private var renderedEventsList = emptyMap() + private var renderedEventsList = emptyList>() private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size) private var currentRoomId: String? = null @@ -236,10 +236,12 @@ class NotificationDrawerManager @Inject constructor(private val context: Context val eventsToRender = synchronized(eventList) { notifiableEventProcessor.process(eventList, currentRoomId).also { eventList.clear() - eventList.addAll(it.values.filterNotNull()) + eventList.addAll(it.onlyKeptEvents()) } } + + if (renderedEventsList == eventsToRender) { Timber.d("Skipping notification update due to event list not changing") } else { diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index d5de0221f6f..88f21a02a69 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -40,14 +40,14 @@ class NotificationFactory @Inject constructor( private fun NotifiableMessageEvent.canNotBeDisplayed() = isRedacted - fun Map.toNotifications(myUserId: String): List { - return map { (roomId, event) -> - when (event) { - null -> OneShotNotification.Removed(key = roomId) - else -> OneShotNotification.Append( + fun List>.toNotifications(myUserId: String): List { + return map { (processed, event) -> + when (processed) { + Processed.REMOVE -> OneShotNotification.Removed(key = event.roomId) + Processed.KEEP -> OneShotNotification.Append( notificationUtils.buildRoomInvitationNotification(event, myUserId), OneShotNotification.Append.Meta( - key = roomId, + key = event.roomId, summaryLine = event.description, isNoisy = event.noisy, timestamp = event.timestamp @@ -58,14 +58,14 @@ class NotificationFactory @Inject constructor( } @JvmName("toNotificationsSimpleNotifiableEvent") - fun Map.toNotifications(myUserId: String): List { - return map { (eventId, event) -> - when (event) { - null -> OneShotNotification.Removed(key = eventId) - else -> OneShotNotification.Append( + fun List>.toNotifications(myUserId: String): List { + return map { (processed, event) -> + when (processed) { + Processed.REMOVE -> OneShotNotification.Removed(key = event.eventId) + Processed.KEEP -> OneShotNotification.Append( notificationUtils.buildSimpleEventNotification(event, myUserId), OneShotNotification.Append.Meta( - key = eventId, + key = event.eventId, summaryLine = event.description, isNoisy = event.noisy, timestamp = event.timestamp diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt index 80391b1e063..31a99810e04 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt @@ -34,7 +34,7 @@ class NotificationRenderer @Inject constructor(private val notificationDisplayer myUserDisplayName: String, myUserAvatarUrl: String?, useCompleteNotificationFormat: Boolean, - eventsToProcess: Map) { + eventsToProcess: List>) { val (roomEvents, simpleEvents, invitationEvents) = eventsToProcess.groupByType() with(notificationFactory) { val roomNotifications = roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) @@ -108,25 +108,28 @@ class NotificationRenderer @Inject constructor(private val notificationDisplayer } } -private fun Map.groupByType(): GroupedNotificationEvents { +private fun List>.groupByType(): GroupedNotificationEvents { val roomIdToEventMap: MutableMap> = LinkedHashMap() - val simpleEvents: MutableMap = LinkedHashMap() - val invitationEvents: MutableMap = LinkedHashMap() - forEach { (_, value) -> - when (value) { - is InviteNotifiableEvent -> invitationEvents[value.roomId] + val simpleEvents: MutableList> = ArrayList() + val invitationEvents: MutableList> = ArrayList() + forEach { + when (val event = it.second) { + is InviteNotifiableEvent -> invitationEvents.add(it.asPair()) is NotifiableMessageEvent -> { - val roomEvents = roomIdToEventMap.getOrPut(value.roomId) { ArrayList() } - roomEvents.add(value) + val roomEvents = roomIdToEventMap.getOrPut(event.roomId) { ArrayList() } + roomEvents.add(event) } - is SimpleNotifiableEvent -> simpleEvents[value.eventId] = value + is SimpleNotifiableEvent -> simpleEvents.add(it.asPair()) } } return GroupedNotificationEvents(roomIdToEventMap, simpleEvents, invitationEvents) } +@Suppress("UNCHECKED_CAST") +private fun Pair.asPair(): Pair = this as Pair + data class GroupedNotificationEvents( val roomEvents: Map>, - val simpleEvents: Map, - val invitationEvents: Map + val simpleEvents: List>, + val invitationEvents: List> ) 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 3e66f82bc38..154763afae3 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 @@ -37,7 +37,7 @@ class NotifiableEventProcessorTest { aSimpleNotifiableEvent(eventId = "event-2") ) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEventsList) result shouldBeEqualTo aProcessedNotificationEvents( simpleEvents = mapOf( @@ -56,7 +56,7 @@ class NotifiableEventProcessorTest { anInviteNotifiableEvent(roomId = "room-2") ) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEventsList) result shouldBeEqualTo aProcessedNotificationEvents( invitationEvents = mapOf( @@ -75,7 +75,7 @@ class NotifiableEventProcessorTest { anInviteNotifiableEvent(roomId = "room-2") ) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEventsList) result shouldBeEqualTo aProcessedNotificationEvents( invitationEvents = mapOf( @@ -91,7 +91,7 @@ class NotifiableEventProcessorTest { val (events) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) outdatedDetector.givenEventIsOutOfDate(events[0]) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEventsList) result shouldBeEqualTo aProcessedNotificationEvents( roomEvents = mapOf( @@ -106,7 +106,7 @@ class NotifiableEventProcessorTest { val (events, originalEvents) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) outdatedDetector.givenEventIsInDate(events[0]) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEventsList) result shouldBeEqualTo aProcessedNotificationEvents( roomEvents = mapOf( @@ -120,7 +120,7 @@ class NotifiableEventProcessorTest { fun `given viewing the same room as message event when processing then removes message`() { val (events) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) - val result = eventProcessor.process(events, currentRoomId = "room-1") + val result = eventProcessor.process(events, currentRoomId = "room-1", renderedEventsList = renderedEventsList) result shouldBeEqualTo aProcessedNotificationEvents( roomEvents = mapOf( From edff292a4dabac6a616b9b8a4bb4936e9863e108 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 11 Oct 2021 16:39:52 +0100 Subject: [PATCH 14/26] diffing the notification events against the currently rendered events allow us to dismiss notifications from removed events --- .../notifications/NotifiableEventProcessor.kt | 21 ++-- .../NotificationDrawerManager.kt | 6 +- .../notifications/NotificationFactory.kt | 12 +- .../notifications/NotificationRenderer.kt | 14 +-- .../features/notifications/ProcessedEvent.kt} | 14 +-- .../NotifiableEventProcessorTest.kt | 109 ++++++++---------- .../notifications/NotificationFactoryTest.kt | 8 +- .../notifications/NotificationRendererTest.kt | 4 +- .../app/test/fakes/FakeNotificationFactory.kt | 3 +- 9 files changed, 85 insertions(+), 106 deletions(-) rename vector/src/{test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt => main/java/im/vector/app/features/notifications/ProcessedEvent.kt} (69%) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt index 782f70645b0..88dc455e20f 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt @@ -17,8 +17,8 @@ package im.vector.app.features.notifications import im.vector.app.features.invite.AutoAcceptInvites -import im.vector.app.features.notifications.Processed.KEEP -import im.vector.app.features.notifications.Processed.REMOVE +import im.vector.app.features.notifications.ProcessedType.KEEP +import im.vector.app.features.notifications.ProcessedType.REMOVE import javax.inject.Inject class NotifiableEventProcessor @Inject constructor( @@ -26,8 +26,8 @@ class NotifiableEventProcessor @Inject constructor( private val autoAcceptInvites: AutoAcceptInvites ) { - fun process(eventList: List, currentRoomId: String?): List> { - return eventList.map { + fun process(eventList: List, currentRoomId: String?, renderedEventsList: List>): List { + val processedEventList = eventList.map { when (it) { is InviteNotifiableEvent -> if (autoAcceptInvites.hideInvites) REMOVE else KEEP is NotifiableMessageEvent -> if (shouldIgnoreMessageEventInRoom(currentRoomId, it.roomId) || outdatedDetector.isMessageOutdated(it)) { @@ -36,16 +36,15 @@ class NotifiableEventProcessor @Inject constructor( is SimpleNotifiableEvent -> KEEP } to it } + + val removedEventsDiff = renderedEventsList.filter { renderedEvent -> + eventList.none { it.eventId == renderedEvent.second.eventId } + }.map { REMOVE to it.second } + + return removedEventsDiff + processedEventList } private fun shouldIgnoreMessageEventInRoom(currentRoomId: String?, roomId: String?): Boolean { return currentRoomId != null && roomId == currentRoomId } } - -enum class Processed { - KEEP, - REMOVE -} - -fun List>.onlyKeptEvents() = filter { it.first == KEEP }.map { it.second } 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 c73b4b2a9c0..b7bb20237cc 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 @@ -56,7 +56,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } private val eventList = loadEventInfo() - private var renderedEventsList = emptyList>() + private var renderedEventsList = emptyList>() private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size) private var currentRoomId: String? = null @@ -234,14 +234,12 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } val eventsToRender = synchronized(eventList) { - notifiableEventProcessor.process(eventList, currentRoomId).also { + notifiableEventProcessor.process(eventList, currentRoomId, renderedEventsList).also { eventList.clear() eventList.addAll(it.onlyKeptEvents()) } } - - if (renderedEventsList == eventsToRender) { Timber.d("Skipping notification update due to event list not changing") } else { diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index 88f21a02a69..fe1671b58bc 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -40,11 +40,11 @@ class NotificationFactory @Inject constructor( private fun NotifiableMessageEvent.canNotBeDisplayed() = isRedacted - fun List>.toNotifications(myUserId: String): List { + fun List>.toNotifications(myUserId: String): List { return map { (processed, event) -> when (processed) { - Processed.REMOVE -> OneShotNotification.Removed(key = event.roomId) - Processed.KEEP -> OneShotNotification.Append( + ProcessedType.REMOVE -> OneShotNotification.Removed(key = event.roomId) + ProcessedType.KEEP -> OneShotNotification.Append( notificationUtils.buildRoomInvitationNotification(event, myUserId), OneShotNotification.Append.Meta( key = event.roomId, @@ -58,11 +58,11 @@ class NotificationFactory @Inject constructor( } @JvmName("toNotificationsSimpleNotifiableEvent") - fun List>.toNotifications(myUserId: String): List { + fun List>.toNotifications(myUserId: String): List { return map { (processed, event) -> when (processed) { - Processed.REMOVE -> OneShotNotification.Removed(key = event.eventId) - Processed.KEEP -> OneShotNotification.Append( + ProcessedType.REMOVE -> OneShotNotification.Removed(key = event.eventId) + ProcessedType.KEEP -> OneShotNotification.Append( notificationUtils.buildSimpleEventNotification(event, myUserId), OneShotNotification.Append.Meta( key = event.eventId, diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt index 31a99810e04..7cf0a8872a3 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt @@ -34,7 +34,7 @@ class NotificationRenderer @Inject constructor(private val notificationDisplayer myUserDisplayName: String, myUserAvatarUrl: String?, useCompleteNotificationFormat: Boolean, - eventsToProcess: List>) { + eventsToProcess: List>) { val (roomEvents, simpleEvents, invitationEvents) = eventsToProcess.groupByType() with(notificationFactory) { val roomNotifications = roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) @@ -108,10 +108,10 @@ class NotificationRenderer @Inject constructor(private val notificationDisplayer } } -private fun List>.groupByType(): GroupedNotificationEvents { +private fun List>.groupByType(): GroupedNotificationEvents { val roomIdToEventMap: MutableMap> = LinkedHashMap() - val simpleEvents: MutableList> = ArrayList() - val invitationEvents: MutableList> = ArrayList() + val simpleEvents: MutableList> = ArrayList() + val invitationEvents: MutableList> = ArrayList() forEach { when (val event = it.second) { is InviteNotifiableEvent -> invitationEvents.add(it.asPair()) @@ -126,10 +126,10 @@ private fun List>.groupByType(): GroupedNotific } @Suppress("UNCHECKED_CAST") -private fun Pair.asPair(): Pair = this as Pair +private fun Pair.asPair(): Pair = this as Pair data class GroupedNotificationEvents( val roomEvents: Map>, - val simpleEvents: List>, - val invitationEvents: List> + val simpleEvents: List>, + val invitationEvents: List> ) diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt b/vector/src/main/java/im/vector/app/features/notifications/ProcessedEvent.kt similarity index 69% rename from vector/src/test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt rename to vector/src/main/java/im/vector/app/features/notifications/ProcessedEvent.kt index 6143c7a9072..0901757d020 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeNotifiableEventProcessor.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/ProcessedEvent.kt @@ -14,13 +14,13 @@ * limitations under the License. */ -package im.vector.app.test.fakes +package im.vector.app.features.notifications -import im.vector.app.features.notifications.NotifiableEventProcessor -import io.mockk.mockk - -class FakeNotifiableEventProcessor { - - val instance = mockk() +typealias ProcessedEvent = Pair +enum class ProcessedType { + KEEP, + REMOVE } + +fun List.onlyKeptEvents() = filter { it.first == ProcessedType.KEEP }.map { it.second } 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 154763afae3..1c0f5f93901 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 @@ -31,120 +31,103 @@ class NotifiableEventProcessorTest { private val eventProcessor = NotifiableEventProcessor(outdatedDetector.instance, autoAcceptInvites) @Test - fun `given simple events when processing then return without mutating`() { - val (events, originalEvents) = createEventsList( + fun `given simple events when processing then keep simple events`() { + val events = listOf( aSimpleNotifiableEvent(eventId = "event-1"), aSimpleNotifiableEvent(eventId = "event-2") ) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEventsList) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) - result shouldBeEqualTo aProcessedNotificationEvents( - simpleEvents = mapOf( - "event-1" to events[0] as SimpleNotifiableEvent, - "event-2" to events[1] as SimpleNotifiableEvent - ) + result shouldBeEqualTo listOf( + ProcessedType.KEEP to events[0], + ProcessedType.KEEP to events[1] ) - events shouldBeEqualTo originalEvents } @Test fun `given invites are auto accepted when processing then remove invitations`() { autoAcceptInvites._isEnabled = true - val events = mutableListOf( + val events = listOf( anInviteNotifiableEvent(roomId = "room-1"), anInviteNotifiableEvent(roomId = "room-2") ) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEventsList) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) - result shouldBeEqualTo aProcessedNotificationEvents( - invitationEvents = mapOf( - "room-1" to null, - "room-2" to null - ) + result shouldBeEqualTo listOf( + ProcessedType.REMOVE to events[0], + ProcessedType.REMOVE to events[1] ) - events shouldBeEqualTo emptyList() } @Test - fun `given invites are not auto accepted when processing then return without mutating`() { + fun `given invites are not auto accepted when processing then keep invitation events`() { autoAcceptInvites._isEnabled = false - val (events, originalEvents) = createEventsList( + val events = listOf( anInviteNotifiableEvent(roomId = "room-1"), anInviteNotifiableEvent(roomId = "room-2") ) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEventsList) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) - result shouldBeEqualTo aProcessedNotificationEvents( - invitationEvents = mapOf( - "room-1" to originalEvents[0] as InviteNotifiableEvent, - "room-2" to originalEvents[1] as InviteNotifiableEvent - ) + result shouldBeEqualTo listOf( + ProcessedType.KEEP to events[0], + ProcessedType.KEEP to events[1] ) - events shouldBeEqualTo originalEvents } @Test - fun `given out of date message event when processing then removes message`() { - val (events) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) + fun `given out of date message event when processing then removes message event`() { + val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) outdatedDetector.givenEventIsOutOfDate(events[0]) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEventsList) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) - result shouldBeEqualTo aProcessedNotificationEvents( - roomEvents = mapOf( - "room-1" to emptyList() - ) + result shouldBeEqualTo listOf( + ProcessedType.REMOVE to events[0], ) - events shouldBeEqualTo emptyList() } @Test - fun `given in date message event when processing then without mutating`() { - val (events, originalEvents) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) + fun `given in date message event when processing then keep message event`() { + val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) outdatedDetector.givenEventIsInDate(events[0]) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEventsList) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) - result shouldBeEqualTo aProcessedNotificationEvents( - roomEvents = mapOf( - "room-1" to listOf(events[0] as NotifiableMessageEvent) - ) + result shouldBeEqualTo listOf( + ProcessedType.KEEP to events[0], ) - events shouldBeEqualTo originalEvents } @Test fun `given viewing the same room as message event when processing then removes message`() { - val (events) = createEventsList(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) + val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) - val result = eventProcessor.process(events, currentRoomId = "room-1", renderedEventsList = renderedEventsList) + val result = eventProcessor.process(events, currentRoomId = "room-1", renderedEventsList = emptyList()) - result shouldBeEqualTo aProcessedNotificationEvents( - roomEvents = mapOf( - "room-1" to emptyList() - ) + result shouldBeEqualTo listOf( + ProcessedType.REMOVE to events[0], ) - events shouldBeEqualTo emptyList() } -} -fun createEventsList(vararg event: NotifiableEvent): Pair, List> { - val mutableEvents = mutableListOf(*event) - val immutableEvents = mutableEvents.toList() - return mutableEvents to immutableEvents -} + @Test + fun `given events are different to rendered events when processing then removes difference`() { + val events = listOf(aSimpleNotifiableEvent(eventId = "event-1")) + val renderedEvents = listOf( + ProcessedType.KEEP to events[0], + ProcessedType.KEEP to anInviteNotifiableEvent(roomId = "event-2") + ) -fun aProcessedNotificationEvents(simpleEvents: Map = emptyMap(), - invitationEvents: Map = emptyMap(), - roomEvents: Map> = emptyMap() -) = GroupedNotificationEvents( - roomEvents = roomEvents, - simpleEvents = simpleEvents, - invitationEvents = invitationEvents, -) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEvents) + + result shouldBeEqualTo listOf( + ProcessedType.REMOVE to renderedEvents[1].second, + ProcessedType.KEEP to renderedEvents[0].second + ) + } +} fun aSimpleNotifiableEvent(eventId: String) = SimpleNotifiableEvent( matrixID = null, 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 fc20f098111..98684f278dd 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 @@ -46,7 +46,7 @@ class NotificationFactoryTest { @Test fun `given a room invitation when mapping to notification then is Append`() = testWith(notificationFactory) { val expectedNotification = notificationUtils.givenBuildRoomInvitationNotificationFor(AN_INVITATION_EVENT, MY_USER_ID) - val roomInvitation = mapOf(A_ROOM_ID to AN_INVITATION_EVENT) + val roomInvitation = listOf(ProcessedType.KEEP to AN_INVITATION_EVENT) val result = roomInvitation.toNotifications(MY_USER_ID) @@ -63,7 +63,7 @@ class NotificationFactoryTest { @Test fun `given a missing event in room invitation when mapping to notification then is Removed`() = testWith(notificationFactory) { - val missingEventRoomInvitation: Map = mapOf(A_ROOM_ID to null) + val missingEventRoomInvitation = listOf(ProcessedType.REMOVE to AN_INVITATION_EVENT) val result = missingEventRoomInvitation.toNotifications(MY_USER_ID) @@ -75,7 +75,7 @@ class NotificationFactoryTest { @Test fun `given a simple event when mapping to notification then is Append`() = testWith(notificationFactory) { val expectedNotification = notificationUtils.givenBuildSimpleInvitationNotificationFor(A_SIMPLE_EVENT, MY_USER_ID) - val roomInvitation = mapOf(AN_EVENT_ID to A_SIMPLE_EVENT) + val roomInvitation = listOf(ProcessedType.KEEP to A_SIMPLE_EVENT) val result = roomInvitation.toNotifications(MY_USER_ID) @@ -92,7 +92,7 @@ class NotificationFactoryTest { @Test fun `given a missing simple event when mapping to notification then is Removed`() = testWith(notificationFactory) { - val missingEventRoomInvitation: Map = mapOf(AN_EVENT_ID to null) + val missingEventRoomInvitation = listOf(ProcessedType.REMOVE to A_SIMPLE_EVENT) val result = missingEventRoomInvitation.toNotifications(MY_USER_ID) diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt index bd0d1e8d3f4..4f65c3861a5 100644 --- a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt @@ -29,8 +29,8 @@ private const val AN_EVENT_ID = "event-id" private const val A_ROOM_ID = "room-id" private const val USE_COMPLETE_NOTIFICATION_FORMAT = true -private val AN_EVENT_LIST = mapOf() -private val A_PROCESSED_EVENTS = GroupedNotificationEvents(emptyMap(), emptyMap(), emptyMap()) +private val AN_EVENT_LIST = listOf>() +private val A_PROCESSED_EVENTS = GroupedNotificationEvents(emptyMap(), emptyList(), emptyList()) private val A_SUMMARY_NOTIFICATION = SummaryNotification.Update(mockk()) private val A_REMOVE_SUMMARY_NOTIFICATION = SummaryNotification.Removed private val A_NOTIFICATION = mockk() diff --git a/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt index cc6f84f813d..a6e7d1a0786 100644 --- a/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt +++ b/vector/src/test/java/im/vector/app/test/fakes/FakeNotificationFactory.kt @@ -16,9 +16,9 @@ package im.vector.app.test.fakes +import im.vector.app.features.notifications.GroupedNotificationEvents import im.vector.app.features.notifications.NotificationFactory import im.vector.app.features.notifications.OneShotNotification -import im.vector.app.features.notifications.GroupedNotificationEvents import im.vector.app.features.notifications.RoomNotification import im.vector.app.features.notifications.SummaryNotification import io.mockk.every @@ -50,7 +50,6 @@ class FakeNotificationFactory { useCompleteNotificationFormat ) } returns summaryNotification - } } } From 28df5a3f7dcc7ba955626cf5a0f3ef808b30a187 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Mon, 11 Oct 2021 17:04:07 +0100 Subject: [PATCH 15/26] ensuring that we remove read messages when they come through by respecting the processed type when creating the notifications --- .../app/features/notifications/NotificationFactory.kt | 11 ++++++++--- .../features/notifications/NotificationRenderer.kt | 10 +++++----- .../features/notifications/NotificationFactoryTest.kt | 7 ++++--- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index fe1671b58bc..6be18371b16 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -27,16 +27,21 @@ class NotificationFactory @Inject constructor( private val summaryGroupMessageCreator: SummaryGroupMessageCreator ) { - fun Map>.toNotifications(myUserDisplayName: String, myUserAvatarUrl: String?): List { + fun Map>>.toNotifications(myUserDisplayName: String, myUserAvatarUrl: String?): List { return map { (roomId, events) -> when { events.hasNoEventsToDisplay() -> RoomNotification.Removed(roomId) - else -> roomGroupMessageCreator.createRoomMessage(events, roomId, myUserDisplayName, myUserAvatarUrl) + else -> { + val messageEvents = events.filter { it.first == ProcessedType.KEEP }.map { it.second } + roomGroupMessageCreator.createRoomMessage(messageEvents, roomId, myUserDisplayName, myUserAvatarUrl) + } } } } - private fun List.hasNoEventsToDisplay() = isEmpty() || all { it.canNotBeDisplayed() } + private fun List>.hasNoEventsToDisplay() = isEmpty() || all { + it.first == ProcessedType.REMOVE || it.second.canNotBeDisplayed() + } private fun NotifiableMessageEvent.canNotBeDisplayed() = isRedacted diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt index 7cf0a8872a3..ceeffd0bfab 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt @@ -34,7 +34,7 @@ class NotificationRenderer @Inject constructor(private val notificationDisplayer myUserDisplayName: String, myUserAvatarUrl: String?, useCompleteNotificationFormat: Boolean, - eventsToProcess: List>) { + eventsToProcess: List) { val (roomEvents, simpleEvents, invitationEvents) = eventsToProcess.groupByType() with(notificationFactory) { val roomNotifications = roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) @@ -108,8 +108,8 @@ class NotificationRenderer @Inject constructor(private val notificationDisplayer } } -private fun List>.groupByType(): GroupedNotificationEvents { - val roomIdToEventMap: MutableMap> = LinkedHashMap() +private fun List.groupByType(): GroupedNotificationEvents { + val roomIdToEventMap: MutableMap>> = LinkedHashMap() val simpleEvents: MutableList> = ArrayList() val invitationEvents: MutableList> = ArrayList() forEach { @@ -117,7 +117,7 @@ private fun List>.groupByType(): GroupedNot is InviteNotifiableEvent -> invitationEvents.add(it.asPair()) is NotifiableMessageEvent -> { val roomEvents = roomIdToEventMap.getOrPut(event.roomId) { ArrayList() } - roomEvents.add(event) + roomEvents.add(it.asPair()) } is SimpleNotifiableEvent -> simpleEvents.add(it.asPair()) } @@ -129,7 +129,7 @@ private fun List>.groupByType(): GroupedNot private fun Pair.asPair(): Pair = this as Pair data class GroupedNotificationEvents( - val roomEvents: Map>, + val roomEvents: Map>>, val simpleEvents: List>, val invitationEvents: List> ) 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 98684f278dd..84f59dcc215 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 @@ -105,7 +105,7 @@ class NotificationFactoryTest { fun `given room with message when mapping to notification then delegates to room group message creator`() = testWith(notificationFactory) { val events = listOf(A_MESSAGE_EVENT) val expectedNotification = roomGroupMessageCreator.givenCreatesRoomMessageFor(events, A_ROOM_ID, MY_USER_ID, MY_AVATAR_URL) - val roomWithMessage = mapOf(A_ROOM_ID to events) + val roomWithMessage = mapOf(A_ROOM_ID to listOf(ProcessedType.KEEP to A_MESSAGE_EVENT)) val result = roomWithMessage.toNotifications(MY_USER_ID, MY_AVATAR_URL) @@ -114,7 +114,8 @@ class NotificationFactoryTest { @Test fun `given a room with no events to display when mapping to notification then is Empty`() = testWith(notificationFactory) { - val emptyRoom: Map> = mapOf(A_ROOM_ID to emptyList()) + val events = listOf(ProcessedType.REMOVE to A_MESSAGE_EVENT) + val emptyRoom = mapOf(A_ROOM_ID to events) val result = emptyRoom.toNotifications(MY_USER_ID, MY_AVATAR_URL) @@ -125,7 +126,7 @@ class NotificationFactoryTest { @Test fun `given a room with only redacted events when mapping to notification then is Empty`() = testWith(notificationFactory) { - val redactedRoom = mapOf(A_ROOM_ID to listOf(A_MESSAGE_EVENT.copy(isRedacted = true))) + val redactedRoom = mapOf(A_ROOM_ID to listOf(ProcessedType.KEEP to A_MESSAGE_EVENT.copy(isRedacted = true))) val result = redactedRoom.toNotifications(MY_USER_ID, MY_AVATAR_URL) From 25c43226768f680bb82faf10ea904cac0da27628 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 13 Oct 2021 11:21:26 +0100 Subject: [PATCH 16/26] adding documentation around the two notifiable event lists which act as our notification source of truth --- .../notifications/NotificationDrawerManager.kt | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 b7bb20237cc..0fb97393310 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 @@ -55,7 +55,21 @@ class NotificationDrawerManager @Inject constructor(private val context: Context backgroundHandler = Handler(handlerThread.looper) } + /** + * The notifiable events to render + * this is our source of truth for notifications, any changes to this list will be rendered as notifications + * when events are removed the previously rendered notifications will be cancelled + * when adding or updating, the notifications will be notified + * + * Events are unique by their properties, we should be careful not to insert multiple events with the same event-id + */ private val eventList = loadEventInfo() + + /** + * The last known rendered notifiable events + * we keep track of them in order to know which events have been removed from the eventList + * allowing us to cancel any notifications previous displayed by now removed events + */ private var renderedEventsList = emptyList>() private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size) private var currentRoomId: String? = null From 2f8be02880723da35237446a9ee9c3651ab51d12 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 13 Oct 2021 11:26:53 +0100 Subject: [PATCH 17/26] fixing line lengths --- .../features/notifications/NotifiableEventProcessor.kt | 2 +- .../features/notifications/NotificationDrawerManager.kt | 8 ++++++-- .../app/features/notifications/NotificationFactory.kt | 4 +++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt index 88dc455e20f..1acfcfc7ec2 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt @@ -26,7 +26,7 @@ class NotifiableEventProcessor @Inject constructor( private val autoAcceptInvites: AutoAcceptInvites ) { - fun process(eventList: List, currentRoomId: String?, renderedEventsList: List>): List { + fun process(eventList: List, currentRoomId: String?, renderedEventsList: List): List { val processedEventList = eventList.map { when (it) { is InviteNotifiableEvent -> if (autoAcceptInvites.hideInvites) REMOVE else KEEP 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 0fb97393310..3db1ab1f845 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 @@ -262,8 +262,12 @@ class NotificationDrawerManager @Inject constructor(private val context: Context val user = session.getUser(session.myUserId) // myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash val myUserDisplayName = user?.toMatrixItem()?.getBestName() ?: session.myUserId - val myUserAvatarUrl = session.contentUrlResolver().resolveThumbnail(user?.avatarUrl, avatarSize, avatarSize, ContentUrlResolver.ThumbnailMethod.SCALE) - + val myUserAvatarUrl = session.contentUrlResolver().resolveThumbnail( + contentUrl = user?.avatarUrl, + width = avatarSize, + height = avatarSize, + method = ContentUrlResolver.ThumbnailMethod.SCALE + ) notificationRenderer.render(session.myUserId, myUserDisplayName, myUserAvatarUrl, useCompleteNotificationFormat, eventsToRender) } } diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index 6be18371b16..019b37d61a9 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -21,13 +21,15 @@ import androidx.core.content.pm.ShortcutInfoCompat import androidx.core.content.pm.ShortcutManagerCompat import javax.inject.Inject +private typealias ProcessedMessageEvent = Pair + class NotificationFactory @Inject constructor( private val notificationUtils: NotificationUtils, private val roomGroupMessageCreator: RoomGroupMessageCreator, private val summaryGroupMessageCreator: SummaryGroupMessageCreator ) { - fun Map>>.toNotifications(myUserDisplayName: String, myUserAvatarUrl: String?): List { + fun Map>.toNotifications(myUserDisplayName: String, myUserAvatarUrl: String?): List { return map { (roomId, events) -> when { events.hasNoEventsToDisplay() -> RoomNotification.Removed(roomId) From 672e7f9ad92c1dcaf2f458bf1efbf2622b95a16f Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 13 Oct 2021 11:30:29 +0100 Subject: [PATCH 18/26] adding missing fixture parameter from rebase --- .../app/features/notifications/NotifiableEventProcessorTest.kt | 1 + 1 file changed, 1 insertion(+) 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 1c0f5f93901..c85d01a6037 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 @@ -147,6 +147,7 @@ fun anInviteNotifiableEvent(roomId: String) = InviteNotifiableEvent( matrixID = null, eventId = "event-id", roomId = roomId, + roomName = "a room name", editedEventId = null, noisy = false, title = "title", From 92d05e7159876dc7a399a4b8018d47b5efd8da3e Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 13 Oct 2021 11:32:48 +0100 Subject: [PATCH 19/26] inlining single use extension functions --- .../app/features/notifications/NotificationFactory.kt | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index 019b37d61a9..afea56c5b93 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -86,9 +86,9 @@ class NotificationFactory @Inject constructor( invitationNotifications: List, simpleNotifications: List, useCompleteNotificationFormat: Boolean): SummaryNotification { - val roomMeta = roomNotifications.mapToMeta() - val invitationMeta = invitationNotifications.mapToMeta() - val simpleMeta = simpleNotifications.mapToMeta() + val roomMeta = roomNotifications.filterIsInstance().map { it.meta } + val invitationMeta = invitationNotifications.filterIsInstance().map { it.meta } + val simpleMeta = simpleNotifications.filterIsInstance().map { it.meta } return when { roomMeta.isEmpty() && invitationMeta.isEmpty() && simpleMeta.isEmpty() -> SummaryNotification.Removed else -> SummaryNotification.Update( @@ -102,11 +102,6 @@ class NotificationFactory @Inject constructor( } } -private fun List.mapToMeta() = filterIsInstance().map { it.meta } - -@JvmName("mapToMetaOneShotNotification") -private fun List.mapToMeta() = filterIsInstance().map { it.meta } - sealed interface RoomNotification { data class Removed(val roomId: String) : RoomNotification data class Message(val notification: Notification, val shortcutInfo: ShortcutInfoCompat?, val meta: Meta) : RoomNotification { From 75e45d1082a711a43d4241d8859902abd6662b8c Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Oct 2021 16:56:15 +0100 Subject: [PATCH 20/26] adding missing parameter from rebase and removing no longer needed singleton annotation --- .../app/features/notifications/NotificationRenderer.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt index ceeffd0bfab..ea54fdaf92b 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt @@ -15,6 +15,7 @@ */ package im.vector.app.features.notifications +import android.content.Context import androidx.annotation.WorkerThread import im.vector.app.features.notifications.NotificationDrawerManager.Companion.ROOM_EVENT_NOTIFICATION_ID import im.vector.app.features.notifications.NotificationDrawerManager.Companion.ROOM_INVITATION_NOTIFICATION_ID @@ -23,11 +24,10 @@ import im.vector.app.features.notifications.NotificationDrawerManager.Companion. import androidx.core.content.pm.ShortcutManagerCompat import timber.log.Timber import javax.inject.Inject -import javax.inject.Singleton -@Singleton class NotificationRenderer @Inject constructor(private val notificationDisplayer: NotificationDisplayer, - private val notificationFactory: NotificationFactory) { + private val notificationFactory: NotificationFactory, + private val appContext: Context) { @WorkerThread fun render(myUserId: String, From 502993749528b4801a56b2bca0deff941c1b246d Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Oct 2021 16:58:31 +0100 Subject: [PATCH 21/26] replacing notification utils usage with the displayer and removing unused method --- .../features/notifications/NotificationDrawerManager.kt | 8 ++------ 1 file changed, 2 insertions(+), 6 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 3db1ab1f845..f324318988e 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 @@ -41,7 +41,7 @@ import javax.inject.Singleton */ @Singleton class NotificationDrawerManager @Inject constructor(private val context: Context, - private val notificationUtils: NotificationUtils, + private val notificationDisplayer: NotificationDisplayer, private val vectorPreferences: VectorPreferences, private val activeSessionDataSource: ActiveSessionDataSource, private val notifiableEventProcessor: NotifiableEventProcessor, @@ -243,7 +243,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context val newSettings = vectorPreferences.useCompleteNotificationFormat() if (newSettings != useCompleteNotificationFormat) { // Settings has changed, remove all current notifications - notificationUtils.cancelAllNotifications() + notificationDisplayer.cancelAllNotifications() useCompleteNotificationFormat = newSettings } @@ -318,10 +318,6 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } } - fun displayDiagnosticNotification() { - notificationUtils.displayDiagnosticNotification() - } - companion object { const val SUMMARY_NOTIFICATION_ID = 0 const val ROOM_MESSAGES_NOTIFICATION_ID = 1 From 60a5bb96c701e75be8a4274e07f5144fb7457ddf Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Oct 2021 16:59:32 +0100 Subject: [PATCH 22/26] removing unused imports --- .../im/vector/app/features/notifications/NotificationFactory.kt | 1 - .../vector/app/features/notifications/NotificationRenderer.kt | 2 +- .../app/features/notifications/RoomGroupMessageCreator.kt | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index afea56c5b93..288268a00c9 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -18,7 +18,6 @@ package im.vector.app.features.notifications import android.app.Notification import androidx.core.content.pm.ShortcutInfoCompat -import androidx.core.content.pm.ShortcutManagerCompat import javax.inject.Inject private typealias ProcessedMessageEvent = Pair diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt index ea54fdaf92b..e72b9c7110a 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt @@ -17,11 +17,11 @@ package im.vector.app.features.notifications import android.content.Context import androidx.annotation.WorkerThread +import androidx.core.content.pm.ShortcutManagerCompat import im.vector.app.features.notifications.NotificationDrawerManager.Companion.ROOM_EVENT_NOTIFICATION_ID import im.vector.app.features.notifications.NotificationDrawerManager.Companion.ROOM_INVITATION_NOTIFICATION_ID import im.vector.app.features.notifications.NotificationDrawerManager.Companion.ROOM_MESSAGES_NOTIFICATION_ID import im.vector.app.features.notifications.NotificationDrawerManager.Companion.SUMMARY_NOTIFICATION_ID -import androidx.core.content.pm.ShortcutManagerCompat import timber.log.Timber import javax.inject.Inject 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 113dc21ebd4..35a1c12d0ce 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 @@ -22,7 +22,6 @@ import android.os.Build import androidx.core.app.NotificationCompat import androidx.core.app.Person import androidx.core.content.pm.ShortcutInfoCompat -import androidx.core.content.pm.ShortcutManagerCompat import androidx.core.graphics.drawable.IconCompat import im.vector.app.R import im.vector.app.core.resources.StringProvider From 63933112ecaa12f1ff080f77a66fee6e043752d5 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Oct 2021 17:19:14 +0100 Subject: [PATCH 23/26] updating tests with shortcut placement changes --- .../app/features/notifications/NotificationRendererTest.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt index 4f65c3861a5..c086a3ee4db 100644 --- a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt @@ -17,6 +17,7 @@ package im.vector.app.features.notifications import android.app.Notification +import im.vector.app.test.fakes.FakeContext import im.vector.app.test.fakes.FakeNotificationDisplayer import im.vector.app.test.fakes.FakeNotificationFactory import io.mockk.mockk @@ -41,12 +42,14 @@ private val ONE_SHOT_META = OneShotNotification.Append.Meta(key = "ignored", sum class NotificationRendererTest { + private val context = FakeContext() private val notificationDisplayer = FakeNotificationDisplayer() private val notificationFactory = FakeNotificationFactory() private val notificationRenderer = NotificationRenderer( notificationDisplayer = notificationDisplayer.instance, - notificationFactory = notificationFactory.instance + notificationFactory = notificationFactory.instance, + appContext = context.instance ) @Test @@ -87,6 +90,7 @@ class NotificationRendererTest { fun `given a room message group notification is added when rendering then show the message notification and update summary`() { givenNotifications(roomNotifications = listOf(RoomNotification.Message( A_NOTIFICATION, + shortcutInfo = null, MESSAGE_META ))) From 63bbc715afad96911c4d3bfe217e0f3e6b79003c Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 14 Oct 2021 17:42:47 +0100 Subject: [PATCH 24/26] increase enum class allowance by 1 --- tools/check/forbidden_strings_in_code.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/check/forbidden_strings_in_code.txt b/tools/check/forbidden_strings_in_code.txt index 8f8625fe1cf..29077c3a76a 100644 --- a/tools/check/forbidden_strings_in_code.txt +++ b/tools/check/forbidden_strings_in_code.txt @@ -160,7 +160,7 @@ Formatter\.formatShortFileSize===1 # android\.text\.TextUtils ### This is not a rule, but a warning: the number of "enum class" has changed. For Json classes, it is mandatory that they have `@JsonClass(generateAdapter = false)`. If the enum is not used as a Json class, change the value in file forbidden_strings_in_code.txt -enum class===106 +enum class===107 ### Do not import temporary legacy classes import org.matrix.android.sdk.internal.legacy.riot===3 From f5de7f7ebe1bf00aee9a9611e1f965d4836ab9ed Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Tue, 19 Oct 2021 11:38:34 +0100 Subject: [PATCH 25/26] using dedicated ProcessedEvent data class instead of type alias for passing around the process notificatiable events - also includes @JvmName on all conflicting extensions for consistency --- .../notifications/NotifiableEventProcessor.kt | 17 ++++--- .../NotificationDrawerManager.kt | 2 +- .../notifications/NotificationFactory.kt | 23 ++++----- .../notifications/NotificationRenderer.kt | 26 +++++----- .../features/notifications/ProcessedEvent.kt | 14 ++++-- .../NotifiableEventProcessorTest.kt | 47 ++++++++++--------- .../notifications/NotificationFactoryTest.kt | 15 +++--- .../notifications/NotificationRendererTest.kt | 2 +- 8 files changed, 80 insertions(+), 66 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt index 1acfcfc7ec2..338c3d58ebf 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt @@ -17,29 +17,32 @@ package im.vector.app.features.notifications import im.vector.app.features.invite.AutoAcceptInvites -import im.vector.app.features.notifications.ProcessedType.KEEP -import im.vector.app.features.notifications.ProcessedType.REMOVE +import im.vector.app.features.notifications.ProcessedEvent.Type.KEEP +import im.vector.app.features.notifications.ProcessedEvent.Type.REMOVE import javax.inject.Inject +private typealias ProcessedEvents = List> + class NotifiableEventProcessor @Inject constructor( private val outdatedDetector: OutdatedEventDetector, private val autoAcceptInvites: AutoAcceptInvites ) { - fun process(eventList: List, currentRoomId: String?, renderedEventsList: List): List { + fun process(eventList: List, currentRoomId: String?, renderedEventsList: ProcessedEvents): ProcessedEvents { val processedEventList = eventList.map { - when (it) { + val type = when (it) { is InviteNotifiableEvent -> if (autoAcceptInvites.hideInvites) REMOVE else KEEP is NotifiableMessageEvent -> if (shouldIgnoreMessageEventInRoom(currentRoomId, it.roomId) || outdatedDetector.isMessageOutdated(it)) { REMOVE } else KEEP is SimpleNotifiableEvent -> KEEP - } to it + } + ProcessedEvent(type, it) } val removedEventsDiff = renderedEventsList.filter { renderedEvent -> - eventList.none { it.eventId == renderedEvent.second.eventId } - }.map { REMOVE to it.second } + eventList.none { it.eventId == renderedEvent.event.eventId } + }.map { ProcessedEvent(REMOVE, it.event) } return removedEventsDiff + processedEventList } 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 f324318988e..5d2212ba1ee 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 @@ -70,7 +70,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context * we keep track of them in order to know which events have been removed from the eventList * allowing us to cancel any notifications previous displayed by now removed events */ - private var renderedEventsList = emptyList>() + private var renderedEventsList = emptyList>() private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size) private var currentRoomId: String? = null diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt index 288268a00c9..5dff009cec9 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationFactory.kt @@ -20,7 +20,7 @@ import android.app.Notification import androidx.core.content.pm.ShortcutInfoCompat import javax.inject.Inject -private typealias ProcessedMessageEvent = Pair +private typealias ProcessedMessageEvents = List> class NotificationFactory @Inject constructor( private val notificationUtils: NotificationUtils, @@ -28,29 +28,30 @@ class NotificationFactory @Inject constructor( private val summaryGroupMessageCreator: SummaryGroupMessageCreator ) { - fun Map>.toNotifications(myUserDisplayName: String, myUserAvatarUrl: String?): List { + fun Map.toNotifications(myUserDisplayName: String, myUserAvatarUrl: String?): List { return map { (roomId, events) -> when { events.hasNoEventsToDisplay() -> RoomNotification.Removed(roomId) else -> { - val messageEvents = events.filter { it.first == ProcessedType.KEEP }.map { it.second } + val messageEvents = events.onlyKeptEvents() roomGroupMessageCreator.createRoomMessage(messageEvents, roomId, myUserDisplayName, myUserAvatarUrl) } } } } - private fun List>.hasNoEventsToDisplay() = isEmpty() || all { - it.first == ProcessedType.REMOVE || it.second.canNotBeDisplayed() + private fun ProcessedMessageEvents.hasNoEventsToDisplay() = isEmpty() || all { + it.type == ProcessedEvent.Type.REMOVE || it.event.canNotBeDisplayed() } private fun NotifiableMessageEvent.canNotBeDisplayed() = isRedacted - fun List>.toNotifications(myUserId: String): List { + @JvmName("toNotificationsInviteNotifiableEvent") + fun List>.toNotifications(myUserId: String): List { return map { (processed, event) -> when (processed) { - ProcessedType.REMOVE -> OneShotNotification.Removed(key = event.roomId) - ProcessedType.KEEP -> OneShotNotification.Append( + ProcessedEvent.Type.REMOVE -> OneShotNotification.Removed(key = event.roomId) + ProcessedEvent.Type.KEEP -> OneShotNotification.Append( notificationUtils.buildRoomInvitationNotification(event, myUserId), OneShotNotification.Append.Meta( key = event.roomId, @@ -64,11 +65,11 @@ class NotificationFactory @Inject constructor( } @JvmName("toNotificationsSimpleNotifiableEvent") - fun List>.toNotifications(myUserId: String): List { + fun List>.toNotifications(myUserId: String): List { return map { (processed, event) -> when (processed) { - ProcessedType.REMOVE -> OneShotNotification.Removed(key = event.eventId) - ProcessedType.KEEP -> OneShotNotification.Append( + ProcessedEvent.Type.REMOVE -> OneShotNotification.Removed(key = event.eventId) + ProcessedEvent.Type.KEEP -> OneShotNotification.Append( notificationUtils.buildSimpleEventNotification(event, myUserId), OneShotNotification.Append.Meta( key = event.eventId, diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt index e72b9c7110a..5afff894029 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt @@ -34,7 +34,7 @@ class NotificationRenderer @Inject constructor(private val notificationDisplayer myUserDisplayName: String, myUserAvatarUrl: String?, useCompleteNotificationFormat: Boolean, - eventsToProcess: List) { + eventsToProcess: List>) { val (roomEvents, simpleEvents, invitationEvents) = eventsToProcess.groupByType() with(notificationFactory) { val roomNotifications = roomEvents.toNotifications(myUserDisplayName, myUserAvatarUrl) @@ -108,28 +108,28 @@ class NotificationRenderer @Inject constructor(private val notificationDisplayer } } -private fun List.groupByType(): GroupedNotificationEvents { - val roomIdToEventMap: MutableMap>> = LinkedHashMap() - val simpleEvents: MutableList> = ArrayList() - val invitationEvents: MutableList> = ArrayList() +private fun List>.groupByType(): GroupedNotificationEvents { + val roomIdToEventMap: MutableMap>> = LinkedHashMap() + val simpleEvents: MutableList> = ArrayList() + val invitationEvents: MutableList> = ArrayList() forEach { - when (val event = it.second) { - is InviteNotifiableEvent -> invitationEvents.add(it.asPair()) + when (val event = it.event) { + is InviteNotifiableEvent -> invitationEvents.add(it.castedToEventType()) is NotifiableMessageEvent -> { val roomEvents = roomIdToEventMap.getOrPut(event.roomId) { ArrayList() } - roomEvents.add(it.asPair()) + roomEvents.add(it.castedToEventType()) } - is SimpleNotifiableEvent -> simpleEvents.add(it.asPair()) + is SimpleNotifiableEvent -> simpleEvents.add(it.castedToEventType()) } } return GroupedNotificationEvents(roomIdToEventMap, simpleEvents, invitationEvents) } @Suppress("UNCHECKED_CAST") -private fun Pair.asPair(): Pair = this as Pair +private fun ProcessedEvent.castedToEventType(): ProcessedEvent = this as ProcessedEvent data class GroupedNotificationEvents( - val roomEvents: Map>>, - val simpleEvents: List>, - val invitationEvents: List> + val roomEvents: Map>>, + val simpleEvents: List>, + val invitationEvents: List> ) diff --git a/vector/src/main/java/im/vector/app/features/notifications/ProcessedEvent.kt b/vector/src/main/java/im/vector/app/features/notifications/ProcessedEvent.kt index 0901757d020..7c58c81f46a 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/ProcessedEvent.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/ProcessedEvent.kt @@ -16,11 +16,15 @@ package im.vector.app.features.notifications -typealias ProcessedEvent = Pair +data class ProcessedEvent( + val type: Type, + val event: T +) { -enum class ProcessedType { - KEEP, - REMOVE + enum class Type { + KEEP, + REMOVE + } } -fun List.onlyKeptEvents() = filter { it.first == ProcessedType.KEEP }.map { it.second } +fun List>.onlyKeptEvents() = filter { it.type == ProcessedEvent.Type.KEEP }.map { it.event } 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 c85d01a6037..342567753cd 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 @@ -16,6 +16,7 @@ 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 org.amshove.kluent.shouldBeEqualTo @@ -39,9 +40,9 @@ class NotifiableEventProcessorTest { val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) - result shouldBeEqualTo listOf( - ProcessedType.KEEP to events[0], - ProcessedType.KEEP to events[1] + result shouldBeEqualTo listOfProcessedEvents( + Type.KEEP to events[0], + Type.KEEP to events[1] ) } @@ -55,9 +56,9 @@ class NotifiableEventProcessorTest { val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) - result shouldBeEqualTo listOf( - ProcessedType.REMOVE to events[0], - ProcessedType.REMOVE to events[1] + result shouldBeEqualTo listOfProcessedEvents( + Type.REMOVE to events[0], + Type.REMOVE to events[1] ) } @@ -71,9 +72,9 @@ class NotifiableEventProcessorTest { val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) - result shouldBeEqualTo listOf( - ProcessedType.KEEP to events[0], - ProcessedType.KEEP to events[1] + result shouldBeEqualTo listOfProcessedEvents( + Type.KEEP to events[0], + Type.KEEP to events[1] ) } @@ -84,8 +85,8 @@ class NotifiableEventProcessorTest { val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) - result shouldBeEqualTo listOf( - ProcessedType.REMOVE to events[0], + result shouldBeEqualTo listOfProcessedEvents( + Type.REMOVE to events[0], ) } @@ -96,8 +97,8 @@ class NotifiableEventProcessorTest { val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) - result shouldBeEqualTo listOf( - ProcessedType.KEEP to events[0], + result shouldBeEqualTo listOfProcessedEvents( + Type.KEEP to events[0], ) } @@ -107,26 +108,30 @@ class NotifiableEventProcessorTest { val result = eventProcessor.process(events, currentRoomId = "room-1", renderedEventsList = emptyList()) - result shouldBeEqualTo listOf( - ProcessedType.REMOVE to events[0], + result shouldBeEqualTo listOfProcessedEvents( + Type.REMOVE to events[0], ) } @Test fun `given events are different to rendered events when processing then removes difference`() { val events = listOf(aSimpleNotifiableEvent(eventId = "event-1")) - val renderedEvents = listOf( - ProcessedType.KEEP to events[0], - ProcessedType.KEEP to anInviteNotifiableEvent(roomId = "event-2") + val renderedEvents = listOf>( + ProcessedEvent(Type.KEEP, events[0]), + ProcessedEvent(Type.KEEP, anInviteNotifiableEvent(roomId = "event-2")) ) val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEvents) - result shouldBeEqualTo listOf( - ProcessedType.REMOVE to renderedEvents[1].second, - ProcessedType.KEEP to renderedEvents[0].second + result shouldBeEqualTo listOfProcessedEvents( + Type.REMOVE to renderedEvents[1].event, + Type.KEEP to renderedEvents[0].event ) } + + private fun listOfProcessedEvents(vararg event: Pair) = event.map { + ProcessedEvent(it.first, it.second) + } } fun aSimpleNotifiableEvent(eventId: String) = SimpleNotifiableEvent( 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 84f59dcc215..d3d48630c90 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 @@ -16,6 +16,7 @@ package im.vector.app.features.notifications +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 @@ -46,7 +47,7 @@ class NotificationFactoryTest { @Test fun `given a room invitation when mapping to notification then is Append`() = testWith(notificationFactory) { val expectedNotification = notificationUtils.givenBuildRoomInvitationNotificationFor(AN_INVITATION_EVENT, MY_USER_ID) - val roomInvitation = listOf(ProcessedType.KEEP to AN_INVITATION_EVENT) + val roomInvitation = listOf(ProcessedEvent(Type.KEEP, AN_INVITATION_EVENT)) val result = roomInvitation.toNotifications(MY_USER_ID) @@ -63,7 +64,7 @@ class NotificationFactoryTest { @Test fun `given a missing event in room invitation when mapping to notification then is Removed`() = testWith(notificationFactory) { - val missingEventRoomInvitation = listOf(ProcessedType.REMOVE to AN_INVITATION_EVENT) + val missingEventRoomInvitation = listOf(ProcessedEvent(Type.REMOVE, AN_INVITATION_EVENT)) val result = missingEventRoomInvitation.toNotifications(MY_USER_ID) @@ -75,7 +76,7 @@ class NotificationFactoryTest { @Test fun `given a simple event when mapping to notification then is Append`() = testWith(notificationFactory) { val expectedNotification = notificationUtils.givenBuildSimpleInvitationNotificationFor(A_SIMPLE_EVENT, MY_USER_ID) - val roomInvitation = listOf(ProcessedType.KEEP to A_SIMPLE_EVENT) + val roomInvitation = listOf(ProcessedEvent(Type.KEEP, A_SIMPLE_EVENT)) val result = roomInvitation.toNotifications(MY_USER_ID) @@ -92,7 +93,7 @@ class NotificationFactoryTest { @Test fun `given a missing simple event when mapping to notification then is Removed`() = testWith(notificationFactory) { - val missingEventRoomInvitation = listOf(ProcessedType.REMOVE to A_SIMPLE_EVENT) + val missingEventRoomInvitation = listOf(ProcessedEvent(Type.REMOVE, A_SIMPLE_EVENT)) val result = missingEventRoomInvitation.toNotifications(MY_USER_ID) @@ -105,7 +106,7 @@ class NotificationFactoryTest { fun `given room with message when mapping to notification then delegates to room group message creator`() = testWith(notificationFactory) { val events = listOf(A_MESSAGE_EVENT) val expectedNotification = roomGroupMessageCreator.givenCreatesRoomMessageFor(events, A_ROOM_ID, MY_USER_ID, MY_AVATAR_URL) - val roomWithMessage = mapOf(A_ROOM_ID to listOf(ProcessedType.KEEP to A_MESSAGE_EVENT)) + val roomWithMessage = mapOf(A_ROOM_ID to listOf(ProcessedEvent(Type.KEEP, A_MESSAGE_EVENT))) val result = roomWithMessage.toNotifications(MY_USER_ID, MY_AVATAR_URL) @@ -114,7 +115,7 @@ class NotificationFactoryTest { @Test fun `given a room with no events to display when mapping to notification then is Empty`() = testWith(notificationFactory) { - val events = listOf(ProcessedType.REMOVE to A_MESSAGE_EVENT) + val events = listOf(ProcessedEvent(Type.REMOVE, A_MESSAGE_EVENT)) val emptyRoom = mapOf(A_ROOM_ID to events) val result = emptyRoom.toNotifications(MY_USER_ID, MY_AVATAR_URL) @@ -126,7 +127,7 @@ class NotificationFactoryTest { @Test fun `given a room with only redacted events when mapping to notification then is Empty`() = testWith(notificationFactory) { - val redactedRoom = mapOf(A_ROOM_ID to listOf(ProcessedType.KEEP to A_MESSAGE_EVENT.copy(isRedacted = true))) + val redactedRoom = mapOf(A_ROOM_ID to listOf(ProcessedEvent(Type.KEEP, A_MESSAGE_EVENT.copy(isRedacted = true)))) val result = redactedRoom.toNotifications(MY_USER_ID, MY_AVATAR_URL) diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt index c086a3ee4db..f726ff1b544 100644 --- a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt @@ -30,7 +30,7 @@ private const val AN_EVENT_ID = "event-id" private const val A_ROOM_ID = "room-id" private const val USE_COMPLETE_NOTIFICATION_FORMAT = true -private val AN_EVENT_LIST = listOf>() +private val AN_EVENT_LIST = listOf>() private val A_PROCESSED_EVENTS = GroupedNotificationEvents(emptyMap(), emptyList(), emptyList()) private val A_SUMMARY_NOTIFICATION = SummaryNotification.Update(mockk()) private val A_REMOVE_SUMMARY_NOTIFICATION = SummaryNotification.Removed From 0fe1b69a61c5a29f8f45dfb67276eaec75e546a4 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Wed, 20 Oct 2021 09:42:19 +0100 Subject: [PATCH 26/26] renaming event lists to give more context and remove the list suffix/inconsistencies --- .../notifications/NotifiableEventProcessor.kt | 10 ++-- .../NotificationDrawerManager.kt | 52 +++++++++---------- .../NotifiableEventProcessorTest.kt | 14 ++--- 3 files changed, 38 insertions(+), 38 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt index 338c3d58ebf..858df81bf69 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt @@ -28,8 +28,8 @@ class NotifiableEventProcessor @Inject constructor( private val autoAcceptInvites: AutoAcceptInvites ) { - fun process(eventList: List, currentRoomId: String?, renderedEventsList: ProcessedEvents): ProcessedEvents { - val processedEventList = eventList.map { + fun process(queuedEvents: List, currentRoomId: String?, renderedEvents: ProcessedEvents): ProcessedEvents { + val processedEvents = queuedEvents.map { val type = when (it) { is InviteNotifiableEvent -> if (autoAcceptInvites.hideInvites) REMOVE else KEEP is NotifiableMessageEvent -> if (shouldIgnoreMessageEventInRoom(currentRoomId, it.roomId) || outdatedDetector.isMessageOutdated(it)) { @@ -40,11 +40,11 @@ class NotifiableEventProcessor @Inject constructor( ProcessedEvent(type, it) } - val removedEventsDiff = renderedEventsList.filter { renderedEvent -> - eventList.none { it.eventId == renderedEvent.event.eventId } + val removedEventsDiff = renderedEvents.filter { renderedEvent -> + queuedEvents.none { it.eventId == renderedEvent.event.eventId } }.map { ProcessedEvent(REMOVE, it.event) } - return removedEventsDiff + processedEventList + return removedEventsDiff + processedEvents } private fun shouldIgnoreMessageEventInRoom(currentRoomId: String?, roomId: String?): Boolean { 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 5d2212ba1ee..c052de650e2 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 @@ -63,14 +63,14 @@ class NotificationDrawerManager @Inject constructor(private val context: Context * * Events are unique by their properties, we should be careful not to insert multiple events with the same event-id */ - private val eventList = loadEventInfo() + private val queuedEvents = loadEventInfo() /** * The last known rendered notifiable events * we keep track of them in order to know which events have been removed from the eventList * allowing us to cancel any notifications previous displayed by now removed events */ - private var renderedEventsList = emptyList>() + private var renderedEvents = emptyList>() private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size) private var currentRoomId: String? = null @@ -105,8 +105,8 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } else { Timber.d("onNotifiableEventReceived(): is push: ${notifiableEvent.canBeReplaced}") } - synchronized(eventList) { - val existing = eventList.firstOrNull { it.eventId == notifiableEvent.eventId } + synchronized(queuedEvents) { + val existing = queuedEvents.firstOrNull { it.eventId == notifiableEvent.eventId } if (existing != null) { if (existing.canBeReplaced) { // Use the event coming from the event stream as it may contains more info than @@ -117,8 +117,8 @@ 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 - eventList.remove(existing) - eventList.add(notifiableEvent) + queuedEvents.remove(existing) + queuedEvents.add(notifiableEvent) } else { // keep the existing one, do not replace } @@ -126,7 +126,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context // Check if this is an edit if (notifiableEvent.editedEventId != null) { // This is an edition - val eventBeforeEdition = eventList.firstOrNull { + val eventBeforeEdition = queuedEvents.firstOrNull { // Edition of an event it.eventId == notifiableEvent.editedEventId || // or edition of an edition @@ -135,9 +135,9 @@ class NotificationDrawerManager @Inject constructor(private val context: Context if (eventBeforeEdition != null) { // Replace the existing notification with the new content - eventList.remove(eventBeforeEdition) + queuedEvents.remove(eventBeforeEdition) - eventList.add(notifiableEvent) + queuedEvents.add(notifiableEvent) } else { // Ignore an edit of a not displayed event in the notification drawer } @@ -148,7 +148,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context Timber.d("onNotifiableEventReceived(): skipping event, already seen") } else { seenEventIds.put(notifiableEvent.eventId) - eventList.add(notifiableEvent) + queuedEvents.add(notifiableEvent) } } } @@ -156,8 +156,8 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } fun onEventRedacted(eventId: String) { - synchronized(eventList) { - eventList.replace(eventId) { + synchronized(queuedEvents) { + queuedEvents.replace(eventId) { when (it) { is InviteNotifiableEvent -> it.copy(isRedacted = true) is NotifiableMessageEvent -> it.copy(isRedacted = true) @@ -171,8 +171,8 @@ class NotificationDrawerManager @Inject constructor(private val context: Context * Clear all known events and refresh the notification drawer */ fun clearAllEvents() { - synchronized(eventList) { - eventList.clear() + synchronized(queuedEvents) { + queuedEvents.clear() } refreshNotificationDrawer() } @@ -194,7 +194,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context */ fun setCurrentRoom(roomId: String?) { var hasChanged: Boolean - synchronized(eventList) { + synchronized(queuedEvents) { hasChanged = roomId != currentRoomId currentRoomId = roomId } @@ -211,8 +211,8 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } private fun removeAll(predicate: (NotifiableEvent) -> Boolean): Boolean { - return synchronized(eventList) { - eventList.removeAll(predicate) + return synchronized(queuedEvents) { + queuedEvents.removeAll(predicate) } } @@ -247,17 +247,17 @@ class NotificationDrawerManager @Inject constructor(private val context: Context useCompleteNotificationFormat = newSettings } - val eventsToRender = synchronized(eventList) { - notifiableEventProcessor.process(eventList, currentRoomId, renderedEventsList).also { - eventList.clear() - eventList.addAll(it.onlyKeptEvents()) + val eventsToRender = synchronized(queuedEvents) { + notifiableEventProcessor.process(queuedEvents, currentRoomId, renderedEvents).also { + queuedEvents.clear() + queuedEvents.addAll(it.onlyKeptEvents()) } } - if (renderedEventsList == eventsToRender) { + if (renderedEvents == eventsToRender) { Timber.d("Skipping notification update due to event list not changing") } else { - renderedEventsList = eventsToRender + renderedEvents = eventsToRender val session = currentSession ?: return val user = session.getUser(session.myUserId) // myUserDisplayName cannot be empty else NotificationCompat.MessagingStyle() will crash @@ -277,8 +277,8 @@ class NotificationDrawerManager @Inject constructor(private val context: Context } fun persistInfo() { - synchronized(eventList) { - if (eventList.isEmpty()) { + synchronized(queuedEvents) { + if (queuedEvents.isEmpty()) { deleteCachedRoomNotifications() return } @@ -286,7 +286,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(eventList, KEY_ALIAS_SECRET_STORAGE, it) + currentSession?.securelyStoreObject(queuedEvents, KEY_ALIAS_SECRET_STORAGE, it) } } catch (e: Throwable) { Timber.e(e, "## Failed to save cached notification info") 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 342567753cd..f6938cb4aee 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 @@ -38,7 +38,7 @@ class NotifiableEventProcessorTest { aSimpleNotifiableEvent(eventId = "event-2") ) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = emptyList()) result shouldBeEqualTo listOfProcessedEvents( Type.KEEP to events[0], @@ -54,7 +54,7 @@ class NotifiableEventProcessorTest { anInviteNotifiableEvent(roomId = "room-2") ) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = emptyList()) result shouldBeEqualTo listOfProcessedEvents( Type.REMOVE to events[0], @@ -70,7 +70,7 @@ class NotifiableEventProcessorTest { anInviteNotifiableEvent(roomId = "room-2") ) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = emptyList()) result shouldBeEqualTo listOfProcessedEvents( Type.KEEP to events[0], @@ -83,7 +83,7 @@ class NotifiableEventProcessorTest { val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) outdatedDetector.givenEventIsOutOfDate(events[0]) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = emptyList()) result shouldBeEqualTo listOfProcessedEvents( Type.REMOVE to events[0], @@ -95,7 +95,7 @@ class NotifiableEventProcessorTest { val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) outdatedDetector.givenEventIsInDate(events[0]) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = emptyList()) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = emptyList()) result shouldBeEqualTo listOfProcessedEvents( Type.KEEP to events[0], @@ -106,7 +106,7 @@ class NotifiableEventProcessorTest { fun `given viewing the same room as message event when processing then removes message`() { val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1")) - val result = eventProcessor.process(events, currentRoomId = "room-1", renderedEventsList = emptyList()) + val result = eventProcessor.process(events, currentRoomId = "room-1", renderedEvents = emptyList()) result shouldBeEqualTo listOfProcessedEvents( Type.REMOVE to events[0], @@ -121,7 +121,7 @@ class NotifiableEventProcessorTest { ProcessedEvent(Type.KEEP, anInviteNotifiableEvent(roomId = "event-2")) ) - val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEventsList = renderedEvents) + val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = renderedEvents) result shouldBeEqualTo listOfProcessedEvents( Type.REMOVE to renderedEvents[1].event,