From 041ade5af89865ca5cd634940bf2525b0d7d52b0 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Sun, 20 Aug 2023 12:26:00 +0530 Subject: [PATCH 1/6] Update ktdoc and directly use the util method for network check --- .../remote/firebase/base/FluentCollectionReference.kt | 9 ++------- .../com/google/android/ground/system/NetworkManager.kt | 5 +---- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/base/FluentCollectionReference.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/base/FluentCollectionReference.kt index 0b7e9c1f4f..9ce642c1da 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/base/FluentCollectionReference.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/base/FluentCollectionReference.kt @@ -32,12 +32,7 @@ protected constructor( protected val defaultDispatcher: CoroutineDispatcher = Dispatchers.Default ) { - /** - * Returns a Completable that completes immediately on subscribe if network is available, or fails - * in error if not. - */ - private fun requireActiveNetwork() = - requireActiveNetwork(reference.firestore.app.applicationContext) + private val context = reference.firestore.app.applicationContext /** * Runs the specified query, returning a Single containing a List of values created by applying @@ -48,7 +43,7 @@ protected constructor( query: Query, mappingFunction: Function ): List { - requireActiveNetwork() + requireActiveNetwork(context) val querySnapshot = query.get().await() return querySnapshot.documents.filter { it.exists() }.map { mappingFunction.apply(it) } } diff --git a/ground/src/main/java/com/google/android/ground/system/NetworkManager.kt b/ground/src/main/java/com/google/android/ground/system/NetworkManager.kt index 6609a53aaa..efbaf788b3 100644 --- a/ground/src/main/java/com/google/android/ground/system/NetworkManager.kt +++ b/ground/src/main/java/com/google/android/ground/system/NetworkManager.kt @@ -31,10 +31,7 @@ object NetworkManager { return networkInfo?.isConnected ?: false } - /** - * Returns a Completable that completes immediately on subscribe if network is available, or fails - * in error if not. - */ + /** Throws an error if network isn't available. */ fun requireActiveNetwork(context: Context) { if (!isNetworkAvailable(context)) { throw ConnectException() From 5f2f339b25f56361767d8e76e71589cfa73a600c Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Sun, 20 Aug 2023 12:29:59 +0530 Subject: [PATCH 2/6] Remove unused method --- .../remote/firebase/base/FluentFirestore.kt | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/base/FluentFirestore.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/base/FluentFirestore.kt index 79139d04ee..3692bee225 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/base/FluentFirestore.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/base/FluentFirestore.kt @@ -16,14 +16,8 @@ package com.google.android.ground.persistence.remote.firebase.base -import com.google.android.ground.rx.annotations.Cold -import com.google.firebase.firestore.DocumentSnapshot import com.google.firebase.firestore.FirebaseFirestore -import com.google.firebase.firestore.QuerySnapshot import com.google.firebase.firestore.WriteBatch -import io.reactivex.Maybe -import io.reactivex.Single -import java8.util.function.Function /** Base class for representing Firestore databases as object hierarchies. */ open class FluentFirestore protected constructor(private val db: FirebaseFirestore) { @@ -32,20 +26,4 @@ open class FluentFirestore protected constructor(private val db: FirebaseFiresto // TODO: Wrap in fluent version of WriteBatch. fun batch(): WriteBatch = db.batch() - - companion object { - /** - * Applies the provided mapping function to each document in the specified query snapshot, if - * present. If no results are present, completes with an empty list. - */ - fun toSingleList( - result: Maybe, - mappingFunction: Function - ): @Cold Single> = - result - .map { querySnapshot: QuerySnapshot -> - querySnapshot.documents.map { mappingFunction.apply(it) } - } - .toSingle(emptyList()) - } } From c30badccefc7472bb9679d63413b4c34d505c901 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Sun, 20 Aug 2023 12:38:02 +0530 Subject: [PATCH 3/6] Remove unused method for getting submissions by loi --- .../firebase/schema/SubmissionDocumentReference.kt | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/SubmissionDocumentReference.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/SubmissionDocumentReference.kt index c01773b9c2..44a0286038 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/SubmissionDocumentReference.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/SubmissionDocumentReference.kt @@ -17,24 +17,14 @@ package com.google.android.ground.persistence.remote.firebase.schema import com.google.android.ground.model.User -import com.google.android.ground.model.locationofinterest.LocationOfInterest import com.google.android.ground.model.mutation.Mutation import com.google.android.ground.model.mutation.SubmissionMutation -import com.google.android.ground.model.submission.Submission import com.google.android.ground.persistence.remote.firebase.base.FluentDocumentReference -import com.google.android.ground.rx.annotations.Cold import com.google.firebase.firestore.DocumentReference -import com.google.firebase.firestore.DocumentSnapshot import com.google.firebase.firestore.WriteBatch -import durdinapps.rxfirebase2.RxFirestore -import io.reactivex.Maybe class SubmissionDocumentReference internal constructor(ref: DocumentReference) : FluentDocumentReference(ref) { - operator fun get(locationOfInterest: LocationOfInterest): @Cold Maybe = - RxFirestore.getDocument(reference()).map { doc: DocumentSnapshot -> - SubmissionConverter.toSubmission(locationOfInterest, doc) - } /** Appends the operation described by the specified mutation to the provided write batch. */ fun addMutationToBatch(mutation: SubmissionMutation, user: User, batch: WriteBatch) { From e6108876144893c97d04bd51819a83c6e1ee99c2 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Sun, 20 Aug 2023 12:51:32 +0530 Subject: [PATCH 4/6] Remove last remaining usage of RxFirebase This was used for streaming lois but has been replaced by sync service --- .../persistence/remote/RemoteDataEvent.kt | 54 ----------------- .../persistence/remote/RemoteDataStore.kt | 10 ---- .../remote/firebase/FirestoreDataStore.kt | 16 ----- .../firebase/schema/LoiCollectionReference.kt | 19 ------ .../firebase/schema/QuerySnapshotConverter.kt | 59 ------------------- .../persistence/remote/FakeRemoteDataStore.kt | 8 --- 6 files changed, 166 deletions(-) delete mode 100644 ground/src/main/java/com/google/android/ground/persistence/remote/RemoteDataEvent.kt delete mode 100644 ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/QuerySnapshotConverter.kt diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/RemoteDataEvent.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/RemoteDataEvent.kt deleted file mode 100644 index 702dc4e0a5..0000000000 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/RemoteDataEvent.kt +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright 2018 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.persistence.remote - -import kotlin.Result.Companion.failure -import kotlin.Result.Companion.success - -/** - * An event returned by the remote data store indicating either an entity was successfully loaded, - * or that it was modified or removed in the remote data store. - * - * @param the type of entity being loaded, modified, or removed. - */ -class RemoteDataEvent -private constructor(val eventType: EventType, val result: Result>) { - - enum class EventType { - ENTITY_LOADED, - ENTITY_MODIFIED, - ENTITY_REMOVED, - ERROR - } - - companion object { - @JvmStatic - fun loaded(entityId: String, entity: T): RemoteDataEvent = - RemoteDataEvent(EventType.ENTITY_LOADED, success(Pair(entityId, entity))) - - @JvmStatic - fun modified(entityId: String, entity: T): RemoteDataEvent = - RemoteDataEvent(EventType.ENTITY_MODIFIED, success(Pair(entityId, entity))) - - @JvmStatic - fun removed(entityId: String): RemoteDataEvent = - RemoteDataEvent(EventType.ENTITY_REMOVED, success(Pair(entityId, null))) - - @JvmStatic - fun error(error: Throwable): RemoteDataEvent = - RemoteDataEvent(EventType.ERROR, failure(error)) - } -} diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/RemoteDataStore.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/RemoteDataStore.kt index 891a67fb05..7713c691ab 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/RemoteDataStore.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/RemoteDataStore.kt @@ -21,8 +21,6 @@ import com.google.android.ground.model.User import com.google.android.ground.model.locationofinterest.LocationOfInterest import com.google.android.ground.model.mutation.Mutation import com.google.android.ground.model.submission.Submission -import com.google.android.ground.rx.annotations.Cold -import io.reactivex.Flowable /** * Defines API for accessing data in a remote data store. Implementations must ensure all @@ -43,14 +41,6 @@ interface RemoteDataStore { */ suspend fun loadTermsOfService(): TermsOfService? - /** - * Returns all LOIs in the specified survey, then continues to emit any remote updates to the set - * of LOIs in the survey until all subscribers have been disposed. - */ - fun loadLocationsOfInterestOnceAndStreamChanges( - survey: Survey - ): @Cold(stateful = true, terminates = false) Flowable> - /** Returns all LOIs in the specified survey. Main-safe. */ suspend fun loadLocationsOfInterest(survey: Survey): List diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/FirestoreDataStore.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/FirestoreDataStore.kt index 202e06e790..b29527979f 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/FirestoreDataStore.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/FirestoreDataStore.kt @@ -25,18 +25,15 @@ import com.google.android.ground.model.mutation.Mutation import com.google.android.ground.model.mutation.SubmissionMutation import com.google.android.ground.model.submission.Submission import com.google.android.ground.persistence.remote.DataStoreException -import com.google.android.ground.persistence.remote.RemoteDataEvent import com.google.android.ground.persistence.remote.RemoteDataStore import com.google.android.ground.persistence.remote.firebase.schema.GroundFirestore import com.google.android.ground.rx.Schedulers -import com.google.android.ground.rx.annotations.Cold import com.google.android.ground.system.ApplicationErrorManager import com.google.firebase.crashlytics.FirebaseCrashlytics import com.google.firebase.firestore.WriteBatch import com.google.firebase.functions.FirebaseFunctions import com.google.firebase.ktx.Firebase import com.google.firebase.messaging.ktx.messaging -import io.reactivex.Flowable import javax.inject.Inject import javax.inject.Singleton import kotlinx.coroutines.CoroutineDispatcher @@ -87,19 +84,6 @@ internal constructor( override suspend fun loadSurveySummaries(user: User): List = withContext(ioDispatcher) { db.surveys().getReadable(user) } - override fun loadLocationsOfInterestOnceAndStreamChanges( - survey: Survey - ): @Cold(stateful = true, terminates = false) Flowable> = - db - .surveys() - .survey(survey.id) - .lois() - .loadOnceAndStreamChanges(survey) - .onErrorResumeNext { e: Throwable -> - if (shouldInterceptException(e)) Flowable.never() else Flowable.error(e) - } - .subscribeOn(schedulers.io()) - override suspend fun loadLocationsOfInterest(survey: Survey) = db.surveys().survey(survey.id).lois().locationsOfInterest(survey) diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/LoiCollectionReference.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/LoiCollectionReference.kt index b42a87718c..fd22f55ea7 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/LoiCollectionReference.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/LoiCollectionReference.kt @@ -18,15 +18,10 @@ package com.google.android.ground.persistence.remote.firebase.schema import com.google.android.ground.model.Survey import com.google.android.ground.model.locationofinterest.LocationOfInterest -import com.google.android.ground.persistence.remote.RemoteDataEvent import com.google.android.ground.persistence.remote.firebase.base.FluentCollectionReference import com.google.android.ground.persistence.remote.firebase.schema.LoiConverter.toLoi -import com.google.android.ground.rx.annotations.Cold import com.google.firebase.firestore.CollectionReference -import com.google.firebase.firestore.DocumentSnapshot import com.google.firebase.firestore.QuerySnapshot -import durdinapps.rxfirebase2.RxFirestore -import io.reactivex.Flowable import kotlinx.coroutines.tasks.await import kotlinx.coroutines.withContext import timber.log.Timber @@ -34,14 +29,6 @@ import timber.log.Timber class LoiCollectionReference internal constructor(ref: CollectionReference) : FluentCollectionReference(ref) { - /** Retrieves all lois in the survey, then streams changes to the remote db incrementally. */ - fun loadOnceAndStreamChanges( - survey: Survey - ): @Cold(terminates = false) Flowable> = - RxFirestore.observeQueryRef(reference()).flatMapIterable { snapshot: QuerySnapshot -> - toRemoteDataEvents(survey, snapshot) - } - fun loi(id: String) = LoiDocumentReference(reference().document(id)) /** Retrieves all LOIs in the specified survey. Main-safe. */ @@ -55,10 +42,4 @@ class LoiCollectionReference internal constructor(ref: CollectionReference) : // Filter out bad results and log. .mapNotNull { it.onFailure { t -> Timber.e(t) }.getOrNull() } } - - private fun toRemoteDataEvents( - survey: Survey, - snapshot: QuerySnapshot - ): Iterable> = - QuerySnapshotConverter.toEvents(snapshot) { doc: DocumentSnapshot -> toLoi(survey, doc) } } diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/QuerySnapshotConverter.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/QuerySnapshotConverter.kt deleted file mode 100644 index 2a7c51ed6c..0000000000 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/QuerySnapshotConverter.kt +++ /dev/null @@ -1,59 +0,0 @@ -/* - * Copyright 2020 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.persistence.remote.firebase.schema - -import com.google.android.ground.persistence.remote.DataStoreException -import com.google.android.ground.persistence.remote.RemoteDataEvent -import com.google.android.ground.persistence.remote.RemoteDataEvent.Companion.error -import com.google.android.ground.persistence.remote.RemoteDataEvent.Companion.loaded -import com.google.android.ground.persistence.remote.RemoteDataEvent.Companion.modified -import com.google.android.ground.persistence.remote.RemoteDataEvent.Companion.removed -import com.google.firebase.firestore.DocumentChange -import com.google.firebase.firestore.DocumentSnapshot -import com.google.firebase.firestore.QuerySnapshot -import java8.util.function.Function -import timber.log.Timber - -/** - * Converts Firestore [com.google.firebase.firestore.QuerySnapshot] to application-specific objects. - */ -internal object QuerySnapshotConverter { - - /** Applies a converter function to document change events in the specified query snapshot. */ - fun toEvents( - snapshot: QuerySnapshot, - converter: Function> - ): Iterable> = - snapshot.documentChanges.map { dc: DocumentChange -> toEvent(dc, converter) } - - private fun toEvent( - dc: DocumentChange, - converter: Function> - ): RemoteDataEvent = - try { - Timber.v("${dc.document.reference.path} ${dc.type}") - val id = dc.document.id - when (dc.type) { - DocumentChange.Type.ADDED -> loaded(id, converter.apply(dc.document).getOrThrow()) - DocumentChange.Type.MODIFIED -> modified(id, converter.apply(dc.document).getOrThrow()) - DocumentChange.Type.REMOVED -> removed(id) - else -> throw DataStoreException("Unknown DocumentChange type: ${dc.type}") - } - } catch (t: Throwable) { - error(t) - } -} diff --git a/sharedTest/src/main/kotlin/com/sharedtest/persistence/remote/FakeRemoteDataStore.kt b/sharedTest/src/main/kotlin/com/sharedtest/persistence/remote/FakeRemoteDataStore.kt index cb49acc7a0..9a34e8d0eb 100644 --- a/sharedTest/src/main/kotlin/com/sharedtest/persistence/remote/FakeRemoteDataStore.kt +++ b/sharedTest/src/main/kotlin/com/sharedtest/persistence/remote/FakeRemoteDataStore.kt @@ -21,9 +21,7 @@ import com.google.android.ground.model.User import com.google.android.ground.model.locationofinterest.LocationOfInterest import com.google.android.ground.model.mutation.Mutation import com.google.android.ground.model.submission.Submission -import com.google.android.ground.persistence.remote.RemoteDataEvent import com.google.android.ground.persistence.remote.RemoteDataStore -import io.reactivex.Flowable import javax.inject.Inject import javax.inject.Singleton @@ -45,12 +43,6 @@ class FakeRemoteDataStore @Inject internal constructor() : RemoteDataStore { override suspend fun loadTermsOfService(): TermsOfService? = termsOfService?.getOrThrow() - // TODO(#1373): Delete once new LOI sync is implemented. - override fun loadLocationsOfInterestOnceAndStreamChanges( - survey: Survey - ): Flowable> = - Flowable.fromIterable(lois).map { RemoteDataEvent.loaded(it.id, it) } - override suspend fun loadLocationsOfInterest(survey: Survey) = lois override suspend fun loadSubmissions(locationOfInterest: LocationOfInterest): List { From 3e4cbf8fdfac7169b8da3bbf68e320ab4b560318 Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Sun, 20 Aug 2023 12:53:00 +0530 Subject: [PATCH 5/6] Remove gradle dependency for RxFirebase --- ground/build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/ground/build.gradle b/ground/build.gradle index 4f850625a4..1430860c93 100644 --- a/ground/build.gradle +++ b/ground/build.gradle @@ -186,7 +186,6 @@ dependencies { implementation 'com.google.firebase:firebase-messaging' implementation 'com.google.firebase:firebase-messaging-directboot' implementation 'com.google.firebase:firebase-messaging-ktx' - implementation 'com.github.FrangSierra:RxFirebase:1.5.7' // Hilt implementation "com.google.dagger:hilt-android:$project.hiltVersion" From fb269e7f8aeaca88713b7a519e3cb0cc2314825f Mon Sep 17 00:00:00 2001 From: Shobhit Agarwal Date: Sun, 20 Aug 2023 12:55:09 +0530 Subject: [PATCH 6/6] Remove unused schedulers object --- .../ground/persistence/remote/firebase/FirestoreDataStore.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/FirestoreDataStore.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/FirestoreDataStore.kt index b29527979f..456b04d0f1 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/FirestoreDataStore.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/FirestoreDataStore.kt @@ -27,7 +27,6 @@ import com.google.android.ground.model.submission.Submission import com.google.android.ground.persistence.remote.DataStoreException import com.google.android.ground.persistence.remote.RemoteDataStore import com.google.android.ground.persistence.remote.firebase.schema.GroundFirestore -import com.google.android.ground.rx.Schedulers import com.google.android.ground.system.ApplicationErrorManager import com.google.firebase.crashlytics.FirebaseCrashlytics import com.google.firebase.firestore.WriteBatch @@ -51,7 +50,6 @@ internal constructor( private val errorManager: ApplicationErrorManager, @IoDispatcher private val ioDispatcher: CoroutineDispatcher, val db: GroundFirestore, - val schedulers: Schedulers ) : RemoteDataStore { /**