Add _tag and _profile search parameters for x-fhir-query (#1938)

* include _tag and _profile as search parameters in x-fhir-query. * include tests for _tag and _profile search parameter translation. * Added _tag and _profile as Constants. * Added tests for x-fhir-query search using _tag and profile params. * Modified _tag and _profile tests. * Use constants for _tag and _profile strings. * Modified tests. * Fix spelling * add standard search parameters to all resources. * include standard search parameters in SearchParameterRepositoryGeneratedTest * remove manual search parameter definition for _tag, _profile from XFhirQueryTranslator. * use generalized names for Tags and Patient. * change to uriFilterCriteria for _profile test. * fix NullPointerException on ResourceIndexer * Changed search parameter type from uri as defined in the current search-parameters.json file to reference as defined in fhir spec * Remove addIndexesFromResourceClass * Code refactor * Code refactor * Fix failing test. * Fix gettting id value for case when idPart is null but IdType has value * Code refactor: Create a separate function for common search parameters defined for all resources. * Update documentation for generating SearchParameterRepository_Generated.kt and SearchParameterRepositoryTestHelper_Generated.kt using codegen. * Changed search parameter type for _profile into Uri type as defined in R4 * Fix tests that uses reference type instead uri for _profile search parameter --------- Co-authored-by: Jing Tang <[email protected]>
google · Jul 11, 2023 · cfd88dc · cfd88dc
1 parent 9cbc1f4
commit cfd88dc
Show file tree

Hide file tree

Showing 6 changed files with 230 additions and 98 deletions.
diff --git a/buildSrc/src/main/kotlin/codegen/SearchParameterRepositoryGenerator.kt b/buildSrc/src/main/kotlin/codegen/SearchParameterRepositoryGenerator.kt
@@ -21,6 +21,7 @@ import com.squareup.kotlinpoet.CodeBlock
 import com.squareup.kotlinpoet.FileSpec
 import com.squareup.kotlinpoet.FunSpec
 import com.squareup.kotlinpoet.KModifier
+import com.squareup.kotlinpoet.ParameterSpec
 import com.squareup.kotlinpoet.ParameterizedTypeName.Companion.parameterizedBy
 import java.io.File
 import java.util.Locale
@@ -41,8 +42,10 @@ import org.hl7.fhir.r4.model.SearchParameter
  * should be regenerated to reflect any change.
  *
  * To do this, replace the content of the file `codegen/src/main/res/search-parameters.json` with
- * the content at `http://www.hl7.org/fhir/search-parameters.json` and run the `main` function in
- * the `codegen` module.
+ * the content at `http://www.hl7.org/fhir/search-parameters.json` and execute the gradle task
+ * `generateSearchParamsTask`. If you are using Android Studio, you can usually find this task in
+ * the Gradle tasks under other. Alternatively, you clean and rebuild the project to ensure changes
+ * take effect.
  */
 internal data class SearchParamDefinition(
   val className: ClassName,
@@ -63,6 +66,7 @@ internal object SearchParameterRepositoryGenerator {
 
   private val searchParamMap: HashMap<String, MutableList<SearchParamDefinition>> = HashMap()
   private val searchParamDefinitionClass = ClassName(indexPackage, "SearchParamDefinition")
+  private val baseResourceSearchParameters = mutableListOf<SearchParamDefinition>()
 
   fun generate(bundle: Bundle, outputPath: File, testOutputPath: File) {
     for (entry in bundle.entry) {
@@ -81,6 +85,15 @@ internal object SearchParameterRepositoryGenerator {
               path = path.value
             )
           )
+        if (hashMapKey == "Resource")
+          baseResourceSearchParameters.add(
+            SearchParamDefinition(
+              className = searchParamDefinitionClass,
+              name = searchParameter.name,
+              paramTypeCode = searchParameter.type.toCode().toUpperCase(Locale.US),
+              path = path.value
+            )
+          )
       }
     }
 
@@ -94,8 +107,33 @@ internal object SearchParameterRepositoryGenerator {
         )
         .addModifiers(KModifier.INTERNAL)
         .addKdoc(generatedComment)
-        .beginControlFlow("return when (resource.fhirType())")
-
+        .beginControlFlow("val resourceSearchParams = when (resource.fhirType())")
+
+    // Function for base resource search parameters
+    val baseParamResourceSpecName = ParameterSpec.builder("resourceName", String::class).build()
+    val getBaseResourceSearchParamListFunction =
+      FunSpec.builder("getBaseResourceSearchParamsList")
+        .addParameter(baseParamResourceSpecName)
+        .apply {
+          addModifiers(KModifier.PRIVATE)
+          returns(
+            ClassName("kotlin.collections", "List").parameterizedBy(searchParamDefinitionClass)
+          )
+          beginControlFlow("return buildList(capacity = %L)", baseResourceSearchParameters.size)
+          baseResourceSearchParameters.forEach { definition ->
+            addStatement(
+              "add(%T(%S, %T.%L, %P))",
+              definition.className,
+              definition.name,
+              Enumerations.SearchParamType::class,
+              definition.paramTypeCode,
+              "$" + "${baseParamResourceSpecName.name}." + definition.path.substringAfter(".")
+            )
+          }
+          endControlFlow() // end buildList
+        }
+        .build()
+    fileSpec.addFunction(getBaseResourceSearchParamListFunction)
     // Helper function used in SearchParameterRepositoryGeneratedTest
     val testHelperFunctionCodeBlock =
       CodeBlock.builder().addStatement("val resourceList = listOf<%T>(", Resource::class.java)
@@ -140,6 +178,11 @@ internal object SearchParameterRepositoryGenerator {
     }
 
     getSearchParamListFunction.addStatement("else -> emptyList()").endControlFlow()
+    // This will now return the list of search parameter for the resource + search parameters
+    // defined in base resource i.e. _profile, _tag, _id, _security, _lastUpdated, _source
+    getSearchParamListFunction.addStatement(
+      "return resourceSearchParams + getBaseResourceSearchParamsList(resource.fhirType())"
+    )
     fileSpec.addFunction(getSearchParamListFunction.build()).build().writeTo(outputPath)
 
     testHelperFunctionCodeBlock.add(")\n")
@@ -175,8 +218,7 @@ internal object SearchParameterRepositoryGenerator {
     return if (searchParam.base.size == 1) {
       mapOf(searchParam.base.single().valueAsString to searchParam.expression)
     } else {
-      searchParam
-        .expression
+      searchParam.expression
         .split("|")
         .groupBy { splitString -> splitString.split(".").first().trim().removePrefix("(") }
         .mapValues { it.value.joinToString(" | ") { join -> join.trim() } }

diff --git a/engine/src/main/java/com/google/android/fhir/index/ResourceIndexer.kt b/engine/src/main/java/com/google/android/fhir/index/ResourceIndexer.kt
@@ -43,17 +43,16 @@ import org.hl7.fhir.r4.model.DecimalType
 import org.hl7.fhir.r4.model.Enumerations.SearchParamType
 import org.hl7.fhir.r4.model.HumanName
 import org.hl7.fhir.r4.model.ICoding
+import org.hl7.fhir.r4.model.IdType
 import org.hl7.fhir.r4.model.Identifier
 import org.hl7.fhir.r4.model.InstantType
 import org.hl7.fhir.r4.model.IntegerType
 import org.hl7.fhir.r4.model.Location
 import org.hl7.fhir.r4.model.Money
-import org.hl7.fhir.r4.model.Patient
 import org.hl7.fhir.r4.model.Period
 import org.hl7.fhir.r4.model.Quantity
 import org.hl7.fhir.r4.model.Reference
 import org.hl7.fhir.r4.model.Resource
-import org.hl7.fhir.r4.model.SearchParameter
 import org.hl7.fhir.r4.model.StringType
 import org.hl7.fhir.r4.model.Timing
 import org.hl7.fhir.r4.model.UriType
@@ -104,73 +103,9 @@ internal class ResourceIndexer(
         }
       }
 
-    addIndexesFromResourceClass(resource, indexBuilder)
     return indexBuilder.build()
   }
 
-  /**
-   * Manually add indexes for [SearchParameter]s defined in [Resource] class. This is because:
-   * 1. There is no clear way defined in the search parameter definitions to figure out the class
-   * hierarchy of the model classes in codegen.
-   * 2. Common [SearchParameter]'s paths are defined for [Resource] class e.g even for the [Patient]
-   * model, the [SearchParameter] expression for id would be `Resource.id` and
-   * [FHIRPathEngine.evaluate] doesn't return anything when [Patient] is passed to the function.
-   */
-  private fun <R : Resource> addIndexesFromResourceClass(
-    resource: R,
-    indexBuilder: ResourceIndices.Builder
-  ) {
-    indexBuilder.addTokenIndex(
-      TokenIndex(
-        "_id",
-        arrayOf(resource.fhirType(), "id").joinToString(separator = "."),
-        null,
-        resource.logicalId
-      )
-    )
-    // Add 'lastUpdated' index to all resources.
-    if (resource.meta.hasLastUpdated()) {
-      val lastUpdatedElement = resource.meta.lastUpdatedElement
-      indexBuilder.addDateTimeIndex(
-        DateTimeIndex(
-          name = "_lastUpdated",
-          path = arrayOf(resource.fhirType(), "meta", "lastUpdated").joinToString(separator = "."),
-          from = lastUpdatedElement.value.time,
-          to = lastUpdatedElement.value.time
-        )
-      )
-    }
-
-    if (resource.meta.hasProfile()) {
-      resource.meta.profile
-        .filter { it.value != null && it.value.isNotEmpty() }
-        .forEach {
-          indexBuilder.addReferenceIndex(
-            ReferenceIndex(
-              "_profile",
-              arrayOf(resource.fhirType(), "meta", "profile").joinToString(separator = "."),
-              it.value
-            )
-          )
-        }
-    }
-
-    if (resource.meta.hasTag()) {
-      resource.meta.tag
-        .filter { it.code != null && it.code!!.isNotEmpty() }
-        .forEach {
-          indexBuilder.addTokenIndex(
-            TokenIndex(
-              "_tag",
-              arrayOf(resource.fhirType(), "meta", "tag").joinToString(separator = "."),
-              it.system ?: "",
-              it.code
-            )
-          )
-        }
-    }
-  }
-
   private fun numberIndex(searchParam: SearchParamDefinition, value: Base): NumberIndex? =
     when (value.fhirType()) {
       "integer" ->
@@ -319,18 +254,34 @@ internal class ResourceIndexer(
       "code",
       "Coding" -> {
         val coding = value as ICoding
-        listOf(TokenIndex(searchParam.name, searchParam.path, coding.system ?: "", coding.code))
+        if (coding.code != null) {
+          listOf(TokenIndex(searchParam.name, searchParam.path, coding.system ?: "", coding.code))
+        } else {
+          listOf()
+        }
+      }
+      "id" -> {
+        val id = value as IdType
+        if (id.value != null) {
+          listOf(TokenIndex(searchParam.name, searchParam.path, null, id.idPart ?: id.value))
+        } else {
+          listOf()
+        }
       }
       else -> listOf()
     }
 
   private fun referenceIndex(searchParam: SearchParamDefinition, value: Base): ReferenceIndex? {
-    return when (value) {
-      is Reference -> value.reference
-      is CanonicalType -> value.value
-      is UriType -> value.value
-      else -> throw UnsupportedOperationException("Value $value is not readable by SDK")
-    }?.let { ReferenceIndex(searchParam.name, searchParam.path, it) }
+    return if (!value.isEmpty) {
+      when (value) {
+        is Reference -> value.reference
+        is CanonicalType -> value.value
+        is UriType -> value.value
+        else -> throw UnsupportedOperationException("Value $value is not readable by SDK")
+      }?.let { ReferenceIndex(searchParam.name, searchParam.path, it) }
+    } else {
+      null
+    }
   }
 
   private fun quantityIndex(searchParam: SearchParamDefinition, value: Base): List<QuantityIndex> =

diff --git a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt
@@ -38,6 +38,8 @@ import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.runBlocking
 import org.hl7.fhir.exceptions.FHIRException
 import org.hl7.fhir.r4.model.Address
+import org.hl7.fhir.r4.model.CanonicalType
+import org.hl7.fhir.r4.model.Coding
 import org.hl7.fhir.r4.model.Enumerations
 import org.hl7.fhir.r4.model.HumanName
 import org.hl7.fhir.r4.model.Meta
@@ -242,6 +244,67 @@ class FhirEngineImplTest {
     assertThat(exception.message).isEqualTo("customParam not found in Patient")
   }
 
+  @Test
+  fun `search() by x-fhir-query should return patients for _tag param`() = runBlocking {
+    val patients =
+      listOf(
+        buildPatient("1", "Patient1", Enumerations.AdministrativeGender.FEMALE).apply {
+          meta = Meta().setTag(mutableListOf(Coding("https://d-tree.org/", "Tag1", "Tag 1")))
+        },
+        buildPatient("2", "Patient2", Enumerations.AdministrativeGender.FEMALE).apply {
+          meta = Meta().setTag(mutableListOf(Coding("http://d-tree.org/", "Tag2", "Tag 2")))
+        }
+      )
+
+    fhirEngine.create(*patients.toTypedArray())
+
+    val result = fhirEngine.search("Patient?_tag=Tag1").map { it as Patient }
+
+    assertThat(result.size).isEqualTo(1)
+    assertThat(result.all { patient -> patient.meta.tag.all { it.code == "Tag1" } }).isTrue()
+  }
+
+  @Test
+  fun `search() by x-fhir-query should return patients for _profile param`() = runBlocking {
+    val patients =
+      listOf(
+        buildPatient("3", "C", Enumerations.AdministrativeGender.FEMALE).apply {
+          meta =
+            Meta()
+              .setProfile(
+                mutableListOf(
+                  CanonicalType(
+                    "http://fhir.org/STU3/StructureDefinition/Example-Patient-Profile-1"
+                  )
+                )
+              )
+        },
+        buildPatient("4", "C", Enumerations.AdministrativeGender.FEMALE).apply {
+          meta =
+            Meta().setProfile(mutableListOf(CanonicalType("http://d-tree.org/Diabetes-Patient")))
+        }
+      )
+
+    fhirEngine.create(*patients.toTypedArray())
+
+    val result =
+      fhirEngine
+        .search(
+          "Patient?_profile=http://fhir.org/STU3/StructureDefinition/Example-Patient-Profile-1"
+        )
+        .map { it as Patient }
+
+    assertThat(result.size).isEqualTo(1)
+    assertThat(
+        result.all { patient ->
+          patient.meta.profile.all {
+            it.value.equals("http://fhir.org/STU3/StructureDefinition/Example-Patient-Profile-1")
+          }
+        }
+      )
+      .isTrue()
+  }
+
   @Test
   fun syncUpload_uploadLocalChange() = runBlocking {
     val localChanges = mutableListOf<LocalChange>()

diff --git a/engine/src/test/java/com/google/android/fhir/index/ResourceIndexerTest.kt b/engine/src/test/java/com/google/android/fhir/index/ResourceIndexerTest.kt
@@ -116,8 +116,8 @@ class ResourceIndexerTest {
         meta = Meta().setProfile(mutableListOf(CanonicalType("Profile/lipid")))
       }
     val resourceIndices = resourceIndexer.index(patient)
-    assertThat(resourceIndices.referenceIndices)
-      .contains(ReferenceIndex("_profile", "Patient.meta.profile", "Profile/lipid"))
+    assertThat(resourceIndices.uriIndices)
+      .contains(UriIndex("_profile", "Patient.meta.profile", "Profile/lipid"))
   }
 
   @Test