Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Mar 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() }
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
Expand Down