[Patch] Fix errors on sync due to multiple instances of singleton (#2380

) * Simplify FirebaseFirestore injection * Rethrow fatal errors * Move logging into provider * Remove unused * Add logging, fix detekt issue
google · Mar 17, 2024 · 1a91d2a · 1a91d2a
1 parent ab262d0
commit 1a91d2a
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 32 deletions.
diff --git a/...n/java/com/google/android/ground/persistence/remote/firebase/FirebaseFirestoreProvider.kt b/...n/java/com/google/android/ground/persistence/remote/firebase/FirebaseFirestoreProvider.kt
@@ -22,12 +22,18 @@ import com.google.firebase.firestore.FirebaseFirestore
 import com.google.firebase.firestore.FirebaseFirestoreSettings
 import javax.inject.Inject
 import javax.inject.Singleton
+import timber.log.Timber
 
 @Singleton
 class FirebaseFirestoreProvider @Inject constructor(settings: FirebaseFirestoreSettings) :
   AsyncSingletonProvider<FirebaseFirestore>({
     FirebaseFirestore.getInstance().also {
-      it.firestoreSettings = settings
+      try {
+        it.firestoreSettings = settings
+      } catch (e: IllegalStateException) {
+        // Logging added for #2377.
+        Timber.e(e, "Singleton provider likely initialized multiple times")
+      }
       FirebaseFirestore.setLoggingEnabled(Config.FIRESTORE_LOGGING_ENABLED)
     }
   })
diff --git a/...src/main/java/com/google/android/ground/persistence/remote/firebase/FirestoreDataStore.kt b/...src/main/java/com/google/android/ground/persistence/remote/firebase/FirestoreDataStore.kt
@@ -27,6 +27,7 @@ import com.google.android.ground.model.mutation.SubmissionMutation
 import com.google.android.ground.model.submission.Submission
 import com.google.android.ground.model.toListItem
 import com.google.android.ground.persistence.remote.RemoteDataStore
+import com.google.android.ground.persistence.remote.firebase.schema.GroundFirestore
 import com.google.firebase.firestore.WriteBatch
 import com.google.firebase.functions.FirebaseFunctions
 import com.google.firebase.ktx.Firebase
@@ -49,11 +50,11 @@ class FirestoreDataStore
 @Inject
 internal constructor(
   private val firebaseFunctions: FirebaseFunctions,
-  private val groundFirestoreProvider: GroundFirestoreProvider,
+  private val firestoreProvider: FirebaseFirestoreProvider,
   @IoDispatcher private val ioDispatcher: CoroutineDispatcher
 ) : RemoteDataStore {
 
-  private suspend fun db() = groundFirestoreProvider.get()
+  private suspend fun db() = GroundFirestore(firestoreProvider.get())
 
   override suspend fun loadSurvey(surveyId: String): Survey =
     withContext(ioDispatcher) { db().surveys().survey(surveyId).get() }

diff --git a/...ain/java/com/google/android/ground/persistence/remote/firebase/GroundFirestoreProvider.kt b/...ain/java/com/google/android/ground/persistence/remote/firebase/GroundFirestoreProvider.kt
diff --git a/ground/src/main/java/com/google/android/ground/persistence/sync/SurveySyncWorker.kt b/ground/src/main/java/com/google/android/ground/persistence/sync/SurveySyncWorker.kt
@@ -52,10 +52,11 @@ constructor(
       Timber.d("Syncing survey $surveyId")
       syncSurvey(surveyId)
     } catch (e: Throwable) {
-      Timber.d(e, "Background survey sync failed")
       return if (this.runAttemptCount > MAX_SYNC_WORKER_RETRY_ATTEMPTS) {
+        Timber.v(e, "Survey sync failed too many times. Giving up.")
         Result.failure()
       } else {
+        Timber.v(e, "Survey sync. Retrying...")
         Result.retry()
       }
     }