lifting the seenIds cache to the event queue rather than chaining it through, allows us to simplify the state model

This commit is contained in:
Adam Brown 2021-11-18 13:31:28 +00:00
parent fb5e3cdfcd
commit f61474cb83
5 changed files with 27 additions and 28 deletions

View file

@ -86,7 +86,7 @@ class NotificationDrawerManager @Inject constructor(private val context: Context
Timber.d("onNotifiableEventReceived(): is push: ${notifiableEvent.canBeReplaced}") Timber.d("onNotifiableEventReceived(): is push: ${notifiableEvent.canBeReplaced}")
} }
add(notifiableEvent, notificationState.seenEventIds) add(notifiableEvent)
} }
/** /**

View file

@ -28,21 +28,21 @@ private const val KEY_ALIAS_SECRET_STORAGE = "notificationMgr"
object NotificationEventPersistence { object NotificationEventPersistence {
fun loadEvents(context: Context, currentSession: Session?): NotificationEventQueue { fun loadEvents(context: Context, currentSession: Session?, factory: (List<NotifiableEvent>) -> NotificationEventQueue): NotificationEventQueue {
try { try {
val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME) val file = File(context.applicationContext.cacheDir, ROOMS_NOTIFICATIONS_FILE_NAME)
if (file.exists()) { if (file.exists()) {
file.inputStream().use { file.inputStream().use {
val events: ArrayList<NotifiableEvent>? = currentSession?.loadSecureSecret(it, KEY_ALIAS_SECRET_STORAGE) val events: ArrayList<NotifiableEvent>? = currentSession?.loadSecureSecret(it, KEY_ALIAS_SECRET_STORAGE)
if (events != null) { if (events != null) {
return NotificationEventQueue(events.toMutableList()) return factory(events)
} }
} }
} }
} catch (e: Throwable) { } catch (e: Throwable) {
Timber.e(e, "## Failed to load cached notification info") Timber.e(e, "## Failed to load cached notification info")
} }
return NotificationEventQueue() return factory(emptyList())
} }
fun persistEvents(queuedEvents: NotificationEventQueue, context: Context, currentSession: Session) { fun persistEvents(queuedEvents: NotificationEventQueue, context: Context, currentSession: Session) {

View file

@ -19,7 +19,14 @@ package im.vector.app.features.notifications
import timber.log.Timber import timber.log.Timber
class NotificationEventQueue( class NotificationEventQueue(
private val queue: MutableList<NotifiableEvent> = mutableListOf() private val queue: MutableList<NotifiableEvent> = 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<String>
) { ) {
fun markRedacted(eventIds: List<String>) { fun markRedacted(eventIds: List<String>) {
@ -57,7 +64,7 @@ class NotificationEventQueue(
queue.clear() queue.clear()
} }
fun add(notifiableEvent: NotifiableEvent, seenEventIds: CircularCache<String>) { fun add(notifiableEvent: NotifiableEvent) {
val existing = findExistingById(notifiableEvent) val existing = findExistingById(notifiableEvent)
val edited = findEdited(notifiableEvent) val edited = findEdited(notifiableEvent)
when { when {

View file

@ -21,7 +21,8 @@ import org.matrix.android.sdk.api.session.Session
class NotificationState( 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 * 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 events are removed the previously rendered notifications will be cancelled
* when adding or updating, the notifications will be notified * 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 * allowing us to cancel any notifications previous displayed by now removed events
*/ */
private val renderedEvents: MutableList<ProcessedEvent<NotifiableEvent>>, private val renderedEvents: MutableList<ProcessedEvent<NotifiableEvent>>,
/**
* 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<String>
) { ) {
fun <T> updateQueuedEvents(drawerManager: NotificationDrawerManager, action: NotificationDrawerManager.(NotificationEventQueue, List<ProcessedEvent<NotifiableEvent>>) -> T): T { fun <T> updateQueuedEvents(drawerManager: NotificationDrawerManager, action: NotificationDrawerManager.(NotificationEventQueue, List<ProcessedEvent<NotifiableEvent>>) -> T): T {
@ -65,13 +59,11 @@ class NotificationState(
companion object { companion object {
fun createInitialNotificationState(context: Context, currentSession: Session?): NotificationState { fun createInitialNotificationState(context: Context, currentSession: Session?): NotificationState {
val queuedEvents = NotificationEventPersistence.loadEvents(context, currentSession) val queuedEvents = NotificationEventPersistence.loadEvents(context, currentSession, factory = { rawEvents ->
return NotificationState( NotificationEventQueue(rawEvents.toMutableList(), seenEventIds = CircularCache.create(cacheSize = 25))
queuedEvents = queuedEvents, })
renderedEvents = queuedEvents.rawEvents().map { ProcessedEvent(ProcessedEvent.Type.KEEP, it) }.toMutableList(), val renderedEvents = queuedEvents.rawEvents().map { ProcessedEvent(ProcessedEvent.Type.KEEP, it) }.toMutableList()
seenEventIds = CircularCache.create(cacheSize = 25) return NotificationState(queuedEvents, renderedEvents)
)
} }
} }
} }

View file

@ -126,7 +126,7 @@ class NotificationEventQueueTest {
fun `given no events when adding then adds event`() { fun `given no events when adding then adds event`() {
val queue = givenQueue(listOf()) val queue = givenQueue(listOf())
queue.add(aSimpleNotifiableEvent(), seenEventIds = seenIdsCache) queue.add(aSimpleNotifiableEvent())
queue.rawEvents() shouldBeEqualTo listOf(aSimpleNotifiableEvent()) queue.rawEvents() shouldBeEqualTo listOf(aSimpleNotifiableEvent())
} }
@ -137,7 +137,7 @@ class NotificationEventQueueTest {
val notifiableEvent = aSimpleNotifiableEvent() val notifiableEvent = aSimpleNotifiableEvent()
seenIdsCache.put(notifiableEvent.eventId) seenIdsCache.put(notifiableEvent.eventId)
queue.add(notifiableEvent, seenEventIds = seenIdsCache) queue.add(notifiableEvent)
queue.rawEvents() shouldBeEqualTo emptyList() queue.rawEvents() shouldBeEqualTo emptyList()
} }
@ -148,7 +148,7 @@ class NotificationEventQueueTest {
val updatedEvent = replaceableEvent.copy(title = "updated title") val updatedEvent = replaceableEvent.copy(title = "updated title")
val queue = givenQueue(listOf(replaceableEvent)) val queue = givenQueue(listOf(replaceableEvent))
queue.add(updatedEvent, seenEventIds = seenIdsCache) queue.add(updatedEvent)
queue.rawEvents() shouldBeEqualTo listOf(updatedEvent) queue.rawEvents() shouldBeEqualTo listOf(updatedEvent)
} }
@ -159,7 +159,7 @@ class NotificationEventQueueTest {
val updatedEvent = nonReplaceableEvent.copy(title = "updated title") val updatedEvent = nonReplaceableEvent.copy(title = "updated title")
val queue = givenQueue(listOf(nonReplaceableEvent)) val queue = givenQueue(listOf(nonReplaceableEvent))
queue.add(updatedEvent, seenEventIds = seenIdsCache) queue.add(updatedEvent)
queue.rawEvents() shouldBeEqualTo listOf(nonReplaceableEvent) 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 updatedEvent = editedEvent.copy(eventId = "1", editedEventId = "id-to-edit", title = "updated title")
val queue = givenQueue(listOf(editedEvent)) val queue = givenQueue(listOf(editedEvent))
queue.add(updatedEvent, seenEventIds = seenIdsCache) queue.add(updatedEvent)
queue.rawEvents() shouldBeEqualTo listOf(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 updatedEvent = editedEvent.copy(eventId = "1", editedEventId = "id-to-edit", title = "updated title")
val queue = givenQueue(listOf(editedEvent)) val queue = givenQueue(listOf(editedEvent))
queue.add(updatedEvent, seenEventIds = seenIdsCache) queue.add(updatedEvent)
queue.rawEvents() shouldBeEqualTo listOf(updatedEvent) queue.rawEvents() shouldBeEqualTo listOf(updatedEvent)
} }
@ -212,5 +212,5 @@ class NotificationEventQueueTest {
queue.rawEvents() shouldBeEqualTo listOf(anInviteNotifiableEvent(roomId = roomId)) queue.rawEvents() shouldBeEqualTo listOf(anInviteNotifiableEvent(roomId = roomId))
} }
private fun givenQueue(events: List<NotifiableEvent>) = NotificationEventQueue(events.toMutableList()) private fun givenQueue(events: List<NotifiableEvent>) = NotificationEventQueue(events.toMutableList(), seenEventIds = seenIdsCache)
} }