Skip to content

Commit 8765d3a

Browse files
tunjidtunjid
committed
Fix tests for batched news resource sync
Change-Id: I37545eba8618edc936eb12fa38628d98f85f52ed
1 parent 1e8d1d9 commit 8765d3a

File tree

4 files changed

+57
-50
lines changed

4 files changed

+57
-50
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ import kotlinx.coroutines.flow.Flow
3434
import kotlinx.coroutines.flow.map
3535
import javax.inject.Inject
3636

37+
// Heuristic value to optimize for serialization and deserialization cost on client and server
38+
// for each news resource batch.
3739
private const val SYNC_BATCH_SIZE = 40
3840

3941
/**

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

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import com.google.samples.apps.nowinandroid.core.data.testdoubles.TestTopicDao
2727
import com.google.samples.apps.nowinandroid.core.data.testdoubles.filteredInterestsIds
2828
import com.google.samples.apps.nowinandroid.core.data.testdoubles.nonPresentInterestsIds
2929
import com.google.samples.apps.nowinandroid.core.database.model.NewsResourceEntity
30+
import com.google.samples.apps.nowinandroid.core.database.model.NewsResourceTopicCrossRef
3031
import com.google.samples.apps.nowinandroid.core.database.model.PopulatedNewsResource
3132
import com.google.samples.apps.nowinandroid.core.database.model.TopicEntity
3233
import com.google.samples.apps.nowinandroid.core.database.model.asExternalModel
@@ -107,8 +108,8 @@ class OfflineFirstNewsRepositoryTest {
107108
)
108109

109110
assertEquals(
110-
emptyList(),
111-
subject.getNewsResources(
111+
expected = emptyList(),
112+
actual = subject.getNewsResources(
112113
query = NewsResourceQuery(
113114
filterTopicIds = nonPresentInterestsIds,
114115
),
@@ -131,14 +132,14 @@ class OfflineFirstNewsRepositoryTest {
131132
.map(PopulatedNewsResource::asExternalModel)
132133

133134
assertEquals(
134-
newsResourcesFromNetwork.map(NewsResource::id),
135-
newsResourcesFromDb.map(NewsResource::id),
135+
newsResourcesFromNetwork.map(NewsResource::id).sorted(),
136+
newsResourcesFromDb.map(NewsResource::id).sorted(),
136137
)
137138

138139
// After sync version should be updated
139140
assertEquals(
140-
network.latestChangeListVersion(CollectionType.NewsResources),
141-
synchronizer.getChangeListVersions().newsResourceVersion,
141+
expected = network.latestChangeListVersion(CollectionType.NewsResources),
142+
actual = synchronizer.getChangeListVersions().newsResourceVersion,
142143
)
143144
}
144145

@@ -172,14 +173,14 @@ class OfflineFirstNewsRepositoryTest {
172173

173174
// Assert that items marked deleted on the network have been deleted locally
174175
assertEquals(
175-
newsResourcesFromNetwork.map(NewsResource::id) - deletedItems,
176-
newsResourcesFromDb.map(NewsResource::id),
176+
expected = (newsResourcesFromNetwork.map(NewsResource::id) - deletedItems).sorted(),
177+
actual = newsResourcesFromDb.map(NewsResource::id).sorted(),
177178
)
178179

179180
// After sync version should be updated
180181
assertEquals(
181-
network.latestChangeListVersion(CollectionType.NewsResources),
182-
synchronizer.getChangeListVersions().newsResourceVersion,
182+
expected = network.latestChangeListVersion(CollectionType.NewsResources),
183+
actual = synchronizer.getChangeListVersions().newsResourceVersion,
183184
)
184185
}
185186

@@ -211,14 +212,14 @@ class OfflineFirstNewsRepositoryTest {
211212
.map(PopulatedNewsResource::asExternalModel)
212213

213214
assertEquals(
214-
newsResourcesFromNetwork.map(NewsResource::id),
215-
newsResourcesFromDb.map(NewsResource::id),
215+
expected = newsResourcesFromNetwork.map(NewsResource::id).sorted(),
216+
actual = newsResourcesFromDb.map(NewsResource::id).sorted(),
216217
)
217218

218219
// After sync version should be updated
219220
assertEquals(
220-
changeList.last().changeListVersion,
221-
synchronizer.getChangeListVersions().newsResourceVersion,
221+
expected = changeList.last().changeListVersion,
222+
actual = synchronizer.getChangeListVersions().newsResourceVersion,
222223
)
223224
}
224225

@@ -228,12 +229,14 @@ class OfflineFirstNewsRepositoryTest {
228229
subject.syncWith(synchronizer)
229230

230231
assertEquals(
231-
network.getNewsResources()
232+
expected = network.getNewsResources()
232233
.map(NetworkNewsResource::topicEntityShells)
233234
.flatten()
234-
.distinctBy(TopicEntity::id),
235-
topicDao.getTopicEntities()
236-
.first(),
235+
.distinctBy(TopicEntity::id)
236+
.sortedBy(TopicEntity::toString),
237+
actual = topicDao.getTopicEntities()
238+
.first()
239+
.sortedBy(TopicEntity::toString),
237240
)
238241
}
239242

@@ -243,11 +246,13 @@ class OfflineFirstNewsRepositoryTest {
243246
subject.syncWith(synchronizer)
244247

245248
assertEquals(
246-
network.getNewsResources()
249+
expected = network.getNewsResources()
247250
.map(NetworkNewsResource::topicCrossReferences)
251+
.flatten()
248252
.distinct()
249-
.flatten(),
250-
newsResourceDao.topicCrossReferences,
253+
.sortedBy(NewsResourceTopicCrossRef::toString),
254+
actual = newsResourceDao.topicCrossReferences
255+
.sortedBy(NewsResourceTopicCrossRef::toString),
251256
)
252257
}
253258
}

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

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,10 @@ import com.google.samples.apps.nowinandroid.core.database.model.NewsResourceEnti
2121
import com.google.samples.apps.nowinandroid.core.database.model.NewsResourceTopicCrossRef
2222
import com.google.samples.apps.nowinandroid.core.database.model.PopulatedNewsResource
2323
import com.google.samples.apps.nowinandroid.core.database.model.TopicEntity
24-
import com.google.samples.apps.nowinandroid.core.model.data.NewsResourceType.Video
2524
import kotlinx.coroutines.flow.Flow
2625
import kotlinx.coroutines.flow.MutableStateFlow
2726
import kotlinx.coroutines.flow.map
2827
import kotlinx.coroutines.flow.update
29-
import kotlinx.datetime.Instant
3028

3129
val filteredInterestsIds = setOf("1")
3230
val nonPresentInterestsIds = setOf("2")
@@ -37,17 +35,7 @@ val nonPresentInterestsIds = setOf("2")
3735
class TestNewsResourceDao : NewsResourceDao {
3836

3937
private var entitiesStateFlow = MutableStateFlow(
40-
listOf(
41-
NewsResourceEntity(
42-
id = "1",
43-
title = "news",
44-
content = "Hilt",
45-
url = "url",
46-
headerImageUrl = "headerImageUrl",
47-
type = Video,
48-
publishDate = Instant.fromEpochMilliseconds(1),
49-
),
50-
),
38+
emptyList<NewsResourceEntity>(),
5139
)
5240

5341
internal var topicCrossReferences: List<NewsResourceTopicCrossRef> = listOf()
@@ -78,7 +66,14 @@ class TestNewsResourceDao : NewsResourceDao {
7866
override suspend fun insertOrIgnoreNewsResources(
7967
entities: List<NewsResourceEntity>,
8068
): List<Long> {
81-
entitiesStateFlow.value = entities
69+
entitiesStateFlow.update { oldValues ->
70+
// Old values come first so new values don't overwrite them
71+
(oldValues + entities)
72+
.distinctBy(NewsResourceEntity::id)
73+
.sortedWith(
74+
compareBy(NewsResourceEntity::publishDate).reversed(),
75+
)
76+
}
8277
// Assume no conflicts on insert
8378
return entities.map { it.id.toLong() }
8479
}
@@ -88,13 +83,22 @@ class TestNewsResourceDao : NewsResourceDao {
8883
}
8984

9085
override suspend fun upsertNewsResources(newsResourceEntities: List<NewsResourceEntity>) {
91-
entitiesStateFlow.value = newsResourceEntities
86+
entitiesStateFlow.update { oldValues ->
87+
// New values come first so they overwrite old values
88+
(newsResourceEntities + oldValues)
89+
.distinctBy(NewsResourceEntity::id)
90+
.sortedWith(
91+
compareBy(NewsResourceEntity::publishDate).reversed(),
92+
)
93+
}
9294
}
9395

9496
override suspend fun insertOrIgnoreTopicCrossRefEntities(
9597
newsResourceTopicCrossReferences: List<NewsResourceTopicCrossRef>,
9698
) {
97-
topicCrossReferences = newsResourceTopicCrossReferences
99+
// Keep old values over new ones
100+
topicCrossReferences = (topicCrossReferences + newsResourceTopicCrossReferences)
101+
.distinctBy { it.newsResourceId to it.topicId }
98102
}
99103

100104
override suspend fun deleteNewsResources(ids: List<String>) {

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

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,7 @@ import kotlinx.coroutines.flow.update
2929
class TestTopicDao : TopicDao {
3030

3131
private var entitiesStateFlow = MutableStateFlow(
32-
listOf(
33-
TopicEntity(
34-
id = "1",
35-
name = "Topic",
36-
shortDescription = "short description",
37-
longDescription = "long description",
38-
url = "URL",
39-
imageUrl = "image URL",
40-
),
41-
),
32+
emptyList<TopicEntity>(),
4233
)
4334

4435
override fun getTopicEntity(topicId: String): Flow<TopicEntity> {
@@ -53,8 +44,10 @@ class TestTopicDao : TopicDao {
5344
.map { topics -> topics.filter { it.id in ids } }
5445

5546
override suspend fun insertOrIgnoreTopics(topicEntities: List<TopicEntity>): List<Long> {
56-
entitiesStateFlow.value = topicEntities
57-
// Assume no conflicts on insert
47+
// Keep old values over new values
48+
entitiesStateFlow.update { oldValues ->
49+
(oldValues + topicEntities).distinctBy(TopicEntity::id)
50+
}
5851
return topicEntities.map { it.id.toLong() }
5952
}
6053

@@ -63,7 +56,10 @@ class TestTopicDao : TopicDao {
6356
}
6457

6558
override suspend fun upsertTopics(entities: List<TopicEntity>) {
66-
entitiesStateFlow.value = entities
59+
// Overwrite old values with new values
60+
entitiesStateFlow.update { oldValues ->
61+
(entities + oldValues).distinctBy(TopicEntity::id)
62+
}
6763
}
6864

6965
override suspend fun deleteTopics(ids: List<String>) {

0 commit comments

Comments
 (0)