diff --git a/app/src/main/java/org/groundplatform/android/data/local/room/dao/LocationOfInterestMutationDao.kt b/app/src/main/java/org/groundplatform/android/data/local/room/dao/LocationOfInterestMutationDao.kt index 3e5cf13b99..55a5ed11e5 100644 --- a/app/src/main/java/org/groundplatform/android/data/local/room/dao/LocationOfInterestMutationDao.kt +++ b/app/src/main/java/org/groundplatform/android/data/local/room/dao/LocationOfInterestMutationDao.kt @@ -41,16 +41,16 @@ interface LocationOfInterestMutationDao : BaseDao - /** Returns how many of the survey's LOIs hold a mutation of another type in one of the states. */ + /** Returns how many of the survey's LOIs have a [type] mutation in one of [allowedStates]. */ @Query( "SELECT COUNT(DISTINCT location_of_interest_id) FROM location_of_interest_mutation " + "WHERE survey_id = :surveyId " + - "AND type != :excludedType " + + "AND type = :type " + "AND state IN (:allowedStates)" ) suspend fun countLocationOfInterestIds( surveyId: String, - excludedType: MutationEntityType, + type: MutationEntityType, vararg allowedStates: MutationEntitySyncStatus, ): Int } diff --git a/app/src/main/java/org/groundplatform/android/data/local/room/stores/RoomLocationOfInterestStore.kt b/app/src/main/java/org/groundplatform/android/data/local/room/stores/RoomLocationOfInterestStore.kt index 49a8728d0c..d510b844a3 100644 --- a/app/src/main/java/org/groundplatform/android/data/local/room/stores/RoomLocationOfInterestStore.kt +++ b/app/src/main/java/org/groundplatform/android/data/local/room/stores/RoomLocationOfInterestStore.kt @@ -115,6 +115,19 @@ class RoomLocationOfInterestStore @Inject internal constructor() : LocalLocation locationOfInterestDao.findById(locationOfInterestId)?.let { locationOfInterestDao.delete(it) } } + override suspend fun safeDeleteLocalLoi(locationOfInterestId: String) { + // One transaction, so a mutation saved right now can't be cascaded away by the delete. + localDatabase.withTransaction { + val pending = + locationOfInterestMutationDao.getMutations( + locationOfInterestId, + MutationEntitySyncStatus.PENDING, + MutationEntitySyncStatus.IN_PROGRESS, + ) + if (pending.isEmpty()) deleteLocationOfInterest(locationOfInterestId) + } + } + override fun getAllSurveyMutations(survey: Survey): Flow> = locationOfInterestMutationDao.getAllMutationsFlow().map { mutations -> mutations.filter { it.surveyId == survey.id }.map { it.toModelObject() } @@ -144,15 +157,15 @@ class RoomLocationOfInterestStore @Inject internal constructor() : LocalLocation locationOfInterestDao.upsertAll(entities) } - override suspend fun countPendingNonDeletedLois(surveyId: String): Int = + override suspend fun countPendingCreatedLois(surveyId: String): Int = locationOfInterestMutationDao.countLocationOfInterestIds( surveyId, - MutationEntityType.DELETE, + MutationEntityType.CREATE, MutationEntitySyncStatus.PENDING, MutationEntitySyncStatus.IN_PROGRESS, ) - override suspend fun deleteNotIn(surveyId: String, ids: List) { + override suspend fun deleteNotIn(surveyId: String, ids: Collection) { val idsToKeep = ids.toSet() localDatabase.withTransaction { locationOfInterestDao diff --git a/app/src/main/java/org/groundplatform/android/data/local/room/stores/RoomSurveySyncStateStore.kt b/app/src/main/java/org/groundplatform/android/data/local/room/stores/RoomSurveySyncStateStore.kt index 716833a526..8db8e6d8b8 100644 --- a/app/src/main/java/org/groundplatform/android/data/local/room/stores/RoomSurveySyncStateStore.kt +++ b/app/src/main/java/org/groundplatform/android/data/local/room/stores/RoomSurveySyncStateStore.kt @@ -17,42 +17,35 @@ package org.groundplatform.android.data.local.room.stores import javax.inject.Inject import kotlin.time.Clock +import org.groundplatform.android.data.local.room.converter.toLocalDataStoreObject import org.groundplatform.android.data.local.room.converter.toModelObject import org.groundplatform.android.data.local.room.dao.SurveySyncStateDao import org.groundplatform.android.data.local.room.dao.insertOrUpdate -import org.groundplatform.android.data.local.room.entity.SurveySyncStateEntity import org.groundplatform.android.data.local.stores.LocalSurveySyncStateStore -import org.groundplatform.android.data.remote.firebase.protobuf.toProto import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.SurveySyncState class RoomSurveySyncStateStore @Inject constructor(private val surveySyncStateDao: SurveySyncStateDao) : LocalSurveySyncStateStore { - override suspend fun get(surveyId: String): SurveySyncState? { - val entity = surveySyncStateDao.get(surveyId) - return entity?.toModelObject() - } + override suspend fun get(surveyId: String): SurveySyncState? = + surveySyncStateDao.get(surveyId)?.toModelObject() - override suspend fun recordIncrementalSync( - surveyId: String, - latestLoiServerTimestamp: Long, - ) { + override suspend fun recordIncrementalSync(surveyId: String, latestLoiServerTimestamp: Long) = surveySyncStateDao.updateLatestLoiServerTimestamp(surveyId, latestLoiServerTimestamp) - } override suspend fun recordFullSync( surveyId: String, latestLoiServerTimestamp: Long, dataVisibility: Survey.DataVisibility?, - ) { + ) = surveySyncStateDao.insertOrUpdate( - SurveySyncStateEntity( - surveyId = surveyId, - latestLoiServerTimestamp = latestLoiServerTimestamp, - lastFullSyncClientTimestamp = Clock.System.now().toEpochMilliseconds(), - syncedDataVisibility = dataVisibility?.toProto()?.ordinal, - ) + SurveySyncState( + surveyId = surveyId, + latestLoiServerTimestamp = latestLoiServerTimestamp, + lastFullSyncClientTimestamp = Clock.System.now().toEpochMilliseconds(), + syncedDataVisibility = dataVisibility, + ) + .toLocalDataStoreObject() ) - } } diff --git a/app/src/main/java/org/groundplatform/android/data/local/stores/LocalLocationOfInterestStore.kt b/app/src/main/java/org/groundplatform/android/data/local/stores/LocalLocationOfInterestStore.kt index f86215a65a..23c0d089f8 100644 --- a/app/src/main/java/org/groundplatform/android/data/local/stores/LocalLocationOfInterestStore.kt +++ b/app/src/main/java/org/groundplatform/android/data/local/stores/LocalLocationOfInterestStore.kt @@ -46,6 +46,9 @@ interface LocalLocationOfInterestStore : /** Deletes LOI from local database. */ suspend fun deleteLocationOfInterest(locationOfInterestId: String) + /** Deletes LOI from local database, keeping it if it has changes still waiting to upload. */ + suspend fun safeDeleteLocalLoi(locationOfInterestId: String) + /** * Returns a [Flow] that emits a [List] of all [LocationOfInterestMutation]s stored in the local * db related to a given [Survey]. A new [List] is emitted on each change to the underlying saved @@ -65,11 +68,8 @@ interface LocalLocationOfInterestStore : /** Inserts or updates all the given LOIs in a single transaction. */ suspend fun insertOrUpdateAll(lois: List) - suspend fun deleteNotIn(surveyId: String, ids: List) + suspend fun deleteNotIn(surveyId: String, ids: Collection) - /** - * Returns the number of survey LOIs with a pending local change that has not yet been synced, - * excluding deletes. - */ - suspend fun countPendingNonDeletedLois(surveyId: String): Int + /** Returns how many of the survey's LOIs were created locally and not uploaded yet. */ + suspend fun countPendingCreatedLois(surveyId: String): Int } diff --git a/app/src/main/java/org/groundplatform/android/data/remote/RemoteDataStore.kt b/app/src/main/java/org/groundplatform/android/data/remote/RemoteDataStore.kt index b791d91934..b4d6b27d9e 100644 --- a/app/src/main/java/org/groundplatform/android/data/remote/RemoteDataStore.kt +++ b/app/src/main/java/org/groundplatform/android/data/remote/RemoteDataStore.kt @@ -50,15 +50,22 @@ interface RemoteDataStore { suspend fun loadTermsOfService(): TermsOfService? /** Returns predefined LOIs in the specified survey. Main-safe. */ - fun loadPredefinedLois(survey: Survey): Flow> + fun loadPredefinedLois(survey: Survey, fromTimestamp: Long?): Flow> /** Returns LOIs owned by the specified user in the specified survey. Main-safe. */ - fun loadUserLois(survey: Survey, ownerUserId: String): Flow> + fun loadUserLois( + survey: Survey, + ownerUserId: String, + fromTimestamp: Long?, + ): Flow> /** * Returns LOIs that have been marked as shared for other participants of the specified survey. */ - fun loadSharedLois(survey: Survey): Flow> + fun loadSharedLois(survey: Survey, fromTimestamp: Long?): Flow> + + /** Returns how many LOIs a sync of the specified survey would fetch. Main-safe. */ + suspend fun countLois(survey: Survey, ownerUserId: String): Long /** * Applies the provided mutations to the remote data store in a single batched transaction. If one diff --git a/app/src/main/java/org/groundplatform/android/data/remote/firebase/FirebaseMessagingService.kt b/app/src/main/java/org/groundplatform/android/data/remote/firebase/FirebaseMessagingService.kt index 75cf2c1c3b..2533be5f7d 100644 --- a/app/src/main/java/org/groundplatform/android/data/remote/firebase/FirebaseMessagingService.kt +++ b/app/src/main/java/org/groundplatform/android/data/remote/firebase/FirebaseMessagingService.kt @@ -20,11 +20,18 @@ import com.google.firebase.messaging.FirebaseMessagingService import com.google.firebase.messaging.RemoteMessage import dagger.hilt.android.AndroidEntryPoint import javax.inject.Inject +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch +import org.groundplatform.android.data.local.stores.LocalLocationOfInterestStore import org.groundplatform.android.data.sync.SurveySyncService +import org.groundplatform.android.di.coroutines.ApplicationScope import timber.log.Timber const val TOPIC_PREFIX = "/topics/" +private const val LOI_ID_KEY = "loiId" +private const val DELETED_KEY = "deleted" + /** * Listens to messages from Firebase Cloud Messaging, and enqueuing re-sync of survey metadata when * receiving. @@ -33,6 +40,8 @@ const val TOPIC_PREFIX = "/topics/" class FirebaseMessagingService : FirebaseMessagingService() { @Inject lateinit var surveySyncService: SurveySyncService + @Inject lateinit var localLoiStore: LocalLocationOfInterestStore + @Inject @ApplicationScope lateinit var externalScope: CoroutineScope /** * Processes new messages, enqueuing a worker to sync the survey with the id specified in the @@ -45,6 +54,12 @@ class FirebaseMessagingService : FirebaseMessagingService() { return } Timber.v("Message received from topic ${remoteMessage.from}") + + // Dropping it here spares the sync the full read it would take to notice the deletion. + remoteMessage.data[LOI_ID_KEY] + ?.takeIf { remoteMessage.data[DELETED_KEY].toBoolean() } + ?.let { externalScope.launch { localLoiStore.safeDeleteLocalLoi(it) } } + surveySyncService.enqueueSync(surveyId) } diff --git a/app/src/main/java/org/groundplatform/android/data/remote/firebase/FirestoreDataStore.kt b/app/src/main/java/org/groundplatform/android/data/remote/firebase/FirestoreDataStore.kt index 1c7addd08d..2af652440a 100644 --- a/app/src/main/java/org/groundplatform/android/data/remote/firebase/FirestoreDataStore.kt +++ b/app/src/main/java/org/groundplatform/android/data/remote/firebase/FirestoreDataStore.kt @@ -23,6 +23,8 @@ import javax.inject.Inject import javax.inject.Singleton import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.buffer import kotlinx.coroutines.flow.emitAll @@ -35,6 +37,7 @@ import org.groundplatform.android.BuildConfig.USE_EMULATORS import org.groundplatform.android.data.remote.RemoteDataStore import org.groundplatform.android.data.remote.firebase.schema.GroundFirestore import org.groundplatform.android.data.remote.firebase.schema.LoiCollectionReference +import org.groundplatform.android.data.remote.firebase.schema.LoiQueryScope import org.groundplatform.android.di.coroutines.IoDispatcher import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.SurveyListItem @@ -84,13 +87,29 @@ internal constructor( ) } - override fun loadPredefinedLois(survey: Survey) = - fetchLoiPages(survey) { fetchPredefined(survey) } - - override fun loadUserLois(survey: Survey, ownerUserId: String) = - fetchLoiPages(survey) { fetchUserDefined(survey, ownerUserId) } - - override fun loadSharedLois(survey: Survey) = fetchLoiPages(survey) { fetchSharedLois(survey) } + override fun loadPredefinedLois(survey: Survey, fromTimestamp: Long?) = + fetchLoiPages(survey) { fetch(survey, LoiQueryScope.Predefined, fromTimestamp) } + + override fun loadUserLois(survey: Survey, ownerUserId: String, fromTimestamp: Long?) = + fetchLoiPages(survey) { fetch(survey, LoiQueryScope.UserDefined(ownerUserId), fromTimestamp) } + + override fun loadSharedLois(survey: Survey, fromTimestamp: Long?) = + fetchLoiPages(survey) { fetch(survey, LoiQueryScope.Shared, fromTimestamp) } + + override suspend fun countLois(survey: Survey, ownerUserId: String): Long = + withContext(ioDispatcher) { + val lois = db().surveys().survey(survey.id).lois() + val fieldData = + if (survey.dataVisibility == Survey.DataVisibility.ALL_SURVEY_PARTICIPANTS) { + LoiQueryScope.Shared + } else { + LoiQueryScope.UserDefined(ownerUserId) + } + // A round trip each, and neither needs the other's answer. + listOf(async { lois.count(LoiQueryScope.Predefined) }, async { lois.count(fieldData) }) + .awaitAll() + .sum() + } /** Emits the pages of LOIs produced by [fetch] against the given survey's LOI collection. */ private fun fetchLoiPages( diff --git a/app/src/main/java/org/groundplatform/android/data/remote/firebase/schema/LoiCollectionReference.kt b/app/src/main/java/org/groundplatform/android/data/remote/firebase/schema/LoiCollectionReference.kt index 0490ccf9cf..be1e874812 100644 --- a/app/src/main/java/org/groundplatform/android/data/remote/firebase/schema/LoiCollectionReference.kt +++ b/app/src/main/java/org/groundplatform/android/data/remote/firebase/schema/LoiCollectionReference.kt @@ -17,15 +17,18 @@ package org.groundplatform.android.data.remote.firebase.schema import androidx.annotation.VisibleForTesting +import com.google.firebase.firestore.AggregateSource import com.google.firebase.firestore.CollectionReference import com.google.firebase.firestore.DocumentSnapshot import com.google.firebase.firestore.FieldPath import com.google.firebase.firestore.Query +import com.google.protobuf.Timestamp import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.tasks.await import org.groundplatform.android.data.remote.firebase.base.FluentCollectionReference import org.groundplatform.android.data.remote.firebase.schema.LoiConverter.toLoi +import org.groundplatform.android.proto.AuditInfo import org.groundplatform.android.proto.LocationOfInterest as LocationOfInterestProto import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.locationofinterest.LocationOfInterest @@ -38,6 +41,14 @@ import timber.log.Timber const val SOURCE_FIELD = LocationOfInterestProto.SOURCE_FIELD_NUMBER.toString() /** Path of field on LOI documents representing the creator of the LOI. */ const val OWNER_FIELD = LocationOfInterestProto.OWNER_ID_FIELD_NUMBER.toString() +/** Path of field on LOI documents representing the last modified server timestamp. */ +@VisibleForTesting +internal val LAST_MODIFIED_SERVER_SECONDS: FieldPath = + FieldPath.of( + LocationOfInterestProto.LAST_MODIFIED_FIELD_NUMBER.toString(), + AuditInfo.SERVER_TIMESTAMP_FIELD_NUMBER.toString(), + Timestamp.SECONDS_FIELD_NUMBER.toString(), + ) /** * Documents per query. Deliberately small since geometry complexity varies widely and is unknown @@ -45,45 +56,69 @@ const val OWNER_FIELD = LocationOfInterestProto.OWNER_ID_FIELD_NUMBER.toString() */ @VisibleForTesting internal const val PAGE_SIZE = 250 +internal sealed interface LoiQueryScope { + val source: LocationOfInterestProto.Source + val ownerUserId: String? + + data object Predefined : LoiQueryScope { + // Use !=false rather than ==true to not break legacy dev surveys. + // TODO: Switch to whereEqualTo(true) once legacy dev surveys deleted or migrated. + // Issue URL: https://github.com/google/ground-android/issues/2375 + override val source = LocationOfInterestProto.Source.IMPORTED + override val ownerUserId: String? = null + } + + data object Shared : LoiQueryScope { + override val source = LocationOfInterestProto.Source.FIELD_DATA + override val ownerUserId: String? = null + } + + data class UserDefined(override val ownerUserId: String) : LoiQueryScope { + override val source = LocationOfInterestProto.Source.FIELD_DATA + } +} + class LoiCollectionReference internal constructor(ref: CollectionReference) : FluentCollectionReference(ref) { fun loi(id: String) = LoiDocumentReference(reference().document(id)) - /** Emits all "predefined" LOIs in the specified survey, one page at a time. Main-safe. */ - fun fetchPredefined(survey: Survey): Flow> = - // Use !=false rather than ==true to not break legacy dev surveys. - // TODO: Switch to whereEqualTo(true) once legacy dev surveys deleted or migrated. - // Issue URL: https://github.com/google/ground-android/issues/2375 - fetchLois( - survey, - reference().whereEqualTo(SOURCE_FIELD, LocationOfInterestProto.Source.IMPORTED.number), - ) - - /** Emits LOIs created by the specified email in the specified survey, a page at a time. */ - fun fetchUserDefined(survey: Survey, ownerUserId: String): Flow> = - fetchLois( - survey, - reference() - .whereEqualTo(SOURCE_FIELD, LocationOfInterestProto.Source.FIELD_DATA.number) - .whereEqualTo(OWNER_FIELD, ownerUserId), - ) - - /** Emits all LOIs visible to data collectors in the given survey, a page at a time. */ - fun fetchSharedLois(survey: Survey): Flow> = - fetchLois( - survey, - reference().whereEqualTo(SOURCE_FIELD, LocationOfInterestProto.Source.FIELD_DATA.number), - ) + /** Emits the survey's LOIs in [scope], one page at a time. */ + internal fun fetch( + survey: Survey, + scope: LoiQueryScope, + fromTimestamp: Long?, + ): Flow> = fetchLois(survey, query(scope), fromTimestamp) + + /** Returns how many LOIs [fetch] would emit for the same [scope]. */ + internal suspend fun count(scope: LoiQueryScope): Long = + query(scope).count().get(AggregateSource.SERVER).await().count + + private fun query(scope: LoiQueryScope): Query { + val query = reference().whereEqualTo(SOURCE_FIELD, scope.source.number) + return scope.ownerUserId?.let { query.whereEqualTo(OWNER_FIELD, it) } ?: query + } /** * Emits the LOIs matching [query], a page at a time. Pages are fetched lazily, so a collector * that saves each page before asking for the next never holds more than one page in memory. */ - private fun fetchLois(survey: Survey, query: Query): Flow> = flow { - val orderedQuery = query.orderBy(FieldPath.documentId()).limit(PAGE_SIZE.toLong()) + private fun fetchLois( + survey: Survey, + query: Query, + fromTimestamp: Long?, + ): Flow> = flow { + val orderedQuery = + if (fromTimestamp == null) { + query.orderBy(FieldPath.documentId()).limit(PAGE_SIZE.toLong()) + } else { + query + .whereGreaterThanOrEqualTo(LAST_MODIFIED_SERVER_SECONDS, fromTimestamp / 1000) + .orderBy(LAST_MODIFIED_SERVER_SECONDS) + .limit(PAGE_SIZE.toLong()) + } - var startAfter: String? = null + var startAfter: DocumentSnapshot? = null var hasMore: Boolean do { @@ -97,7 +132,7 @@ class LoiCollectionReference internal constructor(ref: CollectionReference) : // Counted in documents fetched, not LOIs emitted: an unreadable document is dropped by the // conversion above but still takes up a place in the page. hasMore = documents.size == PAGE_SIZE - startAfter = documents.last().id + startAfter = documents.last() } while (hasMore) } diff --git a/app/src/main/java/org/groundplatform/android/repository/LocationOfInterestRepository.kt b/app/src/main/java/org/groundplatform/android/repository/LocationOfInterestRepository.kt index b4d123adb8..5f579a15bd 100644 --- a/app/src/main/java/org/groundplatform/android/repository/LocationOfInterestRepository.kt +++ b/app/src/main/java/org/groundplatform/android/repository/LocationOfInterestRepository.kt @@ -17,6 +17,7 @@ package org.groundplatform.android.repository import javax.inject.Inject import javax.inject.Singleton +import kotlin.time.Clock import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.firstOrNull @@ -29,6 +30,7 @@ import org.groundplatform.android.data.uuid.OfflineUuidGenerator import org.groundplatform.android.system.auth.AuthenticationManager import org.groundplatform.domain.model.Role import org.groundplatform.domain.model.Survey +import org.groundplatform.domain.model.SurveySyncMode import org.groundplatform.domain.model.geometry.Geometry import org.groundplatform.domain.model.job.Job import org.groundplatform.domain.model.locationofinterest.LocationOfInterest @@ -58,44 +60,63 @@ constructor( private val uuidGenerator: OfflineUuidGenerator, private val authenticationManager: AuthenticationManager, ) : LocationOfInterestRepositoryInterface { - override suspend fun syncLocationsOfInterest(survey: Survey) { - val ownerUserId = authenticationManager.getAuthenticatedUser().id + override suspend fun syncLocationsOfInterest(survey: Survey, mode: SurveySyncMode): Long { + Timber.d("Syncing LOIs of survey ${survey.id}: $mode") + return syncLois(survey, authenticationManager.getAuthenticatedUser().id, mode) + } + /** Reads the survey's LOIs into the local db, returning the newest server timestamp it saw. */ + private suspend fun syncLois(survey: Survey, ownerUserId: String, mode: SurveySyncMode): Long { + val syncFromTimestamp = (mode as? SurveySyncMode.Incremental)?.fromTimestamp // Single-page buffering. Persist immediately to avoid OOM on geometry-heavy surveys. val syncedLoiIds = mutableSetOf() - syncedLoiIds += savePages(remoteDataStore.loadPredefinedLois(survey)) - // Shared LOIs are visible to all survey participants, so a user's own LOIs are already - // included. - syncedLoiIds += - if (survey.dataVisibility == Survey.DataVisibility.ALL_SURVEY_PARTICIPANTS) { - savePages(remoteDataStore.loadSharedLois(survey)) - } else { - savePages(remoteDataStore.loadUserLois(survey, ownerUserId)) + var newestLoiTimestamp = syncFromTimestamp ?: 0L + + suspend fun savePages(pages: Flow>) { + pages.collect { page -> + localLoiStore.insertOrUpdateAll(page) + syncedLoiIds += page.map { it.id } + newestLoiTimestamp = + maxOf( + newestLoiTimestamp, + page.maxOfOrNull { it.lastModified.serverTimestamp ?: 0L } ?: 0L, + ) } + } + savePages(remoteDataStore.loadPredefinedLois(survey, syncFromTimestamp)) + if (survey.dataVisibility == Survey.DataVisibility.ALL_SURVEY_PARTICIPANTS) { + savePages(remoteDataStore.loadSharedLois(survey, syncFromTimestamp)) + } else { + savePages(remoteDataStore.loadUserLois(survey, ownerUserId, syncFromTimestamp)) + } - val mutations = localLoiStore.getAllSurveyMutations(survey).firstOrNull().orEmpty() - - // NOTE(#2652): Don't delete pending locations of interest, since we can accidentally delete - // them here if we get to this routine before they can be synced up to the remote database. - val pendingLois = - mutations - .asSequence() - .filter { it.syncStatus in setOf(SyncStatus.PENDING, SyncStatus.IN_PROGRESS) } - .map { it.locationOfInterestId } - .toList() + Timber.d( + "Synced ${syncedLoiIds.size} LOIs of survey. Newest server timestamp $newestLoiTimestamp" + ) + + if (mode is SurveySyncMode.Full) { + // NOTE(#2652): Don't delete pending locations of interest, since we can accidentally delete + // them here if we get to this routine before they can be synced up to the remote database. + val pendingLoiIds = + localLoiStore + .getAllSurveyMutations(survey) + .firstOrNull() + .orEmpty() + .filter { it.syncStatus in setOf(SyncStatus.PENDING, SyncStatus.IN_PROGRESS) } + .map { it.locationOfInterestId } + localLoiStore.deleteNotIn(survey.id, syncedLoiIds + pendingLoiIds) + } - // Delete LOIs in local db not returned in latest list from server, skipping pending mutations. - localLoiStore.deleteNotIn(survey.id, syncedLoiIds.toList() + pendingLois) + return minOf(newestLoiTimestamp, Clock.System.now().toEpochMilliseconds()) } - /** Saves each page of [pages] as it arrives, returning the ids of every LOI saved. */ - private suspend fun savePages(pages: Flow>): Set { - val savedIds = mutableSetOf() - pages.collect { page -> - localLoiStore.insertOrUpdateAll(page) - savedIds += page.map { it.id } - } - return savedIds + override suspend fun hasMissedRemoteDeletions(survey: Survey): Boolean { + val expectedRemoteCount = + localLoiStore.getLoiCount(survey.id) - localLoiStore.countPendingCreatedLois(survey.id) + if (expectedRemoteCount <= 0) return false + + val ownerUserId = authenticationManager.getAuthenticatedUser().id + return remoteDataStore.countLois(survey, ownerUserId) < expectedRemoteCount } override suspend fun getOfflineLoi(surveyId: String, loiId: String): LocationOfInterest? { diff --git a/app/src/main/java/org/groundplatform/android/repository/SurveyRepository.kt b/app/src/main/java/org/groundplatform/android/repository/SurveyRepository.kt index 0539a2edc4..063f6a7d03 100644 --- a/app/src/main/java/org/groundplatform/android/repository/SurveyRepository.kt +++ b/app/src/main/java/org/groundplatform/android/repository/SurveyRepository.kt @@ -34,10 +34,13 @@ import kotlinx.coroutines.withTimeout import org.groundplatform.android.FirebaseCrashLogger import org.groundplatform.android.data.local.LocalValueStore import org.groundplatform.android.data.local.stores.LocalSurveyStore +import org.groundplatform.android.data.local.stores.LocalSurveySyncStateStore import org.groundplatform.android.data.remote.RemoteDataStore import org.groundplatform.android.di.coroutines.ApplicationScope import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.SurveyListItem +import org.groundplatform.domain.model.SurveySyncMode +import org.groundplatform.domain.model.SurveySyncState import org.groundplatform.domain.model.User import org.groundplatform.domain.repository.SurveyRepositoryInterface import timber.log.Timber @@ -53,6 +56,7 @@ constructor( @ApplicationScope private val externalScope: CoroutineScope, private val firebaseCrashLogger: FirebaseCrashLogger, private val localSurveyStore: LocalSurveyStore, + private val localSurveySyncStateStore: LocalSurveySyncStateStore, private val localValueStore: LocalValueStore, private val remoteDataStore: RemoteDataStore, ) : SurveyRepositoryInterface { @@ -86,6 +90,26 @@ constructor( override fun getOfflineSurveys(): Flow> = localSurveyStore.surveys + override suspend fun getSyncState(surveyId: String): SurveySyncState? = + localSurveySyncStateStore.get(surveyId) + + override suspend fun recordSyncState( + survey: Survey, + mode: SurveySyncMode, + latestLoiServerTimestamp: Long, + ) { + when (mode) { + is SurveySyncMode.Full -> + localSurveySyncStateStore.recordFullSync( + survey.id, + latestLoiServerTimestamp, + survey.dataVisibility, + ) + is SurveySyncMode.Incremental -> + localSurveySyncStateStore.recordIncrementalSync(survey.id, latestLoiServerTimestamp) + } + } + override suspend fun removeOfflineSurvey(surveyId: String) { getOfflineSurvey(surveyId)?.let { localSurveyStore.deleteSurvey(it) } } diff --git a/app/src/test/java/org/groundplatform/android/data/local/LocalLocationOfInterestStoreTest.kt b/app/src/test/java/org/groundplatform/android/data/local/LocalLocationOfInterestStoreTest.kt index f95873ff07..6260badce9 100644 --- a/app/src/test/java/org/groundplatform/android/data/local/LocalLocationOfInterestStoreTest.kt +++ b/app/src/test/java/org/groundplatform/android/data/local/LocalLocationOfInterestStoreTest.kt @@ -38,7 +38,6 @@ import org.groundplatform.android.data.local.stores.LocalLocationOfInterestStore import org.groundplatform.android.data.local.stores.LocalSubmissionStore import org.groundplatform.android.data.local.stores.LocalSurveyStore import org.groundplatform.android.data.local.stores.LocalUserStore -import org.groundplatform.android.proto.geometry import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.User import org.groundplatform.domain.model.geometry.Coordinates @@ -213,6 +212,30 @@ class LocalLocationOfInterestStoreTest : BaseHiltTest() { } } + @Test + fun safeDeleteLocalLoi() = runWithTestDispatcher { + localUserStore.insertOrUpdateUser(TEST_USER) + localSurveyStore.insertOrUpdateSurvey(TEST_SURVEY) + // Saved straight to the db, so nothing is queued for upload. + localLoiStore.insertOrUpdate(FakeData.LOCATION_OF_INTEREST) + + localLoiStore.safeDeleteLocalLoi(FakeData.LOI_ID) + + assertThat(localLoiStore.getLocationOfInterest(TEST_SURVEY, FakeData.LOI_ID)).isNull() + } + + @Test + fun `deleteUnlessPendingUpload keeps an loi whose changes are still waiting`() = + runWithTestDispatcher { + localUserStore.insertOrUpdateUser(TEST_USER) + localSurveyStore.insertOrUpdateSurvey(TEST_SURVEY) + localLoiStore.applyAndEnqueue(TEST_LOI_MUTATION) + + localLoiStore.safeDeleteLocalLoi(FakeData.LOI_ID) + + assertThat(localLoiStore.getLocationOfInterest(TEST_SURVEY, FakeData.LOI_ID)).isNotNull() + } + @Test fun `parse vertices when empty string`() { assertThat(parseVertices("")).isEqualTo(listOf()) @@ -344,48 +367,45 @@ class LocalLocationOfInterestStoreTest : BaseHiltTest() { } @Test - fun `countPendingNonDeletedLois counts an loi with unsynced changes once`() = - runWithTestDispatcher { - localUserStore.insertOrUpdateUser(TEST_USER) - localSurveyStore.insertOrUpdateSurvey(TEST_SURVEY) - localLoiStore.insertOrUpdate(testLoi("queued")) - localLoiStore.enqueue(queuedMutation("queued")) - // A second queued change to the same LOI must not count it twice. - localLoiStore.enqueue( - queuedMutation("queued", type = Mutation.Type.UPDATE, status = SyncStatus.IN_PROGRESS) - ) + fun `countPendingCreatedLois counts an loi with unsynced changes once`() = runWithTestDispatcher { + localUserStore.insertOrUpdateUser(TEST_USER) + localSurveyStore.insertOrUpdateSurvey(TEST_SURVEY) + localLoiStore.insertOrUpdate(testLoi("queued")) + localLoiStore.enqueue(queuedMutation("queued")) + // A second queued change to the same LOI must not count it twice. + localLoiStore.enqueue( + queuedMutation("queued", type = Mutation.Type.UPDATE, status = SyncStatus.IN_PROGRESS) + ) - assertThat(localLoiStore.countPendingNonDeletedLois(TEST_SURVEY.id)).isEqualTo(1) - } + assertThat(localLoiStore.countPendingCreatedLois(TEST_SURVEY.id)).isEqualTo(1) + } @Test - fun `countPendingNonDeletedLois skips queued deletes and synced changes`() = - runWithTestDispatcher { - localUserStore.insertOrUpdateUser(TEST_USER) - localSurveyStore.insertOrUpdateSurvey(TEST_SURVEY) - localLoiStore.insertOrUpdate(testLoi("going away")) - localLoiStore.insertOrUpdate(testLoi("settled")) - localLoiStore.enqueue(queuedMutation("going away", type = Mutation.Type.DELETE)) - localLoiStore.enqueue(queuedMutation("settled", status = SyncStatus.COMPLETED)) + fun `countPendingCreatedLois skips queued deletes and synced changes`() = runWithTestDispatcher { + localUserStore.insertOrUpdateUser(TEST_USER) + localSurveyStore.insertOrUpdateSurvey(TEST_SURVEY) + localLoiStore.insertOrUpdate(testLoi("going away")) + localLoiStore.insertOrUpdate(testLoi("settled")) + localLoiStore.enqueue(queuedMutation("going away", type = Mutation.Type.DELETE)) + localLoiStore.enqueue(queuedMutation("settled", status = SyncStatus.COMPLETED)) - assertThat(localLoiStore.countPendingNonDeletedLois(TEST_SURVEY.id)).isEqualTo(0) - } + assertThat(localLoiStore.countPendingCreatedLois(TEST_SURVEY.id)).isEqualTo(0) + } @Test - fun `countPendingNonDeletedLois still counts an loi that is also queued for both creation and deletion`() = + fun `countPendingCreatedLois skips an loi that only has a queued update`() = runWithTestDispatcher { localUserStore.insertOrUpdateUser(TEST_USER) localSurveyStore.insertOrUpdateSurvey(TEST_SURVEY) - localLoiStore.insertOrUpdate(testLoi("both")) - localLoiStore.enqueue(queuedMutation("both")) - // The delete row is skipped, but the create is not. - localLoiStore.enqueue(queuedMutation("both", type = Mutation.Type.DELETE)) + localLoiStore.insertOrUpdate(testLoi("edited")) + // The server already has this LOI, only the edit is waiting to upload. + localLoiStore.enqueue(queuedMutation("edited", type = Mutation.Type.UPDATE)) - assertThat(localLoiStore.countPendingNonDeletedLois(TEST_SURVEY.id)).isEqualTo(1) + assertThat(localLoiStore.countPendingCreatedLois(TEST_SURVEY.id)).isEqualTo(0) } @Test - fun `countPendingNonDeletedLois only counts the survey it was asked about`() = + fun `countPendingCreatedLois only counts the survey it was asked about`() = runWithTestDispatcher { localUserStore.insertOrUpdateUser(TEST_USER) localSurveyStore.insertOrUpdateSurvey(TEST_SURVEY) @@ -393,8 +413,8 @@ class LocalLocationOfInterestStoreTest : BaseHiltTest() { localLoiStore.insertOrUpdate(testLoi("other", surveyId = OTHER_SURVEY.id)) localLoiStore.enqueue(queuedMutation("other", surveyId = OTHER_SURVEY.id)) - assertThat(localLoiStore.countPendingNonDeletedLois(TEST_SURVEY.id)).isEqualTo(0) - assertThat(localLoiStore.countPendingNonDeletedLois(OTHER_SURVEY.id)).isEqualTo(1) + assertThat(localLoiStore.countPendingCreatedLois(TEST_SURVEY.id)).isEqualTo(0) + assertThat(localLoiStore.countPendingCreatedLois(OTHER_SURVEY.id)).isEqualTo(1) } @Test diff --git a/app/src/test/java/org/groundplatform/android/data/remote/FakeRemoteDataStore.kt b/app/src/test/java/org/groundplatform/android/data/remote/FakeRemoteDataStore.kt index 7569879dc4..584e0f27b6 100644 --- a/app/src/test/java/org/groundplatform/android/data/remote/FakeRemoteDataStore.kt +++ b/app/src/test/java/org/groundplatform/android/data/remote/FakeRemoteDataStore.kt @@ -56,6 +56,7 @@ class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore { val loadUserLoisCall = FakeCall> { userLois } val loadSharedLoisCall = FakeCall> { sharedLois } + var loiCount: (Survey) -> Long = { Long.MAX_VALUE } override fun getRestrictedSurveyList(user: User): Flow> = flowOf(surveys.map { it.toListItem(false) }) @@ -67,8 +68,10 @@ class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore { override suspend fun loadTermsOfService(): TermsOfService? = termsOfService?.getOrThrow() - override fun loadPredefinedLois(survey: Survey): Flow> = - predefinedLoiPages ?: flowOf(predefinedLois) + override fun loadPredefinedLois( + survey: Survey, + fromTimestamp: Long?, + ): Flow> = predefinedLoiPages ?: flowOf(predefinedLois) override suspend fun applyMutations(mutations: List, user: User) { if (applyMutationError != null) { @@ -88,12 +91,16 @@ class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore { userProfileRefreshCount++ } - override fun loadUserLois(survey: Survey, ownerUserId: String): Flow> = - flow { - emit(loadUserLoisCall(survey)) - } + override fun loadUserLois( + survey: Survey, + ownerUserId: String, + fromTimestamp: Long?, + ): Flow> = flow { emit(loadUserLoisCall(survey)) } - override fun loadSharedLois(survey: Survey): Flow> = flow { - emit(loadSharedLoisCall(survey)) - } + override fun loadSharedLois( + survey: Survey, + fromTimestamp: Long?, + ): Flow> = flow { emit(loadSharedLoisCall(survey)) } + + override suspend fun countLois(survey: Survey, ownerUserId: String): Long = loiCount(survey) } diff --git a/app/src/test/java/org/groundplatform/android/data/remote/firebase/FirebaseMessagingSurveyTest.kt b/app/src/test/java/org/groundplatform/android/data/remote/firebase/FirebaseMessagingSurveyTest.kt index 2acd0e1fa7..e4721a0199 100644 --- a/app/src/test/java/org/groundplatform/android/data/remote/firebase/FirebaseMessagingSurveyTest.kt +++ b/app/src/test/java/org/groundplatform/android/data/remote/firebase/FirebaseMessagingSurveyTest.kt @@ -17,6 +17,9 @@ package org.groundplatform.android.data.remote.firebase import com.google.firebase.messaging.RemoteMessage +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import org.groundplatform.android.data.local.stores.LocalLocationOfInterestStore import org.groundplatform.android.data.sync.SurveySyncService import org.junit.Before import org.junit.Rule @@ -30,12 +33,14 @@ import org.mockito.Mockito.`when` import org.mockito.junit.MockitoJUnit import org.mockito.junit.MockitoJUnitRunner import org.mockito.junit.MockitoRule +import org.mockito.kotlin.verifyBlocking @RunWith(MockitoJUnitRunner::class) class FirebaseMessagingSurveyTest { @JvmField @Rule val rule: MockitoRule = MockitoJUnit.rule() @Mock private lateinit var surveySyncService: SurveySyncService @Mock private lateinit var remoteMessage: RemoteMessage + @Mock private lateinit var localLoiStore: LocalLocationOfInterestStore private lateinit var messagingService: FirebaseMessagingService @@ -43,6 +48,8 @@ class FirebaseMessagingSurveyTest { fun setUp() { messagingService = FirebaseMessagingService() messagingService.surveySyncService = surveySyncService + messagingService.localLoiStore = localLoiStore + messagingService.externalScope = CoroutineScope(UnconfinedTestDispatcher()) } @Test @@ -55,6 +62,26 @@ class FirebaseMessagingSurveyTest { verify(surveySyncService).enqueueSync(surveyId) } + @Test + fun `drops an loi the message reports as deleted`() { + `when`(remoteMessage.from).thenReturn("/topics/survey") + `when`(remoteMessage.data).thenReturn(mapOf("loiId" to "loi1", "deleted" to "true")) + + messagingService.onMessageReceived(remoteMessage) + + verifyBlocking(localLoiStore) { safeDeleteLocalLoi("loi1") } + } + + @Test + fun `keeps an loi the message only reports as changed`() { + `when`(remoteMessage.from).thenReturn("/topics/survey") + `when`(remoteMessage.data).thenReturn(mapOf("loiId" to "loi1")) + + messagingService.onMessageReceived(remoteMessage) + + verifyBlocking(localLoiStore, never()) { safeDeleteLocalLoi(anyString()) } + } + @Test fun `ignores null topic`() { `when`(remoteMessage.from).thenReturn(null) diff --git a/app/src/test/java/org/groundplatform/android/data/remote/firebase/schema/LoiCollectionReferenceTest.kt b/app/src/test/java/org/groundplatform/android/data/remote/firebase/schema/LoiCollectionReferenceTest.kt index 1094bdc807..3e4c9e8d7a 100644 --- a/app/src/test/java/org/groundplatform/android/data/remote/firebase/schema/LoiCollectionReferenceTest.kt +++ b/app/src/test/java/org/groundplatform/android/data/remote/firebase/schema/LoiCollectionReferenceTest.kt @@ -17,6 +17,9 @@ package org.groundplatform.android.data.remote.firebase.schema import com.google.android.gms.tasks.Tasks import com.google.common.truth.Truth.assertThat +import com.google.firebase.firestore.AggregateQuery +import com.google.firebase.firestore.AggregateQuerySnapshot +import com.google.firebase.firestore.AggregateSource import com.google.firebase.firestore.CollectionReference import com.google.firebase.firestore.DocumentSnapshot import com.google.firebase.firestore.FieldPath @@ -45,6 +48,7 @@ import org.mockito.Mock import org.mockito.MockitoAnnotations import org.mockito.kotlin.any import org.mockito.kotlin.doReturn +import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify @@ -80,7 +84,8 @@ class LoiCollectionReferenceTest { fun `fetch stops after a page shorter than the page size`() = runTest { pages = mockPages(3) - val emitted = loiCollectionReference.fetchPredefined(SURVEY).toList() + val emitted = + loiCollectionReference.fetch(SURVEY, LoiQueryScope.Predefined, fromTimestamp = null).toList() assertThat(emitted.flatten().map { it.id }).containsExactly("loi0", "loi1", "loi2").inOrder() assertThat(pagesFetched).isEqualTo(1) @@ -90,7 +95,8 @@ class LoiCollectionReferenceTest { fun `fetch keeps requesting while pages come back full`() = runTest { pages = mockPages(PAGE_SIZE, PAGE_SIZE, 2) - val emitted = loiCollectionReference.fetchPredefined(SURVEY).toList() + val emitted = + loiCollectionReference.fetch(SURVEY, LoiQueryScope.Predefined, fromTimestamp = null).toList() // One emission per page, and the short third page ends it. assertThat(emitted.map { it.size }).containsExactly(PAGE_SIZE, PAGE_SIZE, 2).inOrder() @@ -100,17 +106,19 @@ class LoiCollectionReferenceTest { @Test fun `fetch resumes each page after the last document of the previous one`() = runTest { pages = mockPages(PAGE_SIZE, 1) + val lastOfFirstPage = pages.first().last() - loiCollectionReference.fetchPredefined(SURVEY).toList() + loiCollectionReference.fetch(SURVEY, LoiQueryScope.Predefined, fromTimestamp = null).toList() - verify(mockQuery).startAfter("loi${PAGE_SIZE - 1}") + verify(mockQuery).startAfter(lastOfFirstPage) } @Test fun `fetch emits nothing when the collection is empty`() = runTest { pages = mockPages(0) - val emitted = loiCollectionReference.fetchPredefined(SURVEY).toList() + val emitted = + loiCollectionReference.fetch(SURVEY, LoiQueryScope.Predefined, fromTimestamp = null).toList() assertThat(emitted).isEmpty() assertThat(pagesFetched).isEqualTo(1) @@ -122,7 +130,8 @@ class LoiCollectionReferenceTest { val brokenFirst = listOf(mockDocument("broken", jobId = "job the survey does not have")) pages = listOf(brokenFirst + fullPage.drop(1), lastPage) - val emitted = loiCollectionReference.fetchPredefined(SURVEY).toList() + val emitted = + loiCollectionReference.fetch(SURVEY, LoiQueryScope.Predefined, fromTimestamp = null).toList() assertThat(emitted.first()).hasSize(PAGE_SIZE - 1) assertThat(emitted.flatten().map { it.id }).doesNotContain("broken") @@ -133,22 +142,80 @@ class LoiCollectionReferenceTest { fun `fetch orders by document id and limits each page`() = runTest { pages = mockPages(1) - loiCollectionReference.fetchPredefined(SURVEY).toList() + loiCollectionReference.fetch(SURVEY, LoiQueryScope.Predefined, fromTimestamp = null).toList() verify(mockQuery).orderBy(FieldPath.documentId()) verify(mockQuery).limit(PAGE_SIZE.toLong()) } + @Test + fun `fetch from a timestamp asks only for lois modified since then`() = runTest { + pages = mockPages(1) + + loiCollectionReference + .fetch(SURVEY, LoiQueryScope.Predefined, fromTimestamp = 987_654_321_000) + .toList() + + verify(mockQuery).whereGreaterThanOrEqualTo(LAST_MODIFIED_SERVER_SECONDS, 987_654_321L) + verify(mockQuery).orderBy(LAST_MODIFIED_SERVER_SECONDS) + verify(mockQuery, never()).orderBy(FieldPath.documentId()) + } + + @Test + fun `fetch for one owner asks only for their lois`() = runTest { + pages = mockPages(1) + + loiCollectionReference + .fetch(SURVEY, LoiQueryScope.UserDefined("user-1"), fromTimestamp = null) + .toList() + + verify(mockQuery).whereEqualTo(OWNER_FIELD, "user-1") + } + + @Test + fun `fetch for shared lois asks for every owner's`() = runTest { + pages = mockPages(1) + + loiCollectionReference.fetch(SURVEY, LoiQueryScope.Shared, fromTimestamp = null).toList() + + verify(mockQuery, never()).whereEqualTo(eq(OWNER_FIELD), any()) + } + + @Test + fun `count for one owner counts only their lois`() = runTest { + mockAggregateCount(42L) + + assertThat(loiCollectionReference.count(LoiQueryScope.UserDefined("user-1"))).isEqualTo(42L) + verify(mockQuery).whereEqualTo(OWNER_FIELD, "user-1") + } + + @Test + fun `count returns the aggregated document count without fetching them`() = runTest { + mockAggregateCount(42L) + + assertThat(loiCollectionReference.count(LoiQueryScope.Predefined)).isEqualTo(42L) + assertThat(pagesFetched).isEqualTo(0) + } + @Test fun `fetch is lazy until collected`() = runTest { pages = mockPages(1) - loiCollectionReference.fetchPredefined(SURVEY) + loiCollectionReference.fetch(SURVEY, LoiQueryScope.Predefined, fromTimestamp = null) verify(mockQuery, never()).get() assertThat(pagesFetched).isEqualTo(0) } + private fun mockAggregateCount(count: Long) { + val aggregateSnapshot = mock { on { this.count } doReturn count } + val aggregateQuery = + mock { + on { get(AggregateSource.SERVER) } doReturn Tasks.forResult(aggregateSnapshot) + } + whenever(mockQuery.count()).thenReturn(aggregateQuery) + } + private fun mockPages(vararg sizes: Int): List> { var next = 0 return sizes.map { size -> diff --git a/app/src/test/java/org/groundplatform/android/repository/LocationOfInterestRepositoryTest.kt b/app/src/test/java/org/groundplatform/android/repository/LocationOfInterestRepositoryTest.kt index 5e8c04239f..287daac84f 100644 --- a/app/src/test/java/org/groundplatform/android/repository/LocationOfInterestRepositoryTest.kt +++ b/app/src/test/java/org/groundplatform/android/repository/LocationOfInterestRepositoryTest.kt @@ -21,6 +21,8 @@ import dagger.hilt.android.testing.BindValue import dagger.hilt.android.testing.HiltAndroidTest import javax.inject.Inject import kotlin.test.assertFailsWith +import kotlin.time.Clock +import kotlin.time.Duration.Companion.days import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flow @@ -33,12 +35,15 @@ import org.groundplatform.android.data.remote.FakeRemoteDataStore import org.groundplatform.android.data.sync.MutationSyncWorkManager import org.groundplatform.android.system.auth.FakeAuthenticationManager import org.groundplatform.domain.model.Survey +import org.groundplatform.domain.model.SurveySyncMode import org.groundplatform.domain.model.geometry.Coordinates import org.groundplatform.domain.model.geometry.LinearRing import org.groundplatform.domain.model.geometry.Point import org.groundplatform.domain.model.geometry.Polygon import org.groundplatform.domain.model.map.Bounds import org.groundplatform.domain.model.mutation.Mutation.Type.CREATE +import org.groundplatform.domain.model.mutation.Mutation.Type.DELETE +import org.groundplatform.domain.model.mutation.Mutation.Type.UPDATE import org.groundplatform.domain.repository.LocationOfInterestRepositoryInterface import org.groundplatform.domain.repository.MutationRepositoryInterface import org.groundplatform.domain.repository.UserRepositoryInterface @@ -153,7 +158,7 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() { listOf(TEST_AREA_OF_INTEREST_1, TEST_AREA_OF_INTEREST_2), ) - locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY) + locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY, SurveySyncMode.Full) assertThat(locationOfInterestRepository.getValidLois(TEST_SURVEY).first()) .containsExactlyElementsIn(TEST_LOCATIONS_OF_INTEREST) @@ -172,7 +177,7 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() { savedWhenPageRequested += localLoiStore.getLoiCount(TEST_SURVEY.id) } - locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY) + locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY, SurveySyncMode.Full) assertThat(savedWhenPageRequested).containsExactly(0, 2, 3).inOrder() } @@ -186,7 +191,7 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() { } assertFailsWith { - locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY) + locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY, SurveySyncMode.Full) } val lois = locationOfInterestRepository.getValidLois(TEST_SURVEY).first() @@ -203,7 +208,7 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() { // Sync again, with the server now returning two of them across separate pages. fakeRemoteDataStore.predefinedLoiPages = flowOf(listOf(TEST_POINT_OF_INTEREST_1), listOf(TEST_AREA_OF_INTEREST_2)) - locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY) + locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY, SurveySyncMode.Full) assertThat(locationOfInterestRepository.getValidLois(TEST_SURVEY).first()) .containsExactly(TEST_POINT_OF_INTEREST_1, TEST_AREA_OF_INTEREST_2) @@ -216,7 +221,7 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() { if (it.id == updated.id) updated else it } - locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY) + locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY, SurveySyncMode.Full) val lois = locationOfInterestRepository.getValidLois(TEST_SURVEY).first() assertThat(lois).contains(updated) @@ -231,12 +236,147 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() { locationOfInterestRepository.applyAndEnqueue(pending.toMutation(CREATE, TEST_USER.id)) fakeRemoteDataStore.predefinedLois = listOf(TEST_POINT_OF_INTEREST_1) - locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY) + locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY, SurveySyncMode.Full) assertThat(locationOfInterestRepository.getOfflineLoi(TEST_SURVEY.id, pending.id)) .isEqualTo(pending) } + @Test + fun `Incremental sync keeps the lois which were already stored intact`() = runWithTestDispatcher { + val newLoi = createPoint("6", COORDINATE_2) + fakeRemoteDataStore.predefinedLois = listOf(newLoi) + + locationOfInterestRepository.syncLocationsOfInterest( + TEST_SURVEY, + SurveySyncMode.Incremental(SERVER_TIMESTAMP), + ) + + // LOIs missing from an incremental response are left alone, not deleted. + assertThat(locationOfInterestRepository.getValidLois(TEST_SURVEY).first()) + .containsExactlyElementsIn(TEST_LOCATIONS_OF_INTEREST + newLoi) + } + + @Test + fun `sync reports the newest server timestamp it has seen`() = runWithTestDispatcher { + val loi = createPoint("6", COORDINATE_2) + fakeRemoteDataStore.predefinedLois = + listOf(loi.copy(lastModified = loi.lastModified.copy(serverTimestamp = SERVER_TIMESTAMP))) + + val result = + locationOfInterestRepository.syncLocationsOfInterest( + TEST_SURVEY, + SurveySyncMode.Incremental(0), + ) + + assertThat(result).isEqualTo(SERVER_TIMESTAMP) + } + + @Test + fun `sync caps a future server timestamp at the current time`() = runWithTestDispatcher { + // Uploaded by a device whose clock is set ahead. + val loi = createPoint("6", COORDINATE_2) + val future = Clock.System.now().toEpochMilliseconds() + 365.days.inWholeMilliseconds + fakeRemoteDataStore.predefinedLois = + listOf(loi.copy(lastModified = loi.lastModified.copy(serverTimestamp = future))) + + val result = + locationOfInterestRepository.syncLocationsOfInterest( + TEST_SURVEY, + SurveySyncMode.Incremental(0), + ) + + assertThat(result).isAtMost(Clock.System.now().toEpochMilliseconds()) + } + + @Test + fun `a pending edit does not hide a missed deletion`() = runWithTestDispatcher { + // The server already has this LOI, only an edit to it is waiting to upload. + val edited = TEST_LOCATIONS_OF_INTEREST.first() + locationOfInterestRepository.applyAndEnqueue(edited.toMutation(UPDATE, TEST_USER.id)) + fakeRemoteDataStore.loiCount = { (TEST_LOCATIONS_OF_INTEREST.size - 1).toLong() } + + assertThat(locationOfInterestRepository.hasMissedRemoteDeletions(TEST_SURVEY)).isTrue() + } + + @Test + fun `a shrunken remote loi count is reported as a missed deletion`() = runWithTestDispatcher { + fakeRemoteDataStore.loiCount = { (TEST_LOCATIONS_OF_INTEREST.size - 1).toLong() } + + assertThat(locationOfInterestRepository.hasMissedRemoteDeletions(TEST_SURVEY)).isTrue() + } + + @Test + fun `sync keeps local lois when the counts agree`() = runWithTestDispatcher { + // Nothing changed remotely, so the incremental fetch comes back empty. + fakeRemoteDataStore.predefinedLois = emptyList() + fakeRemoteDataStore.loiCount = { TEST_LOCATIONS_OF_INTEREST.size.toLong() } + + locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY, SurveySyncMode.Incremental(0)) + + assertThat(locationOfInterestRepository.getValidLois(TEST_SURVEY).first()) + .containsExactlyElementsIn(TEST_LOCATIONS_OF_INTEREST) + } + + @Test + fun `sync ignores a local count inflated by lois still waiting to upload`() = + runWithTestDispatcher { + // Created locally and not yet uploaded, so the server cannot know about it. + val pending = + LOCATION_OF_INTEREST.copy(customId = "", lastModified = LOCATION_OF_INTEREST.created) + locationOfInterestRepository.applyAndEnqueue(pending.toMutation(CREATE, TEST_USER.id)) + fakeRemoteDataStore.predefinedLois = emptyList() + fakeRemoteDataStore.loiCount = { TEST_LOCATIONS_OF_INTEREST.size.toLong() } + + locationOfInterestRepository.syncLocationsOfInterest( + TEST_SURVEY, + SurveySyncMode.Incremental(0), + ) + + assertThat(locationOfInterestRepository.getValidLois(TEST_SURVEY).first()) + .containsAtLeastElementsIn(TEST_LOCATIONS_OF_INTEREST) + } + + @Test + fun `a count gap left by an loi waiting to upload is not a missed deletion`() = + runWithTestDispatcher { + // Created locally and not uploaded yet, so the server can't have it. + val pending = + LOCATION_OF_INTEREST.copy(customId = "", lastModified = LOCATION_OF_INTEREST.created) + locationOfInterestRepository.applyAndEnqueue(pending.toMutation(CREATE, TEST_USER.id)) + fakeRemoteDataStore.loiCount = { TEST_LOCATIONS_OF_INTEREST.size.toLong() } + + assertThat(locationOfInterestRepository.hasMissedRemoteDeletions(TEST_SURVEY)).isFalse() + } + + @Test + fun `a missed deletion is still reported while a delete waits to upload`() = + runWithTestDispatcher { + // Deleted locally, so it already left the local count and can't hide the missed deletion. + locationOfInterestRepository.applyAndEnqueue( + TEST_POINT_OF_INTEREST_2.toMutation(DELETE, TEST_USER.id) + ) + val remaining = + TEST_LOCATIONS_OF_INTEREST - TEST_POINT_OF_INTEREST_1 - TEST_POINT_OF_INTEREST_2 + fakeRemoteDataStore.loiCount = { remaining.size.toLong() } + + assertThat(locationOfInterestRepository.hasMissedRemoteDeletions(TEST_SURVEY)).isTrue() + } + + @Test + fun `sync reads the lois it was asked for without counting them`() = runWithTestDispatcher { + var counted = 0 + fakeRemoteDataStore.loiCount = { + counted++ + TEST_LOCATIONS_OF_INTEREST.size.toLong() + } + + locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY, SurveySyncMode.Full) + locationOfInterestRepository.syncLocationsOfInterest(TEST_SURVEY, SurveySyncMode.Incremental(0)) + + assertThat(counted).isEqualTo(0) + } + @Test fun `loi within bounds when out of bounds returns empty list`() = runWithTestDispatcher { val southwest = Coordinates(-60.0, -60.0) @@ -377,6 +517,7 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() { private val COORDINATE_1 = Coordinates(-20.0, -20.0) private val COORDINATE_2 = Coordinates(0.0, 0.0) private val COORDINATE_3 = Coordinates(20.0, 20.0) + private const val SERVER_TIMESTAMP = 1_700_000_000_000 private val AREA_OF_INTEREST = FakeData.AREA_OF_INTEREST private val LOCATION_OF_INTEREST = FakeData.LOCATION_OF_INTEREST diff --git a/app/src/test/java/org/groundplatform/android/repository/SurveyRepositoryTest.kt b/app/src/test/java/org/groundplatform/android/repository/SurveyRepositoryTest.kt index b95b089a4c..82560b98f3 100644 --- a/app/src/test/java/org/groundplatform/android/repository/SurveyRepositoryTest.kt +++ b/app/src/test/java/org/groundplatform/android/repository/SurveyRepositoryTest.kt @@ -25,6 +25,8 @@ import org.groundplatform.android.BaseHiltTest import org.groundplatform.android.FakeData.SURVEY import org.groundplatform.android.data.local.stores.LocalSurveyStore import org.groundplatform.android.data.remote.FakeRemoteDataStore +import org.groundplatform.domain.model.Survey +import org.groundplatform.domain.model.SurveySyncMode import org.groundplatform.domain.repository.SurveyRepositoryInterface import org.groundplatform.domain.usecases.survey.ActivateSurveyUseCase import org.junit.Before @@ -47,6 +49,49 @@ class SurveyRepositoryTest : BaseHiltTest() { fakeRemoteDataStore.surveys = listOf(SURVEY) } + @Test + fun `getSyncState returns null for a survey which has never been synced`() = + runWithTestDispatcher { + localSurveyStore.insertOrUpdateSurvey(SURVEY) + + assertThat(surveyRepository.getSyncState(SURVEY.id)).isNull() + } + + @Test + fun `recordSyncState stores the timestamp and the visibility after a full read`() = + runWithTestDispatcher { + val survey = SURVEY.copy(dataVisibility = Survey.DataVisibility.ALL_SURVEY_PARTICIPANTS) + localSurveyStore.insertOrUpdateSurvey(survey) + + surveyRepository.recordSyncState(survey, SurveySyncMode.Full, TEST_LATEST_LOI_TIMESTAMP) + + val state = checkNotNull(surveyRepository.getSyncState(survey.id)) + assertThat(state.latestLoiServerTimestamp).isEqualTo(TEST_LATEST_LOI_TIMESTAMP) + assertThat(state.syncedDataVisibility).isEqualTo(survey.dataVisibility) + assertThat(state.lastFullSyncClientTimestamp).isGreaterThan(0) + } + + @Test + fun `recordSyncState updates only the timestamp after an incremental read`() = + runWithTestDispatcher { + val survey = SURVEY.copy(dataVisibility = Survey.DataVisibility.ALL_SURVEY_PARTICIPANTS) + localSurveyStore.insertOrUpdateSurvey(survey) + surveyRepository.recordSyncState(survey, SurveySyncMode.Full, TEST_LATEST_LOI_TIMESTAMP) + val afterFullRead = checkNotNull(surveyRepository.getSyncState(survey.id)) + + surveyRepository.recordSyncState( + survey, + SurveySyncMode.Incremental(TEST_LATEST_LOI_TIMESTAMP), + TEST_LATEST_LOI_TIMESTAMP + 1, + ) + + val state = checkNotNull(surveyRepository.getSyncState(survey.id)) + assertThat(state.latestLoiServerTimestamp).isEqualTo(TEST_LATEST_LOI_TIMESTAMP + 1) + assertThat(state.lastFullSyncClientTimestamp) + .isEqualTo(afterFullRead.lastFullSyncClientTimestamp) + assertThat(state.syncedDataVisibility).isEqualTo(afterFullRead.syncedDataVisibility) + } + @Test fun `setting selectedSurveyId updates the active survey`() = runWithTestDispatcher { localSurveyStore.insertOrUpdateSurvey(SURVEY) @@ -95,4 +140,8 @@ class SurveyRepositoryTest : BaseHiltTest() { surveyRepository.getRemoteSurvey(SURVEY.id) } } + + companion object { + private const val TEST_LATEST_LOI_TIMESTAMP = 987654321L + } } diff --git a/app/src/test/java/org/groundplatform/android/ui/home/mapcontainer/HomeScreenMapContainerViewModelTest.kt b/app/src/test/java/org/groundplatform/android/ui/home/mapcontainer/HomeScreenMapContainerViewModelTest.kt index 6446ad3db0..f747e4ef5e 100644 --- a/app/src/test/java/org/groundplatform/android/ui/home/mapcontainer/HomeScreenMapContainerViewModelTest.kt +++ b/app/src/test/java/org/groundplatform/android/ui/home/mapcontainer/HomeScreenMapContainerViewModelTest.kt @@ -85,6 +85,8 @@ class HomeScreenMapContainerViewModelTest : BaseHiltTest() { // Setup survey and LOIs remoteDataStore.surveys = listOf(SURVEY) remoteDataStore.predefinedLois = listOf(LOCATION_OF_INTEREST) + // Activating a survey syncs its LOIs, and the sync reports back where it left off. + whenever(loiRepository.syncLocationsOfInterest(any(), any())).thenReturn(0L) activateSurvey(SURVEY.id) advanceUntilIdle() whenever(loiRepository.getWithinBounds(SURVEY, BOUNDS)) diff --git a/core/domain/src/commonMain/kotlin/org/groundplatform/domain/model/SurveySyncMode.kt b/core/domain/src/commonMain/kotlin/org/groundplatform/domain/model/SurveySyncMode.kt new file mode 100644 index 0000000000..2cce694602 --- /dev/null +++ b/core/domain/src/commonMain/kotlin/org/groundplatform/domain/model/SurveySyncMode.kt @@ -0,0 +1,23 @@ +/* + * Copyright 2026 Google LLC + * + * 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 org.groundplatform.domain.model + +/** How much of a survey's locations of interest the next sync should read. */ +sealed interface SurveySyncMode { + data object Full : SurveySyncMode + + data class Incremental(val fromTimestamp: Long) : SurveySyncMode +} diff --git a/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/LocationOfInterestRepositoryInterface.kt b/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/LocationOfInterestRepositoryInterface.kt index 3e057c04af..73b575d52a 100644 --- a/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/LocationOfInterestRepositoryInterface.kt +++ b/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/LocationOfInterestRepositoryInterface.kt @@ -17,6 +17,7 @@ package org.groundplatform.domain.repository import kotlinx.coroutines.flow.Flow import org.groundplatform.domain.model.Survey +import org.groundplatform.domain.model.SurveySyncMode import org.groundplatform.domain.model.geometry.Geometry import org.groundplatform.domain.model.job.Job import org.groundplatform.domain.model.locationofinterest.LocationOfInterest @@ -24,8 +25,14 @@ import org.groundplatform.domain.model.map.Bounds import org.groundplatform.domain.model.mutation.LocationOfInterestMutation interface LocationOfInterestRepositoryInterface { - /** Mirrors locations of interest in the specified survey from the remote db into the local db. */ - suspend fun syncLocationsOfInterest(survey: Survey) + /** + * Mirrors locations of interest in the specified survey from the remote db into the local db, + * reading as much of them as [mode] calls for. Returns the newest server timestamp it saw. + */ + suspend fun syncLocationsOfInterest(survey: Survey, mode: SurveySyncMode): Long + + /** Returns whether the local db holds an LOI which a full sync would find gone from remote. */ + suspend fun hasMissedRemoteDeletions(survey: Survey): Boolean /** This only works if the survey and location of interests are already cached to local db. */ suspend fun getOfflineLoi(surveyId: String, loiId: String): LocationOfInterest? diff --git a/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/SurveyRepositoryInterface.kt b/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/SurveyRepositoryInterface.kt index b2daa0daeb..e8cd7297e7 100644 --- a/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/SurveyRepositoryInterface.kt +++ b/core/domain/src/commonMain/kotlin/org/groundplatform/domain/repository/SurveyRepositoryInterface.kt @@ -19,6 +19,8 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.StateFlow import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.SurveyListItem +import org.groundplatform.domain.model.SurveySyncMode +import org.groundplatform.domain.model.SurveySyncState import org.groundplatform.domain.model.User /** Maintains the state of currently active survey. */ @@ -32,6 +34,12 @@ interface SurveyRepositoryInterface { suspend fun saveSurvey(survey: Survey) + /** Returns what the last sync of the given survey left behind, or null if none has run. */ + suspend fun getSyncState(surveyId: String): SurveySyncState? + + /** Records where a [mode] sync of [survey] left off, for the next one to resume from. */ + suspend fun recordSyncState(survey: Survey, mode: SurveySyncMode, latestLoiServerTimestamp: Long) + suspend fun getRemoteSurvey(surveyId: String): Survey? fun getRemoteSurveys(user: User): Flow> diff --git a/core/domain/src/commonMain/kotlin/org/groundplatform/domain/usecases/survey/SyncSurveyUseCase.kt b/core/domain/src/commonMain/kotlin/org/groundplatform/domain/usecases/survey/SyncSurveyUseCase.kt index c0c14d9722..e12d98e979 100644 --- a/core/domain/src/commonMain/kotlin/org/groundplatform/domain/usecases/survey/SyncSurveyUseCase.kt +++ b/core/domain/src/commonMain/kotlin/org/groundplatform/domain/usecases/survey/SyncSurveyUseCase.kt @@ -16,7 +16,10 @@ package org.groundplatform.domain.usecases.survey import co.touchlab.kermit.Logger +import kotlin.time.Clock +import kotlin.time.Duration.Companion.days import org.groundplatform.domain.model.Survey +import org.groundplatform.domain.model.SurveySyncMode import org.groundplatform.domain.repository.LocationOfInterestRepositoryInterface import org.groundplatform.domain.repository.SurveyRepositoryInterface @@ -43,7 +46,26 @@ class SyncSurveyUseCase( private suspend fun syncSurvey(survey: Survey) { surveyRepository.saveSurvey(survey) - loiRepository.syncLocationsOfInterest(survey) + val mode = syncMode(survey) + val latestLoiServerTimestamp = loiRepository.syncLocationsOfInterest(survey, mode) + surveyRepository.recordSyncState(survey, mode, latestLoiServerTimestamp) Logger.d("Synced survey ${survey.id}") } + + private suspend fun syncMode(survey: Survey): SurveySyncMode { + val syncState = surveyRepository.getSyncState(survey.id) + return when { + syncState == null -> SurveySyncMode.Full + survey.dataVisibility != syncState.syncedDataVisibility -> SurveySyncMode.Full + Clock.System.now().toEpochMilliseconds() - syncState.lastFullSyncClientTimestamp > + FULL_SYNC_INTERVAL_MILLIS -> SurveySyncMode.Full + loiRepository.hasMissedRemoteDeletions(survey) -> SurveySyncMode.Full + else -> SurveySyncMode.Incremental(syncState.latestLoiServerTimestamp) + } + } + + internal companion object { + // Periodic full survey reads prevent local incremental syncs from drifting from the server. + val FULL_SYNC_INTERVAL_MILLIS = 7.days.inWholeMilliseconds + } } diff --git a/core/domain/src/commonTest/kotlin/org/groundplatform/domain/usecases/survey/ActivateSurveyUseCaseTest.kt b/core/domain/src/commonTest/kotlin/org/groundplatform/domain/usecases/survey/ActivateSurveyUseCaseTest.kt index e55a6094c6..5944333ac4 100644 --- a/core/domain/src/commonTest/kotlin/org/groundplatform/domain/usecases/survey/ActivateSurveyUseCaseTest.kt +++ b/core/domain/src/commonTest/kotlin/org/groundplatform/domain/usecases/survey/ActivateSurveyUseCaseTest.kt @@ -22,6 +22,7 @@ import kotlin.test.assertFailsWith import kotlin.test.assertNull import kotlin.test.assertTrue import kotlinx.coroutines.test.runTest +import org.groundplatform.domain.model.SurveySyncMode import org.groundplatform.testing.FakeDataGenerator import org.groundplatform.testing.FakeLocationOfInterestRepository import org.groundplatform.testing.FakeSurveyRepository @@ -46,6 +47,18 @@ class ActivateSurveyUseCaseTest { assertEquals(survey, surveyRepository.getOfflineSurvey(survey.id)) } + @Test + fun `Do a full sync on a survey which isn't available offline yet`() = runTest { + val survey = FakeDataGenerator.newSurvey(id = "survey-1") + surveyRepository.remoteSurveys = listOf(survey) + + activateSurvey(survey.id) + + // Nothing of the survey is stored yet, so there is no cursor to resume from: removing a survey + // takes its sync state along with it. + assertEquals(SurveySyncMode.Full, loiRepository.lastSyncMode) + } + @Test fun `Throws error when survey can't be made available offline`() = runTest { surveyRepository.onGetRemoteSurveyCall.overrideBehavior { error("Remote failed") } diff --git a/core/domain/src/commonTest/kotlin/org/groundplatform/domain/usecases/survey/SyncSurveyUseCaseTest.kt b/core/domain/src/commonTest/kotlin/org/groundplatform/domain/usecases/survey/SyncSurveyUseCaseTest.kt index cdfd58a5a8..1fd00c5fff 100644 --- a/core/domain/src/commonTest/kotlin/org/groundplatform/domain/usecases/survey/SyncSurveyUseCaseTest.kt +++ b/core/domain/src/commonTest/kotlin/org/groundplatform/domain/usecases/survey/SyncSurveyUseCaseTest.kt @@ -20,7 +20,11 @@ import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertNull import kotlin.test.assertTrue +import kotlin.time.Clock import kotlinx.coroutines.test.runTest +import org.groundplatform.domain.model.Survey +import org.groundplatform.domain.model.SurveySyncMode +import org.groundplatform.domain.model.SurveySyncState import org.groundplatform.testing.FakeDataGenerator import org.groundplatform.testing.FakeLocationOfInterestRepository import org.groundplatform.testing.FakeSurveyRepository @@ -57,4 +61,100 @@ class SyncSurveyUseCaseTest { assertFailsWith { syncSurvey(FakeDataGenerator.newSurvey().id) } } + + @Test + fun `reads every LOI when the survey has never been synced`() = runTest { + assertEquals(SurveySyncMode.Full, executeSync(syncState = null)) + } + + @Test + fun `reads every LOI when the last sync covered a different survey data visibility setting`() = + runTest { + val state = + SurveySyncState( + surveyId = FakeDataGenerator.newSurvey().id, + latestLoiServerTimestamp = TEST_LATEST_LOI_TIMESTAMP, + lastFullSyncClientTimestamp = Clock.System.now().toEpochMilliseconds(), + syncedDataVisibility = Survey.DataVisibility.ALL_SURVEY_PARTICIPANTS, + ) + + assertEquals(SurveySyncMode.Full, executeSync(state)) + } + + @Test + fun `reads every LOI when the last full sync fell out of the message backlog`() = runTest { + val state = + SurveySyncState( + surveyId = FakeDataGenerator.newSurvey().id, + latestLoiServerTimestamp = TEST_LATEST_LOI_TIMESTAMP, + lastFullSyncClientTimestamp = + Clock.System.now().toEpochMilliseconds() - + SyncSurveyUseCase.FULL_SYNC_INTERVAL_MILLIS * 2, + syncedDataVisibility = null, + ) + + assertEquals(SurveySyncMode.Full, executeSync(state)) + } + + @Test + fun `resumes from the last cursor while the backlog still reaches it`() = runTest { + val state = + SurveySyncState( + surveyId = FakeDataGenerator.newSurvey().id, + latestLoiServerTimestamp = TEST_LATEST_LOI_TIMESTAMP, + lastFullSyncClientTimestamp = + Clock.System.now().toEpochMilliseconds() - + SyncSurveyUseCase.FULL_SYNC_INTERVAL_MILLIS / 2, + syncedDataVisibility = null, + ) + + assertEquals(SurveySyncMode.Incremental(TEST_LATEST_LOI_TIMESTAMP), executeSync(state)) + } + + @Test + fun `does not look for missed deletions when a full read is already due`() = runTest { + executeSync(syncState = null) + + assertEquals(0, loiRepository.hasMissedRemoteDeletionsCall.callCount) + } + + @Test + fun `reads every LOI when a deletion was missed`() = runTest { + loiRepository.hasMissedRemoteDeletionsCall.overrideBehavior { true } + val state = + SurveySyncState( + surveyId = FakeDataGenerator.newSurvey().id, + latestLoiServerTimestamp = TEST_LATEST_LOI_TIMESTAMP, + lastFullSyncClientTimestamp = Clock.System.now().toEpochMilliseconds(), + syncedDataVisibility = null, + ) + + assertEquals(SurveySyncMode.Full, executeSync(state)) + } + + @Test + fun `records where the sync of the LOIs left off`() = runTest { + val survey = FakeDataGenerator.newSurvey() + surveyRepository.remoteSurveys = listOf(survey) + loiRepository.latestLoiServerTimestamp = TEST_LATEST_LOI_TIMESTAMP + + syncSurvey(survey.id) + + assertEquals(SurveySyncMode.Full, surveyRepository.lastRecordedSyncMode) + assertEquals(TEST_LATEST_LOI_TIMESTAMP, surveyRepository.lastRecordedLoiServerTimestamp) + } + + private suspend fun executeSync(syncState: SurveySyncState?): SurveySyncMode? { + val survey = FakeDataGenerator.newSurvey() + surveyRepository.remoteSurveys = listOf(survey) + surveyRepository.syncState = syncState + + syncSurvey(survey.id) + + return loiRepository.lastSyncMode + } + + companion object { + private const val TEST_LATEST_LOI_TIMESTAMP = 987654321L + } } diff --git a/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeLocationOfInterestRepository.kt b/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeLocationOfInterestRepository.kt index 8d0848efe5..636afa83ae 100644 --- a/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeLocationOfInterestRepository.kt +++ b/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeLocationOfInterestRepository.kt @@ -18,6 +18,7 @@ package org.groundplatform.testing import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flowOf import org.groundplatform.domain.model.Survey +import org.groundplatform.domain.model.SurveySyncMode import org.groundplatform.domain.model.geometry.Geometry import org.groundplatform.domain.model.job.Job import org.groundplatform.domain.model.locationofinterest.LocationOfInterest @@ -29,12 +30,23 @@ class FakeLocationOfInterestRepository : LocationOfInterestRepositoryInterface { var offlineLoi = FakeDataGenerator.newLocationOfInterest() var hasValidLois = true + var latestLoiServerTimestamp = 0L + + val hasMissedRemoteDeletionsCall = FakeCall { false } + val syncLocationsOfInterestCall = FakeCall {} - override suspend fun syncLocationsOfInterest(survey: Survey) { + var lastSyncMode: SurveySyncMode? = null + + override suspend fun syncLocationsOfInterest(survey: Survey, mode: SurveySyncMode): Long { + lastSyncMode = mode syncLocationsOfInterestCall(survey) + return latestLoiServerTimestamp } + override suspend fun hasMissedRemoteDeletions(survey: Survey) = + hasMissedRemoteDeletionsCall(survey) + override suspend fun getOfflineLoi(surveyId: String, loiId: String): LocationOfInterest = offlineLoi diff --git a/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeSurveyRepository.kt b/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeSurveyRepository.kt index 4ddd59a085..71d98d75d7 100644 --- a/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeSurveyRepository.kt +++ b/core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeSurveyRepository.kt @@ -21,6 +21,8 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import org.groundplatform.domain.model.Survey import org.groundplatform.domain.model.SurveyListItem +import org.groundplatform.domain.model.SurveySyncMode +import org.groundplatform.domain.model.SurveySyncState import org.groundplatform.domain.model.User import org.groundplatform.domain.repository.SurveyRepositoryInterface @@ -41,6 +43,12 @@ class FakeSurveyRepository : SurveyRepositoryInterface { var remoteSurveys: List = emptyList() + var syncState: SurveySyncState? = null + + var lastRecordedSyncMode: SurveySyncMode? = null + + var lastRecordedLoiServerTimestamp: Long? = null + val remoteListItemsFlow = MutableStateFlow>(emptyList()) var remoteListItems: List get() = remoteListItemsFlow.value @@ -59,6 +67,17 @@ class FakeSurveyRepository : SurveyRepositoryInterface { offlineSurveys = offlineSurveys.filterNot { it.id == survey.id } + survey } + override suspend fun getSyncState(surveyId: String): SurveySyncState? = syncState + + override suspend fun recordSyncState( + survey: Survey, + mode: SurveySyncMode, + latestLoiServerTimestamp: Long, + ) { + lastRecordedSyncMode = mode + lastRecordedLoiServerTimestamp = latestLoiServerTimestamp + } + override suspend fun getRemoteSurvey(surveyId: String): Survey? = onGetRemoteSurveyCall(surveyId) override fun getRemoteSurveys(user: User): Flow> = remoteListItemsFlow