Merge branch 'master' into anandwana001/2749/remove-dynamic-view

ground/src/main/java/com/google/android/ground/Config.kt

@@ -44,9 +44,6 @@ object Config {
    */
   const val CLUSTERING_ZOOM_THRESHOLD = 14f
 
-  /** Maximum number of attempts for retrying unsuccessful media uploads. */
-  const val MAX_MEDIA_UPLOAD_RETRY_COUNT = 5
-
   // TODO(#1730): Make sub-paths configurable and stop hardcoding here.
   const val DEFAULT_MOG_TILE_LOCATION = "/offline-imagery/default"
   private const val DEFAULT_MOG_MIN_ZOOM = 8

ground/src/main/java/com/google/android/ground/ExcludeFromJacocoGeneratedReport.kt

@@ -0,0 +1,21 @@
+/*
+ * Copyright 2024 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 com.google.android.ground
+
+// https://stackoverflow.com/questions/47824761/how-would-i-add-an-annotation-to-exclude-a-method-from-a-jacoco-code-coverage-re
+@Retention(AnnotationRetention.RUNTIME)
+@Target(AnnotationTarget.FUNCTION)
+annotation class ExcludeFromJacocoGeneratedReport

ground/src/main/java/com/google/android/ground/PermissionDeniedDialog.kt

@@ -108,6 +108,7 @@ private fun SignupLink(signupLink: String) {
 }
 
 @Preview
+@ExcludeFromJacocoGeneratedReport
 @Composable
 fun PermissionDeniedDialogWithSignupLinkPreview() {
   AppTheme {
@@ -116,6 +117,7 @@ fun PermissionDeniedDialogWithSignupLinkPreview() {
 }
 
 @Preview
+@ExcludeFromJacocoGeneratedReport
 @Composable
 fun PermissionDeniedDialogWithoutSignupLinkPreview() {
   AppTheme { PermissionDeniedDialog(signupLink = "", onSignOut = {}, onCloseApp = {}) }

ground/src/main/java/com/google/android/ground/model/mutation/SubmissionMutation.kt

@@ -38,15 +38,8 @@ data class SubmissionMutation(
 
   override fun toString(): String = super.toString() + "deltas= $deltas"
 
-  fun incrementRetryCount() = this.copy(retryCount = this.retryCount + 1)
-
   fun updateSyncStatus(status: SyncStatus) = this.copy(syncStatus = status)
 
-  /** Returns true if this mutation is in a state in which it is ready for media upload. */
-  fun mediaUploadPending() =
-    this.syncStatus == SyncStatus.MEDIA_UPLOAD_PENDING ||
-      this.syncStatus == SyncStatus.MEDIA_UPLOAD_AWAITING_RETRY
-
   fun getPhotoData(): List<PhotoTaskData> =
     deltas.map { it.newTaskData }.filterIsInstance<PhotoTaskData>().filter { !it.isEmpty() }
 }

ground/src/main/java/com/google/android/ground/model/submission/UploadQueueEntry.kt

@@ -0,0 +1,36 @@
+/*
+ * Copyright 2024 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 com.google.android.ground.model.submission
+
+import com.google.android.ground.model.mutation.LocationOfInterestMutation
+import com.google.android.ground.model.mutation.Mutation
+import com.google.android.ground.model.mutation.SubmissionMutation
+import java.util.Date
+
+/**
+ * A set of changes to be applied to the remote datastore, initiated by the user by completing the
+ * data collection flow and clicking "Submit".
+ */
+data class UploadQueueEntry(
+  val userId: String,
+  val clientTimestamp: Date,
+  val uploadStatus: Mutation.SyncStatus,
+  val loiMutation: LocationOfInterestMutation?,
+  val submissionMutation: SubmissionMutation?,
+) {
+  fun mutations(): List<Mutation> = listOfNotNull(loiMutation, submissionMutation)
+}

ground/src/main/java/com/google/android/ground/persistence/sync/LocalMutationSyncWorker.kt

@@ -18,29 +18,23 @@ package com.google.android.ground.persistence.sync
 import android.content.Context
 import androidx.hilt.work.HiltWorker
 import androidx.work.CoroutineWorker
-import androidx.work.Data
 import androidx.work.ListenableWorker.Result.retry
 import androidx.work.ListenableWorker.Result.success
 import androidx.work.WorkerParameters
-import com.google.android.ground.model.User
 import com.google.android.ground.model.mutation.Mutation
-import com.google.android.ground.persistence.local.room.fields.MutationEntitySyncStatus.FAILED
-import com.google.android.ground.persistence.local.room.fields.MutationEntitySyncStatus.IN_PROGRESS
-import com.google.android.ground.persistence.local.room.fields.MutationEntitySyncStatus.PENDING
 import com.google.android.ground.persistence.remote.RemoteDataStore
-import com.google.android.ground.persistence.sync.LocalMutationSyncWorker.Companion.createInputData
 import com.google.android.ground.repository.MutationRepository
 import com.google.android.ground.repository.UserRepository
+import com.google.android.ground.util.priority
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.withContext
 import timber.log.Timber
 
 /**
- * A worker that syncs local changes to the remote data store. Each instance handles mutations for a
- * specific map location of interest, whose id is provided in the [Data] object built by
- * [createInputData] and provided to the worker request while being enqueued.
+ * A worker that uploads all pending local changes to the remote data store. Larger uploads (photos)
+ * are then delegated to [MediaUploadWorkManager], which is enqueued and run in parallel.
  */
 @HiltWorker
 class LocalMutationSyncWorker
@@ -54,73 +48,39 @@ constructor(
   private val userRepository: UserRepository,
 ) : CoroutineWorker(context, params) {
 
-  private val locationOfInterestId: String =
-    params.inputData.getString(LOCATION_OF_INTEREST_ID_PARAM_KEY)!!
-
-  override suspend fun doWork(): Result = withContext(Dispatchers.IO) { doWorkInternal() }
-
-  private suspend fun doWorkInternal(): Result =
-    try {
-      val mutations = getIncompleteMutations()
-      Timber.d("Syncing ${mutations.size} changes for LOI $locationOfInterestId")
-      val result = processMutations(mutations)
-      mediaUploadWorkManager.enqueueSyncWorker(locationOfInterestId)
-      if (result) success() else retry()
-    } catch (t: Throwable) {
-      Timber.e(t, "Failed to sync changes for LOI $locationOfInterestId")
-      retry()
+  override suspend fun doWork(): Result =
+    withContext(Dispatchers.IO) {
+      val queue = mutationRepository.getIncompleteUploads()
+      Timber.d("Uploading ${queue.size} additions / changes")
+      val results = queue.map { processMutations(it.mutations()) }
+      if (results.any { it }) mediaUploadWorkManager.enqueueSyncWorker()
+      if (results.all { it }) success() else retry()
     }
 
   /**
-   * Attempts to fetch all mutations from the [MutationRepository] that are `PENDING`, `FAILED`, or
-   * `IN_PROGRESS` state. The latter should never occur since only on worker should be scheduled per
-   * LOI at a given time.
-   */
-  private suspend fun getIncompleteMutations(): List<Mutation> =
-    mutationRepository.getMutations(locationOfInterestId, PENDING, FAILED, IN_PROGRESS)
-
-  /**
-   * Applies mutations to remote data store. Once successful, removes them from the local db.
+   * Applies mutations to remote data store, updating their status in the queue accordingly. Catches
+   * and handles all exceptions.
    *
-   * @return `true` if the mutations were successfully synced with [RemoteDataStore].
+   * @return `true` if all mutations were successfully synced with [RemoteDataStore], `false` if at
+   *   least one failed.
    */
   private suspend fun processMutations(mutations: List<Mutation>): Boolean {
     if (mutations.isEmpty()) return true
-    return try {
+    try {
       val user = userRepository.getAuthenticatedUser()
-      filterMutationsByUser(mutations, user)
-        .takeIf { it.isNotEmpty() }
-        ?.let {
-          mutationRepository.markAsInProgress(it)
-          remoteDataStore.applyMutations(it, user)
-          mutationRepository.finalizePendingMutationsForMediaUpload(it)
-        }
-      true
+      mutationRepository.markAsInProgress(mutations)
+      // TODO(https://github.com/google/ground-android/issues/2883):
+      //   Apply mutations via repository layer rather than accessing data store directly.
+      remoteDataStore.applyMutations(mutations, user)
+      mutationRepository.finalizePendingMutationsForMediaUpload(mutations)
+
+      return true
     } catch (t: Throwable) {
       // Mark all mutations as having failed since the remote datastore only commits when all
       // mutations have succeeded.
       mutationRepository.markAsFailed(mutations, t)
-      Timber.e(t, "Failed to sync survey ${mutations.first().surveyId}")
-      false
-    }
-  }
-
-  private fun filterMutationsByUser(mutations: List<Mutation>, user: User): List<Mutation> {
-    val userIds = mutations.map { it.userId }.toSet()
-    if (userIds.size != 1) {
-      Timber.e("Expected exactly 1 user, but found ${userIds.size}")
+      Timber.log(t.priority(), t, "Failed to sync local data")
+      return false
     }
-    val (validMutations, invalidMutations) = mutations.partition { it.userId == user.id }
-    invalidMutations.forEach { Timber.e("Invalid mutation: $it") }
-    return validMutations
-  }
-
-  companion object {
-    internal const val LOCATION_OF_INTEREST_ID_PARAM_KEY = "locationOfInterestId"
-
-    /** Returns a new work [Data] object containing the specified location of interest id. */
-    @JvmStatic
-    fun createInputData(locationOfInterestId: String): Data =
-      Data.Builder().putString(LOCATION_OF_INTEREST_ID_PARAM_KEY, locationOfInterestId).build()
   }
 }

ground/src/main/java/com/google/android/ground/persistence/sync/MediaUploadWorkManager.kt

@@ -19,7 +19,6 @@ import androidx.work.ExistingWorkPolicy
 import androidx.work.NetworkType
 import androidx.work.WorkManager
 import com.google.android.ground.persistence.local.LocalValueStore
-import com.google.android.ground.persistence.sync.MediaUploadWorker.Companion.createInputData
 import javax.inject.Inject
 
 /** Enqueues media upload work to be performed in the background. */
@@ -38,13 +37,12 @@ constructor(private val workManager: WorkManager, private val localValueStore: L
    * This method returns as soon as the worker is added to the work queue, not when the work
    * completes.
    */
-  fun enqueueSyncWorker(locationOfInterestId: String) {
-    val workInputData = createInputData(locationOfInterestId)
+  fun enqueueSyncWorker() {
     val request =
       WorkRequestBuilder()
         .setWorkerClass(MediaUploadWorker::class.java)
         .setNetworkType(preferredNetworkType())
-        .buildWorkerRequest(workInputData)
+        .buildWorkerRequest()
     workManager.enqueueUniqueWork(
       MediaUploadWorker::class.java.name,
       ExistingWorkPolicy.APPEND_OR_REPLACE,