From 05be2855d86f3070aceb1c67be67b424ddec7050 Mon Sep 17 00:00:00 2001 From: Adetunji Dahunsi Date: Tue, 4 Apr 2023 09:24:32 -0400 Subject: [PATCH 1/6] Wire up backend requested sync Change-Id: I1d4485b589c7e94527a2a02f371cd3f030231622 --- .../repository/OfflineFirstNewsRepository.kt | 49 ++++---- .../OfflineFirstNewsRepositoryTest.kt | 106 ++++++++++++++---- .../data/testdoubles/TestNewsResourceDao.kt | 36 +++--- .../core/database/dao/NewsResourceDao.kt | 7 -- .../apps/nowinandroid/sync/di/SyncModule.kt | 7 ++ .../sync/initializers/SyncWorkHelpers.kt | 1 + .../sync/services/SyncNotificationsService.kt | 3 +- .../sync/status/StubSyncSubscriber.kt | 31 +++++ .../sync/status/SyncSubscriber.kt | 24 ++++ .../nowinandroid/sync/workers/SyncWorker.kt | 4 + .../apps/nowinandroid/sync/di/SyncModule.kt | 51 +++++++++ .../sync/status/FirebaseSyncSubscriber.kt | 35 ++++++ 12 files changed, 291 insertions(+), 63 deletions(-) rename sync/work/src/{main => demo}/java/com/google/samples/apps/nowinandroid/sync/di/SyncModule.kt (81%) create mode 100644 sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/status/StubSyncSubscriber.kt create mode 100644 sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/status/SyncSubscriber.kt create mode 100644 sync/work/src/prod/java/com/google/samples/apps/nowinandroid/sync/di/SyncModule.kt create mode 100644 sync/work/src/prod/java/com/google/samples/apps/nowinandroid/sync/status/FirebaseSyncSubscriber.kt 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 02c58d855..27590b0b7 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 @@ -27,6 +27,7 @@ import com.google.samples.apps.nowinandroid.core.database.model.PopulatedNewsRes import com.google.samples.apps.nowinandroid.core.database.model.TopicEntity import com.google.samples.apps.nowinandroid.core.database.model.asExternalModel import com.google.samples.apps.nowinandroid.core.datastore.ChangeListVersions +import com.google.samples.apps.nowinandroid.core.datastore.NiaPreferencesDataSource import com.google.samples.apps.nowinandroid.core.model.data.NewsResource import com.google.samples.apps.nowinandroid.core.network.NiaNetworkDataSource import com.google.samples.apps.nowinandroid.core.network.model.NetworkNewsResource @@ -45,6 +46,7 @@ private const val SYNC_BATCH_SIZE = 40 * Reads are exclusively from local storage to support offline access. */ class OfflineFirstNewsRepository @Inject constructor( + private val niaPreferencesDataSource: NiaPreferencesDataSource, private val newsResourceDao: NewsResourceDao, private val topicDao: TopicDao, private val network: NiaNetworkDataSource, @@ -72,15 +74,25 @@ class OfflineFirstNewsRepository @Inject constructor( }, modelDeleter = newsResourceDao::deleteNewsResources, modelUpdater = { changedIds -> + val userData = niaPreferencesDataSource.userData.first() + val hasOnBoarded = userData.shouldHideOnboarding + val followedTopicIds = userData.followedTopics + // TODO: Make this more efficient, there is no need to retrieve populated // news resources when all that's needed are the ids - val existingNewsResourceIds = newsResourceDao.getNewsResources( - useFilterNewsIds = true, - filterNewsIds = changedIds.toSet(), - ) - .first() - .map { it.entity.id } - .toSet() + val existingFollowedChangedNewsResourceIds = when { + hasOnBoarded -> newsResourceDao.getNewsResources( + useFilterTopicIds = true, + filterTopicIds = followedTopicIds, + useFilterNewsIds = true, + filterNewsIds = changedIds.toSet(), + ) + .first() + .map { it.entity.id } + .toSet() + // No need to retrieve anything if notifications won't be sent + else -> emptySet() + } changedIds.chunked(SYNC_BATCH_SIZE).forEach { chunkedIds -> val networkNewsResources = network.getNewsResources(ids = chunkedIds) @@ -106,19 +118,18 @@ class OfflineFirstNewsRepository @Inject constructor( ) } - val addedNewsResources = newsResourceDao.getNewsResources( - useFilterNewsIds = true, - filterNewsIds = changedIds.toSet(), - ) - .first() - .filter { !existingNewsResourceIds.contains(it.entity.id) } - .map(PopulatedNewsResource::asExternalModel) + if (hasOnBoarded) { + val addedNewsResources = newsResourceDao.getNewsResources( + useFilterTopicIds = true, + filterTopicIds = followedTopicIds, + useFilterNewsIds = true, + filterNewsIds = changedIds.toSet() - existingFollowedChangedNewsResourceIds, + ) + .first() + .map(PopulatedNewsResource::asExternalModel) - // TODO: Define business logic for notifications on first time sync. - // we probably do not want to send notifications on first install. - // We can easily check if the change list version is 0 and not send notifications - // if it is. - if (addedNewsResources.isNotEmpty()) notifier.onNewsAdded(addedNewsResources) + if (addedNewsResources.isNotEmpty()) notifier.onNewsAdded(addedNewsResources) + } }, ) } 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 6cdbf67d0..9e6834f8f 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 @@ -34,6 +34,7 @@ import com.google.samples.apps.nowinandroid.core.database.model.asExternalModel import com.google.samples.apps.nowinandroid.core.datastore.NiaPreferencesDataSource import com.google.samples.apps.nowinandroid.core.datastore.test.testUserPreferencesDataStore 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.network.model.NetworkChangeList import com.google.samples.apps.nowinandroid.core.network.model.NetworkNewsResource import com.google.samples.apps.nowinandroid.core.testing.notifications.TestNotifier @@ -46,6 +47,7 @@ import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder import kotlin.test.assertEquals +import kotlin.test.assertTrue class OfflineFirstNewsRepositoryTest { @@ -53,6 +55,8 @@ class OfflineFirstNewsRepositoryTest { private lateinit var subject: OfflineFirstNewsRepository + private lateinit var niaPreferencesDataSource: NiaPreferencesDataSource + private lateinit var newsResourceDao: TestNewsResourceDao private lateinit var topicDao: TestTopicDao @@ -68,17 +72,19 @@ class OfflineFirstNewsRepositoryTest { @Before fun setup() { + niaPreferencesDataSource = NiaPreferencesDataSource( + tmpFolder.testUserPreferencesDataStore(testScope), + ) newsResourceDao = TestNewsResourceDao() topicDao = TestTopicDao() network = TestNiaNetworkDataSource() notifier = TestNotifier() synchronizer = TestSynchronizer( - NiaPreferencesDataSource( - tmpFolder.testUserPreferencesDataStore(testScope), - ), + niaPreferencesDataSource, ) subject = OfflineFirstNewsRepository( + niaPreferencesDataSource = niaPreferencesDataSource, newsResourceDao = newsResourceDao, topicDao = topicDao, network = network, @@ -130,6 +136,8 @@ class OfflineFirstNewsRepositoryTest { @Test fun offlineFirstNewsRepository_sync_pulls_from_network() = testScope.runTest { + // User has not onboarded + niaPreferencesDataSource.setShouldHideOnboarding(false) subject.syncWith(synchronizer) val newsResourcesFromNetwork = network.getNewsResources() @@ -151,16 +159,16 @@ class OfflineFirstNewsRepositoryTest { actual = synchronizer.getChangeListVersions().newsResourceVersion, ) - // Notifier should have been called with new news resources - assertEquals( - expected = newsResourcesFromDb.map(NewsResource::id).sorted(), - actual = notifier.addedNewsResources.first().map(NewsResource::id).sorted(), - ) + // Notifier should not have been called + assertTrue(notifier.addedNewsResources.isEmpty()) } @Test fun offlineFirstNewsRepository_sync_deletes_items_marked_deleted_on_network() = testScope.runTest { + // User has not onboarded + niaPreferencesDataSource.setShouldHideOnboarding(false) + val newsResourcesFromNetwork = network.getNewsResources() .map(NetworkNewsResource::asEntity) .map(NewsResourceEntity::asExternalModel) @@ -198,17 +206,16 @@ class OfflineFirstNewsRepositoryTest { actual = synchronizer.getChangeListVersions().newsResourceVersion, ) - // Notifier should have been called with news resources from network that are not - // deleted - assertEquals( - expected = (newsResourcesFromNetwork.map(NewsResource::id) - deletedItems).sorted(), - actual = notifier.addedNewsResources.first().map(NewsResource::id).sorted(), - ) + // Notifier should not have been called + assertTrue(notifier.addedNewsResources.isEmpty()) } @Test fun offlineFirstNewsRepository_incremental_sync_pulls_from_network() = testScope.runTest { + // User has not onboarded + niaPreferencesDataSource.setShouldHideOnboarding(false) + // Set news version to 7 synchronizer.updateChangeListVersions { copy(newsResourceVersion = 7) @@ -244,11 +251,8 @@ class OfflineFirstNewsRepositoryTest { actual = synchronizer.getChangeListVersions().newsResourceVersion, ) - // Notifier should have been called with only added news resources from network - assertEquals( - expected = newsResourcesFromNetwork.map(NewsResource::id).sorted(), - actual = notifier.addedNewsResources.first().map(NewsResource::id).sorted(), - ) + // Notifier should not have been called + assertTrue(notifier.addedNewsResources.isEmpty()) } @Test @@ -283,4 +287,68 @@ class OfflineFirstNewsRepositoryTest { .sortedBy(NewsResourceTopicCrossRef::toString), ) } + + @Test + fun offlineFirstNewsRepository_sends_notifications_for_newly_synced_news_that_is_followed() = + testScope.runTest { + // User has onboarded + niaPreferencesDataSource.setShouldHideOnboarding(true) + + val networkNewsResources = network.getNewsResources() + + // Follow roughly half the topics + val followedTopicIds = networkNewsResources + .flatMap(NetworkNewsResource::topicEntityShells) + .mapNotNull { topic -> + when (topic.id.chars().sum() % 2) { + 0 -> topic.id + else -> null + } + } + .toSet() + + // Set followed topics + niaPreferencesDataSource.setFollowedTopicIds(followedTopicIds) + + subject.syncWith(synchronizer) + + // Notifier should have been called with only news resources that have topics + // that the user follows + assertEquals( + expected = networkNewsResources + .filter { (it.topics intersect followedTopicIds).isNotEmpty() } + .map(NetworkNewsResource::id) + .sorted(), + actual = notifier.addedNewsResources.first().map(NewsResource::id).sorted(), + ) + } + + @Test + fun offlineFirstNewsRepository_does_not_send_notifications_for_existing_news_resources() = + testScope.runTest { + // User has onboarded + niaPreferencesDataSource.setShouldHideOnboarding(true) + + val networkNewsResources = network.getNewsResources() + .map(NetworkNewsResource::asEntity) + + val newsResources = networkNewsResources + .map(NewsResourceEntity::asExternalModel) + + // Prepopulate dao with news resources + newsResourceDao.upsertNewsResources(networkNewsResources) + + val followedTopicIds = newsResources + .flatMap(NewsResource::topics) + .map(Topic::id) + .toSet() + + // Follow all topics + niaPreferencesDataSource.setFollowedTopicIds(followedTopicIds) + + subject.syncWith(synchronizer) + + // Notifier should not have been called bc all news resources existed previously + assertTrue(notifier.addedNewsResources.isEmpty()) + } } 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 bb1ac20ab..d5d8932e7 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 @@ -47,7 +47,11 @@ class TestNewsResourceDao : NewsResourceDao { filterNewsIds: Set, ): Flow> = entitiesStateFlow - .map { it.map(NewsResourceEntity::asPopulatedNewsResource) } + .map { newsResourceEntities -> + newsResourceEntities.map { entity -> + entity.asPopulatedNewsResource(topicCrossReferences) + } + } .map { resources -> var result = resources if (useFilterTopicIds) { @@ -78,10 +82,6 @@ class TestNewsResourceDao : NewsResourceDao { return entities.map { it.id.toLong() } } - override suspend fun updateNewsResources(entities: List) { - throw NotImplementedError("Unused in tests") - } - override suspend fun upsertNewsResources(newsResourceEntities: List) { entitiesStateFlow.update { oldValues -> // New values come first so they overwrite old values @@ -109,16 +109,20 @@ class TestNewsResourceDao : NewsResourceDao { } } -private fun NewsResourceEntity.asPopulatedNewsResource() = PopulatedNewsResource( +private fun NewsResourceEntity.asPopulatedNewsResource( + topicCrossReferences: List, +) = PopulatedNewsResource( entity = this, - topics = listOf( - TopicEntity( - id = filteredInterestsIds.random(), - name = "name", - shortDescription = "short description", - longDescription = "long description", - url = "URL", - imageUrl = "image URL", - ), - ), + topics = topicCrossReferences + .filter { it.newsResourceId == id } + .map { newsResourceTopicCrossRef -> + TopicEntity( + id = newsResourceTopicCrossRef.topicId, + name = "name", + shortDescription = "short description", + longDescription = "long description", + url = "URL", + imageUrl = "image URL", + ) + }, ) diff --git a/core/database/src/main/java/com/google/samples/apps/nowinandroid/core/database/dao/NewsResourceDao.kt b/core/database/src/main/java/com/google/samples/apps/nowinandroid/core/database/dao/NewsResourceDao.kt index 782e5c87a..a05507a8b 100644 --- a/core/database/src/main/java/com/google/samples/apps/nowinandroid/core/database/dao/NewsResourceDao.kt +++ b/core/database/src/main/java/com/google/samples/apps/nowinandroid/core/database/dao/NewsResourceDao.kt @@ -21,7 +21,6 @@ import androidx.room.Insert import androidx.room.OnConflictStrategy import androidx.room.Query import androidx.room.Transaction -import androidx.room.Update import androidx.room.Upsert import com.google.samples.apps.nowinandroid.core.database.model.NewsResourceEntity import com.google.samples.apps.nowinandroid.core.database.model.NewsResourceTopicCrossRef @@ -72,12 +71,6 @@ interface NewsResourceDao { @Insert(onConflict = OnConflictStrategy.IGNORE) suspend fun insertOrIgnoreNewsResources(entities: List): List - /** - * Updates [entities] in the db that match the primary key, and no-ops if they don't - */ - @Update - suspend fun updateNewsResources(entities: List) - /** * Inserts or updates [newsResourceEntities] in the db under the specified primary keys */ diff --git a/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/di/SyncModule.kt b/sync/work/src/demo/java/com/google/samples/apps/nowinandroid/sync/di/SyncModule.kt similarity index 81% rename from sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/di/SyncModule.kt rename to sync/work/src/demo/java/com/google/samples/apps/nowinandroid/sync/di/SyncModule.kt index bbc45dc42..40d094cd2 100644 --- a/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/di/SyncModule.kt +++ b/sync/work/src/demo/java/com/google/samples/apps/nowinandroid/sync/di/SyncModule.kt @@ -17,6 +17,8 @@ package com.google.samples.apps.nowinandroid.sync.di import com.google.samples.apps.nowinandroid.core.data.util.SyncManager +import com.google.samples.apps.nowinandroid.sync.status.StubSyncSubscriber +import com.google.samples.apps.nowinandroid.sync.status.SyncSubscriber import com.google.samples.apps.nowinandroid.sync.status.WorkManagerSyncManager import dagger.Binds import dagger.Module @@ -30,4 +32,9 @@ interface SyncModule { fun bindsSyncStatusMonitor( syncStatusMonitor: WorkManagerSyncManager, ): SyncManager + + @Binds + fun bindsSyncSubscriber( + syncSubscriber: StubSyncSubscriber, + ): SyncSubscriber } diff --git a/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/initializers/SyncWorkHelpers.kt b/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/initializers/SyncWorkHelpers.kt index 334b3f0c7..a3cff5fb9 100644 --- a/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/initializers/SyncWorkHelpers.kt +++ b/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/initializers/SyncWorkHelpers.kt @@ -27,6 +27,7 @@ import androidx.work.ForegroundInfo import androidx.work.NetworkType import com.google.samples.apps.nowinandroid.sync.R +const val SYNC_TOPIC = "sync" private const val SyncNotificationId = 0 private const val SyncNotificationChannelID = "SyncNotificationChannel" diff --git a/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/services/SyncNotificationsService.kt b/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/services/SyncNotificationsService.kt index ab318776a..1d182dda1 100644 --- a/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/services/SyncNotificationsService.kt +++ b/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/services/SyncNotificationsService.kt @@ -19,11 +19,10 @@ package com.google.samples.apps.nowinandroid.sync.services import com.google.firebase.messaging.FirebaseMessagingService import com.google.firebase.messaging.RemoteMessage import com.google.samples.apps.nowinandroid.core.data.util.SyncManager +import com.google.samples.apps.nowinandroid.sync.initializers.SYNC_TOPIC import dagger.hilt.android.AndroidEntryPoint import javax.inject.Inject -private const val SYNC_TOPIC = "sync" - @AndroidEntryPoint class SyncNotificationsService : FirebaseMessagingService() { diff --git a/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/status/StubSyncSubscriber.kt b/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/status/StubSyncSubscriber.kt new file mode 100644 index 000000000..0ef90fb29 --- /dev/null +++ b/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/status/StubSyncSubscriber.kt @@ -0,0 +1,31 @@ +/* + * 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.sync.status + +import android.util.Log +import javax.inject.Inject + +private const val TAG = "StubSyncSubscriber" + +/** + * Stub implementation of [SyncSubscriber] + */ +class StubSyncSubscriber @Inject constructor() : SyncSubscriber { + override suspend fun subscribe() { + Log.d(TAG, "Subscribing to sync") + } +} diff --git a/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/status/SyncSubscriber.kt b/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/status/SyncSubscriber.kt new file mode 100644 index 000000000..b1845b070 --- /dev/null +++ b/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/status/SyncSubscriber.kt @@ -0,0 +1,24 @@ +/* + * 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.sync.status + +/** + * Subscribes to backend requested synchronization + */ +interface SyncSubscriber { + suspend fun subscribe() +} diff --git a/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/workers/SyncWorker.kt b/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/workers/SyncWorker.kt index 211940ddb..d8f1ef91c 100644 --- a/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/workers/SyncWorker.kt +++ b/sync/work/src/main/java/com/google/samples/apps/nowinandroid/sync/workers/SyncWorker.kt @@ -34,6 +34,7 @@ import com.google.samples.apps.nowinandroid.core.network.Dispatcher import com.google.samples.apps.nowinandroid.core.network.NiaDispatchers.IO import com.google.samples.apps.nowinandroid.sync.initializers.SyncConstraints import com.google.samples.apps.nowinandroid.sync.initializers.syncForegroundInfo +import com.google.samples.apps.nowinandroid.sync.status.SyncSubscriber import dagger.assisted.Assisted import dagger.assisted.AssistedInject import kotlinx.coroutines.CoroutineDispatcher @@ -54,6 +55,7 @@ class SyncWorker @AssistedInject constructor( private val newsRepository: NewsRepository, @Dispatcher(IO) private val ioDispatcher: CoroutineDispatcher, private val analyticsHelper: AnalyticsHelper, + private val syncSubscriber: SyncSubscriber, ) : CoroutineWorker(appContext, workerParams), Synchronizer { override suspend fun getForegroundInfo(): ForegroundInfo = @@ -63,6 +65,8 @@ class SyncWorker @AssistedInject constructor( traceAsync("Sync", 0) { analyticsHelper.logSyncStarted() + syncSubscriber.subscribe() + // First sync the repositories in parallel val syncedSuccessfully = awaitAll( async { topicRepository.sync() }, diff --git a/sync/work/src/prod/java/com/google/samples/apps/nowinandroid/sync/di/SyncModule.kt b/sync/work/src/prod/java/com/google/samples/apps/nowinandroid/sync/di/SyncModule.kt new file mode 100644 index 000000000..af4508406 --- /dev/null +++ b/sync/work/src/prod/java/com/google/samples/apps/nowinandroid/sync/di/SyncModule.kt @@ -0,0 +1,51 @@ +/* + * 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.sync.di + +import com.google.firebase.ktx.Firebase +import com.google.firebase.messaging.FirebaseMessaging +import com.google.firebase.messaging.ktx.messaging +import com.google.samples.apps.nowinandroid.core.data.util.SyncManager +import com.google.samples.apps.nowinandroid.sync.status.FirebaseSyncSubscriber +import com.google.samples.apps.nowinandroid.sync.status.SyncSubscriber +import com.google.samples.apps.nowinandroid.sync.status.WorkManagerSyncManager +import dagger.Binds +import dagger.Module +import dagger.Provides +import dagger.hilt.InstallIn +import dagger.hilt.components.SingletonComponent +import javax.inject.Singleton + +@Module +@InstallIn(SingletonComponent::class) +interface SyncModule { + @Binds + fun bindsSyncStatusMonitor( + syncStatusMonitor: WorkManagerSyncManager, + ): SyncManager + + @Binds + fun bindsSyncSubscriber( + syncSubscriber: FirebaseSyncSubscriber, + ): SyncSubscriber + + companion object { + @Provides + @Singleton + fun provideFirebaseMessaging(): FirebaseMessaging = Firebase.messaging + } +} diff --git a/sync/work/src/prod/java/com/google/samples/apps/nowinandroid/sync/status/FirebaseSyncSubscriber.kt b/sync/work/src/prod/java/com/google/samples/apps/nowinandroid/sync/status/FirebaseSyncSubscriber.kt new file mode 100644 index 000000000..c2405bccc --- /dev/null +++ b/sync/work/src/prod/java/com/google/samples/apps/nowinandroid/sync/status/FirebaseSyncSubscriber.kt @@ -0,0 +1,35 @@ +/* + * 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.sync.status + +import com.google.firebase.messaging.FirebaseMessaging +import com.google.samples.apps.nowinandroid.sync.initializers.SYNC_TOPIC +import kotlinx.coroutines.tasks.await +import javax.inject.Inject + +/** + * Implementation of [SyncSubscriber] that subscribes to the FCM [SYNC_TOPIC] + */ +class FirebaseSyncSubscriber @Inject constructor( + private val firebaseMessaging: FirebaseMessaging, +) : SyncSubscriber { + override suspend fun subscribe() { + firebaseMessaging + .subscribeToTopic(SYNC_TOPIC) + .await() + } +} From 022cd92f4c23a884dbad517965d17b3dd09f2f1d Mon Sep 17 00:00:00 2001 From: Adetunji Dahunsi Date: Tue, 25 Apr 2023 05:38:24 -0400 Subject: [PATCH 2/6] Update core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstNewsRepository.kt Co-authored-by: Don Turner --- .../core/data/repository/OfflineFirstNewsRepository.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 27590b0b7..5467deb73 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 @@ -75,7 +75,7 @@ class OfflineFirstNewsRepository @Inject constructor( modelDeleter = newsResourceDao::deleteNewsResources, modelUpdater = { changedIds -> val userData = niaPreferencesDataSource.userData.first() - val hasOnBoarded = userData.shouldHideOnboarding + val hasOnboarded = userData.shouldHideOnboarding val followedTopicIds = userData.followedTopics // TODO: Make this more efficient, there is no need to retrieve populated From 337c940d66b18823807679bb40bba01316ab5d09 Mon Sep 17 00:00:00 2001 From: Adetunji Dahunsi Date: Tue, 25 Apr 2023 05:41:30 -0400 Subject: [PATCH 3/6] Add comments explaining backend sync and made variable names easier to grok Change-Id: I90cd7444de95efa20bf243a922a772f7849a23ec --- .../core/data/repository/OfflineFirstNewsRepository.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 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 5467deb73..3e22103b9 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 @@ -80,8 +80,8 @@ class OfflineFirstNewsRepository @Inject constructor( // TODO: Make this more efficient, there is no need to retrieve populated // news resources when all that's needed are the ids - val existingFollowedChangedNewsResourceIds = when { - hasOnBoarded -> newsResourceDao.getNewsResources( + val existingNewsResourceIdsThatHaveChanged = when { + hasOnboarded -> newsResourceDao.getNewsResources( useFilterTopicIds = true, filterTopicIds = followedTopicIds, useFilterNewsIds = true, @@ -94,6 +94,7 @@ class OfflineFirstNewsRepository @Inject constructor( else -> emptySet() } + // Obtain the news resources which have changed from the network and upsert them locally changedIds.chunked(SYNC_BATCH_SIZE).forEach { chunkedIds -> val networkNewsResources = network.getNewsResources(ids = chunkedIds) @@ -118,12 +119,12 @@ class OfflineFirstNewsRepository @Inject constructor( ) } - if (hasOnBoarded) { + if (hasOnboarded) { val addedNewsResources = newsResourceDao.getNewsResources( useFilterTopicIds = true, filterTopicIds = followedTopicIds, useFilterNewsIds = true, - filterNewsIds = changedIds.toSet() - existingFollowedChangedNewsResourceIds, + filterNewsIds = changedIds.toSet() - existingNewsResourceIdsThatHaveChanged, ) .first() .map(PopulatedNewsResource::asExternalModel) From f86d174c1bb235bc4bfff9f424013545e4d4b23d Mon Sep 17 00:00:00 2001 From: Adetunji Dahunsi Date: Tue, 25 Apr 2023 05:44:59 -0400 Subject: [PATCH 4/6] Used more explicit variable names in tests Change-Id: I2e0ebead70441eced05eb786aaa7683bfefa8976 --- .../data/repository/OfflineFirstNewsRepositoryTest.kt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) 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 9e6834f8f..d3c1851f9 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 @@ -312,13 +312,15 @@ class OfflineFirstNewsRepositoryTest { subject.syncWith(synchronizer) + val followedNewsResourcesFromNetwork = networkNewsResources + .filter { (it.topics intersect followedTopicIds).isNotEmpty() } + .map(NetworkNewsResource::id) + .sorted() + // Notifier should have been called with only news resources that have topics // that the user follows assertEquals( - expected = networkNewsResources - .filter { (it.topics intersect followedTopicIds).isNotEmpty() } - .map(NetworkNewsResource::id) - .sorted(), + expected = followedNewsResourcesFromNetwork, actual = notifier.addedNewsResources.first().map(NewsResource::id).sorted(), ) } From b3f2502ec2846f3c27840c06e20c2062faf836d8 Mon Sep 17 00:00:00 2001 From: Adetunji Dahunsi Date: Tue, 25 Apr 2023 06:58:50 -0400 Subject: [PATCH 5/6] Update core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstNewsRepositoryTest.kt Co-authored-by: Don Turner --- .../core/data/repository/OfflineFirstNewsRepositoryTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d3c1851f9..c24918475 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 @@ -312,7 +312,7 @@ class OfflineFirstNewsRepositoryTest { subject.syncWith(synchronizer) - val followedNewsResourcesFromNetwork = networkNewsResources + val followedNewsResourceIdsFromNetwork = networkNewsResources .filter { (it.topics intersect followedTopicIds).isNotEmpty() } .map(NetworkNewsResource::id) .sorted() From 6b834b6f4c93c8b92e457ba96bd3808bdea0e05e Mon Sep 17 00:00:00 2001 From: Adetunji Dahunsi Date: Tue, 25 Apr 2023 06:59:34 -0400 Subject: [PATCH 6/6] Applied code review suggestions Change-Id: I12fab8e0d27a8a1805e0063054ec3382bebd78f1 --- .../core/data/repository/OfflineFirstNewsRepositoryTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c24918475..a38d9c621 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 @@ -320,7 +320,7 @@ class OfflineFirstNewsRepositoryTest { // Notifier should have been called with only news resources that have topics // that the user follows assertEquals( - expected = followedNewsResourcesFromNetwork, + expected = followedNewsResourceIdsFromNetwork, actual = notifier.addedNewsResources.first().map(NewsResource::id).sorted(), ) }