diffing historical event timestamps to avoid acting on them as new events

- fixes an issue where the database window would pull in legacy events after marking new events as read
This commit is contained in:
Adam Brown 2022-07-13 23:03:58 +01:00
parent 63618276b7
commit b0a438ee98
2 changed files with 59 additions and 13 deletions

View File

@ -34,10 +34,12 @@ private fun Flow<UnreadNotifications>.onlyRenderableChanges(): Flow<UnreadNotifi
log(AppLogTag.NOTIFICATION, "Ignoring unread change due to no renderable changes") log(AppLogTag.NOTIFICATION, "Ignoring unread change due to no renderable changes")
false false
} }
inferredCurrentNotifications.isEmpty() && diff.removed.isNotEmpty() -> { inferredCurrentNotifications.isEmpty() && diff.removed.isNotEmpty() -> {
log(AppLogTag.NOTIFICATION, "Ignoring unread change due to no currently showing messages and changes are all messages marked as read") log(AppLogTag.NOTIFICATION, "Ignoring unread change due to no currently showing messages and changes are all messages marked as read")
false false
} }
else -> true else -> true
} }
} }
@ -45,29 +47,48 @@ private fun Flow<UnreadNotifications>.onlyRenderableChanges(): Flow<UnreadNotifi
} }
private fun Flow<Map<RoomOverview, List<RoomEvent>>>.mapWithDiff(): Flow<Pair<Map<RoomOverview, List<RoomEvent>>, NotificationDiff>> { private fun Flow<Map<RoomOverview, List<RoomEvent>>>.mapWithDiff(): Flow<Pair<Map<RoomOverview, List<RoomEvent>>, NotificationDiff>> {
val previousUnreadEvents = mutableMapOf<RoomId, List<EventId>>() val previousUnreadEvents = mutableMapOf<RoomId, List<TimestampedEventId>>()
return this.map { each -> return this.map { each ->
val allUnreadIds = each.toIds() val allUnreadIds = each.toTimestampedIds()
val notificationDiff = calculateDiff(allUnreadIds, previousUnreadEvents) val notificationDiff = calculateDiff(allUnreadIds, previousUnreadEvents)
previousUnreadEvents.clearAndPutAll(allUnreadIds) previousUnreadEvents.clearAndPutAll(allUnreadIds)
each to notificationDiff each to notificationDiff
} }
} }
private fun calculateDiff(allUnread: Map<RoomId, List<EventId>>, previousUnread: Map<RoomId, List<EventId>>?): NotificationDiff { private fun calculateDiff(allUnread: Map<RoomId, List<TimestampedEventId>>, previousUnread: Map<RoomId, List<TimestampedEventId>>?): NotificationDiff {
val previousLatestEventTimestamps = previousUnread.toLatestTimestamps()
val newRooms = allUnread.filter { !previousUnread.containsKey(it.key) }.keys val newRooms = allUnread.filter { !previousUnread.containsKey(it.key) }.keys
val unchanged = previousUnread?.filter { allUnread.containsKey(it.key) && it.value == allUnread[it.key] } ?: emptyMap()
val changedOrNew = allUnread.filterNot { unchanged.containsKey(it.key) } val unchanged = previousUnread?.filter {
allUnread.containsKey(it.key) && (it.value == allUnread[it.key])
} ?: emptyMap()
val changedOrNew = allUnread.filterNot { unchanged.containsKey(it.key) }.mapValues { (key, value) ->
val isChangedRoom = !newRooms.contains(key)
if (isChangedRoom) {
val latest = previousLatestEventTimestamps[key] ?: 0L
value.filter {
val isExistingEvent = (previousUnread?.get(key)?.contains(it) ?: false)
!isExistingEvent && it.second > latest
}
} else {
value
}
}.filter { it.value.isNotEmpty() }
val removed = previousUnread?.filter { !allUnread.containsKey(it.key) } ?: emptyMap() val removed = previousUnread?.filter { !allUnread.containsKey(it.key) } ?: emptyMap()
return NotificationDiff(unchanged, changedOrNew, removed, newRooms) return NotificationDiff(unchanged.toEventIds(), changedOrNew.toEventIds(), removed.toEventIds(), newRooms)
} }
private fun List<RoomEvent>.toEventIds() = this.map { it.eventId } private fun Map<RoomId, List<TimestampedEventId>>?.toLatestTimestamps() = this?.mapValues { it.value.maxOf { it.second } } ?: emptyMap()
private fun Map<RoomOverview, List<RoomEvent>>.toIds() = this private fun Map<RoomId, List<TimestampedEventId>>.toEventIds() = this.mapValues { it.value.map { it.first } }
private fun Map<RoomOverview, List<RoomEvent>>.toTimestampedIds() = this
.mapValues { it.value.toEventIds() } .mapValues { it.value.toEventIds() }
.mapKeys { it.key.roomId } .mapKeys { it.key.roomId }
private fun List<RoomEvent>.toEventIds() = this.map { it.eventId to it.utcTimestamp }
private fun <T> Flow<T>.avoidShowingPreviousNotificationsOnLaunch() = drop(1) private fun <T> Flow<T>.avoidShowingPreviousNotificationsOnLaunch() = drop(1)
data class NotificationDiff( data class NotificationDiff(
@ -76,3 +97,5 @@ data class NotificationDiff(
val removed: Map<RoomId, List<EventId>>, val removed: Map<RoomId, List<EventId>>,
val newRooms: Set<RoomId> val newRooms: Set<RoomId>
) )
typealias TimestampedEventId = Pair<EventId, Long>

View File

@ -3,8 +3,11 @@ package app.dapk.st.notifications
import app.dapk.st.matrix.sync.RoomEvent import app.dapk.st.matrix.sync.RoomEvent
import app.dapk.st.matrix.sync.RoomOverview import app.dapk.st.matrix.sync.RoomOverview
import fake.FakeRoomStore import fake.FakeRoomStore
import fixture.*
import fixture.NotificationDiffFixtures.aNotificationDiff import fixture.NotificationDiffFixtures.aNotificationDiff
import fixture.aRoomId
import fixture.aRoomMessageEvent
import fixture.aRoomOverview
import fixture.anEventId
import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.toList import kotlinx.coroutines.flow.toList
import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.runTest
@ -12,8 +15,8 @@ import org.amshove.kluent.shouldBeEqualTo
import org.junit.Test import org.junit.Test
private val NO_UNREADS = emptyMap<RoomOverview, List<RoomEvent>>() private val NO_UNREADS = emptyMap<RoomOverview, List<RoomEvent>>()
private val A_MESSAGE = aRoomMessageEvent(eventId = anEventId("1"), content = "hello") private val A_MESSAGE = aRoomMessageEvent(eventId = anEventId("1"), content = "hello", utcTimestamp = 1000)
private val A_MESSAGE_2 = aRoomMessageEvent(eventId = anEventId("2"), content = "world") private val A_MESSAGE_2 = aRoomMessageEvent(eventId = anEventId("2"), content = "world", utcTimestamp = 2000)
private val A_ROOM_OVERVIEW = aRoomOverview(roomId = aRoomId("1")) private val A_ROOM_OVERVIEW = aRoomOverview(roomId = aRoomId("1"))
private val A_ROOM_OVERVIEW_2 = aRoomOverview(roomId = aRoomId("2")) private val A_ROOM_OVERVIEW_2 = aRoomOverview(roomId = aRoomId("2"))
@ -48,7 +51,7 @@ class ObserveUnreadRenderNotificationsUseCaseTest {
changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE), changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE),
newRooms = setOf(A_ROOM_OVERVIEW.roomId) newRooms = setOf(A_ROOM_OVERVIEW.roomId)
), ),
A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE, A_MESSAGE_2)) A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE_2))
) )
} }
@ -61,7 +64,7 @@ class ObserveUnreadRenderNotificationsUseCaseTest {
val result = useCase.invoke().toList() val result = useCase.invoke().toList()
result shouldBeEqualTo listOf( result shouldBeEqualTo listOf(
A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE, A_MESSAGE_2)) A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE_2))
) )
} }
@ -76,6 +79,26 @@ class ObserveUnreadRenderNotificationsUseCaseTest {
result shouldBeEqualTo emptyList() result shouldBeEqualTo emptyList()
} }
@Test
fun `given new and then historical message, when reading a message, then only emits the latest`() = runTest {
fakeRoomStore.givenUnreadEvents(
flowOf(
NO_UNREADS,
A_ROOM_OVERVIEW.withUnreads(A_MESSAGE),
A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE.copy(eventId = anEventId("old"), utcTimestamp = -1))
)
)
val result = useCase.invoke().toList()
result shouldBeEqualTo listOf(
A_ROOM_OVERVIEW.withUnreads(A_MESSAGE) to aNotificationDiff(
changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE),
newRooms = setOf(A_ROOM_OVERVIEW.roomId)
),
)
}
@Test @Test
fun `given initial unreads, when reading a duplicate unread, then emits nothing`() = runTest { fun `given initial unreads, when reading a duplicate unread, then emits nothing`() = runTest {
fakeRoomStore.givenUnreadEvents( fakeRoomStore.givenUnreadEvents(