From 3d38081e47d43afa079839545675ee4b0899208b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=E2=89=A1ZRS?= <12814349+LZRS@users.noreply.github.com> Date: Wed, 18 Dec 2024 18:04:28 +0300 Subject: [PATCH] Fix failing test for disjoint filters with nested query --- .../android/fhir/db/impl/DatabaseImplTest.kt | 137 +++++++++--------- .../google/android/fhir/search/MoreSearch.kt | 26 ++-- .../google/android/fhir/search/SearchTest.kt | 22 ++- 3 files changed, 98 insertions(+), 87 deletions(-) diff --git a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt b/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt index 46407e9cea..77b84ec5bb 100644 --- a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt +++ b/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt @@ -2809,78 +2809,79 @@ class DatabaseImplTest { } @Test - fun test_search_multiple_param_disjunction_covid_immunization_records() = runBlocking { - val resources = - listOf( - Immunization().apply { - id = "immunization-1" - vaccineCode = - CodeableConcept( - Coding( - "http://id.who.int/icd11/mms", - "XM1NL1", - "COVID-19 vaccine, inactivated virus", - ), - ) - status = Immunization.ImmunizationStatus.COMPLETED - }, - Immunization().apply { - id = "immunization-2" - vaccineCode = - CodeableConcept( - Coding( - "http://id.who.int/icd11/mms", - "XM5DF6", - "COVID-19 vaccine, live attenuated virus", - ), - ) - status = Immunization.ImmunizationStatus.COMPLETED - }, - Immunization().apply { - id = "immunization-3" - vaccineCode = - CodeableConcept( - Coding("http://id.who.int/icd11/mms", "XM6AT1", "COVID-19 vaccine, DNA based"), - ) - status = Immunization.ImmunizationStatus.COMPLETED - }, - Immunization().apply { - id = "immunization-4" - vaccineCode = - CodeableConcept( - Coding( - "http://hl7.org/fhir/sid/cvx", - "140", - "Influenza, seasonal, injectable, preservative free", - ), - ) - status = Immunization.ImmunizationStatus.COMPLETED - }, - ) + fun test_search_multiple_param_disjunction_covid_immunization_records() { + runBlocking { + val resources = + listOf( + Immunization().apply { + id = "immunization-1" + vaccineCode = + CodeableConcept( + Coding( + "http://id.who.int/icd11/mms", + "XM1NL1", + "COVID-19 vaccine, inactivated virus", + ), + ) + status = Immunization.ImmunizationStatus.COMPLETED + }, + Immunization().apply { + id = "immunization-2" + vaccineCode = + CodeableConcept( + Coding( + "http://id.who.int/icd11/mms", + "XM5DF6", + "COVID-19 vaccine, live attenuated virus", + ), + ) + status = Immunization.ImmunizationStatus.COMPLETED + }, + Immunization().apply { + id = "immunization-3" + vaccineCode = + CodeableConcept( + Coding("http://id.who.int/icd11/mms", "XM6AT1", "COVID-19 vaccine, DNA based"), + ) + status = Immunization.ImmunizationStatus.COMPLETED + }, + Immunization().apply { + id = "immunization-4" + vaccineCode = + CodeableConcept( + Coding( + "http://hl7.org/fhir/sid/cvx", + "140", + "Influenza, seasonal, injectable, preservative free", + ), + ) + status = Immunization.ImmunizationStatus.COMPLETED + }, + ) - database.insert(*resources.toTypedArray()) + database.insert(*resources.toTypedArray()) - val result = - database.search( - Search(ResourceType.Immunization) - .apply { - filter( - Immunization.VACCINE_CODE, - { value = of(Coding("http://id.who.int/icd11/mms", "XM1NL1", "")) }, - ) + val result = + database.search( + Search(ResourceType.Immunization) + .apply { + filter( + Immunization.VACCINE_CODE, + { value = of(Coding("http://id.who.int/icd11/mms", "XM1NL1", "")) }, + ) - filter( - Immunization.VACCINE_CODE, - { value = of(Coding("http://id.who.int/icd11/mms", "XM5DF6", "")) }, - ) - operation = Operation.OR - } - .getQuery(), - ) + filter( + Immunization.VACCINE_CODE, + { value = of(Coding("http://id.who.int/icd11/mms", "XM5DF6", "")) }, + ) + operation = Operation.OR + } + .getQuery(), + ) - assertThat(result.map { it.resource.vaccineCode.codingFirstRep.code }) - .containsExactly("XM1NL1", "XM5DF6") - .inOrder() + assertThat(result.map { it.resource.vaccineCode.codingFirstRep.code }) + .containsExactly("XM1NL1", "XM5DF6") + } } @Test diff --git a/engine/src/main/java/com/google/android/fhir/search/MoreSearch.kt b/engine/src/main/java/com/google/android/fhir/search/MoreSearch.kt index daa577b68f..a6231c8bdc 100644 --- a/engine/src/main/java/com/google/android/fhir/search/MoreSearch.kt +++ b/engine/src/main/java/com/google/android/fhir/search/MoreSearch.kt @@ -364,17 +364,13 @@ internal fun Search.getQuery( val sortArgs = join.args val filterQuery = getFilterQueries() - val filterQueryStatement = - filterQuery.joinToString(separator = "${operation.logicalOperator} ") { - // spotless:off - """ - a.resourceUuid IN ( - ${it.query} - ) - - """.trimIndent() - // spotless:on + val filterQueryJoinOperator = + when (operation) { + Operation.OR -> "\nUNION\n" + Operation.AND -> "\nINTERSECT\n" } + val filterQueryStatement = + filterQuery.joinToString(separator = filterQueryJoinOperator) { it.query.trimIndent() } val filterQueryArgs = filterQuery.flatMap { it.args } var limitStatement = "" @@ -398,8 +394,14 @@ internal fun Search.getQuery( val filterStatement = listOf(filterQueryStatement, nestedQueryFilterStatement) .filter { it.isNotBlank() } - .joinToString(separator = " AND ") - .ifBlank { "a.resourceType = ?" } + .joinToString(separator = "\nINTERSECT\n") + .takeIf { it.isNotBlank() } + ?.let { """a.resourceUuid IN ( + $it + ) + """ } + ?: "a.resourceType = ?" + val filterArgs = (filterQueryArgs + nestedQueryFilterArgs).ifEmpty { listOf(type.name) } val whereArgs = mutableListOf() diff --git a/engine/src/test/java/com/google/android/fhir/search/SearchTest.kt b/engine/src/test/java/com/google/android/fhir/search/SearchTest.kt index d78796b424..cec0f6f810 100644 --- a/engine/src/test/java/com/google/android/fhir/search/SearchTest.kt +++ b/engine/src/test/java/com/google/android/fhir/search/SearchTest.kt @@ -1872,7 +1872,7 @@ class SearchTest { } @Test - fun search_patient_multiple_given_disjoint_has_condition_diabetes() { + fun search_patient_multiple_given_disjoint_has_condition_diabetes_or_hypertension() { val query = Search(ResourceType.Patient) .apply { @@ -1881,6 +1881,11 @@ class SearchTest { Condition.CODE, { value = of(Coding("http://snomed.info/sct", "44054006", "Diabetes")) }, ) + filter( + Condition.CODE, + { value = of(Coding("http://snomed.info/sct", "827069000", "Hypertension stage 1")) }, + ) + operation = Operation.OR } filter( @@ -1907,8 +1912,7 @@ class SearchTest { """ SELECT a.resourceUuid, a.serializedResource FROM ResourceEntity a - WHERE a.resourceType = ? - AND a.resourceUuid IN ( + WHERE a.resourceUuid IN ( SELECT resourceUuid FROM StringIndexEntity WHERE resourceType = ? AND index_name = ? AND index_value = ? UNION @@ -1920,8 +1924,10 @@ class SearchTest { WHERE a.resourceType = ? AND a.resourceId IN ( SELECT substr(a.index_value, 9) FROM ReferenceIndexEntity a - WHERE a.resourceType = ? AND a.index_name = ? - AND a.resourceUuid IN ( + WHERE a.index_name = ? AND a.resourceUuid IN ( + SELECT resourceUuid FROM TokenIndexEntity + WHERE resourceType = ? AND index_name = ? AND (index_value = ? AND IFNULL(index_system,'') = ?) + UNION SELECT resourceUuid FROM TokenIndexEntity WHERE resourceType = ? AND index_name = ? AND (index_value = ? AND IFNULL(index_system,'') = ?) ) @@ -1934,7 +1940,6 @@ class SearchTest { assertThat(query.args) .isEqualTo( listOf( - "Patient", "Patient", "given", "John", @@ -1942,12 +1947,15 @@ class SearchTest { "given", "Jane", "Patient", - "Condition", "subject", "Condition", "code", "44054006", "http://snomed.info/sct", + "Condition", + "code", + "827069000", + "http://snomed.info/sct", ), ) }