From f61474cb83409f834dbc1c4404e1ab5ab20a2171 Mon Sep 17 00:00:00 2001 From: Adam Brown Date: Thu, 18 Nov 2021 13:31:28 +0000 Subject: [PATCH] lifting the seenIds cache to the event queue rather than chaining it through, allows us to simplify the state model --- .../NotificationDrawerManager.kt | 2 +- .../NotificationEventPersistence.kt | 6 ++--- .../notifications/NotificationEventQueue.kt | 11 ++++++++-- .../notifications/NotificationState.kt | 22 ++++++------------- .../NotificationEventQueueTest.kt | 14 ++++++------ 5 files changed, 27 insertions(+), 28 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 696e965963..81db029d8b 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 @@ -86,7 +86,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context Timber.d("onNotifiableEventReceived(): is push: ${notifiableEvent.canBeReplaced}") } - add(notifiableEvent, notificationState.seenEventIds) + add(notifiableEvent) } /** diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationEventPersistence.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationEventPersistence.kt index 65a3203d50..11cc247b48 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationEventPersistence.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationEventPersistence.kt @@ -28,21 +28,21 @@ private const val KEY_ALIAS_SECRET_STORAGE = "notificationMgr" object NotificationEventPersistence { - fun loadEvents(context: Context, currentSession: Session?): NotificationEventQueue { + fun loadEvents(context: Context, currentSession: Session?, factory: (List) -> NotificationEventQueue): NotificationEventQueue { try { val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME) if (file.exists()) { file.inputStream().use { val events: ArrayList? = currentSession?.loadSecureSecret(it, KEY_ALIAS_SECRET_STORAGE) if (events != null) { - return NotificationEventQueue(events.toMutableList()) + return factory(events) } } } } catch (e: Throwable) { Timber.e(e, "## Failed to load cached notification info") } - return NotificationEventQueue() + return factory(emptyList()) } fun persistEvents(queuedEvents: NotificationEventQueue, context: Context, currentSession: Session) { diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt index 83beb4fa02..2dc86f0aa0 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt @@ -19,7 +19,14 @@ package im.vector.app.features.notifications import timber.log.Timber class NotificationEventQueue( - private val queue: MutableList = mutableListOf() + private val queue: MutableList = mutableListOf(), + + /** + * An in memory FIFO cache of the seen events. + * Acts as a notification debouncer to stop already dismissed push notifications from + * displaying again when the /sync response is delayed. + */ + private val seenEventIds: CircularCache ) { fun markRedacted(eventIds: List) { @@ -57,7 +64,7 @@ class NotificationEventQueue( queue.clear() } - fun add(notifiableEvent: NotifiableEvent, seenEventIds: CircularCache) { + fun add(notifiableEvent: NotifiableEvent) { val existing = findExistingById(notifiableEvent) val edited = findEdited(notifiableEvent) when { diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationState.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationState.kt index 4cb44609c0..d772aaa749 100644 --- a/vector/src/main/java/im/vector/app/features/notifications/NotificationState.kt +++ b/vector/src/main/java/im/vector/app/features/notifications/NotificationState.kt @@ -21,7 +21,8 @@ import org.matrix.android.sdk.api.session.Session class NotificationState( /** - * The notifiable events to render + * The notifiable events queued for rendering or currently rendered + * * 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 @@ -36,13 +37,6 @@ class NotificationState( * allowing us to cancel any notifications previous displayed by now removed events */ private val renderedEvents: MutableList>, - - /** - * An in memory FIFO cache of the seen events. - * Acts as a notification debouncer to stop already dismissed push notifications from - * displaying again when the /sync response is delayed. - */ - val seenEventIds: CircularCache ) { fun updateQueuedEvents(drawerManager: NotificationDrawerManager, action: NotificationDrawerManager.(NotificationEventQueue, List>) -> T): T { @@ -65,13 +59,11 @@ class NotificationState( companion object { fun createInitialNotificationState(context: Context, currentSession: Session?): NotificationState { - val queuedEvents = NotificationEventPersistence.loadEvents(context, currentSession) - return NotificationState( - queuedEvents = queuedEvents, - renderedEvents = queuedEvents.rawEvents().map { ProcessedEvent(ProcessedEvent.Type.KEEP, it) }.toMutableList(), - seenEventIds = CircularCache.create(cacheSize = 25) - ) + val queuedEvents = NotificationEventPersistence.loadEvents(context, currentSession, factory = { rawEvents -> + NotificationEventQueue(rawEvents.toMutableList(), seenEventIds = CircularCache.create(cacheSize = 25)) + }) + val renderedEvents = queuedEvents.rawEvents().map { ProcessedEvent(ProcessedEvent.Type.KEEP, it) }.toMutableList() + return NotificationState(queuedEvents, renderedEvents) } } } - diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationEventQueueTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationEventQueueTest.kt index 26c0030d26..3429f882f8 100644 --- a/vector/src/test/java/im/vector/app/features/notifications/NotificationEventQueueTest.kt +++ b/vector/src/test/java/im/vector/app/features/notifications/NotificationEventQueueTest.kt @@ -126,7 +126,7 @@ class NotificationEventQueueTest { fun `given no events when adding then adds event`() { val queue = givenQueue(listOf()) - queue.add(aSimpleNotifiableEvent(), seenEventIds = seenIdsCache) + queue.add(aSimpleNotifiableEvent()) queue.rawEvents() shouldBeEqualTo listOf(aSimpleNotifiableEvent()) } @@ -137,7 +137,7 @@ class NotificationEventQueueTest { val notifiableEvent = aSimpleNotifiableEvent() seenIdsCache.put(notifiableEvent.eventId) - queue.add(notifiableEvent, seenEventIds = seenIdsCache) + queue.add(notifiableEvent) queue.rawEvents() shouldBeEqualTo emptyList() } @@ -148,7 +148,7 @@ class NotificationEventQueueTest { val updatedEvent = replaceableEvent.copy(title = "updated title") val queue = givenQueue(listOf(replaceableEvent)) - queue.add(updatedEvent, seenEventIds = seenIdsCache) + queue.add(updatedEvent) queue.rawEvents() shouldBeEqualTo listOf(updatedEvent) } @@ -159,7 +159,7 @@ class NotificationEventQueueTest { val updatedEvent = nonReplaceableEvent.copy(title = "updated title") val queue = givenQueue(listOf(nonReplaceableEvent)) - queue.add(updatedEvent, seenEventIds = seenIdsCache) + queue.add(updatedEvent) queue.rawEvents() shouldBeEqualTo listOf(nonReplaceableEvent) } @@ -170,7 +170,7 @@ class NotificationEventQueueTest { val updatedEvent = editedEvent.copy(eventId = "1", editedEventId = "id-to-edit", title = "updated title") val queue = givenQueue(listOf(editedEvent)) - queue.add(updatedEvent, seenEventIds = seenIdsCache) + queue.add(updatedEvent) queue.rawEvents() shouldBeEqualTo listOf(updatedEvent) } @@ -181,7 +181,7 @@ class NotificationEventQueueTest { val updatedEvent = editedEvent.copy(eventId = "1", editedEventId = "id-to-edit", title = "updated title") val queue = givenQueue(listOf(editedEvent)) - queue.add(updatedEvent, seenEventIds = seenIdsCache) + queue.add(updatedEvent) queue.rawEvents() shouldBeEqualTo listOf(updatedEvent) } @@ -212,5 +212,5 @@ class NotificationEventQueueTest { queue.rawEvents() shouldBeEqualTo listOf(anInviteNotifiableEvent(roomId = roomId)) } - private fun givenQueue(events: List) = NotificationEventQueue(events.toMutableList()) + private fun givenQueue(events: List) = NotificationEventQueue(events.toMutableList(), seenEventIds = seenIdsCache) }