From 49ca1e7c9b2df97a03043dd5af7fdd5b4e20d6c2 Mon Sep 17 00:00:00 2001 From: sufyanAbbasi Date: Wed, 24 Jul 2024 12:00:37 -0700 Subject: [PATCH] Read and load Surveys, Jobs, and Tasks from proto (#2553) * Survey, Job, Task proto conversion (Task TODO) * Use `addAll` method for lists * Use `addAll` method for lists * Fix tests * Fix tests p2 * Fix tests p3 * Add tests for SurveyConverter * Add tests for ConditionConverter * Add tests for JobConverter * Add tests for TaskConverter * Add taskID to Condition * Update Copyright year * Update Copyright year * Update Copyright year * Use Kotlin DSLs instead of Builders * Fix linter errors * Small edit to Int converter * Remove backwards compatibility --------- Co-authored-by: Gino Miceli <228050+gino-m@users.noreply.github.com> Co-authored-by: Shobhit Agarwal --- .../com/google/android/ground/model/Survey.kt | 3 - .../local/room/converter/ConverterExt.kt | 2 - .../local/room/stores/RoomSurveyStore.kt | 6 - .../protobuf/FirestoreToProtobufExt.kt | 31 ++- .../protobuf/MessageLiteReflectionExt.kt | 35 ++- .../firebase/schema/ConditionConverter.kt | 18 ++ .../firebase/schema/JobCollectionReference.kt | 38 +++ .../remote/firebase/schema/JobConverter.kt | 29 ++- .../schema/MultipleChoiceConverter.kt | 15 +- .../remote/firebase/schema/OptionConverter.kt | 4 + .../remote/firebase/schema/SurveyConverter.kt | 43 +--- .../remote/firebase/schema/SurveyDocument.kt | 1 - .../schema/SurveyDocumentReference.kt | 7 +- .../schema/SurveysCollectionReference.kt | 2 +- .../remote/firebase/schema/TaskConverter.kt | 57 +++++ .../google/android/ground/model/SurveyTest.kt | 1 - .../persistence/local/LocalDataStoreTests.kt | 15 -- .../firebase/schema/ConditionConverterTest.kt | 36 +++ .../firebase/schema/JobConverterTest.kt | 75 ++++++ .../schema/MultipleChoiceConverterTest.kt | 35 +++ .../firebase/schema/SurveyConverterTest.kt | 48 ++-- .../firebase/schema/TaskConverterTest.kt | 226 ++++++++++++++++++ .../ground/ui/home/HomeScreenFragmentTest.kt | 5 +- .../main/kotlin/com/sharedtest/FakeData.kt | 15 +- 24 files changed, 643 insertions(+), 104 deletions(-) create mode 100644 ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/JobCollectionReference.kt create mode 100644 ground/src/test/java/com/google/android/ground/persistence/remote/firebase/schema/JobConverterTest.kt create mode 100644 ground/src/test/java/com/google/android/ground/persistence/remote/firebase/schema/TaskConverterTest.kt diff --git a/ground/src/main/java/com/google/android/ground/model/Survey.kt b/ground/src/main/java/com/google/android/ground/model/Survey.kt index c2f77aed51..b56c145458 100644 --- a/ground/src/main/java/com/google/android/ground/model/Survey.kt +++ b/ground/src/main/java/com/google/android/ground/model/Survey.kt @@ -15,7 +15,6 @@ */ package com.google.android.ground.model -import com.google.android.ground.model.imagery.TileSource import com.google.android.ground.model.job.Job /** Configuration, schema, and ACLs for a single survey. */ @@ -24,8 +23,6 @@ data class Survey( val title: String, val description: String, val jobMap: Map, - // TODO(#1730): Remove tileSources from survey. - val tileSources: List = listOf(), val acl: Map = mapOf(), ) { val jobs: Collection diff --git a/ground/src/main/java/com/google/android/ground/persistence/local/room/converter/ConverterExt.kt b/ground/src/main/java/com/google/android/ground/persistence/local/room/converter/ConverterExt.kt index ee16809dd5..fb0fb5bd37 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/local/room/converter/ConverterExt.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/local/room/converter/ConverterExt.kt @@ -386,14 +386,12 @@ fun SubmissionMutation.toLocalDataStoreObject() = fun SurveyEntityAndRelations.toModelObject(): Survey { val jobMap = jobEntityAndRelations.map { it.toModelObject() }.associateBy { it.id } - val tileSources = tileSourceEntityAndRelations.map { it.toModelObject() } return Survey( surveyEntity.id, surveyEntity.title!!, surveyEntity.description!!, jobMap.toPersistentMap(), - tileSources.toPersistentList(), surveyEntity.acl?.toStringMap()!!, ) } diff --git a/ground/src/main/java/com/google/android/ground/persistence/local/room/stores/RoomSurveyStore.kt b/ground/src/main/java/com/google/android/ground/persistence/local/room/stores/RoomSurveyStore.kt index 3c3b31fe3e..b30581b382 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/local/room/stores/RoomSurveyStore.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/local/room/stores/RoomSurveyStore.kt @@ -71,7 +71,6 @@ class RoomSurveyStore @Inject internal constructor() : LocalSurveyStore { jobDao.deleteBySurveyId(survey.id) insertOrUpdateJobs(survey.id, survey.jobs) tileSourceDao.deleteBySurveyId(survey.id) - insertOfflineBaseMapSources(survey) } /** @@ -124,9 +123,4 @@ class RoomSurveyStore @Inject internal constructor() : LocalSurveyStore { private suspend fun insertOrUpdateJobs(surveyId: String, jobs: Collection) = jobs.forEach { insertOrUpdateJob(surveyId, it) } - - private suspend fun insertOfflineBaseMapSources(survey: Survey) = - survey.tileSources.forEach { - tileSourceDao.insertOrUpdate(it.toLocalDataStoreObject(surveyId = survey.id)) - } } diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/protobuf/FirestoreToProtobufExt.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/protobuf/FirestoreToProtobufExt.kt index 665e8f79c7..f4795d7a28 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/protobuf/FirestoreToProtobufExt.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/protobuf/FirestoreToProtobufExt.kt @@ -62,7 +62,11 @@ private fun FirestoreMap?.copyInto(builder: MessageBuilder) { private fun FirestoreMapEntry.copyInto(builder: MessageBuilder) { toMessageField(builder::class)?.also { (k, v) -> - if (v is MessageMap) builder.putAllOrLog(k, v) else builder.setOrLog(k, v) + when (v) { + is MessageMap -> builder.putAllOrLog(k, v) + is List<*> -> builder.addAllOrLog(k, v) + else -> builder.setOrLog(k, v) + } } } @@ -94,6 +98,15 @@ private fun FirestoreValue.toMessageValue( val fieldType = builderType.getFieldTypeByName(fieldName) return if (fieldType.isSubclassOf(Map::class)) { (this as FirestoreMap).toMessageMap(builderType.getMapValueType(fieldName)) + } else if (fieldType.isSubclassOf(List::class)) { + val elementType = builderType.getListElementFieldTypeByName(fieldName) + (this as List).map { + if (elementType.isSubclassOf(GeneratedMessageLite::class)) { + (elementType as KClass).parseFrom(it as FirestoreMap) + } else { + it.toMessageValue(elementType) + } + } } else { toMessageValue(fieldType) } @@ -103,11 +116,23 @@ private fun FirestoreValue.toMessageValue( private fun FirestoreValue.toMessageValue(targetType: KClass<*>): MessageValue = if (targetType == String::class) { this as String + } else if (targetType == Int::class) { + if (this is Long) { + this.toInt() + } else { + this + } + } else if (targetType == Boolean::class) { + this as Boolean } else if (targetType.isSubclassOf(GeneratedMessageLite::class)) { (targetType as KClass).parseFrom(this as FirestoreMap) } else if (targetType.isSubclassOf(EnumLite::class)) { - require(this is Int) { "Expected Int but got ${this::class}" } - (targetType as KClass).findByNumber(this) + var number = this + if (number is Long) { + number = number.toInt() + } + require(number is Int) { "Expected Int but got ${number::class}" } + (targetType as KClass).findByNumber(number) ?: throw IllegalArgumentException("Unrecognized enum number $this") } else { throw UnsupportedOperationException("Unsupported message field type $targetType") diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/protobuf/MessageLiteReflectionExt.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/protobuf/MessageLiteReflectionExt.kt index ebd449c8c6..396a4b6822 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/protobuf/MessageLiteReflectionExt.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/protobuf/MessageLiteReflectionExt.kt @@ -70,8 +70,20 @@ fun KClass.getMapValueType(key: String): KClass<*> { ?: throw NoSuchMethodError("$mapValueGetterName method") } +@Suppress("StringLiteralDuplication", "SwallowedException") fun KClass.getFieldTypeByName(fieldName: String): KClass<*> = - java.getDeclaredMethod("get${fieldName.toUpperCamelCase()}").returnType?.kotlin + try { + java.getDeclaredMethod("get${fieldName.toUpperCamelCase()}").returnType?.kotlin + ?: throw UnsupportedOperationException("Getter not found for field $fieldName") + } catch (e: NoSuchMethodException) { + // Could be a list type instead. Check for a `getFieldList()` method. + java.getDeclaredMethod("get${fieldName.toUpperCamelCase()}List").returnType?.kotlin + ?: throw UnsupportedOperationException("Getter not found for field $fieldName") + } + +fun KClass.getListElementFieldTypeByName(fieldName: String): KClass<*> = + // Each list field has a getter with an index. + java.getDeclaredMethod("get${fieldName.toUpperCamelCase()}", Int::class.java).returnType?.kotlin ?: throw UnsupportedOperationException("Getter not found for field $fieldName") private fun MessageBuilder.getSetterByFieldName(fieldName: String): KFunction<*> = @@ -81,6 +93,13 @@ private fun MessageBuilder.getSetterByFieldName(fieldName: String): KFunction<*> it.name == "set${fieldName.toUpperCamelCase()}" && !it.parameters[1].type.isBuilder() } ?: throw UnsupportedOperationException("Setter not found for field $fieldName") +private fun MessageBuilder.getAddAllByFieldName(fieldName: String): KFunction<*> = + // Message fields generated two setters; ignore the Builder's setter in favor of the + // message setter. + this::class.declaredFunctions.find { + it.name == "addAll${fieldName.toUpperCamelCase()}" && !it.parameters[1].type.isBuilder() + } ?: throw UnsupportedOperationException("addAll not found for field $fieldName") + private fun KType.isBuilder() = (classifier as KClass<*>).isSubclassOf(GeneratedMessageLite.Builder::class) @@ -91,6 +110,7 @@ private fun MessageBuilder.getPutAllByFieldName(fieldName: String): KFunction<*> fun KClass.newBuilderForType() = java.getDeclaredMethod("newBuilder").invoke(null) as MessageBuilder +@Suppress("StringLiteralDuplication") fun MessageBuilder.setOrLog(fieldName: MessageFieldName, value: MessageValue) { try { set(fieldName, value) @@ -99,6 +119,15 @@ fun MessageBuilder.setOrLog(fieldName: MessageFieldName, value: MessageValue) { } } +@Suppress("StringLiteralDuplication") +fun MessageBuilder.addAllOrLog(fieldName: MessageFieldName, value: MessageValue) { + try { + addAll(fieldName, value) + } catch (e: Throwable) { + Timber.e(e, "Skipping incompatible value in ${javaClass}: $fieldName=$value") + } +} + fun KClass.getFieldName(fieldNumber: MessageFieldNumber): MessageFieldName = getStaticFields() .find { it.name.endsWith(FIELD_NUMBER_CONST_SUFFIX) && it.get(null) == fieldNumber } @@ -154,6 +183,10 @@ private fun MessageBuilder.set(fieldName: MessageFieldName, value: MessageValue) getSetterByFieldName(fieldName).call(this, value) } +private fun MessageBuilder.addAll(fieldName: MessageFieldName, value: MessageValue) { + getAddAllByFieldName(fieldName).call(this, value) +} + private fun MessageBuilder.putAll(fieldName: MessageFieldName, value: MessageMap) { getPutAllByFieldName(fieldName).call(this, value) } diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/ConditionConverter.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/ConditionConverter.kt index 2cbac46897..4a44f9a4ee 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/ConditionConverter.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/ConditionConverter.kt @@ -20,10 +20,28 @@ import com.google.android.ground.model.task.Condition import com.google.android.ground.model.task.Condition.MatchType import com.google.android.ground.model.task.Expression import com.google.android.ground.model.task.Expression.ExpressionType +import com.google.android.ground.proto.Task import timber.log.Timber /** Converts between Firestore nested objects and [Condition] instances. */ internal object ConditionConverter { + + fun Task.Condition.toCondition(): Condition? { + if (conditionTypeCase != Task.Condition.ConditionTypeCase.MULTIPLE_CHOICE) { + Timber.e("Unsupported conditionType: $conditionTypeCase") + return null + } + val expressions = + listOf( + Expression( + ExpressionType.ANY_OF_SELECTED, + taskId = multipleChoice.taskId, + multipleChoice.optionIdsList.toSet(), + ) + ) + return Condition(MatchType.MATCH_ANY, expressions) + } + fun ConditionNestedObject.toCondition(): Condition? { val matchType = matchType.toMatchType() if (matchType == MatchType.UNKNOWN) { diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/JobCollectionReference.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/JobCollectionReference.kt new file mode 100644 index 0000000000..f7f111066b --- /dev/null +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/JobCollectionReference.kt @@ -0,0 +1,38 @@ +/* + * Copyright 2024 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.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 + +class JobCollectionReference internal constructor(ref: CollectionReference) : + FluentCollectionReference(ref) { + fun get(): Flow> = callbackFlow { + reference() + .get() + .addOnSuccessListener { trySend(it.documents.map { doc -> JobConverter.toJob(doc) }) } + .addOnFailureListener { trySend(listOf()) } + + awaitClose { + // Cannot cancel or detach listeners here. + } + } +} diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/JobConverter.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/JobConverter.kt index b8a0e98fac..ac8d718ad4 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/JobConverter.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/JobConverter.kt @@ -17,7 +17,12 @@ package com.google.android.ground.persistence.remote.firebase.schema import com.google.android.ground.model.job.Job -import com.google.android.ground.model.task.Task +import com.google.android.ground.model.job.Style as StyleModel +import com.google.android.ground.model.task.Task as TaskModel +import com.google.android.ground.persistence.remote.DataStoreException +import com.google.android.ground.persistence.remote.firebase.protobuf.parseFrom +import com.google.android.ground.proto.Job as JobProto +import com.google.firebase.firestore.DocumentSnapshot import kotlinx.collections.immutable.toPersistentMap /** Converts between Firestore documents and [Job] instances. */ @@ -25,7 +30,7 @@ internal object JobConverter { @JvmStatic fun toJob(id: String, obj: JobNestedObject): Job { - val taskMap = mutableMapOf() + val taskMap = mutableMapOf() obj.tasks?.let { it.entries .mapNotNull { (key, value) -> TaskConverter.toTask(key, value) } @@ -39,4 +44,24 @@ internal object JobConverter { tasks = taskMap.toPersistentMap(), ) } + + @Throws(DataStoreException::class) + fun toJob(doc: DocumentSnapshot): Job { + if (!doc.exists()) throw DataStoreException("Missing job") + val jobProto = JobProto::class.parseFrom(doc) + val taskMap = jobProto.tasksList.associate { it.id to TaskConverter.toTask(it) } + val strategy = + if (taskMap.values.map { it.isAddLoiTask }.none()) { + Job.DataCollectionStrategy.PREDEFINED + } else { + Job.DataCollectionStrategy.MIXED + } + return Job( + id = jobProto.id.ifEmpty { doc.id }, + style = StyleModel(jobProto.style.color), + name = jobProto.name, + strategy = strategy, + tasks = taskMap.toPersistentMap(), + ) + } } diff --git a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/MultipleChoiceConverter.kt b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/MultipleChoiceConverter.kt index ffa58304dd..9b86f1ed35 100644 --- a/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/MultipleChoiceConverter.kt +++ b/ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/MultipleChoiceConverter.kt @@ -18,12 +18,25 @@ package com.google.android.ground.persistence.remote.firebase.schema import com.google.android.ground.model.task.MultipleChoice import com.google.android.ground.model.task.Option +import com.google.android.ground.proto.Task.MultipleChoiceQuestion import com.google.android.ground.util.Enums.toEnum import kotlinx.collections.immutable.toPersistentList internal object MultipleChoiceConverter { + fun toMultipleChoice(em: MultipleChoiceQuestion): MultipleChoice { + var options: List