Skip to content

Commit

Permalink
Prevent map from freezing while syncing changes to remote survey and …
Browse files Browse the repository at this point in the history
…LOIs (#2865)

* EntityState -> EntityDeletionState

* Minor renames

* Rename `state` in `SubmissionEntity`

* Missed renames

* More renames

* Inline function

* Keep feature manager in sync on clear

* Stop fetching jobs on main thread

* Optimize map updates
  • Loading branch information
gino-m authored Nov 22, 2024
1 parent ad4cdc0 commit 59ae309
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,14 @@ package com.google.android.ground.persistence.remote.firebase.schema
import com.google.android.ground.model.job.Job
import com.google.android.ground.persistence.remote.firebase.base.FluentCollectionReference
import com.google.firebase.firestore.CollectionReference
import kotlinx.coroutines.channels.awaitClose
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.callbackFlow
import kotlinx.coroutines.tasks.await
import kotlinx.coroutines.withContext

class JobCollectionReference internal constructor(ref: CollectionReference) :
FluentCollectionReference(ref) {
fun get(): Flow<List<Job>> = callbackFlow {
reference()
.get()
.addOnSuccessListener { trySend(it.documents.map { doc -> JobConverter.toJob(doc) }) }
.addOnFailureListener { trySend(listOf()) }

awaitClose {
// Cannot cancel or detach listeners here.
suspend fun get(): List<Job> =
withContext(ioDispatcher) {
val docs = reference().get().await()
docs.map { doc -> JobConverter.toJob(doc) }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package com.google.android.ground.persistence.remote.firebase.schema
import com.google.android.ground.model.Survey
import com.google.android.ground.persistence.remote.firebase.base.FluentDocumentReference
import com.google.firebase.firestore.DocumentReference
import kotlinx.coroutines.flow.firstOrNull
import kotlinx.coroutines.tasks.await

private const val LOIS = "lois"
Expand All @@ -36,8 +35,10 @@ class SurveyDocumentReference internal constructor(ref: DocumentReference) :
private fun jobs() = JobCollectionReference(reference().collection(JOBS))

suspend fun get(): Survey {
val document = reference().get().await()
val jobs = jobs().get().firstOrNull() ?: listOf()
return SurveyConverter.toSurvey(document, jobs)
val surveyDoc = reference().get().await()
// TODO(https://github.com/google/ground-android/issues/2864): Move jobs fetch to outside this
// DocumentReference class.
val jobs = jobs().get()
return SurveyConverter.toSurvey(surveyDoc, jobs)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import javax.inject.Inject
import javax.inject.Singleton
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.firstOrNull
import kotlinx.coroutines.flow.map
import timber.log.Timber
Expand Down Expand Up @@ -144,10 +143,6 @@ constructor(
fun getValidLois(survey: Survey): Flow<Set<LocationOfInterest>> =
localLoiStore.getValidLois(survey)

/** Returns a list of geometries associated with the given [Survey]. */
suspend fun getAllGeometries(survey: Survey): List<Geometry> =
getValidLois(survey).first().map { it.geometry }

/** Returns a flow of all [LocationOfInterest] within the map bounds (viewport). */
fun getWithinBounds(survey: Survey, bounds: Bounds): Flow<List<LocationOfInterest>> =
getValidLois(survey)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.merge
Expand Down Expand Up @@ -258,7 +259,7 @@ constructor(
}

// Compute the default viewport which includes all LOIs in the given survey.
val geometries = locationOfInterestRepository.getAllGeometries(survey)
val geometries = locationOfInterestRepository.getValidLois(survey).first().map { it.geometry }
return geometries.toBounds()?.let { NewCameraPositionViaBounds(bounds = it, padding = 100) }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,18 @@ import com.google.android.ground.ui.map.Feature
import com.google.android.ground.ui.map.FeatureType
import com.google.android.ground.ui.map.isLocationOfInterest
import javax.inject.Inject
import kotlin.time.Duration.Companion.milliseconds
import kotlinx.collections.immutable.toPersistentSet
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.debounce
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.flatMapLatest
Expand Down Expand Up @@ -127,12 +131,17 @@ internal constructor(
// TODO: Since we depend on survey stream from repo anyway, this transformation can be moved
// into the repository.

@OptIn(FlowPreview::class)
mapLoiFeatures =
activeSurvey.flatMapLatest {
if (it == null) flowOf(setOf())
else
getLocationOfInterestFeatures(it)
.combine(selectedLoiIdFlow, this::updatedLoiSelectedStates)
.debounce(1000.milliseconds)
.distinctUntilChanged()
.combine(selectedLoiIdFlow) { loiFeatures, selectedLoiId ->
updatedLoiSelectedStates(loiFeatures, selectedLoiId)
}
}

isZoomedInFlow =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ class GoogleMapsFragment : SupportMapFragment(), MapFragment {

override fun clear() {
map.clear()
featureManager.setFeatures(emptySet())
}

companion object {
Expand Down

0 comments on commit 59ae309

Please sign in to comment.