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

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

Closed
dubdabasoduba opened this issue Mar 18, 2024 · 16 comments · Fixed by #2518
Closed

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

dubdabasoduba opened this issue Mar 18, 2024 · 16 comments · Fixed by #2518
Assignees
Labels
effort:small Small effort - 2 days P1 High priority issue type:bug Something isn't working

Comments

@dubdabasoduba
Copy link
Collaborator

dubdabasoduba commented Mar 18, 2024

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
Copy link
Collaborator

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

@fredhersch fredhersch added the type:bug Something isn't working label Apr 1, 2024
@MJ1998
Copy link
Collaborator

MJ1998 commented Apr 16, 2024

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
Copy link

syedowaisali commented Apr 16, 2024

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
Copy link
Collaborator

pld commented Apr 17, 2024

hi @MJ1998 any update on this? thank you!

@MJ1998
Copy link
Collaborator

MJ1998 commented Apr 18, 2024

Which KnowledgeManager.install API are you using ?

@syedowaisali
Copy link

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

@MJ1998
Copy link
Collaborator

MJ1998 commented Apr 18, 2024

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

@syedowaisali
Copy link

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

@MJ1998
Copy link
Collaborator

MJ1998 commented Apr 18, 2024

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

@MJ1998
Copy link
Collaborator

MJ1998 commented Apr 24, 2024

@syedowaisali Did you try the above PR ?

@syedowaisali
Copy link

@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
Copy link
Collaborator

MJ1998 commented May 1, 2024

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

@syedowaisali
Copy link

  @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
Copy link
Collaborator

MJ1998 commented May 7, 2024

Are you saying you have a different issue in fhircore ?

@MJ1998 MJ1998 assigned MJ1998 and unassigned jingtang10 May 7, 2024
@syedowaisali
Copy link

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

@MJ1998 MJ1998 added P1 High priority issue effort:small Small effort - 2 days labels May 9, 2024
@MJ1998
Copy link
Collaborator

MJ1998 commented May 9, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort:small Small effort - 2 days P1 High priority issue type:bug Something isn't working
Projects
Status: Complete
Development

Successfully merging a pull request may close this issue.

7 participants