Address review feedback

core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/model/SearchResult.kt.kt

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2023 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.data.model
+
+import com.google.samples.apps.nowinandroid.core.model.data.NewsResource
+import com.google.samples.apps.nowinandroid.core.model.data.Topic
+
+/** */
+data class SearchResult(
+    val topics: List<Topic> = emptyList(),
+    val newsResources: List<NewsResource> = emptyList(),
+)

core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/DefaultSearchContentsRepository.kt

@@ -16,6 +16,7 @@
 
 package com.google.samples.apps.nowinandroid.core.data.repository
 
+import com.google.samples.apps.nowinandroid.core.data.model.SearchResult
 import com.google.samples.apps.nowinandroid.core.database.dao.NewsResourceDao
 import com.google.samples.apps.nowinandroid.core.database.dao.NewsResourceFtsDao
 import com.google.samples.apps.nowinandroid.core.database.dao.TopicDao
@@ -29,7 +30,6 @@ import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.flatMapLatest
-import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.mapLatest
 import kotlinx.coroutines.withContext
 import javax.inject.Inject

core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/SearchContentsRepository.kt

@@ -16,8 +16,7 @@
 
 package com.google.samples.apps.nowinandroid.core.data.repository
 
-import com.google.samples.apps.nowinandroid.core.model.data.NewsResource
-import com.google.samples.apps.nowinandroid.core.model.data.Topic
+import com.google.samples.apps.nowinandroid.core.data.model.SearchResult
 import kotlinx.coroutines.flow.Flow
 
 /**
@@ -35,8 +34,3 @@ interface SearchContentsRepository {
      */
     fun searchContents(searchQuery: String): Flow<SearchResult>
 }
-
-data class SearchResult(
-    val topics: List<Topic> = emptyList(),
-    val newsResources: List<NewsResource> = emptyList(),
-)

core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/fake/FakeSearchContentsRepository.kt

@@ -16,9 +16,10 @@
 
 package com.google.samples.apps.nowinandroid.core.data.repository.fake
 
+import com.google.samples.apps.nowinandroid.core.data.model.SearchResult
 import com.google.samples.apps.nowinandroid.core.data.repository.SearchContentsRepository
-import com.google.samples.apps.nowinandroid.core.data.repository.SearchResult
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.flowOf
 import javax.inject.Inject
 
 /**
@@ -26,8 +27,6 @@ import javax.inject.Inject
  */
 class FakeSearchContentsRepository @Inject constructor() : SearchContentsRepository {
 
-    override suspend fun populateFtsData() {}
-    override fun searchContents(searchQuery: String): Flow<SearchResult> {
-        TODO("Not yet implemented")
-    }
+    override suspend fun populateFtsData() { /* no-op */ }
+    override fun searchContents(searchQuery: String): Flow<SearchResult> = flowOf()
 }

core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetSearchContentsUseCase.kt

@@ -16,11 +16,12 @@
 
 package com.google.samples.apps.nowinandroid.core.domain
 
+import com.google.samples.apps.nowinandroid.core.data.model.SearchResult
 import com.google.samples.apps.nowinandroid.core.data.repository.SearchContentsRepository
-import com.google.samples.apps.nowinandroid.core.data.repository.SearchResult
 import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository
 import com.google.samples.apps.nowinandroid.core.domain.model.FollowableTopic
 import com.google.samples.apps.nowinandroid.core.domain.model.UserNewsResource
+import com.google.samples.apps.nowinandroid.core.domain.model.UserSearchResult
 import com.google.samples.apps.nowinandroid.core.model.data.UserData
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.combine
@@ -41,7 +42,7 @@ class GetSearchContentsUseCase @Inject constructor(
             .mapToUserSearchResult(userDataRepository.userData)
 }
 
-fun Flow<SearchResult>.mapToUserSearchResult(userDataStream: Flow<UserData>): Flow<UserSearchResult> =
+private fun Flow<SearchResult>.mapToUserSearchResult(userDataStream: Flow<UserData>): Flow<UserSearchResult> =
     combine(userDataStream) { searchResult, userData ->
         UserSearchResult(
             topics = searchResult.topics.map { topic ->
@@ -58,8 +59,3 @@ fun Flow<SearchResult>.mapToUserSearchResult(userDataStream: Flow<UserData>): Fl
             },
         )
     }
-
-data class UserSearchResult(
-    val topics: List<FollowableTopic> = emptyList(),
-    val newsResources: List<UserNewsResource> = emptyList(),
-)

core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/model/UserSearchResult.kt

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2023 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.domain.model
+
+import com.google.samples.apps.nowinandroid.core.data.model.SearchResult
+
+/**
+ * An entity of [SearchResult] with additional user information such as whether the user is
+ * following a topic.
+ */
+data class UserSearchResult(
+    val topics: List<FollowableTopic> = emptyList(),
+    val newsResources: List<UserNewsResource> = emptyList(),
+)

core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/repository/TestSearchContentsRepository.kt

@@ -16,8 +16,8 @@
 
 package com.google.samples.apps.nowinandroid.core.testing.repository
 
+import com.google.samples.apps.nowinandroid.core.data.model.SearchResult
 import com.google.samples.apps.nowinandroid.core.data.repository.SearchContentsRepository
-import com.google.samples.apps.nowinandroid.core.data.repository.SearchResult
 import com.google.samples.apps.nowinandroid.core.model.data.NewsResource
 import com.google.samples.apps.nowinandroid.core.model.data.Topic
 import kotlinx.coroutines.flow.Flow
@@ -28,23 +28,21 @@ class TestSearchContentsRepository : SearchContentsRepository {
     private val cachedTopics: MutableList<Topic> = mutableListOf()
     private val cachedNewsResources: MutableList<NewsResource> = mutableListOf()
 
-    override suspend fun populateFtsData() {}
-
-    override fun searchContents(searchQuery: String): Flow<SearchResult> {
-        return flowOf(
-            SearchResult(
-                topics = cachedTopics.filter {
-                    it.name.contains(searchQuery) ||
-                        it.shortDescription.contains(searchQuery) ||
-                        it.longDescription.contains(searchQuery)
-                },
-                newsResources = cachedNewsResources.filter {
-                    it.content.contains(searchQuery) ||
-                        it.title.contains(searchQuery)
-                },
-            ),
-        )
-    }
+    override suspend fun populateFtsData() { /* no-op */ }
+
+    override fun searchContents(searchQuery: String): Flow<SearchResult> = flowOf(
+        SearchResult(
+            topics = cachedTopics.filter {
+                it.name.contains(searchQuery) ||
+                    it.shortDescription.contains(searchQuery) ||
+                    it.longDescription.contains(searchQuery)
+            },
+            newsResources = cachedNewsResources.filter {
+                it.content.contains(searchQuery) ||
+                    it.title.contains(searchQuery)
+            },
+        ),
+    )
 
     /**
      * Test only method to add the topics to the stored list in memory

feature/search/src/main/java/com/google/samples/apps/nowinandroid/feature/search/SearchScreen.kt

@@ -151,7 +151,7 @@ internal fun SearchScreen(
 
 @Composable
 fun EmptySearchResultBody(
-    onInterestsClick: () -> Unit = {},
+    onInterestsClick: () -> Unit,
     searchQuery: String,
 ) {
     Column(horizontalAlignment = Alignment.CenterHorizontally) {
@@ -206,8 +206,8 @@ private fun SearchResultBody(
     topics: List<FollowableTopic>,
     newsResources: List<UserNewsResource>,
     onFollowButtonClick: (String, Boolean) -> Unit,
-    onNewsResourcesCheckedChanged: (String, Boolean) -> Unit = { _, _ -> },
-    onTopicClick: (String) -> Unit = {},
+    onNewsResourcesCheckedChanged: (String, Boolean) -> Unit,
+    onTopicClick: (String) -> Unit,
 ) {
     if (topics.isNotEmpty()) {
         Text(
@@ -260,8 +260,8 @@ private fun SearchResultBody(
 @Composable
 private fun SearchToolbar(
     modifier: Modifier = Modifier,
-    onBackClick: () -> Unit = {},
-    onSearchQueryChanged: (String) -> Unit = {},
+    onBackClick: () -> Unit,
+    onSearchQueryChanged: (String) -> Unit,
     searchQuery: String = "",
 ) {
     Row(
@@ -337,7 +337,10 @@ private fun SearchTextField(
 @Composable
 private fun SearchToolbarPreview() {
     NiaTheme {
-        SearchToolbar()
+        SearchToolbar(
+            onBackClick = {},
+            onSearchQueryChanged = {},
+        )
     }
 }
 
@@ -345,7 +348,10 @@ private fun SearchToolbarPreview() {
 @Composable
 private fun EmptySearchResultColumnPreview() {
     NiaTheme {
-        EmptySearchResultBody(searchQuery = "C++")
+        EmptySearchResultBody(
+            onInterestsClick = {},
+            searchQuery = "C++",
+        )
     }
 }
 

feature/search/src/main/java/com/google/samples/apps/nowinandroid/feature/search/SearchViewModel.kt

@@ -20,11 +20,13 @@ import androidx.lifecycle.SavedStateHandle
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import com.google.samples.apps.nowinandroid.core.domain.GetSearchContentsUseCase
+import com.google.samples.apps.nowinandroid.core.result.Result
+import com.google.samples.apps.nowinandroid.core.result.asResult
 import com.google.samples.apps.nowinandroid.feature.search.SearchResultUiState.LoadFailed
+import com.google.samples.apps.nowinandroid.feature.search.SearchResultUiState.Loading
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
-import kotlinx.coroutines.flow.catch
 import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.flow.map
@@ -38,20 +40,30 @@ class SearchViewModel @Inject constructor(
     private val savedStateHandle: SavedStateHandle,
 ) : ViewModel() {
 
-    val searchQuery = savedStateHandle.getStateFlow("searchQuery", "")
+    val searchQuery = savedStateHandle.getStateFlow(SEARCH_QUERY, "")
 
     val searchResultUiState: StateFlow<SearchResultUiState> =
         searchQuery.flatMapLatest { query ->
-            if (query.length < 2) {
+            if (query.length < SEARCH_QUERY_MIN_LENGTH) {
                 flowOf(SearchResultUiState.EmptyQuery)
             } else {
-                getSearchContentsUseCase(query).map {
-                    SearchResultUiState.Success(
-                        topics = it.topics,
-                        newsResources = it.newsResources,
-                    )
-                }.catch {
-                    flowOf(LoadFailed)
+                getSearchContentsUseCase(query).asResult().map {
+                    when (it) {
+                        is Result.Success -> {
+                            SearchResultUiState.Success(
+                                topics = it.data.topics,
+                                newsResources = it.data.newsResources,
+                            )
+                        }
+
+                        is Result.Loading -> {
+                            Loading
+                        }
+
+                        is Result.Error -> {
+                            LoadFailed
+                        }
+                    }
                 }
             }
         }.stateIn(
@@ -61,6 +73,10 @@ class SearchViewModel @Inject constructor(
         )
 
     fun onSearchQueryChanged(query: String) {
-        savedStateHandle["searchQuery"] = query
+        savedStateHandle[SEARCH_QUERY] = query
     }
 }
+
+/** Minimum length where search query is considered as [SearchResultUiState.EmptyQuery] */
+const val SEARCH_QUERY_MIN_LENGTH = 2
+const val SEARCH_QUERY = "searchQuery"

feature/search/src/main/java/com/google/samples/apps/nowinandroid/feature/search/navigation/SearchNavigation.kt

@@ -31,7 +31,7 @@ fun NavController.navigateToSearch(navOptions: NavOptions? = null) {
 fun NavGraphBuilder.searchScreen(
     onBackClick: () -> Unit,
     onInterestsClick: () -> Unit,
-    onTopicClick: (String) -> Unit = {},
+    onTopicClick: (String) -> Unit,
 ) {
     // TODO: Handle back stack for each top-level destination. At the moment each top-level
     // destination may have own search screen's back stack.

feature/search/src/test/java/com/google/samples/apps/nowinandroid/feature/search/SearchViewModelTest.kt

@@ -76,7 +76,7 @@ class SearchViewModelTest {
 
     @Test
     fun emptyResultIsReturned_withNotMatchingQuery() = runTest {
-        val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.searchQuery.collect() }
+        val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.searchResultUiState.collect() }
 
         viewModel.onSearchQueryChanged("XXX")
         searchContentsRepository.addNewsResources(newsResourcesTestData)