Core library bump 6.3.7

[dotasek]

This is a version bump of the org.hl7.fhir.core library from 6.1.2.2 to 6.3.7.

The trivial changes are matching changing method signatures and class changes.

The more difficult changes revolve around the resource validation. HAPI provides its own implementations of IWorkerContext, which are used by core's InstanceValidator to validate resources.

InstanceValidator has changed significantly with regard to logic, particularly regarding code validation:

org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java

See comments for discussion of some of these changes.

[github-actions[bot]]

Formatting check succeeded!

[dotasek]

@jamesagnew These are the passthrough message failures I was discussing:

https://github.com/hapifhir/hapi-fhir/blob/c13d2f5e2945f26057d99d23ed4b63c7b4ccf4e3/hapi-fhir-validation/src/test/java/org/hl7/fhir/r4/validation/FhirInstanceValidatorR4Test.java#L1753

https://github.com/hapifhir/hapi-fhir/blob/c13d2f5e2945f26057d99d23ed4b63c7b4ccf4e3/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ValidateTest.java#L1624

It looks to me, from tracing, that core is no longer producing these, but it's difficult to tell because the code varies so drastically. Are these passthrough messages critical?

[dotasek]

The change I have tried to tackle so far is InstanceValidator's reliance on a new method called processTxIssues: https://github.com/hapifhir/org.hl7.fhir.core/blob/f2fe93a1d7dfe66d1c70a7ef4379ed511a81e1c7/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/instance/InstanceValidator.java#L1500

This gets used throughout the validation process to determine validity. Unlike the previous versions of the validator, this processes OperationOutcomeIssueComponent entries in the ValidationResult via getIssues(). Since previous versions of HAPI did not produce these issues at all, this method would always return true once this version bump was introduced, resulting in inaccurate validation results.

~~An attempt has been made with this PR to introduce appropriate issues for each ValidationResult.~~

(Updated 2024/05/10)

All of our tested IValidationSupport implementations are now producing issues that match as closely as possible the expected results from the core validator.

[dotasek]

@jamesagnew this is a failure that appears to no longer be a valid test:

https://github.com/hapifhir/hapi-fhir/blob/c13d2f5e2945f26057d99d23ed4b63c7b4ccf4e3/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/interceptor/validation/ValidationMessageSuppressingInterceptorTest.java#L73

If I run this with previous versions of the Validator, it does show me the expected error message:

java -jar org.hl7.fhir.validation.cli-6.1.2.2.jar -version 4.0 resource.json -ig CodeSystem-dummy-loinc.json -ig StructureDefinition-us-core-pulse-oximetry.json -profile http://hl7.org/fhir/us/core/StructureDefinition/us-core-pulse-oximetry
...
  Error @ Observation.code (line 32, col3): The provided code 'http://loinc.org#59408-5 ('Oxygen saturation in Arterial blood by Pulse oximetry')' was not found in the value set 'http://hl7.org/fhir/ValueSet/observation-vitalsignresult|4.0.1' (from Tx-Server)
  Error @ Observation.code (line 32, col3): The provided code 'urn:iso:std:iso:11073:10101#150456 ('MDC_PULS_OXIM_SAT_O2')' was not found in the value set 'http://hl7.org/fhir/ValueSet/observation-vitalsignresult|4.0.1' (from Tx-Server)
  Information @ Observation.code.coding[2] (line 44, col5): This element does not match any known slice defined in the profile http://hl7.org/fhir/us/core/StructureDefinition/us-core-pulse-oximetry|3.1.1

However, the new version of the validator passes:

  Information @ Observation.code (line 32, col3): Reference to draft CodeSystem urn:iso:std:iso:11073:10101|2023-04-26
  Information @ Observation.code (line 32, col3): Reference to experimental CodeSystem http://loinc.org|20160128
  Information @ Observation (line 1, col2): Validate Observation against the Oxygen saturation profile (http://hl7.org/fhir/StructureDefinition/oxygensat) which is required by the FHIR specification because the LOINC code 2708-6 was found
  Information @ Observation.code.coding[2] (line 44, col5): This element does not match any known slice defined in the profile http://hl7.org/fhir/us/core/StructureDefinition/us-core-pulse-oximetry|3.1.1 (this may not be a problem, but you should check that it's not intended to match a slice)
  Warning @ Observation (line 1, col2): Best Practice Recommendation: In general, all observations should have a performer

I'm going to force a bad loinc code:

{
  "system": "http://loinc.org",
  "code": "not-a-real-code",
  "display": "Oxygen saturation in Arterial blood by Pulse oximetry"
},

... and filter out the two errors that stem from that. The original intent for that interceptor test must have been US-CORE related, but now I think it's just about preserving the message filtering.

[dotasek]

This commit decrements the SQL query/commit counts for validation.

45cf37e

Since InstanceValidation in the core library dictates much of this work, and the logic behind terminology validation has changed, it is reasonable that these counts would be different.

If these are decreased, and the validation test changes are valid and passing, I think we can assume these new counts to be correct.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.43%. Comparing base (497b9f2) to head (5b83768). Report is 171 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #5772      +/-   ##
============================================
+ Coverage     83.39%   83.43%   +0.04%     
- Complexity    26927    27231     +304     
============================================
  Files          1681     1695      +14     
  Lines        103965   105330    +1365     
  Branches      13189    13317     +128     
============================================
+ Hits          86702    87883    +1181     
- Misses        11613    11743     +130     
- Partials       5650     5704      +54     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[dotasek]

Approved pending resolution of comments and also strongly suggest getting James to look at the most senstive production code changes.

I wholeheartedly agree. Thank you for the initial review, it's shaking out changes that should be commented on for the record.

Attending to code comments now. I've tagged James wherever he comes into the conversation.

[heathfrankel]

Hi @dotasek and @jamesagnew . Has there been any progress on this PR. We are waiting on the resolution of this terminology passthrough validation issue for the AU Sparked Reference Server in readiness for our AU Core Vendor Testing events at the end of the month. Is there anything we can do to help progress this?

[jamesagnew]

Hi @dotasek and @jamesagnew . Has there been any progress on this PR. We are waiting on the resolution of this terminology passthrough validation issue for the AU Sparked Reference Server in readiness for our AU Core Vendor Testing events at the end of the month. Is there anything we can do to help progress this?

@heathfrankel This is being actively worked on by several people, but it's nontrivial. David has been leading the work, but there are a number of breaking API changes, and at least one breaking functional validation change that we need to account for before we can land this. Hopefully we'll see this land in the next week or 2.

[dotasek]

Hi @dotasek and @jamesagnew . Has there been any progress on this PR. We are waiting on the resolution of this terminology passthrough validation issue for the AU Sparked Reference Server in readiness for our AU Core Vendor Testing events at the end of the month. Is there anything we can do to help progress this?

@heathfrankel I echo what James has said and I would also suggest that any additional code review would be helpful.

[dotasek]

For https://github.com/hapifhir/hapi-fhir/pull/5772/commits/4f491e8ee31d75abc64b2613154232b08efd8ff4 there was a change in the underlying test. The history of this test seems to have waffled back and forth between expected responses, so I referred to the original request:

https://github.com/hapifhir/hapi-fhir/issues/4461

The test uses patient-with-single-comm-lang-en_US-UNDERSCORE.json, which contains en_US, an invalid language code. It is singular; there are no valid codes included in the coding, and therefore this should always fail, regardless of the value of EnabledValidationForCodingsLogicalAnd

Edit: On reviewing the original ticket, technically, this is a logical ANY applied to a set, not an AND. I feel like it should be renamed.