Review comments: Changed the search result type to include the search…

… param as well.

engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt

@@ -3244,7 +3244,7 @@ class DatabaseImplTest {
  revInclude<Condition>(Condition.SUBJECT) {
  filter(Condition.CODE, { value = of(diabetesCodeableConcept) })
  filter(Condition.CODE, { value = of(migraineCodeableConcept) })
- operations(Operation.OR)
+ operation = Operation.OR
  }
 
  revInclude<Encounter>(Encounter.SUBJECT) {
@@ -3266,7 +3266,7 @@ class DatabaseImplTest {
  modifier = StringFilterModifier.STARTS_WITH
  }
  )
- operations(Operation.AND)
+ operation = Operation.AND
  }
 
  include<Organization>(Patient.ORGANIZATION) {
@@ -3278,37 +3278,84 @@ class DatabaseImplTest {
  }
  )
  filter(Practitioner.ACTIVE, { value = of(true) })
- operations(Operation.AND)
+ operation = Operation.AND
  }
  }
  .execute<Patient>(database)
 
  assertThat(result[0].resource.logicalId).isEqualTo("pa-01")
- assertThat(result[0].included!![ResourceType.Practitioner]!!.map { it.logicalId })
+ assertThat(
+ result[0]
+ .included!![ResourceType.Practitioner]!![Patient.GENERAL_PRACTITIONER.paramName]!!.map {
+ it.logicalId
+ }
+ )
  .containsExactly("gp-01", "gp-02")
- assertThat(result[0].included!![ResourceType.Organization]!!.map { it.logicalId })
+ assertThat(
+ result[0].included!![ResourceType.Organization]!![Patient.ORGANIZATION.paramName]!!.map {
+ it.logicalId
+ }
+ )
  .containsExactly("org-01")
- assertThat(result[0].revIncluded!![ResourceType.Condition]!!.map { it.logicalId })
+ assertThat(
+ result[0].revIncluded!![ResourceType.Condition]!![Condition.SUBJECT.paramName]!!.map {
+ it.logicalId
+ }
+ )
  .containsExactly("con-01-pa-01", "con-03-pa-01")
- assertThat(result[0].revIncluded!![ResourceType.Encounter]!!.map { it.logicalId })
+ assertThat(
+ result[0].revIncluded!![ResourceType.Encounter]!![Encounter.SUBJECT.paramName]!!.map {
+ it.logicalId
+ }
+ )
  .containsExactly("en-01-pa-01", "en-02-pa-01")
 
  assertThat(result[1].resource.logicalId).isEqualTo("pa-02")
- assertThat(result[1].included!![ResourceType.Practitioner]!!.map { it.logicalId })
+ assertThat(
+ result[1]
+ .included!![ResourceType.Practitioner]!![Patient.GENERAL_PRACTITIONER.paramName]!!.map {
+ it.logicalId
+ }
+ )
  .containsExactly("gp-01", "gp-02")
- assertThat(result[1].included!![ResourceType.Organization]!!.map { it.logicalId })
+ assertThat(
+ result[1].included!![ResourceType.Organization]!![Patient.ORGANIZATION.paramName]!!.map {
+ it.logicalId
+ }
+ )
  .containsExactly("org-02")
- assertThat(result[1].revIncluded!![ResourceType.Condition]!!.map { it.logicalId })
+ assertThat(
+ result[1].revIncluded!![ResourceType.Condition]!![Condition.SUBJECT.paramName]!!.map {
+ it.logicalId
+ }
+ )
  .containsExactly("con-01-pa-02", "con-03-pa-02")
- assertThat(result[1].revIncluded!![ResourceType.Encounter]!!.map { it.logicalId })
+ assertThat(
+ result[1].revIncluded!![ResourceType.Encounter]!![Encounter.SUBJECT.paramName]!!.map {
+ it.logicalId
+ }
+ )
  .containsExactly("en-01-pa-02", "en-02-pa-02")
 
  assertThat(result[2].resource.logicalId).isEqualTo("pa-03")
- assertThat(result[2].included!![ResourceType.Practitioner]!!.map { it.logicalId })
+ assertThat(
+ result[2]
+ .included!![ResourceType.Practitioner]!![Patient.GENERAL_PRACTITIONER.paramName]!!.map {
+ it.logicalId
+ }
+ )
  .containsExactly("gp-01", "gp-02")
- assertThat(result[2].revIncluded!![ResourceType.Condition]!!.map { it.logicalId })
+ assertThat(
+ result[2].revIncluded!![ResourceType.Condition]!![Condition.SUBJECT.paramName]!!.map {
+ it.logicalId
+ }
+ )
  .containsExactly("con-01-pa-03", "con-03-pa-03")
- assertThat(result[2].revIncluded!![ResourceType.Encounter]!!.map { it.logicalId })
+ assertThat(
+ result[2].revIncluded!![ResourceType.Encounter]!![Encounter.SUBJECT.paramName]!!.map {
+ it.logicalId
+ }
+ )
  .containsExactly("en-01-pa-03", "en-02-pa-03")
  }
 

engine/src/main/java/com/google/android/fhir/FhirEngine.kt

@@ -128,7 +128,9 @@ suspend inline fun <reified R : Resource> FhirEngine.delete(id: String) {
  delete(getResourceType(R::class.java), id)
 }
 
-typealias ReferencedResources = Map<ResourceType, List<Resource>>
+typealias SearchParamName = String
+
+typealias ReferencedResources = Map<ResourceType, Map<SearchParamName, List<Resource>>>
 
 /** It contains the searched resource and referenced resources as per the search query. */
 data class SearchResult<R : Resource>(

engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt

@@ -189,12 +189,15 @@ internal class DatabaseImpl(
 
  override suspend fun searchReferencedResources(query: SearchQuery): List<IndexedIdAndResource> {
  return db.withTransaction {
- resourceDao.getResourcesRev(SimpleSQLiteQuery(query.query, query.args.toTypedArray())).map {
- IndexedIdAndResource(
- it.idOfBaseResourceOnWhichThisMatchedInc ?: it.idOfBaseResourceOnWhichThisMatchedRev!!,
- iParser.parseResource(it.serializedResource) as Resource
- )
- }
+ resourceDao
+ .getReferencedResources(SimpleSQLiteQuery(query.query, query.args.toTypedArray()))
+ .map {
+ IndexedIdAndResource(
+ it.matchingIndex,
+ it.idOfBaseResourceOnWhichThisMatchedInc ?: it.idOfBaseResourceOnWhichThisMatchedRev!!,
+ iParser.parseResource(it.serializedResource) as Resource
+ )
+ }
  }
  }
 

engine/src/main/java/com/google/android/fhir/db/impl/dao/ResourceDao.kt

@@ -152,7 +152,7 @@ internal abstract class ResourceDao {
  @RawQuery abstract suspend fun getResources(query: SupportSQLiteQuery): List<String>
 
  @RawQuery
- abstract suspend fun getResourcesRev(
+ abstract suspend fun getReferencedResources(
  query: SupportSQLiteQuery
  ): List<IndexedIdAndSerializedResource>
 
@@ -286,13 +286,23 @@ internal abstract class ResourceDao {
  }
 }
 
+/**
+ * Data class representing the value returned by [getReferencedResources]. The optional fields may
+ * or may-not contain values based on the search query.
+ */
 internal data class IndexedIdAndSerializedResource(
+ @ColumnInfo(name = "index_name") val matchingIndex: String,
  @ColumnInfo(name = "index_value") val idOfBaseResourceOnWhichThisMatchedRev: String?,
  @ColumnInfo(name = "resourceId") val idOfBaseResourceOnWhichThisMatchedInc: String?,
  val serializedResource: String
 )
 
+/**
+ * Data class representing an included or revIncluded [Resource], index on which the match was done
+ * and the id of the base [Resource] for which this [Resource] has been included.
+ */
 internal data class IndexedIdAndResource(
+ val matchingIndex: String,
  val idOfBaseResourceOnWhichThisMatched: String,
  val resource: Resource
 )

engine/src/main/java/com/google/android/fhir/search/ISearch.kt → engine/src/main/java/com/google/android/fhir/search/BaseSearch.kt

@@ -31,10 +31,22 @@ import com.google.android.fhir.search.filter.StringParamFilterCriterion
 import com.google.android.fhir.search.filter.TokenParamFilterCriterion
 import com.google.android.fhir.search.filter.UriParamFilterCriterion
 
-@DslMarker @Target(AnnotationTarget.CLASS, AnnotationTarget.TYPE) annotation class ISearchDsl
+@DslMarker @Target(AnnotationTarget.CLASS, AnnotationTarget.TYPE) annotation class BaseSearchDsl
 
-@ISearchDsl
-interface ISearch {
+/**
+ * Defines the basic functionality provided by the search including [filter], [sort] and logical
+ * [operation] on filters.
+ */
+@BaseSearchDsl
+interface BaseSearch {
+ /** Logical operator between the filters. */
+ var operation: Operation
+
+ /** Count of the maximum expected search results. */
+ var count: Int?
+
+ /** Index from which the matching search results should be returned. */
+ var from: Int?
 
  fun filter(
  stringParameter: StringClientParam,
@@ -83,6 +95,4 @@ interface ISearch {
  fun sort(parameter: NumberClientParam, order: Order)
 
  fun sort(parameter: DateClientParam, order: Order)
-
- fun operations(operation: Operation)
 }

engine/src/main/java/com/google/android/fhir/search/MoreSearch.kt

@@ -26,6 +26,7 @@ import com.google.android.fhir.SearchResult
 import com.google.android.fhir.UcumValue
 import com.google.android.fhir.UnitConverter
 import com.google.android.fhir.db.Database
+import com.google.android.fhir.db.impl.dao.IndexedIdAndResource
 import com.google.android.fhir.epochDay
 import com.google.android.fhir.logicalId
 import com.google.android.fhir.ucumUrl
@@ -68,16 +69,14 @@ internal suspend fun <R : Resource> Search.execute(database: Database): List<Sea
  included =
  includedResources
  ?.filter { it.idOfBaseResourceOnWhichThisMatched == baseResource.logicalId }
- ?.map { it.resource }
- ?.groupBy { it.resourceType },
+ ?.toReferencedResources(),
  revIncluded =
  revIncludedResources
  ?.filter {
  it.idOfBaseResourceOnWhichThisMatched ==
  "${baseResource.fhirType()}/${baseResource.logicalId}"
  }
- ?.map { it.resource }
- ?.groupBy { it.resourceType }
+ ?.toReferencedResources()
  )
  }
 }
@@ -133,7 +132,7 @@ private fun Search.getRevIncludeQuery(includeIds: List<String>): SearchQuery {
  return SearchQuery(
  query =
  """
- SELECT a.index_value, b.serializedResource 
+ SELECT a.index_name, a.index_value, b.serializedResource 
  FROM ReferenceIndexEntity a 
  JOIN ResourceEntity b
  ON a.resourceUuid = b.resourceUuid
@@ -144,6 +143,10 @@ private fun Search.getRevIncludeQuery(includeIds: List<String>): SearchQuery {
  )
 }
 
+private fun List<IndexedIdAndResource>.toReferencedResources() =
+ groupBy { it.resource.resourceType }
+ .mapValues { it.value.groupBy { it.matchingIndex }.mapValues { it.value.map { it.resource } } }
+
 private fun Search.getIncludeQuery(includeIds: List<String>): SearchQuery {
  var matchQuery = ""
  val args = mutableListOf<Any>(type.name)
@@ -187,7 +190,7 @@ private fun Search.getIncludeQuery(includeIds: List<String>): SearchQuery {
  return SearchQuery(
  query =
  """
- SELECT a.resourceId, c.serializedResource from ResourceEntity a 
+ SELECT b.index_name, a.resourceId, c.serializedResource from ResourceEntity a 
  JOIN ReferenceIndexEntity b 
  On a.resourceUuid = b.resourceUuid
  AND a.resourceType = ?

engine/src/main/java/com/google/android/fhir/search/NestedSearch.kt

@@ -41,7 +41,7 @@ internal data class NestedContext(val parentType: ResourceType, val param: IPara
  */
 inline fun <reified R : Resource> Search.has(
  referenceParam: ReferenceClientParam,
- init: @ISearchDsl ISearch.() -> Unit
+ init: @BaseSearchDsl BaseSearch.() -> Unit
 ) {
  nestedSearches.add(
  NestedSearch(referenceParam, Search(type = R::class.java.newInstance().resourceType)).apply {
@@ -71,7 +71,7 @@ inline fun <reified R : Resource> Search.has(
  */
 inline fun <reified R : Resource> Search.include(
  referenceParam: ReferenceClientParam,
- init: @ISearchDsl ISearch.() -> Unit
+ init: @BaseSearchDsl BaseSearch.() -> Unit = {}
 ) {
  forwardIncludes.add(
  NestedSearch(referenceParam, Search(type = R::class.java.newInstance().resourceType)).apply {
@@ -103,7 +103,7 @@ inline fun <reified R : Resource> Search.include(
 fun Search.include(
  resourceType: ResourceType,
  referenceParam: ReferenceClientParam,
- init: @ISearchDsl ISearch.() -> Unit = {}
+ init: @BaseSearchDsl BaseSearch.() -> Unit = {}
 ) {
  forwardIncludes.add(
  NestedSearch(referenceParam, Search(type = resourceType)).apply { search.init() }
@@ -133,7 +133,7 @@ fun Search.include(
  */
 inline fun <reified R : Resource> Search.revInclude(
  referenceParam: ReferenceClientParam,
- init: @ISearchDsl ISearch.() -> Unit = {}
+ init: @BaseSearchDsl BaseSearch.() -> Unit = {}
 ) {
 
  revIncludes.add(
@@ -167,7 +167,7 @@ inline fun <reified R : Resource> Search.revInclude(
 fun Search.revInclude(
  resourceType: ResourceType,
  referenceParam: ReferenceClientParam,
- init: @ISearchDsl ISearch.() -> Unit = {}
+ init: @BaseSearchDsl BaseSearch.() -> Unit = {}
 ) {
  revIncludes.add(NestedSearch(referenceParam, Search(type = resourceType)).apply { search.init() })
 }
@@ -189,7 +189,7 @@ fun Search.revInclude(
 fun Search.has(
  resourceType: ResourceType,
  referenceParam: ReferenceClientParam,
- init: @ISearchDsl ISearch.() -> Unit
+ init: @BaseSearchDsl BaseSearch.() -> Unit
 ) {
  nestedSearches.add(
  NestedSearch(referenceParam, Search(type = resourceType)).apply { search.init() }

engine/src/main/java/com/google/android/fhir/search/SearchDsl.kt

@@ -41,7 +41,11 @@ import com.google.android.fhir.search.filter.UriParamFilterCriterion
 import org.hl7.fhir.r4.model.ResourceType
 
 @SearchDslMarker
-class Search(val type: ResourceType, var count: Int? = null, var from: Int? = null) : ISearch {
+class Search(
+ val type: ResourceType,
+ override var count: Int? = null,
+ override var from: Int? = null
+) : BaseSearch {
  internal val stringFilterCriteria = mutableListOf<StringParamFilterCriteria>()
  internal val dateTimeFilterCriteria = mutableListOf<DateClientParamFilterCriteria>()
  internal val numberFilterCriteria = mutableListOf<NumberParamFilterCriteria>()
@@ -55,7 +59,7 @@ class Search(val type: ResourceType, var count: Int? = null, var from: Int? = nu
  @PublishedApi internal var revIncludes = mutableListOf<NestedSearch>()
  @PublishedApi internal var forwardIncludes = mutableListOf<NestedSearch>()
 
- var operation = Operation.AND
+ override var operation = Operation.AND
 
  override fun filter(
  stringParameter: StringClientParam,
@@ -143,10 +147,6 @@ class Search(val type: ResourceType, var count: Int? = null, var from: Int? = nu
  sort = parameter
  this.order = order
  }
-
- override fun operations(operation: Operation) {
- this.operation = operation
- }
 }
 
 enum class Order {