Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

inferno appears to be stripping dataAbsentReason fields (FI-1520) #67

Closed
nathanloyer opened this issue Apr 5, 2022 · 32 comments
Closed
Labels
remove constraint The change should not cause a system that previously passed to now fail. v2.1.0 Release v2.1.0

Comments

@nathanloyer
Copy link
Contributor

There is more detail on this issue here, but I will summarize again below.

inferno-framework/inferno-core#154

Our server is failing tests for FHIR Resource Validation of certain Device resources. Here is one example

{
  "extension": [
    {
      "valueReference": {
        "reference": "Organization/234234"
      },
      "url": "https://fhir.athena.io/StructureDefinition/ah-practice"
    },
    {
      "valueReference": {
        "reference": "Organization/234234324"
      },
      "url": "https://fhir.athena.io/StructureDefinition/ah-chart-sharing-group"
    }
  ],
  "resourceType": "Device",
  "_serialNumber": {
    "extension": [
      {
        "url": "http: //hl7.org/fhir/StructureDefinition/data-absent-reason",
        "valueCode": "unknown"
      }
    ]
  },
  "status": "entered-in-error",
  "meta": {
    "profile": [
      "http://hl7.org/fhir/us/core/StructureDefinition/us-core-implantable-device"
    ]
  },
  "_distinctIdentifier": {
    "extension": [
      {
        "url": "http: //hl7.org/fhir/StructureDefinition/data-absent-reason",
        "valueCode": "unknown"
      }
    ]
  },
  "_expirationDate": {
    "extension": [
      {
        "url": "http://hl7.org/fhir/StructureDefinition/data-absent-reason",
        "valueCode": "unknown"
      }
    ]
  },
  "_lotNumber": {
    "extension": [
      {
        "url": "http: //hl7.org/fhir/StructureDefinition/data-absent-reason",
        "valueCode": "unknown"
      }
    ]
  },
  "patient": {
    "reference": "Patient/6757"
  },
  "_manufactureDate": {
    "extension": [
      {
        "url": "http://hl7.org/fhir/StructureDefinition/data-absent-reason",
        "valueCode": "unknown"
      }
    ]
  },
  "id": "9879",
  "type": {
    "text": "Peripheral vascular guidewire, manual"
  },
  "udiCarrier": [
    {
      "deviceIdentifier": "00827002005112",
      "carrierHRF": "(01)00827002005112",
      "jurisdiction": "http://hl7.org/fhir/NamingSystem/fda-udi"
    }
  ]
}

You will see that we have a lot of data absent reasons, because in this case we do not have the data in our database necessary to populate these fields.

We have just confirmed that the request that Inferno is sending to the FHIR Validator is removing the data absent reasons, which is causing us to fail validation. Here is an example for the above data:

{
  "id": "9879",
  "meta": {
    "profile": [
      "http://hl7.org/fhir/us/core/StructureDefinition/us-core-implantable-device|3.1.1"
    ]
  },
  "extension": [
    {
      "url": "https://fhir.athena.io/StructureDefinition/ah-practice",
      "valueReference": {
        "reference": "Organization/234234"
      }
    },
    {
      "url": "https://fhir.athena.io/StructureDefinition/ah-chart-sharing-group",
      "valueReference": {
        "reference": "Organization/234234324"
      }
    }
  ],
  "udiCarrier": [
    {
      "deviceIdentifier": "00827002008776",
      "jurisdiction": "http://hl7.org/fhir/NamingSystem/fda-udi",
      "carrierHRF": "(01)00827002008776"
    }
  ],
  "status": "entered-in-error",
  "type": {
    "text": "Peripheral vascular guidewire, manual"
  },
  "patient": {
    "reference": "Patient/6757"
  },
  "resourceType": "Device"
}
@nathanloyer
Copy link
Contributor Author

The error message we get in the test is

us-core-9: 'For implantable medical devices that have UDI information, at least one of the Production Identifiers (UDI-PI) SHALL be present.' Rule 'For implantable medical devices that have UDI information, at least one of the Production Identifiers (UDI-PI) SHALL be present.' Failed

@Jammjammjamm
Copy link
Contributor

This is related to an issue with our FHIR models library not supporting primitive extensions. We have a workaround, but we're not using it in this location, so I'll be addressing this ASAP.

@dwebe003
Copy link

dwebe003 commented Apr 5, 2022

@Jammjammjamm I actually came across a minor issue related to Implantable Devices as well.

Much of our current Device data lacks some information, namely serialNumber, lotNumber, manufactureDate, and expirationDate.
For serialNumber and lotNumber, I am able to simply pass in "Unknown" if that data is not found. However, for manufactureDate and expirationDate I am unable to do this because the setManufactureDate() and setExpirationDate() java methods only accept a Date object.

Passing in null makes it so manufactureDate and expirationDate are omitted from the FHIR response-- but this is bad because Inferno expects these fields. In the cases of not having these dates, we decided to pass in new Date(0), which sets them to "the beginning of time" (aka January 1970). But we fear that doing this could be misinterpreted.

Do you have any suggestions for dealing with this?

@nathanloyer
Copy link
Contributor Author

nathanloyer commented Apr 5, 2022

@dwebe003 This doesn't appear to be directly related to the problem this issue is discussing. I suggest you move this to chat.fhir.org or create another issue.

Since you're using Java, I'm assuming you are using HAPI? I'm sure they have a way to set the data absent reasons in the data models, which is what we did in the data in the issue description.

"_serialNumber": {
    "extension": [
      {
        "url": "http: //hl7.org/fhir/StructureDefinition/data-absent-reason",
        "valueCode": "unknown"
      }
    ]
  },

Our server is written in Perl, so we're not using HAPI here, but I'm sure they have a way to set the data like this. This is what you should be doing instead of setting the strings to unknown or 1970

@nathanloyer
Copy link
Contributor Author

I was just playing around with hapi...


import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.parser.IParser;
import java.util.ArrayList;
import org.hl7.fhir.r4.model.CodeType;
import org.hl7.fhir.r4.model.DateTimeType;
import org.hl7.fhir.r4.model.Device;
import org.hl7.fhir.r4.model.Extension;

public class TestScript {
  public static void main(String[] args) {
    final FhirContext ctx = FhirContext.forR4();
    final IParser parser = ctx.newJsonParser();

    Device device = new Device();

    ArrayList<Extension> dar = new ArrayList<>();
    Extension unknown = new Extension()
        .setUrl("http: //hl7.org/fhir/StructureDefinition/data-absent-reason")
        .setValue(new CodeType("unknown"));
    dar.add(unknown);
    DateTimeType dt = new DateTimeType();
    dt.setExtension(dar);
    device.setManufactureDateElement(dt);
    System.out.println(parser.encodeResourceToString(device));
  }
}
{"resourceType":"Device","_manufactureDate":{"extension":[{"url":"http: //hl7.org/fhir/StructureDefinition/data-absent-reason","valueCode":"unknown"}]}}

@nathanloyer
Copy link
Contributor Author

back on the original issue, @Jammjammjamm I have found that there was another error we are getting in our tests for DocumentReference that looks to me to be the same root cause

image

The second one for us-core-6. When I ran our resource directly against the fhir validator, the second error message went away. We still had the error for the first error about the contentType. So I just want to make sure the proposed fix would address the issue for all resources and not just Device. I also want to note that we see the same errors reported when we run the single patient tests, so this is not only an issue when running the bulk tests.

@dwebe003
Copy link

dwebe003 commented Apr 5, 2022

@nathanloyer Sorry about hijacking this issue! I mistakenly thought your issue was resolved, and thought mine was too similar to make a new thread. Apologies.

But thank you for the idea! I coded some logic to use setManufactureDateElement/setExpirationDateElement, in the way you provided, when no dates are given. Had no idea what those methods were used for until now.

@Jammjammjamm
Copy link
Contributor

The issue isn't specific to particular resources, but affects the validation of any resource with primitive extensions.

@yunwwang yunwwang added the WIP Work In Progress label Apr 6, 2022
@yunwwang yunwwang changed the title inferno appears to be stripping dataAbsentReason fields inferno appears to be stripping dataAbsentReason fields (FI-1520) Apr 6, 2022
@yunwwang
Copy link
Contributor

yunwwang commented Apr 6, 2022

@nathanloyer For the DocumentReference one, I believe the intention for the US Core author is that an attachment must have either embedded data or url. What is the use care for a DocumentReference with empty content?

@yunwwang yunwwang added remove constraint The change should not cause a system that previously passed to now fail. resolved Fix is approved and merged. Working ticket closed and removed WIP Work In Progress labels Apr 6, 2022
@nathanloyer
Copy link
Contributor Author

I'll try out the fix you merged today.

For the content type thing, I'll have to investigate more with the devs that implemented that resource and get back to you. Will likely open a new thread for that in zulip if we can't figure that one out.

@Jammjammjamm
Copy link
Contributor

That fix was in inferno_core, so you won't be able to easily test it out until we do a new release. We'll be doing a prerelease soon, so once that is out I can let you know and give you instructions on using it.

@nathanloyer
Copy link
Contributor Author

yeah, I was trying to pick up the changes to inferno_core the same way that I had been picking up changes to the other packages, which I think it was you @Jammjammjamm that originally suggested it to me. But it doesn't work when I try that.

# frozen_string_literal: true

source "https://rubygems.org"

gemspec

group :development, :test do
  gem 'rubocop', '~> 1.9'
  gem 'rubocop-rspec', require: false
  gem 'rubyXL'
  gem 'inferno_core', git: 'https://github.com/inferno-framework/inferno-core.git', ref: '155e4c0'
  gem 'us_core_test_kit', git: 'https://github.com/inferno-framework/us-core-test-kit.git', ref: 'fa1c7cf'
end

@nathanloyer
Copy link
Contributor Author

image

@Jammjammjamm
Copy link
Contributor

Yes, loading up test kits from git will generally work fine, but loading inferno_core from git won't because it needs the built js files which are included in the published gem, but not in git.

@nathanloyer
Copy link
Contributor Author

ok, thanks. When do you expect the prerelease to be available?

@Jammjammjamm
Copy link
Contributor

Hopefully today or tomorrow.

@nathanloyer
Copy link
Contributor Author

Perfect, thanks. Is there a way I can subscribe to automatic updates when that happens?

@Jammjammjamm
Copy link
Contributor

I'm not sure what you mean by automatic updates. I'll make a post in this issue when it's out.

@nathanloyer
Copy link
Contributor Author

I mean any way to get a notification or email or something when prereleases are created. I think I get github notifications when the actual release goes out because I'm watching the repos, but I'm not sure if there's a way to do the same for prereleases

@Jammjammjamm
Copy link
Contributor

If you go to rubygems.org, make an account, and then go to a gem's page (like inferno_core or onc_g10_certification_test_kit), on the right under links there's a subscribe link. I've never used that, but if it doesn't give you notifications on new releases, then I'm not sure what it's for. There's also an rss link there which you can use to get a feed for releases.

@nathanloyer
Copy link
Contributor Author

@Jammjammjamm
Copy link
Contributor

Ok, we just released 0.3.0.rc1 of inferno_core. So you can update the gemspec to point to that version of inferno_core, and then run bundle update inferno_core if you have ruby set up locally. If you don't have ruby setup locally, you can add RUN bundle update inferno_core to the Dockerfile above RUN bundle install. That should be all that is needed to test the changes in the new core version. A prerelease version of the g10 test kit will be coming a bit later.

@nathanloyer
Copy link
Contributor Author

On the g10 update, there are also changes merged into main for the us-core-test-kit that we need, so just wanted to confirm if all the other dependencies of the g10 kit are also going to get released and have the new versions linked to in g10 before this next release.

@nathanloyer
Copy link
Contributor Author

oh right, the fix was in the smart app kit, and looks like the new version has been released there today, so guess I'm answering my own questions :D

https://github.com/inferno-framework/smart-app-launch-test-kit/releases/tag/v0.1.2

@nathanloyer
Copy link
Contributor Author

I'm struggling to keep track of everything... so there are fixes we need in inferno-core@0.3.0, smart-app-launch-test-kit@0.1.2 and us-core-test-kit@main

looks like us-core-test-kit has a release in the works as well

inferno-framework/us-core-test-kit#30

@Jammjammjamm
Copy link
Contributor

Inferno core has the fix for validating primitive extensions, smart has the fix for PKCE length. I don't know if the us core test kit has any changes that you specifically care about, but it has some breaking changes that will make using the new version more complicated than just bumping the version.

@nathanloyer
Copy link
Contributor Author

us-core-test-kit does have changes we need. It was this PR

inferno-framework/us-core-test-kit#24

I've actually patched a fix for the g10 repo to use the latest us-core-test-kit into our local fork of the g10 repo. Right now we're deploying using a forked g10 kit until we're done iterating on getting tests passing, then we plan to create our own kit and pulling in the g10 kit as a dependency. Everything got working with just renaming the us core modules to have the additional USCore311 namespaceing. I could turn that into a PR for this repo if that would help accelerate a new g10 release with the latest us core kit changes.

@Jammjammjamm
Copy link
Contributor

We just released 2.1.0.rc1 for g10. Can take a minute for ruby gems to make it available.

@nathanloyer
Copy link
Contributor Author

oh, great, will try that out now

@Jammjammjamm
Copy link
Contributor

We also have it up on inferno-dev.healthit.gov now.

@nathanloyer
Copy link
Contributor Author

confirmed this is working yesterday. thanks for the help

@yunwwang
Copy link
Contributor

Reopened. This issue will be group with other issues for release v2.1

@yunwwang yunwwang reopened this Apr 12, 2022
@yunwwang yunwwang added v2.1.0 Release v2.1.0 and removed resolved Fix is approved and merged. Working ticket closed labels Apr 12, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
remove constraint The change should not cause a system that previously passed to now fail. v2.1.0 Release v2.1.0
Projects
None yet
Development

No branches or pull requests

4 participants