extracting notification logic to its own class with unit tests

core/src/main/kotlin/app/dapk/st/core/extensions/MapExtensions.kt

@@ -1,3 +1,8 @@
 package app.dapk.st.core.extensions
 
-fun <K, V> Map<K, V>?.containsKey(key: K) = this?.containsKey(key) ?: false
+fun <K, V> Map<K, V>?.containsKey(key: K) = this?.containsKey(key) ?: false
+
+fun <K, V> MutableMap<K,V>.clearAndPutAll(input: Map<K, V>) {
+    this.clear()
+    this.putAll(input)
+}

features/notifications/build.gradle

@@ -14,4 +14,10 @@ dependencies {
 
     implementation platform('com.google.firebase:firebase-bom:29.0.3')
     implementation 'com.google.firebase:firebase-messaging'
+
+    kotlinTest(it)
+
+    androidImportFixturesWorkaround(project, project(":core"))
+    androidImportFixturesWorkaround(project, project(":matrix:common"))
+    androidImportFixturesWorkaround(project, project(":matrix:services:sync"))
 }

features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationsModule.kt

@@ -30,8 +30,8 @@ class NotificationsModule(
     fun firebasePushTokenUseCase() = firebasePushTokenUseCase
     fun roomStore() = roomStore
     fun notificationsUseCase() = NotificationsUseCase(
-        roomStore,
         NotificationRenderer(notificationManager(), NotificationFactory(iconLoader, context, intentFactory)),
+        ObserveUnreadNotificationsUseCaseImpl(roomStore),
         NotificationChannels(notificationManager()),
     )
 

features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationsUseCase.kt

@@ -2,78 +2,33 @@ package app.dapk.st.notifications
 
 import app.dapk.st.core.AppLogTag.NOTIFICATION
 import app.dapk.st.core.log
-import app.dapk.st.matrix.common.EventId
-import app.dapk.st.matrix.common.RoomId
 import app.dapk.st.matrix.sync.RoomEvent
 import app.dapk.st.matrix.sync.RoomOverview
-import app.dapk.st.matrix.sync.RoomStore
-import kotlinx.coroutines.flow.*
+import kotlinx.coroutines.flow.collect
+import kotlinx.coroutines.flow.onEach
 
 class NotificationsUseCase(
-    private val roomStore: RoomStore,
     private val notificationRenderer: NotificationRenderer,
+    private val observeRenderableUnreadEventsUseCase: ObserveUnreadNotificationsUseCase,
     notificationChannels: NotificationChannels,
 ) {
 
-    private val inferredCurrentNotifications = mutableMapOf<RoomId, List<RoomEvent>>()
-    private var previousUnreadEvents: Map<RoomId, List<EventId>>? = null
-
     init {
         notificationChannels.initChannels()
     }
 
-    data class NotificationDiff(
-        val unchanged: Map<RoomId, List<EventId>>,
-        val changedOrNew: Map<RoomId, List<EventId>>,
-        val removed: Map<RoomId, List<EventId>>
-    )
-
     suspend fun listenForNotificationChanges() {
-        roomStore.observeUnread()
-            .map { each ->
-                val allUnreadIds = each.toIds()
-                val notificationDiff = calculateDiff(allUnreadIds, previousUnreadEvents)
-                previousUnreadEvents = allUnreadIds
-                each to notificationDiff
-            }
-            .skipFirst()
-            .onEach { (each, diff) ->
-                when {
-                    diff.changedOrNew.isEmpty() && diff.removed.isEmpty() -> {
-                        log(NOTIFICATION, "Ignoring unread change due to no renderable changes")
-                    }
-                    inferredCurrentNotifications.isEmpty() && diff.removed.isNotEmpty() -> {
-                        log(NOTIFICATION, "Ignoring unread change due to no currently showing messages and changes are all messages marked as read")
-                    }
-                    else -> renderUnreadChange(each, diff)
-                }
-            }
+        observeRenderableUnreadEventsUseCase()
+            .onEach { (each, diff) -> renderUnreadChange(each, diff) }
             .collect()
     }
 
-    private fun calculateDiff(allUnread: Map<RoomId, List<EventId>>, previousUnread: Map<RoomId, List<EventId>>?): NotificationDiff {
-        val unchanged = previousUnread?.filter { allUnread.containsKey(it.key) && it.value == allUnread[it.key] } ?: emptyMap()
-        val changedOrNew = allUnread.filterNot { unchanged.containsKey(it.key) }
-        val removed = previousUnread?.filter { !unchanged.containsKey(it.key) } ?: emptyMap()
-        return NotificationDiff(unchanged, changedOrNew, removed)
-    }
-
     private suspend fun renderUnreadChange(allUnread: Map<RoomOverview, List<RoomEvent>>, diff: NotificationDiff) {
         log(NOTIFICATION, "unread changed - render notifications")
-        inferredCurrentNotifications.clear()
-        inferredCurrentNotifications.putAll(allUnread.mapKeys { it.key.roomId })
-
         notificationRenderer.render(
             allUnread = allUnread,
             removedRooms = diff.removed.keys,
             roomsWithNewEvents = diff.changedOrNew.keys
         )
     }
-
-    private fun <T> Flow<T>.skipFirst() = drop(1)
 }
-
-private fun List<RoomEvent>.toEventIds() = this.map { it.eventId }
-private fun Map<RoomOverview, List<RoomEvent>>.toIds() = this
-    .mapValues { it.value.toEventIds() }
-    .mapKeys { it.key.roomId }

features/notifications/src/main/kotlin/app/dapk/st/notifications/ObserveUnreadNotificationsUseCaseImpl.kt

@@ -0,0 +1,75 @@
+package app.dapk.st.notifications
+
+import app.dapk.st.core.AppLogTag
+import app.dapk.st.core.extensions.clearAndPutAll
+import app.dapk.st.core.log
+import app.dapk.st.matrix.common.EventId
+import app.dapk.st.matrix.common.RoomId
+import app.dapk.st.matrix.sync.RoomEvent
+import app.dapk.st.matrix.sync.RoomOverview
+import app.dapk.st.matrix.sync.RoomStore
+import kotlinx.coroutines.flow.*
+
+typealias UnreadNotifications = Pair<Map<RoomOverview, List<RoomEvent>>, NotificationDiff>
+internal typealias ObserveUnreadNotificationsUseCase = suspend () -> Flow<UnreadNotifications>
+
+class ObserveUnreadNotificationsUseCaseImpl(private val roomStore: RoomStore) : ObserveUnreadNotificationsUseCase {
+
+    override suspend fun invoke(): Flow<UnreadNotifications> {
+        return roomStore.observeUnread()
+            .mapWithDiff()
+            .avoidShowingPreviousNotificationsOnLaunch()
+            .onlyRenderableChanges()
+    }
+
+}
+
+private fun Flow<UnreadNotifications>.onlyRenderableChanges(): Flow<UnreadNotifications> {
+    val inferredCurrentNotifications = mutableMapOf<RoomId, List<RoomEvent>>()
+    return this
+        .filter { (_, diff) ->
+            when {
+                diff.changedOrNew.isEmpty() && diff.removed.isEmpty() -> {
+                    log(AppLogTag.NOTIFICATION, "Ignoring unread change due to no renderable changes")
+                    false
+                }
+                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")
+                    false
+                }
+                else -> true
+            }
+        }
+        .onEach { (allUnread, _) -> inferredCurrentNotifications.clearAndPutAll(allUnread.mapKeys { it.key.roomId }) }
+}
+
+private fun Flow<Map<RoomOverview, List<RoomEvent>>>.mapWithDiff(): Flow<Pair<Map<RoomOverview, List<RoomEvent>>, NotificationDiff>> {
+    val previousUnreadEvents = mutableMapOf<RoomId, List<EventId>>()
+    return this.map { each ->
+        val allUnreadIds = each.toIds()
+        val notificationDiff = calculateDiff(allUnreadIds, previousUnreadEvents)
+        previousUnreadEvents.clearAndPutAll(allUnreadIds)
+        each to notificationDiff
+    }
+}
+
+private fun calculateDiff(allUnread: Map<RoomId, List<EventId>>, previousUnread: Map<RoomId, List<EventId>>?): NotificationDiff {
+    val unchanged = previousUnread?.filter { allUnread.containsKey(it.key) && it.value == allUnread[it.key] } ?: emptyMap()
+    val changedOrNew = allUnread.filterNot { unchanged.containsKey(it.key) }
+    val removed = previousUnread?.filter { !allUnread.containsKey(it.key) } ?: emptyMap()
+    return NotificationDiff(unchanged, changedOrNew, removed)
+}
+
+private fun List<RoomEvent>.toEventIds() = this.map { it.eventId }
+
+private fun Map<RoomOverview, List<RoomEvent>>.toIds() = this
+    .mapValues { it.value.toEventIds() }
+    .mapKeys { it.key.roomId }
+
+private fun <T> Flow<T>.avoidShowingPreviousNotificationsOnLaunch() = drop(1)
+
+data class NotificationDiff(
+    val unchanged: Map<RoomId, List<EventId>>,
+    val changedOrNew: Map<RoomId, List<EventId>>,
+    val removed: Map<RoomId, List<EventId>>
+)

features/notifications/src/test/kotlin/app/dapk/st/notifications/ObserveUnreadNotificationsUseCaseTest.kt

@@ -0,0 +1,102 @@
+package app.dapk.st.notifications
+
+import app.dapk.st.matrix.common.EventId
+import app.dapk.st.matrix.common.RoomId
+import app.dapk.st.matrix.sync.RoomEvent
+import app.dapk.st.matrix.sync.RoomOverview
+import fake.FakeRoomStore
+import fixture.aRoomId
+import fixture.aRoomMessageEvent
+import fixture.aRoomOverview
+import fixture.anEventId
+import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.flow.toList
+import kotlinx.coroutines.test.runTest
+import org.amshove.kluent.shouldBeEqualTo
+import org.junit.Test
+
+private val NO_UNREADS = emptyMap<RoomOverview, List<RoomEvent>>()
+val A_MESSAGE = aRoomMessageEvent(eventId = anEventId("1"), content = "hello")
+val A_MESSAGE_2 = aRoomMessageEvent(eventId = anEventId("2"), content = "world")
+val A_ROOM_OVERVIEW = aRoomOverview(roomId = aRoomId("1"))
+val A_ROOM_OVERVIEW_2 = aRoomOverview(roomId = aRoomId("2"))
+
+class ObserveUnreadNotificationsUseCaseTest {
+
+    private val fakeRoomStore = FakeRoomStore()
+
+    private val useCase = ObserveUnreadNotificationsUseCaseImpl(fakeRoomStore)
+
+    @Test
+    fun `given no initial unreads, when receiving new message, then emits message`() = runTest {
+        givenNoInitialUnreads(A_ROOM_OVERVIEW.withUnreads(A_MESSAGE))
+
+        val result = useCase.invoke().toList()
+
+        result shouldBeEqualTo listOf(
+            A_ROOM_OVERVIEW.withUnreads(A_MESSAGE) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE))
+        )
+    }
+
+    @Test
+    fun `given no initial unreads, when receiving multiple messages, then emits messages`() = runTest {
+        givenNoInitialUnreads(A_ROOM_OVERVIEW.withUnreads(A_MESSAGE), A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2))
+
+        val result = useCase.invoke().toList()
+
+        result shouldBeEqualTo listOf(
+            A_ROOM_OVERVIEW.withUnreads(A_MESSAGE) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE)),
+            A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE, A_MESSAGE_2))
+        )
+    }
+
+    @Test
+    fun `given initial unreads, when receiving new message, then emits all messages`() = runTest {
+        fakeRoomStore.givenUnreadEvents(
+            flowOf(A_ROOM_OVERVIEW.withUnreads(A_MESSAGE), A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2))
+        )
+
+        val result = useCase.invoke().toList()
+
+        result shouldBeEqualTo listOf(
+            A_ROOM_OVERVIEW.withUnreads(A_MESSAGE, A_MESSAGE_2) to aNotificationDiff(changedOrNew = A_ROOM_OVERVIEW.toDiff(A_MESSAGE, A_MESSAGE_2))
+        )
+    }
+
+    @Test
+    fun `given initial unreads, when reading a message, then emits nothing`() = runTest {
+        fakeRoomStore.givenUnreadEvents(
+            flowOf(A_ROOM_OVERVIEW.withUnreads(A_MESSAGE) + A_ROOM_OVERVIEW_2.withUnreads(A_MESSAGE_2), A_ROOM_OVERVIEW.withUnreads(A_MESSAGE))
+        )
+
+        val result = useCase.invoke().toList()
+
+        result shouldBeEqualTo emptyList()
+    }
+
+    @Test
+    fun `given initial unreads, when reading a duplicate unread, then emits nothing`() = runTest {
+        fakeRoomStore.givenUnreadEvents(
+            flowOf(A_ROOM_OVERVIEW.withUnreads(A_MESSAGE), A_ROOM_OVERVIEW.withUnreads(A_MESSAGE))
+        )
+
+        val result = useCase.invoke().toList()
+
+        result shouldBeEqualTo emptyList()
+    }
+
+    private fun givenNoInitialUnreads(vararg unreads: Map<RoomOverview, List<RoomEvent>>) {
+        fakeRoomStore.givenUnreadEvents(
+            flowOf(NO_UNREADS, *unreads)
+        )
+    }
+}
+
+private fun aNotificationDiff(
+    unchanged: Map<RoomId, List<EventId>> = emptyMap(),
+    changedOrNew: Map<RoomId, List<EventId>> = emptyMap(),
+    removed: Map<RoomId, List<EventId>> = emptyMap()
+) = NotificationDiff(unchanged, changedOrNew, removed)
+
+private fun RoomOverview.withUnreads(vararg events: RoomEvent) = mapOf(this to events.toList())
+private fun RoomOverview.toDiff(vararg events: RoomEvent) = mapOf(this.roomId to events.map { it.eventId })

matrix/services/sync/src/main/kotlin/app/dapk/st/matrix/sync/internal/DefaultSyncService.kt

@@ -63,7 +63,7 @@ internal class DefaultSyncService(
                     EventLookupUseCase(roomStore)
                 ),
                 RoomOverviewProcessor(roomMembersService),
-                UnreadEventsUseCase(roomStore, logger),
+                UnreadEventsProcessor(roomStore, logger),
                 EphemeralEventsUseCase(roomMembersService, syncEventsFlow),
             ),
             roomRefresher,

matrix/services/sync/src/main/kotlin/app/dapk/st/matrix/sync/internal/sync/RoomProcessor.kt

@@ -13,7 +13,7 @@ internal class RoomProcessor(
     private val roomDataSource: RoomDataSource,
     private val timelineEventsProcessor: TimelineEventsProcessor,
     private val roomOverviewProcessor: RoomOverviewProcessor,
-    private val unreadEventsUseCase: UnreadEventsUseCase,
+    private val unreadEventsProcessor: UnreadEventsProcessor,
     private val ephemeralEventsUseCase: EphemeralEventsUseCase,
 ) {
 
@@ -29,7 +29,7 @@ internal class RoomProcessor(
         )
 
         val overview = createRoomOverview(distinctEvents, roomToProcess, previousState)
-        unreadEventsUseCase.processUnreadState(overview, previousState?.roomOverview, newEvents, roomToProcess.userCredentials.userId, isInitialSync)
+        unreadEventsProcessor.processUnreadState(overview, previousState?.roomOverview, newEvents, roomToProcess.userCredentials.userId, isInitialSync)
 
         return RoomState(overview, distinctEvents).also {
             roomDataSource.persist(roomToProcess.roomId, previousState, it)

matrix/services/sync/src/main/kotlin/app/dapk/st/matrix/sync/internal/sync/UnreadEventsProcessor.kt

@@ -8,7 +8,7 @@ import app.dapk.st.matrix.sync.RoomEvent
 import app.dapk.st.matrix.sync.RoomOverview
 import app.dapk.st.matrix.sync.RoomStore
 
-internal class UnreadEventsUseCase(
+internal class UnreadEventsProcessor(
     private val roomStore: RoomStore,
     private val logger: MatrixLogger,
 ) {

matrix/services/sync/src/test/kotlin/app/dapk/st/matrix/sync/internal/sync/UnreadEventsProcessorTest.kt

@@ -14,18 +14,18 @@ private val A_ROOM_MESSAGE_FROM_OTHER = aRoomMessageEvent(
     author = aRoomMember(id = aUserId("a-different-user"))
 )
 
-internal class UnreadEventsUseCaseTest {
+internal class UnreadEventsProcessorTest {
 
     private val fakeRoomStore = FakeRoomStore()
 
-    private val unreadEventsUseCase = UnreadEventsUseCase(
+    private val unreadEventsProcessor = UnreadEventsProcessor(
         fakeRoomStore,
         FakeMatrixLogger()
     )
 
     @Test
     fun `given initial sync when processing unread then does mark any events as unread`() = runTest {
-        unreadEventsUseCase.processUnreadState(
+        unreadEventsProcessor.processUnreadState(
             isInitialSync = true,
             overview = aRoomOverview(),
             previousState = null,
@@ -40,7 +40,7 @@ internal class UnreadEventsUseCaseTest {
     fun `given read marker has changed when processing unread then marks room read`() = runTest {
         fakeRoomStore.expect { it.markRead(RoomId(any())) }
 
-        unreadEventsUseCase.processUnreadState(
+        unreadEventsProcessor.processUnreadState(
             isInitialSync = false,
             overview = A_ROOM_OVERVIEW.copy(readMarker = anEventId("an-updated-marker")),
             previousState = A_ROOM_OVERVIEW,
@@ -55,7 +55,7 @@ internal class UnreadEventsUseCaseTest {
     fun `given new events from other users when processing unread then inserts events as unread`() = runTest {
         fakeRoomStore.expect { it.insertUnread(RoomId(any()), any()) }
 
-        unreadEventsUseCase.processUnreadState(
+        unreadEventsProcessor.processUnreadState(
             isInitialSync = false,
             overview = A_ROOM_OVERVIEW,
             previousState = null,

matrix/services/sync/src/testFixtures/kotlin/fake/FakeRoomStore.kt

@@ -3,10 +3,13 @@ package fake
 import app.dapk.st.matrix.common.EventId
 import app.dapk.st.matrix.common.RoomId
 import app.dapk.st.matrix.sync.RoomEvent
+import app.dapk.st.matrix.sync.RoomOverview
 import app.dapk.st.matrix.sync.RoomStore
 import io.mockk.coEvery
 import io.mockk.coVerify
+import io.mockk.every
 import io.mockk.mockk
+import kotlinx.coroutines.flow.Flow
 
 class FakeRoomStore : RoomStore by mockk() {
 
@@ -27,4 +30,8 @@ class FakeRoomStore : RoomStore by mockk() {
         coEvery { findEvent(eventId) } returns result
     }
 
+    fun givenUnreadEvents(unreadEvents: Flow<Map<RoomOverview, List<RoomEvent>>>) {
+        every { observeUnread() } returns unreadEvents
+    }
+
 }