Merge pull request #592 from android/tj/news-resource-query

Add NewsResourceQuery to better query encapsulation

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

@@ -21,18 +21,30 @@ import com.google.samples.apps.nowinandroid.core.model.data.NewsResource
 import kotlinx.coroutines.flow.Flow
 
 /**
- * Data layer implementation for [NewsResource]
+ * Encapsulation class for query parameters for [NewsResource]
  */
-interface NewsRepository : Syncable {
+data class NewsResourceQuery(
+    /**
+     * Topic ids to filter for. Null means any topic id will match.
+     */
+    val filterTopicIds: Set<String>? = null,
     /**
-     * Returns available news resources as a stream.
+     * News ids to filter for. Null means any news id will match.
      */
-    fun getNewsResources(): Flow<List<NewsResource>>
+    val filterNewsIds: Set<String>? = null,
+)
 
+/**
+ * Data layer implementation for [NewsResource]
+ */
+interface NewsRepository : Syncable {
     /**
-     * Returns available news resources as a stream filtered by topics.
+     * Returns available news resources that match the specified [query].
      */
     fun getNewsResources(
-        filterTopicIds: Set<String> = emptySet(),
+        query: NewsResourceQuery = NewsResourceQuery(
+            filterTopicIds = null,
+            filterNewsIds = null,
+        ),
     ): Flow<List<NewsResource>>
 }

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

@@ -44,14 +44,13 @@ class OfflineFirstNewsRepository @Inject constructor(
     private val network: NiaNetworkDataSource,
 ) : NewsRepository {
 
-    override fun getNewsResources(): Flow<List<NewsResource>> =
-        newsResourceDao.getNewsResources()
-            .map { it.map(PopulatedNewsResource::asExternalModel) }
-
     override fun getNewsResources(
-        filterTopicIds: Set<String>,
+        query: NewsResourceQuery,
     ): Flow<List<NewsResource>> = newsResourceDao.getNewsResources(
-        filterTopicIds = filterTopicIds,
+        useFilterTopicIds = query.filterTopicIds != null,
+        filterTopicIds = query.filterTopicIds ?: emptySet(),
+        useFilterNewsIds = query.filterNewsIds != null,
+        filterNewsIds = query.filterNewsIds ?: emptySet(),
     )
         .map { it.map(PopulatedNewsResource::asExternalModel) }
 

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

@@ -19,6 +19,7 @@ package com.google.samples.apps.nowinandroid.core.data.repository.fake
 import com.google.samples.apps.nowinandroid.core.data.Synchronizer
 import com.google.samples.apps.nowinandroid.core.data.model.asEntity
 import com.google.samples.apps.nowinandroid.core.data.repository.NewsRepository
+import com.google.samples.apps.nowinandroid.core.data.repository.NewsResourceQuery
 import com.google.samples.apps.nowinandroid.core.database.model.NewsResourceEntity
 import com.google.samples.apps.nowinandroid.core.database.model.asExternalModel
 import com.google.samples.apps.nowinandroid.core.model.data.NewsResource
@@ -43,26 +44,28 @@ class FakeNewsRepository @Inject constructor(
     private val datasource: FakeNiaNetworkDataSource,
 ) : NewsRepository {
 
-    override fun getNewsResources(): Flow<List<NewsResource>> =
-        flow {
-            emit(
-                datasource.getNewsResources()
-                    .map(NetworkNewsResource::asEntity)
-                    .map(NewsResourceEntity::asExternalModel),
-            )
-        }.flowOn(ioDispatcher)
-
     override fun getNewsResources(
-        filterTopicIds: Set<String>,
+        query: NewsResourceQuery,
     ): Flow<List<NewsResource>> =
         flow {
             emit(
                 datasource
                     .getNewsResources()
-                    .filter { it.topics.intersect(filterTopicIds).isNotEmpty() }
+                    .filter { networkNewsResource ->
+                        // Filter out any news resources which don't match the current query.
+                        // If no query parameters (filterTopicIds or filterNewsIds) are specified
+                        // then the news resource is returned.
+                        listOfNotNull(
+                            true,
+                            query.filterNewsIds?.contains(networkNewsResource.id),
+                            query.filterTopicIds?.let { filterTopicIds ->
+                                networkNewsResource.topics.intersect(filterTopicIds).isNotEmpty()
+                            },
+                        )
+                            .all(true::equals)
+                    }
                     .map(NetworkNewsResource::asEntity)
                     .map(NewsResourceEntity::asExternalModel),
-
             )
         }.flowOn(ioDispatcher)
 

core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstNewsRepositoryTest.kt

@@ -92,13 +92,16 @@ class OfflineFirstNewsRepositoryTest {
     fun offlineFirstNewsRepository_news_resources_for_topic_is_backed_by_news_resource_dao() =
         runTest {
             assertEquals(
-                newsResourceDao.getNewsResources(
+                expected = newsResourceDao.getNewsResources(
                     filterTopicIds = filteredInterestsIds,
+                    useFilterTopicIds = true,
                 )
                     .first()
                     .map(PopulatedNewsResource::asExternalModel),
-                subject.getNewsResources(
-                    filterTopicIds = filteredInterestsIds,
+                actual = subject.getNewsResources(
+                    query = NewsResourceQuery(
+                        filterTopicIds = filteredInterestsIds,
+                    ),
                 )
                     .first(),
             )
@@ -106,7 +109,9 @@ class OfflineFirstNewsRepositoryTest {
             assertEquals(
                 emptyList(),
                 subject.getNewsResources(
-                    filterTopicIds = nonPresentInterestsIds,
+                    query = NewsResourceQuery(
+                        filterTopicIds = nonPresentInterestsIds,
+                    ),
                 )
                     .first(),
             )

core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/testdoubles/TestNewsResourceDao.kt

@@ -52,19 +52,27 @@ class TestNewsResourceDao : NewsResourceDao {
 
     internal var topicCrossReferences: List<NewsResourceTopicCrossRef> = listOf()
 
-    override fun getNewsResources(): Flow<List<PopulatedNewsResource>> =
-        entitiesStateFlow.map {
-            it.map(NewsResourceEntity::asPopulatedNewsResource)
-        }
-
     override fun getNewsResources(
+        useFilterTopicIds: Boolean,
         filterTopicIds: Set<String>,
+        useFilterNewsIds: Boolean,
+        filterNewsIds: Set<String>,
     ): Flow<List<PopulatedNewsResource>> =
-        getNewsResources()
+        entitiesStateFlow
+            .map { it.map(NewsResourceEntity::asPopulatedNewsResource) }
             .map { resources ->
-                resources.filter { resource ->
-                    resource.topics.any { it.id in filterTopicIds }
+                var result = resources
+                if (useFilterTopicIds) {
+                    result = result.filter { resource ->
+                        resource.topics.any { it.id in filterTopicIds }
+                    }
+                }
+                if (useFilterNewsIds) {
+                    result = result.filter { resource ->
+                        resource.entity.id in filterNewsIds
+                    }
                 }
+                result
             }
 
     override suspend fun insertOrIgnoreNewsResources(

core/database/src/androidTest/java/com/google/samples/apps/nowinandroid/core/database/dao/NewsResourceDaoTest.kt

@@ -84,6 +84,44 @@ class NewsResourceDaoTest {
         )
     }
 
+    @Test
+    fun newsResourceDao_filters_items_by_news_ids_by_descending_publish_date() = runTest {
+        val newsResourceEntities = listOf(
+            testNewsResource(
+                id = "0",
+                millisSinceEpoch = 0,
+            ),
+            testNewsResource(
+                id = "1",
+                millisSinceEpoch = 3,
+            ),
+            testNewsResource(
+                id = "2",
+                millisSinceEpoch = 1,
+            ),
+            testNewsResource(
+                id = "3",
+                millisSinceEpoch = 2,
+            ),
+        )
+        newsResourceDao.upsertNewsResources(
+            newsResourceEntities,
+        )
+
+        val savedNewsResourceEntities = newsResourceDao.getNewsResources(
+            useFilterNewsIds = true,
+            filterNewsIds = setOf("3", "0"),
+        )
+            .first()
+
+        assertEquals(
+            listOf("3", "0"),
+            savedNewsResourceEntities.map {
+                it.entity.id
+            },
+        )
+    }
+
     @Test
     fun newsResourceDao_filters_items_by_topic_ids_by_descending_publish_date() = runTest {
         val topicEntities = listOf(
@@ -132,6 +170,7 @@ class NewsResourceDaoTest {
         )
 
         val filteredNewsResources = newsResourceDao.getNewsResources(
+            useFilterTopicIds = true,
             filterTopicIds = topicEntities
                 .map(TopicEntity::id)
                 .toSet(),
@@ -143,6 +182,68 @@ class NewsResourceDaoTest {
         )
     }
 
+    @Test
+    fun newsResourceDao_filters_items_by_news_and_topic_ids_by_descending_publish_date() = runTest {
+        val topicEntities = listOf(
+            testTopicEntity(
+                id = "1",
+                name = "1",
+            ),
+            testTopicEntity(
+                id = "2",
+                name = "2",
+            ),
+        )
+        val newsResourceEntities = listOf(
+            testNewsResource(
+                id = "0",
+                millisSinceEpoch = 0,
+            ),
+            testNewsResource(
+                id = "1",
+                millisSinceEpoch = 3,
+            ),
+            testNewsResource(
+                id = "2",
+                millisSinceEpoch = 1,
+            ),
+            testNewsResource(
+                id = "3",
+                millisSinceEpoch = 2,
+            ),
+        )
+        val newsResourceTopicCrossRefEntities = topicEntities.mapIndexed { index, topicEntity ->
+            NewsResourceTopicCrossRef(
+                newsResourceId = index.toString(),
+                topicId = topicEntity.id,
+            )
+        }
+
+        topicDao.insertOrIgnoreTopics(
+            topicEntities = topicEntities,
+        )
+        newsResourceDao.upsertNewsResources(
+            newsResourceEntities,
+        )
+        newsResourceDao.insertOrIgnoreTopicCrossRefEntities(
+            newsResourceTopicCrossRefEntities,
+        )
+
+        val filteredNewsResources = newsResourceDao.getNewsResources(
+            useFilterTopicIds = true,
+            filterTopicIds = topicEntities
+                .map(TopicEntity::id)
+                .toSet(),
+            useFilterNewsIds = true,
+            filterNewsIds = setOf("1"),
+        ).first()
+
+        assertEquals(
+            listOf("1"),
+            filteredNewsResources.map { it.entity.id },
+        )
+    }
+
     @Test
     fun newsResourceDao_deletes_items_by_ids() =
         runTest {

core/database/src/main/java/com/google/samples/apps/nowinandroid/core/database/dao/NewsResourceDao.kt

@@ -34,29 +34,36 @@ import kotlinx.coroutines.flow.Flow
  */
 @Dao
 interface NewsResourceDao {
-    @Transaction
-    @Query(
-        value = """
-            SELECT * FROM news_resources
-            ORDER BY publish_date DESC
-    """,
-    )
-    fun getNewsResources(): Flow<List<PopulatedNewsResource>>
 
+    /**
+     * Fetches news resources that match the query parameters
+     */
     @Transaction
     @Query(
         value = """
             SELECT * FROM news_resources
-            WHERE id in
-            (
-                SELECT news_resource_id FROM news_resources_topics
-                WHERE topic_id IN (:filterTopicIds)
-            )
+            WHERE 
+                CASE WHEN :useFilterNewsIds
+                    THEN id IN (:filterNewsIds)
+                    ELSE 1
+                END
+             AND
+                CASE WHEN :useFilterTopicIds
+                    THEN id IN
+                        (
+                            SELECT news_resource_id FROM news_resources_topics
+                            WHERE topic_id IN (:filterTopicIds)
+                        )
+                    ELSE 1
+                END
             ORDER BY publish_date DESC
     """,
     )
     fun getNewsResources(
+        useFilterTopicIds: Boolean = false,
         filterTopicIds: Set<String> = emptySet(),
+        useFilterNewsIds: Boolean = false,
+        filterNewsIds: Set<String> = emptySet(),
     ): Flow<List<PopulatedNewsResource>>
 
     /**

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

@@ -17,6 +17,7 @@
 package com.google.samples.apps.nowinandroid.core.domain
 
 import com.google.samples.apps.nowinandroid.core.data.repository.NewsRepository
+import com.google.samples.apps.nowinandroid.core.data.repository.NewsResourceQuery
 import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository
 import com.google.samples.apps.nowinandroid.core.domain.model.UserNewsResource
 import com.google.samples.apps.nowinandroid.core.domain.model.mapToUserNewsResources
@@ -38,17 +39,14 @@ class GetUserNewsResourcesUseCase @Inject constructor(
     /**
      * Returns a list of UserNewsResources which match the supplied set of topic ids.
      *
-     * @param filterTopicIds - A set of topic ids used to filter the list of news resources. If
-     * this is empty the list of news resources will not be filtered.
+     * @param query - Summary of query parameters for news resources.
      */
     operator fun invoke(
-        filterTopicIds: Set<String> = emptySet(),
+        query: NewsResourceQuery = NewsResourceQuery(),
     ): Flow<List<UserNewsResource>> =
-        if (filterTopicIds.isEmpty()) {
-            newsRepository.getNewsResources()
-        } else {
-            newsRepository.getNewsResources(filterTopicIds = filterTopicIds)
-        }.mapToUserNewsResources(userDataRepository.userData)
+        newsRepository.getNewsResources(
+            query = query,
+        ).mapToUserNewsResources(userDataRepository.userData)
 }
 
 private fun Flow<List<NewsResource>>.mapToUserNewsResources(

core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt

@@ -16,6 +16,7 @@
 
 package com.google.samples.apps.nowinandroid.core.domain
 
+import com.google.samples.apps.nowinandroid.core.data.repository.NewsResourceQuery
 import com.google.samples.apps.nowinandroid.core.domain.model.mapToUserNewsResources
 import com.google.samples.apps.nowinandroid.core.model.data.NewsResource
 import com.google.samples.apps.nowinandroid.core.model.data.NewsResourceType.Video
@@ -67,7 +68,11 @@ class GetUserNewsResourcesUseCaseTest {
     @Test
     fun whenFilteredByTopicId_matchingNewsResourcesAreReturned() = runTest {
         // Obtain a stream of user news resources for the given topic id.
-        val userNewsResources = useCase(filterTopicIds = setOf(sampleTopic1.id))
+        val userNewsResources = useCase(
+            NewsResourceQuery(
+                filterTopicIds = setOf(sampleTopic1.id),
+            ),
+        )
 
         // Send test data into the repositories.
         newsRepository.sendNewsResources(sampleNewsResources)

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

@@ -18,6 +18,7 @@ package com.google.samples.apps.nowinandroid.core.testing.repository
 
 import com.google.samples.apps.nowinandroid.core.data.Synchronizer
 import com.google.samples.apps.nowinandroid.core.data.repository.NewsRepository
+import com.google.samples.apps.nowinandroid.core.data.repository.NewsResourceQuery
 import com.google.samples.apps.nowinandroid.core.model.data.NewsResource
 import com.google.samples.apps.nowinandroid.core.model.data.Topic
 import kotlinx.coroutines.channels.BufferOverflow
@@ -33,13 +34,20 @@ class TestNewsRepository : NewsRepository {
     private val newsResourcesFlow: MutableSharedFlow<List<NewsResource>> =
         MutableSharedFlow(replay = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST)
 
-    override fun getNewsResources(): Flow<List<NewsResource>> = newsResourcesFlow
-
-    override fun getNewsResources(filterTopicIds: Set<String>): Flow<List<NewsResource>> =
-        getNewsResources().map { newsResources ->
-            newsResources.filter {
-                it.topics.map(Topic::id).intersect(filterTopicIds).isNotEmpty()
+    override fun getNewsResources(query: NewsResourceQuery): Flow<List<NewsResource>> =
+        newsResourcesFlow.map { newsResources ->
+            var result = newsResources
+            query.filterTopicIds?.let { filterTopicIds ->
+                result = newsResources.filter {
+                    it.topics.map(Topic::id).intersect(filterTopicIds).isNotEmpty()
+                }
+            }
+            query.filterNewsIds?.let { filterNewsIds ->
+                result = newsResources.filter {
+                    filterNewsIds.contains(it.id)
+                }
             }
+            result
         }
 
     /**

feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt

@@ -18,6 +18,7 @@ package com.google.samples.apps.nowinandroid.feature.foryou
 
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
+import com.google.samples.apps.nowinandroid.core.data.repository.NewsResourceQuery
 import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository
 import com.google.samples.apps.nowinandroid.core.data.util.SyncStatusMonitor
 import com.google.samples.apps.nowinandroid.core.domain.GetFollowableTopicsUseCase
@@ -127,7 +128,11 @@ private fun UserDataRepository.getFollowedUserNewsResources(
         if (followedTopics == null) {
             flowOf(emptyList())
         } else {
-            getUserNewsResources(filterTopicIds = followedTopics)
+            getUserNewsResources(
+                NewsResourceQuery(
+                    filterTopicIds = followedTopics,
+                ),
+            )
         }
     }
 

feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt

@@ -19,6 +19,7 @@ package com.google.samples.apps.nowinandroid.feature.topic
 import androidx.lifecycle.SavedStateHandle
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
+import com.google.samples.apps.nowinandroid.core.data.repository.NewsResourceQuery
 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.decoder.StringDecoder
@@ -120,9 +121,11 @@ private fun topicUiState(
                         ),
                     )
                 }
+
                 is Result.Loading -> {
                     TopicUiState.Loading
                 }
+
                 is Result.Error -> {
                     TopicUiState.Error
                 }
@@ -137,7 +140,9 @@ private fun newsUiState(
 ): Flow<NewsUiState> {
     // Observe news
     val newsStream: Flow<List<UserNewsResource>> = getSaveableNewsResources(
-        filterTopicIds = setOf(element = topicId),
+        NewsResourceQuery(
+            filterTopicIds = setOf(element = topicId),
+        ),
     )
 
     // Observe bookmarks
@@ -156,9 +161,11 @@ private fun newsUiState(
                     val news = newsToBookmarksResult.data.first
                     NewsUiState.Success(news)
                 }
+
                 is Result.Loading -> {
                     NewsUiState.Loading
                 }
+
                 is Result.Error -> {
                     NewsUiState.Error
                 }