From 54b1a909bcf912e2ac28c79ebbdb0ddc1d7ca327 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 7 Oct 2026 16:01:14 +0200 Subject: [PATCH] Display an error in the room select screen if the selected account has an invalid token All the accounts are proposed in the account switch, even the ones with an invalid token, so that the user can still see them. If the selected account has an invalid token, or is invalidated while the screen is displayed (for instance signed out from another device), the error is displayed instead of the room list, the selected rooms are cleared, and the client of this account is not used anymore. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../roomselect/impl/RoomSelectPresenter.kt | 33 +++++-- .../impl/RoomSelectPresenterTest.kt | 92 ++++++++++++++++++- 2 files changed, 115 insertions(+), 10 deletions(-) diff --git a/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt index 0850cbc4e9..3de9d22ba8 100644 --- a/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt +++ b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt @@ -38,7 +38,6 @@ import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch import timber.log.Timber @@ -67,11 +66,24 @@ class RoomSelectPresenter( var isSearchActive by remember { mutableStateOf(false) } var isAccountListExpanded by remember { mutableStateOf(false) } var selectedSessionId by remember { mutableStateOf(initialSessionId) } + val sessions by remember { sessionStore.sessionsFlow() }.collectAsState(initial = emptyList()) + // All the accounts are proposed, even the ones with an invalid token. Selecting them displays an error. val accounts by remember { - sessionStore.sessionsFlow().map { list -> - list.map { it.toMatrixUser() } + derivedStateOf { sessions.map { it.toMatrixUser() } } + } + // The selected session can have an invalid token, or be invalidated while this screen is displayed, for instance if it is + // signed out from another device. The invalidation of the initial session, which is the current one, is handled by the application. + val isSelectedSessionInvalid by remember { + derivedStateOf { + selectedSessionId != initialSessionId && sessions.none { it.userId == selectedSessionId.value && it.isTokenValid } } - }.collectAsState(initial = emptyList()) + } + LaunchedEffect(isSelectedSessionInvalid) { + if (isSelectedSessionInvalid) { + // The rooms of an invalid session cannot be used + selectedRooms = persistentListOf() + } + } val selectedAccount by remember { derivedStateOf { // Fallback to a MatrixUser with only the userId until the sessions are loaded @@ -87,12 +99,14 @@ class RoomSelectPresenter( val coroutineScope = rememberCoroutineScope() // Create one data source per session. When the selected session changes, the previous // data source is removed from the composition and its coroutine scope is cancelled. - val (dataSource, hasRoomListError) = key(selectedSessionId) { + val (selectedSessionDataSource, hasClientError) = key(selectedSessionId) { val dataSourceCoroutineScope = rememberCoroutineScope() // The sessions are restored at startup, so the client should already be in memory. - // A null value means that the client is being restored. - val roomListServiceResult by produceState(matrixClientProvider.getOrNull(selectedSessionId)?.roomListService?.let { Result.success(it) }) { - if (value == null) { + // A null value means that the client is being restored, or that the session is invalid: its client cannot be restored, + // and an in-memory client would have been destroyed, so do not try to get it. + val initialRoomListService = if (isSelectedSessionInvalid) null else matrixClientProvider.getOrNull(selectedSessionId)?.roomListService + val roomListServiceResult by produceState(initialRoomListService?.let { Result.success(it) }) { + if (value == null && !isSelectedSessionInvalid) { value = matrixClientProvider.getOrRestore(selectedSessionId) .onFailure { Timber.e(it, "Failed to get the client for $selectedSessionId") } .map { it.roomListService } @@ -104,6 +118,9 @@ class RoomSelectPresenter( } dataSource to (roomListServiceResult?.isFailure == true) } + // Do not use the data source of an invalid session, its client has been destroyed + val dataSource = selectedSessionDataSource.takeIf { !isSelectedSessionInvalid } + val hasRoomListError = hasClientError || isSelectedSessionInvalid val searchQuery = queryState.text.toString() LaunchedEffect(dataSource, searchQuery) { diff --git a/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt b/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt index 983d676dec..0694cc27f0 100644 --- a/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt +++ b/libraries/roomselect/impl/src/test/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenterTest.kt @@ -9,6 +9,7 @@ package io.element.android.libraries.roomselect.impl import androidx.compose.foundation.text.input.setTextAndPlaceCursorAtEnd +import app.cash.turbine.Event import com.google.common.truth.Truth.assertThat import io.element.android.libraries.designsystem.theme.components.SearchBarResultState import io.element.android.libraries.matrix.api.MatrixClientProvider @@ -35,6 +36,7 @@ import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.awaitLastSequentialItem import io.element.android.tests.testutils.consumeItemsUntilPredicate import io.element.android.tests.testutils.lambda.assert +import io.element.android.tests.testutils.lambda.lambdaError import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.test import io.element.android.tests.testutils.testCoroutineDispatchers @@ -141,6 +143,92 @@ class RoomSelectPresenterTest { } } + @Test + fun `present - accounts with an invalid token are displayed, selecting one displays an error`() = runTest { + val presenter = createRoomSelectPresenter( + mode = RoomSelectMode.Share, + sessionStore = InMemorySessionStore( + initialList = listOf( + aSessionData(sessionId = A_SESSION_ID.value, userDisplayName = "Alice", isTokenValid = true), + aSessionData(sessionId = A_SESSION_ID_2.value, userDisplayName = "Bob", isTokenValid = false), + ), + ), + // The client of the invalid session must not be requested + matrixClientProvider = FakeMatrixClientProvider { sessionId -> + if (sessionId == A_SESSION_ID) Result.success(FakeMatrixClient(sessionId = sessionId)) else lambdaError() + }, + ) + presenter.test { + val state = awaitLastSequentialItem() + assertThat(state.otherAccounts.map { it.userId }).containsExactly(A_SESSION_ID_2) + assertThat(state.hasRoomListError).isFalse() + state.eventSink(RoomSelectEvent.SelectAccount(A_SESSION_ID_2)) + val errorState = consumeItemsUntilPredicate { it.selectedAccount.userId == A_SESSION_ID_2 && it.hasRoomListError }.last() + // The account information is still available + assertThat(errorState.selectedAccount.displayName).isEqualTo("Bob") + assertThat(errorState.otherAccounts.map { it.userId }).containsExactly(A_SESSION_ID) + } + } + + @Test + fun `present - the selected account is invalidated, an error is displayed and the selected rooms are cleared`() = runTest { + val roomSummary2 = aRoomSummary(roomId = A_ROOM_ID_2) + val roomListServices = mapOf( + A_SESSION_ID to FakeRoomListService(createRoomListLambda = { FakeDynamicRoomList(summaries = MutableStateFlow(listOf(aRoomSummary()))) }), + A_SESSION_ID_2 to FakeRoomListService(createRoomListLambda = { FakeDynamicRoomList(summaries = MutableStateFlow(listOf(roomSummary2))) }), + ) + val sessionStore = aSessionStoreWithTwoAccounts() + val presenter = createRoomSelectPresenter( + mode = RoomSelectMode.Share, + sessionStore = sessionStore, + matrixClientProvider = FakeMatrixClientProvider { sessionId -> + Result.success(FakeMatrixClient(sessionId = sessionId, roomListService = roomListServices.getValue(sessionId))) + }, + ) + presenter.test { + val state = awaitLastSequentialItem() + state.eventSink(RoomSelectEvent.SelectAccount(A_SESSION_ID_2)) + val otherAccountState = consumeItemsUntilPredicate { + it.selectedAccount.userId == A_SESSION_ID_2 && + (it.resultState as? SearchBarResultState.Results)?.results?.map { room -> room.roomId } == listOf(A_ROOM_ID_2) + }.last() + otherAccountState.eventSink(RoomSelectEvent.ToggleSelectedRoom(roomSummary2.toSelectRoomInfo())) + assertThat(consumeItemsUntilPredicate { it.selectedRooms.isNotEmpty() }.last().selectedRooms).hasSize(1) + // The session of the selected account is invalidated, for instance signed out from another device + sessionStore.updateData(sessionStore.getSession(A_SESSION_ID_2.value)!!.copy(isTokenValid = false)) + val errorState = consumeItemsUntilPredicate { it.hasRoomListError && it.selectedRooms.isEmpty() }.last() + assertThat(errorState.selectedAccount.userId).isEqualTo(A_SESSION_ID_2) + assertThat(errorState.selectedAccount.displayName).isEqualTo("Bob") + assertThat(errorState.resultState).isInstanceOf(SearchBarResultState.Initial::class.java) + // Go back to the current account: the invalid account is still proposed + errorState.eventSink(RoomSelectEvent.SelectAccount(A_SESSION_ID)) + val finalState = consumeItemsUntilPredicate { it.selectedAccount.userId == A_SESSION_ID && !it.hasRoomListError }.last() + assertThat(finalState.showAccountSwitch).isTrue() + assertThat(finalState.otherAccounts.map { it.userId }).containsExactly(A_SESSION_ID_2) + } + } + + @Test + fun `present - another account is invalidated, it is still proposed and there is no error`() = runTest { + val sessionStore = aSessionStoreWithTwoAccounts() + val presenter = createRoomSelectPresenter( + mode = RoomSelectMode.Share, + sessionStore = sessionStore, + ) + presenter.test { + val state = awaitLastSequentialItem() + assertThat(state.otherAccounts.map { it.userId }).containsExactly(A_SESSION_ID_2) + sessionStore.updateData(sessionStore.getSession(A_SESSION_ID_2.value)!!.copy(isTokenValid = false)) + testScheduler.advanceUntilIdle() + // The state may not change at all + val finalState = cancelAndConsumeRemainingEvents().filterIsInstance>().lastOrNull()?.value ?: state + assertThat(finalState.selectedAccount.userId).isEqualTo(A_SESSION_ID) + assertThat(finalState.otherAccounts.map { it.userId }).containsExactly(A_SESSION_ID_2) + assertThat(finalState.hasRoomListError).isFalse() + assertThat(finalState.showAccountSwitch).isTrue() + } + } + @Test fun `present - select another account displays the rooms of this account`() = runTest { val roomSummary1 = aRoomSummary(roomId = A_ROOM_ID) @@ -191,8 +279,8 @@ class RoomSelectPresenterTest { private fun aSessionStoreWithTwoAccounts() = InMemorySessionStore( initialList = listOf( - aSessionData(sessionId = A_SESSION_ID.value, userDisplayName = "Alice"), - aSessionData(sessionId = A_SESSION_ID_2.value, userDisplayName = "Bob"), + aSessionData(sessionId = A_SESSION_ID.value, userDisplayName = "Alice", isTokenValid = true), + aSessionData(sessionId = A_SESSION_ID_2.value, userDisplayName = "Bob", isTokenValid = true), ), )