diffing the notification events against the currently rendered events allow us to dismiss notifications from removed events

This commit is contained in:
Adam Brown 2021-10-11 16:39:52 +01:00
parent b27fb264fc
commit 0bdc65b47f
9 changed files with 86 additions and 107 deletions

View file

@ -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<NotifiableEvent>, currentRoomId: String?): List<Pair<Processed, NotifiableEvent>> {
return eventList.map {
fun process(eventList: List<NotifiableEvent>, currentRoomId: String?, renderedEventsList: List<Pair<ProcessedType, NotifiableEvent>>): List<ProcessedEvent> {
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<Pair<Processed, NotifiableEvent>>.onlyKeptEvents() = filter { it.first == KEEP }.map { it.second }

View file

@ -56,7 +56,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
}
private val eventList = loadEventInfo()
private var renderedEventsList = emptyList<Pair<Processed, NotifiableEvent>>()
private var renderedEventsList = emptyList<Pair<ProcessedType, NotifiableEvent>>()
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 {

View file

@ -40,11 +40,11 @@ class NotificationFactory @Inject constructor(
private fun NotifiableMessageEvent.canNotBeDisplayed() = isRedacted
fun List<Pair<Processed, InviteNotifiableEvent>>.toNotifications(myUserId: String): List<OneShotNotification> {
fun List<Pair<ProcessedType, InviteNotifiableEvent>>.toNotifications(myUserId: String): List<OneShotNotification> {
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<Pair<Processed, SimpleNotifiableEvent>>.toNotifications(myUserId: String): List<OneShotNotification> {
fun List<Pair<ProcessedType, SimpleNotifiableEvent>>.toNotifications(myUserId: String): List<OneShotNotification> {
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,

View file

@ -34,7 +34,7 @@ class NotificationRenderer @Inject constructor(private val notificationDisplayer
myUserDisplayName: String,
myUserAvatarUrl: String?,
useCompleteNotificationFormat: Boolean,
eventsToProcess: List<Pair<Processed, NotifiableEvent>>) {
eventsToProcess: List<Pair<ProcessedType, NotifiableEvent>>) {
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<Pair<Processed, NotifiableEvent>>.groupByType(): GroupedNotificationEvents {
private fun List<Pair<ProcessedType, NotifiableEvent>>.groupByType(): GroupedNotificationEvents {
val roomIdToEventMap: MutableMap<String, MutableList<NotifiableMessageEvent>> = LinkedHashMap()
val simpleEvents: MutableList<Pair<Processed, SimpleNotifiableEvent>> = ArrayList()
val invitationEvents: MutableList<Pair<Processed, InviteNotifiableEvent>> = ArrayList()
val simpleEvents: MutableList<Pair<ProcessedType, SimpleNotifiableEvent>> = ArrayList()
val invitationEvents: MutableList<Pair<ProcessedType, InviteNotifiableEvent>> = ArrayList()
forEach {
when (val event = it.second) {
is InviteNotifiableEvent -> invitationEvents.add(it.asPair())
@ -126,10 +126,10 @@ private fun List<Pair<Processed, NotifiableEvent>>.groupByType(): GroupedNotific
}
@Suppress("UNCHECKED_CAST")
private fun <T: NotifiableEvent> Pair<Processed, *>.asPair(): Pair<Processed, T> = this as Pair<Processed, T>
private fun <T : NotifiableEvent> Pair<ProcessedType, *>.asPair(): Pair<ProcessedType, T> = this as Pair<ProcessedType, T>
data class GroupedNotificationEvents(
val roomEvents: Map<String, List<NotifiableMessageEvent>>,
val simpleEvents: List<Pair<Processed, SimpleNotifiableEvent>>,
val invitationEvents: List<Pair<Processed, InviteNotifiableEvent>>
val simpleEvents: List<Pair<ProcessedType, SimpleNotifiableEvent>>,
val invitationEvents: List<Pair<ProcessedType, InviteNotifiableEvent>>
)

View file

@ -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<NotifiableEventProcessor>()
typealias ProcessedEvent = Pair<ProcessedType, NotifiableEvent>
enum class ProcessedType {
KEEP,
REMOVE
}
fun List<ProcessedEvent>.onlyKeptEvents() = filter { it.first == ProcessedType.KEEP }.map { it.second }

View file

@ -31,121 +31,104 @@ 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<NotifiableEvent>(
val events = listOf<NotifiableEvent>(
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],
)
}
@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 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
)
events shouldBeEqualTo emptyList()
}
}
fun createEventsList(vararg event: NotifiableEvent): Pair<MutableList<NotifiableEvent>, List<NotifiableEvent>> {
val mutableEvents = mutableListOf(*event)
val immutableEvents = mutableEvents.toList()
return mutableEvents to immutableEvents
}
fun aProcessedNotificationEvents(simpleEvents: Map<String, SimpleNotifiableEvent?> = emptyMap(),
invitationEvents: Map<String, InviteNotifiableEvent?> = emptyMap(),
roomEvents: Map<String, List<NotifiableMessageEvent>> = emptyMap()
) = GroupedNotificationEvents(
roomEvents = roomEvents,
simpleEvents = simpleEvents,
invitationEvents = invitationEvents,
)
fun aSimpleNotifiableEvent(eventId: String) = SimpleNotifiableEvent(
matrixID = null,
eventId = eventId,

View file

@ -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<String, InviteNotifiableEvent?> = 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<String, SimpleNotifiableEvent?> = mapOf(AN_EVENT_ID to null)
val missingEventRoomInvitation = listOf(ProcessedType.REMOVE to A_SIMPLE_EVENT)
val result = missingEventRoomInvitation.toNotifications(MY_USER_ID)

View file

@ -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<String, NotifiableEvent?>()
private val A_PROCESSED_EVENTS = GroupedNotificationEvents(emptyMap(), emptyMap(), emptyMap())
private val AN_EVENT_LIST = listOf<Pair<ProcessedType, NotifiableEvent>>()
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<Notification>()

View file

@ -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
}
}
}