From 0be6d130257c919b9c55d3655c2358a9b2ad628b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 6 Nov 2025 12:22:48 +0100 Subject: [PATCH 01/17] MatrixAuthenticationService: remove `fun getHomeserverDetails(): StateFlow`. The MatrixHomeServerDetails are now return by `setHomeserver` --- .../login/impl/changeserver/ChangeServerPresenter.kt | 8 +++----- .../android/features/login/impl/login/LoginHelper.kt | 3 +-- .../matrix/api/auth/MatrixAuthenticationService.kt | 9 ++++++--- .../matrix/impl/auth/RustMatrixAuthenticationService.kt | 7 ++----- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt index 4df9eb12d5..d3db7496c5 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt @@ -60,11 +60,9 @@ class ChangeServerPresenter( title = data.title, accountProviderUrl = data.url, ) - authenticationService.setHomeserver(data.url).map { - authenticationService.getHomeserverDetails().value!! - // Valid, remember user choice - accountProviderDataSource.userSelection(data) - }.getOrThrow() + authenticationService.setHomeserver(data.url).getOrThrow() + // Homeserver is valid, remember user choice + accountProviderDataSource.userSelection(data) }.runCatchingUpdatingState(changeServerAction, errorTransform = ChangeServerError::from) } } diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginHelper.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginHelper.kt index 82ee87c372..8dbd2a4df3 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginHelper.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginHelper.kt @@ -65,8 +65,7 @@ class LoginHelper( loginHint: String?, ) = coroutineScope.launch { suspend { - authenticationService.setHomeserver(homeserverUrl).map { - val matrixHomeServerDetails = authenticationService.getHomeserverDetails().value!! + authenticationService.setHomeserver(homeserverUrl).map { matrixHomeServerDetails -> if (matrixHomeServerDetails.supportsOidcLogin) { // Retrieve the details right now val oidcPrompt = if (isAccountCreation) OidcPrompt.Create else OidcPrompt.Login diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixAuthenticationService.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixAuthenticationService.kt index 38777944ee..1255e40a14 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixAuthenticationService.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixAuthenticationService.kt @@ -13,7 +13,6 @@ import io.element.android.libraries.matrix.api.auth.external.ExternalSession import io.element.android.libraries.matrix.api.auth.qrlogin.MatrixQrCodeLoginData import io.element.android.libraries.matrix.api.auth.qrlogin.QrCodeLoginStep import io.element.android.libraries.matrix.api.core.SessionId -import kotlinx.coroutines.flow.StateFlow interface MatrixAuthenticationService { /** @@ -22,8 +21,12 @@ interface MatrixAuthenticationService { * Generally this method should not be used directly, prefer using [MatrixClientProvider.getOrRestore] instead. */ suspend fun restoreSession(sessionId: SessionId): Result - fun getHomeserverDetails(): StateFlow - suspend fun setHomeserver(homeserver: String): Result + + /** + * Set the homeserver to use for authentication, and return its details. + */ + suspend fun setHomeserver(homeserver: String): Result + suspend fun login(username: String, password: String): Result /** diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt index 9859358ae6..7ed523812f 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt @@ -37,7 +37,6 @@ import io.element.android.libraries.sessionstorage.api.LoginType import io.element.android.libraries.sessionstorage.api.SessionStore import kotlinx.coroutines.CancellationException import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.withContext import org.matrix.rustcomponents.sdk.Client import org.matrix.rustcomponents.sdk.ClientBuilder @@ -111,9 +110,7 @@ class RustMatrixAuthenticationService( return passphrase } - override fun getHomeserverDetails(): StateFlow = currentHomeserver - - override suspend fun setHomeserver(homeserver: String): Result = + override suspend fun setHomeserver(homeserver: String): Result = withContext(coroutineDispatchers.io) { val emptySessionPath = rotateSessionPath() runCatchingExceptions { @@ -123,7 +120,7 @@ class RustMatrixAuthenticationService( currentClient = client val homeServerDetails = client.homeserverLoginDetails().map() - currentHomeserver.value = homeServerDetails.copy(url = homeserver) + homeServerDetails.copy(url = homeserver) }.onFailure { clear() }.mapFailure { failure -> From cee6475eb859a5fe35771d8181345188b00ea498 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 6 Nov 2025 12:29:08 +0100 Subject: [PATCH 02/17] Do not override the value of url returned by the SDK --- .../matrix/impl/auth/RustMatrixAuthenticationService.kt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt index 7ed523812f..dc93b9aaf5 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt @@ -36,7 +36,6 @@ import io.element.android.libraries.matrix.impl.paths.SessionPathsFactory import io.element.android.libraries.sessionstorage.api.LoginType import io.element.android.libraries.sessionstorage.api.SessionStore import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.withContext import org.matrix.rustcomponents.sdk.Client import org.matrix.rustcomponents.sdk.ClientBuilder @@ -66,7 +65,6 @@ class RustMatrixAuthenticationService( // Ideally it would be possible to get the sessionPath from the Client to avoid doing this. private var sessionPaths: SessionPaths? = null private var currentClient: Client? = null - private var currentHomeserver = MutableStateFlow(null) private val newMatrixClientObservers = mutableListOf<(MatrixClient) -> Unit>() override fun listenToNewMatrixClients(lambda: (MatrixClient) -> Unit) { @@ -119,8 +117,7 @@ class RustMatrixAuthenticationService( } currentClient = client - val homeServerDetails = client.homeserverLoginDetails().map() - homeServerDetails.copy(url = homeserver) + client.homeserverLoginDetails().map() }.onFailure { clear() }.mapFailure { failure -> From 4cc5cf008407e4f36370f2a713632d63c8fcc906 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 6 Nov 2025 14:23:41 +0100 Subject: [PATCH 03/17] Fix test --- .../changeserver/ChangeServerPresenterTest.kt | 14 ++- .../ChooseAccountProviderPresenterTest.kt | 7 +- .../ConfirmAccountProviderPresenterTest.kt | 91 +++++++++++++------ .../LoginPasswordPresenterTest.kt | 28 ++++-- .../onboarding/OnBoardingPresenterTest.kt | 7 +- .../auth/FakeMatrixAuthenticationService.kt | 23 +---- 6 files changed, 111 insertions(+), 59 deletions(-) diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt index 3b88ab03c0..47f9fee728 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt @@ -18,6 +18,7 @@ import io.element.android.features.wellknown.test.FakeWellknownRetriever import io.element.android.features.wellknown.test.anElementWellKnown import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.core.uri.ensureProtocol +import io.element.android.libraries.matrix.test.AN_EXCEPTION import io.element.android.libraries.matrix.test.A_HOMESERVER import io.element.android.libraries.matrix.test.A_HOMESERVER_URL import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService @@ -46,7 +47,11 @@ class ChangeServerPresenterTest { @Test fun `present - change server ok`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER) + }, + ) createPresenter( authenticationService = authenticationService, enterpriseService = FakeEnterpriseService( @@ -55,7 +60,6 @@ class ChangeServerPresenterTest { ).test { val initialState = awaitItem() assertThat(initialState.changeServerAction).isEqualTo(AsyncData.Uninitialized) - authenticationService.givenHomeserver(A_HOMESERVER) initialState.eventSink.invoke(ChangeServerEvents.ChangeServer(AccountProvider(url = A_HOMESERVER_URL))) val loadingState = awaitItem() assertThat(loadingState.changeServerAction).isInstanceOf(AsyncData.Loading::class.java) @@ -66,10 +70,16 @@ class ChangeServerPresenterTest { @Test fun `present - change server error`() = runTest { + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.failure(AN_EXCEPTION) + }, + ) createPresenter( enterpriseService = FakeEnterpriseService( isAllowedToConnectToHomeserverResult = { true }, ), + authenticationService = authenticationService, ).test { val initialState = awaitItem() assertThat(initialState.changeServerAction).isEqualTo(AsyncData.Uninitialized) diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/chooseaccountprovider/ChooseAccountProviderPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/chooseaccountprovider/ChooseAccountProviderPresenterTest.kt index 2e13b0e555..96dc5b87ff 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/chooseaccountprovider/ChooseAccountProviderPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/chooseaccountprovider/ChooseAccountProviderPresenterTest.kt @@ -95,7 +95,11 @@ class ChooseAccountProviderPresenterTest { @Test fun `present - select account provider and continue - error then clear error`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.failure(AN_EXCEPTION) + }, + ) val presenter = createPresenter( enterpriseService = FakeEnterpriseService( defaultHomeserverListResult = { listOf(ACCOUNT_PROVIDER_FROM_CONFIG_1, ACCOUNT_PROVIDER_FROM_CONFIG_2) }, @@ -111,7 +115,6 @@ class ChooseAccountProviderPresenterTest { } awaitItem().also { assertThat(it.selectedAccountProvider).isEqualTo(accountProvider1) - authenticationService.givenChangeServerError(AN_EXCEPTION) it.eventSink(ChooseAccountProviderEvents.Continue) skipItems(1) // Loading diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt index 3978d3be6e..82c2caff61 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt @@ -53,11 +53,14 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - continue password login`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER) + }, + ) val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, ) - authenticationService.givenHomeserver(A_HOMESERVER) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -75,11 +78,14 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - continue oidc`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER_OIDC) + }, + ) val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, ) - authenticationService.givenHomeserver(A_HOMESERVER_OIDC) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -97,13 +103,16 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - oidc - cancel with failure`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER_OIDC) + }, + ) val defaultOidcActionFlow = FakeOidcActionFlow() val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, defaultOidcActionFlow = defaultOidcActionFlow, ) - authenticationService.givenHomeserver(A_HOMESERVER_OIDC) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -125,13 +134,16 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - oidc - cancel with success`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER_OIDC) + }, + ) val defaultOidcActionFlow = FakeOidcActionFlow() val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, defaultOidcActionFlow = defaultOidcActionFlow, ) - authenticationService.givenHomeserver(A_HOMESERVER_OIDC) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -152,13 +164,16 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - oidc - cancel to unblock`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER_OIDC) + }, + ) val defaultOidcActionFlow = FakeOidcActionFlow() val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, defaultOidcActionFlow = defaultOidcActionFlow, ) - authenticationService.givenHomeserver(A_HOMESERVER_OIDC) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -175,13 +190,16 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - oidc - success with failure`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER_OIDC) + }, + ) val defaultOidcActionFlow = FakeOidcActionFlow() val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, defaultOidcActionFlow = defaultOidcActionFlow, ) - authenticationService.givenHomeserver(A_HOMESERVER_OIDC) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -205,13 +223,16 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - oidc - success with success`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER_OIDC) + }, + ) val defaultOidcActionFlow = FakeOidcActionFlow() val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, defaultOidcActionFlow = defaultOidcActionFlow, ) - authenticationService.givenHomeserver(A_HOMESERVER_OIDC) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { @@ -232,7 +253,11 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - submit fails`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.failure(AN_EXCEPTION) + }, + ) val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, ) @@ -240,7 +265,6 @@ class ConfirmAccountProviderPresenterTest { presenter.present() }.test { val initialState = awaitItem() - authenticationService.givenChangeServerError(RuntimeException()) initialState.eventSink.invoke(ConfirmAccountProviderEvents.Continue) skipItems(1) // Loading val failureState = awaitItem() @@ -251,7 +275,11 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - clear error`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.failure(AN_EXCEPTION) + }, + ) val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, ) @@ -261,7 +289,6 @@ class ConfirmAccountProviderPresenterTest { val initialState = awaitItem() // Submit will return an error - authenticationService.givenChangeServerError(AN_EXCEPTION) initialState.eventSink(ConfirmAccountProviderEvents.Continue) skipItems(1) // Loading @@ -279,8 +306,11 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - confirm account creation without oidc and without url generates an error`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() - authenticationService.givenHomeserver(A_HOMESERVER) + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER) + }, + ) val presenter = createConfirmAccountProviderPresenter( params = ConfirmAccountProviderPresenter.Params(isAccountCreation = true), matrixAuthenticationService = authenticationService, @@ -306,8 +336,11 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - confirm account creation with oidc is successful`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() - authenticationService.givenHomeserver(A_HOMESERVER_OIDC) + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER_OIDC) + }, + ) val presenter = createConfirmAccountProviderPresenter( params = ConfirmAccountProviderPresenter.Params(isAccountCreation = true), matrixAuthenticationService = authenticationService, @@ -327,8 +360,11 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - confirm account creation with oidc and url continues with oidc`() = runTest { val aUrl = "aUrl" - val authenticationService = FakeMatrixAuthenticationService() - authenticationService.givenHomeserver(A_HOMESERVER_OIDC) + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER_OIDC) + }, + ) val presenter = createConfirmAccountProviderPresenter( params = ConfirmAccountProviderPresenter.Params(isAccountCreation = true), matrixAuthenticationService = authenticationService, @@ -349,8 +385,11 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - confirm account creation without oidc and with url continuing with url`() = runTest { val aUrl = "aUrl" - val authenticationService = FakeMatrixAuthenticationService() - authenticationService.givenHomeserver(A_HOMESERVER) + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER) + }, + ) val presenter = createConfirmAccountProviderPresenter( params = ConfirmAccountProviderPresenter.Params(isAccountCreation = true), matrixAuthenticationService = authenticationService, diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt index af158ec0da..fd048d1db0 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt @@ -42,8 +42,11 @@ class LoginPasswordPresenterTest { @Test fun `present - enter login and password`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() - authenticationService.givenHomeserver(A_HOMESERVER) + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER) + }, + ) createLoginPasswordPresenter( authenticationService = authenticationService, ).test { @@ -61,8 +64,11 @@ class LoginPasswordPresenterTest { @Test fun `present - submit`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() - authenticationService.givenHomeserver(A_HOMESERVER) + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER) + }, + ) createLoginPasswordPresenter( authenticationService = authenticationService, ).test { @@ -81,8 +87,11 @@ class LoginPasswordPresenterTest { @Test fun `present - submit with error`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() - authenticationService.givenHomeserver(A_HOMESERVER) + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER) + }, + ) createLoginPasswordPresenter( authenticationService = authenticationService, ).test { @@ -102,8 +111,11 @@ class LoginPasswordPresenterTest { @Test fun `present - clear error`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() - authenticationService.givenHomeserver(A_HOMESERVER) + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(A_HOMESERVER) + }, + ) createLoginPasswordPresenter( authenticationService = authenticationService, ).test { diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/onboarding/OnBoardingPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/onboarding/OnBoardingPresenterTest.kt index 16f6c649fa..1cbbda450b 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/onboarding/OnBoardingPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/onboarding/OnBoardingPresenterTest.kt @@ -214,7 +214,11 @@ class OnBoardingPresenterTest { @Test fun `present - default account provider - login and clear error`() = runTest { - val authenticationService = FakeMatrixAuthenticationService() + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.failure(AN_EXCEPTION) + }, + ) val presenter = createPresenter( params = OnBoardingNode.Params( accountProvider = A_HOMESERVER_URL, @@ -231,7 +235,6 @@ class OnBoardingPresenterTest { skipItems(3) awaitItem().also { assertThat(it.defaultAccountProvider).isEqualTo(A_HOMESERVER_URL) - authenticationService.givenChangeServerError(AN_EXCEPTION) it.eventSink(OnBoardingEvents.OnSignIn(A_HOMESERVER_URL)) skipItems(1) // Loading diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/auth/FakeMatrixAuthenticationService.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/auth/FakeMatrixAuthenticationService.kt index f1554df1d0..7901eafeea 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/auth/FakeMatrixAuthenticationService.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/auth/FakeMatrixAuthenticationService.kt @@ -22,8 +22,6 @@ import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.tests.testutils.lambda.lambdaError import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.simulateLongTask -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow val A_OIDC_DATA = OidcDetails(url = "a-url") @@ -31,13 +29,12 @@ class FakeMatrixAuthenticationService( var matrixClientResult: ((SessionId) -> Result)? = null, var loginWithQrCodeResult: (qrCodeData: MatrixQrCodeLoginData, progress: (QrCodeLoginStep) -> Unit) -> Result = lambdaRecorder Unit, Result> { _, _ -> Result.success(A_SESSION_ID) }, - private val importCreatedSessionLambda: (ExternalSession) -> Result = { lambdaError() } + private val importCreatedSessionLambda: (ExternalSession) -> Result = { lambdaError() }, + private val setHomeserverResult: (String) -> Result = { lambdaError() }, ) : MatrixAuthenticationService { - private val homeserver = MutableStateFlow(null) private var oidcError: Throwable? = null private var oidcCancelError: Throwable? = null private var loginError: Throwable? = null - private var changeServerError: Throwable? = null private var matrixClient: MatrixClient? = null private var onAuthenticationListener: ((MatrixClient) -> Unit)? = null @@ -53,16 +50,8 @@ class FakeMatrixAuthenticationService( } } - override fun getHomeserverDetails(): StateFlow { - return homeserver - } - - fun givenHomeserver(homeserver: MatrixHomeServerDetails) { - this.homeserver.value = homeserver - } - - override suspend fun setHomeserver(homeserver: String): Result = simulateLongTask { - changeServerError?.let { Result.failure(it) } ?: Result.success(Unit) + override suspend fun setHomeserver(homeserver: String): Result = simulateLongTask { + setHomeserverResult(homeserver) } override suspend fun login(username: String, password: String): Result = simulateLongTask { @@ -115,10 +104,6 @@ class FakeMatrixAuthenticationService( loginError = throwable } - fun givenChangeServerError(throwable: Throwable?) { - changeServerError = throwable - } - fun givenMatrixClient(matrixClient: MatrixClient) { this.matrixClient = matrixClient } From 6617db0ce611d0eb44b020fd79eeb87407c7454e Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 6 Nov 2025 14:32:00 +0100 Subject: [PATCH 04/17] MatrixHomeServerDetails does not need to be Parcelable --- .../libraries/matrix/api/auth/MatrixHomeServerDetails.kt | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetails.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetails.kt index c2f0741571..ce3af70121 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetails.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetails.kt @@ -7,12 +7,8 @@ package io.element.android.libraries.matrix.api.auth -import android.os.Parcelable -import kotlinx.parcelize.Parcelize - -@Parcelize data class MatrixHomeServerDetails( val url: String, val supportsPasswordLogin: Boolean, val supportsOidcLogin: Boolean, -) : Parcelable +) From 8fa2c6c85fb79c7d331cfc11e90f4c4d7aeb0538 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 6 Nov 2025 14:39:59 +0100 Subject: [PATCH 05/17] Remove A_HOMESERVER and A_HOMESERVER_OIDC from TestData and replace by local `fun aMatrixHomeServerDetails()`. --- .../login/impl/MatrixHomeServerDetails.kt | 21 ++++++++++++++++ .../changeserver/ChangeServerPresenterTest.kt | 4 +-- .../ConfirmAccountProviderPresenterTest.kt | 25 +++++++++---------- .../LoginPasswordPresenterTest.kt | 10 ++++---- .../android/libraries/matrix/test/TestData.kt | 3 --- 5 files changed, 40 insertions(+), 23 deletions(-) create mode 100644 features/login/impl/src/test/kotlin/io/element/android/features/login/impl/MatrixHomeServerDetails.kt diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/MatrixHomeServerDetails.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/MatrixHomeServerDetails.kt new file mode 100644 index 0000000000..1bf32d20e2 --- /dev/null +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/MatrixHomeServerDetails.kt @@ -0,0 +1,21 @@ +/* + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.features.login.impl + +import io.element.android.libraries.matrix.api.auth.MatrixHomeServerDetails +import io.element.android.libraries.matrix.test.A_HOMESERVER_URL + +fun aMatrixHomeServerDetails( + url: String = A_HOMESERVER_URL, + supportsPasswordLogin: Boolean = false, + supportsOidcLogin: Boolean = false, +) = MatrixHomeServerDetails( + url = url, + supportsPasswordLogin = supportsPasswordLogin, + supportsOidcLogin = supportsOidcLogin, +) diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt index 47f9fee728..d723796265 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt @@ -10,6 +10,7 @@ package io.element.android.features.login.impl.changeserver import com.google.common.truth.Truth.assertThat import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.enterprise.test.FakeEnterpriseService +import io.element.android.features.login.impl.aMatrixHomeServerDetails import io.element.android.features.login.impl.accesscontrol.DefaultAccountProviderAccessControl import io.element.android.features.login.impl.accountprovider.AccountProvider import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource @@ -19,7 +20,6 @@ import io.element.android.features.wellknown.test.anElementWellKnown import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.core.uri.ensureProtocol import io.element.android.libraries.matrix.test.AN_EXCEPTION -import io.element.android.libraries.matrix.test.A_HOMESERVER import io.element.android.libraries.matrix.test.A_HOMESERVER_URL import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService import io.element.android.libraries.wellknown.api.ElementWellKnown @@ -49,7 +49,7 @@ class ChangeServerPresenterTest { fun `present - change server ok`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER) + Result.success(aMatrixHomeServerDetails()) }, ) createPresenter( diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt index 82c2caff61..792abdf1a4 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt @@ -13,6 +13,7 @@ import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.appconfig.AuthenticationConfig import io.element.android.features.enterprise.test.FakeEnterpriseService +import io.element.android.features.login.impl.aMatrixHomeServerDetails import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource import io.element.android.features.login.impl.login.LoginMode import io.element.android.features.login.impl.screens.createaccount.AccountCreationNotSupported @@ -22,8 +23,6 @@ import io.element.android.features.login.impl.web.WebClientUrlForAuthenticationR import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService import io.element.android.libraries.matrix.test.AN_EXCEPTION -import io.element.android.libraries.matrix.test.A_HOMESERVER -import io.element.android.libraries.matrix.test.A_HOMESERVER_OIDC import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService import io.element.android.libraries.oidc.api.OidcAction import io.element.android.libraries.oidc.api.OidcActionFlow @@ -55,7 +54,7 @@ class ConfirmAccountProviderPresenterTest { fun `present - continue password login`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER) + Result.success(aMatrixHomeServerDetails(supportsPasswordLogin = true)) }, ) val presenter = createConfirmAccountProviderPresenter( @@ -80,7 +79,7 @@ class ConfirmAccountProviderPresenterTest { fun `present - continue oidc`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER_OIDC) + Result.success(aMatrixHomeServerDetails(supportsOidcLogin = true)) }, ) val presenter = createConfirmAccountProviderPresenter( @@ -105,7 +104,7 @@ class ConfirmAccountProviderPresenterTest { fun `present - oidc - cancel with failure`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER_OIDC) + Result.success(aMatrixHomeServerDetails(supportsOidcLogin = true)) }, ) val defaultOidcActionFlow = FakeOidcActionFlow() @@ -136,7 +135,7 @@ class ConfirmAccountProviderPresenterTest { fun `present - oidc - cancel with success`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER_OIDC) + Result.success(aMatrixHomeServerDetails(supportsOidcLogin = true)) }, ) val defaultOidcActionFlow = FakeOidcActionFlow() @@ -166,7 +165,7 @@ class ConfirmAccountProviderPresenterTest { fun `present - oidc - cancel to unblock`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER_OIDC) + Result.success(aMatrixHomeServerDetails(supportsOidcLogin = true)) }, ) val defaultOidcActionFlow = FakeOidcActionFlow() @@ -192,7 +191,7 @@ class ConfirmAccountProviderPresenterTest { fun `present - oidc - success with failure`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER_OIDC) + Result.success(aMatrixHomeServerDetails(supportsOidcLogin = true)) }, ) val defaultOidcActionFlow = FakeOidcActionFlow() @@ -225,7 +224,7 @@ class ConfirmAccountProviderPresenterTest { fun `present - oidc - success with success`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER_OIDC) + Result.success(aMatrixHomeServerDetails(supportsOidcLogin = true)) }, ) val defaultOidcActionFlow = FakeOidcActionFlow() @@ -308,7 +307,7 @@ class ConfirmAccountProviderPresenterTest { fun `present - confirm account creation without oidc and without url generates an error`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER) + Result.success(aMatrixHomeServerDetails()) }, ) val presenter = createConfirmAccountProviderPresenter( @@ -338,7 +337,7 @@ class ConfirmAccountProviderPresenterTest { fun `present - confirm account creation with oidc is successful`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER_OIDC) + Result.success(aMatrixHomeServerDetails(supportsOidcLogin = true)) }, ) val presenter = createConfirmAccountProviderPresenter( @@ -362,7 +361,7 @@ class ConfirmAccountProviderPresenterTest { val aUrl = "aUrl" val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER_OIDC) + Result.success(aMatrixHomeServerDetails(supportsOidcLogin = true)) }, ) val presenter = createConfirmAccountProviderPresenter( @@ -387,7 +386,7 @@ class ConfirmAccountProviderPresenterTest { val aUrl = "aUrl" val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER) + Result.success(aMatrixHomeServerDetails()) }, ) val presenter = createConfirmAccountProviderPresenter( diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt index fd048d1db0..bfdc0a6785 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt @@ -10,11 +10,11 @@ package io.element.android.features.login.impl.screens.loginpassword import com.google.common.truth.Truth.assertThat import io.element.android.appconfig.AuthenticationConfig import io.element.android.features.enterprise.test.FakeEnterpriseService +import io.element.android.features.login.impl.aMatrixHomeServerDetails import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.test.AN_EXCEPTION -import io.element.android.libraries.matrix.test.A_HOMESERVER import io.element.android.libraries.matrix.test.A_PASSWORD import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.A_USER_NAME @@ -44,7 +44,7 @@ class LoginPasswordPresenterTest { fun `present - enter login and password`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER) + Result.success(aMatrixHomeServerDetails()) }, ) createLoginPasswordPresenter( @@ -66,7 +66,7 @@ class LoginPasswordPresenterTest { fun `present - submit`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER) + Result.success(aMatrixHomeServerDetails()) }, ) createLoginPasswordPresenter( @@ -89,7 +89,7 @@ class LoginPasswordPresenterTest { fun `present - submit with error`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER) + Result.success(aMatrixHomeServerDetails()) }, ) createLoginPasswordPresenter( @@ -113,7 +113,7 @@ class LoginPasswordPresenterTest { fun `present - clear error`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(A_HOMESERVER) + Result.success(aMatrixHomeServerDetails()) }, ) createLoginPasswordPresenter( diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/TestData.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/TestData.kt index 8c22c90cd7..dc1817cf12 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/TestData.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/TestData.kt @@ -8,7 +8,6 @@ package io.element.android.libraries.matrix.test import androidx.annotation.ColorInt -import io.element.android.libraries.matrix.api.auth.MatrixHomeServerDetails import io.element.android.libraries.matrix.api.core.DeviceId import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.RoomAlias @@ -79,8 +78,6 @@ const val AN_ACCOUNT_PROVIDER = "matrix.org" const val AN_ACCOUNT_PROVIDER_2 = "element.io" const val AN_ACCOUNT_PROVIDER_3 = "other.io" -val A_HOMESERVER = MatrixHomeServerDetails(A_HOMESERVER_URL, supportsPasswordLogin = true, supportsOidcLogin = false) -val A_HOMESERVER_OIDC = MatrixHomeServerDetails(A_HOMESERVER_URL, supportsPasswordLogin = false, supportsOidcLogin = true) val A_ROOM_NOTIFICATION_MODE = RoomNotificationMode.MUTE const val AN_AVATAR_URL = "mxc://data" From e3ed75d19ed036e0cb18e1edd4259610b731ed4d Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 6 Nov 2025 15:07:44 +0100 Subject: [PATCH 06/17] Ensure user cannot select unsupported homeserver. In this case show the appropriate error (parity with iOS) --- .../changeserver/ChangeServerPresenter.kt | 9 +++++- .../changeserver/ChangeServerStateProvider.kt | 8 +++++ .../login/impl/error/ChangeServerError.kt | 1 + .../changeserver/ChangeServerPresenterTest.kt | 29 ++++++++++++++++++- 4 files changed, 45 insertions(+), 2 deletions(-) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt index d3db7496c5..71d4e97c3b 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt @@ -13,6 +13,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import dev.zacsweers.metro.Inject +import io.element.android.features.login.impl.R import io.element.android.features.login.impl.accesscontrol.DefaultAccountProviderAccessControl import io.element.android.features.login.impl.accountprovider.AccountProvider import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource @@ -60,7 +61,13 @@ class ChangeServerPresenter( title = data.title, accountProviderUrl = data.url, ) - authenticationService.setHomeserver(data.url).getOrThrow() + val details = authenticationService.setHomeserver(data.url).getOrThrow() + if (details.supportsOidcLogin.not() && details.supportsPasswordLogin.not()) { + // Unsupported homeserver + throw ChangeServerError.Error( + messageId = R.string.screen_login_error_unsupported_authentication, + ) + } // Homeserver is valid, remember user choice accountProviderDataSource.userSelection(data) }.runCatchingUpdatingState(changeServerAction, errorTransform = ChangeServerError::from) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt index a97ff2dda1..8a7d015750 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt @@ -8,6 +8,7 @@ package io.element.android.features.login.impl.changeserver import androidx.compose.ui.tooling.preview.PreviewParameterProvider +import io.element.android.features.login.impl.R import io.element.android.features.login.impl.error.ChangeServerError import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.ui.strings.CommonStrings @@ -34,6 +35,13 @@ open class ChangeServerStateProvider : PreviewParameterProvider error is AuthenticationException.SlidingSyncVersion -> SlidingSyncAlert is AuthenticationException.Oidc -> Error(messageStr = error.message) is AccountProviderAccessException.NeedElementProException -> NeedElementPro( diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt index d723796265..ce026b447c 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt @@ -10,6 +10,7 @@ package io.element.android.features.login.impl.changeserver import com.google.common.truth.Truth.assertThat import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.enterprise.test.FakeEnterpriseService +import io.element.android.features.login.impl.R import io.element.android.features.login.impl.aMatrixHomeServerDetails import io.element.android.features.login.impl.accesscontrol.DefaultAccountProviderAccessControl import io.element.android.features.login.impl.accountprovider.AccountProvider @@ -49,7 +50,7 @@ class ChangeServerPresenterTest { fun `present - change server ok`() = runTest { val authenticationService = FakeMatrixAuthenticationService( setHomeserverResult = { - Result.success(aMatrixHomeServerDetails()) + Result.success(aMatrixHomeServerDetails(supportsOidcLogin = true)) }, ) createPresenter( @@ -95,6 +96,32 @@ class ChangeServerPresenterTest { } } + @Test + fun `present - change server unsupported server`() = runTest { + val authenticationService = FakeMatrixAuthenticationService( + setHomeserverResult = { + Result.success(aMatrixHomeServerDetails()) + }, + ) + createPresenter( + enterpriseService = FakeEnterpriseService( + isAllowedToConnectToHomeserverResult = { true }, + ), + authenticationService = authenticationService, + ).test { + val initialState = awaitItem() + assertThat(initialState.changeServerAction).isEqualTo(AsyncData.Uninitialized) + initialState.eventSink.invoke(ChangeServerEvents.ChangeServer(AccountProvider(url = A_HOMESERVER_URL))) + val loadingState = awaitItem() + assertThat(loadingState.changeServerAction).isInstanceOf(AsyncData.Loading::class.java) + val failureState = awaitItem() + assertThat(failureState.changeServerAction).isInstanceOf(AsyncData.Failure::class.java) + assertThat(failureState.changeServerAction.errorOrNull()).isEqualTo( + ChangeServerError.Error(R.string.screen_login_error_unsupported_authentication) + ) + } + } + @Test fun `present - change server not allowed error`() = runTest { val isAllowedToConnectToHomeserverResult = lambdaRecorder { false } From a9958505d375d8b6d7e20a5f4b40b72bbe280d44 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 6 Nov 2025 15:21:50 +0100 Subject: [PATCH 07/17] Always let the user try what they have entered, to get an explicit error if they continue --- .../features/login/impl/resolver/HomeserverResolver.kt | 7 ++++--- .../SearchAccountProviderPresenterTest.kt | 8 +++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/resolver/HomeserverResolver.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/resolver/HomeserverResolver.kt index c43839517c..4cf1d416e8 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/resolver/HomeserverResolver.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/resolver/HomeserverResolver.kt @@ -52,9 +52,10 @@ class HomeserverResolver( } } } - // If list is empty, and the user has entered an URL, do not block the user. - if (currentList.isEmpty() && trimmedUserInput.isValidUrl()) { - emit(listOf(HomeserverData(homeserverUrl = trimmedUserInput))) + // If list is empty, and candidateBase is a valid an URL, do not block the user. + // A unsupported homeserver / homeserver not found error will be displayed if the user continues + if (currentList.isEmpty() && candidateBase.isValidUrl()) { + emit(listOf(HomeserverData(homeserverUrl = candidateBase))) } } diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderPresenterTest.kt index 67453119c7..f4c193ced2 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderPresenterTest.kt @@ -85,7 +85,13 @@ class SearchAccountProviderPresenterTest { assertThat(withInputState.userInput).isEqualTo("test") assertThat(initialState.userInputResult).isEqualTo(AsyncData.Uninitialized) assertThat(awaitItem().userInputResult).isInstanceOf(AsyncData.Loading::class.java) - assertThat(awaitItem().userInputResult).isEqualTo(AsyncData.Uninitialized) + assertThat(awaitItem().userInputResult).isEqualTo( + AsyncData.Success( + listOf( + aHomeserverData(homeserverUrl = "https://test"), + ) + ) + ) } } From fd0ef1ae7a12fd81c2feac61daf85cfc6f23eae1 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 6 Nov 2025 15:22:19 +0100 Subject: [PATCH 08/17] Small cleanup --- .../searchaccountprovider/SearchAccountProviderStateProvider.kt | 2 +- .../notifications/DefaultActiveNotificationsProviderTest.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderStateProvider.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderStateProvider.kt index 3dd7a3d8c5..97cc861911 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderStateProvider.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderStateProvider.kt @@ -43,5 +43,5 @@ fun aHomeserverDataList(): List { fun aHomeserverData( homeserverUrl: String = AuthenticationConfig.MATRIX_ORG_URL, ): HomeserverData { - return HomeserverData(homeserverUrl = homeserverUrl,) + return HomeserverData(homeserverUrl = homeserverUrl) } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultActiveNotificationsProviderTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultActiveNotificationsProviderTest.kt index a0ce8b2edb..b06842ccdb 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultActiveNotificationsProviderTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultActiveNotificationsProviderTest.kt @@ -44,7 +44,7 @@ class DefaultActiveNotificationsProviderTest { @Test fun `getMembershipNotificationsForSession returns only membership notifications for that session id`() { val activeNotifications = listOf( - aStatusBarNotification(id = notificationIdProvider.getRoomMessagesNotificationId(A_SESSION_ID), groupId = A_SESSION_ID.value,), + aStatusBarNotification(id = notificationIdProvider.getRoomMessagesNotificationId(A_SESSION_ID), groupId = A_SESSION_ID.value), aStatusBarNotification(id = notificationIdProvider.getSummaryNotificationId(A_SESSION_ID_2), groupId = A_SESSION_ID_2.value), aStatusBarNotification( id = notificationIdProvider.getRoomInvitationNotificationId(A_SESSION_ID_2), From 5b472fdc313ee05df8d06b32d03d5756efee16a8 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 6 Nov 2025 15:25:39 +0100 Subject: [PATCH 09/17] Use presenter test extension --- .../ChangeAccountProviderPresenterTest.kt | 16 ++--- .../ConfirmAccountProviderPresenterTest.kt | 60 +++++-------------- .../CreateAccountPresenterTest.kt | 28 +++------ .../qrcode/intro/QrCodeIntroPresenterTest.kt | 16 ++--- .../qrcode/scan/QrCodeScanPresenterTest.kt | 15 +---- .../SearchAccountProviderPresenterTest.kt | 24 ++------ 6 files changed, 39 insertions(+), 120 deletions(-) diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/changeaccountprovider/ChangeAccountProviderPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/changeaccountprovider/ChangeAccountProviderPresenterTest.kt index f2e933390b..99fa65876d 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/changeaccountprovider/ChangeAccountProviderPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/changeaccountprovider/ChangeAccountProviderPresenterTest.kt @@ -7,9 +7,6 @@ package io.element.android.features.login.impl.screens.changeaccountprovider -import app.cash.molecule.RecompositionMode -import app.cash.molecule.moleculeFlow -import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.enterprise.test.FakeEnterpriseService @@ -18,6 +15,7 @@ import io.element.android.features.login.impl.changeserver.aChangeServerState import io.element.android.libraries.matrix.test.AN_ACCOUNT_PROVIDER import io.element.android.libraries.matrix.test.AN_ACCOUNT_PROVIDER_2 import io.element.android.tests.testutils.WarmUpRule +import io.element.android.tests.testutils.test import kotlinx.coroutines.test.runTest import org.junit.Rule import org.junit.Test @@ -34,9 +32,7 @@ class ChangeAccountProviderPresenterTest { defaultHomeserverListResult = { emptyList() } ), ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() assertThat(initialState.accountProviders).isEqualTo( listOf( @@ -63,9 +59,7 @@ class ChangeAccountProviderPresenterTest { } ), ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() assertThat(initialState.accountProviders).isEqualTo( listOf( @@ -99,9 +93,7 @@ class ChangeAccountProviderPresenterTest { } ), ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() assertThat(initialState.accountProviders).isEqualTo( listOf( diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt index 792abdf1a4..182b7d6e27 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt @@ -7,9 +7,6 @@ package io.element.android.features.login.impl.screens.confirmaccountprovider -import app.cash.molecule.RecompositionMode -import app.cash.molecule.moleculeFlow -import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.appconfig.AuthenticationConfig import io.element.android.features.enterprise.test.FakeEnterpriseService @@ -28,6 +25,7 @@ import io.element.android.libraries.oidc.api.OidcAction import io.element.android.libraries.oidc.api.OidcActionFlow import io.element.android.libraries.oidc.test.customtab.FakeOidcActionFlow import io.element.android.tests.testutils.WarmUpRule +import io.element.android.tests.testutils.test import kotlinx.coroutines.test.runTest import org.junit.Rule import org.junit.Test @@ -39,9 +37,7 @@ class ConfirmAccountProviderPresenterTest { @Test fun `present - initial test`() = runTest { val presenter = createConfirmAccountProviderPresenter() - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() assertThat(initialState.isAccountCreation).isFalse() assertThat(initialState.submitEnabled).isTrue() @@ -60,9 +56,7 @@ class ConfirmAccountProviderPresenterTest { val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(ConfirmAccountProviderEvents.Continue) val loadingState = awaitItem() @@ -85,9 +79,7 @@ class ConfirmAccountProviderPresenterTest { val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(ConfirmAccountProviderEvents.Continue) val loadingState = awaitItem() @@ -112,9 +104,7 @@ class ConfirmAccountProviderPresenterTest { matrixAuthenticationService = authenticationService, defaultOidcActionFlow = defaultOidcActionFlow, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(ConfirmAccountProviderEvents.Continue) val loadingState = awaitItem() @@ -143,9 +133,7 @@ class ConfirmAccountProviderPresenterTest { matrixAuthenticationService = authenticationService, defaultOidcActionFlow = defaultOidcActionFlow, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(ConfirmAccountProviderEvents.Continue) val loadingState = awaitItem() @@ -173,9 +161,7 @@ class ConfirmAccountProviderPresenterTest { matrixAuthenticationService = authenticationService, defaultOidcActionFlow = defaultOidcActionFlow, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(ConfirmAccountProviderEvents.Continue) val loadingState = awaitItem() @@ -199,9 +185,7 @@ class ConfirmAccountProviderPresenterTest { matrixAuthenticationService = authenticationService, defaultOidcActionFlow = defaultOidcActionFlow, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(ConfirmAccountProviderEvents.Continue) val loadingState = awaitItem() @@ -232,9 +216,7 @@ class ConfirmAccountProviderPresenterTest { matrixAuthenticationService = authenticationService, defaultOidcActionFlow = defaultOidcActionFlow, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(ConfirmAccountProviderEvents.Continue) val loadingState = awaitItem() @@ -260,9 +242,7 @@ class ConfirmAccountProviderPresenterTest { val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(ConfirmAccountProviderEvents.Continue) skipItems(1) // Loading @@ -282,9 +262,7 @@ class ConfirmAccountProviderPresenterTest { val presenter = createConfirmAccountProviderPresenter( matrixAuthenticationService = authenticationService, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() // Submit will return an error @@ -317,9 +295,7 @@ class ConfirmAccountProviderPresenterTest { throw AccountCreationNotSupported() }, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink(ConfirmAccountProviderEvents.Continue) skipItems(1) // Loading @@ -344,9 +320,7 @@ class ConfirmAccountProviderPresenterTest { params = ConfirmAccountProviderPresenter.Params(isAccountCreation = true), matrixAuthenticationService = authenticationService, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink(ConfirmAccountProviderEvents.Continue) skipItems(1) // Loading @@ -369,9 +343,7 @@ class ConfirmAccountProviderPresenterTest { matrixAuthenticationService = authenticationService, webClientUrlForAuthenticationRetriever = FakeWebClientUrlForAuthenticationRetriever { aUrl }, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink(ConfirmAccountProviderEvents.Continue) skipItems(1) // Loading @@ -394,9 +366,7 @@ class ConfirmAccountProviderPresenterTest { matrixAuthenticationService = authenticationService, webClientUrlForAuthenticationRetriever = FakeWebClientUrlForAuthenticationRetriever { aUrl }, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink(ConfirmAccountProviderEvents.Continue) skipItems(1) // Loading diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/createaccount/CreateAccountPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/createaccount/CreateAccountPresenterTest.kt index 8d70173d11..63f68b2221 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/createaccount/CreateAccountPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/createaccount/CreateAccountPresenterTest.kt @@ -7,9 +7,6 @@ package io.element.android.features.login.impl.screens.createaccount -import app.cash.molecule.RecompositionMode -import app.cash.molecule.moleculeFlow -import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.core.meta.BuildMeta @@ -26,6 +23,7 @@ import io.element.android.libraries.matrix.test.verification.FakeSessionVerifica import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value +import io.element.android.tests.testutils.test import kotlinx.coroutines.test.runTest import org.junit.Rule import org.junit.Test @@ -37,9 +35,7 @@ class CreateAccountPresenterTest { @Test fun `present - initial state`() = runTest { val presenter = createPresenter() - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() assertThat(initialState.url).isEqualTo("aUrl") assertThat(initialState.pageProgress).isEqualTo(0) @@ -51,9 +47,7 @@ class CreateAccountPresenterTest { @Test fun `present - set up progress update the state`() = runTest { val presenter = createPresenter() - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink(CreateAccountEvents.SetPageProgress(33)) assertThat(awaitItem().pageProgress).isEqualTo(33) @@ -65,9 +59,7 @@ class CreateAccountPresenterTest { val presenter = createPresenter( messageParser = FakeMessageParser { error("An error") } ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink(CreateAccountEvents.OnMessageReceived("")) assertThat(awaitItem().createAction).isInstanceOf(AsyncAction.Failure::class.java) @@ -77,9 +69,7 @@ class CreateAccountPresenterTest { @Test fun `present - receiving a message containing isTrusted is ignored`() = runTest { val presenter = createPresenter() - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink(CreateAccountEvents.OnMessageReceived("isTrusted")) } @@ -98,9 +88,7 @@ class CreateAccountPresenterTest { messageParser = FakeMessageParser(lambda), clientProvider = clientProvider, ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink(CreateAccountEvents.OnMessageReceived("aMessage")) assertThat(awaitItem().createAction.isLoading()).isTrue() @@ -118,9 +106,7 @@ class CreateAccountPresenterTest { ), messageParser = FakeMessageParser { anExternalSession() } ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink(CreateAccountEvents.OnMessageReceived("")) assertThat(awaitItem().createAction.isLoading()).isTrue() diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/qrcode/intro/QrCodeIntroPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/qrcode/intro/QrCodeIntroPresenterTest.kt index 83686b56c6..6cc34fd32f 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/qrcode/intro/QrCodeIntroPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/qrcode/intro/QrCodeIntroPresenterTest.kt @@ -7,14 +7,12 @@ package io.element.android.features.login.impl.screens.qrcode.intro -import app.cash.molecule.RecompositionMode -import app.cash.molecule.moleculeFlow -import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.libraries.core.meta.BuildMeta import io.element.android.libraries.matrix.test.core.aBuildMeta import io.element.android.libraries.permissions.test.FakePermissionsPresenter import io.element.android.libraries.permissions.test.FakePermissionsPresenterFactory +import io.element.android.tests.testutils.test import kotlinx.coroutines.test.runTest import org.junit.Test @@ -22,9 +20,7 @@ class QrCodeIntroPresenterTest { @Test fun `present - initial state`() = runTest { val presenter = createQrCodeIntroPresenter() - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { awaitItem().run { assertThat(appName).isEqualTo("AppName") assertThat(desktopAppName).isEqualTo("DesktopAppName") @@ -39,9 +35,7 @@ class QrCodeIntroPresenterTest { val permissionsPresenter = FakePermissionsPresenter().apply { setPermissionGranted() } val permissionsPresenterFactory = FakePermissionsPresenterFactory(permissionsPresenter) val presenter = createQrCodeIntroPresenter(permissionsPresenterFactory = permissionsPresenterFactory) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { awaitItem().eventSink(QrCodeIntroEvents.Continue) assertThat(awaitItem().canContinue).isTrue() } @@ -52,9 +46,7 @@ class QrCodeIntroPresenterTest { val permissionsPresenter = FakePermissionsPresenter() val permissionsPresenterFactory = FakePermissionsPresenterFactory(permissionsPresenter) val presenter = createQrCodeIntroPresenter(permissionsPresenterFactory = permissionsPresenterFactory) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { awaitItem().eventSink(QrCodeIntroEvents.Continue) assertThat(awaitItem().cameraPermissionState.showDialog).isTrue() } diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/qrcode/scan/QrCodeScanPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/qrcode/scan/QrCodeScanPresenterTest.kt index a4d594399f..058f417fd1 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/qrcode/scan/QrCodeScanPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/qrcode/scan/QrCodeScanPresenterTest.kt @@ -7,9 +7,6 @@ package io.element.android.features.login.impl.screens.qrcode.scan -import app.cash.molecule.RecompositionMode -import app.cash.molecule.moleculeFlow -import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.enterprise.test.FakeEnterpriseService @@ -34,9 +31,7 @@ class QrCodeScanPresenterTest { @Test fun `present - initial state`() = runTest { val presenter = createQrCodeScanPresenter() - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { awaitItem().run { assertThat(isScanning).isTrue() assertThat(authenticationAction.isUninitialized()).isTrue() @@ -114,9 +109,7 @@ class QrCodeScanPresenterTest { parseQrCodeLoginDataResult = { Result.failure(Exception("Failed to parse QR code")) } ) val presenter = createQrCodeScanPresenter(qrCodeLoginDataFactory = qrCodeLoginDataFactory) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink(QrCodeScanEvents.QrCodeScanned(byteArrayOf())) assertThat(awaitItem().isScanning).isFalse() @@ -140,9 +133,7 @@ class QrCodeScanPresenterTest { } qrCodeLoginManager.resetAction = resetAction val presenter = createQrCodeScanPresenter(qrCodeLoginDataFactory = qrCodeLoginDataFactory, qrCodeLoginManager = qrCodeLoginManager) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { // Skip initial item skipItems(1) diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderPresenterTest.kt index f4c193ced2..f19743bac0 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/searchaccountprovider/SearchAccountProviderPresenterTest.kt @@ -7,9 +7,6 @@ package io.element.android.features.login.impl.screens.searchaccountprovider -import app.cash.molecule.RecompositionMode -import app.cash.molecule.moleculeFlow -import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.features.login.impl.changeserver.aChangeServerState import io.element.android.features.login.impl.resolver.HomeserverResolver @@ -18,6 +15,7 @@ import io.element.android.libraries.matrix.test.auth.FakeHomeServerLoginCompatib import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value +import io.element.android.tests.testutils.test import io.element.android.tests.testutils.testCoroutineDispatchers import kotlinx.coroutines.test.runTest import org.junit.Rule @@ -34,9 +32,7 @@ class SearchAccountProviderPresenterTest { homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeLoginCompatibilityChecker), changeServerPresenter = { aChangeServerState() } ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() assertThat(initialState.userInput).isEmpty() assertThat(initialState.userInputResult).isEqualTo(AsyncData.Uninitialized) @@ -50,9 +46,7 @@ class SearchAccountProviderPresenterTest { homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeLoginCompatibilityChecker), changeServerPresenter = { aChangeServerState() } ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(SearchAccountProviderEvents.UserInput("https://test.org")) val withInputState = awaitItem() @@ -76,9 +70,7 @@ class SearchAccountProviderPresenterTest { homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeWellknownRetriever), changeServerPresenter = { aChangeServerState() } ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(SearchAccountProviderEvents.UserInput("test")) val withInputState = awaitItem() @@ -111,9 +103,7 @@ class SearchAccountProviderPresenterTest { homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeLoginCompatibilityChecker), changeServerPresenter = { aChangeServerState() } ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(SearchAccountProviderEvents.UserInput("test")) val withInputState = awaitItem() @@ -153,9 +143,7 @@ class SearchAccountProviderPresenterTest { homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeLoginCompatibilityChecker), changeServerPresenter = { aChangeServerState() } ) - moleculeFlow(RecompositionMode.Immediate) { - presenter.present() - }.test { + presenter.test { val initialState = awaitItem() initialState.eventSink.invoke(SearchAccountProviderEvents.UserInput("test")) val withInputState = awaitItem() From 0657a201ead6eddf9ebe8a560c584764602b9016 Mon Sep 17 00:00:00 2001 From: ElementBot Date: Thu, 6 Nov 2025 14:58:50 +0000 Subject: [PATCH 10/17] Update screenshots --- ...tures.login.impl.changeserver_ChangeServerView_Day_5_en.png | 3 +++ ...res.login.impl.changeserver_ChangeServerView_Night_5_en.png | 3 +++ 2 files changed, 6 insertions(+) create mode 100644 tests/uitests/src/test/snapshots/images/features.login.impl.changeserver_ChangeServerView_Day_5_en.png create mode 100644 tests/uitests/src/test/snapshots/images/features.login.impl.changeserver_ChangeServerView_Night_5_en.png diff --git a/tests/uitests/src/test/snapshots/images/features.login.impl.changeserver_ChangeServerView_Day_5_en.png b/tests/uitests/src/test/snapshots/images/features.login.impl.changeserver_ChangeServerView_Day_5_en.png new file mode 100644 index 0000000000..1125024a5a --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.login.impl.changeserver_ChangeServerView_Day_5_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ae6d230961018f1128ea655529a4287dbe3bbbc2762079d0c4f335313d2f2ab0 +size 25166 diff --git a/tests/uitests/src/test/snapshots/images/features.login.impl.changeserver_ChangeServerView_Night_5_en.png b/tests/uitests/src/test/snapshots/images/features.login.impl.changeserver_ChangeServerView_Night_5_en.png new file mode 100644 index 0000000000..777f0bb627 --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.login.impl.changeserver_ChangeServerView_Night_5_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:1502711e6305f9adf4719b28c1fe3755089ae6c42c6c66e8e5bf73ab2bbb65c1 +size 23874 From b61ce1b19cf46a5a196accb4be7c5f111af0eeba Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 7 Nov 2025 09:41:57 +0100 Subject: [PATCH 11/17] Create specific errors for Invalid or Unsupporte homeserver. --- .../impl/changeserver/ChangeServerPresenter.kt | 6 +----- .../changeserver/ChangeServerStateProvider.kt | 5 +---- .../login/impl/changeserver/ChangeServerView.kt | 16 ++++++++++++++++ .../login/impl/error/ChangeServerError.kt | 5 +++-- .../impl/error/ChangeServerErrorProvider.kt | 6 ++---- .../features/login/impl/login/LoginModeView.kt | 11 +++++++++++ .../changeserver/ChangeServerPresenterTest.kt | 3 +-- 7 files changed, 35 insertions(+), 17 deletions(-) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt index 71d4e97c3b..fc114aaa6b 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt @@ -13,7 +13,6 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import dev.zacsweers.metro.Inject -import io.element.android.features.login.impl.R import io.element.android.features.login.impl.accesscontrol.DefaultAccountProviderAccessControl import io.element.android.features.login.impl.accountprovider.AccountProvider import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource @@ -63,10 +62,7 @@ class ChangeServerPresenter( ) val details = authenticationService.setHomeserver(data.url).getOrThrow() if (details.supportsOidcLogin.not() && details.supportsPasswordLogin.not()) { - // Unsupported homeserver - throw ChangeServerError.Error( - messageId = R.string.screen_login_error_unsupported_authentication, - ) + throw ChangeServerError.UnsupportedServer } // Homeserver is valid, remember user choice accountProviderDataSource.userSelection(data) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt index 8a7d015750..554c9b545e 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt @@ -8,7 +8,6 @@ package io.element.android.features.login.impl.changeserver import androidx.compose.ui.tooling.preview.PreviewParameterProvider -import io.element.android.features.login.impl.R import io.element.android.features.login.impl.error.ChangeServerError import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.ui.strings.CommonStrings @@ -37,9 +36,7 @@ open class ChangeServerStateProvider : PreviewParameterProvider { when (val error = state.changeServerAction.error as? ChangeServerError) { + ChangeServerError.InvalidServer -> + ErrorDialog( + modifier = modifier, + content = stringResource(R.string.screen_change_server_error_invalid_homeserver), + onSubmit = { + eventSink.invoke(ChangeServerEvents.ClearError) + } + ) + ChangeServerError.UnsupportedServer -> + ErrorDialog( + modifier = modifier, + content = stringResource(R.string.screen_login_error_unsupported_authentication), + onSubmit = { + eventSink.invoke(ChangeServerEvents.ClearError) + } + ) is ChangeServerError.Error -> { ErrorDialog( modifier = modifier, diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt index a814e61aaa..2c4550953e 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt @@ -11,7 +11,6 @@ import androidx.annotation.StringRes import androidx.compose.runtime.Composable import androidx.compose.runtime.ReadOnlyComposable import androidx.compose.ui.res.stringResource -import io.element.android.features.login.impl.R import io.element.android.features.login.impl.changeserver.AccountProviderAccessException import io.element.android.libraries.matrix.api.auth.AuthenticationException import io.element.android.libraries.ui.strings.CommonStrings @@ -37,6 +36,8 @@ sealed class ChangeServerError : Exception() { ) : ChangeServerError() data object SlidingSyncAlert : ChangeServerError() + data object InvalidServer : ChangeServerError() + data object UnsupportedServer : ChangeServerError() companion object { fun from(error: Throwable): ChangeServerError = when (error) { @@ -51,7 +52,7 @@ sealed class ChangeServerError : Exception() { unauthorisedAccountProviderTitle = error.unauthorisedAccountProviderTitle, authorisedAccountProviderTitles = error.authorisedAccountProviderTitles, ) - else -> Error(messageId = R.string.screen_change_server_error_invalid_homeserver) + else -> InvalidServer } } } diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerErrorProvider.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerErrorProvider.kt index 333347851a..4a9a7e934b 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerErrorProvider.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerErrorProvider.kt @@ -8,14 +8,11 @@ package io.element.android.features.login.impl.error import androidx.compose.ui.tooling.preview.PreviewParameterProvider -import io.element.android.features.login.impl.R class ChangeServerErrorProvider : PreviewParameterProvider { override val values: Sequence get() = sequenceOf( - ChangeServerError.Error( - messageId = R.string.screen_change_server_error_invalid_homeserver, - ), + ChangeServerError.InvalidServer, ChangeServerError.Error( messageStr = "An error description", ), @@ -28,5 +25,6 @@ class ChangeServerErrorProvider : PreviewParameterProvider { authorisedAccountProviderTitles = listOf("provider.org", "provider.io"), ), ChangeServerError.SlidingSyncAlert, + ChangeServerError.UnsupportedServer, ) } diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginModeView.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginModeView.kt index c3fe5eac47..4c97897cce 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginModeView.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginModeView.kt @@ -41,6 +41,17 @@ fun LoginModeView( when (val error = loginMode.error) { is ChangeServerError -> { when (error) { + ChangeServerError.InvalidServer -> + ErrorDialog( + content = stringResource(R.string.screen_change_server_error_invalid_homeserver), + onSubmit = onClearError, + ) + is ChangeServerError.UnsupportedServer -> { + ErrorDialog( + content = stringResource(R.string.screen_login_error_unsupported_authentication), + onSubmit = onClearError, + ) + } is ChangeServerError.Error -> { ErrorDialog( content = error.message(), diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt index ce026b447c..d1090cb237 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt @@ -10,7 +10,6 @@ package io.element.android.features.login.impl.changeserver import com.google.common.truth.Truth.assertThat import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.enterprise.test.FakeEnterpriseService -import io.element.android.features.login.impl.R import io.element.android.features.login.impl.aMatrixHomeServerDetails import io.element.android.features.login.impl.accesscontrol.DefaultAccountProviderAccessControl import io.element.android.features.login.impl.accountprovider.AccountProvider @@ -117,7 +116,7 @@ class ChangeServerPresenterTest { val failureState = awaitItem() assertThat(failureState.changeServerAction).isInstanceOf(AsyncData.Failure::class.java) assertThat(failureState.changeServerAction.errorOrNull()).isEqualTo( - ChangeServerError.Error(R.string.screen_login_error_unsupported_authentication) + ChangeServerError.UnsupportedServer ) } } From e12726f40526f30edd97bf1295881fcf7ff13fd9 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 7 Nov 2025 09:52:04 +0100 Subject: [PATCH 12/17] Improve error mapping --- .../changeserver/ChangeServerStateProvider.kt | 3 +-- .../login/impl/changeserver/ChangeServerView.kt | 3 ++- .../login/impl/error/ChangeServerError.kt | 14 ++------------ .../features/login/impl/login/LoginModeView.kt | 4 ++-- .../matrix/api/auth/AuthenticationException.kt | 15 +++++++++------ .../matrix/impl/auth/AuthenticationException.kt | 1 - 6 files changed, 16 insertions(+), 24 deletions(-) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt index 554c9b545e..020d5a0851 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerStateProvider.kt @@ -10,13 +10,12 @@ package io.element.android.features.login.impl.changeserver import androidx.compose.ui.tooling.preview.PreviewParameterProvider import io.element.android.features.login.impl.error.ChangeServerError import io.element.android.libraries.architecture.AsyncData -import io.element.android.libraries.ui.strings.CommonStrings open class ChangeServerStateProvider : PreviewParameterProvider { override val values: Sequence get() = sequenceOf( aChangeServerState(), - aChangeServerState(changeServerAction = AsyncData.Failure(ChangeServerError.Error(CommonStrings.error_unknown))), + aChangeServerState(changeServerAction = AsyncData.Failure(ChangeServerError.Error(null))), aChangeServerState(changeServerAction = AsyncData.Failure(ChangeServerError.SlidingSyncAlert)), aChangeServerState( changeServerAction = AsyncData.Failure( diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerView.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerView.kt index a9784580d1..bf13250f7f 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerView.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerView.kt @@ -26,6 +26,7 @@ import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog import io.element.android.libraries.designsystem.preview.ElementPreview import io.element.android.libraries.designsystem.preview.PreviewsDayNight import io.element.android.libraries.designsystem.theme.LocalBuildMeta +import io.element.android.libraries.ui.strings.CommonStrings @Composable fun ChangeServerView( @@ -58,7 +59,7 @@ fun ChangeServerView( is ChangeServerError.Error -> { ErrorDialog( modifier = modifier, - content = error.message(), + content = error.messageStr ?: stringResource(CommonStrings.error_unknown), onSubmit = { eventSink.invoke(ChangeServerEvents.ClearError) } diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt index 2c4550953e..cd5dcf95b3 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt @@ -7,23 +7,13 @@ package io.element.android.features.login.impl.error -import androidx.annotation.StringRes -import androidx.compose.runtime.Composable -import androidx.compose.runtime.ReadOnlyComposable -import androidx.compose.ui.res.stringResource import io.element.android.features.login.impl.changeserver.AccountProviderAccessException import io.element.android.libraries.matrix.api.auth.AuthenticationException -import io.element.android.libraries.ui.strings.CommonStrings sealed class ChangeServerError : Exception() { data class Error( - @StringRes val messageId: Int? = null, val messageStr: String? = null, - ) : ChangeServerError() { - @Composable - @ReadOnlyComposable - fun message(): String = messageStr ?: stringResource(messageId ?: CommonStrings.error_unknown) - } + ) : ChangeServerError() data class NeedElementPro( val unauthorisedAccountProviderTitle: String, @@ -52,7 +42,7 @@ sealed class ChangeServerError : Exception() { unauthorisedAccountProviderTitle = error.unauthorisedAccountProviderTitle, authorisedAccountProviderTitles = error.authorisedAccountProviderTitles, ) - else -> InvalidServer + else -> Error(messageStr = error.message) } } } diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginModeView.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginModeView.kt index 4c97897cce..1c5d1f929a 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginModeView.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/login/LoginModeView.kt @@ -54,7 +54,7 @@ fun LoginModeView( } is ChangeServerError.Error -> { ErrorDialog( - content = error.message(), + content = error.messageStr ?: stringResource(CommonStrings.error_unknown), onSubmit = onClearError, ) } @@ -102,7 +102,7 @@ fun LoginModeView( } is AuthenticationException.AccountAlreadyLoggedIn -> { ErrorDialog( - content = stringResource(CommonStrings.error_account_already_logged_in, error.message.orEmpty()), + content = stringResource(CommonStrings.error_account_already_logged_in, error.userId), onSubmit = onClearError, ) } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/AuthenticationException.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/AuthenticationException.kt index 03e8d57150..7370135729 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/AuthenticationException.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/AuthenticationException.kt @@ -7,10 +7,13 @@ package io.element.android.libraries.matrix.api.auth -sealed class AuthenticationException(message: String) : Exception(message) { - class AccountAlreadyLoggedIn(userId: String) : AuthenticationException(userId) - class InvalidServerName(message: String) : AuthenticationException(message) - class SlidingSyncVersion(message: String) : AuthenticationException(message) - class Oidc(message: String) : AuthenticationException(message) - class Generic(message: String) : AuthenticationException(message) +sealed class AuthenticationException(message: String?) : Exception(message) { + data class AccountAlreadyLoggedIn( + val userId: String, + ) : AuthenticationException(null) + + class InvalidServerName(message: String?) : AuthenticationException(message) + class SlidingSyncVersion(message: String?) : AuthenticationException(message) + class Oidc(message: String?) : AuthenticationException(message) + class Generic(message: String?) : AuthenticationException(message) } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt index 05eb4c4d5f..1960fe7c60 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt @@ -12,7 +12,6 @@ import org.matrix.rustcomponents.sdk.ClientBuildException import org.matrix.rustcomponents.sdk.OidcException fun Throwable.mapAuthenticationException(): AuthenticationException { - val message = this.message ?: "Unknown error" return when (this) { is AuthenticationException -> this is ClientBuildException -> when (this) { From 98792c9562b6c349a93d11220ba7a6f993f8a8ba Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 7 Nov 2025 10:04:48 +0100 Subject: [PATCH 13/17] Improve error mapping --- .../features/login/impl/error/ChangeServerError.kt | 13 +++++++++++-- .../matrix/api/auth/AuthenticationException.kt | 1 + .../matrix/impl/auth/AuthenticationException.kt | 2 +- .../impl/auth/AuthenticationExceptionMappingTest.kt | 6 +++--- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt index cd5dcf95b3..02b71b7291 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/error/ChangeServerError.kt @@ -32,8 +32,17 @@ sealed class ChangeServerError : Exception() { companion object { fun from(error: Throwable): ChangeServerError = when (error) { is ChangeServerError -> error - is AuthenticationException.SlidingSyncVersion -> SlidingSyncAlert - is AuthenticationException.Oidc -> Error(messageStr = error.message) + is AuthenticationException -> { + when (error) { + is AuthenticationException.SlidingSyncVersion -> SlidingSyncAlert + is AuthenticationException.InvalidServerName, + is AuthenticationException.ServerUnreachable -> InvalidServer + // AccountAlreadyLoggedIn error should not happen at this point + is AuthenticationException.AccountAlreadyLoggedIn -> Error(messageStr = error.message) + is AuthenticationException.Generic -> Error(messageStr = error.message) + is AuthenticationException.Oidc -> Error(messageStr = error.message) + } + } is AccountProviderAccessException.NeedElementProException -> NeedElementPro( unauthorisedAccountProviderTitle = error.unauthorisedAccountProviderTitle, applicationId = error.applicationId, diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/AuthenticationException.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/AuthenticationException.kt index 7370135729..954a8e9de2 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/AuthenticationException.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/AuthenticationException.kt @@ -14,6 +14,7 @@ sealed class AuthenticationException(message: String?) : Exception(message) { class InvalidServerName(message: String?) : AuthenticationException(message) class SlidingSyncVersion(message: String?) : AuthenticationException(message) + class ServerUnreachable(message: String?) : AuthenticationException(message) class Oidc(message: String?) : AuthenticationException(message) class Generic(message: String?) : AuthenticationException(message) } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt index 1960fe7c60..ba43065fad 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationException.kt @@ -19,7 +19,7 @@ fun Throwable.mapAuthenticationException(): AuthenticationException { is ClientBuildException.InvalidServerName -> AuthenticationException.InvalidServerName(message) is ClientBuildException.SlidingSyncVersion -> AuthenticationException.SlidingSyncVersion(message) is ClientBuildException.Sdk -> AuthenticationException.Generic(message) - is ClientBuildException.ServerUnreachable -> AuthenticationException.Generic(message) + is ClientBuildException.ServerUnreachable -> AuthenticationException.ServerUnreachable(message) is ClientBuildException.SlidingSync -> AuthenticationException.Generic(message) is ClientBuildException.WellKnownDeserializationException -> AuthenticationException.Generic(message) is ClientBuildException.WellKnownLookupFailed -> AuthenticationException.Generic(message) diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationExceptionMappingTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationExceptionMappingTest.kt index c73ba1424b..db37fc40b5 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationExceptionMappingTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationExceptionMappingTest.kt @@ -16,10 +16,10 @@ import org.matrix.rustcomponents.sdk.OidcException class AuthenticationExceptionMappingTest { @Test - fun `mapping an exception with no message returns 'Unknown error' message`() { + fun `mapping an exception with no message returns null message`() { val exception = Exception() val mappedException = exception.mapAuthenticationException() - assertThat(mappedException.message).isEqualTo("Unknown error") + assertThat(mappedException.message).isEqualTo(null) } @Test @@ -46,7 +46,7 @@ class AuthenticationExceptionMappingTest { assertThat(ClientBuildException.Sdk("SDK issue").mapAuthenticationException()) .isException("SDK issue") assertThat(ClientBuildException.ServerUnreachable("Server unreachable").mapAuthenticationException()) - .isException("Server unreachable") + .isException("Server unreachable") assertThat(ClientBuildException.SlidingSync("Sliding Sync").mapAuthenticationException()) .isException("Sliding Sync") assertThat(ClientBuildException.WellKnownDeserializationException("WellKnown Deserialization").mapAuthenticationException()) From 70e3e768aa8c0d80d8c479cc1ec2741cfaeb11cc Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 7 Nov 2025 10:18:45 +0100 Subject: [PATCH 14/17] Avoid usage of `not()` and add unit tests. --- .../changeserver/ChangeServerPresenter.kt | 2 +- .../changeserver/ChangeServerPresenterTest.kt | 2 +- .../ConfirmAccountProviderPresenterTest.kt | 2 +- .../LoginPasswordPresenterTest.kt | 2 +- .../api/auth/MatrixHomeServerDetails.kt | 4 +- .../api/auth/MatrixHomeServerDetailsTest.kt | 50 +++++++++++++++++++ .../test/auth}/MatrixHomeServerDetails.kt | 2 +- 7 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetailsTest.kt rename {features/login/impl/src/test/kotlin/io/element/android/features/login/impl => libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/auth}/MatrixHomeServerDetails.kt (92%) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt index fc114aaa6b..463662b6f1 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt @@ -61,7 +61,7 @@ class ChangeServerPresenter( accountProviderUrl = data.url, ) val details = authenticationService.setHomeserver(data.url).getOrThrow() - if (details.supportsOidcLogin.not() && details.supportsPasswordLogin.not()) { + if (details.isSupported) { throw ChangeServerError.UnsupportedServer } // Homeserver is valid, remember user choice diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt index d1090cb237..8fd12fb589 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenterTest.kt @@ -10,7 +10,6 @@ package io.element.android.features.login.impl.changeserver import com.google.common.truth.Truth.assertThat import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.enterprise.test.FakeEnterpriseService -import io.element.android.features.login.impl.aMatrixHomeServerDetails import io.element.android.features.login.impl.accesscontrol.DefaultAccountProviderAccessControl import io.element.android.features.login.impl.accountprovider.AccountProvider import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource @@ -22,6 +21,7 @@ import io.element.android.libraries.core.uri.ensureProtocol import io.element.android.libraries.matrix.test.AN_EXCEPTION import io.element.android.libraries.matrix.test.A_HOMESERVER_URL import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService +import io.element.android.libraries.matrix.test.auth.aMatrixHomeServerDetails import io.element.android.libraries.wellknown.api.ElementWellKnown import io.element.android.libraries.wellknown.api.WellknownRetriever import io.element.android.libraries.wellknown.api.WellknownRetrieverResult diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt index 182b7d6e27..cd845eb401 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/confirmaccountprovider/ConfirmAccountProviderPresenterTest.kt @@ -10,7 +10,6 @@ package io.element.android.features.login.impl.screens.confirmaccountprovider import com.google.common.truth.Truth.assertThat import io.element.android.appconfig.AuthenticationConfig import io.element.android.features.enterprise.test.FakeEnterpriseService -import io.element.android.features.login.impl.aMatrixHomeServerDetails import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource import io.element.android.features.login.impl.login.LoginMode import io.element.android.features.login.impl.screens.createaccount.AccountCreationNotSupported @@ -21,6 +20,7 @@ import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService import io.element.android.libraries.matrix.test.AN_EXCEPTION import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService +import io.element.android.libraries.matrix.test.auth.aMatrixHomeServerDetails import io.element.android.libraries.oidc.api.OidcAction import io.element.android.libraries.oidc.api.OidcActionFlow import io.element.android.libraries.oidc.test.customtab.FakeOidcActionFlow diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt index bfdc0a6785..3a7efd3bb2 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt +++ b/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/screens/loginpassword/LoginPasswordPresenterTest.kt @@ -10,7 +10,6 @@ package io.element.android.features.login.impl.screens.loginpassword import com.google.common.truth.Truth.assertThat import io.element.android.appconfig.AuthenticationConfig import io.element.android.features.enterprise.test.FakeEnterpriseService -import io.element.android.features.login.impl.aMatrixHomeServerDetails import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource import io.element.android.libraries.architecture.AsyncData import io.element.android.libraries.matrix.api.core.SessionId @@ -19,6 +18,7 @@ import io.element.android.libraries.matrix.test.A_PASSWORD import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.A_USER_NAME import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService +import io.element.android.libraries.matrix.test.auth.aMatrixHomeServerDetails import io.element.android.tests.testutils.WarmUpRule import io.element.android.tests.testutils.test import kotlinx.coroutines.test.runTest diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetails.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetails.kt index ce3af70121..2244258eb8 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetails.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetails.kt @@ -11,4 +11,6 @@ data class MatrixHomeServerDetails( val url: String, val supportsPasswordLogin: Boolean, val supportsOidcLogin: Boolean, -) +) { + val isSupported = supportsPasswordLogin || supportsOidcLogin +} diff --git a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetailsTest.kt b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetailsTest.kt new file mode 100644 index 0000000000..39f8759ca3 --- /dev/null +++ b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/auth/MatrixHomeServerDetailsTest.kt @@ -0,0 +1,50 @@ +/* + * Copyright 2025 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.android.libraries.matrix.api.auth + +import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.matrix.test.auth.aMatrixHomeServerDetails +import org.junit.Test + +class MatrixHomeServerDetailsTest { + @Test + fun `if homeserver supports oidc, then it is supported`() { + val sut = aMatrixHomeServerDetails( + supportsOidcLogin = true, + supportsPasswordLogin = false, + ) + assertThat(sut.isSupported).isTrue() + } + + @Test + fun `if homeserver supports password, then it is supported`() { + val sut = aMatrixHomeServerDetails( + supportsOidcLogin = false, + supportsPasswordLogin = true, + ) + assertThat(sut.isSupported).isTrue() + } + + @Test + fun `if homeserver supports both, then it is supported`() { + val sut = aMatrixHomeServerDetails( + supportsOidcLogin = true, + supportsPasswordLogin = true, + ) + assertThat(sut.isSupported).isTrue() + } + + @Test + fun `if homeserver supports none, then it is not supported`() { + val sut = aMatrixHomeServerDetails( + supportsOidcLogin = false, + supportsPasswordLogin = false, + ) + assertThat(sut.isSupported).isFalse() + } +} diff --git a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/MatrixHomeServerDetails.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/auth/MatrixHomeServerDetails.kt similarity index 92% rename from features/login/impl/src/test/kotlin/io/element/android/features/login/impl/MatrixHomeServerDetails.kt rename to libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/auth/MatrixHomeServerDetails.kt index 1bf32d20e2..379034a002 100644 --- a/features/login/impl/src/test/kotlin/io/element/android/features/login/impl/MatrixHomeServerDetails.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/auth/MatrixHomeServerDetails.kt @@ -5,7 +5,7 @@ * Please see LICENSE files in the repository root for full details. */ -package io.element.android.features.login.impl +package io.element.android.libraries.matrix.test.auth import io.element.android.libraries.matrix.api.auth.MatrixHomeServerDetails import io.element.android.libraries.matrix.test.A_HOMESERVER_URL From cc01e19bf7a25c28f9f46c25f075c1d315434415 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 7 Nov 2025 11:05:13 +0100 Subject: [PATCH 15/17] Use isNull() --- .../matrix/impl/auth/AuthenticationExceptionMappingTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationExceptionMappingTest.kt b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationExceptionMappingTest.kt index db37fc40b5..e84b42ea2f 100644 --- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationExceptionMappingTest.kt +++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/auth/AuthenticationExceptionMappingTest.kt @@ -19,7 +19,7 @@ class AuthenticationExceptionMappingTest { fun `mapping an exception with no message returns null message`() { val exception = Exception() val mappedException = exception.mapAuthenticationException() - assertThat(mappedException.message).isEqualTo(null) + assertThat(mappedException.message).isNull() } @Test From 3fd961c3292475fcde812f4e3142567a6d919e87 Mon Sep 17 00:00:00 2001 From: ElementBot Date: Fri, 7 Nov 2025 11:04:44 +0000 Subject: [PATCH 16/17] Update screenshots --- .../features.login.impl.login_LoginModeView_Day_5_en.png | 4 ++-- .../features.login.impl.login_LoginModeView_Day_6_en.png | 3 +++ .../features.login.impl.login_LoginModeView_Night_5_en.png | 4 ++-- .../features.login.impl.login_LoginModeView_Night_6_en.png | 3 +++ 4 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Day_6_en.png create mode 100644 tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Night_6_en.png diff --git a/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Day_5_en.png b/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Day_5_en.png index 2e7635cfdb..1125024a5a 100644 --- a/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Day_5_en.png +++ b/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Day_5_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:4eaf3d650155e9a779cf796cef116eaba0f1d6722b229332b224475393a88178 -size 16828 +oid sha256:ae6d230961018f1128ea655529a4287dbe3bbbc2762079d0c4f335313d2f2ab0 +size 25166 diff --git a/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Day_6_en.png b/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Day_6_en.png new file mode 100644 index 0000000000..2e7635cfdb --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Day_6_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4eaf3d650155e9a779cf796cef116eaba0f1d6722b229332b224475393a88178 +size 16828 diff --git a/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Night_5_en.png b/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Night_5_en.png index 10ca16827e..777f0bb627 100644 --- a/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Night_5_en.png +++ b/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Night_5_en.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:59f062f54833df71be9d7c4e785bb01013a10642e0d863bf7ef2abd8862b93c8 -size 15476 +oid sha256:1502711e6305f9adf4719b28c1fe3755089ae6c42c6c66e8e5bf73ab2bbb65c1 +size 23874 diff --git a/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Night_6_en.png b/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Night_6_en.png new file mode 100644 index 0000000000..10ca16827e --- /dev/null +++ b/tests/uitests/src/test/snapshots/images/features.login.impl.login_LoginModeView_Night_6_en.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:59f062f54833df71be9d7c4e785bb01013a10642e0d863bf7ef2abd8862b93c8 +size 15476 From fe2a317aa7d400026af989bcf9605b316b389216 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 7 Nov 2025 14:11:37 +0100 Subject: [PATCH 17/17] Oups, thanks unit tests! --- .../features/login/impl/changeserver/ChangeServerPresenter.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt index 463662b6f1..0223c1c235 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerPresenter.kt @@ -61,7 +61,7 @@ class ChangeServerPresenter( accountProviderUrl = data.url, ) val details = authenticationService.setHomeserver(data.url).getOrThrow() - if (details.isSupported) { + if (!details.isSupported) { throw ChangeServerError.UnsupportedServer } // Homeserver is valid, remember user choice