From ea35e36afff0849bcfb323fa1a8c540762db5b86 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 28 Jul 2023 16:22:30 +0200 Subject: [PATCH 1/4] Timeline: move TimelineEncryptedHistoryPostProcessor off the main thread --- libraries/matrix/impl/build.gradle.kts | 1 + .../impl/timeline/RustMatrixTimeline.kt | 1 + .../TimelineEncryptedHistoryPostProcessor.kt | 11 +++++++--- ...melineEncryptedHistoryPostProcessorTest.kt | 20 +++++++++++-------- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/libraries/matrix/impl/build.gradle.kts b/libraries/matrix/impl/build.gradle.kts index 112cc0777c..499e36988c 100644 --- a/libraries/matrix/impl/build.gradle.kts +++ b/libraries/matrix/impl/build.gradle.kts @@ -45,4 +45,5 @@ dependencies { testImplementation(libs.test.junit) testImplementation(libs.test.truth) testImplementation(projects.libraries.matrix.test) + testImplementation(libs.coroutines.test) } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt index 796b48ebb3..1a5bbf2e2a 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustMatrixTimeline.kt @@ -81,6 +81,7 @@ class RustMatrixTimeline( lastLoginTimestamp = lastLoginTimestamp, isRoomEncrypted = matrixRoom.isEncrypted, paginationStateFlow = _paginationState, + dispatcher = dispatcher, ) private val timelineItemFactory = MatrixTimelineItemMapper( diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt index 0fe12e6391..b273bef21b 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessor.kt @@ -19,18 +19,23 @@ package io.element.android.libraries.matrix.impl.timeline.postprocessor import io.element.android.libraries.matrix.api.timeline.MatrixTimeline import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.getAndUpdate +import kotlinx.coroutines.withContext +import timber.log.Timber import java.util.Date class TimelineEncryptedHistoryPostProcessor( + private val dispatcher: CoroutineDispatcher, private val lastLoginTimestamp: Date?, private val isRoomEncrypted: Boolean, private val paginationStateFlow: MutableStateFlow, ) { - fun process(items: List): List { - if (!isRoomEncrypted || lastLoginTimestamp == null) return items + suspend fun process(items: List): List = withContext(dispatcher) { + Timber.d("Process on Thread=${Thread.currentThread()}") + if (!isRoomEncrypted || lastLoginTimestamp == null) return@withContext items val filteredItems = replaceWithEncryptionHistoryBannerIfNeeded(items) // Disable back pagination @@ -43,7 +48,7 @@ class TimelineEncryptedHistoryPostProcessor( ) } } - return filteredItems + filteredItems } private fun replaceWithEncryptionHistoryBannerIfNeeded(list: List): List { diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt index 91f0bc1883..3b04895bfb 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/timeline/postprocessor/TimelineEncryptedHistoryPostProcessorTest.kt @@ -22,6 +22,9 @@ import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem import io.element.android.libraries.matrix.test.room.anEventTimelineItem import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runTest import org.junit.Test import java.util.Date @@ -30,7 +33,7 @@ class TimelineEncryptedHistoryPostProcessorTest { private val defaultLastLoginTimestamp = Date(1689061264L) @Test - fun `given an unencrypted room, nothing is done`() { + fun `given an unencrypted room, nothing is done`() = runTest { val processor = createPostProcessor(isRoomEncrypted = false) val items = listOf( MatrixTimelineItem.Event(0L, anEventTimelineItem()) @@ -39,7 +42,7 @@ class TimelineEncryptedHistoryPostProcessorTest { } @Test - fun `given a null lastLoginTimestamp, nothing is done`() { + fun `given a null lastLoginTimestamp, nothing is done`() = runTest { val processor = createPostProcessor(lastLoginTimestamp = null) val items = listOf( MatrixTimelineItem.Event(0L, anEventTimelineItem()) @@ -48,14 +51,14 @@ class TimelineEncryptedHistoryPostProcessorTest { } @Test - fun `given an empty list, nothing is done`() { + fun `given an empty list, nothing is done`() = runTest { val processor = createPostProcessor() val items = emptyList() assertThat(processor.process(items)).isSameInstanceAs(items) } @Test - fun `given a list with no items before lastLoginTimestamp, nothing is done`() { + fun `given a list with no items before lastLoginTimestamp, nothing is done`() = runTest { val processor = createPostProcessor() val items = listOf( MatrixTimelineItem.Event(0L, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time + 1)) @@ -64,7 +67,7 @@ class TimelineEncryptedHistoryPostProcessorTest { } @Test - fun `given a list with an item with equal timestamp as lastLoginTimestamp, it's replaced`() { + fun `given a list with an item with equal timestamp as lastLoginTimestamp, it's replaced`() = runTest { val processor = createPostProcessor() val items = listOf( MatrixTimelineItem.Event(0L, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time)) @@ -74,7 +77,7 @@ class TimelineEncryptedHistoryPostProcessorTest { } @Test - fun `given a list with an item with a lower timestamp than lastLoginTimestamp, it's replaced`() { + fun `given a list with an item with a lower timestamp than lastLoginTimestamp, it's replaced`() = runTest { val processor = createPostProcessor() val items = listOf( MatrixTimelineItem.Event(0L, anEventTimelineItem(timestamp = defaultLastLoginTimestamp.time - 1)) @@ -85,7 +88,7 @@ class TimelineEncryptedHistoryPostProcessorTest { } @Test - fun `given a list with several with lower or equal timestamps than lastLoginTimestamp, they're replaced and the user can't back paginate`() { + fun `given a list with several with lower or equal timestamps than lastLoginTimestamp, they're replaced and the user can't back paginate`() = runTest { val paginationStateFlow = MutableStateFlow(MatrixTimeline.PaginationState(hasMoreToLoadBackwards = true, isBackPaginating = false)) val processor = createPostProcessor(paginationStateFlow = paginationStateFlow) val items = listOf( @@ -102,7 +105,7 @@ class TimelineEncryptedHistoryPostProcessorTest { assertThat(paginationStateFlow.value).isEqualTo(MatrixTimeline.PaginationState(hasMoreToLoadBackwards = false, isBackPaginating = false)) } - private fun createPostProcessor( + private fun TestScope.createPostProcessor( lastLoginTimestamp: Date? = defaultLastLoginTimestamp, isRoomEncrypted: Boolean = true, paginationStateFlow: MutableStateFlow = @@ -111,5 +114,6 @@ class TimelineEncryptedHistoryPostProcessorTest { lastLoginTimestamp = lastLoginTimestamp, isRoomEncrypted = isRoomEncrypted, paginationStateFlow = paginationStateFlow, + dispatcher = StandardTestDispatcher(testScheduler) ) } From d63a3e2d7799de63f11fb188de1fcc284f599be1 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 28 Jul 2023 17:55:43 +0200 Subject: [PATCH 2/4] Messages: remove some blocking code from main thread --- .../features/messages/impl/MessagesPresenter.kt | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt index acaaf54c9e..bdb5f2c2ba 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt @@ -122,17 +122,12 @@ class MessagesPresenter @AssistedInject constructor( } val inviteProgress = remember { mutableStateOf>(Async.Uninitialized) } - - val showReinvitePrompt by remember( - hasDismissedInviteDialog, - composerState.hasFocus, - syncUpdateFlow, - ) { - derivedStateOf { - !hasDismissedInviteDialog && composerState.hasFocus && room.isDirect && room.activeMemberCount == 1L + var showReinvitePrompt by remember { mutableStateOf(false) } + LaunchedEffect(hasDismissedInviteDialog, composerState.hasFocus, syncUpdateFlow) { + withContext(dispatchers.io) { + showReinvitePrompt = !hasDismissedInviteDialog && composerState.hasFocus && room.isDirect && room.activeMemberCount == 1L } } - val networkConnectionStatus by networkMonitor.connectivity.collectAsState() val snackbarMessage by snackbarDispatcher.collectSnackbarMessageAsState() From fa9fa6969703bd3e19ecf07814909961d1e2f106 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 28 Jul 2023 17:56:17 +0200 Subject: [PATCH 3/4] Makes sure NotificationService is suspendable --- .../matrix/api/notification/NotificationService.kt | 2 +- .../android/libraries/matrix/impl/RustMatrixClient.kt | 2 +- .../matrix/impl/notification/RustNotificationService.kt | 9 ++++++--- .../matrix/test/notification/FakeNotificationService.kt | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt index 2046252930..4113953203 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/notification/NotificationService.kt @@ -21,5 +21,5 @@ import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId interface NotificationService { - fun getNotification(userId: SessionId, roomId: RoomId, eventId: EventId, filterByPushRules: Boolean): Result + suspend fun getNotification(userId: SessionId, roomId: RoomId, eventId: EventId, filterByPushRules: Boolean): Result } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt index 305bcaaba3..061810e3cd 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt @@ -103,7 +103,7 @@ class RustMatrixClient constructor( builder.finish() } - private val notificationService = RustNotificationService(sessionId, notificationClient, clock) + private val notificationService = RustNotificationService(sessionId, notificationClient, dispatchers, clock) private val isLoggingOut = AtomicBoolean(false) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt index 2c4258da11..bb281ba4d7 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/notification/RustNotificationService.kt @@ -16,29 +16,32 @@ package io.element.android.libraries.matrix.impl.notification +import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.notification.NotificationData import io.element.android.libraries.matrix.api.notification.NotificationService import io.element.android.services.toolbox.api.systemclock.SystemClock +import kotlinx.coroutines.withContext import org.matrix.rustcomponents.sdk.NotificationClient import org.matrix.rustcomponents.sdk.use class RustNotificationService( sessionId: SessionId, private val notificationClient: NotificationClient, + private val dispatchers: CoroutineDispatchers, clock: SystemClock, ) : NotificationService { private val notificationMapper: NotificationMapper = NotificationMapper(sessionId, clock) - override fun getNotification( + override suspend fun getNotification( userId: SessionId, roomId: RoomId, eventId: EventId, filterByPushRules: Boolean, - ): Result { - return runCatching { + ): Result = withContext(dispatchers.io) { + runCatching { val item = notificationClient.getNotificationWithSlidingSync(roomId.value, eventId.value) item?.use { notificationMapper.map(eventId, roomId, it) diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt index 9eb5a20ba4..5b863e65f8 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/notification/FakeNotificationService.kt @@ -23,7 +23,7 @@ import io.element.android.libraries.matrix.api.notification.NotificationData import io.element.android.libraries.matrix.api.notification.NotificationService class FakeNotificationService : NotificationService { - override fun getNotification(userId: SessionId, roomId: RoomId, eventId: EventId, filterByPushRules: Boolean): Result { + override suspend fun getNotification(userId: SessionId, roomId: RoomId, eventId: EventId, filterByPushRules: Boolean): Result { return Result.success(null) } } From f26e3b4979d67fb4f26266bdd7ede450d3528e62 Mon Sep 17 00:00:00 2001 From: ganfra Date: Mon, 31 Jul 2023 11:58:49 +0200 Subject: [PATCH 4/4] Fix CI --- .../android/features/messages/impl/MessagesPresenter.kt | 1 - .../android/features/messages/MessagesPresenterTest.kt | 8 ++++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt index bdb5f2c2ba..67499b81a1 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt @@ -21,7 +21,6 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.MutableState import androidx.compose.runtime.collectAsState -import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt index b47ac55d35..7ebfecfae6 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt @@ -361,11 +361,15 @@ class MessagesPresenterTest { assertThat(initialState.showReinvitePrompt).isFalse() // When the input field is focused we show the alert initialState.composerState.eventSink(MessageComposerEvents.FocusChanged(true)) - val focusedState = awaitItem() + val focusedState = consumeItemsUntilPredicate { state -> + state.showReinvitePrompt + }.last() assertThat(focusedState.showReinvitePrompt).isTrue() // If it's dismissed then we stop showing the alert initialState.eventSink(MessagesEvents.InviteDialogDismissed(InviteDialogAction.Cancel)) - val dismissedState = awaitItem() + val dismissedState = consumeItemsUntilPredicate { state -> + !state.showReinvitePrompt + }.last() assertThat(dismissedState.showReinvitePrompt).isFalse() } }