Use the SDK Client to check whether a homeserver is compatible (#5664)
* Use the SDK `Client` to check whether a HS is compatible * Remove usage of unused `WellKnown`, keep `ElementWellKnown` * Make `HomeServerLoginCompatibilityChecker.check` return `true/false` values to distinguish non-valid homeservers from a failed check * Use `inMemoryStore` and `serverNameOrHomeserverUrl` * Do some cleanup of `isValid` and `isWellknownValid` * Make the debounce for starting the search a bit higher, as checking for the homeservers seems more resource-intensive now
This commit is contained in:
parent
8a4d0c7bee
commit
7aa564e74d
27 changed files with 195 additions and 341 deletions
|
|
@ -13,5 +13,4 @@ data class AccountProvider(
|
|||
val subtitle: String? = null,
|
||||
val isPublic: Boolean = false,
|
||||
val isMatrixOrg: Boolean = false,
|
||||
val isValid: Boolean = false,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ open class AccountProviderProvider : PreviewParameterProvider<AccountProvider> {
|
|||
get() = sequenceOf(
|
||||
anAccountProvider(),
|
||||
anAccountProvider().copy(subtitle = null),
|
||||
anAccountProvider().copy(subtitle = null, title = "invalid", isValid = false),
|
||||
anAccountProvider().copy(subtitle = null, title = "invalid"),
|
||||
anAccountProvider().copy(subtitle = null, title = "Other", isPublic = false, isMatrixOrg = false),
|
||||
// Add other state here
|
||||
)
|
||||
|
|
@ -26,11 +26,9 @@ fun anAccountProvider(
|
|||
subtitle: String? = "Matrix.org is an open network for secure, decentralized communication.",
|
||||
isPublic: Boolean = true,
|
||||
isMatrixOrg: Boolean = true,
|
||||
isValid: Boolean = true,
|
||||
) = AccountProvider(
|
||||
url = url,
|
||||
subtitle = subtitle,
|
||||
isPublic = isPublic,
|
||||
isMatrixOrg = isMatrixOrg,
|
||||
isValid = isValid,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -10,6 +10,4 @@ package io.element.android.features.login.impl.resolver
|
|||
data class HomeserverData(
|
||||
// The computed homeserver url, for which a wellknown file has been retrieved, or just a valid Url
|
||||
val homeserverUrl: String,
|
||||
// True if a wellknown file has been found and is valid. If false, it means that the [homeserverUrl] is valid
|
||||
val isWellknownValid: Boolean,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -8,19 +8,16 @@
|
|||
package io.element.android.features.login.impl.resolver
|
||||
|
||||
import dev.zacsweers.metro.Inject
|
||||
import io.element.android.libraries.core.bool.orFalse
|
||||
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
|
||||
import io.element.android.libraries.core.coroutine.parallelMap
|
||||
import io.element.android.libraries.core.data.tryOrNull
|
||||
import io.element.android.libraries.core.uri.ensureProtocol
|
||||
import io.element.android.libraries.core.uri.isValidUrl
|
||||
import io.element.android.libraries.wellknown.api.WellKnown
|
||||
import io.element.android.libraries.wellknown.api.WellknownRetriever
|
||||
import io.element.android.libraries.matrix.api.auth.HomeServerLoginCompatibilityChecker
|
||||
import kotlinx.coroutines.currentCoroutineContext
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.flow
|
||||
import kotlinx.coroutines.withContext
|
||||
import kotlinx.coroutines.withTimeout
|
||||
import timber.log.Timber
|
||||
import java.util.Collections
|
||||
|
||||
/**
|
||||
|
|
@ -29,7 +26,7 @@ import java.util.Collections
|
|||
@Inject
|
||||
class HomeserverResolver(
|
||||
private val dispatchers: CoroutineDispatchers,
|
||||
private val wellknownRetriever: WellknownRetriever,
|
||||
private val homeServerLoginCompatibilityChecker: HomeServerLoginCompatibilityChecker,
|
||||
) {
|
||||
fun resolve(userInput: String): Flow<List<HomeserverData>> = flow {
|
||||
val flowContext = currentCoroutineContext()
|
||||
|
|
@ -41,20 +38,14 @@ class HomeserverResolver(
|
|||
// Run all the requests in parallel
|
||||
withContext(dispatchers.io) {
|
||||
list.parallelMap { url ->
|
||||
val wellKnown = tryOrNull {
|
||||
withTimeout(5000) {
|
||||
wellknownRetriever.getWellKnown(url)
|
||||
}
|
||||
}
|
||||
val isValid = wellKnown?.dataOrNull()?.isValid().orFalse()
|
||||
val isValid = homeServerLoginCompatibilityChecker.check(url)
|
||||
.onFailure { Timber.w(it, "Failed to check compatibility with homeserver $url") }
|
||||
.getOrNull()
|
||||
?: return@parallelMap
|
||||
|
||||
// Emit the list as soon as possible
|
||||
if (isValid) {
|
||||
// Emit the list as soon as possible
|
||||
currentList.add(
|
||||
HomeserverData(
|
||||
homeserverUrl = url,
|
||||
isWellknownValid = true,
|
||||
)
|
||||
)
|
||||
currentList.add(HomeserverData(homeserverUrl = url))
|
||||
withContext(flowContext) {
|
||||
emit(currentList.toList())
|
||||
}
|
||||
|
|
@ -63,14 +54,7 @@ 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,
|
||||
isWellknownValid = false,
|
||||
)
|
||||
)
|
||||
)
|
||||
emit(listOf(HomeserverData(homeserverUrl = trimmedUserInput)))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -88,7 +72,3 @@ class HomeserverResolver(
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
private fun WellKnown.isValid(): Boolean {
|
||||
return homeServer?.baseURL?.isNotBlank().orFalse()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -37,7 +37,6 @@ class ChangeAccountProviderPresenter(
|
|||
subtitle = null,
|
||||
isPublic = url == AuthenticationConfig.MATRIX_ORG_URL,
|
||||
isMatrixOrg = url == AuthenticationConfig.MATRIX_ORG_URL,
|
||||
isValid = true,
|
||||
)
|
||||
}
|
||||
.toImmutableList()
|
||||
|
|
|
|||
|
|
@ -67,7 +67,6 @@ class ChooseAccountProviderPresenter(
|
|||
subtitle = null,
|
||||
isPublic = url == AuthenticationConfig.MATRIX_ORG_URL,
|
||||
isMatrixOrg = url == AuthenticationConfig.MATRIX_ORG_URL,
|
||||
isValid = true,
|
||||
)
|
||||
}
|
||||
.toImmutableList()
|
||||
|
|
|
|||
|
|
@ -57,14 +57,14 @@ class SearchAccountProviderPresenter(
|
|||
userInput = userInput,
|
||||
userInputResult = data.value,
|
||||
changeServerState = changeServerState,
|
||||
eventSink = ::handleEvents
|
||||
eventSink = ::handleEvents,
|
||||
)
|
||||
}
|
||||
|
||||
private fun CoroutineScope.onUserInput(userInput: String, data: MutableState<AsyncData<List<HomeserverData>>>) = launch {
|
||||
data.value = AsyncData.Uninitialized
|
||||
// Debounce
|
||||
delay(300)
|
||||
delay(500)
|
||||
data.value = AsyncData.Loading()
|
||||
homeserverResolver.resolve(userInput).collect {
|
||||
data.value = AsyncData.Success(it)
|
||||
|
|
|
|||
|
|
@ -34,18 +34,14 @@ fun aSearchAccountProviderState(
|
|||
|
||||
fun aHomeserverDataList(): List<HomeserverData> {
|
||||
return listOf(
|
||||
aHomeserverData(isWellknownValid = true),
|
||||
aHomeserverData(homeserverUrl = "https://no.sliding.sync", isWellknownValid = true),
|
||||
aHomeserverData(homeserverUrl = "https://invalid", isWellknownValid = false),
|
||||
aHomeserverData(homeserverUrl = AuthenticationConfig.MATRIX_ORG_URL),
|
||||
aHomeserverData(homeserverUrl = "https://no.sliding.sync"),
|
||||
aHomeserverData(homeserverUrl = "https://invalid"),
|
||||
)
|
||||
}
|
||||
|
||||
fun aHomeserverData(
|
||||
homeserverUrl: String = AuthenticationConfig.MATRIX_ORG_URL,
|
||||
isWellknownValid: Boolean = true,
|
||||
): HomeserverData {
|
||||
return HomeserverData(
|
||||
homeserverUrl = homeserverUrl,
|
||||
isWellknownValid = isWellknownValid,
|
||||
)
|
||||
return HomeserverData(homeserverUrl = homeserverUrl,)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -192,7 +192,6 @@ private fun HomeserverData.toAccountProvider(): AccountProvider {
|
|||
// There is no need to know for other servers right now
|
||||
isPublic = isMatrixOrg,
|
||||
isMatrixOrg = isMatrixOrg,
|
||||
isValid = isWellknownValid,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -33,7 +33,6 @@ class AccountProviderDataSourceTest {
|
|||
subtitle = null,
|
||||
isPublic = true,
|
||||
isMatrixOrg = true,
|
||||
isValid = false,
|
||||
)
|
||||
)
|
||||
}
|
||||
|
|
@ -55,7 +54,6 @@ class AccountProviderDataSourceTest {
|
|||
subtitle = null,
|
||||
isPublic = true,
|
||||
isMatrixOrg = true,
|
||||
isValid = false,
|
||||
)
|
||||
)
|
||||
}
|
||||
|
|
@ -77,7 +75,6 @@ class AccountProviderDataSourceTest {
|
|||
subtitle = null,
|
||||
isPublic = true,
|
||||
isMatrixOrg = true,
|
||||
isValid = false,
|
||||
)
|
||||
)
|
||||
}
|
||||
|
|
@ -98,7 +95,6 @@ class AccountProviderDataSourceTest {
|
|||
subtitle = null,
|
||||
isPublic = false,
|
||||
isMatrixOrg = false,
|
||||
isValid = false,
|
||||
)
|
||||
)
|
||||
sut.reset()
|
||||
|
|
|
|||
|
|
@ -46,7 +46,6 @@ class ChangeAccountProviderPresenterTest {
|
|||
subtitle = null,
|
||||
isPublic = true,
|
||||
isMatrixOrg = true,
|
||||
isValid = true,
|
||||
)
|
||||
)
|
||||
)
|
||||
|
|
@ -76,7 +75,6 @@ class ChangeAccountProviderPresenterTest {
|
|||
subtitle = null,
|
||||
isPublic = true,
|
||||
isMatrixOrg = true,
|
||||
isValid = true,
|
||||
),
|
||||
AccountProvider(
|
||||
url = "https://element.io",
|
||||
|
|
@ -84,7 +82,6 @@ class ChangeAccountProviderPresenterTest {
|
|||
subtitle = null,
|
||||
isPublic = false,
|
||||
isMatrixOrg = false,
|
||||
isValid = true,
|
||||
)
|
||||
)
|
||||
)
|
||||
|
|
@ -114,7 +111,6 @@ class ChangeAccountProviderPresenterTest {
|
|||
subtitle = null,
|
||||
isPublic = true,
|
||||
isMatrixOrg = true,
|
||||
isValid = true,
|
||||
)
|
||||
)
|
||||
)
|
||||
|
|
|
|||
|
|
@ -37,14 +37,12 @@ class ChooseAccountProviderPresenterTest {
|
|||
subtitle = null,
|
||||
isPublic = false,
|
||||
isMatrixOrg = false,
|
||||
isValid = true,
|
||||
)
|
||||
val accountProvider2 = AccountProvider(
|
||||
url = ACCOUNT_PROVIDER_FROM_CONFIG_2.ensureProtocol(),
|
||||
subtitle = null,
|
||||
isPublic = false,
|
||||
isMatrixOrg = false,
|
||||
isValid = true,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -13,12 +13,8 @@ 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
|
||||
import io.element.android.features.wellknown.test.FakeWellknownRetriever
|
||||
import io.element.android.libraries.architecture.AsyncData
|
||||
import io.element.android.libraries.matrix.test.A_HOMESERVER_URL
|
||||
import io.element.android.libraries.wellknown.api.WellKnown
|
||||
import io.element.android.libraries.wellknown.api.WellKnownBaseConfig
|
||||
import io.element.android.libraries.wellknown.api.WellknownRetrieverResult
|
||||
import io.element.android.libraries.matrix.test.auth.FakeHomeServerLoginCompatibilityChecker
|
||||
import io.element.android.tests.testutils.WarmUpRule
|
||||
import io.element.android.tests.testutils.lambda.lambdaRecorder
|
||||
import io.element.android.tests.testutils.lambda.value
|
||||
|
|
@ -33,9 +29,9 @@ class SearchAccountProviderPresenterTest {
|
|||
|
||||
@Test
|
||||
fun `present - initial state`() = runTest {
|
||||
val fakeWellknownRetriever = FakeWellknownRetriever()
|
||||
val fakeLoginCompatibilityChecker = FakeHomeServerLoginCompatibilityChecker(checkResult = { Result.success(true) })
|
||||
val presenter = SearchAccountProviderPresenter(
|
||||
homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeWellknownRetriever),
|
||||
homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeLoginCompatibilityChecker),
|
||||
changeServerPresenter = { aChangeServerState() }
|
||||
)
|
||||
moleculeFlow(RecompositionMode.Immediate) {
|
||||
|
|
@ -47,9 +43,35 @@ class SearchAccountProviderPresenterTest {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `present - error while checking login compatibility`() = runTest {
|
||||
val fakeLoginCompatibilityChecker = FakeHomeServerLoginCompatibilityChecker(checkResult = { Result.failure(IllegalStateException("Oops")) })
|
||||
val presenter = SearchAccountProviderPresenter(
|
||||
homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeLoginCompatibilityChecker),
|
||||
changeServerPresenter = { aChangeServerState() }
|
||||
)
|
||||
moleculeFlow(RecompositionMode.Immediate) {
|
||||
presenter.present()
|
||||
}.test {
|
||||
val initialState = awaitItem()
|
||||
initialState.eventSink.invoke(SearchAccountProviderEvents.UserInput("https://test.org"))
|
||||
val withInputState = awaitItem()
|
||||
assertThat(withInputState.userInput).isEqualTo("https://test.org")
|
||||
assertThat(initialState.userInputResult).isEqualTo(AsyncData.Uninitialized)
|
||||
assertThat(awaitItem().userInputResult).isInstanceOf(AsyncData.Loading::class.java)
|
||||
assertThat(awaitItem().userInputResult).isEqualTo(
|
||||
AsyncData.Success(
|
||||
listOf(
|
||||
aHomeserverData(homeserverUrl = "https://test.org")
|
||||
)
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `present - enter text no result`() = runTest {
|
||||
val fakeWellknownRetriever = FakeWellknownRetriever()
|
||||
val fakeWellknownRetriever = FakeHomeServerLoginCompatibilityChecker(checkResult = { Result.success(false) })
|
||||
val presenter = SearchAccountProviderPresenter(
|
||||
homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeWellknownRetriever),
|
||||
changeServerPresenter = { aChangeServerState() }
|
||||
|
|
@ -67,48 +89,20 @@ class SearchAccountProviderPresenterTest {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `present - enter valid url no wellknown`() = runTest {
|
||||
val fakeWellknownRetriever = FakeWellknownRetriever()
|
||||
val presenter = SearchAccountProviderPresenter(
|
||||
homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeWellknownRetriever),
|
||||
changeServerPresenter = { aChangeServerState() }
|
||||
)
|
||||
moleculeFlow(RecompositionMode.Immediate) {
|
||||
presenter.present()
|
||||
}.test {
|
||||
val initialState = awaitItem()
|
||||
initialState.eventSink.invoke(SearchAccountProviderEvents.UserInput("https://test.org"))
|
||||
val withInputState = awaitItem()
|
||||
assertThat(withInputState.userInput).isEqualTo("https://test.org")
|
||||
assertThat(initialState.userInputResult).isEqualTo(AsyncData.Uninitialized)
|
||||
assertThat(awaitItem().userInputResult).isInstanceOf(AsyncData.Loading::class.java)
|
||||
assertThat(awaitItem().userInputResult).isEqualTo(
|
||||
AsyncData.Success(
|
||||
listOf(
|
||||
aHomeserverData(homeserverUrl = "https://test.org", isWellknownValid = false)
|
||||
)
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `present - enter text one result with wellknown`() = runTest {
|
||||
val getWellKnownResult = lambdaRecorder<String, WellknownRetrieverResult<WellKnown>> {
|
||||
val checkResult = lambdaRecorder<String, Result<Boolean>> {
|
||||
when (it) {
|
||||
"https://test.org" -> WellknownRetrieverResult.NotFound
|
||||
"https://test.com" -> WellknownRetrieverResult.NotFound
|
||||
"https://test.io" -> WellknownRetrieverResult.Success(aWellKnown())
|
||||
"https://test" -> WellknownRetrieverResult.NotFound
|
||||
"https://test.org" -> Result.success(false)
|
||||
"https://test.com" -> Result.success(false)
|
||||
"https://test.io" -> Result.success(true)
|
||||
"https://test" -> Result.success(false)
|
||||
else -> error("should not happen")
|
||||
}
|
||||
}
|
||||
val fakeWellknownRetriever = FakeWellknownRetriever(
|
||||
getWellKnownResult = getWellKnownResult,
|
||||
)
|
||||
val fakeLoginCompatibilityChecker = FakeHomeServerLoginCompatibilityChecker(checkResult = checkResult)
|
||||
val presenter = SearchAccountProviderPresenter(
|
||||
homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeWellknownRetriever),
|
||||
homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeLoginCompatibilityChecker),
|
||||
changeServerPresenter = { aChangeServerState() }
|
||||
)
|
||||
moleculeFlow(RecompositionMode.Immediate) {
|
||||
|
|
@ -127,7 +121,7 @@ class SearchAccountProviderPresenterTest {
|
|||
)
|
||||
)
|
||||
)
|
||||
getWellKnownResult.assertions().isCalledExactly(4)
|
||||
checkResult.assertions().isCalledExactly(4)
|
||||
.withSequence(
|
||||
listOf(value("https://test.org")),
|
||||
listOf(value("https://test.com")),
|
||||
|
|
@ -139,20 +133,18 @@ class SearchAccountProviderPresenterTest {
|
|||
|
||||
@Test
|
||||
fun `present - enter text two results with wellknown`() = runTest {
|
||||
val getWellKnownResult = lambdaRecorder<String, WellknownRetrieverResult<WellKnown>> {
|
||||
val checkResult = lambdaRecorder<String, Result<Boolean>> {
|
||||
when (it) {
|
||||
"https://test.org" -> WellknownRetrieverResult.Success(aWellKnown())
|
||||
"https://test.com" -> WellknownRetrieverResult.NotFound
|
||||
"https://test.io" -> WellknownRetrieverResult.Success(aWellKnown())
|
||||
"https://test" -> WellknownRetrieverResult.NotFound
|
||||
"https://test.org" -> Result.success(true)
|
||||
"https://test.com" -> Result.success(false)
|
||||
"https://test.io" -> Result.success(true)
|
||||
"https://test" -> Result.success(false)
|
||||
else -> error("should not happen")
|
||||
}
|
||||
}
|
||||
val fakeWellknownRetriever = FakeWellknownRetriever(
|
||||
getWellKnownResult = getWellKnownResult,
|
||||
)
|
||||
val fakeLoginCompatibilityChecker = FakeHomeServerLoginCompatibilityChecker(checkResult = checkResult)
|
||||
val presenter = SearchAccountProviderPresenter(
|
||||
homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeWellknownRetriever),
|
||||
homeserverResolver = HomeserverResolver(testCoroutineDispatchers(), fakeLoginCompatibilityChecker),
|
||||
changeServerPresenter = { aChangeServerState() }
|
||||
)
|
||||
moleculeFlow(RecompositionMode.Immediate) {
|
||||
|
|
@ -179,7 +171,7 @@ class SearchAccountProviderPresenterTest {
|
|||
)
|
||||
)
|
||||
)
|
||||
getWellKnownResult.assertions().isCalledExactly(4)
|
||||
checkResult.assertions().isCalledExactly(4)
|
||||
.withSequence(
|
||||
listOf(value("https://test.org")),
|
||||
listOf(value("https://test.com")),
|
||||
|
|
@ -188,15 +180,4 @@ class SearchAccountProviderPresenterTest {
|
|||
)
|
||||
}
|
||||
}
|
||||
|
||||
private fun aWellKnown(): WellKnown {
|
||||
return WellKnown(
|
||||
homeServer = WellKnownBaseConfig(
|
||||
baseURL = A_HOMESERVER_URL
|
||||
),
|
||||
identityServer = WellKnownBaseConfig(
|
||||
baseURL = A_HOMESERVER_URL
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue