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
492 stars 293 forks source link

Canonical Types are not being handled yet in fhir sdk #694

Closed MuhammadSalman-7214 closed 3 years ago

MuhammadSalman-7214 commented 3 years ago

Describe the bug Any resource containing canonical references is not storing in the database. Reason is Class ResourceIndexer line 249 where it tries to cast canonical type into reference entity, which is throwing class cast exception. So far resources checked for this problem are Activity Definition, Observation, Library and Plan Definition. Standard url for hl7 is https://www.hl7.org/fhir/plandefinition-example.json.html, (See Library Property)

Component Syncing

To Reproduce In attached screenshot Library is valid url as per the documentation, but it will fail to store this resource in the database during syncing when it will try to convert this canonical url into reference entity type, similarly it can be seen for any such url.

Expected behavior Canonical type should remain as canonical type instead of casting as reference entity

Screenshots Screenshot Added Screenshot from 2021-07-29 15-00-38

Smartphone (please complete the following information):

Additional context Add any other context about the problem here.

Would you like to work on the issue? No Screenshot from 2021-07-29 14-43-34

jingtang10 commented 3 years ago

thanks for creating the issue!

MuhammadSalman-7214 commented 3 years ago

your welcome @jingtang10

maimoonak commented 3 years ago

The FHIR sdk treats Canonical and Reference in same way and the StringParam could be Reference or Canonical type. We need to resolve the type explicitly later. The type is resolved and returned by org.hl7.fhir.r4.utils.FhirPathEngine

Screenshot from 2021-08-24 14-52-18

maimoonak commented 3 years ago

For Library the issue is on depends-on (relatedArtifact.resource) property which is type canonical but mapped as reference in Library class.

@SearchParamDefinition(name="depends-on", path="Library.relatedArtifact.where(type='depends-on').resource", 
description="What resource is being referenced", type="reference" )
  public static final String SP_DEPENDS_ON = "depends-on";
  public static final ca.uhn.fhir.rest.gclient.ReferenceClientParam DEPENDS_ON = 
new ca.uhn.fhir.rest.gclient.ReferenceClientParam(SP_DEPENDS_ON);

Canonical Types can be a reference to other Resource OR anything else, however, I am still trying to understand difference between Canonical and Reference where both can have have reference to Relative/Absolute/Contained path/url

Changing code to

    return value.let {
      when (value) {
        is Reference -> value.reference
        is CanonicalType -> value.asStringValue()
        else -> throw UnsupportedOperationException("Value $value is not readable by SDK")
      }
    }?.let { ReferenceIndex(searchParam.name, searchParam.path, it) }

resolved the issue for Library however, the problem for Observation looks different (under progress)

maimoonak commented 3 years ago

ActivityDefinition works fine with above changes

PlanDefinition crashes as the action.definition can have multiple types canonical/uri . which leads to crash as it is mapped on reference

  @SearchParamDefinition(name="definition", path="PlanDefinition.action.definition", description="Activity or plan definitions used by plan definition", 
type="reference", target={ActivityDefinition.class, PlanDefinition.class, Questionnaire.class } )

Adding condition for UriType sorts out the issue

    return value.let {
      when (value) {
        is Reference -> value.reference
        is CanonicalType -> value.asStringValue()
        is UriType -> value.value
        else -> throw UnsupportedOperationException("Value $value is not readable by SDK")
      }
    }?.let { ReferenceIndex(searchParam.name, searchParam.path, it) }
maimoonak commented 3 years ago

The Observation on HapiServer crashes with UnsupportedOperation when observation is of type quantity. It somehow tries ti interpret the quantity and reaches to code is not supported yet, so this is probably an issue from HapiLibrary

Observation:

{
 "fullUrl": "http://hapi.fhir.org/baseR4/Observation/64301",
 "resource": {
   "resourceType": "Observation",
.................
   "code": {
     "coding": [ {
       "system": "http://loinc.org",
       "code": "33914-3",
...............
   },
...........,
   "valueQuantity": {
     "value": 88,
      "unit": "ml/mn/1,73m2",
      "system": "http://unitsofmeasure.org"
    },
    "interpretation": [ {
      "coding": [ {
        "system": "http://terminology.hl7.org/CodeSystem/v3-ObservationInterpretation",
        "code": "N",
        "display": "Normal"
      } ]
    } ]
  },
.........
}

HAPI FHIR Code

FhirPathEngine
  private boolean qtyEqual(Quantity left, Quantity right) {
    if (worker.getUcumService() != null) {
      DecimalType dl = qtyToCanonical(left);
      DecimalType dr = qtyToCanonical(right);
      i............................
  }
HapiWorkerContext
  @Override
  public UcumService getUcumService() {
    throw new UnsupportedOperationException();
  }
maimoonak commented 3 years ago

During syncing Questionnaire with HAPI I also encountered OutOfMemory issue. questionnaire_outofmemory_exception.txt

Initially doubt was that its due to huge number of questionnaires (1543) but from logs we can see that it is paginated with page size of 20. So it seems that it occurs somewhere around http://hapi.fhir.org/baseR4?_getpages=e3f46ec4-94ea-4437-aa9c-7a65ef2d5c21&_getpagesoffset=720&_count=20&_pretty=true&_bundletype=searchset

There could be 2 possible reasons

This does NOT occur on ONA server

maimoonak commented 3 years ago

The handled reference type works for following resources on ONA-server Patient, RelatedPerson, Observation, RiskAssessment, Encounter, Immunization, Flag, StructureMap, Library, Questionnaire, QuestionnaireResponse, Location, Organization, Practitioner, PlanDefinition,

Observation crashes on quantity-type

maimoonak commented 3 years ago

Practitioner: on HAPI

Failure(java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String org.hl7.fhir.r4.model.Enumerations$AdministrativeGender.getSystem()' on a null object reference)

around 2021-08-26 16:15:59.116 I/okhttp.OkHttpClient: <-- 200 OK http://hapi.fhir.org/baseR4?_getpages=9a543b7e-117f-4981-8824-575fb0f9b26d&_getpagesoffset=1280&_count=20&_pretty=true&_bundletype=searchset (298ms, unknown-length body)

maimoonak commented 3 years ago

The PR adds the handling of Canonical / Uri type on 'reference' search params https://github.com/google/android-fhir/pull/757.

Issues resolved

Other relevant issues found

jingtang10 commented 3 years ago

The Observation on HapiServer crashes with UnsupportedOperation when observation is of type quantity. It somehow tries ti interpret the quantity and reaches to code is not supported yet, so this is probably an issue from HapiLibrary

Observation:

{
 "fullUrl": "http://hapi.fhir.org/baseR4/Observation/64301",
 "resource": {
   "resourceType": "Observation",
.................
   "code": {
     "coding": [ {
       "system": "http://loinc.org",
       "code": "33914-3",
...............
   },
...........,
   "valueQuantity": {
     "value": 88,
      "unit": "ml/mn/1,73m2",
      "system": "http://unitsofmeasure.org"
    },
    "interpretation": [ {
      "coding": [ {
        "system": "http://terminology.hl7.org/CodeSystem/v3-ObservationInterpretation",
        "code": "N",
        "display": "Normal"
      } ]
    } ]
  },
.........
}

HAPI FHIR Code

FhirPathEngine
  private boolean qtyEqual(Quantity left, Quantity right) {
    if (worker.getUcumService() != null) {
      DecimalType dl = qtyToCanonical(left);
      DecimalType dr = qtyToCanonical(right);
      i............................
  }
HapiWorkerContext
  @Override
  public UcumService getUcumService() {
    throw new UnsupportedOperationException();
  }

thanks @maimoonak can you create a separate issue with stack trace for this one?

EDIT: it would be great to have a minimal observation and code snippet to reproduce this issue -- and we probably should file a bug on hapi's repo directly.

jingtang10 commented 3 years ago

During syncing Questionnaire with HAPI I also encountered OutOfMemory issue. questionnaire_outofmemory_exception.txt

Initially doubt was that its due to huge number of questionnaires (1543) but from logs we can see that it is paginated with page size of 20. So it seems that it occurs somewhere around http://hapi.fhir.org/baseR4?_getpages=e3f46ec4-94ea-4437-aa9c-7a65ef2d5c21&_getpagesoffset=720&_count=20&_pretty=true&_bundletype=searchset

There could be 2 possible reasons

  • Any Questionnaire around this page size is huge
  • The GC is unable to kill existing objects (sync, insert, indexing occurs in parallel asynchronously) and at the end it eats up heapspace

This does NOT occur on ONA server

and this?

jingtang10 commented 3 years ago

Thanks @maimoonak for sending the fix. It's the right approach. I just want to clarify one thing: the search types are not necessarily mapped 1-to-1 with the actual types of the fields you're searching. For example, a token search can be on a code, but can also be on a uri, or even a boolean field (see this link https://www.hl7.org/fhir/search.html#token). This distinction is the root of the problem - we weren't handling different value types correctly under the reference type search parameters.

so yeah given this i believe your fix is the right approach.

maimoonak commented 3 years ago