Fix transition when base URL is actually incorrect (#196)

This commit is contained in:
Kirill Kamakin
2024-01-13 12:18:12 +01:00
committed by GitHub
parent 4c6e480737
commit c8f1f477cc
12 changed files with 54 additions and 35 deletions

View File

@@ -1,5 +1,6 @@
package gq.kirmanak.mealient.data.baseurl package gq.kirmanak.mealient.data.baseurl
import gq.kirmanak.mealient.datasource.models.VersionResponse
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
interface ServerInfoRepo { interface ServerInfoRepo {
@@ -8,7 +9,7 @@ interface ServerInfoRepo {
suspend fun getUrl(): String? suspend fun getUrl(): String?
suspend fun tryBaseURL(baseURL: String): Result<Unit> suspend fun tryBaseURL(baseURL: String): Result<VersionResponse>
} }

View File

@@ -1,6 +1,7 @@
package gq.kirmanak.mealient.data.baseurl package gq.kirmanak.mealient.data.baseurl
import gq.kirmanak.mealient.datasource.ServerUrlProvider import gq.kirmanak.mealient.datasource.ServerUrlProvider
import gq.kirmanak.mealient.datasource.models.VersionResponse
import gq.kirmanak.mealient.logging.Logger import gq.kirmanak.mealient.logging.Logger
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import javax.inject.Inject import javax.inject.Inject
@@ -20,18 +21,11 @@ class ServerInfoRepoImpl @Inject constructor(
return result return result
} }
override suspend fun tryBaseURL(baseURL: String): Result<Unit> { override suspend fun tryBaseURL(baseURL: String): Result<VersionResponse> {
val oldBaseUrl = serverInfoStorage.getBaseURL() return versionDataSource.runCatching {
serverInfoStorage.storeBaseURL(baseURL) requestVersion(baseURL)
}.onSuccess {
try { serverInfoStorage.storeBaseURL(baseURL)
versionDataSource.requestVersion()
} catch (e: Throwable) {
serverInfoStorage.storeBaseURL(oldBaseUrl)
return Result.failure(e)
} }
return Result.success(Unit)
} }
} }

View File

@@ -4,5 +4,5 @@ import gq.kirmanak.mealient.datasource.models.VersionResponse
interface VersionDataSource { interface VersionDataSource {
suspend fun requestVersion(): VersionResponse suspend fun requestVersion(baseURL: String): VersionResponse
} }

View File

@@ -8,7 +8,7 @@ class VersionDataSourceImpl @Inject constructor(
private val dataSource: MealieDataSource, private val dataSource: MealieDataSource,
) : VersionDataSource { ) : VersionDataSource {
override suspend fun requestVersion(): VersionResponse { override suspend fun requestVersion(baseURL: String): VersionResponse {
return dataSource.getVersionInfo() return dataSource.getVersionInfo(baseURL)
} }
} }

View File

@@ -77,7 +77,7 @@ internal class BaseURLViewModel @Inject constructor(
return return
} }
val result: Result<Unit> = serverInfoRepo.tryBaseURL(url).recoverCatching { val result = serverInfoRepo.tryBaseURL(url).recoverCatching {
logger.e(it) { "checkBaseURL: trying to recover, had prefix = $hasPrefix" } logger.e(it) { "checkBaseURL: trying to recover, had prefix = $hasPrefix" }
val certificateError = it.findCauseAsInstanceOf<CertificateCombinedException>() val certificateError = it.findCauseAsInstanceOf<CertificateCombinedException>()
if (certificateError != null) { if (certificateError != null) {

View File

@@ -2,6 +2,7 @@ package gq.kirmanak.mealient.data.baseurl
import com.google.common.truth.Truth.assertThat import com.google.common.truth.Truth.assertThat
import gq.kirmanak.mealient.test.AuthImplTestData.TEST_BASE_URL import gq.kirmanak.mealient.test.AuthImplTestData.TEST_BASE_URL
import gq.kirmanak.mealient.test.AuthImplTestData.VERSION_RESPONSE
import gq.kirmanak.mealient.test.BaseUnitTest import gq.kirmanak.mealient.test.BaseUnitTest
import io.mockk.coEvery import io.mockk.coEvery
import io.mockk.coVerify import io.mockk.coVerify
@@ -48,10 +49,16 @@ class ServerInfoRepoTest : BaseUnitTest() {
@Test @Test
fun `when tryBaseURL succeeds expect call to storage`() = runTest { fun `when tryBaseURL succeeds expect call to storage`() = runTest {
coEvery { storage.getBaseURL() } returns null coEvery { dataSource.requestVersion(TEST_BASE_URL) } returns VERSION_RESPONSE
subject.tryBaseURL(TEST_BASE_URL) subject.tryBaseURL(TEST_BASE_URL)
coVerify { coVerify {
storage.storeBaseURL(eq(TEST_BASE_URL)) storage.storeBaseURL(eq(TEST_BASE_URL))
} }
} }
@Test
fun `when tryBaseURL succeeds expect response`() = runTest {
coEvery { dataSource.requestVersion(TEST_BASE_URL) } returns VERSION_RESPONSE
assertThat(subject.tryBaseURL(TEST_BASE_URL)).isEqualTo(Result.success(VERSION_RESPONSE))
}
} }

View File

@@ -1,6 +1,7 @@
package gq.kirmanak.mealient.test package gq.kirmanak.mealient.test
import gq.kirmanak.mealient.datasource.models.GetUserInfoResponse import gq.kirmanak.mealient.datasource.models.GetUserInfoResponse
import gq.kirmanak.mealient.datasource.models.VersionResponse
object AuthImplTestData { object AuthImplTestData {
const val TEST_USERNAME = "TEST_USERNAME" const val TEST_USERNAME = "TEST_USERNAME"
@@ -11,4 +12,5 @@ object AuthImplTestData {
val FAVORITE_RECIPES_LIST = listOf("cake", "porridge") val FAVORITE_RECIPES_LIST = listOf("cake", "porridge")
val USER_INFO = GetUserInfoResponse("userId", FAVORITE_RECIPES_LIST) val USER_INFO = GetUserInfoResponse("userId", FAVORITE_RECIPES_LIST)
val VERSION_RESPONSE = VersionResponse("1.0.0")
} }

View File

@@ -9,6 +9,7 @@ import gq.kirmanak.mealient.data.recipes.RecipeRepo
import gq.kirmanak.mealient.datasource.NetworkError import gq.kirmanak.mealient.datasource.NetworkError
import gq.kirmanak.mealient.datasource.TrustedCertificatesStore import gq.kirmanak.mealient.datasource.TrustedCertificatesStore
import gq.kirmanak.mealient.test.AuthImplTestData.TEST_BASE_URL import gq.kirmanak.mealient.test.AuthImplTestData.TEST_BASE_URL
import gq.kirmanak.mealient.test.AuthImplTestData.VERSION_RESPONSE
import gq.kirmanak.mealient.test.BaseUnitTest import gq.kirmanak.mealient.test.BaseUnitTest
import io.mockk.coEvery import io.mockk.coEvery
import io.mockk.coVerify import io.mockk.coVerify
@@ -91,7 +92,7 @@ internal class BaseURLViewModelTest : BaseUnitTest() {
private fun TestScope.setupSaveBaseUrlWithOldUrl() { private fun TestScope.setupSaveBaseUrlWithOldUrl() {
coEvery { serverInfoRepo.getUrl() } returns TEST_BASE_URL coEvery { serverInfoRepo.getUrl() } returns TEST_BASE_URL
coEvery { serverInfoRepo.tryBaseURL(any()) } returns Result.success(Unit) coEvery { serverInfoRepo.tryBaseURL(any()) } returns Result.success(VERSION_RESPONSE)
subject.saveBaseUrl(TEST_BASE_URL) subject.saveBaseUrl(TEST_BASE_URL)
advanceUntilIdle() advanceUntilIdle()
} }
@@ -116,7 +117,7 @@ internal class BaseURLViewModelTest : BaseUnitTest() {
private fun TestScope.setupSaveBaseUrlWithNewUrl() { private fun TestScope.setupSaveBaseUrlWithNewUrl() {
coEvery { serverInfoRepo.getUrl() } returns null coEvery { serverInfoRepo.getUrl() } returns null
coEvery { serverInfoRepo.tryBaseURL(any()) } returns Result.success(Unit) coEvery { serverInfoRepo.tryBaseURL(any()) } returns Result.success(VERSION_RESPONSE)
subject.saveBaseUrl(TEST_BASE_URL) subject.saveBaseUrl(TEST_BASE_URL)
advanceUntilIdle() advanceUntilIdle()
} }
@@ -135,7 +136,7 @@ internal class BaseURLViewModelTest : BaseUnitTest() {
coEvery { serverInfoRepo.getUrl() } returns null coEvery { serverInfoRepo.getUrl() } returns null
val err = NetworkError.MalformedUrl(SSLHandshakeException("test")) val err = NetworkError.MalformedUrl(SSLHandshakeException("test"))
coEvery { serverInfoRepo.tryBaseURL("https://test") } returns Result.failure(err) coEvery { serverInfoRepo.tryBaseURL("https://test") } returns Result.failure(err)
coEvery { serverInfoRepo.tryBaseURL("http://test") } returns Result.success(Unit) coEvery { serverInfoRepo.tryBaseURL("http://test") } returns Result.success(VERSION_RESPONSE)
subject.saveBaseUrl("test") subject.saveBaseUrl("test")
coVerifyOrder { coVerifyOrder {
serverInfoRepo.tryBaseURL("https://test") serverInfoRepo.tryBaseURL("https://test")

View File

@@ -35,7 +35,7 @@ interface MealieDataSource {
password: String, password: String,
): String ): String
suspend fun getVersionInfo(): VersionResponse suspend fun getVersionInfo(baseURL: String): VersionResponse
suspend fun requestRecipes( suspend fun requestRecipes(
page: Int, page: Int,

View File

@@ -28,7 +28,7 @@ internal interface MealieService {
slug: String, slug: String,
): GetRecipeResponse ): GetRecipeResponse
suspend fun getVersion(): VersionResponse suspend fun getVersion(baseURL: String): VersionResponse
suspend fun getRecipeSummary(page: Int, perPage: Int): GetRecipesResponse suspend fun getRecipeSummary(page: Int, perPage: Int): GetRecipesResponse

View File

@@ -65,16 +65,18 @@ internal class MealieDataSourceImpl @Inject constructor(
throw if (errorDetail.detail == "Unauthorized") NetworkError.Unauthorized(it) else it throw if (errorDetail.detail == "Unauthorized") NetworkError.Unauthorized(it) else it
} }
override suspend fun getVersionInfo(): VersionResponse = networkRequestWrapper.makeCall( override suspend fun getVersionInfo(baseURL: String): VersionResponse =
block = { service.getVersion() }, networkRequestWrapper.makeCall(
logMethod = { "getVersionInfo" }, block = { service.getVersion(baseURL) },
).getOrElse { logMethod = { "getVersionInfo" },
throw when (it) { logParameters = { "baseURL = $baseURL" }
is ResponseException, is NoTransformationFoundException -> NetworkError.NotMealie(it) ).getOrElse {
is SocketTimeoutException, is SocketException -> NetworkError.NoServerConnection(it) throw when (it) {
else -> NetworkError.MalformedUrl(it) is ResponseException, is NoTransformationFoundException -> NetworkError.NotMealie(it)
is SocketTimeoutException, is SocketException -> NetworkError.NoServerConnection(it)
else -> NetworkError.MalformedUrl(it)
}
} }
}
override suspend fun requestRecipes( override suspend fun requestRecipes(
page: Int, page: Int,

View File

@@ -76,9 +76,9 @@ internal class MealieServiceKtor @Inject constructor(
}.body() }.body()
} }
override suspend fun getVersion(): VersionResponse { override suspend fun getVersion(baseURL: String): VersionResponse {
return httpClient.get { return httpClient.get {
endpoint("/api/app/about") endpoint(baseURL, "/api/app/about")
}.body() }.body()
} }
@@ -198,9 +198,21 @@ internal class MealieServiceKtor @Inject constructor(
private suspend fun HttpRequestBuilder.endpoint( private suspend fun HttpRequestBuilder.endpoint(
path: String, path: String,
block: URLBuilder.() -> Unit = {} block: URLBuilder.() -> Unit = {},
) { ) {
val baseUrl = checkNotNull(serverUrlProvider.getUrl()) { "Server URL is not set" } val baseUrl = checkNotNull(serverUrlProvider.getUrl()) { "Server URL is not set" }
endpoint(
baseUrl = baseUrl,
path = path,
block = block
)
}
private fun HttpRequestBuilder.endpoint(
baseUrl: String,
path: String,
block: URLBuilder.() -> Unit = {},
) {
url { url {
takeFrom(baseUrl) takeFrom(baseUrl)
path(path) path(path)