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

Fix "FirebaseFirestore has already been started" sync failures #2834

Merged
merged 3 commits into from
Nov 16, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -70,6 +70,6 @@ abstract class RemotePersistenceModule {
return FirebaseStorage.getInstance().reference
}

@Provides @Singleton fun firebaseFuntions(): FirebaseFunctions = Firebase.functions
@Provides @Singleton fun firebaseFunctions(): FirebaseFunctions = Firebase.functions
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import javax.inject.Singleton
@Singleton
class FirebaseFirestoreProvider @Inject constructor(settings: FirebaseFirestoreSettings) :
AsyncSingletonProvider<FirebaseFirestore>({
// WARNING: `FirebaseFirestore.getInstance()` should only be called here and nowhere
// else since settings can only be set on first call. Inject FirebaseFirestore instead
// of calling `getInstance()` directly.
FirebaseFirestore.getInstance().apply {
setFirestoreSettings(this, settings)
FirebaseFirestore.setLoggingEnabled(Config.FIRESTORE_LOGGING_ENABLED)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@
package com.google.android.ground.persistence.remote.firebase

import com.google.android.ground.persistence.uuid.OfflineUuidGenerator
import com.google.firebase.firestore.FirebaseFirestore
import javax.inject.Inject

class FirestoreUuidGenerator @Inject constructor() : OfflineUuidGenerator {
class FirestoreUuidGenerator
@Inject
constructor(private val firebaseFirestoreProvider: FirebaseFirestoreProvider) :
OfflineUuidGenerator {

// TODO: Check if this be replaced with the underlying implementation directly Util.autoId()
// Also remove Fake implementation if the dependency on firebase init is removed.
override fun generateUuid(): String =
FirebaseFirestore.getInstance().collection(ID_COLLECTION).document().id
override suspend fun generateUuid(): String =
firebaseFirestoreProvider.get().collection(ID_COLLECTION).document().id

companion object {
const val ID_COLLECTION = "/ids"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ interface OfflineUuidGenerator {
* Implementations should ensure that the probability of collision is so small to be considered
* insignificant.
*/
fun generateUuid(): String
suspend fun generateUuid(): String
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,18 @@ constructor(
private val rootDir: File?
get() = context.getExternalFilesDir(Environment.DIRECTORY_PICTURES)

private fun createImageFilename(fieldId: String): String =
private suspend fun createImageFilename(fieldId: String): String =
fieldId + "-" + uuidGenerator.generateUuid() + Config.PHOTO_EXT

fun createImageFile(fieldId: String): File = File(rootDir, createImageFilename(fieldId))
suspend fun createImageFile(fieldId: String): File = File(rootDir, createImageFilename(fieldId))

/**
* Creates a new file from bitmap and saves under external app directory.
*
* @throws IOException If path is not accessible or error occurs while saving file
*/
@Throws(IOException::class)
fun savePhoto(bitmap: Bitmap, fieldId: String): File =
suspend fun savePhoto(bitmap: Bitmap, fieldId: String): File =
createImageFile(fieldId).apply {
FileOutputStream(this).use { fos -> bitmap.compress(Bitmap.CompressFormat.JPEG, 100, fos) }
Timber.d("Photo saved %s : %b", path, exists())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,8 @@ internal constructor(

/** Persists the changes locally and enqueues a worker to sync with remote datastore. */
private fun saveChanges(deltas: List<ValueDelta>) {
val collectionId = offlineUuidGenerator.generateUuid()
externalScope.launch(ioDispatcher) {
val collectionId = offlineUuidGenerator.generateUuid()
submitDataUseCase.invoke(loiId, job, surveyId, deltas, customLoiName, collectionId)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import androidx.compose.runtime.remember
import androidx.compose.ui.platform.ComposeView
import androidx.compose.ui.res.stringResource
import androidx.core.content.FileProvider
import androidx.lifecycle.lifecycleScope
import com.google.android.ground.BuildConfig
import com.google.android.ground.R
import com.google.android.ground.coroutines.ApplicationScope
Expand Down Expand Up @@ -66,7 +67,7 @@ class PhotoTaskFragment : AbstractTaskFragment<PhotoTaskViewModel>() {
// Registers a callback to execute after a user captures a photo from the on-device camera.
private var capturePhotoLauncher: ActivityResultLauncher<Uri> =
registerForActivityResult(ActivityResultContracts.TakePicture()) { result: Boolean ->
if (result) viewModel.savePhotoTaskData(capturedPhotoUri)
externalScope.launch { if (result) viewModel.savePhotoTaskData(capturedPhotoUri) }
}

private var hasRequestedPermissionsOnResume = false
Expand Down Expand Up @@ -169,10 +170,12 @@ class PhotoTaskFragment : AbstractTaskFragment<PhotoTaskViewModel>() {
}

fun onTakePhoto() {
obtainCapturePhotoPermissions { launchPhotoCapture(viewModel.task.id) }
obtainCapturePhotoPermissions {
lifecycleScope.launch { launchPhotoCapture(viewModel.task.id) }
}
}

private fun launchPhotoCapture(taskId: String) {
private suspend fun launchPhotoCapture(taskId: String) {
try {
val photoFile = userMediaRepository.createImageFile(taskId)
capturedPhotoUri =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ constructor(
* Saves photo data stored on an on-device URI in Ground-associated storage and prepares it for
* inclusion in a data collection submission.
*/
fun savePhotoTaskData(uri: Uri) {
suspend fun savePhotoTaskData(uri: Uri) {
val currentTask = taskWaitingForPhoto
requireNotNull(currentTask) { "Photo captured but no task waiting for the result" }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.android.ground.ui.datacollection.tasks.point

import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.viewModelScope
import com.google.android.ground.model.geometry.Point
import com.google.android.ground.model.job.Job
import com.google.android.ground.model.job.getDefaultColor
Expand All @@ -29,6 +30,7 @@ import com.google.android.ground.ui.map.CameraPosition
import com.google.android.ground.ui.map.Feature
import com.google.android.ground.ui.map.FeatureType
import javax.inject.Inject
import kotlinx.coroutines.launch

class DropPinTaskViewModel
@Inject
Expand Down Expand Up @@ -66,13 +68,14 @@ constructor(
dropMarker(point)
}

private fun dropMarker(point: Point) {
val feature = createFeature(point)
features.postValue(setOf(feature))
}
private fun dropMarker(point: Point) =
viewModelScope.launch {
val feature = createFeature(point)
features.postValue(setOf(feature))
}

/** Creates a new map [Feature] representing the point placed by the user. */
private fun createFeature(point: Point): Feature =
private suspend fun createFeature(point: Point): Feature =
Feature(
id = uuidGenerator.generateUuid(),
type = FeatureType.USER_POINT.ordinal,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import timber.log.Timber

@SharedViewModel
Expand Down Expand Up @@ -178,21 +179,22 @@ internal constructor(
}

/** Updates the [Feature] drawn on map based on the value of [vertices]. */
private fun refreshMap() {
_draftArea.value =
if (vertices.isEmpty()) {
null
} else {
Feature(
id = uuidGenerator.generateUuid(),
type = FeatureType.USER_POLYGON.ordinal,
geometry = LineString(vertices),
style = Feature.Style(strokeColor, Feature.VertexStyle.CIRCLE),
clusterable = false,
selected = true,
)
}
}
private fun refreshMap() =
viewModelScope.launch {
_draftArea.value =
if (vertices.isEmpty()) {
null
} else {
Feature(
id = uuidGenerator.generateUuid(),
type = FeatureType.USER_POLYGON.ordinal,
geometry = LineString(vertices),
style = Feature.Style(strokeColor, Feature.VertexStyle.CIRCLE),
clusterable = false,
selected = true,
)
}
}

override fun validate(task: Task, taskData: TaskData?): Int? {
// Invalid response for draw area task.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ import com.google.android.ground.persistence.uuid.OfflineUuidGenerator
import javax.inject.Inject

class FakeUuidGenerator @Inject internal constructor() : OfflineUuidGenerator {
override fun generateUuid(): String = "TEST UUID"
override suspend fun generateUuid(): String = "TEST UUID"
}
Loading