From 8765d3ac57a9e365446e6ec9cb71cc256be3672d Mon Sep 17 00:00:00 2001 From: Adetunji Dahunsi Date: Wed, 22 Feb 2023 03:19:16 -0500 Subject: [PATCH] Fix tests for batched news resource sync Change-Id: I37545eba8618edc936eb12fa38628d98f85f52ed --- .../repository/OfflineFirstNewsRepository.kt | 2 + .../OfflineFirstNewsRepositoryTest.kt | 47 ++++++++++--------- .../data/testdoubles/TestNewsResourceDao.kt | 36 +++++++------- .../core/data/testdoubles/TestTopicDao.kt | 22 ++++----- 4 files changed, 57 insertions(+), 50 deletions(-) diff --git a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstNewsRepository.kt b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstNewsRepository.kt index e98090cba..c16355d69 100644 --- a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstNewsRepository.kt +++ b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstNewsRepository.kt @@ -34,6 +34,8 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map import javax.inject.Inject +// Heuristic value to optimize for serialization and deserialization cost on client and server +// for each news resource batch. private const val SYNC_BATCH_SIZE = 40 /** diff --git a/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstNewsRepositoryTest.kt b/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstNewsRepositoryTest.kt index 9f43d3441..b2c35056f 100644 --- a/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstNewsRepositoryTest.kt +++ b/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstNewsRepositoryTest.kt @@ -27,6 +27,7 @@ import com.google.samples.apps.nowinandroid.core.data.testdoubles.TestTopicDao import com.google.samples.apps.nowinandroid.core.data.testdoubles.filteredInterestsIds import com.google.samples.apps.nowinandroid.core.data.testdoubles.nonPresentInterestsIds import com.google.samples.apps.nowinandroid.core.database.model.NewsResourceEntity +import com.google.samples.apps.nowinandroid.core.database.model.NewsResourceTopicCrossRef import com.google.samples.apps.nowinandroid.core.database.model.PopulatedNewsResource import com.google.samples.apps.nowinandroid.core.database.model.TopicEntity import com.google.samples.apps.nowinandroid.core.database.model.asExternalModel @@ -107,8 +108,8 @@ class OfflineFirstNewsRepositoryTest { ) assertEquals( - emptyList(), - subject.getNewsResources( + expected = emptyList(), + actual = subject.getNewsResources( query = NewsResourceQuery( filterTopicIds = nonPresentInterestsIds, ), @@ -131,14 +132,14 @@ class OfflineFirstNewsRepositoryTest { .map(PopulatedNewsResource::asExternalModel) assertEquals( - newsResourcesFromNetwork.map(NewsResource::id), - newsResourcesFromDb.map(NewsResource::id), + newsResourcesFromNetwork.map(NewsResource::id).sorted(), + newsResourcesFromDb.map(NewsResource::id).sorted(), ) // After sync version should be updated assertEquals( - network.latestChangeListVersion(CollectionType.NewsResources), - synchronizer.getChangeListVersions().newsResourceVersion, + expected = network.latestChangeListVersion(CollectionType.NewsResources), + actual = synchronizer.getChangeListVersions().newsResourceVersion, ) } @@ -172,14 +173,14 @@ class OfflineFirstNewsRepositoryTest { // Assert that items marked deleted on the network have been deleted locally assertEquals( - newsResourcesFromNetwork.map(NewsResource::id) - deletedItems, - newsResourcesFromDb.map(NewsResource::id), + expected = (newsResourcesFromNetwork.map(NewsResource::id) - deletedItems).sorted(), + actual = newsResourcesFromDb.map(NewsResource::id).sorted(), ) // After sync version should be updated assertEquals( - network.latestChangeListVersion(CollectionType.NewsResources), - synchronizer.getChangeListVersions().newsResourceVersion, + expected = network.latestChangeListVersion(CollectionType.NewsResources), + actual = synchronizer.getChangeListVersions().newsResourceVersion, ) } @@ -211,14 +212,14 @@ class OfflineFirstNewsRepositoryTest { .map(PopulatedNewsResource::asExternalModel) assertEquals( - newsResourcesFromNetwork.map(NewsResource::id), - newsResourcesFromDb.map(NewsResource::id), + expected = newsResourcesFromNetwork.map(NewsResource::id).sorted(), + actual = newsResourcesFromDb.map(NewsResource::id).sorted(), ) // After sync version should be updated assertEquals( - changeList.last().changeListVersion, - synchronizer.getChangeListVersions().newsResourceVersion, + expected = changeList.last().changeListVersion, + actual = synchronizer.getChangeListVersions().newsResourceVersion, ) } @@ -228,12 +229,14 @@ class OfflineFirstNewsRepositoryTest { subject.syncWith(synchronizer) assertEquals( - network.getNewsResources() + expected = network.getNewsResources() .map(NetworkNewsResource::topicEntityShells) .flatten() - .distinctBy(TopicEntity::id), - topicDao.getTopicEntities() - .first(), + .distinctBy(TopicEntity::id) + .sortedBy(TopicEntity::toString), + actual = topicDao.getTopicEntities() + .first() + .sortedBy(TopicEntity::toString), ) } @@ -243,11 +246,13 @@ class OfflineFirstNewsRepositoryTest { subject.syncWith(synchronizer) assertEquals( - network.getNewsResources() + expected = network.getNewsResources() .map(NetworkNewsResource::topicCrossReferences) + .flatten() .distinct() - .flatten(), - newsResourceDao.topicCrossReferences, + .sortedBy(NewsResourceTopicCrossRef::toString), + actual = newsResourceDao.topicCrossReferences + .sortedBy(NewsResourceTopicCrossRef::toString), ) } } diff --git a/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/testdoubles/TestNewsResourceDao.kt b/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/testdoubles/TestNewsResourceDao.kt index baeff4a66..bb1ac20ab 100644 --- a/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/testdoubles/TestNewsResourceDao.kt +++ b/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/testdoubles/TestNewsResourceDao.kt @@ -21,12 +21,10 @@ import com.google.samples.apps.nowinandroid.core.database.model.NewsResourceEnti import com.google.samples.apps.nowinandroid.core.database.model.NewsResourceTopicCrossRef import com.google.samples.apps.nowinandroid.core.database.model.PopulatedNewsResource import com.google.samples.apps.nowinandroid.core.database.model.TopicEntity -import com.google.samples.apps.nowinandroid.core.model.data.NewsResourceType.Video import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.update -import kotlinx.datetime.Instant val filteredInterestsIds = setOf("1") val nonPresentInterestsIds = setOf("2") @@ -37,17 +35,7 @@ val nonPresentInterestsIds = setOf("2") class TestNewsResourceDao : NewsResourceDao { private var entitiesStateFlow = MutableStateFlow( - listOf( - NewsResourceEntity( - id = "1", - title = "news", - content = "Hilt", - url = "url", - headerImageUrl = "headerImageUrl", - type = Video, - publishDate = Instant.fromEpochMilliseconds(1), - ), - ), + emptyList(), ) internal var topicCrossReferences: List = listOf() @@ -78,7 +66,14 @@ class TestNewsResourceDao : NewsResourceDao { override suspend fun insertOrIgnoreNewsResources( entities: List, ): List { - entitiesStateFlow.value = entities + entitiesStateFlow.update { oldValues -> + // Old values come first so new values don't overwrite them + (oldValues + entities) + .distinctBy(NewsResourceEntity::id) + .sortedWith( + compareBy(NewsResourceEntity::publishDate).reversed(), + ) + } // Assume no conflicts on insert return entities.map { it.id.toLong() } } @@ -88,13 +83,22 @@ class TestNewsResourceDao : NewsResourceDao { } override suspend fun upsertNewsResources(newsResourceEntities: List) { - entitiesStateFlow.value = newsResourceEntities + entitiesStateFlow.update { oldValues -> + // New values come first so they overwrite old values + (newsResourceEntities + oldValues) + .distinctBy(NewsResourceEntity::id) + .sortedWith( + compareBy(NewsResourceEntity::publishDate).reversed(), + ) + } } override suspend fun insertOrIgnoreTopicCrossRefEntities( newsResourceTopicCrossReferences: List, ) { - topicCrossReferences = newsResourceTopicCrossReferences + // Keep old values over new ones + topicCrossReferences = (topicCrossReferences + newsResourceTopicCrossReferences) + .distinctBy { it.newsResourceId to it.topicId } } override suspend fun deleteNewsResources(ids: List) { diff --git a/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/testdoubles/TestTopicDao.kt b/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/testdoubles/TestTopicDao.kt index 8ac0dc0b8..c0cef479f 100644 --- a/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/testdoubles/TestTopicDao.kt +++ b/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/testdoubles/TestTopicDao.kt @@ -29,16 +29,7 @@ import kotlinx.coroutines.flow.update class TestTopicDao : TopicDao { private var entitiesStateFlow = MutableStateFlow( - listOf( - TopicEntity( - id = "1", - name = "Topic", - shortDescription = "short description", - longDescription = "long description", - url = "URL", - imageUrl = "image URL", - ), - ), + emptyList(), ) override fun getTopicEntity(topicId: String): Flow { @@ -53,8 +44,10 @@ class TestTopicDao : TopicDao { .map { topics -> topics.filter { it.id in ids } } override suspend fun insertOrIgnoreTopics(topicEntities: List): List { - entitiesStateFlow.value = topicEntities - // Assume no conflicts on insert + // Keep old values over new values + entitiesStateFlow.update { oldValues -> + (oldValues + topicEntities).distinctBy(TopicEntity::id) + } return topicEntities.map { it.id.toLong() } } @@ -63,7 +56,10 @@ class TestTopicDao : TopicDao { } override suspend fun upsertTopics(entities: List) { - entitiesStateFlow.value = entities + // Overwrite old values with new values + entitiesStateFlow.update { oldValues -> + (entities + oldValues).distinctBy(TopicEntity::id) + } } override suspend fun deleteTopics(ids: List) {