Remove duplicated survey downloads during sync

app/src/main/java/org/groundplatform/android/repository/LocationOfInterestRepository.kt

@@ -65,10 +65,14 @@ constructor(
     // Single-page buffering. Persist immediately to avoid OOM on geometry-heavy surveys.
     val syncedLoiIds = mutableSetOf<String>()
     syncedLoiIds += savePages(remoteDataStore.loadPredefinedLois(survey))
-    syncedLoiIds += savePages(remoteDataStore.loadUserLois(survey, ownerUserId))
-    if (survey.dataVisibility == Survey.DataVisibility.ALL_SURVEY_PARTICIPANTS) {
-      syncedLoiIds += savePages(remoteDataStore.loadSharedLois(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))
+      }
 
     val mutations = localLoiStore.getAllSurveyMutations(survey).firstOrNull().orEmpty()
 

app/src/test/java/org/groundplatform/android/data/remote/FakeRemoteDataStore.kt

@@ -18,6 +18,7 @@ package org.groundplatform.android.data.remote
 import javax.inject.Inject
 import javax.inject.Singleton
 import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.flowOf
 import org.groundplatform.domain.model.Survey
 import org.groundplatform.domain.model.SurveyListItem
@@ -26,6 +27,7 @@ import org.groundplatform.domain.model.User
 import org.groundplatform.domain.model.locationofinterest.LocationOfInterest
 import org.groundplatform.domain.model.mutation.Mutation
 import org.groundplatform.domain.model.toListItem
+import org.groundplatform.testing.FakeCall
 
 @Singleton
 class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore {
@@ -51,6 +53,9 @@ class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore {
   var applyMutationError: Error? = null
 
   val subscribedSurveyIds = mutableSetOf<String>()
+  val loadUserLoisCall = FakeCall<Survey, List<LocationOfInterest>> { userLois }
+
+  val loadSharedLoisCall = FakeCall<Survey, List<LocationOfInterest>> { sharedLois }
 
   override fun getRestrictedSurveyList(user: User): Flow<List<SurveyListItem>> =
     flowOf(surveys.map { it.toListItem(false) })
@@ -84,7 +89,11 @@ class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore {
   }
 
   override fun loadUserLois(survey: Survey, ownerUserId: String): Flow<List<LocationOfInterest>> =
-    flowOf(userLois)
+    flow {
+      emit(loadUserLoisCall(survey))
+    }
 
-  override fun loadSharedLois(survey: Survey): Flow<List<LocationOfInterest>> = flowOf(sharedLois)
+  override fun loadSharedLois(survey: Survey): Flow<List<LocationOfInterest>> = flow {
+    emit(loadSharedLoisCall(survey))
+  }
 }

app/src/test/java/org/groundplatform/android/repository/LocationOfInterestRepositoryTest.kt

@@ -86,6 +86,10 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() {
       fakeRemoteDataStore.predefinedLois = TEST_LOCATIONS_OF_INTEREST
       activateSurvey(TEST_SURVEY.id)
       advanceUntilIdle()
+
+      // Clear query records
+      fakeRemoteDataStore.loadUserLoisCall.reset()
+      fakeRemoteDataStore.loadSharedLoisCall.reset()
     }
   }
 
@@ -307,7 +311,7 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() {
       val sharedLoi = FakeData.LOCATION_OF_INTEREST.copy(id = "shared_id")
       fakeRemoteDataStore.predefinedLois = listOf(predefinedLoi)
       fakeRemoteDataStore.userLois = listOf(userLoi)
-      fakeRemoteDataStore.sharedLois = listOf(sharedLoi)
+      fakeRemoteDataStore.sharedLois = listOf(userLoi, sharedLoi)
 
       val expected = setOf(predefinedLoi, userLoi, sharedLoi)
 
@@ -318,6 +322,22 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() {
       assertThat(actual).isEqualTo(expected)
     }
 
+  @Test
+  fun `should not query user LOIs separately when shared LOIs already include them`() =
+    runWithTestDispatcher {
+      val survey = TEST_SURVEY.copy(dataVisibility = Survey.DataVisibility.ALL_SURVEY_PARTICIPANTS)
+      fakeRemoteDataStore.surveys = listOf(survey)
+
+      val userLoi = FakeData.LOCATION_OF_INTEREST.copy(id = "user_id")
+      fakeRemoteDataStore.userLois = listOf(userLoi)
+      fakeRemoteDataStore.sharedLois = listOf(userLoi)
+
+      syncSurvey(survey.id)
+
+      assertThat(fakeRemoteDataStore.loadUserLoisCall.callCount).isEqualTo(0)
+      assertThat(fakeRemoteDataStore.loadSharedLoisCall.callCount).isEqualTo(1)
+    }
+
   @Test
   fun `should not load shared LOIs when visibility is not ALL_SURVEY_PARTICIPANTS`() =
     runWithTestDispatcher {
@@ -337,8 +357,22 @@ class LocationOfInterestRepositoryTest : BaseHiltTest() {
       val actual = locationOfInterestRepository.getValidLois(survey).first()
 
       assertThat(actual).isEqualTo(expected)
+      assertThat(fakeRemoteDataStore.loadUserLoisCall.callCount).isEqualTo(1)
+      assertThat(fakeRemoteDataStore.loadSharedLoisCall.callCount).isEqualTo(0)
     }
 
+  @Test
+  fun `should only query user LOIs when visibility is UNSPECIFIED`() = runWithTestDispatcher {
+    val survey = TEST_SURVEY.copy(dataVisibility = Survey.DataVisibility.UNSPECIFIED)
+    fakeRemoteDataStore.surveys = listOf(survey)
+    fakeRemoteDataStore.userLois = listOf(FakeData.LOCATION_OF_INTEREST.copy(id = "user_id"))
+
+    syncSurvey(survey.id)
+
+    assertThat(fakeRemoteDataStore.loadUserLoisCall.callCount).isEqualTo(1)
+    assertThat(fakeRemoteDataStore.loadSharedLoisCall.callCount).isEqualTo(0)
+  }
+
   companion object {
     private val COORDINATE_1 = Coordinates(-20.0, -20.0)
     private val COORDINATE_2 = Coordinates(0.0, 0.0)

core/testing/src/commonMain/kotlin/org/groundplatform/testing/FakeCall.kt

@@ -38,6 +38,10 @@ class FakeCall<I, O>(private var behavior: suspend (I) -> O) {
     return behavior(input)
   }
 
+  fun reset() {
+    _calls.clear()
+  }
+
   /** Replaces the current behavior with [newBehavior]. */
   fun overrideBehavior(newBehavior: suspend (I) -> O) {
     behavior = newBehavior