Merge pull request #6569 from grzesiek2010/COLLECT-6568

Fixed reading properties from lists with special characters
getodk · Jan 21, 2025 · 41b5cc6 · 41b5cc6
2 parents 8c76211 + c663beb
commit 41b5cc6
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 50 deletions.
diff --git a/...app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt b/...app/src/main/java/org/odk/collect/android/database/entities/DatabaseEntitiesRepository.kt
@@ -181,12 +181,6 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
         }
     }
 
-    override fun clear() {
-        databaseConnection.withConnection {
-            dropAllTablesFromDB(writableDatabase)
-        }
-    }
-
     override fun addList(list: String) {
         if (!listExists(list)) {
             createList(list)
@@ -232,7 +226,7 @@ class DatabaseEntitiesRepository(context: Context, dbPath: String) : EntitiesRep
         }
 
         val propertyExists = databaseConnection.withConnection {
-            readableDatabase.doesColumnExist(list, EntitiesTable.getPropertyColumn(property))
+            readableDatabase.doesColumnExist(quote(list), EntitiesTable.getPropertyColumn(property))
         }
 
         return if (propertyExists) {

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
@@ -313,27 +313,6 @@ abstract class EntitiesRepositoryTest {
         assertThat(repository.getEntities("favourite.wines")[0], sameEntityAs(wine))
     }
 
-    @Test
-    fun `#clear deletes all entities`() {
-        val repository = buildSubject()
-
-        val wine = Entity.New("1", "Léoville Barton 2008")
-        val whisky = Entity.New("2", "Lagavulin 16")
-        repository.save("wines", wine)
-        repository.save("whiskys", whisky)
-
-        repository.clear()
-        assertThat(repository.getLists().size, equalTo(0))
-        assertThat(repository.getEntities("wines").size, equalTo(0))
-        assertThat(repository.getEntities("whiskys").size, equalTo(0))
-
-        repository.addList("wines")
-        assertThat(repository.getEntities("wines").size, equalTo(0))
-
-        repository.addList("whiskys")
-        assertThat(repository.getEntities("whiskys").size, equalTo(0))
-    }
-
     @Test
     fun `#save can save multiple entities`() {
         val repository = buildSubject()
@@ -537,7 +516,7 @@ abstract class EntitiesRepositoryTest {
     }
 
     @Test
-    fun `#getByAllByProperty returns entities with matching property value`() {
+    fun `#getAllByProperty returns entities with matching property value`() {
         val repository = buildSubject()
 
         val leoville = Entity.New(
@@ -562,7 +541,7 @@ abstract class EntitiesRepositoryTest {
     }
 
     @Test
-    fun `#getByAllByProperty returns entities without property when searching for empty string`() {
+    fun `#getAllByProperty returns entities without property when searching for empty string`() {
         val repository = buildSubject()
 
         val leoville = Entity.New(
@@ -586,7 +565,7 @@ abstract class EntitiesRepositoryTest {
     }
 
     @Test
-    fun `#getByAllByProperty returns entities when searching for empty string for property that doesn't exist`() {
+    fun `#getAllByProperty returns entities when searching for empty string for property that doesn't exist`() {
         val repository = buildSubject()
 
         val leoville = Entity.New(
@@ -600,7 +579,7 @@ abstract class EntitiesRepositoryTest {
     }
 
     @Test
-    fun `#getByAllByProperty returns empty list when searching for non empty string for property that doesn't exist`() {
+    fun `#getAllByProperty returns empty list when searching for non empty string for property that doesn't exist`() {
         val repository = buildSubject()
 
         val leoville = Entity.New(
@@ -659,6 +638,32 @@ abstract class EntitiesRepositoryTest {
         assertThat(repository.getAllByProperty("wines", "vintage", "1983"), equalTo(emptyList()))
     }
 
+    @Test
+    fun `#getAllByProperty supports list names with dots and dashes`() {
+        val repository = buildSubject()
+
+        val leoville = Entity.New(
+            "1",
+            "Léoville Barton 2008",
+            properties = listOf("vintage" to "2008")
+        )
+
+        repository.save("favourite-wines", leoville)
+        repository.save("favourite.wines", leoville)
+
+        var wines = repository.getEntities("favourite-wines")
+        assertThat(
+            repository.getAllByProperty("favourite-wines", "vintage", "2008"),
+            containsInAnyOrder(wines.first { it.id == "1" })
+        )
+
+        wines = repository.getEntities("favourite.wines")
+        assertThat(
+            repository.getAllByProperty("favourite.wines", "vintage", "2008"),
+            containsInAnyOrder(wines.first { it.id == "1" })
+        )
+    }
+
     @Test
     fun `#getCount returns 0 when a list is empty`() {
         val repository = buildSubject()

diff --git a/entities/src/main/java/org/odk/collect/entities/browser/EntitiesViewModel.kt b/entities/src/main/java/org/odk/collect/entities/browser/EntitiesViewModel.kt
@@ -29,11 +29,4 @@ class EntitiesViewModel(
 
         return result
     }
-
-    fun clearAll() {
-        scheduler.immediate {
-            entitiesRepository.clear()
-            _lists.postValue(entitiesRepository.getLists().toList())
-        }
-    }
 }
diff --git a/entities/src/main/java/org/odk/collect/entities/storage/EntitiesRepository.kt b/entities/src/main/java/org/odk/collect/entities/storage/EntitiesRepository.kt
@@ -5,7 +5,6 @@ interface EntitiesRepository {
     fun getLists(): Set<String>
     fun getEntities(list: String): List<Entity.Saved>
     fun getCount(list: String): Int
-    fun clear()
     fun addList(list: String)
     fun delete(id: String)
     fun getById(list: String, id: String): Entity.Saved?

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
@@ -31,11 +31,6 @@ class InMemEntitiesRepository : EntitiesRepository {
         return getEntities(list).count()
     }
 
-    override fun clear() {
-        entities.clear()
-        lists.clear()
-    }
-
     override fun addList(list: String) {
         lists.add(list)
     }

diff --git a/entities/src/test/java/org/odk/collect/entities/LocalEntityUseCasesTest.kt b/entities/src/test/java/org/odk/collect/entities/LocalEntityUseCasesTest.kt
@@ -526,11 +526,6 @@ private class MeasurableEntitiesRepository(private val wrapped: EntitiesReposito
         return wrapped.getCount(list)
     }
 
-    override fun clear() {
-        accesses += 1
-        wrapped.clear()
-    }
-
     override fun addList(list: String) {
         accesses += 1
         wrapped.addList(list)