From 26c26acd049f7080013eb3796918d6029a15bb53 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Sun, 8 Dec 2024 13:22:30 +0100 Subject: [PATCH 1/3] Added a failing test --- .../collect/android/entities/EntitiesRepositoryTest.kt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt b/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt index 001049a6489..e10726dc5d2 100644 --- a/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt +++ b/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt @@ -757,5 +757,14 @@ abstract class EntitiesRepositoryTest { savedEntities = repository.getEntities("things") assertThat(savedEntities[0].properties.size, equalTo(1)) assertThat(savedEntities[0].properties[0].first, equalTo("prop")) + + /** + * Attempt to save again to ensure that duplicate properties are correctly compared, + * even if they appear in a different order. + */ + repository.save("things", entity.copy(properties = listOf(Pair("Prop", "value"), Pair("prop", "value")))) + savedEntities = repository.getEntities("things") + assertThat(savedEntities[0].properties.size, equalTo(1)) + assertThat(savedEntities[0].properties[0].first, equalTo("prop")) } } From 4a98a40520392095e2ae53e3c2a5dc80a570bf16 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Sun, 8 Dec 2024 13:22:53 +0100 Subject: [PATCH 2/3] Fixed the filtering of duplicate columns --- .../database/entities/DatabaseEntitiesRepository.kt | 2 +- .../collect/entities/storage/InMemEntitiesRepository.kt | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt index 33f8b08fe4d..87d8b39ece6 100644 --- a/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt +++ b/collect_app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt @@ -383,7 +383,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep val missingColumns = entity.properties .map { EntitiesTable.getPropertyColumn(it.first) } .distinctBy { it.lowercase() } - .filterNot { columnNames.contains(it) } + .filterNot { columnName -> columnNames.any { it.equals(columnName, ignoreCase = true) } } if (missingColumns.isNotEmpty()) { databaseConnection.resetTransaction { diff --git a/entities/src/main/java/org/odk/collect/entities/storage/InMemEntitiesRepository.kt b/entities/src/main/java/org/odk/collect/entities/storage/InMemEntitiesRepository.kt index e467df1a104..dc07a81d9b1 100644 --- a/entities/src/main/java/org/odk/collect/entities/storage/InMemEntitiesRepository.kt +++ b/entities/src/main/java/org/odk/collect/entities/storage/InMemEntitiesRepository.kt @@ -121,13 +121,15 @@ class InMemEntitiesRepository : EntitiesRepository { private fun updateLists(list: String, entity: Entity) { lists.add(list) - listProperties.getOrPut(list) { + val properties = listProperties.getOrPut(list) { mutableSetOf() - }.addAll( + } + properties.addAll( entity .properties - .distinctBy { it.first.lowercase() } .map { it.first } + .distinctBy { it.lowercase() } + .filterNot { properties.any { property -> property.equals(it, ignoreCase = true) } } ) } From 98643acc479c4593a00450bedb46c6cf9ba74413 Mon Sep 17 00:00:00 2001 From: Grzegorz Orczykowski Date: Mon, 9 Dec 2024 13:22:59 +0100 Subject: [PATCH 3/3] Improved tests --- .../entities/EntitiesRepositoryTest.kt | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt b/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt index e10726dc5d2..6a09b148bc9 100644 --- a/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt +++ b/collect_app/src/test/java/org/odk/collect/android/entities/EntitiesRepositoryTest.kt @@ -736,7 +736,7 @@ abstract class EntitiesRepositoryTest { } @Test - fun `#save ignores case-insensitive duplicate properties`() { + fun `#save ignores case-insensitive duplicate new properties`() { val repository = buildSubject() val entity = Entity.New( "1", @@ -745,24 +745,26 @@ abstract class EntitiesRepositoryTest { ) repository.save("things", entity) - var savedEntities = repository.getEntities("things") + val savedEntities = repository.getEntities("things") assertThat(savedEntities[0].properties.size, equalTo(1)) assertThat(savedEntities[0].properties[0].first, equalTo("prop")) + } + + @Test + fun `#save ignores case-insensitive duplicate properties if one of them has already been saved`() { + val repository = buildSubject() + val entity = Entity.New( + "1", + "One", + properties = listOf(Pair("prop", "value")) + ) - /** - * Attempt to save again to ensure that duplicate properties are correctly compared, not only - * within the current entity list, but also against properties already saved in the database. - */ repository.save("things", entity) - savedEntities = repository.getEntities("things") + var savedEntities = repository.getEntities("things") assertThat(savedEntities[0].properties.size, equalTo(1)) assertThat(savedEntities[0].properties[0].first, equalTo("prop")) - /** - * Attempt to save again to ensure that duplicate properties are correctly compared, - * even if they appear in a different order. - */ - repository.save("things", entity.copy(properties = listOf(Pair("Prop", "value"), Pair("prop", "value")))) + repository.save("things", entity.copy(properties = listOf(Pair("Prop", "value")))) savedEntities = repository.getEntities("things") assertThat(savedEntities[0].properties.size, equalTo(1)) assertThat(savedEntities[0].properties[0].first, equalTo("prop"))