diff --git a/app/src/main/java/org/groundplatform/android/data/remote/firebase/schema/SurveysCollectionReference.kt b/app/src/main/java/org/groundplatform/android/data/remote/firebase/schema/SurveysCollectionReference.kt index 20fc6936e2..932a03ea83 100644 --- a/app/src/main/java/org/groundplatform/android/data/remote/firebase/schema/SurveysCollectionReference.kt +++ b/app/src/main/java/org/groundplatform/android/data/remote/firebase/schema/SurveysCollectionReference.kt @@ -18,9 +18,9 @@ package org.groundplatform.android.data.remote.firebase.schema import com.google.firebase.firestore.CollectionReference import com.google.firebase.firestore.FieldPath -import com.google.firebase.firestore.snapshots import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.tasks.await import org.groundplatform.android.data.remote.firebase.base.FluentCollectionReference import org.groundplatform.android.proto.Role import org.groundplatform.android.proto.Survey as SurveyProto @@ -39,27 +39,38 @@ class SurveysCollectionReference internal constructor(ref: CollectionReference) fun survey(id: String) = SurveyDocumentReference(reference().document(id)) + /** + * Emits the surveys readable by [user] once per collection. Deliberately a one-shot read rather + * than a realtime listener, which bills every document again on each reconnect. + */ fun getReadable(user: User): Flow> { val references = reference() .whereIn(STATE, ALLOWED_STATES) .whereIn(FieldPath.of(ACL_FIELD, user.email), ALLOWED_ROLES) - return references.snapshots().map { snapshot -> - snapshot.documents - .mapNotNull { SurveyConverter.toSurvey(it) } - .filter { survey -> - survey.generalAccess == Survey.GeneralAccess.GENERAL_ACCESS_UNSPECIFIED || - survey.generalAccess == Survey.GeneralAccess.RESTRICTED || - survey.generalAccess == Survey.GeneralAccess.UNLISTED - } + return flow { + val snapshot = references.get().await() + val surveys = + snapshot.documents + .mapNotNull { SurveyConverter.toSurvey(it) } + .filter { survey -> + survey.generalAccess == Survey.GeneralAccess.GENERAL_ACCESS_UNSPECIFIED || + survey.generalAccess == Survey.GeneralAccess.RESTRICTED || + survey.generalAccess == Survey.GeneralAccess.UNLISTED + } + emit(surveys) } } - fun getPublicReadable(): Flow> = - reference() - .whereEqualTo(GENERAL_ACCESS_FIELD, SurveyProto.GeneralAccess.PUBLIC_VALUE) - .whereIn(STATE, listOf(SurveyProto.State.READY_VALUE)) - .snapshots() - .map { snapshot -> snapshot.documents.mapNotNull(SurveyConverter::toSurvey) } + /** Emits the public surveys once per collection. See [getReadable]. */ + fun getPublicReadable(): Flow> = flow { + val snapshot = + reference() + .whereEqualTo(GENERAL_ACCESS_FIELD, SurveyProto.GeneralAccess.PUBLIC_VALUE) + .whereIn(STATE, listOf(SurveyProto.State.READY_VALUE)) + .get() + .await() + emit(snapshot.documents.mapNotNull(SurveyConverter::toSurvey)) + } } diff --git a/app/src/main/java/org/groundplatform/android/data/sync/SurveySyncService.kt b/app/src/main/java/org/groundplatform/android/data/sync/SurveySyncService.kt index e48c62f986..b1acd1a7ed 100644 --- a/app/src/main/java/org/groundplatform/android/data/sync/SurveySyncService.kt +++ b/app/src/main/java/org/groundplatform/android/data/sync/SurveySyncService.kt @@ -52,8 +52,10 @@ class SurveySyncService @Inject constructor(private val workManager: WorkManager companion object { /** * How long to wait before syncing. A single change often triggers several updates in quick - * succession, and each run re-reads the whole survey, so it is cheaper to let them settle. + * succession, and each run re-reads the whole survey, so it is cheaper to let them settle. Kept + * long to bound Firestore reads when many data collectors share a survey, as every change + * notifies each of their devices. */ - private const val SYNC_DELAY_MILLIS = 10_000L + private const val SYNC_DELAY_MILLIS = 30 * 60 * 1000L } } diff --git a/app/src/test/java/org/groundplatform/android/data/remote/firebase/schema/SurveysCollectionReferenceTest.kt b/app/src/test/java/org/groundplatform/android/data/remote/firebase/schema/SurveysCollectionReferenceTest.kt index a1e5cb1043..b4794144bb 100644 --- a/app/src/test/java/org/groundplatform/android/data/remote/firebase/schema/SurveysCollectionReferenceTest.kt +++ b/app/src/test/java/org/groundplatform/android/data/remote/firebase/schema/SurveysCollectionReferenceTest.kt @@ -16,16 +16,13 @@ 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.CollectionReference import com.google.firebase.firestore.DocumentSnapshot -import com.google.firebase.firestore.EventListener import com.google.firebase.firestore.FieldPath -import com.google.firebase.firestore.ListenerRegistration -import com.google.firebase.firestore.MetadataChanges import com.google.firebase.firestore.Query import com.google.firebase.firestore.QuerySnapshot -import java.util.concurrent.Executor import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest import org.groundplatform.android.FakeData @@ -62,16 +59,11 @@ class SurveysCollectionReferenceTest { whenever(mockCollectionReference.whereIn(any(), any())).thenReturn(mockQuery) whenever(mockCollectionReference.whereIn(any(), any())).thenReturn(mockQuery) + whenever(mockCollectionReference.whereEqualTo(any(), any())).thenReturn(mockQuery) whenever(mockQuery.whereIn(any(), any())).thenReturn(mockQuery) whenever(mockQuery.whereIn(any(), any())).thenReturn(mockQuery) - val mockRegistration: ListenerRegistration = mock() - whenever(mockQuery.addSnapshotListener(any(), any(), any())) - .thenAnswer { invocation -> - val listener = invocation.getArgument>(2) - listener.onEvent(mockQuerySnapshot, null) - mockRegistration - } + whenever(mockQuery.get()).thenReturn(Tasks.forResult(mockQuerySnapshot)) surveysCollectionReference = SurveysCollectionReference(mockCollectionReference) } @@ -142,6 +134,52 @@ class SurveysCollectionReferenceTest { ) } + @Test + fun `getPublicReadable returns public surveys`() = runTest { + val doc = createDocumentSnapshot("public", SurveyProto.GeneralAccess.PUBLIC) + whenever(mockQuerySnapshot.documents).thenReturn(listOf(doc)) + + val result = surveysCollectionReference.getPublicReadable().first() + + assertThat(result.map { it.id }).containsExactly("public") + } + + @Test + fun `getPublicReadable returns empty list when there are no public surveys`() = runTest { + whenever(mockQuerySnapshot.documents).thenReturn(emptyList()) + + val result = surveysCollectionReference.getPublicReadable().first() + + assertThat(result).isEmpty() + } + + @Test + fun `getPublicReadable queries for public access and ready state`() = runTest { + whenever(mockQuerySnapshot.documents).thenReturn(emptyList()) + + surveysCollectionReference.getPublicReadable().first() + + verify(mockCollectionReference) + .whereEqualTo( + eq(SurveyProto.GENERAL_ACCESS_FIELD_NUMBER.toString()), + eq(SurveyProto.GeneralAccess.PUBLIC_VALUE), + ) + verify(mockQuery) + .whereIn( + eq(SurveyProto.STATE_FIELD_NUMBER.toString()), + eq(listOf(SurveyProto.State.READY_VALUE)), + ) + } + + @Test + fun `getPublicReadable reads once instead of listening for changes`() = runTest { + whenever(mockQuerySnapshot.documents).thenReturn(emptyList()) + + surveysCollectionReference.getPublicReadable().first() + + verify(mockQuery).get() + } + private fun createDocumentSnapshot( id: String, generalAccess: SurveyProto.GeneralAccess,