IGDD-3285 - FHIR searchset: label joins as include, return only what the caller asked for - #283
Merged
Conversation
R4 defines include as "added to the results because of a join", so a client filtering on match could not tell the hits from the resources joined in to support them. Also close the reference graph: the mandatory 1..1 patient and the authority Organization resolved to nothing in the delivered bundle. A target that cannot be retained drops its reference element, keeping identifier and display. A recommendation query now returns the Z42 evaluated history as include. Its dose number, series doses, authority and program eligibility reach no other call, because /Immunization sends Z34 and receives Z32. Bumps v2tofhir to 2.5.1-SNAPSHOT for the history/forecast split. BREAKING CHANGE: _include and _revinclude results report search.mode=include, not match. IGDD-3285
austinmoody
marked this pull request as ready for review
August 17, 2026 01:17
Contributor
|
note: "Driven through the mocked hub. The captures are deliberately not committed — they carry live The Mocked Hub does NOT contain live vendor IIS patient demographics, that is all TEST patient data produced from download generated by AIRA. |
keithboone
requested changes
Aug 17, 2026
keithboone
left a comment
keithboone
left a comment
Contributor
There was a problem hiding this comment.
There are TWO SonarCube comments on this PR that were NOT addressed by it.
I would like to review the content with @austinmoody more thoroughly.
keithboone
approved these changes
Aug 17, 2026
keithboone
left a comment
keithboone
left a comment
Contributor
There was a problem hiding this comment.
Be sure to fix SonarQube issues.
TL;DR: a FHIR query now returns the resource type you asked for and nothing else. Want the Patient, the Organization, the Location, or the Z42 evaluated history too? Ask for them with _include / _revinclude. What was happening A plain GET /Immunization also returned the Patient. A plain GET /ImmunizationRecommendation also returned the Patient, the schedule Organization, and every Immunization in the evaluated history. Nobody asked for any of it. That was added to stop mandatory 1..1 references like Immunization.patient from pointing at a resource the bundle did not contain, but it meant you could not predict a response from the query that produced it. What changed - Removed retainReferencedResources: being referenced is no longer a reason to be returned. - Removed the Z42 branch in preFilter: the evaluated history is no longer returned unasked. - Removed clearUnresolvableReferences: a reference whose target is not in the bundle keeps the value the conversion produced, instead of being stripped down to identifier + display. This is what develop already did, and no defect was ever raised against it. Kept from b714b59: an _include / _revinclude hit is still labelled search.mode=include, not match, so filtering on match still gives you exactly the hits you asked for. To get the old payload back GET .../ImmunizationRecommendation?_include=*:*&_revinclude=Immunization Same entries as before. One difference: the PractitionerRole -> Practitioner reference now keeps its literal value. Three things we learned running this 1. _revinclude only works from a resource already in the bundle. The code finds "things pointing at you" via bookkeeping stored on the pointed-at resource, so that resource has to be there first. The old auto-retain hid this by always including the Patient. This is why _revinclude=Immunization needs _include=ImmunizationRecommendation:patient beside it. 2. _revinclude never reaches a resource that only Resource:source white-listed. A white-listed resource is returned but never walked, so _revinclude=Provenance returns nothing - and never did, on this branch or on develop. 3. protocolApplied.authority resolves via _include=Immunization:authority, not the recommendation's. That Organization is registered on the Immunization. Also bumps v2tofhir to 2.5.2-SNAPSHOT, the version every behavior above was verified against. Scope: FHIR REST responses only. No change to the SOAP/HL7 v2 path, the outbound query, the transformation pipeline, or any organization configuration. Tests: 251 pass, Checkstyle clean, OWASP high finding is 5.3 (under the 7.0 gate). BREAKING CHANGE: FHIR query responses no longer include resources the caller did not request. Callers relying on the Patient, Organization, Location, or Z42 evaluated history arriving unasked must add the matching _include / _revinclude parameters. See docs/fhir/fhir-api.md. Refs: openspec/changes/fhir-searchset-strict-includes
…ta key SonarQube flagged the "Resource" literal in matchesSource as duplicating RESOURCE_KEY. Merging them would be wrong: they spell the same by coincidence and mean unrelated things. - RESOURCE_KEY is an internal v2tofhir user-data key. ParserUtils.createReference stores a Reference's target under it, and v2tofhir is free to rename it. - The literal in matchesSource is the resource-type token in the caller's URL - the "Resource" in _include=Resource:source:<type>. It is public API surface, documented in docs/fhir/rsp-to-fhir.md. Sharing one constant would let a rename of either silently change the other, with no compile error. So this adds SOURCE_INCLUDE_TYPE for the URL token, with a comment recording that the collision is coincidental and must stay separate. Neither line was touched by fcb3e77; SonarCloud surfaced a pre-existing issue because that commit removed the third RESOURCE_KEY usage. No behavior change. 251 tests pass, Checkstyle clean.
Contributor
Author
SonarQube items taken care of. Running through some other tests. |
…b sends Z42
TS_TC_07f queried /ImmunizationRecommendation and asserted a forecast came back.
IZ Gateway Hub does not currently respond with Z42, so no dev fixture produces a
forecast, and the case failed with no ImmunizationRecommendation in the bundle:
Verify the recommendation is the match
expected undefined to equal 'match'
Not a code problem. A missing requested type cannot be caused by this branch -
the removed auto-retain only ever added entries, so pre-change code would still
have returned the recommendation as match. The bundle genuinely had no forecast
in it, because nothing asked the Hub for one before: the collection sends Z34 in
four places and Z44 nowhere.
Removed from both the Certificate and JWT Okta folders. TS_TC_07d (no Patient by
default) and TS_TC_07e (Patient arrives with _include) stay - both pass against
dev, and 07e passing confirms dev is running this branch, since pre-change code
labelled an _include hit match rather than include.
The recovery recipe stays covered by the unit test
recoveryParametersReproduceThePreChangePayload, which drives the Z42 fixture
directly. Re-add the integration case once the Hub returns Z42.
Said white-listed resources are not traversed. They are. Real cause: v2tofhir gives Provenance a bare id, so a type-qualified _revinclude compares against null. Wildcard works. Also: 15 Provenance was 23; "the registered names" listed 11 of 26; three _revinclude rows need a forward _include first. Spec: include-labelling requirement moved to MODIFIED - its scenario would have contradicted the new reverse-include rule after archive.
austinmoody
changed the title
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HUMAN REVIEWER - see comment notes in https://izgateway.atlassian.net/browse/IGDD-3285 for examples and info
Why
Branch carries two OpenSpec changes, both in
FhirController's searchset filter (post-processesevery FHIR query response):
fix-fhir-searchset-include-mode— every_include/_revincludehit was labelledsearch.mode = "match". Client could not tell hits from joined supporting resources.fhir-searchset-strict-includes— first change also auto-retained referenced resources(
Patienton every query, scheduleOrganizationand Z42 evaluated history on/ImmunizationRecommendation) to close dangling mandatory 1..1 references. That made responseshape unpredictable from the query. Second change reverts to strict contract: caller gets what
caller asked for.
Net behavior vs develop
match),OperationOutcome(outcome), resources thecaller joined in (
include). Everything else removed.identifier,display— same as develop. Caller resolves or ignores._include=ImmunizationRecommendation:patient&_revinclude=Immunization:patient, plus_include=Immunization:authorityto resolveprotocolApplied.authorityin-bundle._include=*:*&_revinclude=Immunizationreproduces the old payload that I had which returned everything by default._revincluderesolves only from resources already retained — evaluated history and forecastObservationreference thePatient, so retain thePatientfirst._include=Resource:source:<type>/Resource:source:*,or a forward
_includereaching them.*for types and search names; unknown names match nothing, not an error.v2tofhirto2.5.2-SNAPSHOTfor the history/forecast split changes (IGDD-3285: fix Z42 history/forecast split for eHealth Exchange pilot v2tofhir#52) and also fixes for Location labeling (fix: assign Location.physicalType codes to the correct PL/LA2 components v2tofhir#53).Tests
252 tests, 0 failures; 70 in
FhirControllerTestswith Z32/Z42 fixtures covering match/includelabelling, strict default, wildcard includes, revinclude anchoring, and literal-reference
preservation.
A couple new Postman tests.
KNOWN GAP our mock data DOES NOT have Z42. Ticket entered into backlog to have that added so that we can add /ImmunizationRecommendation tests to Postman.
Docs / Spec
docs/fhir/fhir-api.mdanddocs/fhir/rsp-to-fhir.mdrewritten for the strict contract. Bothchanges archived under
openspec/changes/archive/;openspec/specs/fhir-searchset-filteringsynced (9 requirements).
IGDD-3285