google / android-fhir

The Android FHIR SDK is a set of Kotlin libraries for building offline-capable, mobile-first healthcare applications using the HL7® FHIR® standard on Android.
https://google.github.io/android-fhir/
Apache License 2.0
458 stars 238 forks source link

Knowledge Manager uses the name instead of URL to fetch resources #2490

Closed dubdabasoduba closed 1 week ago

dubdabasoduba commented 2 months ago

Describe the bug @jingtang10 @MJ1998 @aditya-07 QQ, is my assumption that Knowledge Manager searches for resources using the URLs right? If yes, I think we have a bug where it is searching by name.

We have a situation where a Library and PlanDefinition have the same name. If the KM fetches the resource by the name it ends up with 2 results and always picks the first item.

https://github.com/WorldHealthOrganization/smart-immunizations/blob/cleanup/input/fsh/plandefinitions/IMMZD5DTMeasles.fsh and https://github.com/WorldHealthOrganization/smart-immunizations/blob/cleanup/input/cql/IMMZD5DTMeasles.cql will end up having the same "name"

owais-vd commented 1 month ago

Hi I've gone through the PlanDefinitionProcessor class, and the problem is coming only if we try to apply the nested plan-def. basically, we are trying to find the PlanDef from the repository with CanonicalType and URL, and the returning result is a Library instead of PlanDef because of the same name. I've attached two screenshots so you notice that the definition is for the PlanDef but the return result is Library also the second screenshot, the search-param has a URL for plan-def but the return result is a library.
Capture

Capture2

cc: @pld @ndegwamartin @MJ1998

MJ1998 commented 1 month ago

Hey Benjamin, while I am catching up with KM clear steps to reproduce this issue will be very helpful. It's easier to follow along the error and understand code better. Requesting to add steps to reproduce. Thanks. @dubdabasoduba

syedowaisali commented 1 month ago

It could be reproduced with any plan-def that has a nested plan-def if a nested plan-def URL ends with the name with any matching library resource name then the end result would be conflicted. Here is the following plan-def that I'm using and it's crashing with the class cast exception with all 3 nested plan-def. IMMZD2DTMeaslesDose0, IMMZD2DTMeaslesHighTx and IMMZD2DTMeaslesSupp

{
  "resourceType": "PlanDefinition",
  "id": "IMMZDTUmbrella",
  "meta": {
    "profile": [
      "http://hl7.org/fhir/uv/cpg/StructureDefinition/cpg-strategydefinition"
    ]
  },
  "url": "http://smart.who.int/ig/smart-immunizations/PlanDefinition/IMMZDTUmbrella",
  "title": "IMMZ.DT.Umbrella",
  "description": "If the child or patient has not been given MCV1 (at 9 months) and MCV2 (between 15-18 months) vaccination",
  "type": {
    "coding": [
      {
        "system": "http://terminology.hl7.org/CodeSystem/plan-definition-type",
        "code": "workflow-definition"
      }
    ]
  },
  "extension": [
    {
      "url": "http://hl7.org/fhir/uv/cpg/StructureDefinition/cpg-knowledgeCapability",
      "valueCode": "computable"
    }
  ],
  "version": "0.1.0",
  "name": "IMMZDTUmbrella",
  "status": "draft",
  "experimental": false,
  "publisher": "World Health Organization (WHO)",
  "action": [
    {
      "title": "Check Immunizations",
      "description": "Check immunization plan definitions to see what is required.",
      "code": [
        {
          "coding": [
            {
              "code": "dispense-medications",
              "system": "http://hl7.org/fhir/uv/cpg/CodeSystem/cpg-common-process-cs"
            }
          ]
        }
      ],
      "selectionBehavior": "all",
      "action": [
        {
          "title": "Immunize patient for Measles Dose 0",
          "description": "Provide measles immunization MCV0",
          "code": [
            {
              "coding": [
                {
                  "code": "dispense-medications",
                  "system": "http://hl7.org/fhir/uv/cpg/CodeSystem/cpg-common-process-cs"
                }
              ]
            }
          ],
          "selectionBehavior": "all",
          "definitionCanonical": "http://smart.who.int/ig/smart-immunizations/PlanDefinition/IMMZD2DTMeaslesDose0"
        },
        {
          "title": "Immunize patient for Measles",
          "description": "Provide measles immunization",
          "code": [
            {
              "coding": [
                {
                  "code": "dispense-medications",
                  "system": "http://hl7.org/fhir/uv/cpg/CodeSystem/cpg-common-process-cs"
                }
              ]
            }
          ],
          "selectionBehavior": "all",
          "definitionCanonical": "http://smart.who.int/ig/smart-immunizations/PlanDefinition/IMMZD2DTMeaslesHighTx"
        },
        {
          "title": "Immunize patient for Measles supplementary dose",
          "description": "Provide measles supplementary dose immunization",
          "code": [
            {
              "coding": [
                {
                  "code": "dispense-medications",
                  "system": "http://hl7.org/fhir/uv/cpg/CodeSystem/cpg-common-process-cs"
                }
              ]
            }
          ],
          "selectionBehavior": "all",
          "definitionCanonical": "http://smart.who.int/ig/smart-immunizations/PlanDefinition/IMMZD2DTMeaslesSupp"
        }
      ]
    }
  ]
}

cc: @MJ1998 @dubdabasoduba

pld commented 1 month ago

hi @MJ1998 any update on this? thank you!

MJ1998 commented 1 month ago

Which KnowledgeManager.install API are you using ?

syedowaisali commented 1 month ago

using 0.1.0-alpha03-preview2-SNAPSHOT. cc: @dubdabasoduba

MJ1998 commented 1 month ago

No I mean which install API? There are 3 install APIs in KnowledgeManager class to add resources to knowledge database.

syedowaisali commented 1 month ago

https://github.com/google/android-fhir/blob/6ff0643c68cfd141252580cf433b9d02ad10b581/knowledge/src/main/java/com/google/android/fhir/knowledge/KnowledgeManager.kt#L117

but i think we don't have an issue with install because we have all the resources in KM db

MJ1998 commented 1 month ago

Ok I think I got the issue without replicating. Can you try above linked Pr to see if the issue is resolved ?

MJ1998 commented 3 weeks ago

@syedowaisali Did you try the above PR ?

syedowaisali commented 3 weeks ago

@MJ1998 I tested it with a test you on your branch and the resources are loading as expected but the test was also working fine without the change that we have in this PR.

I think that's not a problem but I'm not sure at this moment and I'm also trying to find out the root cause of this bug. cc: @dubdabasoduba

MJ1998 commented 2 weeks ago

Can you share your test ? I might be able to help point out.. @syedowaisali

syedowaisali commented 2 weeks ago
  @Test
  fun `check nested plan-def are readable`() = runTest {
    val npmPackage = FhirNpmPackage("measles", "0.1.0")
    val root = File(javaClass.getResource("/measles")!!.file)
    knowledgeManager.install(npmPackage, root)

    assertThat(knowledgeManager.loadResources(resourceType = "Library", name = "IMMZD2DTMeaslesDose0")).isNotNull()
    assertThat(knowledgeManager.loadResources(resourceType = "Library", name = "IMMZD2DTMeaslesHighTx")).isNotNull()
    assertThat(knowledgeManager.loadResources(resourceType = "Library", name = "IMMZD2DTMeaslesSupp")).isNotNull()
    assertThat(knowledgeManager.loadResources(resourceType = "PlanDefinition", url = "http://smart.who.int/ig/smart-immunizations/PlanDefinition/IMMZD2DTMeaslesDose0")).isNotNull()
    assertThat(knowledgeManager.loadResources(resourceType = "PlanDefinition", url = "http://smart.who.int/ig/smart-immunizations/PlanDefinition/IMMZD2DTMeaslesHighTx")).isNotNull()
    assertThat(knowledgeManager.loadResources(resourceType = "PlanDefinition", url = "http://smart.who.int/ig/smart-immunizations/PlanDefinition/IMMZD2DTMeaslesSupp")).isNotNull()
    assertThat(knowledgeManager.loadResources(resourceType = "PlanDefinition",name = "IMMZDTUmbrella",)).isNotNull()
  }
  @Test
  fun searchByUrl() = runBlockingOnWorkerThread {

    loadFile("/measles/Library-IMMZD2DTMeaslesDose0.json", ::importToFhirEngine)
    loadFile("/measles/PlanDefinition-IMMZD2DTMeaslesDose0.json", ::importToFhirEngine)

    val searchParams = Searches.byUrl("http://smart.who.int/ig/smart-immunizations/PlanDefinition/IMMZD2DTMeaslesDose0")
  val repo = FhirEngineRepository(fhirContext, fhirEngine)
    assertThat(
            repo.search(Bundle::class.java, PlanDefinition::class.java, searchParams)?.entryFirstRep?.resource
    ).isInstanceOf(PlanDefinition::class.java)
  }

@MJ1998 Both tests are working fine but I think we have a different issue that needs to be investigated I"m unable to debug on because the breakpoint gets stuck. cc: @dubdabasoduba

MJ1998 commented 1 week ago

Are you saying you have a different issue in fhircore ?

syedowaisali commented 1 week ago

maybe but until we completely debug the code on our side. cc: @dubdabasoduba

MJ1998 commented 1 week ago

I think I know what might be causing an error. There is another bug in the KM where loading by Url "AND Version" causes problem.

Try the current PR.