Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return unique include and revInclude resources per search filter match. #2544

Merged
merged 20 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e1edb37
Return unique include and revInclude resources per match
aditya-07 May 14, 2024
74c76cf
Merge branch 'master' into ak/distinctIncludedResources
aditya-07 May 16, 2024
871fba5
Spotless
aditya-07 May 16, 2024
88e18e0
Merge branch 'master' into ak/distinctIncludedResources
aditya-07 May 27, 2024
7d957b2
Review comment: Use SELECT DISTINCT sql query and uodate tests
aditya-07 May 27, 2024
9eb8c20
Merge branch 'master' into ak/distinctIncludedResources
aditya-07 May 30, 2024
735da6a
Merge branch 'master' into ak/distinctIncludedResources
aditya-07 Jun 13, 2024
6bbba91
Merge branch 'master' into ak/distinctIncludedResources
aditya-07 Jun 18, 2024
b9ff17d
Review commenyts: Updated code and comments
aditya-07 Jun 18, 2024
5c36415
Merge branch 'master' into ak/distinctIncludedResources
aditya-07 Jun 18, 2024
141a17b
Merge branch 'master' into ak/distinctIncludedResources
aditya-07 Jun 19, 2024
6f44ed9
Merge branch 'master' into ak/distinctIncludedResources
aditya-07 Jun 20, 2024
8e390ea
Refactored code to use GROUPBY + HAVING for sorting
aditya-07 Jun 26, 2024
726c318
Removed print statements
aditya-07 Jun 26, 2024
a0dd859
Merge branch 'master' into ak/distinctIncludedResources
aditya-07 Jun 26, 2024
a43b1aa
fixed failing test
aditya-07 Jun 26, 2024
697ba31
Merge branch 'master' into ak/distinctIncludedResources
aditya-07 Jul 9, 2024
8a11e27
Merge branch 'master' into ak/distinctIncludedResources
aditya-07 Jul 23, 2024
178b54a
Review comments: Separated the ascending/descending test cases. Updat…
aditya-07 Jul 24, 2024
944bc58
Updated comments
aditya-07 Jul 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import org.hl7.fhir.r4.model.DecimalType
import org.hl7.fhir.r4.model.Encounter
import org.hl7.fhir.r4.model.Enumerations
import org.hl7.fhir.r4.model.Extension
import org.hl7.fhir.r4.model.Group
import org.hl7.fhir.r4.model.HumanName
import org.hl7.fhir.r4.model.Identifier
import org.hl7.fhir.r4.model.Immunization
Expand Down Expand Up @@ -4086,6 +4087,223 @@ class DatabaseImplTest {
assertThat(searchedObservations[0].logicalId).isEqualTo(locallyCreatedObservationResourceId)
}

@Test
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
fun included_results_should_have_distinct_resources() = runBlocking {
/**
* A person has multiple first names. Searching a group including Patient sorted by
* Patient.GIVEN should return single copy of of the Patient resource.
*
* In order to sort Patients by given name, engine joins ResourceEntity table with
* StringIndexEntity table which may result in multiple rows for a single Patient if the person
* has more than one given name.
*
* So the expectation is that the search result should only include distinct Patients and not
* repeated Patients (one for each given name).
*/
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
val group =
Group().apply {
id = "group"
addMember(Group.GroupMemberComponent(Reference("Patient/p1")))
addMember(Group.GroupMemberComponent(Reference("Patient/p2")))
}
val p1 =
Patient().apply {
id = "p1"
addName(
HumanName().apply {
family = "Cooper"
addGiven("3")
addGiven("1")
},
)
}

val p2 =
Patient().apply {
id = "p2"
addName(
HumanName().apply {
family = "Cooper"
addGiven("2")
addGiven("4")
},
)
}
database.insert(group, p1, p2)

val result =
Search(ResourceType.Group)
.apply { include<Patient>(Group.MEMBER) { sort(Patient.GIVEN, Order.ASCENDING) } }
.execute<Patient>(database)

assertThat(result)
.comparingElementsUsing(SearchResultCorrespondence)
.displayingDiffsPairedBy { it.resource.logicalId }
.contains(SearchResult(group, mapOf(Group.MEMBER.paramName to listOf(p2, p1)), null))
}

@Test
fun revIncluded_results_should_have_distinct_resources() = runBlocking {
// An encounter has multiple location and Searching a group revIncluding encounter with results
// sorted by Encounter.LOCATION_PERIOD should return single copy of the Encounter resource.

val group =
Group().apply {
id = "group"
addMember(Group.GroupMemberComponent(Reference("Patient/multiple-first-names")))
}

val encounter =
Encounter().apply {
id = "encounter-multiple-locations"

subject = Reference("Group/group")

addLocation().apply {
location = Reference("Location/1")
period =
Period().apply {
startElement = DateTimeType("2024-03-13T10:00:00-05:30")
endElement = DateTimeType("2024-03-13T10:30:00-05:30")
}
}

addLocation().apply {
location = Reference("Location/2")
period =
Period().apply {
startElement = DateTimeType("2024-03-13T11:00:00-05:30")
endElement = DateTimeType("2024-03-13T11:30:00-05:30")
}
}

addLocation().apply {
location = Reference("Location/3")
period =
Period().apply {
startElement = DateTimeType("2024-03-13T09:00:00-05:30")
endElement = DateTimeType("2024-03-13T09:30:00-05:30")
}
}
}
database.insert(group, encounter)

val result =
Search(ResourceType.Group)
.apply {
revInclude<Encounter>(Encounter.SUBJECT) {
sort(Encounter.LOCATION_PERIOD, Order.ASCENDING)
}
}
.execute<Patient>(database)

assertThat(result)
.comparingElementsUsing(SearchResultCorrespondence)
.displayingDiffsPairedBy { it.resource.logicalId }
.contains(
SearchResult(
group,
null,
mapOf(Pair(ResourceType.Encounter, Encounter.SUBJECT.paramName) to listOf(encounter)),
),
)
}

@Test
fun included_and_revIncluded_results_should_have_distinct_resources() = runBlocking {
// A person has multiple first names and encounter has multiple location
// Searching a group including Patient and revIncluding encounter with results sorted by
// Patient.GIVEN and Encounter.LOCATION_PERIOD should return single copies of the resources.
MJ1998 marked this conversation as resolved.
Show resolved Hide resolved

val group =
Group().apply {
id = "group"
addMember(Group.GroupMemberComponent(Reference("Patient/multiple-first-names")))
}
val patient =
Patient().apply {
id = "multiple-first-names"

addName(
HumanName().apply {
family = "LastName"
addGiven("FirstName-01")
addGiven("FirstName-02")
addGiven("FirstName-03")
},
)
}

val encounter =
Encounter().apply {
id = "encounter-multiple-locations"

subject = Reference("Group/group")

addLocation().apply {
location = Reference("Location/1")
period =
Period().apply {
startElement = DateTimeType("2024-03-13T10:00:00-05:30")
endElement = DateTimeType("2024-03-13T10:30:00-05:30")
}
}

addLocation().apply {
location = Reference("Location/2")
period =
Period().apply {
startElement = DateTimeType("2024-03-13T11:00:00-05:30")
endElement = DateTimeType("2024-03-13T11:30:00-05:30")
}
}

addLocation().apply {
location = Reference("Location/3")
period =
Period().apply {
startElement = DateTimeType("2024-03-13T09:00:00-05:30")
endElement = DateTimeType("2024-03-13T09:30:00-05:30")
}
}
}
database.insert(group, patient, encounter)

val result =
Search(ResourceType.Group)
.apply {
include<Patient>(Group.MEMBER) {
filter(
Patient.GIVEN,
{
value = "FirstName"
modifier = StringFilterModifier.STARTS_WITH
},
)

sort(Patient.GIVEN, Order.ASCENDING)
}

revInclude<Encounter>(Encounter.SUBJECT) {
sort(Encounter.LOCATION_PERIOD, Order.ASCENDING)
}
}
.execute<Patient>(database)

assertThat(result)
.comparingElementsUsing(SearchResultCorrespondence)
.displayingDiffsPairedBy { it.resource.logicalId }
.contains(
SearchResult(
group,
mapOf(
Group.MEMBER.paramName to listOf(patient),
),
mapOf(Pair(ResourceType.Encounter, Encounter.SUBJECT.paramName) to listOf(encounter)),
),
)
}

private companion object {
const val mockEpochTimeStamp = 1628516301000
const val TEST_PATIENT_1_ID = "test_patient_1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ internal abstract class ResourceDao {
it.copy(
serializedResource = iParser.encodeResourceToString(resource),
lastUpdatedLocal = timeOfLocalChange,
lastUpdatedRemote = resource.meta.lastUpdated?.toInstant() ?: it.lastUpdatedRemote,
MJ1998 marked this conversation as resolved.
Show resolved Hide resolved
)
updateChanges(entity, resource)
}
Expand Down Expand Up @@ -129,11 +130,6 @@ internal abstract class ResourceDao {
createLocalLastUpdatedIndex(resource.resourceType, InstantType(Date.from(instant))),
)
}
entity.lastUpdatedRemote?.let { instant ->
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
addDateTimeIndex(
createLastUpdatedIndex(resource.resourceType, InstantType(Date.from(instant))),
)
}
}
.build()
updateIndicesForResource(index, resource.resourceType, entity.resourceUuid)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2022-2023 Google LLC
* Copyright 2022-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.
Expand Down Expand Up @@ -30,6 +30,8 @@ import com.google.android.fhir.search.filter.ReferenceParamFilterCriterion
import com.google.android.fhir.search.filter.StringParamFilterCriterion
import com.google.android.fhir.search.filter.TokenParamFilterCriterion
import com.google.android.fhir.search.filter.UriParamFilterCriterion
import org.hl7.fhir.r4.model.HumanName
import org.hl7.fhir.r4.model.Patient

@DslMarker @Target(AnnotationTarget.CLASS, AnnotationTarget.TYPE) annotation class BaseSearchDsl

Expand Down Expand Up @@ -90,9 +92,65 @@ interface BaseSearch {
operation: Operation = Operation.OR,
)

/**
* When sorting is applied on a field with repeated values (e.g. [Patient.GIVEN] ), the order is
* defined by the order of the repeated values in the resource (e.g. [HumanName.given] for
* [Patient]).
*
* If there are two Patients p1 and p2 as follows
*
* ```
* {
* "resourceType": "Patient",
* "id": "p1",
* "name": [
* {
* "family": "Cooper",
* "given": [
* "3",
* "1"
* ]
* }
* ]
* }
* ```
*
* AND
*
* ```
* {
* "resourceType": "Patient",
* "id": "p2",
* "name": [
* {
* "family": "Cooper",
* "given": [
* "2",
* "4"
* ]
* }
* ]
* }
* ```
*
* Then sorting the patients with their given i.e [Patient.GIVEN] depends on the first given in
* the first name (i.e **3** and **2**).
*/
fun sort(parameter: StringClientParam, order: Order)

/**
* When sorting is applied on a field with repeated values, the order is defined by the order of
* for repeated values in the resource.
*
* @see sort(parameter: StringClientParam, order: Order) for more details.
*/
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
fun sort(parameter: NumberClientParam, order: Order)

/**
* When sorting is applied on a field with repeated values, the order is defined by the order of
* for repeated values in the resource.
*
* @see sort(parameter: StringClientParam, order: Order) for more details.
*/
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
fun sort(parameter: DateClientParam, order: Order)
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ internal fun Search.getRevIncludeQuery(includeIds: List<String>): SearchQuery {
args.addAll(join.args)
val filterQuery = generateFilterQuery(it)
"""
SELECT rie.index_name, rie.index_value, re.serializedResource
SELECT DISTINCT rie.index_name, rie.index_value, re.serializedResource
FROM ResourceEntity re
JOIN ReferenceIndexEntity rie
ON re.resourceUuid = rie.resourceUuid
Expand Down Expand Up @@ -198,7 +198,7 @@ internal fun Search.getIncludeQuery(includeIds: List<UUID>): SearchQuery {
args.addAll(join.args)
val filterQuery = generateFilterQuery(it)
"""
SELECT rie.index_name, rie.resourceUuid, re.serializedResource
SELECT DISTINCT rie.index_name, rie.resourceUuid, re.serializedResource
FROM ResourceEntity re
JOIN ReferenceIndexEntity rie
ON re.resourceType||"/"||re.resourceId = rie.index_value
Expand Down
Loading
Loading