diff --git a/collect_app/src/main/java/org/odk/collect/android/formentry/FormEntryUseCases.kt b/collect_app/src/main/java/org/odk/collect/android/formentry/FormEntryUseCases.kt index 56c604c74b9..85be7e5007f 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formentry/FormEntryUseCases.kt +++ b/collect_app/src/main/java/org/odk/collect/android/formentry/FormEntryUseCases.kt @@ -22,7 +22,7 @@ import org.odk.collect.forms.Form import org.odk.collect.forms.FormsRepository import org.odk.collect.forms.instances.Instance import org.odk.collect.forms.instances.InstancesRepository -import org.odk.collect.shared.DebugLogger +import org.odk.collect.shared.debug.DebugLogger import java.io.File object FormEntryUseCases { diff --git a/collect_app/src/main/java/org/odk/collect/android/formentry/saving/DiskFormSaver.java b/collect_app/src/main/java/org/odk/collect/android/formentry/saving/DiskFormSaver.java index 1d49a427502..84a219affb3 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formentry/saving/DiskFormSaver.java +++ b/collect_app/src/main/java/org/odk/collect/android/formentry/saving/DiskFormSaver.java @@ -8,7 +8,7 @@ import org.odk.collect.android.utilities.MediaUtils; import org.odk.collect.entities.storage.EntitiesRepository; import org.odk.collect.forms.instances.InstancesRepository; -import org.odk.collect.shared.DebugLogger; +import org.odk.collect.shared.debug.DebugLogger; import java.util.ArrayList; diff --git a/collect_app/src/main/java/org/odk/collect/android/formentry/saving/FormSaveViewModel.java b/collect_app/src/main/java/org/odk/collect/android/formentry/saving/FormSaveViewModel.java index 5dead6cad85..af7728e40a0 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formentry/saving/FormSaveViewModel.java +++ b/collect_app/src/main/java/org/odk/collect/android/formentry/saving/FormSaveViewModel.java @@ -40,7 +40,7 @@ import org.odk.collect.forms.instances.InstancesRepository; import org.odk.collect.forms.savepoints.SavepointsRepository; import org.odk.collect.material.MaterialProgressDialogFragment; -import org.odk.collect.shared.DebugLogger; +import org.odk.collect.shared.debug.DebugLogger; import org.odk.collect.shared.strings.Md5; import org.odk.collect.utilities.Result; diff --git a/collect_app/src/main/java/org/odk/collect/android/formentry/saving/FormSaver.java b/collect_app/src/main/java/org/odk/collect/android/formentry/saving/FormSaver.java index d36bf77c375..5b1cedc9a11 100644 --- a/collect_app/src/main/java/org/odk/collect/android/formentry/saving/FormSaver.java +++ b/collect_app/src/main/java/org/odk/collect/android/formentry/saving/FormSaver.java @@ -7,7 +7,7 @@ import org.odk.collect.android.utilities.MediaUtils; import org.odk.collect.entities.storage.EntitiesRepository; import org.odk.collect.forms.instances.InstancesRepository; -import org.odk.collect.shared.DebugLogger; +import org.odk.collect.shared.debug.DebugLogger; import java.util.ArrayList; diff --git a/collect_app/src/main/java/org/odk/collect/android/injection/config/ProjectDependencyModuleFactory.kt b/collect_app/src/main/java/org/odk/collect/android/injection/config/ProjectDependencyModuleFactory.kt index 6f33e85b0cd..08ee66b2980 100644 --- a/collect_app/src/main/java/org/odk/collect/android/injection/config/ProjectDependencyModuleFactory.kt +++ b/collect_app/src/main/java/org/odk/collect/android/injection/config/ProjectDependencyModuleFactory.kt @@ -2,7 +2,6 @@ package org.odk.collect.android.injection.config import org.odk.collect.android.entities.EntitiesRepositoryProvider import org.odk.collect.android.formmanagement.OpenRosaClientProvider -import org.odk.collect.android.projects.FileDebugLogger import org.odk.collect.android.projects.ProjectDependencyModule import org.odk.collect.android.storage.StoragePathProvider import org.odk.collect.android.storage.StoragePaths @@ -10,9 +9,10 @@ import org.odk.collect.android.utilities.ChangeLockProvider import org.odk.collect.android.utilities.FormsRepositoryProvider import org.odk.collect.android.utilities.InstancesRepositoryProvider import org.odk.collect.android.utilities.SavepointsRepositoryProvider +import org.odk.collect.entities.debug.EntitiesDebugLogger import org.odk.collect.projects.ProjectDependencyFactory import org.odk.collect.settings.SettingsProvider -import org.odk.collect.shared.DebugLogger +import org.odk.collect.shared.debug.DebugLogger import java.io.File import javax.inject.Inject @@ -46,6 +46,6 @@ class ProjectDependencyModuleFactory @Inject constructor( private class DebugLoggerFactory(private val storagePathProvider: ProjectDependencyFactory) : ProjectDependencyFactory { override fun create(projectId: String): DebugLogger { - return FileDebugLogger(File(storagePathProvider.create(projectId).rootDir, "debug.log")) + return EntitiesDebugLogger(File(storagePathProvider.create(projectId).rootDir, "debug.log")) } } diff --git a/collect_app/src/main/java/org/odk/collect/android/projects/FileDebugLogger.kt b/collect_app/src/main/java/org/odk/collect/android/projects/FileDebugLogger.kt deleted file mode 100644 index a74a802edf8..00000000000 --- a/collect_app/src/main/java/org/odk/collect/android/projects/FileDebugLogger.kt +++ /dev/null @@ -1,20 +0,0 @@ -package org.odk.collect.android.projects - -import org.odk.collect.android.BuildConfig -import org.odk.collect.shared.DebugLogger -import java.io.File -import java.time.LocalDateTime - -class FileDebugLogger(private val file: File) : DebugLogger { - - override fun log(tag: String, message: String) { - if (enabled) { - val line = "${LocalDateTime.now()} $tag \"$message\"\n" - file.appendText(line) - } - } - - companion object { - private val enabled = BuildConfig.DEBUG - } -} diff --git a/collect_app/src/main/java/org/odk/collect/android/projects/ProjectDependencyModule.kt b/collect_app/src/main/java/org/odk/collect/android/projects/ProjectDependencyModule.kt index 802326df846..56fc59c3234 100644 --- a/collect_app/src/main/java/org/odk/collect/android/projects/ProjectDependencyModule.kt +++ b/collect_app/src/main/java/org/odk/collect/android/projects/ProjectDependencyModule.kt @@ -10,7 +10,7 @@ import org.odk.collect.forms.instances.InstancesRepository import org.odk.collect.forms.savepoints.SavepointsRepository import org.odk.collect.projects.ProjectDependencyFactory import org.odk.collect.projects.projectDependency -import org.odk.collect.shared.DebugLogger +import org.odk.collect.shared.debug.DebugLogger import org.odk.collect.shared.settings.Settings /** diff --git a/collect_app/src/main/java/org/odk/collect/android/tasks/SaveFormToDisk.java b/collect_app/src/main/java/org/odk/collect/android/tasks/SaveFormToDisk.java index 37b0acbaecc..fa97a84d516 100644 --- a/collect_app/src/main/java/org/odk/collect/android/tasks/SaveFormToDisk.java +++ b/collect_app/src/main/java/org/odk/collect/android/tasks/SaveFormToDisk.java @@ -62,7 +62,7 @@ import org.odk.collect.forms.instances.Instance; import org.odk.collect.forms.instances.InstancesRepository; import org.odk.collect.maps.MapPoint; -import org.odk.collect.shared.DebugLogger; +import org.odk.collect.shared.debug.DebugLogger; import org.odk.collect.shared.files.FileExt; import java.io.File; diff --git a/collect_app/src/test/java/org/odk/collect/android/formentry/audit/FormSaveViewModelTest.java b/collect_app/src/test/java/org/odk/collect/android/formentry/audit/FormSaveViewModelTest.java index b6912aad56d..5a1d1f221ab 100644 --- a/collect_app/src/test/java/org/odk/collect/android/formentry/audit/FormSaveViewModelTest.java +++ b/collect_app/src/test/java/org/odk/collect/android/formentry/audit/FormSaveViewModelTest.java @@ -49,7 +49,7 @@ import org.odk.collect.formstest.InMemInstancesRepository; import org.odk.collect.formstest.InMemSavepointsRepository; import org.odk.collect.projects.Project; -import org.odk.collect.shared.DebugLogger; +import org.odk.collect.shared.debug.DebugLogger; import org.odk.collect.shared.TempFiles; import org.odk.collect.testshared.FakeScheduler; import org.odk.collect.utilities.Result; diff --git a/entities/build.gradle.kts b/entities/build.gradle.kts index 9fe4f5be585..56d7fdc7117 100644 --- a/entities/build.gradle.kts +++ b/entities/build.gradle.kts @@ -48,6 +48,7 @@ dependencies { implementation(project(":strings")) implementation(project(":shared")) implementation(project(":androidshared")) + implementation(project(":analytics")) implementation(project(":material")) implementation(project(":async")) implementation(project(":lists")) diff --git a/entities/src/main/java/org/odk/collect/entities/LocalEntityUseCases.kt b/entities/src/main/java/org/odk/collect/entities/LocalEntityUseCases.kt index 58eccf08882..89ce78b0e89 100644 --- a/entities/src/main/java/org/odk/collect/entities/LocalEntityUseCases.kt +++ b/entities/src/main/java/org/odk/collect/entities/LocalEntityUseCases.kt @@ -2,16 +2,18 @@ package org.odk.collect.entities import org.apache.commons.csv.CSVRecord import org.javarosa.core.model.instance.SecondaryInstanceCSVParserBuilder +import org.odk.collect.entities.debug.EntityEvent import org.odk.collect.entities.javarosa.finalization.EntitiesExtra import org.odk.collect.entities.javarosa.finalization.FormEntity import org.odk.collect.entities.javarosa.parse.EntitySchema +import org.odk.collect.entities.javarosa.parse.isV4UUID import org.odk.collect.entities.javarosa.spec.EntityAction import org.odk.collect.entities.server.EntitySource import org.odk.collect.entities.storage.EntitiesRepository import org.odk.collect.entities.storage.Entity import org.odk.collect.entities.storage.findEntityById import org.odk.collect.forms.MediaFile -import org.odk.collect.shared.DebugLogger +import org.odk.collect.shared.debug.DebugLogger import java.io.File import java.util.UUID @@ -24,32 +26,37 @@ object LocalEntityUseCases { debugLogger: DebugLogger? = null ) { formEntities?.entities?.forEach { formEntity -> - when (formEntity.action) { - EntityAction.CREATE -> saveNewEntity(formEntity, entitiesRepository, debugLogger) - - EntityAction.UPDATE -> { - val existing = entitiesRepository.findEntityById(formEntity.dataset, formEntity.id) - if (existing != null) { - saveUpdatedEntity(formEntity, existing, entitiesRepository) + if (formEntity.id.isV4UUID()) { + when (formEntity.action) { + EntityAction.CREATE -> saveNewEntity(formEntity, entitiesRepository, debugLogger) + + EntityAction.UPDATE -> { + val existing = entitiesRepository.findEntityById(formEntity.dataset, formEntity.id) + if (existing != null) { + saveUpdatedEntity(formEntity, existing, entitiesRepository) + } else { + debugLogger?.log(EntityEvent.UpdateNoMatch(formEntity)) + } } - } - EntityAction.UPSERT -> { - val existing = entitiesRepository.findEntityById(formEntity.dataset, formEntity.id) - if (existing == null) { - saveNewEntity(formEntity, entitiesRepository, debugLogger) - } else { - saveUpdatedEntity(formEntity, existing, entitiesRepository) + EntityAction.UPSERT -> { + val existing = entitiesRepository.findEntityById(formEntity.dataset, formEntity.id) + if (existing == null) { + saveNewEntity(formEntity, entitiesRepository, debugLogger) + } else { + saveUpdatedEntity(formEntity, existing, entitiesRepository) + } } } - } - } + } else { + val event = if (formEntity.id.isNullOrBlank()) { + EntityEvent.NoId(formEntity) + } else { + EntityEvent.InvalidId(formEntity) + } - formEntities?.invalidEntities?.forEach { - debugLogger?.log( - "Entities", - "Failed to create/update dataset=${it.dataset}, id=${it.id}, label=${it.label}" - ) + debugLogger?.log(event) + } } } @@ -64,7 +71,7 @@ object LocalEntityUseCases { entitiesRepository.save( formEntity.dataset, Entity.New( - formEntity.id, + formEntity.id!!, formEntity.label, 1, formEntity.properties, @@ -73,10 +80,7 @@ object LocalEntityUseCases { ) } } else { - debugLogger?.log( - "Entities", - "Failed to create dataset=${formEntity.dataset}, id=${formEntity.id}, label=${formEntity.label}" - ) + debugLogger?.log(EntityEvent.CreateNoLabel(formEntity)) } } diff --git a/entities/src/main/java/org/odk/collect/entities/analytics/AnalyticsEvents.kt b/entities/src/main/java/org/odk/collect/entities/analytics/AnalyticsEvents.kt new file mode 100644 index 00000000000..814d9d32dc6 --- /dev/null +++ b/entities/src/main/java/org/odk/collect/entities/analytics/AnalyticsEvents.kt @@ -0,0 +1,23 @@ +package org.odk.collect.entities.analytics + +object AnalyticsEvents { + /** + * Tracks how often an entity update is attempted but no entity with a matching ID is found. + */ + const val ENTITY_UPDATE_NO_MATCH = "EntityUpdateNoMatch" + + /** + * Tracks how often an entity creation is attempted but the label is blank. + */ + const val ENTITY_CREATE_NO_LABEL = "EntityCreateNoLabel" + + /** + * Tracks how often an entity is defined in a form but has no ID. + */ + const val ENTITY_WITH_NO_ID = "EntityWithNoId" + + /** + * Tracks how often an entity is defined in a form but has an invalid ID (not a V4 UUID). + */ + const val ENTITY_WITH_INVALID_ID = "EntityWithInvalidId" +} diff --git a/entities/src/main/java/org/odk/collect/entities/debug/EntitiesDebugLogger.kt b/entities/src/main/java/org/odk/collect/entities/debug/EntitiesDebugLogger.kt new file mode 100644 index 00000000000..034d4a7389c --- /dev/null +++ b/entities/src/main/java/org/odk/collect/entities/debug/EntitiesDebugLogger.kt @@ -0,0 +1,24 @@ +package org.odk.collect.entities.debug + +import org.odk.collect.analytics.Analytics +import org.odk.collect.entities.BuildConfig +import org.odk.collect.shared.debug.DebugEvent +import org.odk.collect.shared.debug.DebugLogger +import java.io.File +import java.time.LocalDateTime + +/** + * A [DebugLogger] that writes each event both as a line in a debug log file (in debug builds) and + * as an analytics event. + */ +class EntitiesDebugLogger(private val file: File) : DebugLogger { + + override fun log(event: DebugEvent) { + if (BuildConfig.DEBUG) { + val line = "${LocalDateTime.now()} Entities \"${event.message}\"\n" + file.appendText(line) + } + + Analytics.log(event.analyticsEvent, "form") + } +} diff --git a/entities/src/main/java/org/odk/collect/entities/debug/EntityEvent.kt b/entities/src/main/java/org/odk/collect/entities/debug/EntityEvent.kt new file mode 100644 index 00000000000..19afdfbb46e --- /dev/null +++ b/entities/src/main/java/org/odk/collect/entities/debug/EntityEvent.kt @@ -0,0 +1,25 @@ +package org.odk.collect.entities.debug + +import org.odk.collect.entities.analytics.AnalyticsEvents +import org.odk.collect.entities.javarosa.finalization.FormEntity +import org.odk.collect.shared.debug.DebugEvent + +sealed class EntityEvent(override val analyticsEvent: String, private val action: String) : DebugEvent { + + abstract val formEntity: FormEntity + + override val message: String + get() = "Failed to $action dataset=${formEntity.dataset}, id=${formEntity.id}, label=${formEntity.label}" + + data class CreateNoLabel(override val formEntity: FormEntity) : + EntityEvent(AnalyticsEvents.ENTITY_CREATE_NO_LABEL, "create") + + data class UpdateNoMatch(override val formEntity: FormEntity) : + EntityEvent(AnalyticsEvents.ENTITY_UPDATE_NO_MATCH, "update") + + data class NoId(override val formEntity: FormEntity) : + EntityEvent(AnalyticsEvents.ENTITY_WITH_NO_ID, "create/update") + + data class InvalidId(override val formEntity: FormEntity) : + EntityEvent(AnalyticsEvents.ENTITY_WITH_INVALID_ID, "create/update") +} diff --git a/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/EntitiesExtra.kt b/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/EntitiesExtra.kt index c0fc4ea6d03..eff96b4db15 100644 --- a/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/EntitiesExtra.kt +++ b/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/EntitiesExtra.kt @@ -2,5 +2,4 @@ package org.odk.collect.entities.javarosa.finalization data class EntitiesExtra( val entities: List = emptyList(), - val invalidEntities: List = emptyList() ) diff --git a/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/EntityFormFinalizationProcessor.kt b/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/EntityFormFinalizationProcessor.kt index 8e378edb460..5bf9c5200eb 100644 --- a/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/EntityFormFinalizationProcessor.kt +++ b/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/EntityFormFinalizationProcessor.kt @@ -6,7 +6,6 @@ import org.javarosa.form.api.FormEntryFinalizationProcessor import org.javarosa.form.api.FormEntryModel import org.odk.collect.entities.javarosa.parse.EntityFormExtra import org.odk.collect.entities.javarosa.parse.SaveTo -import org.odk.collect.entities.javarosa.parse.isV4UUID import org.odk.collect.entities.javarosa.spec.EntityAction import org.odk.collect.entities.javarosa.spec.EntityFormParser @@ -37,12 +36,7 @@ class EntityFormFinalizationProcessor : FormEntryFinalizationProcessor { mainInstance ) - if (entity != null) { - extra.copy(entities = extra.entities + entity) - } else { - val invalidEntity = InvalidEntity(dataset, id, label) - extra.copy(invalidEntities = extra.invalidEntities + invalidEntity) - } + extra.copy(entities = extra.entities + entity) } else { extra } @@ -60,7 +54,7 @@ class EntityFormFinalizationProcessor : FormEntryFinalizationProcessor { saveTos: List, action: EntityAction, mainInstance: FormInstance - ): FormEntity? { + ): FormEntity { val entityGroupRef = elementRef.getParentRef().getParentRef() val fields = saveTos.mapNotNull { saveTo -> if (!entityGroupRef.genericize().equals(saveTo.entityGroupReference)) { @@ -79,10 +73,6 @@ class EntityFormFinalizationProcessor : FormEntryFinalizationProcessor { } } - return if (id.isV4UUID()) { - FormEntity(action, dataset, id, label, fields) - } else { - null - } + return FormEntity(action, dataset, id, label, fields) } } diff --git a/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/FormEntity.kt b/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/FormEntity.kt index fc9aef9ba40..5538ed3f4af 100644 --- a/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/FormEntity.kt +++ b/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/FormEntity.kt @@ -5,7 +5,7 @@ import org.odk.collect.entities.javarosa.spec.EntityAction data class FormEntity( @JvmField val action: EntityAction, @JvmField val dataset: String, - @JvmField val id: String, + @JvmField val id: String?, @JvmField val label: String, @JvmField val properties: List> ) diff --git a/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/InvalidEntity.kt b/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/InvalidEntity.kt deleted file mode 100644 index 2ab7356a371..00000000000 --- a/entities/src/main/java/org/odk/collect/entities/javarosa/finalization/InvalidEntity.kt +++ /dev/null @@ -1,7 +0,0 @@ -package org.odk.collect.entities.javarosa.finalization - -data class InvalidEntity( - val dataset: String, - val id: String?, - val label: String? -) diff --git a/entities/src/test/java/org/odk/collect/entities/LocalEntityUseCasesTest.kt b/entities/src/test/java/org/odk/collect/entities/LocalEntityUseCasesTest.kt index b35af6bd5d8..5785520905e 100644 --- a/entities/src/test/java/org/odk/collect/entities/LocalEntityUseCasesTest.kt +++ b/entities/src/test/java/org/odk/collect/entities/LocalEntityUseCasesTest.kt @@ -10,9 +10,9 @@ import org.hamcrest.text.IsBlankString.blankOrNullString import org.junit.Test import org.mockito.kotlin.mock import org.mockito.kotlin.verify +import org.odk.collect.entities.debug.EntityEvent import org.odk.collect.entities.javarosa.finalization.EntitiesExtra import org.odk.collect.entities.javarosa.finalization.FormEntity -import org.odk.collect.entities.javarosa.finalization.InvalidEntity import org.odk.collect.entities.javarosa.parse.EntitySchema import org.odk.collect.entities.javarosa.spec.EntityAction import org.odk.collect.entities.server.EntitySource @@ -21,7 +21,7 @@ import org.odk.collect.entities.storage.Entity import org.odk.collect.entities.storage.EntityList import org.odk.collect.entities.storage.InMemEntitiesRepository import org.odk.collect.formstest.FormFixtures -import org.odk.collect.shared.DebugLogger +import org.odk.collect.shared.debug.DebugLogger import org.odk.collect.shared.Query import org.odk.collect.shared.TempFiles import java.io.File @@ -37,7 +37,7 @@ class LocalEntityUseCasesTest { entitiesRepository.addList("things") val formEntity = - FormEntity(EntityAction.CREATE, "things", "id", "label", listOf("property" to "value")) + FormEntity(EntityAction.CREATE, "things", UUID.randomUUID().toString(), "label", listOf("property" to "value")) val formEntities = EntitiesExtra(listOf(formEntity)) LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) @@ -49,12 +49,25 @@ class LocalEntityUseCasesTest { assertThat(entities[0].branchId, not(blankOrNullString())) } + @Test + fun `#updateLocalEntitiesFromForm does not save a new entity on create if id is not a valid UUID`() { + entitiesRepository.addList("things") + + val formEntity = + FormEntity(EntityAction.CREATE, "things", "id", "label", listOf("property" to "value")) + val formEntities = EntitiesExtra(listOf(formEntity)) + LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) + + val entities = entitiesRepository.query("things") + assertThat(entities.size, equalTo(0)) + } + @Test fun `#updateLocalEntitiesFromForm does not save a new entity on create if label is blank`() { entitiesRepository.addList("things") val formEntity = - FormEntity(EntityAction.CREATE, "things", "id", "", listOf("property" to "value")) + FormEntity(EntityAction.CREATE, "things", UUID.randomUUID().toString(), "", listOf("property" to "value")) val formEntities = EntitiesExtra(listOf(formEntity)) LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) @@ -65,7 +78,7 @@ class LocalEntityUseCasesTest { @Test fun `#updateLocalEntitiesFromForm does not save a new entity on create if the list doesn't already exist`() { val formEntity = - FormEntity(EntityAction.CREATE, "things", "id", "label", listOf("property" to "value")) + FormEntity(EntityAction.CREATE, "things", UUID.randomUUID().toString(), "label", listOf("property" to "value")) val formEntities = EntitiesExtra(listOf(formEntity)) LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) @@ -74,7 +87,7 @@ class LocalEntityUseCasesTest { } @Test - fun `#updateLocalEntitiesFromForm increments version on update`() { + fun `#updateLocalEntitiesFromForm does not update an entity if id in not a valid UUID`() { entitiesRepository.save( "things", Entity.New( @@ -85,7 +98,30 @@ class LocalEntityUseCasesTest { ) val formEntity = - FormEntity(EntityAction.UPDATE, "things", "id", "label", emptyList()) + FormEntity(EntityAction.UPDATE, "things", "id", "new_label", emptyList()) + val formEntities = EntitiesExtra(listOf(formEntity)) + + LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) + val entities = entitiesRepository.query("things") + assertThat(entities.size, equalTo(1)) + assertThat(entities[0].label, equalTo("label")) + assertThat(entities[0].version, equalTo(1)) + } + + @Test + fun `#updateLocalEntitiesFromForm increments version on update`() { + val id = UUID.randomUUID().toString() + entitiesRepository.save( + "things", + Entity.New( + id, + "label", + version = 1 + ) + ) + + val formEntity = + FormEntity(EntityAction.UPDATE, "things", id, "label", emptyList()) val formEntities = EntitiesExtra(listOf(formEntity)) LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) @@ -96,10 +132,11 @@ class LocalEntityUseCasesTest { @Test fun `#updateLocalEntitiesFromForm updates properties on update`() { + val id = UUID.randomUUID().toString() entitiesRepository.save( "things", Entity.New( - "id", + id, "label", version = 1, properties = listOf("prop" to "value") @@ -107,7 +144,7 @@ class LocalEntityUseCasesTest { ) val formEntity = - FormEntity(EntityAction.UPDATE, "things", "id", "label", listOf("prop" to "value 2")) + FormEntity(EntityAction.UPDATE, "things", id, "label", listOf("prop" to "value 2")) val formEntities = EntitiesExtra(listOf(formEntity)) LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) @@ -119,10 +156,11 @@ class LocalEntityUseCasesTest { @Test fun `#updateLocalEntitiesFromForm updates properties and does not change label on update if label is blank`() { + val id = UUID.randomUUID().toString() entitiesRepository.save( "things", Entity.New( - "id", + id, "label", version = 1, properties = listOf("prop" to "value") @@ -130,7 +168,7 @@ class LocalEntityUseCasesTest { ) val formEntity = - FormEntity(EntityAction.UPDATE, "things", "id", " ", listOf("prop" to "value 2")) + FormEntity(EntityAction.UPDATE, "things", id, " ", listOf("prop" to "value 2")) val formEntities = EntitiesExtra(listOf(formEntity)) LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) @@ -146,7 +184,7 @@ class LocalEntityUseCasesTest { entitiesRepository.addList("things") val formEntity = - FormEntity(EntityAction.UPSERT, "things", "id", "label", listOf("property" to "value")) + FormEntity(EntityAction.UPSERT, "things", UUID.randomUUID().toString(), "label", listOf("property" to "value")) val formEntities = EntitiesExtra(listOf(formEntity)) LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) @@ -163,7 +201,7 @@ class LocalEntityUseCasesTest { entitiesRepository.addList("things") val formEntity = - FormEntity(EntityAction.UPSERT, "things", "id", "", listOf("property" to "value")) + FormEntity(EntityAction.UPSERT, "things", UUID.randomUUID().toString(), "", listOf("property" to "value")) val formEntities = EntitiesExtra(listOf(formEntity)) LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) @@ -173,17 +211,18 @@ class LocalEntityUseCasesTest { @Test fun `#updateLocalEntitiesFromForm updates an existing entity on upsert if it exists`() { + val id = UUID.randomUUID().toString() entitiesRepository.save( "things", Entity.New( - "id", + id, "label", version = 1 ) ) val formEntity = - FormEntity(EntityAction.UPSERT, "things", "id", "new label", emptyList()) + FormEntity(EntityAction.UPSERT, "things", id, "new label", emptyList()) val formEntities = EntitiesExtra(listOf(formEntity)) LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) @@ -207,7 +246,7 @@ class LocalEntityUseCasesTest { ) val formEntity = - FormEntity(EntityAction.UPDATE, "things", "id", "label", emptyList()) + FormEntity(EntityAction.UPDATE, "things", UUID.randomUUID().toString(), "label", emptyList()) val formEntities = EntitiesExtra(listOf(formEntity)) LocalEntityUseCases.updateLocalEntitiesFromForm(formEntities, entitiesRepository) @@ -220,7 +259,7 @@ class LocalEntityUseCasesTest { @Test fun `#updateLocalEntitiesFromForm does not save updated entity that doesn't already exist`() { val formEntity = - FormEntity(EntityAction.UPDATE, "things", "1", "1", emptyList()) + FormEntity(EntityAction.UPDATE, "things", UUID.randomUUID().toString(), "1", emptyList()) val formEntities = EntitiesExtra(listOf(formEntity)) entitiesRepository.addList("things") @@ -231,8 +270,17 @@ class LocalEntityUseCasesTest { @Test fun `#updateLocalEntitiesFromForm logs invalid entities`() { val debugLogger = mock() + val id = UUID.randomUUID().toString() + val formEntity1 = + FormEntity(EntityAction.CREATE, "things", "", "label", emptyList()) + val formEntity2 = + FormEntity(EntityAction.CREATE, "things", "id", "label", emptyList()) + val formEntity3 = + FormEntity(EntityAction.CREATE, "things", id, "", emptyList()) + val formEntity4 = + FormEntity(EntityAction.UPDATE, "things", id, "", emptyList()) val formEntities = - EntitiesExtra(emptyList(), listOf(InvalidEntity("things", "id", "label"))) + EntitiesExtra(listOf(formEntity1, formEntity2, formEntity3, formEntity4)) LocalEntityUseCases.updateLocalEntitiesFromForm( formEntities, @@ -240,10 +288,10 @@ class LocalEntityUseCasesTest { debugLogger ) - verify(debugLogger).log( - "Entities", - "Failed to create/update dataset=things, id=id, label=label" - ) + verify(debugLogger).log(EntityEvent.NoId(formEntity1)) + verify(debugLogger).log(EntityEvent.InvalidId(formEntity2)) + verify(debugLogger).log(EntityEvent.CreateNoLabel(formEntity3)) + verify(debugLogger).log(EntityEvent.UpdateNoMatch(formEntity4)) } @Test diff --git a/entities/src/test/java/org/odk/collect/entities/javarosa/EntitiesTest.kt b/entities/src/test/java/org/odk/collect/entities/javarosa/EntitiesTest.kt index fe6ba668821..d2287358353 100644 --- a/entities/src/test/java/org/odk/collect/entities/javarosa/EntitiesTest.kt +++ b/entities/src/test/java/org/odk/collect/entities/javarosa/EntitiesTest.kt @@ -678,128 +678,6 @@ class EntitiesTest { ) } - @Test - fun `filling form with create without an id makes invalid entity available`() { - val scenario = Scenario.init( - html( - listOf(Pair("entities", "http://www.opendatakit.org/xforms/entities")), - head( - title("Create entity form"), - model( - listOf(Pair("entities:entities-version", "2024.1.0")), - mainInstance( - t( - "data id=\"create-entity-form\"", - t("id"), - t("name"), - t( - "meta", - t("entity dataset=\"people\" create=\"1\" id=\"\"", - t("label") - ) - ) - ) - ), - bind("/data/id").type("string"), - bind("/data/meta/entity/@id").type("string").calculate("/data/id"), - bind("/data/meta/entity/label").type("string").calculate("/data/name") - ) - ), - body( - input("/data/id"), - input("/data/name") - ) - ) - ) - - scenario.formEntryController.addPostProcessor(EntityFormFinalizationProcessor()) - scenario.finalizeInstance() - - val entitiesExtra = scenario.formEntryController.model.extras.get(EntitiesExtra::class.java) - val (entities, invalidEntities) = entitiesExtra - assertThat(entities.size, equalTo(0)) - assertThat(invalidEntities.size, equalTo(1)) - assertThat(invalidEntities[0].dataset, equalTo("people")) - assertThat(invalidEntities[0].id, equalTo(null)) - } - - @Test - fun `filling form with blank label makes invalid entity available`() { - val scenario = Scenario.init( - html( - listOf(Pair("entities", "http://www.opendatakit.org/xforms/entities")), - head( - title("Create entity form"), - model( - listOf(Pair("entities:entities-version", "2024.1.0")), - mainInstance( - t( - "data id=\"create-entity-form\"", - t("name"), - t("meta", entityNode("people", CREATE)) - ) - ), - bind("/data/name").type("string").withSaveTo("name"), - entityLabelBind("/data/name"), - ) - ), - body( - input("/data/name") - ) - ) - ) - - scenario.formEntryController.addPostProcessor(EntityFormFinalizationProcessor()) - scenario.answer("/data/name", " ") - scenario.finalizeInstance() - - val entitiesExtra = scenario.formEntryController.model.extras.get(EntitiesExtra::class.java) - val (entities, invalidEntities) = entitiesExtra - assertThat(entities.size, equalTo(0)) - assertThat(invalidEntities.size, equalTo(1)) - assertThat(invalidEntities[0].dataset, equalTo("people")) - assertThat(invalidEntities[0].label, equalTo(" ")) - } - - @Test - fun `filling fom with non-UUID id makes invalid entity available`() { - val scenario = Scenario.init( - html( - listOf(Pair("entities", "http://www.opendatakit.org/xforms/entities")), - head( - title("Create entity form"), - model( - listOf(Pair("entities:entities-version", "2024.1.0")), - mainInstance( - t( - "data id=\"create-entity-form\"", - t("name"), - t("meta", entityNode("people", CREATE)) - ) - ), - bind("/data/name").type("string").withSaveTo("name"), - entityLabelBind("/data/name"), - setvalue("odk-instance-first-load", "/data/meta/entity/@id", "1") - ) - ), - body( - input("/data/name") - ) - ) - ) - - scenario.formEntryController.addPostProcessor(EntityFormFinalizationProcessor()) - scenario.answer("/data/name", "Dylan") - scenario.finalizeInstance() - - val entitiesExtra = scenario.formEntryController.model.extras.get(EntitiesExtra::class.java) - val (entities, invalidEntities) = entitiesExtra - assertThat(entities.size, equalTo(0)) - assertThat(invalidEntities.size, equalTo(1)) - assertThat(invalidEntities[0].dataset, equalTo("people")) - assertThat(invalidEntities[0].id, equalTo("1")) - } - @Test fun `filling form with update makes entity available`() { val scenario = Scenario.init( @@ -846,42 +724,6 @@ class EntitiesTest { assertThat(entities[0].action, equalTo(EntityAction.UPDATE)) } - @Test - fun `filling form with update without an id does not make entity available`() { - val scenario = Scenario.init( - html( - listOf(Pair("entities", "http://www.opendatakit.org/xforms/entities")), - head( - title("Update entity form"), - model( - listOf(Pair("entities:entities-version", "2024.1.0")), - mainInstance( - t( - "data id=\"update-entity-form\"", - t("id"), - t( - "meta", - t("entity dataset=\"people\" update=\"1\" id=\"\" baseVersion=\"\"") - ) - ) - ), - bind("/data/id").type("string"), - bind("/data/meta/entity/@id").type("string").calculate("/data/id").readonly() - ) - ), - body( - input("/data/id") - ) - ) - ) - - scenario.formEntryController.addPostProcessor(EntityFormFinalizationProcessor()) - scenario.finalizeInstance() - - val entities = scenario.formEntryController.model.extras.get(EntitiesExtra::class.java).entities - assertThat(entities.size, equalTo(0)) - } - @Test fun `filling form with create and update makes entity available with upsert action`() { val scenario = Scenario.init( diff --git a/shared/src/main/java/org/odk/collect/shared/DebugLogger.kt b/shared/src/main/java/org/odk/collect/shared/DebugLogger.kt deleted file mode 100644 index 45a1643fd5d..00000000000 --- a/shared/src/main/java/org/odk/collect/shared/DebugLogger.kt +++ /dev/null @@ -1,5 +0,0 @@ -package org.odk.collect.shared - -interface DebugLogger { - fun log(tag: String, message: String) -} diff --git a/shared/src/main/java/org/odk/collect/shared/debug/DebugEvent.kt b/shared/src/main/java/org/odk/collect/shared/debug/DebugEvent.kt new file mode 100644 index 00000000000..29a0825853e --- /dev/null +++ b/shared/src/main/java/org/odk/collect/shared/debug/DebugEvent.kt @@ -0,0 +1,6 @@ +package org.odk.collect.shared.debug + +interface DebugEvent { + val message: String + val analyticsEvent: String +} diff --git a/shared/src/main/java/org/odk/collect/shared/debug/DebugLogger.kt b/shared/src/main/java/org/odk/collect/shared/debug/DebugLogger.kt new file mode 100644 index 00000000000..cc2067a2e8b --- /dev/null +++ b/shared/src/main/java/org/odk/collect/shared/debug/DebugLogger.kt @@ -0,0 +1,9 @@ +package org.odk.collect.shared.debug + +/** + * Records [DebugEvent]s. Implementations decide how to surface them - for example as a line in a + * debug log file and/or an analytics event. + */ +interface DebugLogger { + fun log(event: DebugEvent) +}