From e8449112b4c616d3ef9c6357c688d03d96d6a50d Mon Sep 17 00:00:00 2001 From: Simon Marquis Date: Sat, 29 Jul 2023 15:07:43 +0200 Subject: [PATCH] Replace StringDecoder injection with simpler API The initial reason for this `StringDecoder` was to make sure the AndroidX navigation route would not use incompatible chars in the resulting String. For that, `Uri.encode(String)` was used, but this is an Android API (`android.net.Uri`). Unit tests would fail because Uri is not mocked (since we are not using Robolectric). Therefore, a `StringDecoder` interface was introduced with 2 implementations: `UriDecoder` and `FakeStringDecoder`, Hilt modules were added, etc. FWIW, the naming of the API was misleading: `StringDecoder.decodeString(encodedString: String)` does not inform how encoded the input is, so `UriDecoder` could "decode" something that was not necessarily uri "encoded". The solution to this problem was to simply use regular Java APIs: - `URLDecoder.decode(urlEncodedTopicId, "UTF-8")` - `URLEncoder.encode(topicId, "UTF-8")` --- .../core/decoder/StringDecoder.kt | 21 ----------- .../nowinandroid/core/decoder/UriDecoder.kt | 24 ------------- .../core/decoder/di/StringDecoderModule.kt | 31 ---------------- .../core/testing/decoder/FakeStringDecoder.kt | 24 ------------- .../testing/di/TestStringDecoderModule.kt | 35 ------------------- .../feature/topic/TopicViewModel.kt | 4 +-- .../topic/navigation/TopicNavigation.kt | 10 +++--- .../feature/topic/TopicViewModelTest.kt | 2 -- 8 files changed, 6 insertions(+), 145 deletions(-) delete mode 100644 core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt delete mode 100644 core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt delete mode 100644 core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt delete mode 100644 core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt delete mode 100644 core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt diff --git a/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt deleted file mode 100644 index 29437cc71..000000000 --- a/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.samples.apps.nowinandroid.core.decoder - -interface StringDecoder { - fun decodeString(encodedString: String): String -} diff --git a/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt deleted file mode 100644 index b114bdab6..000000000 --- a/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.samples.apps.nowinandroid.core.decoder - -import android.net.Uri -import javax.inject.Inject - -class UriDecoder @Inject constructor() : StringDecoder { - override fun decodeString(encodedString: String): String = Uri.decode(encodedString) -} diff --git a/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt deleted file mode 100644 index e39b0e1f8..000000000 --- a/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.samples.apps.nowinandroid.core.decoder.di - -import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder -import com.google.samples.apps.nowinandroid.core.decoder.UriDecoder -import dagger.Binds -import dagger.Module -import dagger.hilt.InstallIn -import dagger.hilt.components.SingletonComponent - -@Module -@InstallIn(SingletonComponent::class) -abstract class StringDecoderModule { - @Binds - abstract fun bindStringDecoder(uriDecoder: UriDecoder): StringDecoder -} diff --git a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt deleted file mode 100644 index 7282661cc..000000000 --- a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.samples.apps.nowinandroid.core.testing.decoder - -import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder -import javax.inject.Inject - -class FakeStringDecoder @Inject constructor() : StringDecoder { - override fun decodeString(encodedString: String): String = encodedString -} diff --git a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt deleted file mode 100644 index 0873ee546..000000000 --- a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.samples.apps.nowinandroid.core.testing.di - -import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder -import com.google.samples.apps.nowinandroid.core.decoder.di.StringDecoderModule -import com.google.samples.apps.nowinandroid.core.testing.decoder.FakeStringDecoder -import dagger.Binds -import dagger.Module -import dagger.hilt.components.SingletonComponent -import dagger.hilt.testing.TestInstallIn - -@Module -@TestInstallIn( - components = [SingletonComponent::class], - replaces = [StringDecoderModule::class], -) -abstract class TestStringDecoderModule { - @Binds - abstract fun bindsStringDecoder(fakeStringDecoder: FakeStringDecoder): StringDecoder -} diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt index 2b2565f9e..7f8c6067e 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt @@ -23,7 +23,6 @@ import com.google.samples.apps.nowinandroid.core.data.repository.NewsResourceQue import com.google.samples.apps.nowinandroid.core.data.repository.TopicsRepository import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository import com.google.samples.apps.nowinandroid.core.data.repository.UserNewsResourceRepository -import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic import com.google.samples.apps.nowinandroid.core.model.data.Topic import com.google.samples.apps.nowinandroid.core.model.data.UserNewsResource @@ -43,13 +42,12 @@ import javax.inject.Inject @HiltViewModel class TopicViewModel @Inject constructor( savedStateHandle: SavedStateHandle, - stringDecoder: StringDecoder, private val userDataRepository: UserDataRepository, topicsRepository: TopicsRepository, userNewsResourceRepository: UserNewsResourceRepository, ) : ViewModel() { - private val topicArgs: TopicArgs = TopicArgs(savedStateHandle, stringDecoder) + private val topicArgs: TopicArgs = TopicArgs(savedStateHandle) val topicId = topicArgs.topicId diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt index 0954a52ac..b012b12b1 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt @@ -16,7 +16,6 @@ package com.google.samples.apps.nowinandroid.feature.topic.navigation -import android.net.Uri import androidx.annotation.VisibleForTesting import androidx.lifecycle.SavedStateHandle import androidx.navigation.NavController @@ -24,19 +23,20 @@ import androidx.navigation.NavGraphBuilder import androidx.navigation.NavType import androidx.navigation.compose.composable import androidx.navigation.navArgument -import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder import com.google.samples.apps.nowinandroid.feature.topic.TopicRoute +import java.net.URLDecoder +import java.net.URLEncoder @VisibleForTesting internal const val topicIdArg = "topicId" internal class TopicArgs(val topicId: String) { - constructor(savedStateHandle: SavedStateHandle, stringDecoder: StringDecoder) : - this(stringDecoder.decodeString(checkNotNull(savedStateHandle[topicIdArg]))) + constructor(savedStateHandle: SavedStateHandle) : + this(URLDecoder.decode(checkNotNull(savedStateHandle[topicIdArg]), "UTF-8")) } fun NavController.navigateToTopic(topicId: String) { - val encodedId = Uri.encode(topicId) + val encodedId = URLEncoder.encode(topicId, "UTF-8") this.navigate("topic_route/$encodedId") { launchSingleTop = true } diff --git a/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt b/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt index ff7a88160..d365232d7 100644 --- a/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt +++ b/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt @@ -22,7 +22,6 @@ import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic import com.google.samples.apps.nowinandroid.core.model.data.NewsResource import com.google.samples.apps.nowinandroid.core.model.data.NewsResourceType.Video import com.google.samples.apps.nowinandroid.core.model.data.Topic -import com.google.samples.apps.nowinandroid.core.testing.decoder.FakeStringDecoder import com.google.samples.apps.nowinandroid.core.testing.repository.TestNewsRepository import com.google.samples.apps.nowinandroid.core.testing.repository.TestTopicsRepository import com.google.samples.apps.nowinandroid.core.testing.repository.TestUserDataRepository @@ -63,7 +62,6 @@ class TopicViewModelTest { fun setup() { viewModel = TopicViewModel( savedStateHandle = SavedStateHandle(mapOf(topicIdArg to testInputTopics[0].topic.id)), - stringDecoder = FakeStringDecoder(), userDataRepository = userDataRepository, topicsRepository = topicsRepository, userNewsResourceRepository = userNewsResourceRepository,