Reduce Firestore reads from survey sync and survey list

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<List<Survey>> {
     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<List<Survey>> =
-    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<List<Survey>> = 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))
+  }
 }

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
   }
 }

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<String>(), any())).thenReturn(mockQuery)
     whenever(mockCollectionReference.whereIn(any<FieldPath>(), any())).thenReturn(mockQuery)
+    whenever(mockCollectionReference.whereEqualTo(any<String>(), any())).thenReturn(mockQuery)
     whenever(mockQuery.whereIn(any<String>(), any())).thenReturn(mockQuery)
     whenever(mockQuery.whereIn(any<FieldPath>(), any())).thenReturn(mockQuery)
 
-    val mockRegistration: ListenerRegistration = mock()
-    whenever(mockQuery.addSnapshotListener(any<Executor>(), any<MetadataChanges>(), any()))
-      .thenAnswer { invocation ->
-        val listener = invocation.getArgument<EventListener<QuerySnapshot>>(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,