From 915fd77daacdaf5c9c119f16811ff86508800619 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sat, 10 Dec 2022 07:40:53 +0100 Subject: [PATCH 01/15] Create impl package in datasource --- .../gq/kirmanak/mealient/datasource/DataSourceModule.kt | 4 ++++ .../mealient/datasource/{ => impl}/CacheBuilderImpl.kt | 3 ++- .../datasource/{ => impl}/NetworkRequestWrapperImpl.kt | 5 ++++- .../mealient/datasource/{ => impl}/OkHttpBuilderImpl.kt | 4 +++- .../mealient/datasource/{ => impl}/RetrofitBuilder.kt | 2 +- .../mealient/datasource/MealieDataSourceV0ImplTest.kt | 1 + 6 files changed, 15 insertions(+), 4 deletions(-) rename datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/{ => impl}/CacheBuilderImpl.kt (92%) rename datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/{ => impl}/NetworkRequestWrapperImpl.kt (83%) rename datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/{ => impl}/OkHttpBuilderImpl.kt (80%) rename datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/{ => impl}/RetrofitBuilder.kt (93%) diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt index afaf344..1582f0c 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt @@ -6,6 +6,10 @@ import dagger.Module import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent +import gq.kirmanak.mealient.datasource.impl.CacheBuilderImpl +import gq.kirmanak.mealient.datasource.impl.NetworkRequestWrapperImpl +import gq.kirmanak.mealient.datasource.impl.OkHttpBuilderImpl +import gq.kirmanak.mealient.datasource.impl.RetrofitBuilder import gq.kirmanak.mealient.datasource.v0.MealieDataSourceV0 import gq.kirmanak.mealient.datasource.v0.MealieDataSourceV0Impl import gq.kirmanak.mealient.datasource.v0.MealieServiceV0 diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/CacheBuilderImpl.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/CacheBuilderImpl.kt similarity index 92% rename from datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/CacheBuilderImpl.kt rename to datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/CacheBuilderImpl.kt index c2403ed..856723e 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/CacheBuilderImpl.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/CacheBuilderImpl.kt @@ -1,8 +1,9 @@ -package gq.kirmanak.mealient.datasource +package gq.kirmanak.mealient.datasource.impl import android.content.Context import android.os.StatFs import dagger.hilt.android.qualifiers.ApplicationContext +import gq.kirmanak.mealient.datasource.CacheBuilder import gq.kirmanak.mealient.logging.Logger import okhttp3.Cache import java.io.File diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/NetworkRequestWrapperImpl.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/NetworkRequestWrapperImpl.kt similarity index 83% rename from datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/NetworkRequestWrapperImpl.kt rename to datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/NetworkRequestWrapperImpl.kt index 5bf01dc..6c2b5b2 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/NetworkRequestWrapperImpl.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/NetworkRequestWrapperImpl.kt @@ -1,5 +1,8 @@ -package gq.kirmanak.mealient.datasource +package gq.kirmanak.mealient.datasource.impl +import gq.kirmanak.mealient.datasource.NetworkError +import gq.kirmanak.mealient.datasource.NetworkRequestWrapper +import gq.kirmanak.mealient.datasource.runCatchingExceptCancel import gq.kirmanak.mealient.logging.Logger import retrofit2.HttpException import javax.inject.Inject diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/OkHttpBuilderImpl.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt similarity index 80% rename from datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/OkHttpBuilderImpl.kt rename to datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt index f3f5cfd..21ca1d1 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/OkHttpBuilderImpl.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt @@ -1,5 +1,7 @@ -package gq.kirmanak.mealient.datasource +package gq.kirmanak.mealient.datasource.impl +import gq.kirmanak.mealient.datasource.CacheBuilder +import gq.kirmanak.mealient.datasource.OkHttpBuilder import okhttp3.Interceptor import okhttp3.OkHttpClient import javax.inject.Inject diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/RetrofitBuilder.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/RetrofitBuilder.kt similarity index 93% rename from datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/RetrofitBuilder.kt rename to datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/RetrofitBuilder.kt index fe6b26e..8a5713f 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/RetrofitBuilder.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/RetrofitBuilder.kt @@ -1,4 +1,4 @@ -package gq.kirmanak.mealient.datasource +package gq.kirmanak.mealient.datasource.impl import gq.kirmanak.mealient.logging.Logger import okhttp3.OkHttpClient diff --git a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieDataSourceV0ImplTest.kt b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieDataSourceV0ImplTest.kt index 29762ec..43a3527 100644 --- a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieDataSourceV0ImplTest.kt +++ b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieDataSourceV0ImplTest.kt @@ -1,6 +1,7 @@ package gq.kirmanak.mealient.datasource import com.google.common.truth.Truth.assertThat +import gq.kirmanak.mealient.datasource.impl.NetworkRequestWrapperImpl import gq.kirmanak.mealient.datasource.v0.MealieDataSourceV0Impl import gq.kirmanak.mealient.datasource.v0.MealieServiceV0 import gq.kirmanak.mealient.datasource.v0.models.GetTokenResponseV0 From 54d0c895a9598f0aa4587267b17af159d32b2e8f Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sat, 10 Dec 2022 08:15:46 +0100 Subject: [PATCH 02/15] Extract Authorization header to an interceptor --- .../mealient/data/auth/impl/AuthRepoImpl.kt | 3 +- .../data/network/MealieDataSourceWrapper.kt | 32 +++++++++---------- .../gq/kirmanak/mealient/di/AuthModule.kt | 5 +++ .../ui/recipes/images/RecipeModelLoader.kt | 26 +++------------ .../network/MealieDataSourceWrapperTest.kt | 30 +++++++---------- .../datasource/AuthenticationProvider.kt | 6 ++++ .../mealient/datasource/DataSourceModule.kt | 10 ++++-- .../datasource/impl/AuthInterceptor.kt | 32 +++++++++++++++++++ .../datasource/v0/MealieDataSourceV0.kt | 4 --- .../datasource/v0/MealieDataSourceV0Impl.kt | 20 +++++------- .../mealient/datasource/v0/MealieServiceV0.kt | 5 --- .../datasource/v1/MealieDataSourceV1.kt | 5 --- .../datasource/v1/MealieDataSourceV1Impl.kt | 25 ++++++--------- .../mealient/datasource/v1/MealieServiceV1.kt | 6 ---- 14 files changed, 103 insertions(+), 106 deletions(-) create mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt create mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt index e963a62..531c0b5 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt @@ -3,6 +3,7 @@ package gq.kirmanak.mealient.data.auth.impl import gq.kirmanak.mealient.data.auth.AuthDataSource import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.auth.AuthStorage +import gq.kirmanak.mealient.datasource.AuthenticationProvider import gq.kirmanak.mealient.datasource.runCatchingExceptCancel import gq.kirmanak.mealient.logging.Logger import kotlinx.coroutines.flow.Flow @@ -15,7 +16,7 @@ class AuthRepoImpl @Inject constructor( private val authStorage: AuthStorage, private val authDataSource: AuthDataSource, private val logger: Logger, -) : AuthRepo { +) : AuthRepo, AuthenticationProvider { override val isAuthorizedFlow: Flow get() = authStorage.authHeaderFlow.map { it != null } diff --git a/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt b/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt index f830766..2af3770 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt @@ -35,12 +35,12 @@ class MealieDataSourceWrapper @Inject constructor( override suspend fun addRecipe( recipe: AddRecipeInfo, - ): String = makeCall { token, url, version -> + ): String = makeCall { url, version -> when (version) { - ServerVersion.V0 -> v0Source.addRecipe(url, token, recipe.toV0Request()) + ServerVersion.V0 -> v0Source.addRecipe(url, recipe.toV0Request()) ServerVersion.V1 -> { - val slug = v1Source.createRecipe(url, token, recipe.toV1CreateRequest()) - v1Source.updateRecipe(url, token, slug, recipe.toV1UpdateRequest()) + val slug = v1Source.createRecipe(url, recipe.toV1CreateRequest()) + v1Source.updateRecipe(url, slug, recipe.toV1UpdateRequest()) slug } } @@ -49,53 +49,53 @@ class MealieDataSourceWrapper @Inject constructor( override suspend fun requestRecipes( start: Int, limit: Int, - ): List = makeCall { token, url, version -> + ): List = makeCall { url, version -> when (version) { ServerVersion.V0 -> { - v0Source.requestRecipes(url, token, start, limit).map { it.toRecipeSummaryInfo() } + v0Source.requestRecipes(url, start, limit).map { it.toRecipeSummaryInfo() } } ServerVersion.V1 -> { // Imagine start is 30 and limit is 15. It means that we already have page 1 and 2, now we need page 3 val page = start / limit + 1 - v1Source.requestRecipes(url, token, page, limit).map { it.toRecipeSummaryInfo() } + v1Source.requestRecipes(url, page, limit).map { it.toRecipeSummaryInfo() } } } } override suspend fun requestRecipeInfo( slug: String, - ): FullRecipeInfo = makeCall { token, url, version -> + ): FullRecipeInfo = makeCall { url, version -> when (version) { - ServerVersion.V0 -> v0Source.requestRecipeInfo(url, token, slug).toFullRecipeInfo() - ServerVersion.V1 -> v1Source.requestRecipeInfo(url, token, slug).toFullRecipeInfo() + ServerVersion.V0 -> v0Source.requestRecipeInfo(url, slug).toFullRecipeInfo() + ServerVersion.V1 -> v1Source.requestRecipeInfo(url, slug).toFullRecipeInfo() } } override suspend fun parseRecipeFromURL( parseRecipeURLInfo: ParseRecipeURLInfo, - ): String = makeCall { token, url, version -> + ): String = makeCall { url, version -> when (version) { ServerVersion.V0 -> { - v0Source.parseRecipeFromURL(url, token, parseRecipeURLInfo.toV0Request()) + v0Source.parseRecipeFromURL(url, parseRecipeURLInfo.toV0Request()) } ServerVersion.V1 -> { - v1Source.parseRecipeFromURL(url, token, parseRecipeURLInfo.toV1Request()) + v1Source.parseRecipeFromURL(url, parseRecipeURLInfo.toV1Request()) } } } - private suspend inline fun makeCall(block: (String?, String, ServerVersion) -> T): T { + private suspend inline fun makeCall(block: (String, ServerVersion) -> T): T { val authHeader = authRepo.getAuthHeader() val url = serverInfoRepo.requireUrl() val version = serverInfoRepo.getVersion() - return runCatchingExceptCancel { block(authHeader, url, version) }.getOrElse { + return runCatchingExceptCancel { block(url, version) }.getOrElse { if (it is NetworkError.Unauthorized) { logger.e { "Unauthorized, trying to invalidate token" } authRepo.invalidateAuthHeader() // Trying again with new authentication header val newHeader = authRepo.getAuthHeader() logger.e { "New token ${if (newHeader == authHeader) "matches" else "doesn't match"} old token" } - if (newHeader == authHeader) throw it else block(newHeader, url, version) + if (newHeader == authHeader) throw it else block(url, version) } else { throw it } diff --git a/app/src/main/java/gq/kirmanak/mealient/di/AuthModule.kt b/app/src/main/java/gq/kirmanak/mealient/di/AuthModule.kt index 4687932..be8828e 100644 --- a/app/src/main/java/gq/kirmanak/mealient/di/AuthModule.kt +++ b/app/src/main/java/gq/kirmanak/mealient/di/AuthModule.kt @@ -14,6 +14,7 @@ import gq.kirmanak.mealient.data.auth.AuthStorage import gq.kirmanak.mealient.data.auth.impl.AuthDataSourceImpl import gq.kirmanak.mealient.data.auth.impl.AuthRepoImpl import gq.kirmanak.mealient.data.auth.impl.AuthStorageImpl +import gq.kirmanak.mealient.datasource.AuthenticationProvider import javax.inject.Singleton @Module @@ -37,6 +38,10 @@ interface AuthModule { @Singleton fun bindAuthRepo(authRepo: AuthRepoImpl): AuthRepo + @Binds + @Singleton + fun bindAuthProvider(authRepo: AuthRepoImpl): AuthenticationProvider + @Binds @Singleton fun bindAuthStorage(authStorageImpl: AuthStorageImpl): AuthStorage diff --git a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/images/RecipeModelLoader.kt b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/images/RecipeModelLoader.kt index a1651b3..9730bce 100644 --- a/app/src/main/java/gq/kirmanak/mealient/ui/recipes/images/RecipeModelLoader.kt +++ b/app/src/main/java/gq/kirmanak/mealient/ui/recipes/images/RecipeModelLoader.kt @@ -1,12 +1,12 @@ package gq.kirmanak.mealient.ui.recipes.images import com.bumptech.glide.load.Options -import com.bumptech.glide.load.model.* +import com.bumptech.glide.load.model.GlideUrl +import com.bumptech.glide.load.model.ModelCache +import com.bumptech.glide.load.model.ModelLoader import com.bumptech.glide.load.model.stream.BaseGlideUrlLoader -import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.recipes.impl.RecipeImageUrlProvider import gq.kirmanak.mealient.database.recipe.entity.RecipeSummaryEntity -import gq.kirmanak.mealient.datasource.DataSourceModule.Companion.AUTHORIZATION_HEADER_NAME import gq.kirmanak.mealient.logging.Logger import kotlinx.coroutines.runBlocking import java.io.InputStream @@ -16,7 +16,6 @@ import javax.inject.Singleton class RecipeModelLoader private constructor( private val recipeImageUrlProvider: RecipeImageUrlProvider, private val logger: Logger, - private val authRepo: AuthRepo, concreteLoader: ModelLoader, cache: ModelCache, ) : BaseGlideUrlLoader(concreteLoader, cache) { @@ -25,13 +24,12 @@ class RecipeModelLoader private constructor( class Factory @Inject constructor( private val recipeImageUrlProvider: RecipeImageUrlProvider, private val logger: Logger, - private val authRepo: AuthRepo, ) { fun build( concreteLoader: ModelLoader, cache: ModelCache, - ) = RecipeModelLoader(recipeImageUrlProvider, logger, authRepo, concreteLoader, cache) + ) = RecipeModelLoader(recipeImageUrlProvider, logger, concreteLoader, cache) } @@ -46,20 +44,4 @@ class RecipeModelLoader private constructor( logger.v { "getUrl() called with: model = $model, width = $width, height = $height, options = $options" } return runBlocking { recipeImageUrlProvider.generateImageUrl(model?.imageId) } } - - override fun getHeaders( - model: RecipeSummaryEntity?, - width: Int, - height: Int, - options: Options? - ): Headers? { - val authorization = runBlocking { authRepo.getAuthHeader() } - return if (authorization.isNullOrBlank()) { - super.getHeaders(model, width, height, options) - } else { - LazyHeaders.Builder() - .setHeader(AUTHORIZATION_HEADER_NAME, authorization) - .build() - } - } } \ No newline at end of file diff --git a/app/src/test/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapperTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapperTest.kt index cc23507..49c02f3 100644 --- a/app/src/test/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapperTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapperTest.kt @@ -57,11 +57,8 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { fun `when makeCall fails with Unauthorized expect it to invalidate token`() = runTest { val slug = "porridge" coEvery { - v0Source.requestRecipeInfo(any(), isNull(), any()) - } throws NetworkError.Unauthorized(IOException()) - coEvery { - v0Source.requestRecipeInfo(any(), eq(TEST_AUTH_HEADER), any()) - } returns PORRIDGE_RECIPE_RESPONSE_V0 + v0Source.requestRecipeInfo(any(), any()) + } throws NetworkError.Unauthorized(IOException()) andThen PORRIDGE_RECIPE_RESPONSE_V0 coEvery { serverInfoRepo.getVersion() } returns TEST_SERVER_VERSION_V0 coEvery { serverInfoRepo.requireUrl() } returns TEST_BASE_URL coEvery { authRepo.getAuthHeader() } returns null andThen TEST_AUTH_HEADER @@ -79,7 +76,7 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { fun `when server version v1 expect requestRecipeInfo to call v1`() = runTest { val slug = "porridge" coEvery { - v1Source.requestRecipeInfo(eq(TEST_BASE_URL), eq(TEST_AUTH_HEADER), eq(slug)) + v1Source.requestRecipeInfo(eq(TEST_BASE_URL), eq(slug)) } returns PORRIDGE_RECIPE_RESPONSE_V1 coEvery { serverInfoRepo.getVersion() } returns TEST_SERVER_VERSION_V1 coEvery { serverInfoRepo.requireUrl() } returns TEST_BASE_URL @@ -87,7 +84,7 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { val actual = subject.requestRecipeInfo(slug) - coVerify { v1Source.requestRecipeInfo(eq(TEST_BASE_URL), eq(TEST_AUTH_HEADER), eq(slug)) } + coVerify { v1Source.requestRecipeInfo(eq(TEST_BASE_URL), eq(slug)) } assertThat(actual).isEqualTo(PORRIDGE_FULL_RECIPE_INFO) } @@ -95,7 +92,7 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { @Test fun `when server version v1 expect requestRecipes to call v1`() = runTest { coEvery { - v1Source.requestRecipes(any(), any(), any(), any()) + v1Source.requestRecipes(any(), any(), any()) } returns listOf(PORRIDGE_RECIPE_SUMMARY_RESPONSE_V1) coEvery { serverInfoRepo.getVersion() } returns TEST_SERVER_VERSION_V1 coEvery { serverInfoRepo.requireUrl() } returns TEST_BASE_URL @@ -106,7 +103,7 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { val page = 5 // 0-9 (1), 10-19 (2), 20-29 (3), 30-39 (4), 40-49 (5) val perPage = 10 coVerify { - v1Source.requestRecipes(eq(TEST_BASE_URL), eq(TEST_AUTH_HEADER), eq(page), eq(perPage)) + v1Source.requestRecipes(eq(TEST_BASE_URL), eq(page), eq(perPage)) } assertThat(actual).isEqualTo(listOf(RECIPE_SUMMARY_PORRIDGE_V1)) @@ -115,7 +112,7 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { @Test fun `when server version v0 expect requestRecipes to call v0`() = runTest { coEvery { - v0Source.requestRecipes(any(), any(), any(), any()) + v0Source.requestRecipes(any(), any(), any()) } returns listOf(PORRIDGE_RECIPE_SUMMARY_RESPONSE_V0) coEvery { serverInfoRepo.getVersion() } returns TEST_SERVER_VERSION_V0 coEvery { serverInfoRepo.requireUrl() } returns TEST_BASE_URL @@ -126,7 +123,7 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { val actual = subject.requestRecipes(start, limit) coVerify { - v0Source.requestRecipes(eq(TEST_BASE_URL), eq(TEST_AUTH_HEADER), eq(start), eq(limit)) + v0Source.requestRecipes(eq(TEST_BASE_URL), eq(start), eq(limit)) } assertThat(actual).isEqualTo(listOf(RECIPE_SUMMARY_PORRIDGE_V0)) @@ -134,7 +131,7 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { @Test(expected = IOException::class) fun `when request fails expect addRecipe to rethrow`() = runTest { - coEvery { v0Source.addRecipe(any(), any(), any()) } throws IOException() + coEvery { v0Source.addRecipe(any(), any()) } throws IOException() coEvery { serverInfoRepo.getVersion() } returns TEST_SERVER_VERSION_V0 coEvery { serverInfoRepo.requireUrl() } returns TEST_BASE_URL coEvery { authRepo.getAuthHeader() } returns TEST_AUTH_HEADER @@ -145,7 +142,7 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { fun `when server version v0 expect addRecipe to call v0`() = runTest { val slug = "porridge" - coEvery { v0Source.addRecipe(any(), any(), any()) } returns slug + coEvery { v0Source.addRecipe(any(), any()) } returns slug coEvery { serverInfoRepo.getVersion() } returns TEST_SERVER_VERSION_V0 coEvery { serverInfoRepo.requireUrl() } returns TEST_BASE_URL coEvery { authRepo.getAuthHeader() } returns TEST_AUTH_HEADER @@ -155,7 +152,6 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { coVerify { v0Source.addRecipe( eq(TEST_BASE_URL), - eq(TEST_AUTH_HEADER), eq(PORRIDGE_ADD_RECIPE_REQUEST_V0), ) } @@ -167,9 +163,9 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { fun `when server version v1 expect addRecipe to call v1`() = runTest { val slug = "porridge" - coEvery { v1Source.createRecipe(any(), any(), any()) } returns slug + coEvery { v1Source.createRecipe(any(), any()) } returns slug coEvery { - v1Source.updateRecipe(any(), any(), any(), any()) + v1Source.updateRecipe(any(), any(), any()) } returns PORRIDGE_RECIPE_RESPONSE_V1 coEvery { serverInfoRepo.getVersion() } returns TEST_SERVER_VERSION_V1 coEvery { serverInfoRepo.requireUrl() } returns TEST_BASE_URL @@ -180,13 +176,11 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { coVerifySequence { v1Source.createRecipe( eq(TEST_BASE_URL), - eq(TEST_AUTH_HEADER), eq(PORRIDGE_CREATE_RECIPE_REQUEST_V1), ) v1Source.updateRecipe( eq(TEST_BASE_URL), - eq(TEST_AUTH_HEADER), eq(slug), eq(PORRIDGE_UPDATE_RECIPE_REQUEST_V1), ) diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt new file mode 100644 index 0000000..ba22dbf --- /dev/null +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt @@ -0,0 +1,6 @@ +package gq.kirmanak.mealient.datasource + +interface AuthenticationProvider { + + suspend fun getAuthHeader(): String? +} \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt index 1582f0c..73ed120 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt @@ -6,6 +6,8 @@ import dagger.Module import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent +import dagger.multibindings.IntoSet +import gq.kirmanak.mealient.datasource.impl.AuthInterceptor import gq.kirmanak.mealient.datasource.impl.CacheBuilderImpl import gq.kirmanak.mealient.datasource.impl.NetworkRequestWrapperImpl import gq.kirmanak.mealient.datasource.impl.OkHttpBuilderImpl @@ -18,6 +20,7 @@ import gq.kirmanak.mealient.datasource.v1.MealieDataSourceV1Impl import gq.kirmanak.mealient.datasource.v1.MealieServiceV1 import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.json.Json +import okhttp3.Interceptor import okhttp3.MediaType.Companion.toMediaType import okhttp3.OkHttpClient import retrofit2.Converter @@ -31,8 +34,6 @@ interface DataSourceModule { companion object { - const val AUTHORIZATION_HEADER_NAME = "Authorization" - @Provides @Singleton fun provideJson(): Json = Json { @@ -87,4 +88,9 @@ interface DataSourceModule { @Binds @Singleton fun bindNetworkRequestWrapper(networkRequestWrapperImpl: NetworkRequestWrapperImpl): NetworkRequestWrapper + + @Binds + @Singleton + @IntoSet + fun bindAuthInterceptor(authInterceptor: AuthInterceptor): Interceptor } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt new file mode 100644 index 0000000..795e773 --- /dev/null +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt @@ -0,0 +1,32 @@ +package gq.kirmanak.mealient.datasource.impl + +import gq.kirmanak.mealient.datasource.AuthenticationProvider +import kotlinx.coroutines.runBlocking +import okhttp3.Interceptor +import okhttp3.Response +import javax.inject.Inject +import javax.inject.Provider +import javax.inject.Singleton + +@Singleton +class AuthInterceptor @Inject constructor( + private val authenticationProvider: Provider, +) : Interceptor { + + override fun intercept(chain: Interceptor.Chain): Response { + val token = runBlocking { authenticationProvider.get().getAuthHeader() } + val request = if (token == null) { + chain.request() + } else { + chain.request() + .newBuilder() + .header(HEADER_NAME, token) + .build() + } + return chain.proceed(request) + } + + companion object { + private const val HEADER_NAME = "Authorization" + } +} diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0.kt index 79e3263..6a6abd2 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0.kt @@ -10,7 +10,6 @@ interface MealieDataSourceV0 { suspend fun addRecipe( baseUrl: String, - token: String?, recipe: AddRecipeRequestV0, ): String @@ -29,20 +28,17 @@ interface MealieDataSourceV0 { suspend fun requestRecipes( baseUrl: String, - token: String?, start: Int, limit: Int, ): List suspend fun requestRecipeInfo( baseUrl: String, - token: String?, slug: String, ): GetRecipeResponseV0 suspend fun parseRecipeFromURL( baseUrl: String, - token: String?, request: ParseRecipeURLRequestV0, ): String } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0Impl.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0Impl.kt index d39485a..f6939d7 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0Impl.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0Impl.kt @@ -26,12 +26,11 @@ class MealieDataSourceV0Impl @Inject constructor( override suspend fun addRecipe( baseUrl: String, - token: String?, recipe: AddRecipeRequestV0, ): String = networkRequestWrapper.makeCallAndHandleUnauthorized( - block = { service.addRecipe("$baseUrl/api/recipes/create", token, recipe) }, + block = { service.addRecipe("$baseUrl/api/recipes/create", recipe) }, logMethod = { "addRecipe" }, - logParameters = { "baseUrl = $baseUrl, token = $token, recipe = $recipe" } + logParameters = { "baseUrl = $baseUrl, recipe = $recipe" } ) override suspend fun authenticate( @@ -64,33 +63,30 @@ class MealieDataSourceV0Impl @Inject constructor( override suspend fun requestRecipes( baseUrl: String, - token: String?, start: Int, limit: Int, ): List = networkRequestWrapper.makeCallAndHandleUnauthorized( - block = { service.getRecipeSummary("$baseUrl/api/recipes/summary", token, start, limit) }, + block = { service.getRecipeSummary("$baseUrl/api/recipes/summary", start, limit) }, logMethod = { "requestRecipes" }, - logParameters = { "baseUrl = $baseUrl, token = $token, start = $start, limit = $limit" } + logParameters = { "baseUrl = $baseUrl, start = $start, limit = $limit" } ) override suspend fun requestRecipeInfo( baseUrl: String, - token: String?, slug: String, ): GetRecipeResponseV0 = networkRequestWrapper.makeCallAndHandleUnauthorized( - block = { service.getRecipe("$baseUrl/api/recipes/$slug", token) }, + block = { service.getRecipe("$baseUrl/api/recipes/$slug") }, logMethod = { "requestRecipeInfo" }, - logParameters = { "baseUrl = $baseUrl, token = $token, slug = $slug" } + logParameters = { "baseUrl = $baseUrl, slug = $slug" } ) override suspend fun parseRecipeFromURL( baseUrl: String, - token: String?, request: ParseRecipeURLRequestV0 ): String = networkRequestWrapper.makeCallAndHandleUnauthorized( - block = { service.createRecipeFromURL("$baseUrl/api/recipes/create-url", token, request) }, + block = { service.createRecipeFromURL("$baseUrl/api/recipes/create-url", request) }, logMethod = { "parseRecipeFromURL" }, - logParameters = { "baseUrl = $baseUrl, token = $token, request = $request" } + logParameters = { "baseUrl = $baseUrl, request = $request" } ) } diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieServiceV0.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieServiceV0.kt index 55771f3..579dada 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieServiceV0.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieServiceV0.kt @@ -1,6 +1,5 @@ package gq.kirmanak.mealient.datasource.v0 -import gq.kirmanak.mealient.datasource.DataSourceModule.Companion.AUTHORIZATION_HEADER_NAME import gq.kirmanak.mealient.datasource.v0.models.* import retrofit2.http.* @@ -17,7 +16,6 @@ interface MealieServiceV0 { @POST suspend fun addRecipe( @Url url: String, - @Header(AUTHORIZATION_HEADER_NAME) token: String?, @Body addRecipeRequestV0: AddRecipeRequestV0, ): String @@ -29,7 +27,6 @@ interface MealieServiceV0 { @GET suspend fun getRecipeSummary( @Url url: String, - @Header(AUTHORIZATION_HEADER_NAME) token: String?, @Query("start") start: Int, @Query("limit") limit: Int, ): List @@ -37,13 +34,11 @@ interface MealieServiceV0 { @GET suspend fun getRecipe( @Url url: String, - @Header(AUTHORIZATION_HEADER_NAME) token: String?, ): GetRecipeResponseV0 @POST suspend fun createRecipeFromURL( @Url url: String, - @Header(AUTHORIZATION_HEADER_NAME) token: String?, @Body request: ParseRecipeURLRequestV0, ): String } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1.kt index a231834..9b6f665 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1.kt @@ -11,13 +11,11 @@ interface MealieDataSourceV1 { suspend fun createRecipe( baseUrl: String, - token: String?, recipe: CreateRecipeRequestV1, ): String suspend fun updateRecipe( baseUrl: String, - token: String?, slug: String, recipe: UpdateRecipeRequestV1, ): GetRecipeResponseV1 @@ -37,20 +35,17 @@ interface MealieDataSourceV1 { suspend fun requestRecipes( baseUrl: String, - token: String?, page: Int, perPage: Int, ): List suspend fun requestRecipeInfo( baseUrl: String, - token: String?, slug: String, ): GetRecipeResponseV1 suspend fun parseRecipeFromURL( baseUrl: String, - token: String?, request: ParseRecipeURLRequestV1, ): String } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1Impl.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1Impl.kt index f7551b0..7bb2822 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1Impl.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1Impl.kt @@ -27,23 +27,21 @@ class MealieDataSourceV1Impl @Inject constructor( override suspend fun createRecipe( baseUrl: String, - token: String?, recipe: CreateRecipeRequestV1 ): String = networkRequestWrapper.makeCallAndHandleUnauthorized( - block = { service.createRecipe("$baseUrl/api/recipes", token, recipe) }, + block = { service.createRecipe("$baseUrl/api/recipes", recipe) }, logMethod = { "createRecipe" }, - logParameters = { "baseUrl = $baseUrl, token = $token, recipe = $recipe" } + logParameters = { "baseUrl = $baseUrl, recipe = $recipe" } ) override suspend fun updateRecipe( baseUrl: String, - token: String?, slug: String, recipe: UpdateRecipeRequestV1 ): GetRecipeResponseV1 = networkRequestWrapper.makeCallAndHandleUnauthorized( - block = { service.updateRecipe("$baseUrl/api/recipes/$slug", token, recipe) }, + block = { service.updateRecipe("$baseUrl/api/recipes/$slug", recipe) }, logMethod = { "updateRecipe" }, - logParameters = { "baseUrl = $baseUrl, token = $token, slug = $slug, recipe = $recipe" } + logParameters = { "baseUrl = $baseUrl, slug = $slug, recipe = $recipe" } ) override suspend fun authenticate( @@ -76,33 +74,30 @@ class MealieDataSourceV1Impl @Inject constructor( override suspend fun requestRecipes( baseUrl: String, - token: String?, page: Int, perPage: Int ): List = networkRequestWrapper.makeCallAndHandleUnauthorized( - block = { service.getRecipeSummary("$baseUrl/api/recipes", token, page, perPage) }, + block = { service.getRecipeSummary("$baseUrl/api/recipes", page, perPage) }, logMethod = { "requestRecipes" }, - logParameters = { "baseUrl = $baseUrl, token = $token, page = $page, perPage = $perPage" } + logParameters = { "baseUrl = $baseUrl, page = $page, perPage = $perPage" } ).items override suspend fun requestRecipeInfo( baseUrl: String, - token: String?, slug: String ): GetRecipeResponseV1 = networkRequestWrapper.makeCallAndHandleUnauthorized( - block = { service.getRecipe("$baseUrl/api/recipes/$slug", token) }, + block = { service.getRecipe("$baseUrl/api/recipes/$slug") }, logMethod = { "requestRecipeInfo" }, - logParameters = { "baseUrl = $baseUrl, token = $token, slug = $slug" } + logParameters = { "baseUrl = $baseUrl, slug = $slug" } ) override suspend fun parseRecipeFromURL( baseUrl: String, - token: String?, request: ParseRecipeURLRequestV1 ): String = networkRequestWrapper.makeCallAndHandleUnauthorized( - block = { service.createRecipeFromURL("$baseUrl/api/recipes/create-url", token, request) }, + block = { service.createRecipeFromURL("$baseUrl/api/recipes/create-url", request) }, logMethod = { "parseRecipeFromURL" }, - logParameters = { "baseUrl = $baseUrl, token = $token, request = $request" } + logParameters = { "baseUrl = $baseUrl, request = $request" } ) diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieServiceV1.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieServiceV1.kt index c5aa05e..004ccf8 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieServiceV1.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieServiceV1.kt @@ -1,6 +1,5 @@ package gq.kirmanak.mealient.datasource.v1 -import gq.kirmanak.mealient.datasource.DataSourceModule.Companion.AUTHORIZATION_HEADER_NAME import gq.kirmanak.mealient.datasource.v1.models.* import retrofit2.http.* @@ -17,14 +16,12 @@ interface MealieServiceV1 { @POST suspend fun createRecipe( @Url url: String, - @Header(AUTHORIZATION_HEADER_NAME) token: String?, @Body addRecipeRequest: CreateRecipeRequestV1, ): String @PATCH suspend fun updateRecipe( @Url url: String, - @Header(AUTHORIZATION_HEADER_NAME) token: String?, @Body addRecipeRequest: UpdateRecipeRequestV1, ): GetRecipeResponseV1 @@ -36,7 +33,6 @@ interface MealieServiceV1 { @GET suspend fun getRecipeSummary( @Url url: String, - @Header(AUTHORIZATION_HEADER_NAME) token: String?, @Query("page") page: Int, @Query("perPage") perPage: Int, ): GetRecipesResponseV1 @@ -44,13 +40,11 @@ interface MealieServiceV1 { @GET suspend fun getRecipe( @Url url: String, - @Header(AUTHORIZATION_HEADER_NAME) token: String?, ): GetRecipeResponseV1 @POST suspend fun createRecipeFromURL( @Url url: String, - @Header(AUTHORIZATION_HEADER_NAME) token: String?, @Body request: ParseRecipeURLRequestV1, ): String } \ No newline at end of file From 9adf37ae33bf87d51a5498e116b126ac65b8fe07 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sat, 10 Dec 2022 09:30:38 +0100 Subject: [PATCH 03/15] Move token invalidation to auth interceptor --- .../data/network/MealieDataSourceWrapper.kt | 20 +-- .../network/MealieDataSourceWrapperTest.kt | 23 +-- .../datasource/AuthenticationProvider.kt | 2 + .../datasource/impl/AuthInterceptor.kt | 42 ++++- .../datasource/AuthInterceptorTest.kt | 164 ++++++++++++++++++ 5 files changed, 202 insertions(+), 49 deletions(-) create mode 100644 datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/AuthInterceptorTest.kt diff --git a/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt b/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt index 2af3770..51be915 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt @@ -2,7 +2,6 @@ package gq.kirmanak.mealient.data.network import gq.kirmanak.mealient.data.add.AddRecipeDataSource import gq.kirmanak.mealient.data.add.AddRecipeInfo -import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.baseurl.ServerInfoRepo import gq.kirmanak.mealient.data.baseurl.ServerVersion import gq.kirmanak.mealient.data.recipes.network.FullRecipeInfo @@ -10,8 +9,6 @@ import gq.kirmanak.mealient.data.recipes.network.RecipeDataSource import gq.kirmanak.mealient.data.recipes.network.RecipeSummaryInfo import gq.kirmanak.mealient.data.share.ParseRecipeDataSource import gq.kirmanak.mealient.data.share.ParseRecipeURLInfo -import gq.kirmanak.mealient.datasource.NetworkError -import gq.kirmanak.mealient.datasource.runCatchingExceptCancel import gq.kirmanak.mealient.datasource.v0.MealieDataSourceV0 import gq.kirmanak.mealient.datasource.v1.MealieDataSourceV1 import gq.kirmanak.mealient.extensions.toFullRecipeInfo @@ -20,17 +17,14 @@ import gq.kirmanak.mealient.extensions.toV0Request import gq.kirmanak.mealient.extensions.toV1CreateRequest import gq.kirmanak.mealient.extensions.toV1Request import gq.kirmanak.mealient.extensions.toV1UpdateRequest -import gq.kirmanak.mealient.logging.Logger import javax.inject.Inject import javax.inject.Singleton @Singleton class MealieDataSourceWrapper @Inject constructor( private val serverInfoRepo: ServerInfoRepo, - private val authRepo: AuthRepo, private val v0Source: MealieDataSourceV0, private val v1Source: MealieDataSourceV1, - private val logger: Logger, ) : AddRecipeDataSource, RecipeDataSource, ParseRecipeDataSource { override suspend fun addRecipe( @@ -85,20 +79,8 @@ class MealieDataSourceWrapper @Inject constructor( } private suspend inline fun makeCall(block: (String, ServerVersion) -> T): T { - val authHeader = authRepo.getAuthHeader() val url = serverInfoRepo.requireUrl() val version = serverInfoRepo.getVersion() - return runCatchingExceptCancel { block(url, version) }.getOrElse { - if (it is NetworkError.Unauthorized) { - logger.e { "Unauthorized, trying to invalidate token" } - authRepo.invalidateAuthHeader() - // Trying again with new authentication header - val newHeader = authRepo.getAuthHeader() - logger.e { "New token ${if (newHeader == authHeader) "matches" else "doesn't match"} old token" } - if (newHeader == authHeader) throw it else block(url, version) - } else { - throw it - } - } + return block(url, version) } } \ No newline at end of file diff --git a/app/src/test/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapperTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapperTest.kt index 49c02f3..464792c 100644 --- a/app/src/test/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapperTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapperTest.kt @@ -3,7 +3,6 @@ package gq.kirmanak.mealient.data.network import com.google.common.truth.Truth.assertThat import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.baseurl.ServerInfoRepo -import gq.kirmanak.mealient.datasource.NetworkError import gq.kirmanak.mealient.datasource.v0.MealieDataSourceV0 import gq.kirmanak.mealient.datasource.v1.MealieDataSourceV1 import gq.kirmanak.mealient.test.AuthImplTestData.TEST_AUTH_HEADER @@ -15,7 +14,6 @@ import gq.kirmanak.mealient.test.RecipeImplTestData.PORRIDGE_ADD_RECIPE_INFO import gq.kirmanak.mealient.test.RecipeImplTestData.PORRIDGE_ADD_RECIPE_REQUEST_V0 import gq.kirmanak.mealient.test.RecipeImplTestData.PORRIDGE_CREATE_RECIPE_REQUEST_V1 import gq.kirmanak.mealient.test.RecipeImplTestData.PORRIDGE_FULL_RECIPE_INFO -import gq.kirmanak.mealient.test.RecipeImplTestData.PORRIDGE_RECIPE_RESPONSE_V0 import gq.kirmanak.mealient.test.RecipeImplTestData.PORRIDGE_RECIPE_RESPONSE_V1 import gq.kirmanak.mealient.test.RecipeImplTestData.PORRIDGE_RECIPE_SUMMARY_RESPONSE_V0 import gq.kirmanak.mealient.test.RecipeImplTestData.PORRIDGE_RECIPE_SUMMARY_RESPONSE_V1 @@ -50,26 +48,7 @@ class MealieDataSourceWrapperTest : BaseUnitTest() { @Before override fun setUp() { super.setUp() - subject = MealieDataSourceWrapper(serverInfoRepo, authRepo, v0Source, v1Source, logger) - } - - @Test - fun `when makeCall fails with Unauthorized expect it to invalidate token`() = runTest { - val slug = "porridge" - coEvery { - v0Source.requestRecipeInfo(any(), any()) - } throws NetworkError.Unauthorized(IOException()) andThen PORRIDGE_RECIPE_RESPONSE_V0 - coEvery { serverInfoRepo.getVersion() } returns TEST_SERVER_VERSION_V0 - coEvery { serverInfoRepo.requireUrl() } returns TEST_BASE_URL - coEvery { authRepo.getAuthHeader() } returns null andThen TEST_AUTH_HEADER - - subject.requestRecipeInfo(slug) - - coVerifySequence { - authRepo.getAuthHeader() - authRepo.invalidateAuthHeader() - authRepo.getAuthHeader() - } + subject = MealieDataSourceWrapper(serverInfoRepo, v0Source, v1Source) } @Test diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt index ba22dbf..b43f93a 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt @@ -3,4 +3,6 @@ package gq.kirmanak.mealient.datasource interface AuthenticationProvider { suspend fun getAuthHeader(): String? + + suspend fun invalidateAuthHeader() } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt index 795e773..5f2fc71 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt @@ -1,9 +1,11 @@ package gq.kirmanak.mealient.datasource.impl +import androidx.annotation.VisibleForTesting import gq.kirmanak.mealient.datasource.AuthenticationProvider import kotlinx.coroutines.runBlocking import okhttp3.Interceptor import okhttp3.Response +import retrofit2.HttpException import javax.inject.Inject import javax.inject.Provider import javax.inject.Singleton @@ -14,19 +16,43 @@ class AuthInterceptor @Inject constructor( ) : Interceptor { override fun intercept(chain: Interceptor.Chain): Response { - val token = runBlocking { authenticationProvider.get().getAuthHeader() } - val request = if (token == null) { - chain.request() + val token = getAuthHeader() + return if (token == null) { + proceedWithAuthHeader(chain) } else { - chain.request() - .newBuilder() - .header(HEADER_NAME, token) - .build() + try { + proceedWithAuthHeader(chain, token) + } catch (e: HttpException) { + if (e.code() in setOf(401, 403)) { + invalidateAuthHeader() + proceedWithAuthHeader(chain, getAuthHeader()) + } else { + throw e + } + } + } + } + + private fun proceedWithAuthHeader(chain: Interceptor.Chain, token: String? = null): Response { + val requestBuilder = chain.request().newBuilder() + val request = if (token == null) { + requestBuilder.removeHeader(HEADER_NAME).build() + } else { + requestBuilder.header(HEADER_NAME, token).build() } return chain.proceed(request) } + private fun getAuthHeader() = runBlocking { + authenticationProvider.get().getAuthHeader() + } + + private fun invalidateAuthHeader() = runBlocking { + authenticationProvider.get().invalidateAuthHeader() + } + companion object { - private const val HEADER_NAME = "Authorization" + @VisibleForTesting + const val HEADER_NAME = "Authorization" } } diff --git a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/AuthInterceptorTest.kt b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/AuthInterceptorTest.kt new file mode 100644 index 0000000..b89c2d8 --- /dev/null +++ b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/AuthInterceptorTest.kt @@ -0,0 +1,164 @@ +package gq.kirmanak.mealient.datasource + +import com.google.common.truth.Truth.assertThat +import gq.kirmanak.mealient.datasource.impl.AuthInterceptor +import gq.kirmanak.mealient.test.BaseUnitTest +import io.mockk.CapturingSlot +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.coVerifySequence +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.slot +import io.mockk.verify +import okhttp3.Interceptor +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Before +import org.junit.Test +import retrofit2.HttpException +import retrofit2.Response as RetrofitResponse + +class AuthInterceptorTest : BaseUnitTest() { + + private lateinit var subject: AuthInterceptor + + @MockK(relaxUnitFun = true) + lateinit var authenticationProvider: AuthenticationProvider + + @MockK(relaxUnitFun = true) + lateinit var chain: Interceptor.Chain + + @Before + override fun setUp() { + super.setUp() + subject = AuthInterceptor { authenticationProvider } + every { chain.request() } returns Request.Builder().url("http://localhost").build() + } + + @Test + fun `when no header then still proceeds`() { + mockProceedCallAndCaptureRequest() + coEvery { authenticationProvider.getAuthHeader() } returns null + subject.intercept(chain) + verify { chain.proceed(any()) } + } + + @Test + fun `when has header then adds header`() { + coEvery { authenticationProvider.getAuthHeader() } returns "token" + val requestSlot = mockProceedCallAndCaptureRequest() + subject.intercept(chain) + assertThat(requestSlot.captured.header(AuthInterceptor.HEADER_NAME)).isEqualTo("token") + } + + @Test(expected = HttpException::class) + fun `when unauthorized but didn't have token then throws`() { + coEvery { authenticationProvider.getAuthHeader() } returns null + mockUnauthorized() + subject.intercept(chain) + } + + @Test(expected = HttpException::class) + fun `when unauthorized and had token then invalidates it`() { + coEvery { authenticationProvider.getAuthHeader() } returns "token" + mockUnauthorized() + subject.intercept(chain) + coVerify { authenticationProvider.invalidateAuthHeader() } + } + + @Test(expected = HttpException::class) + fun `when not found and had token then throws exception`() { + coEvery { authenticationProvider.getAuthHeader() } returns "token" + val requestSlot = slot() + every { + chain.proceed(capture(requestSlot)) + } answers { + throw HttpException(RetrofitResponse.error(404, "".toResponseBody())) + } + subject.intercept(chain) + } + + @Test + fun `when not found and had token then does not invalidate it`() { + coEvery { authenticationProvider.getAuthHeader() } returns "token" + val requestSlot = slot() + every { + chain.proceed(capture(requestSlot)) + } answers { + throw HttpException(RetrofitResponse.error(404, "".toResponseBody())) + } + try { + subject.intercept(chain) + } catch (e: HttpException) { + coVerify(inverse = true) { authenticationProvider.invalidateAuthHeader() } + } + } + + @Test + fun `when unauthorized and had token then calls again with new token`() { + coEvery { authenticationProvider.getAuthHeader() } returns "token" andThen "newToken" + val requests = mutableListOf() + every { + chain.proceed(capture(requests)) + } answers { + throw HttpException(RetrofitResponse.error(401, "".toResponseBody())) + } andThenAnswer { + buildResponse(requests[1]) + } + subject.intercept(chain) + coVerifySequence { + authenticationProvider.getAuthHeader() + authenticationProvider.invalidateAuthHeader() + authenticationProvider.getAuthHeader() + } + assertThat(requests[0].header(AuthInterceptor.HEADER_NAME)).isEqualTo("token") + assertThat(requests[1].header(AuthInterceptor.HEADER_NAME)).isEqualTo("newToken") + } + + @Test + fun `when had token but now does not then removes it`() { + coEvery { authenticationProvider.getAuthHeader() } returns null + val mockRequest = Request.Builder() + .url("http://localhost") + .header(AuthInterceptor.HEADER_NAME, "token") + .build() + every { chain.request() } returns mockRequest + val requestSlot = mockProceedCallAndCaptureRequest() + subject.intercept(chain) + assertThat(requestSlot.captured.header(AuthInterceptor.HEADER_NAME)).isNull() + } + + private fun mockUnauthorized() { + val requestSlot = slot() + every { + chain.proceed(capture(requestSlot)) + } answers { + throw HttpException(RetrofitResponse.error(401, "".toResponseBody())) + } + } + + private fun mockProceedCallAndCaptureRequest(): CapturingSlot { + val requestSlot = slot() + every { + chain.proceed(capture(requestSlot)) + } answers { + buildResponse(requestSlot.captured) + } + return requestSlot + } + + private fun buildResponse( + request: Request, + code: Int = 200, + protocol: Protocol = Protocol.HTTP_2, + message: String = if (code == 200) "OK" else "NOT OK", + ) = Response.Builder() + .code(code) + .request(request) + .protocol(protocol) + .message(message) + .build() +} \ No newline at end of file From ac20105b9bbd364f28ec785f18c00e0321a59366 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sat, 10 Dec 2022 10:32:13 +0100 Subject: [PATCH 04/15] Create OkHttp authenticator without token invalidation --- .../datasource/AuthenticationProvider.kt | 1 - .../mealient/datasource/DataSourceModule.kt | 8 +- .../datasource/impl/AuthInterceptor.kt | 58 ------- .../datasource/impl/MealieAuthenticator.kt | 40 +++++ .../datasource/impl/OkHttpBuilderImpl.kt | 3 + .../datasource/AuthInterceptorTest.kt | 164 ------------------ .../datasource/MealieAuthenticatorTest.kt | 90 ++++++++++ 7 files changed, 136 insertions(+), 228 deletions(-) delete mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt create mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt delete mode 100644 datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/AuthInterceptorTest.kt create mode 100644 datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt index b43f93a..7df0e88 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt @@ -4,5 +4,4 @@ interface AuthenticationProvider { suspend fun getAuthHeader(): String? - suspend fun invalidateAuthHeader() } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt index 73ed120..2ffa990 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt @@ -6,9 +6,8 @@ import dagger.Module import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent -import dagger.multibindings.IntoSet -import gq.kirmanak.mealient.datasource.impl.AuthInterceptor import gq.kirmanak.mealient.datasource.impl.CacheBuilderImpl +import gq.kirmanak.mealient.datasource.impl.MealieAuthenticator import gq.kirmanak.mealient.datasource.impl.NetworkRequestWrapperImpl import gq.kirmanak.mealient.datasource.impl.OkHttpBuilderImpl import gq.kirmanak.mealient.datasource.impl.RetrofitBuilder @@ -20,7 +19,7 @@ import gq.kirmanak.mealient.datasource.v1.MealieDataSourceV1Impl import gq.kirmanak.mealient.datasource.v1.MealieServiceV1 import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.json.Json -import okhttp3.Interceptor +import okhttp3.Authenticator import okhttp3.MediaType.Companion.toMediaType import okhttp3.OkHttpClient import retrofit2.Converter @@ -91,6 +90,5 @@ interface DataSourceModule { @Binds @Singleton - @IntoSet - fun bindAuthInterceptor(authInterceptor: AuthInterceptor): Interceptor + fun bindAuthenticator(mealieAuthenticator: MealieAuthenticator): Authenticator } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt deleted file mode 100644 index 5f2fc71..0000000 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt +++ /dev/null @@ -1,58 +0,0 @@ -package gq.kirmanak.mealient.datasource.impl - -import androidx.annotation.VisibleForTesting -import gq.kirmanak.mealient.datasource.AuthenticationProvider -import kotlinx.coroutines.runBlocking -import okhttp3.Interceptor -import okhttp3.Response -import retrofit2.HttpException -import javax.inject.Inject -import javax.inject.Provider -import javax.inject.Singleton - -@Singleton -class AuthInterceptor @Inject constructor( - private val authenticationProvider: Provider, -) : Interceptor { - - override fun intercept(chain: Interceptor.Chain): Response { - val token = getAuthHeader() - return if (token == null) { - proceedWithAuthHeader(chain) - } else { - try { - proceedWithAuthHeader(chain, token) - } catch (e: HttpException) { - if (e.code() in setOf(401, 403)) { - invalidateAuthHeader() - proceedWithAuthHeader(chain, getAuthHeader()) - } else { - throw e - } - } - } - } - - private fun proceedWithAuthHeader(chain: Interceptor.Chain, token: String? = null): Response { - val requestBuilder = chain.request().newBuilder() - val request = if (token == null) { - requestBuilder.removeHeader(HEADER_NAME).build() - } else { - requestBuilder.header(HEADER_NAME, token).build() - } - return chain.proceed(request) - } - - private fun getAuthHeader() = runBlocking { - authenticationProvider.get().getAuthHeader() - } - - private fun invalidateAuthHeader() = runBlocking { - authenticationProvider.get().invalidateAuthHeader() - } - - companion object { - @VisibleForTesting - const val HEADER_NAME = "Authorization" - } -} diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt new file mode 100644 index 0000000..bf5df70 --- /dev/null +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt @@ -0,0 +1,40 @@ +package gq.kirmanak.mealient.datasource.impl + +import androidx.annotation.VisibleForTesting +import gq.kirmanak.mealient.datasource.AuthenticationProvider +import kotlinx.coroutines.runBlocking +import okhttp3.Authenticator +import okhttp3.Request +import okhttp3.Response +import okhttp3.Route +import javax.inject.Inject +import javax.inject.Provider +import javax.inject.Singleton + +@Singleton +class MealieAuthenticator @Inject constructor( + private val authenticationProvider: Provider, +) : Authenticator { + + override fun authenticate(route: Route?, response: Response): Request? { + val supportsBearer = response.challenges().any { it.scheme == BEARER_SCHEME } + val request = response.request + return if (supportsBearer && request.header(HEADER_NAME) == null) { + getAuthHeader()?.let { request.copyWithHeader(HEADER_NAME, it) } + } else { + null // Either Bearer is not supported or we've already tried to authenticate + } + } + + private fun getAuthHeader() = runBlocking { authenticationProvider.get().getAuthHeader() } + + companion object { + @VisibleForTesting + const val HEADER_NAME = "Authorization" + private const val BEARER_SCHEME = "Bearer" + } +} + +private fun Request.copyWithHeader(name: String, value: String): Request { + return newBuilder().header(name, value).build() +} \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt index 21ca1d1..bb2a9a6 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt @@ -2,6 +2,7 @@ package gq.kirmanak.mealient.datasource.impl import gq.kirmanak.mealient.datasource.CacheBuilder import gq.kirmanak.mealient.datasource.OkHttpBuilder +import okhttp3.Authenticator import okhttp3.Interceptor import okhttp3.OkHttpClient import javax.inject.Inject @@ -12,10 +13,12 @@ class OkHttpBuilderImpl @Inject constructor( private val cacheBuilder: CacheBuilder, // Use @JvmSuppressWildcards because otherwise dagger can't inject it (https://stackoverflow.com/a/43149382) private val interceptors: Set<@JvmSuppressWildcards Interceptor>, + private val authenticator: Authenticator, ) : OkHttpBuilder { override fun buildOkHttp(): OkHttpClient = OkHttpClient.Builder() .apply { interceptors.forEach(::addNetworkInterceptor) } .cache(cacheBuilder.buildCache()) + .authenticator(authenticator) .build() } \ No newline at end of file diff --git a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/AuthInterceptorTest.kt b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/AuthInterceptorTest.kt deleted file mode 100644 index b89c2d8..0000000 --- a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/AuthInterceptorTest.kt +++ /dev/null @@ -1,164 +0,0 @@ -package gq.kirmanak.mealient.datasource - -import com.google.common.truth.Truth.assertThat -import gq.kirmanak.mealient.datasource.impl.AuthInterceptor -import gq.kirmanak.mealient.test.BaseUnitTest -import io.mockk.CapturingSlot -import io.mockk.coEvery -import io.mockk.coVerify -import io.mockk.coVerifySequence -import io.mockk.every -import io.mockk.impl.annotations.MockK -import io.mockk.slot -import io.mockk.verify -import okhttp3.Interceptor -import okhttp3.Protocol -import okhttp3.Request -import okhttp3.Response -import okhttp3.ResponseBody.Companion.toResponseBody -import org.junit.Before -import org.junit.Test -import retrofit2.HttpException -import retrofit2.Response as RetrofitResponse - -class AuthInterceptorTest : BaseUnitTest() { - - private lateinit var subject: AuthInterceptor - - @MockK(relaxUnitFun = true) - lateinit var authenticationProvider: AuthenticationProvider - - @MockK(relaxUnitFun = true) - lateinit var chain: Interceptor.Chain - - @Before - override fun setUp() { - super.setUp() - subject = AuthInterceptor { authenticationProvider } - every { chain.request() } returns Request.Builder().url("http://localhost").build() - } - - @Test - fun `when no header then still proceeds`() { - mockProceedCallAndCaptureRequest() - coEvery { authenticationProvider.getAuthHeader() } returns null - subject.intercept(chain) - verify { chain.proceed(any()) } - } - - @Test - fun `when has header then adds header`() { - coEvery { authenticationProvider.getAuthHeader() } returns "token" - val requestSlot = mockProceedCallAndCaptureRequest() - subject.intercept(chain) - assertThat(requestSlot.captured.header(AuthInterceptor.HEADER_NAME)).isEqualTo("token") - } - - @Test(expected = HttpException::class) - fun `when unauthorized but didn't have token then throws`() { - coEvery { authenticationProvider.getAuthHeader() } returns null - mockUnauthorized() - subject.intercept(chain) - } - - @Test(expected = HttpException::class) - fun `when unauthorized and had token then invalidates it`() { - coEvery { authenticationProvider.getAuthHeader() } returns "token" - mockUnauthorized() - subject.intercept(chain) - coVerify { authenticationProvider.invalidateAuthHeader() } - } - - @Test(expected = HttpException::class) - fun `when not found and had token then throws exception`() { - coEvery { authenticationProvider.getAuthHeader() } returns "token" - val requestSlot = slot() - every { - chain.proceed(capture(requestSlot)) - } answers { - throw HttpException(RetrofitResponse.error(404, "".toResponseBody())) - } - subject.intercept(chain) - } - - @Test - fun `when not found and had token then does not invalidate it`() { - coEvery { authenticationProvider.getAuthHeader() } returns "token" - val requestSlot = slot() - every { - chain.proceed(capture(requestSlot)) - } answers { - throw HttpException(RetrofitResponse.error(404, "".toResponseBody())) - } - try { - subject.intercept(chain) - } catch (e: HttpException) { - coVerify(inverse = true) { authenticationProvider.invalidateAuthHeader() } - } - } - - @Test - fun `when unauthorized and had token then calls again with new token`() { - coEvery { authenticationProvider.getAuthHeader() } returns "token" andThen "newToken" - val requests = mutableListOf() - every { - chain.proceed(capture(requests)) - } answers { - throw HttpException(RetrofitResponse.error(401, "".toResponseBody())) - } andThenAnswer { - buildResponse(requests[1]) - } - subject.intercept(chain) - coVerifySequence { - authenticationProvider.getAuthHeader() - authenticationProvider.invalidateAuthHeader() - authenticationProvider.getAuthHeader() - } - assertThat(requests[0].header(AuthInterceptor.HEADER_NAME)).isEqualTo("token") - assertThat(requests[1].header(AuthInterceptor.HEADER_NAME)).isEqualTo("newToken") - } - - @Test - fun `when had token but now does not then removes it`() { - coEvery { authenticationProvider.getAuthHeader() } returns null - val mockRequest = Request.Builder() - .url("http://localhost") - .header(AuthInterceptor.HEADER_NAME, "token") - .build() - every { chain.request() } returns mockRequest - val requestSlot = mockProceedCallAndCaptureRequest() - subject.intercept(chain) - assertThat(requestSlot.captured.header(AuthInterceptor.HEADER_NAME)).isNull() - } - - private fun mockUnauthorized() { - val requestSlot = slot() - every { - chain.proceed(capture(requestSlot)) - } answers { - throw HttpException(RetrofitResponse.error(401, "".toResponseBody())) - } - } - - private fun mockProceedCallAndCaptureRequest(): CapturingSlot { - val requestSlot = slot() - every { - chain.proceed(capture(requestSlot)) - } answers { - buildResponse(requestSlot.captured) - } - return requestSlot - } - - private fun buildResponse( - request: Request, - code: Int = 200, - protocol: Protocol = Protocol.HTTP_2, - message: String = if (code == 200) "OK" else "NOT OK", - ) = Response.Builder() - .code(code) - .request(request) - .protocol(protocol) - .message(message) - .build() -} \ No newline at end of file diff --git a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt new file mode 100644 index 0000000..41d8714 --- /dev/null +++ b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt @@ -0,0 +1,90 @@ +package gq.kirmanak.mealient.datasource + +import com.google.common.truth.Truth.assertThat +import gq.kirmanak.mealient.datasource.impl.MealieAuthenticator +import gq.kirmanak.mealient.datasource.impl.MealieAuthenticator.Companion.HEADER_NAME +import gq.kirmanak.mealient.test.BaseUnitTest +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.impl.annotations.MockK +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import org.junit.Before +import org.junit.Test + +class MealieAuthenticatorTest : BaseUnitTest() { + + private lateinit var subject: MealieAuthenticator + + @MockK + lateinit var authenticationProvider: AuthenticationProvider + + @Before + override fun setUp() { + super.setUp() + subject = MealieAuthenticator { authenticationProvider } + } + + @Test + fun `when bearer is not supported expect authenticate to return null`() { + val response = buildResponse(challenges = null) + assertThat(subject.authenticate(null, response)).isNull() + } + + @Test + fun `when no auth header exists expect authenticate to return null`() { + coEvery { authenticationProvider.getAuthHeader() } returns null + val response = buildResponse() + assertThat(subject.authenticate(null, response)).isNull() + } + + @Test + fun `when no auth header exists expect authenticate to call provider`() { + coEvery { authenticationProvider.getAuthHeader() } returns null + val response = buildResponse() + subject.authenticate(null, response) + coVerify { authenticationProvider.getAuthHeader() } + } + + @Test + fun `when no auth header was set expect authenticate to return null`() { + val response = buildResponse(authHeader = "token") + assertThat(subject.authenticate(null, response)).isNull() + } + + @Test + fun `when auth header exists expect authenticate to return request`() { + coEvery { authenticationProvider.getAuthHeader() } returns "token" + val response = buildResponse() + val actualHeader = subject.authenticate(null, response)?.header(HEADER_NAME) + assertThat(actualHeader).isEqualTo("token") + } + + private fun buildResponse( + url: String = "http://localhost", + code: Int = 401, + message: String = "Unauthorized", + protocol: Protocol = Protocol.HTTP_2, + challenges: String? = "Bearer", + authHeader: String? = null, + ): Response { + val request = buildRequest(authHeader, url) + return Response.Builder().apply { + request(request) + code(code) + message(message) + protocol(protocol) + if (challenges != null) header("WWW-Authenticate", challenges) + }.build() + } + + private fun buildRequest( + authHeader: String? = null, + url: String = "http://localhost", + ): Request = Request.Builder().apply { + url(url) + if (authHeader != null) header(HEADER_NAME, authHeader) + }.build() + +} \ No newline at end of file From 9c48f1b5630994e9a1f94a3d8ee7c083e8300c4e Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 10:17:13 +0100 Subject: [PATCH 05/15] Support invalidation in MealieAuthenticator --- .../datasource/AuthenticationProvider.kt | 1 + .../datasource/impl/MealieAuthenticator.kt | 18 ++++++++++--- .../datasource/MealieAuthenticatorTest.kt | 27 +++++++++++++++++-- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt index 7df0e88..b43f93a 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt @@ -4,4 +4,5 @@ interface AuthenticationProvider { suspend fun getAuthHeader(): String? + suspend fun invalidateAuthHeader() } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt index bf5df70..96ca52f 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt @@ -18,16 +18,26 @@ class MealieAuthenticator @Inject constructor( override fun authenticate(route: Route?, response: Response): Request? { val supportsBearer = response.challenges().any { it.scheme == BEARER_SCHEME } + if (!supportsBearer) { + return null + } + val request = response.request - return if (supportsBearer && request.header(HEADER_NAME) == null) { - getAuthHeader()?.let { request.copyWithHeader(HEADER_NAME, it) } - } else { - null // Either Bearer is not supported or we've already tried to authenticate + val previousHeader = request.header(HEADER_NAME) + ?: return getAuthHeader()?.let { request.copyWithHeader(HEADER_NAME, it) } + + invalidateAuthHeader() + return getAuthHeader()?.takeUnless { it == previousHeader }?.let { + request.copyWithHeader(HEADER_NAME, it) } } private fun getAuthHeader() = runBlocking { authenticationProvider.get().getAuthHeader() } + private fun invalidateAuthHeader() { + runBlocking { authenticationProvider.get().invalidateAuthHeader() } + } + companion object { @VisibleForTesting const val HEADER_NAME = "Authorization" diff --git a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt index 41d8714..a9949c0 100644 --- a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt +++ b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt @@ -17,7 +17,7 @@ class MealieAuthenticatorTest : BaseUnitTest() { private lateinit var subject: MealieAuthenticator - @MockK + @MockK(relaxUnitFun = true) lateinit var authenticationProvider: AuthenticationProvider @Before @@ -48,11 +48,34 @@ class MealieAuthenticatorTest : BaseUnitTest() { } @Test - fun `when no auth header was set expect authenticate to return null`() { + fun `when had auth header but can't invalidate expect authenticate return null`() { + coEvery { authenticationProvider.getAuthHeader() } returns null val response = buildResponse(authHeader = "token") assertThat(subject.authenticate(null, response)).isNull() } + @Test + fun `when had auth header and invalidate doesn't change it expect authenticate return null`() { + coEvery { authenticationProvider.getAuthHeader() } returns "token" + val response = buildResponse(authHeader = "token") + assertThat(subject.authenticate(null, response)).isNull() + } + + @Test + fun `when had auth header and invalidate succeeds expect authenticate return new`() { + coEvery { authenticationProvider.getAuthHeader() } returns "newToken" + val response = buildResponse(authHeader = "token") + assertThat(subject.authenticate(null, response)?.header(HEADER_NAME)).isEqualTo("newToken") + } + + @Test + fun `when had auth header expect authenticate to invalidate it`() { + coEvery { authenticationProvider.getAuthHeader() } returns null + val response = buildResponse(authHeader = "token") + subject.authenticate(null, response) + coVerify { authenticationProvider.invalidateAuthHeader() } + } + @Test fun `when auth header exists expect authenticate to return request`() { coEvery { authenticationProvider.getAuthHeader() } returns "token" From 79b857810a1fe287718227a0dbf9076dbd252e5f Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 10:24:35 +0100 Subject: [PATCH 06/15] Remove makeCall helper fun --- .../data/network/MealieDataSourceWrapper.kt | 65 +++++++------------ 1 file changed, 24 insertions(+), 41 deletions(-) diff --git a/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt b/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt index 51be915..3f48597 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/network/MealieDataSourceWrapper.kt @@ -27,60 +27,43 @@ class MealieDataSourceWrapper @Inject constructor( private val v1Source: MealieDataSourceV1, ) : AddRecipeDataSource, RecipeDataSource, ParseRecipeDataSource { - override suspend fun addRecipe( - recipe: AddRecipeInfo, - ): String = makeCall { url, version -> - when (version) { - ServerVersion.V0 -> v0Source.addRecipe(url, recipe.toV0Request()) - ServerVersion.V1 -> { - val slug = v1Source.createRecipe(url, recipe.toV1CreateRequest()) - v1Source.updateRecipe(url, slug, recipe.toV1UpdateRequest()) - slug - } + private suspend fun getVersion(): ServerVersion = serverInfoRepo.getVersion() + + private suspend fun getUrl(): String = serverInfoRepo.requireUrl() + + override suspend fun addRecipe(recipe: AddRecipeInfo): String = when (getVersion()) { + ServerVersion.V0 -> v0Source.addRecipe(getUrl(), recipe.toV0Request()) + ServerVersion.V1 -> { + val slug = v1Source.createRecipe(getUrl(), recipe.toV1CreateRequest()) + v1Source.updateRecipe(getUrl(), slug, recipe.toV1UpdateRequest()) + slug } } override suspend fun requestRecipes( start: Int, limit: Int, - ): List = makeCall { url, version -> - when (version) { - ServerVersion.V0 -> { - v0Source.requestRecipes(url, start, limit).map { it.toRecipeSummaryInfo() } - } - ServerVersion.V1 -> { - // Imagine start is 30 and limit is 15. It means that we already have page 1 and 2, now we need page 3 - val page = start / limit + 1 - v1Source.requestRecipes(url, page, limit).map { it.toRecipeSummaryInfo() } - } + ): List = when (getVersion()) { + ServerVersion.V0 -> { + v0Source.requestRecipes(getUrl(), start, limit).map { it.toRecipeSummaryInfo() } + } + ServerVersion.V1 -> { + // Imagine start is 30 and limit is 15. It means that we already have page 1 and 2, now we need page 3 + val page = start / limit + 1 + v1Source.requestRecipes(getUrl(), page, limit).map { it.toRecipeSummaryInfo() } } } - override suspend fun requestRecipeInfo( - slug: String, - ): FullRecipeInfo = makeCall { url, version -> - when (version) { - ServerVersion.V0 -> v0Source.requestRecipeInfo(url, slug).toFullRecipeInfo() - ServerVersion.V1 -> v1Source.requestRecipeInfo(url, slug).toFullRecipeInfo() - } + override suspend fun requestRecipeInfo(slug: String): FullRecipeInfo = when (getVersion()) { + ServerVersion.V0 -> v0Source.requestRecipeInfo(getUrl(), slug).toFullRecipeInfo() + ServerVersion.V1 -> v1Source.requestRecipeInfo(getUrl(), slug).toFullRecipeInfo() } override suspend fun parseRecipeFromURL( parseRecipeURLInfo: ParseRecipeURLInfo, - ): String = makeCall { url, version -> - when (version) { - ServerVersion.V0 -> { - v0Source.parseRecipeFromURL(url, parseRecipeURLInfo.toV0Request()) - } - ServerVersion.V1 -> { - v1Source.parseRecipeFromURL(url, parseRecipeURLInfo.toV1Request()) - } - } + ): String = when (getVersion()) { + ServerVersion.V0 -> v0Source.parseRecipeFromURL(getUrl(), parseRecipeURLInfo.toV0Request()) + ServerVersion.V1 -> v1Source.parseRecipeFromURL(getUrl(), parseRecipeURLInfo.toV1Request()) } - private suspend inline fun makeCall(block: (String, ServerVersion) -> T): T { - val url = serverInfoRepo.requireUrl() - val version = serverInfoRepo.getVersion() - return block(url, version) - } } \ No newline at end of file From a560db8bb61df13425a811d4a0076e7daa57fc98 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 11:29:50 +0100 Subject: [PATCH 07/15] Revert "Support invalidation in MealieAuthenticator" This reverts commit 9c48f1b5630994e9a1f94a3d8ee7c083e8300c4e. --- .../datasource/AuthenticationProvider.kt | 1 - .../datasource/impl/MealieAuthenticator.kt | 18 +++---------- .../datasource/MealieAuthenticatorTest.kt | 27 ++----------------- 3 files changed, 6 insertions(+), 40 deletions(-) diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt index b43f93a..7df0e88 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt @@ -4,5 +4,4 @@ interface AuthenticationProvider { suspend fun getAuthHeader(): String? - suspend fun invalidateAuthHeader() } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt index 96ca52f..bf5df70 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt @@ -18,26 +18,16 @@ class MealieAuthenticator @Inject constructor( override fun authenticate(route: Route?, response: Response): Request? { val supportsBearer = response.challenges().any { it.scheme == BEARER_SCHEME } - if (!supportsBearer) { - return null - } - val request = response.request - val previousHeader = request.header(HEADER_NAME) - ?: return getAuthHeader()?.let { request.copyWithHeader(HEADER_NAME, it) } - - invalidateAuthHeader() - return getAuthHeader()?.takeUnless { it == previousHeader }?.let { - request.copyWithHeader(HEADER_NAME, it) + return if (supportsBearer && request.header(HEADER_NAME) == null) { + getAuthHeader()?.let { request.copyWithHeader(HEADER_NAME, it) } + } else { + null // Either Bearer is not supported or we've already tried to authenticate } } private fun getAuthHeader() = runBlocking { authenticationProvider.get().getAuthHeader() } - private fun invalidateAuthHeader() { - runBlocking { authenticationProvider.get().invalidateAuthHeader() } - } - companion object { @VisibleForTesting const val HEADER_NAME = "Authorization" diff --git a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt index a9949c0..41d8714 100644 --- a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt +++ b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt @@ -17,7 +17,7 @@ class MealieAuthenticatorTest : BaseUnitTest() { private lateinit var subject: MealieAuthenticator - @MockK(relaxUnitFun = true) + @MockK lateinit var authenticationProvider: AuthenticationProvider @Before @@ -48,34 +48,11 @@ class MealieAuthenticatorTest : BaseUnitTest() { } @Test - fun `when had auth header but can't invalidate expect authenticate return null`() { - coEvery { authenticationProvider.getAuthHeader() } returns null + fun `when no auth header was set expect authenticate to return null`() { val response = buildResponse(authHeader = "token") assertThat(subject.authenticate(null, response)).isNull() } - @Test - fun `when had auth header and invalidate doesn't change it expect authenticate return null`() { - coEvery { authenticationProvider.getAuthHeader() } returns "token" - val response = buildResponse(authHeader = "token") - assertThat(subject.authenticate(null, response)).isNull() - } - - @Test - fun `when had auth header and invalidate succeeds expect authenticate return new`() { - coEvery { authenticationProvider.getAuthHeader() } returns "newToken" - val response = buildResponse(authHeader = "token") - assertThat(subject.authenticate(null, response)?.header(HEADER_NAME)).isEqualTo("newToken") - } - - @Test - fun `when had auth header expect authenticate to invalidate it`() { - coEvery { authenticationProvider.getAuthHeader() } returns null - val response = buildResponse(authHeader = "token") - subject.authenticate(null, response) - coVerify { authenticationProvider.invalidateAuthHeader() } - } - @Test fun `when auth header exists expect authenticate to return request`() { coEvery { authenticationProvider.getAuthHeader() } returns "token" From c99f9fea881c60fe2d3c54386d57fa8343264ec8 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 11:57:13 +0100 Subject: [PATCH 08/15] Create API token when signing in --- .../mealient/data/auth/AuthDataSource.kt | 2 + .../kirmanak/mealient/data/auth/AuthRepo.kt | 4 -- .../mealient/data/auth/AuthStorage.kt | 8 --- .../data/auth/impl/AuthDataSourceImpl.kt | 20 +++++-- .../mealient/data/auth/impl/AuthRepoImpl.kt | 23 ++------ .../data/auth/impl/AuthStorageImpl.kt | 14 ----- .../data/auth/impl/AuthRepoImplTest.kt | 57 ++++--------------- .../data/auth/impl/AuthStorageImplTest.kt | 16 ------ .../mealient/test/AuthImplTestData.kt | 2 + .../datasource/AuthenticationProvider.kt | 2 + .../datasource/impl/MealieAuthenticator.kt | 21 +++++-- .../datasource/v0/MealieDataSourceV0.kt | 6 ++ .../datasource/v0/MealieDataSourceV0Impl.kt | 11 +++- .../mealient/datasource/v0/MealieServiceV0.kt | 6 ++ .../v0/models/CreateApiTokenRequestV0.kt | 9 +++ .../datasource/v1/MealieDataSourceV1.kt | 7 +++ .../datasource/v1/MealieDataSourceV1Impl.kt | 11 +++- .../mealient/datasource/v1/MealieServiceV1.kt | 6 ++ .../v1/models/CreateApiTokenRequestV1.kt | 9 +++ .../v1/models/CreateApiTokenResponseV1.kt | 9 +++ .../datasource/MealieAuthenticatorTest.kt | 11 +++- 21 files changed, 130 insertions(+), 124 deletions(-) create mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/models/CreateApiTokenRequestV0.kt create mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/models/CreateApiTokenRequestV1.kt create mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/models/CreateApiTokenResponseV1.kt diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthDataSource.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthDataSource.kt index 576ccab..8ea9b20 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthDataSource.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthDataSource.kt @@ -5,4 +5,6 @@ interface AuthDataSource { * Tries to acquire authentication token using the provided credentials */ suspend fun authenticate(username: String, password: String): String + + suspend fun createApiToken(name: String): String } \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthRepo.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthRepo.kt index 4e2b660..c07f800 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthRepo.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthRepo.kt @@ -10,9 +10,5 @@ interface AuthRepo { suspend fun getAuthHeader(): String? - suspend fun requireAuthHeader(): String - suspend fun logout() - - suspend fun invalidateAuthHeader() } \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthStorage.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthStorage.kt index 8486e6a..faddce7 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthStorage.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/AuthStorage.kt @@ -9,12 +9,4 @@ interface AuthStorage { suspend fun setAuthHeader(authHeader: String?) suspend fun getAuthHeader(): String? - - suspend fun setEmail(email: String?) - - suspend fun getEmail(): String? - - suspend fun setPassword(password: String?) - - suspend fun getPassword(): String? } \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthDataSourceImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthDataSourceImpl.kt index f42444b..8834372 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthDataSourceImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthDataSourceImpl.kt @@ -4,7 +4,9 @@ import gq.kirmanak.mealient.data.auth.AuthDataSource import gq.kirmanak.mealient.data.baseurl.ServerInfoRepo import gq.kirmanak.mealient.data.baseurl.ServerVersion import gq.kirmanak.mealient.datasource.v0.MealieDataSourceV0 +import gq.kirmanak.mealient.datasource.v0.models.CreateApiTokenRequestV0 import gq.kirmanak.mealient.datasource.v1.MealieDataSourceV1 +import gq.kirmanak.mealient.datasource.v1.models.CreateApiTokenRequestV1 import javax.inject.Inject import javax.inject.Singleton @@ -15,14 +17,20 @@ class AuthDataSourceImpl @Inject constructor( private val v1Source: MealieDataSourceV1, ) : AuthDataSource { + private suspend fun getVersion(): ServerVersion = serverInfoRepo.getVersion() + + private suspend fun getUrl(): String = serverInfoRepo.requireUrl() + override suspend fun authenticate( username: String, password: String, - ): String { - val baseUrl = serverInfoRepo.requireUrl() - return when (serverInfoRepo.getVersion()) { - ServerVersion.V0 -> v0Source.authenticate(baseUrl, username, password) - ServerVersion.V1 -> v1Source.authenticate(baseUrl, username, password) - } + ): String = when (getVersion()) { + ServerVersion.V0 -> v0Source.authenticate(getUrl(), username, password) + ServerVersion.V1 -> v1Source.authenticate(getUrl(), username, password) + } + + override suspend fun createApiToken(name: String): String = when (getVersion()) { + ServerVersion.V0 -> v0Source.createApiToken(getUrl(), CreateApiTokenRequestV0(name)) + ServerVersion.V1 -> v1Source.createApiToken(getUrl(), CreateApiTokenRequestV1(name)).token } } \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt index 531c0b5..cb241cb 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImpl.kt @@ -4,7 +4,6 @@ import gq.kirmanak.mealient.data.auth.AuthDataSource import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.auth.AuthStorage import gq.kirmanak.mealient.datasource.AuthenticationProvider -import gq.kirmanak.mealient.datasource.runCatchingExceptCancel import gq.kirmanak.mealient.logging.Logger import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map @@ -24,34 +23,20 @@ class AuthRepoImpl @Inject constructor( override suspend fun authenticate(email: String, password: String) { logger.v { "authenticate() called with: email = $email, password = $password" } val token = authDataSource.authenticate(email, password) - val header = AUTH_HEADER_FORMAT.format(token) - authStorage.setAuthHeader(header) - authStorage.setEmail(email) - authStorage.setPassword(password) + authStorage.setAuthHeader(AUTH_HEADER_FORMAT.format(token)) + val apiToken = authDataSource.createApiToken(API_TOKEN_NAME) + authStorage.setAuthHeader(AUTH_HEADER_FORMAT.format(apiToken)) } override suspend fun getAuthHeader(): String? = authStorage.getAuthHeader() - override suspend fun requireAuthHeader(): String = checkNotNull(getAuthHeader()) { - "Auth header is null when it was required" - } - override suspend fun logout() { logger.v { "logout() called" } - authStorage.setEmail(null) - authStorage.setPassword(null) authStorage.setAuthHeader(null) } - override suspend fun invalidateAuthHeader() { - logger.v { "invalidateAuthHeader() called" } - val email = authStorage.getEmail() ?: return - val password = authStorage.getPassword() ?: return - runCatchingExceptCancel { authenticate(email, password) } - .onFailure { logout() } // Clear all known values to avoid reusing them - } - companion object { private const val AUTH_HEADER_FORMAT = "Bearer %s" + private const val API_TOKEN_NAME = "Mealient" } } \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImpl.kt index aa83e67..820e25f 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImpl.kt @@ -32,14 +32,6 @@ class AuthStorageImpl @Inject constructor( override suspend fun getAuthHeader(): String? = getString(AUTH_HEADER_KEY) - override suspend fun setEmail(email: String?) = putString(EMAIL_KEY, email) - - override suspend fun getEmail(): String? = getString(EMAIL_KEY) - - override suspend fun setPassword(password: String?) = putString(PASSWORD_KEY, password) - - override suspend fun getPassword(): String? = getString(PASSWORD_KEY) - private suspend fun putString( key: String, value: String? @@ -57,11 +49,5 @@ class AuthStorageImpl @Inject constructor( companion object { @VisibleForTesting const val AUTH_HEADER_KEY = "authHeader" - - @VisibleForTesting - const val EMAIL_KEY = "email" - - @VisibleForTesting - const val PASSWORD_KEY = "password" } } \ No newline at end of file diff --git a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt index db98128..fa38ed2 100644 --- a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthRepoImplTest.kt @@ -6,6 +6,8 @@ import gq.kirmanak.mealient.data.auth.AuthRepo import gq.kirmanak.mealient.data.auth.AuthStorage import gq.kirmanak.mealient.data.baseurl.ServerInfoRepo import gq.kirmanak.mealient.datasource.runCatchingExceptCancel +import gq.kirmanak.mealient.test.AuthImplTestData.TEST_API_AUTH_HEADER +import gq.kirmanak.mealient.test.AuthImplTestData.TEST_API_TOKEN import gq.kirmanak.mealient.test.AuthImplTestData.TEST_AUTH_HEADER import gq.kirmanak.mealient.test.AuthImplTestData.TEST_BASE_URL import gq.kirmanak.mealient.test.AuthImplTestData.TEST_PASSWORD @@ -51,13 +53,15 @@ class AuthRepoImplTest : BaseUnitTest() { @Test fun `when authenticate successfully then saves to storage`() = runTest { coEvery { serverInfoRepo.getVersion() } returns TEST_SERVER_VERSION_V0 - coEvery { dataSource.authenticate(eq(TEST_USERNAME), eq(TEST_PASSWORD)) } returns TEST_TOKEN + coEvery { dataSource.authenticate(any(), any()) } returns TEST_TOKEN coEvery { serverInfoRepo.requireUrl() } returns TEST_BASE_URL + coEvery { dataSource.createApiToken(any()) } returns TEST_API_TOKEN subject.authenticate(TEST_USERNAME, TEST_PASSWORD) - coVerifyAll { + coVerify { + dataSource.authenticate(eq(TEST_USERNAME), eq(TEST_PASSWORD)) storage.setAuthHeader(TEST_AUTH_HEADER) - storage.setEmail(TEST_USERNAME) - storage.setPassword(TEST_PASSWORD) + dataSource.createApiToken(eq("Mealient")) + storage.setAuthHeader(TEST_API_AUTH_HEADER) } confirmVerified(storage) } @@ -71,50 +75,9 @@ class AuthRepoImplTest : BaseUnitTest() { } @Test - fun `when logout then removes email, password and header`() = runTest { + fun `when logout expect header removal`() = runTest { subject.logout() - coVerifyAll { - storage.setEmail(null) - storage.setPassword(null) - storage.setAuthHeader(null) - } + coVerify { storage.setAuthHeader(null) } confirmVerified(storage) } - - @Test - fun `when invalidate then does not authenticate without email`() = runTest { - coEvery { storage.getEmail() } returns null - coEvery { storage.getPassword() } returns TEST_PASSWORD - subject.invalidateAuthHeader() - confirmVerified(dataSource) - } - - @Test - fun `when invalidate then does not authenticate without password`() = runTest { - coEvery { storage.getEmail() } returns TEST_USERNAME - coEvery { storage.getPassword() } returns null - subject.invalidateAuthHeader() - confirmVerified(dataSource) - } - - @Test - fun `when invalidate with credentials then calls authenticate`() = runTest { - coEvery { storage.getEmail() } returns TEST_USERNAME - coEvery { storage.getPassword() } returns TEST_PASSWORD - coEvery { serverInfoRepo.getVersion() } returns TEST_SERVER_VERSION_V0 - coEvery { serverInfoRepo.requireUrl() } returns TEST_BASE_URL - coEvery { dataSource.authenticate(eq(TEST_USERNAME), eq(TEST_PASSWORD)) } returns TEST_TOKEN - subject.invalidateAuthHeader() - coVerifyAll { dataSource.authenticate(eq(TEST_USERNAME), eq(TEST_PASSWORD)) } - } - - @Test - fun `when invalidate with credentials and auth fails then clears email`() = runTest { - coEvery { storage.getEmail() } returns "invalid" - coEvery { storage.getPassword() } returns "" - coEvery { serverInfoRepo.requireUrl() } returns TEST_BASE_URL - coEvery { dataSource.authenticate(any(), any()) } throws RuntimeException() - subject.invalidateAuthHeader() - coVerify { storage.setEmail(null) } - } } \ No newline at end of file diff --git a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt index ed8f816..5b0eab1 100644 --- a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt @@ -8,12 +8,8 @@ import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.testing.HiltAndroidTest import gq.kirmanak.mealient.data.auth.AuthStorage import gq.kirmanak.mealient.data.auth.impl.AuthStorageImpl.Companion.AUTH_HEADER_KEY -import gq.kirmanak.mealient.data.auth.impl.AuthStorageImpl.Companion.EMAIL_KEY -import gq.kirmanak.mealient.data.auth.impl.AuthStorageImpl.Companion.PASSWORD_KEY import gq.kirmanak.mealient.logging.Logger import gq.kirmanak.mealient.test.AuthImplTestData.TEST_AUTH_HEADER -import gq.kirmanak.mealient.test.AuthImplTestData.TEST_PASSWORD -import gq.kirmanak.mealient.test.AuthImplTestData.TEST_USERNAME import gq.kirmanak.mealient.test.FakeLogger import gq.kirmanak.mealient.test.HiltRobolectricTest import io.mockk.MockKAnnotations @@ -55,16 +51,4 @@ class AuthStorageImplTest : HiltRobolectricTest() { fun `when authHeader is observed then sends null if nothing saved`() = runTest { assertThat(subject.authHeaderFlow.first()).isEqualTo(null) } - - @Test - fun `when setEmail then edits shared preferences`() = runTest { - subject.setEmail(TEST_USERNAME) - assertThat(sharedPreferences.getString(EMAIL_KEY, null)).isEqualTo(TEST_USERNAME) - } - - @Test - fun `when getPassword then reads shared preferences`() = runTest { - sharedPreferences.edit(commit = true) { putString(PASSWORD_KEY, TEST_PASSWORD) } - assertThat(subject.getPassword()).isEqualTo(TEST_PASSWORD) - } } \ No newline at end of file diff --git a/app/src/test/java/gq/kirmanak/mealient/test/AuthImplTestData.kt b/app/src/test/java/gq/kirmanak/mealient/test/AuthImplTestData.kt index 3e6da0d..a613a84 100644 --- a/app/src/test/java/gq/kirmanak/mealient/test/AuthImplTestData.kt +++ b/app/src/test/java/gq/kirmanak/mealient/test/AuthImplTestData.kt @@ -8,6 +8,8 @@ object AuthImplTestData { const val TEST_BASE_URL = "https://example.com/" const val TEST_TOKEN = "TEST_TOKEN" const val TEST_AUTH_HEADER = "Bearer TEST_TOKEN" + const val TEST_API_TOKEN = "TEST_API_TOKEN" + const val TEST_API_AUTH_HEADER = "Bearer TEST_API_TOKEN" const val TEST_VERSION = "v0.5.6" val TEST_SERVER_VERSION_V0 = ServerVersion.V0 val TEST_SERVER_VERSION_V1 = ServerVersion.V1 diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt index 7df0e88..e8d17a8 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/AuthenticationProvider.kt @@ -4,4 +4,6 @@ interface AuthenticationProvider { suspend fun getAuthHeader(): String? + suspend fun logout() + } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt index bf5df70..5fe4cf5 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt @@ -12,21 +12,30 @@ import javax.inject.Provider import javax.inject.Singleton @Singleton +// TODO has to be interceptor, otherwise only public recipes are visible class MealieAuthenticator @Inject constructor( - private val authenticationProvider: Provider, + private val authenticationProviderProvider: Provider, ) : Authenticator { + private val authenticationProvider: AuthenticationProvider + get() = authenticationProviderProvider.get() + override fun authenticate(route: Route?, response: Response): Request? { val supportsBearer = response.challenges().any { it.scheme == BEARER_SCHEME } val request = response.request - return if (supportsBearer && request.header(HEADER_NAME) == null) { - getAuthHeader()?.let { request.copyWithHeader(HEADER_NAME, it) } - } else { - null // Either Bearer is not supported or we've already tried to authenticate + return when { + request.header(HEADER_NAME) != null -> { + logout() + null + } + supportsBearer -> getAuthHeader()?.let { request.copyWithHeader(HEADER_NAME, it) } + else -> null } } - private fun getAuthHeader() = runBlocking { authenticationProvider.get().getAuthHeader() } + private fun getAuthHeader() = runBlocking { authenticationProvider.getAuthHeader() } + + private fun logout() = runBlocking { authenticationProvider.logout() } companion object { @VisibleForTesting diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0.kt index 6a6abd2..92b70f6 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0.kt @@ -1,6 +1,7 @@ package gq.kirmanak.mealient.datasource.v0 import gq.kirmanak.mealient.datasource.v0.models.AddRecipeRequestV0 +import gq.kirmanak.mealient.datasource.v0.models.CreateApiTokenRequestV0 import gq.kirmanak.mealient.datasource.v0.models.GetRecipeResponseV0 import gq.kirmanak.mealient.datasource.v0.models.GetRecipeSummaryResponseV0 import gq.kirmanak.mealient.datasource.v0.models.ParseRecipeURLRequestV0 @@ -41,4 +42,9 @@ interface MealieDataSourceV0 { baseUrl: String, request: ParseRecipeURLRequestV0, ): String + + suspend fun createApiToken( + baseUrl: String, + request: CreateApiTokenRequestV0, + ): String } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0Impl.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0Impl.kt index f6939d7..473a3f4 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0Impl.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieDataSourceV0Impl.kt @@ -4,6 +4,7 @@ import gq.kirmanak.mealient.datasource.NetworkError import gq.kirmanak.mealient.datasource.NetworkRequestWrapper import gq.kirmanak.mealient.datasource.decode import gq.kirmanak.mealient.datasource.v0.models.AddRecipeRequestV0 +import gq.kirmanak.mealient.datasource.v0.models.CreateApiTokenRequestV0 import gq.kirmanak.mealient.datasource.v0.models.ErrorDetailV0 import gq.kirmanak.mealient.datasource.v0.models.GetRecipeResponseV0 import gq.kirmanak.mealient.datasource.v0.models.GetRecipeSummaryResponseV0 @@ -86,7 +87,15 @@ class MealieDataSourceV0Impl @Inject constructor( ): String = networkRequestWrapper.makeCallAndHandleUnauthorized( block = { service.createRecipeFromURL("$baseUrl/api/recipes/create-url", request) }, logMethod = { "parseRecipeFromURL" }, - logParameters = { "baseUrl = $baseUrl, request = $request" } + logParameters = { "baseUrl = $baseUrl, request = $request" }, + ) + override suspend fun createApiToken( + baseUrl: String, + request: CreateApiTokenRequestV0, + ): String = networkRequestWrapper.makeCallAndHandleUnauthorized( + block = { service.createApiToken("$baseUrl/api/users/api-tokens", request) }, + logMethod = { "createApiToken" }, + logParameters = { "baseUrl = $baseUrl, request = $request" } ) } diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieServiceV0.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieServiceV0.kt index 579dada..79b8ef7 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieServiceV0.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/MealieServiceV0.kt @@ -41,4 +41,10 @@ interface MealieServiceV0 { @Url url: String, @Body request: ParseRecipeURLRequestV0, ): String + + @POST + suspend fun createApiToken( + @Url url: String, + @Body request: CreateApiTokenRequestV0, + ): String } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/models/CreateApiTokenRequestV0.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/models/CreateApiTokenRequestV0.kt new file mode 100644 index 0000000..67315ec --- /dev/null +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v0/models/CreateApiTokenRequestV0.kt @@ -0,0 +1,9 @@ +package gq.kirmanak.mealient.datasource.v0.models + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +@Serializable +data class CreateApiTokenRequestV0( + @SerialName("name") val name: String, +) diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1.kt index 9b6f665..5bc2b02 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1.kt @@ -1,5 +1,7 @@ package gq.kirmanak.mealient.datasource.v1 +import gq.kirmanak.mealient.datasource.v1.models.CreateApiTokenRequestV1 +import gq.kirmanak.mealient.datasource.v1.models.CreateApiTokenResponseV1 import gq.kirmanak.mealient.datasource.v1.models.CreateRecipeRequestV1 import gq.kirmanak.mealient.datasource.v1.models.GetRecipeResponseV1 import gq.kirmanak.mealient.datasource.v1.models.GetRecipeSummaryResponseV1 @@ -48,4 +50,9 @@ interface MealieDataSourceV1 { baseUrl: String, request: ParseRecipeURLRequestV1, ): String + + suspend fun createApiToken( + baseUrl: String, + request: CreateApiTokenRequestV1, + ): CreateApiTokenResponseV1 } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1Impl.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1Impl.kt index 7bb2822..aa009d8 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1Impl.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieDataSourceV1Impl.kt @@ -3,6 +3,8 @@ package gq.kirmanak.mealient.datasource.v1 import gq.kirmanak.mealient.datasource.NetworkError import gq.kirmanak.mealient.datasource.NetworkRequestWrapper import gq.kirmanak.mealient.datasource.decode +import gq.kirmanak.mealient.datasource.v1.models.CreateApiTokenRequestV1 +import gq.kirmanak.mealient.datasource.v1.models.CreateApiTokenResponseV1 import gq.kirmanak.mealient.datasource.v1.models.CreateRecipeRequestV1 import gq.kirmanak.mealient.datasource.v1.models.ErrorDetailV1 import gq.kirmanak.mealient.datasource.v1.models.GetRecipeResponseV1 @@ -98,8 +100,15 @@ class MealieDataSourceV1Impl @Inject constructor( block = { service.createRecipeFromURL("$baseUrl/api/recipes/create-url", request) }, logMethod = { "parseRecipeFromURL" }, logParameters = { "baseUrl = $baseUrl, request = $request" } - ) + override suspend fun createApiToken( + baseUrl: String, + request: CreateApiTokenRequestV1 + ): CreateApiTokenResponseV1 = networkRequestWrapper.makeCallAndHandleUnauthorized( + block = { service.createApiToken("$baseUrl/api/users/api-tokens", request) }, + logMethod = { "createApiToken" }, + logParameters = { "baseUrl = $baseUrl, request = $request" } + ) } diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieServiceV1.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieServiceV1.kt index 004ccf8..85271cf 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieServiceV1.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/MealieServiceV1.kt @@ -47,4 +47,10 @@ interface MealieServiceV1 { @Url url: String, @Body request: ParseRecipeURLRequestV1, ): String + + @POST + suspend fun createApiToken( + @Url url: String, + @Body request: CreateApiTokenRequestV1, + ): CreateApiTokenResponseV1 } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/models/CreateApiTokenRequestV1.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/models/CreateApiTokenRequestV1.kt new file mode 100644 index 0000000..a50325e --- /dev/null +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/models/CreateApiTokenRequestV1.kt @@ -0,0 +1,9 @@ +package gq.kirmanak.mealient.datasource.v1.models + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +@Serializable +data class CreateApiTokenRequestV1( + @SerialName("name") val name: String, +) diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/models/CreateApiTokenResponseV1.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/models/CreateApiTokenResponseV1.kt new file mode 100644 index 0000000..6c10baa --- /dev/null +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/v1/models/CreateApiTokenResponseV1.kt @@ -0,0 +1,9 @@ +package gq.kirmanak.mealient.datasource.v1.models + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +@Serializable +data class CreateApiTokenResponseV1( + @SerialName("token") val token: String, +) \ No newline at end of file diff --git a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt index 41d8714..708d4c7 100644 --- a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt +++ b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt @@ -17,7 +17,7 @@ class MealieAuthenticatorTest : BaseUnitTest() { private lateinit var subject: MealieAuthenticator - @MockK + @MockK(relaxUnitFun = true) lateinit var authenticationProvider: AuthenticationProvider @Before @@ -48,11 +48,18 @@ class MealieAuthenticatorTest : BaseUnitTest() { } @Test - fun `when no auth header was set expect authenticate to return null`() { + fun `when an auth header was set expect authenticate to return null`() { val response = buildResponse(authHeader = "token") assertThat(subject.authenticate(null, response)).isNull() } + @Test + fun `when an auth header was set expect authenticate to logout`() { + val response = buildResponse(authHeader = "token") + subject.authenticate(null, response) + coVerify { authenticationProvider.logout() } + } + @Test fun `when auth header exists expect authenticate to return request`() { coEvery { authenticationProvider.getAuthHeader() } returns "token" From dd313def962f4efb3df81afc7ab2ed5eed1f10d4 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 16:58:04 +0100 Subject: [PATCH 09/15] Replace Authenticator with Interceptor --- .../mealient/datasource/DataSourceModule.kt | 8 +- .../datasource/impl/AuthInterceptor.kt | 43 +++++++ .../datasource/impl/MealieAuthenticator.kt | 49 -------- .../datasource/impl/OkHttpBuilderImpl.kt | 3 - .../datasource/MealieAuthenticatorTest.kt | 97 --------------- .../datasource/impl/AuthInterceptorTest.kt | 111 ++++++++++++++++++ 6 files changed, 159 insertions(+), 152 deletions(-) create mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt delete mode 100644 datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt delete mode 100644 datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt create mode 100644 datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptorTest.kt diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt index 2ffa990..73ed120 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/DataSourceModule.kt @@ -6,8 +6,9 @@ import dagger.Module import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent +import dagger.multibindings.IntoSet +import gq.kirmanak.mealient.datasource.impl.AuthInterceptor import gq.kirmanak.mealient.datasource.impl.CacheBuilderImpl -import gq.kirmanak.mealient.datasource.impl.MealieAuthenticator import gq.kirmanak.mealient.datasource.impl.NetworkRequestWrapperImpl import gq.kirmanak.mealient.datasource.impl.OkHttpBuilderImpl import gq.kirmanak.mealient.datasource.impl.RetrofitBuilder @@ -19,7 +20,7 @@ import gq.kirmanak.mealient.datasource.v1.MealieDataSourceV1Impl import gq.kirmanak.mealient.datasource.v1.MealieServiceV1 import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.json.Json -import okhttp3.Authenticator +import okhttp3.Interceptor import okhttp3.MediaType.Companion.toMediaType import okhttp3.OkHttpClient import retrofit2.Converter @@ -90,5 +91,6 @@ interface DataSourceModule { @Binds @Singleton - fun bindAuthenticator(mealieAuthenticator: MealieAuthenticator): Authenticator + @IntoSet + fun bindAuthInterceptor(authInterceptor: AuthInterceptor): Interceptor } \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt new file mode 100644 index 0000000..89b2acf --- /dev/null +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptor.kt @@ -0,0 +1,43 @@ +package gq.kirmanak.mealient.datasource.impl + +import androidx.annotation.VisibleForTesting +import gq.kirmanak.mealient.datasource.AuthenticationProvider +import gq.kirmanak.mealient.logging.Logger +import kotlinx.coroutines.runBlocking +import okhttp3.Interceptor +import okhttp3.Response +import javax.inject.Inject +import javax.inject.Provider +import javax.inject.Singleton + +@Singleton +class AuthInterceptor @Inject constructor( + private val logger: Logger, + private val authenticationProviderProvider: Provider, +) : Interceptor { + + private val authenticationProvider: AuthenticationProvider + get() = authenticationProviderProvider.get() + + override fun intercept(chain: Interceptor.Chain): Response { + logger.v { "intercept() was called" } + val header = getAuthHeader() + val request = chain.request().let { + if (header == null) it else it.newBuilder().header(HEADER_NAME, header).build() + } + logger.d { "Sending header $HEADER_NAME=${request.header(HEADER_NAME)}" } + return chain.proceed(request).also { + logger.v { "Response code is ${it.code}" } + if (it.code == 401 && header != null) logout() + } + } + + private fun getAuthHeader() = runBlocking { authenticationProvider.getAuthHeader() } + + private fun logout() = runBlocking { authenticationProvider.logout() } + + companion object { + @VisibleForTesting + const val HEADER_NAME = "Authorization" + } +} \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt deleted file mode 100644 index 5fe4cf5..0000000 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/MealieAuthenticator.kt +++ /dev/null @@ -1,49 +0,0 @@ -package gq.kirmanak.mealient.datasource.impl - -import androidx.annotation.VisibleForTesting -import gq.kirmanak.mealient.datasource.AuthenticationProvider -import kotlinx.coroutines.runBlocking -import okhttp3.Authenticator -import okhttp3.Request -import okhttp3.Response -import okhttp3.Route -import javax.inject.Inject -import javax.inject.Provider -import javax.inject.Singleton - -@Singleton -// TODO has to be interceptor, otherwise only public recipes are visible -class MealieAuthenticator @Inject constructor( - private val authenticationProviderProvider: Provider, -) : Authenticator { - - private val authenticationProvider: AuthenticationProvider - get() = authenticationProviderProvider.get() - - override fun authenticate(route: Route?, response: Response): Request? { - val supportsBearer = response.challenges().any { it.scheme == BEARER_SCHEME } - val request = response.request - return when { - request.header(HEADER_NAME) != null -> { - logout() - null - } - supportsBearer -> getAuthHeader()?.let { request.copyWithHeader(HEADER_NAME, it) } - else -> null - } - } - - private fun getAuthHeader() = runBlocking { authenticationProvider.getAuthHeader() } - - private fun logout() = runBlocking { authenticationProvider.logout() } - - companion object { - @VisibleForTesting - const val HEADER_NAME = "Authorization" - private const val BEARER_SCHEME = "Bearer" - } -} - -private fun Request.copyWithHeader(name: String, value: String): Request { - return newBuilder().header(name, value).build() -} \ No newline at end of file diff --git a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt index bb2a9a6..21ca1d1 100644 --- a/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt +++ b/datasource/src/main/kotlin/gq/kirmanak/mealient/datasource/impl/OkHttpBuilderImpl.kt @@ -2,7 +2,6 @@ package gq.kirmanak.mealient.datasource.impl import gq.kirmanak.mealient.datasource.CacheBuilder import gq.kirmanak.mealient.datasource.OkHttpBuilder -import okhttp3.Authenticator import okhttp3.Interceptor import okhttp3.OkHttpClient import javax.inject.Inject @@ -13,12 +12,10 @@ class OkHttpBuilderImpl @Inject constructor( private val cacheBuilder: CacheBuilder, // Use @JvmSuppressWildcards because otherwise dagger can't inject it (https://stackoverflow.com/a/43149382) private val interceptors: Set<@JvmSuppressWildcards Interceptor>, - private val authenticator: Authenticator, ) : OkHttpBuilder { override fun buildOkHttp(): OkHttpClient = OkHttpClient.Builder() .apply { interceptors.forEach(::addNetworkInterceptor) } .cache(cacheBuilder.buildCache()) - .authenticator(authenticator) .build() } \ No newline at end of file diff --git a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt deleted file mode 100644 index 708d4c7..0000000 --- a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/MealieAuthenticatorTest.kt +++ /dev/null @@ -1,97 +0,0 @@ -package gq.kirmanak.mealient.datasource - -import com.google.common.truth.Truth.assertThat -import gq.kirmanak.mealient.datasource.impl.MealieAuthenticator -import gq.kirmanak.mealient.datasource.impl.MealieAuthenticator.Companion.HEADER_NAME -import gq.kirmanak.mealient.test.BaseUnitTest -import io.mockk.coEvery -import io.mockk.coVerify -import io.mockk.impl.annotations.MockK -import okhttp3.Protocol -import okhttp3.Request -import okhttp3.Response -import org.junit.Before -import org.junit.Test - -class MealieAuthenticatorTest : BaseUnitTest() { - - private lateinit var subject: MealieAuthenticator - - @MockK(relaxUnitFun = true) - lateinit var authenticationProvider: AuthenticationProvider - - @Before - override fun setUp() { - super.setUp() - subject = MealieAuthenticator { authenticationProvider } - } - - @Test - fun `when bearer is not supported expect authenticate to return null`() { - val response = buildResponse(challenges = null) - assertThat(subject.authenticate(null, response)).isNull() - } - - @Test - fun `when no auth header exists expect authenticate to return null`() { - coEvery { authenticationProvider.getAuthHeader() } returns null - val response = buildResponse() - assertThat(subject.authenticate(null, response)).isNull() - } - - @Test - fun `when no auth header exists expect authenticate to call provider`() { - coEvery { authenticationProvider.getAuthHeader() } returns null - val response = buildResponse() - subject.authenticate(null, response) - coVerify { authenticationProvider.getAuthHeader() } - } - - @Test - fun `when an auth header was set expect authenticate to return null`() { - val response = buildResponse(authHeader = "token") - assertThat(subject.authenticate(null, response)).isNull() - } - - @Test - fun `when an auth header was set expect authenticate to logout`() { - val response = buildResponse(authHeader = "token") - subject.authenticate(null, response) - coVerify { authenticationProvider.logout() } - } - - @Test - fun `when auth header exists expect authenticate to return request`() { - coEvery { authenticationProvider.getAuthHeader() } returns "token" - val response = buildResponse() - val actualHeader = subject.authenticate(null, response)?.header(HEADER_NAME) - assertThat(actualHeader).isEqualTo("token") - } - - private fun buildResponse( - url: String = "http://localhost", - code: Int = 401, - message: String = "Unauthorized", - protocol: Protocol = Protocol.HTTP_2, - challenges: String? = "Bearer", - authHeader: String? = null, - ): Response { - val request = buildRequest(authHeader, url) - return Response.Builder().apply { - request(request) - code(code) - message(message) - protocol(protocol) - if (challenges != null) header("WWW-Authenticate", challenges) - }.build() - } - - private fun buildRequest( - authHeader: String? = null, - url: String = "http://localhost", - ): Request = Request.Builder().apply { - url(url) - if (authHeader != null) header(HEADER_NAME, authHeader) - }.build() - -} \ No newline at end of file diff --git a/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptorTest.kt b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptorTest.kt new file mode 100644 index 0000000..16cb895 --- /dev/null +++ b/datasource/src/test/kotlin/gq/kirmanak/mealient/datasource/impl/AuthInterceptorTest.kt @@ -0,0 +1,111 @@ +package gq.kirmanak.mealient.datasource.impl + +import com.google.common.truth.Truth.assertThat +import gq.kirmanak.mealient.datasource.AuthenticationProvider +import gq.kirmanak.mealient.datasource.impl.AuthInterceptor.Companion.HEADER_NAME +import gq.kirmanak.mealient.test.BaseUnitTest +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.impl.annotations.MockK +import io.mockk.slot +import okhttp3.Interceptor +import okhttp3.Protocol +import okhttp3.Request +import okhttp3.Response +import org.junit.Before +import org.junit.Test + +class AuthInterceptorTest : BaseUnitTest() { + + private lateinit var subject: Interceptor + + @MockK(relaxed = true) + lateinit var authenticationProvider: AuthenticationProvider + + @MockK(relaxed = true) + lateinit var chain: Interceptor.Chain + + @Before + override fun setUp() { + super.setUp() + subject = AuthInterceptor(logger) { authenticationProvider } + } + + @Test + fun `when intercept is called expect header to be retrieved`() { + subject.intercept(chain) + coVerify { authenticationProvider.getAuthHeader() } + } + + @Test + fun `when intercept is called and no header expect no header`() { + coEvery { authenticationProvider.getAuthHeader() } returns null + coEvery { chain.request() } returns buildRequest() + val requestSlot = slot() + coEvery { chain.proceed(capture(requestSlot)) } returns buildResponse() + subject.intercept(chain) + assertThat(requestSlot.captured.header(HEADER_NAME)).isNull() + } + + @Test + fun `when intercept is called and no header expect no logout`() { + coEvery { authenticationProvider.getAuthHeader() } returns null + coEvery { chain.request() } returns buildRequest() + coEvery { chain.proceed(any()) } returns buildResponse(code = 200) + subject.intercept(chain) + coVerify(inverse = true) { authenticationProvider.logout() } + } + + @Test + fun `when intercept is called with no header and auth fails expect no logout`() { + coEvery { authenticationProvider.getAuthHeader() } returns null + coEvery { chain.request() } returns buildRequest() + coEvery { chain.proceed(any()) } returns buildResponse(code = 401) + subject.intercept(chain) + coVerify(inverse = true) { authenticationProvider.logout() } + } + + @Test + fun `when intercept is called and there is a header expect a header`() { + coEvery { authenticationProvider.getAuthHeader() } returns "header" + coEvery { chain.request() } returns buildRequest() + val requestSlot = slot() + coEvery { chain.proceed(capture(requestSlot)) } returns buildResponse() + subject.intercept(chain) + assertThat(requestSlot.captured.header(HEADER_NAME)).isEqualTo("header") + } + + @Test + fun `when intercept is called and there is a header that authenticates expect no logout`() { + coEvery { authenticationProvider.getAuthHeader() } returns "header" + coEvery { chain.request() } returns buildRequest() + coEvery { chain.proceed(any()) } returns buildResponse(code = 200) + subject.intercept(chain) + coVerify(inverse = true) { authenticationProvider.logout() } + } + + @Test + fun `when intercept is called and there was a header but still 401 expect logout`() { + coEvery { authenticationProvider.getAuthHeader() } returns "header" + coEvery { chain.request() } returns buildRequest() + coEvery { chain.proceed(any()) } returns buildResponse(code = 401) + subject.intercept(chain) + coVerify { authenticationProvider.logout() } + } + + private fun buildResponse( + url: String = "http://localhost", + code: Int = 200, + message: String = if (code == 200) "OK" else "Unauthorized", + protocol: Protocol = Protocol.HTTP_2, + ) = Response.Builder().apply { + request(buildRequest(url)) + code(code) + message(message) + protocol(protocol) + }.build() + + private fun buildRequest( + url: String = "http://localhost", + ) = Request.Builder().url(url).build() +} \ No newline at end of file From 3720b1aadbe241efba8b084724b98eae9ce3394b Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 17:45:22 +0100 Subject: [PATCH 10/15] Create API token when updating from 24 --- app/src/main/java/gq/kirmanak/mealient/App.kt | 11 +++++ .../configuration/BuildConfigurationImpl.kt | 19 ++++++++ .../migration/From24AuthMigrationExecutor.kt | 41 ++++++++++++++++++ .../data/migration/MigrationDetector.kt | 6 +++ .../data/migration/MigrationDetectorImpl.kt | 43 +++++++++++++++++++ .../data/migration/MigrationExecutor.kt | 8 ++++ .../data/storage/PreferencesStorage.kt | 2 + .../data/storage/PreferencesStorageImpl.kt | 11 ++++- .../mealient/di}/ArchitectureModule.kt | 4 +- .../kirmanak/mealient/di/MigrationModule.kt | 26 +++++++++++ .../configuration/BuildConfiguration.kt | 2 + .../configuration/BuildConfigurationImpl.kt | 14 ------ 12 files changed, 170 insertions(+), 17 deletions(-) create mode 100644 app/src/main/java/gq/kirmanak/mealient/data/configuration/BuildConfigurationImpl.kt create mode 100644 app/src/main/java/gq/kirmanak/mealient/data/migration/From24AuthMigrationExecutor.kt create mode 100644 app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetector.kt create mode 100644 app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt create mode 100644 app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationExecutor.kt rename {architecture/src/main/kotlin/gq/kirmanak/mealient/architecture => app/src/main/java/gq/kirmanak/mealient/di}/ArchitectureModule.kt (78%) create mode 100644 app/src/main/java/gq/kirmanak/mealient/di/MigrationModule.kt delete mode 100644 architecture/src/main/kotlin/gq/kirmanak/mealient/architecture/configuration/BuildConfigurationImpl.kt diff --git a/app/src/main/java/gq/kirmanak/mealient/App.kt b/app/src/main/java/gq/kirmanak/mealient/App.kt index 090b6c6..df1cc2d 100644 --- a/app/src/main/java/gq/kirmanak/mealient/App.kt +++ b/app/src/main/java/gq/kirmanak/mealient/App.kt @@ -4,7 +4,12 @@ import android.app.Application import com.google.android.material.color.DynamicColors import dagger.hilt.android.HiltAndroidApp import gq.kirmanak.mealient.architecture.configuration.BuildConfiguration +import gq.kirmanak.mealient.data.migration.MigrationDetector import gq.kirmanak.mealient.logging.Logger +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch import javax.inject.Inject @HiltAndroidApp @@ -16,9 +21,15 @@ class App : Application() { @Inject lateinit var buildConfiguration: BuildConfiguration + @Inject + lateinit var migrationDetector: MigrationDetector + + private val appCoroutineScope = CoroutineScope(Dispatchers.Main + Job()) + override fun onCreate() { super.onCreate() logger.v { "onCreate() called" } DynamicColors.applyToActivitiesIfAvailable(this) + appCoroutineScope.launch { migrationDetector.executeMigrations() } } } diff --git a/app/src/main/java/gq/kirmanak/mealient/data/configuration/BuildConfigurationImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/configuration/BuildConfigurationImpl.kt new file mode 100644 index 0000000..beb9ac5 --- /dev/null +++ b/app/src/main/java/gq/kirmanak/mealient/data/configuration/BuildConfigurationImpl.kt @@ -0,0 +1,19 @@ +package gq.kirmanak.mealient.data.configuration + +import gq.kirmanak.mealient.BuildConfig +import gq.kirmanak.mealient.architecture.configuration.BuildConfiguration +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class BuildConfigurationImpl @Inject constructor() : BuildConfiguration { + + @get:JvmName("_isDebug") + private val isDebug by lazy { BuildConfig.DEBUG } + + private val versionCode by lazy { BuildConfig.VERSION_CODE } + + override fun isDebug(): Boolean = isDebug + + override fun versionCode(): Int = versionCode +} \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/migration/From24AuthMigrationExecutor.kt b/app/src/main/java/gq/kirmanak/mealient/data/migration/From24AuthMigrationExecutor.kt new file mode 100644 index 0000000..638c8bd --- /dev/null +++ b/app/src/main/java/gq/kirmanak/mealient/data/migration/From24AuthMigrationExecutor.kt @@ -0,0 +1,41 @@ +package gq.kirmanak.mealient.data.migration + +import android.content.SharedPreferences +import androidx.core.content.edit +import gq.kirmanak.mealient.data.auth.AuthRepo +import gq.kirmanak.mealient.datasource.runCatchingExceptCancel +import gq.kirmanak.mealient.datastore.DataStoreModule.Companion.ENCRYPTED +import gq.kirmanak.mealient.logging.Logger +import javax.inject.Inject +import javax.inject.Named +import javax.inject.Singleton + +@Singleton +class From24AuthMigrationExecutor @Inject constructor( + @Named(ENCRYPTED) private val sharedPreferences: SharedPreferences, + private val authRepo: AuthRepo, + private val logger: Logger, +) : MigrationExecutor { + + override val migratingFrom: Int = 24 + + override suspend fun executeMigration() { + logger.v { "executeMigration() was called" } + val email = sharedPreferences.getString(EMAIL_KEY, null) + val password = sharedPreferences.getString(PASSWORD_KEY, null) + if (email != null && password != null) { + runCatchingExceptCancel { authRepo.authenticate(email, password) } + .onFailure { logger.e(it) { "API token creation failed" } } + .onSuccess { logger.i { "Created API token during migration" } } + } + sharedPreferences.edit { + remove(EMAIL_KEY) + remove(PASSWORD_KEY) + } + } + + companion object { + private const val EMAIL_KEY = "email" + private const val PASSWORD_KEY = "password" + } +} \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetector.kt b/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetector.kt new file mode 100644 index 0000000..ee9ec4a --- /dev/null +++ b/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetector.kt @@ -0,0 +1,6 @@ +package gq.kirmanak.mealient.data.migration + +interface MigrationDetector { + + suspend fun executeMigrations() +} \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt new file mode 100644 index 0000000..dd6ec66 --- /dev/null +++ b/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt @@ -0,0 +1,43 @@ +package gq.kirmanak.mealient.data.migration + +import gq.kirmanak.mealient.architecture.configuration.BuildConfiguration +import gq.kirmanak.mealient.data.storage.PreferencesStorage +import gq.kirmanak.mealient.datasource.runCatchingExceptCancel +import gq.kirmanak.mealient.logging.Logger +import javax.inject.Inject +import javax.inject.Singleton + +@Singleton +class MigrationDetectorImpl @Inject constructor( + private val preferencesStorage: PreferencesStorage, + private val migrationExecutors: Set<@JvmSuppressWildcards MigrationExecutor>, + private val buildConfiguration: BuildConfiguration, + private val logger: Logger, +) : MigrationDetector { + + + override suspend fun executeMigrations() { + val key = preferencesStorage.lastExecutedMigrationVersionKey + + val lastVersion = preferencesStorage.getValue(key) ?: VERSION_BEFORE_MIGRATION_IMPLEMENTED + val currentVersion = buildConfiguration.versionCode() + logger.i { "Last migration version is $lastVersion, current is $currentVersion" } + + if (lastVersion != currentVersion) { + migrationExecutors + .filter { it.migratingFrom <= lastVersion } + .forEach { + runCatchingExceptCancel { it.executeMigration() } + .onFailure { logger.e(it) { "Migration executor failed: $it" } } + .onSuccess { logger.i { "Migration executor succeeded: $it" } } + } + } + + preferencesStorage.storeValues(Pair(key, currentVersion)) + } + + + companion object { + private const val VERSION_BEFORE_MIGRATION_IMPLEMENTED = 24 + } +} \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationExecutor.kt b/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationExecutor.kt new file mode 100644 index 0000000..aeb9249 --- /dev/null +++ b/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationExecutor.kt @@ -0,0 +1,8 @@ +package gq.kirmanak.mealient.data.migration + +interface MigrationExecutor { + + val migratingFrom: Int + + suspend fun executeMigration() +} \ No newline at end of file diff --git a/app/src/main/java/gq/kirmanak/mealient/data/storage/PreferencesStorage.kt b/app/src/main/java/gq/kirmanak/mealient/data/storage/PreferencesStorage.kt index df88285..403dfd7 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/storage/PreferencesStorage.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/storage/PreferencesStorage.kt @@ -11,6 +11,8 @@ interface PreferencesStorage { val isDisclaimerAcceptedKey: Preferences.Key + val lastExecutedMigrationVersionKey: Preferences.Key + suspend fun getValue(key: Preferences.Key): T? suspend fun requireValue(key: Preferences.Key): T diff --git a/app/src/main/java/gq/kirmanak/mealient/data/storage/PreferencesStorageImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/storage/PreferencesStorageImpl.kt index f9e7bfa..6cd6077 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/storage/PreferencesStorageImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/storage/PreferencesStorageImpl.kt @@ -4,9 +4,15 @@ import androidx.datastore.core.DataStore import androidx.datastore.preferences.core.Preferences import androidx.datastore.preferences.core.booleanPreferencesKey import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.intPreferencesKey import androidx.datastore.preferences.core.stringPreferencesKey import gq.kirmanak.mealient.logging.Logger -import kotlinx.coroutines.flow.* +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onCompletion +import kotlinx.coroutines.flow.onEach import javax.inject.Inject import javax.inject.Singleton @@ -22,6 +28,9 @@ class PreferencesStorageImpl @Inject constructor( override val isDisclaimerAcceptedKey = booleanPreferencesKey("isDisclaimedAccepted") + override val lastExecutedMigrationVersionKey: Preferences.Key = + intPreferencesKey("lastExecutedMigrationVersion") + override suspend fun getValue(key: Preferences.Key): T? { val value = dataStore.data.first()[key] logger.v { "getValue() returned: $value for $key" } diff --git a/architecture/src/main/kotlin/gq/kirmanak/mealient/architecture/ArchitectureModule.kt b/app/src/main/java/gq/kirmanak/mealient/di/ArchitectureModule.kt similarity index 78% rename from architecture/src/main/kotlin/gq/kirmanak/mealient/architecture/ArchitectureModule.kt rename to app/src/main/java/gq/kirmanak/mealient/di/ArchitectureModule.kt index 59a979e..c322301 100644 --- a/architecture/src/main/kotlin/gq/kirmanak/mealient/architecture/ArchitectureModule.kt +++ b/app/src/main/java/gq/kirmanak/mealient/di/ArchitectureModule.kt @@ -1,11 +1,11 @@ -package gq.kirmanak.mealient.architecture +package gq.kirmanak.mealient.di import dagger.Binds import dagger.Module import dagger.hilt.InstallIn import dagger.hilt.components.SingletonComponent import gq.kirmanak.mealient.architecture.configuration.BuildConfiguration -import gq.kirmanak.mealient.architecture.configuration.BuildConfigurationImpl +import gq.kirmanak.mealient.data.configuration.BuildConfigurationImpl import javax.inject.Singleton @Module diff --git a/app/src/main/java/gq/kirmanak/mealient/di/MigrationModule.kt b/app/src/main/java/gq/kirmanak/mealient/di/MigrationModule.kt new file mode 100644 index 0000000..2ffd796 --- /dev/null +++ b/app/src/main/java/gq/kirmanak/mealient/di/MigrationModule.kt @@ -0,0 +1,26 @@ +package gq.kirmanak.mealient.di + +import dagger.Binds +import dagger.Module +import dagger.hilt.InstallIn +import dagger.hilt.components.SingletonComponent +import dagger.multibindings.IntoSet +import gq.kirmanak.mealient.data.migration.From24AuthMigrationExecutor +import gq.kirmanak.mealient.data.migration.MigrationDetector +import gq.kirmanak.mealient.data.migration.MigrationDetectorImpl +import gq.kirmanak.mealient.data.migration.MigrationExecutor +import javax.inject.Singleton + +@Module +@InstallIn(SingletonComponent::class) +interface MigrationModule { + + @Binds + @Singleton + @IntoSet + fun bindFrom24AuthMigrationExecutor(from24AuthMigrationExecutor: From24AuthMigrationExecutor): MigrationExecutor + + @Binds + @Singleton + fun bindMigrationDetector(migrationDetectorImpl: MigrationDetectorImpl): MigrationDetector +} \ No newline at end of file diff --git a/architecture/src/main/kotlin/gq/kirmanak/mealient/architecture/configuration/BuildConfiguration.kt b/architecture/src/main/kotlin/gq/kirmanak/mealient/architecture/configuration/BuildConfiguration.kt index e076468..a34ab9e 100644 --- a/architecture/src/main/kotlin/gq/kirmanak/mealient/architecture/configuration/BuildConfiguration.kt +++ b/architecture/src/main/kotlin/gq/kirmanak/mealient/architecture/configuration/BuildConfiguration.kt @@ -3,4 +3,6 @@ package gq.kirmanak.mealient.architecture.configuration interface BuildConfiguration { fun isDebug(): Boolean + + fun versionCode(): Int } \ No newline at end of file diff --git a/architecture/src/main/kotlin/gq/kirmanak/mealient/architecture/configuration/BuildConfigurationImpl.kt b/architecture/src/main/kotlin/gq/kirmanak/mealient/architecture/configuration/BuildConfigurationImpl.kt deleted file mode 100644 index b660cd9..0000000 --- a/architecture/src/main/kotlin/gq/kirmanak/mealient/architecture/configuration/BuildConfigurationImpl.kt +++ /dev/null @@ -1,14 +0,0 @@ -package gq.kirmanak.mealient.architecture.configuration - -import gq.kirmanak.mealient.architecture.BuildConfig -import javax.inject.Inject -import javax.inject.Singleton - -@Singleton -class BuildConfigurationImpl @Inject constructor() : BuildConfiguration { - - @get:JvmName("_isDebug") - private val isDebug by lazy { BuildConfig.DEBUG } - - override fun isDebug(): Boolean = isDebug -} \ No newline at end of file From e36cb124319bf5b2fc556e348bbe35387c35816f Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 17:45:46 +0100 Subject: [PATCH 11/15] Bump version to 0.3.10 --- app/build.gradle.kts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 2041ff7..4d38cfd 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -15,8 +15,8 @@ plugins { android { defaultConfig { applicationId = "gq.kirmanak.mealient" - versionCode = 24 - versionName = "0.3.9" + versionCode = 25 + versionName = "0.3.10" } signingConfigs { From 901d37570ac10156dbd36cac3680233036babf62 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 17:49:52 +0100 Subject: [PATCH 12/15] Fix migration logs --- .../mealient/data/migration/MigrationDetectorImpl.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt index dd6ec66..397038f 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt @@ -26,10 +26,10 @@ class MigrationDetectorImpl @Inject constructor( if (lastVersion != currentVersion) { migrationExecutors .filter { it.migratingFrom <= lastVersion } - .forEach { - runCatchingExceptCancel { it.executeMigration() } - .onFailure { logger.e(it) { "Migration executor failed: $it" } } - .onSuccess { logger.i { "Migration executor succeeded: $it" } } + .forEach { executor -> + runCatchingExceptCancel { executor.executeMigration() } + .onFailure { logger.e(it) { "Migration executor failed: $executor" } } + .onSuccess { logger.i { "Migration executor succeeded: $executor" } } } } From 31089eb499df6fdaa5637d355b5085d1f417aeb7 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 17:58:59 +0100 Subject: [PATCH 13/15] Fix binding set of interceptors twice in release --- .../gq/kirmanak/mealient/ReleaseModule.kt | 20 ------------------- 1 file changed, 20 deletions(-) delete mode 100644 datasource/src/release/java/gq/kirmanak/mealient/ReleaseModule.kt diff --git a/datasource/src/release/java/gq/kirmanak/mealient/ReleaseModule.kt b/datasource/src/release/java/gq/kirmanak/mealient/ReleaseModule.kt deleted file mode 100644 index 1be726e..0000000 --- a/datasource/src/release/java/gq/kirmanak/mealient/ReleaseModule.kt +++ /dev/null @@ -1,20 +0,0 @@ -package gq.kirmanak.mealient - -import dagger.Module -import dagger.Provides -import dagger.hilt.InstallIn -import dagger.hilt.components.SingletonComponent -import okhttp3.Interceptor -import javax.inject.Singleton - -@Module -@InstallIn(SingletonComponent::class) -object ReleaseModule { - - // Release version of the application doesn't have any interceptors but this Set - // is required by Dagger, so an empty Set is provided here - // Use @JvmSuppressWildcards because otherwise dagger can't inject it (https://stackoverflow.com/a/43149382) - @Provides - @Singleton - fun provideInterceptors(): Set<@JvmSuppressWildcards Interceptor> = emptySet() -} From 7a5cbbb5a3b1a517dadb40093e96cca25c32e122 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 18:19:32 +0100 Subject: [PATCH 14/15] Add MigrationDetectorImpl tests --- .../data/migration/MigrationDetectorImpl.kt | 2 +- .../migration/MigrationDetectorImplTest.kt | 88 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 app/src/test/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImplTest.kt diff --git a/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt b/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt index 397038f..aeb41f2 100644 --- a/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt +++ b/app/src/main/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImpl.kt @@ -25,7 +25,7 @@ class MigrationDetectorImpl @Inject constructor( if (lastVersion != currentVersion) { migrationExecutors - .filter { it.migratingFrom <= lastVersion } + .filter { it.migratingFrom >= lastVersion } .forEach { executor -> runCatchingExceptCancel { executor.executeMigration() } .onFailure { logger.e(it) { "Migration executor failed: $executor" } } diff --git a/app/src/test/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImplTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImplTest.kt new file mode 100644 index 0000000..ab74f95 --- /dev/null +++ b/app/src/test/java/gq/kirmanak/mealient/data/migration/MigrationDetectorImplTest.kt @@ -0,0 +1,88 @@ +package gq.kirmanak.mealient.data.migration + +import androidx.datastore.preferences.core.intPreferencesKey +import gq.kirmanak.mealient.architecture.configuration.BuildConfiguration +import gq.kirmanak.mealient.data.storage.PreferencesStorage +import gq.kirmanak.mealient.test.BaseUnitTest +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.impl.annotations.MockK +import io.mockk.mockk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.runTest +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class MigrationDetectorImplTest : BaseUnitTest() { + + @MockK(relaxUnitFun = true) + lateinit var buildConfiguration: BuildConfiguration + + @MockK(relaxUnitFun = true) + lateinit var preferencesStorage: PreferencesStorage + + @Test + fun `when last version matches current expect no executors to be called`() = runTest { + val executors = setOf(mockk(), mockk()) + val key = intPreferencesKey("key") + every { preferencesStorage.lastExecutedMigrationVersionKey } returns intPreferencesKey("key") + coEvery { preferencesStorage.getValue(key) } returns 25 + coEvery { buildConfiguration.versionCode() } returns 25 + buildSubject(executors).executeMigrations() + executors.forEach { + coVerify(inverse = true) { it.migratingFrom } + coVerify(inverse = true) { it.executeMigration() } + } + } + + @Test + fun `when last version is 24 and current is 25 expect all executors to be checked`() = runTest { + val firstExecutor = mockk() + every { firstExecutor.migratingFrom } returns 3 + + val secondExecutor = mockk() + every { secondExecutor.migratingFrom } returns 5 + + val executors = setOf(firstExecutor, secondExecutor) + + val key = intPreferencesKey("key") + every { preferencesStorage.lastExecutedMigrationVersionKey } returns intPreferencesKey("key") + + coEvery { preferencesStorage.getValue(key) } returns 24 + coEvery { buildConfiguration.versionCode() } returns 25 + + buildSubject(executors).executeMigrations() + executors.forEach { + coVerify { it.migratingFrom } + coVerify(inverse = true) { it.executeMigration() } + } + } + + @Test + fun `when last version is 24 and current is 25 expect the executor to be called`() = runTest { + val firstExecutor = mockk(relaxUnitFun = true) + every { firstExecutor.migratingFrom } returns 24 + + val executors = setOf(firstExecutor) + + val key = intPreferencesKey("key") + every { preferencesStorage.lastExecutedMigrationVersionKey } returns intPreferencesKey("key") + + coEvery { preferencesStorage.getValue(key) } returns 24 + coEvery { buildConfiguration.versionCode() } returns 25 + + buildSubject(executors).executeMigrations() + executors.forEach { + coVerify { it.migratingFrom } + coVerify { it.executeMigration() } + } + } + + private fun buildSubject(executors: Set) = MigrationDetectorImpl( + preferencesStorage = preferencesStorage, + migrationExecutors = executors, + buildConfiguration = buildConfiguration, + logger = logger, + ) +} \ No newline at end of file From fd89dbad3b2a3b0a2645ba2755246d3be9e55724 Mon Sep 17 00:00:00 2001 From: Kirill Kamakin Date: Sun, 11 Dec 2022 18:33:45 +0100 Subject: [PATCH 15/15] Add From24AuthMigrationExecutorTest tests --- .../data/auth/impl/AuthStorageImplTest.kt | 4 - .../disclaimer/DisclaimerStorageImplTest.kt | 1 + .../From24AuthMigrationExecutorTest.kt | 81 +++++++++++++++++++ .../mealient/test/HiltRobolectricTest.kt | 3 + 4 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 app/src/test/java/gq/kirmanak/mealient/data/migration/From24AuthMigrationExecutorTest.kt diff --git a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt index 5b0eab1..8076179 100644 --- a/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/data/auth/impl/AuthStorageImplTest.kt @@ -8,9 +8,7 @@ import dagger.hilt.android.qualifiers.ApplicationContext import dagger.hilt.android.testing.HiltAndroidTest import gq.kirmanak.mealient.data.auth.AuthStorage import gq.kirmanak.mealient.data.auth.impl.AuthStorageImpl.Companion.AUTH_HEADER_KEY -import gq.kirmanak.mealient.logging.Logger import gq.kirmanak.mealient.test.AuthImplTestData.TEST_AUTH_HEADER -import gq.kirmanak.mealient.test.FakeLogger import gq.kirmanak.mealient.test.HiltRobolectricTest import io.mockk.MockKAnnotations import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -28,8 +26,6 @@ class AuthStorageImplTest : HiltRobolectricTest() { @ApplicationContext lateinit var context: Context - private val logger: Logger = FakeLogger() - lateinit var subject: AuthStorage lateinit var sharedPreferences: SharedPreferences diff --git a/app/src/test/java/gq/kirmanak/mealient/data/disclaimer/DisclaimerStorageImplTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/disclaimer/DisclaimerStorageImplTest.kt index 80a5bb5..dbf6dd1 100644 --- a/app/src/test/java/gq/kirmanak/mealient/data/disclaimer/DisclaimerStorageImplTest.kt +++ b/app/src/test/java/gq/kirmanak/mealient/data/disclaimer/DisclaimerStorageImplTest.kt @@ -11,6 +11,7 @@ import javax.inject.Inject @OptIn(ExperimentalCoroutinesApi::class) @HiltAndroidTest class DisclaimerStorageImplTest : HiltRobolectricTest() { + @Inject lateinit var subject: DisclaimerStorageImpl diff --git a/app/src/test/java/gq/kirmanak/mealient/data/migration/From24AuthMigrationExecutorTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/migration/From24AuthMigrationExecutorTest.kt new file mode 100644 index 0000000..23098a9 --- /dev/null +++ b/app/src/test/java/gq/kirmanak/mealient/data/migration/From24AuthMigrationExecutorTest.kt @@ -0,0 +1,81 @@ +package gq.kirmanak.mealient.data.migration + +import android.content.Context +import android.content.SharedPreferences +import androidx.core.content.edit +import com.google.common.truth.Truth.assertThat +import dagger.hilt.android.qualifiers.ApplicationContext +import dagger.hilt.android.testing.HiltAndroidTest +import gq.kirmanak.mealient.data.auth.AuthRepo +import gq.kirmanak.mealient.test.HiltRobolectricTest +import io.mockk.MockKAnnotations +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Test +import java.io.IOException +import javax.inject.Inject + +@OptIn(ExperimentalCoroutinesApi::class) +@HiltAndroidTest +class From24AuthMigrationExecutorTest : HiltRobolectricTest() { + + @Inject + @ApplicationContext + lateinit var context: Context + + @MockK(relaxUnitFun = true) + lateinit var authRepo: AuthRepo + + private lateinit var subject: MigrationExecutor + private lateinit var sharedPreferences: SharedPreferences + + @Before + fun setUp() { + MockKAnnotations.init(this) + sharedPreferences = context.getSharedPreferences("test", Context.MODE_PRIVATE) + subject = From24AuthMigrationExecutor(sharedPreferences, authRepo, logger) + } + + @Test + fun `when there were email and password expect authentication`() = runTest { + sharedPreferences.edit(commit = true) { + putString("email", "email_value") + putString("password", "pass_value") + } + subject.executeMigration() + coVerify { authRepo.authenticate(eq("email_value"), eq("pass_value")) } + } + + @Test + fun `when there were email and password expect them gone`() = runTest { + sharedPreferences.edit(commit = true) { + putString("email", "email_value") + putString("password", "pass_value") + } + subject.executeMigration() + assertThat(sharedPreferences.getString("email", null)).isNull() + assertThat(sharedPreferences.getString("password", null)).isNull() + } + + @Test + fun `when there is email and password but authenticate fails expect values gone`() = runTest { + sharedPreferences.edit(commit = true) { + putString("email", "email_value") + putString("password", "pass_value") + } + coEvery { authRepo.authenticate(any(), any()) } throws IOException() + subject.executeMigration() + assertThat(sharedPreferences.getString("email", null)).isNull() + assertThat(sharedPreferences.getString("password", null)).isNull() + } + + @Test + fun `when there was no email and password expect no authentication`() = runTest { + subject.executeMigration() + coVerify(inverse = true) { authRepo.authenticate(any(), any()) } + } +} \ No newline at end of file diff --git a/testing/src/main/kotlin/gq/kirmanak/mealient/test/HiltRobolectricTest.kt b/testing/src/main/kotlin/gq/kirmanak/mealient/test/HiltRobolectricTest.kt index 0fd38d7..63aee1f 100644 --- a/testing/src/main/kotlin/gq/kirmanak/mealient/test/HiltRobolectricTest.kt +++ b/testing/src/main/kotlin/gq/kirmanak/mealient/test/HiltRobolectricTest.kt @@ -3,6 +3,7 @@ package gq.kirmanak.mealient.test import androidx.test.ext.junit.runners.AndroidJUnit4 import dagger.hilt.android.testing.HiltAndroidRule import dagger.hilt.android.testing.HiltTestApplication +import gq.kirmanak.mealient.logging.Logger import org.junit.Before import org.junit.Rule import org.junit.runner.RunWith @@ -15,6 +16,8 @@ abstract class HiltRobolectricTest { @get:Rule var hiltRule = HiltAndroidRule(this) + protected val logger: Logger = FakeLogger() + @Before fun inject() { hiltRule.inject()