From a7655dc4387d662b35861c5d1f7d574dd673c8e1 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Thu, 22 Oct 2026 17:24:25 +0200 Subject: [PATCH 1/3] Reduce Firestore reads from survey sync and survey list --- .../schema/SurveysCollectionReference.kt | 43 ++++++++++++------- .../android/data/sync/SurveySyncService.kt | 5 ++- .../schema/SurveysCollectionReferenceTest.kt | 13 +----- 3 files changed, 32 insertions(+), 29 deletions(-) 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..1ffaf6f74d 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,9 @@ 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..8de1563477 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 @@ -65,13 +62,7 @@ class SurveysCollectionReferenceTest { 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) } From 8802dfd3f2d0be6f268e94e85244f437dc3cb340 Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Thu, 22 Oct 2026 17:47:53 +0200 Subject: [PATCH 2/3] Fix code quality checks failing on SurveySyncService --- .../groundplatform/android/data/sync/SurveySyncService.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 1ffaf6f74d..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,9 @@ 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. Kept long to bound Firestore reads when many data - * collectors share a survey, as every change notifies each of their devices. + * 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 = 30 * 60 * 1000L } From 419fbb467c127461a7eb608714f1e7539a3ce44e Mon Sep 17 00:00:00 2001 From: Roberto Fontanarosa Date: Thu, 22 Oct 2026 17:51:30 +0200 Subject: [PATCH 3/3] Add tests for getPublicReadable one-shot read --- .../schema/SurveysCollectionReferenceTest.kt | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) 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 8de1563477..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 @@ -59,6 +59,7 @@ 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) @@ -133,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,