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

Populate Bundle.entry.response.outcome with validation warnings #2281

Closed
lmsurpre opened this issue Apr 23, 2021 · 1 comment
Closed

Populate Bundle.entry.response.outcome with validation warnings #2281

lmsurpre opened this issue Apr 23, 2021 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@lmsurpre
Copy link
Member

lmsurpre commented Apr 23, 2021

Is your feature request related to a problem? Please describe.
While reviewing our bundle processing code, I noticed that we pass validation warnings back through Bundle.entry.resource (at least in some cases).

This is the proper location for errors, but for warnings the spec defines a supplemental location: Bundle.entry.response.outcome

From the spec:

For a POST/PUT operation, this is the equivalent outcome that would be returned for prefer = operationoutcome - except that the resource is always returned whether or not the outcome is returned.
This outcome is not used for error responses in batch/transaction, only for hints and warnings. In a batch operation, the error will be in Bundle.entry.response, and for transaction, there will be a single OperationOutcome instead of a bundle in the case of an error.

Describe the solution you'd like
Lets start adding our validation outcomes to Bundle.entry.response.outcome.

However, I still like having clients be able to control whether the resource content comes back in the Bundle.entry.resource or not, so I propose we do this:

  • always populate Bundle.entry.response.outcome with validation hints / warnings
  • when return=minimal (default), omit the resource contents from Bundle.entry.resource
  • when return=OperationOutcome, write an interaction-related (not validation-related) OperationOutcome in the Bundle.entry.resource
  • when return=representation, write the resulting resource contents to the response Bundle.entry.resource

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Acceptance Criteria

Additional context
There is plenty of room for cleanup here as well.

  1. We loop through the entire bundle for each method, whereas we really only need to iterate this list once or twice.
  2. We create a responseBundle up front and then we insert the entries one-at-a-time while we process them. But that was designed when our model was mutable...each "insert" is actually copying the bundle into a new builder and then building a brand new bundle. All of this copying is unnecessary and could slow things down / bloat the memory usage for large bundle requests.
@lmsurpre lmsurpre self-assigned this Apr 23, 2021
@lmsurpre lmsurpre added this to the Sprint 2021-06 milestone Apr 23, 2021
lmsurpre added a commit that referenced this issue Apr 23, 2021
1. FHIRRestHelper.validateBundle now returns a sparse array (modeled as
a map) of bundle response entries with warnings. It used to construct an
entire response bundle.

2. FHIRRestHelper.processEntriesByMethod replaces
FHIRRestHelper.processEntriesForMethod and iterates the request entries
a single time, instead of once-per-method.

3. Entries are collected into an array instead of "modifying" the
responseBundle with each one (which forced lots of needless copying
since our model is now immutable).

I also made significant formatting changes in FHIRRestHelper.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 23, 2021
1. FHIRRestHelper.validateBundle now returns a sparse array (modeled as
a map) of bundle response entries with warnings. It used to construct an
entire response bundle.

2. FHIRRestHelper.processEntriesByMethod replaces
FHIRRestHelper.processEntriesForMethod and iterates the request entries
a single time, instead of once-per-method.

3. Entries are collected into an array instead of "modifying" the
responseBundle with each one (which forced lots of needless copying
since our model is now immutable).

I also made significant formatting changes in FHIRRestHelper.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 23, 2021
1. FHIRRestHelper.validateBundle now returns a sparse array (modeled as
a map) of bundle response entries with warnings. It used to construct an
entire response bundle.

2. FHIRRestHelper.processEntriesByMethod replaces
FHIRRestHelper.processEntriesForMethod and iterates the request entries
a single time, instead of once-per-method.

3. Entries are collected into an array instead of "modifying" the
responseBundle with each one (which forced lots of needless copying
since our model is now immutable).

I also made significant formatting changes in FHIRRestHelper.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Apr 24, 2021
1. FHIRRestHelper.validateBundle now returns a sparse array (modeled as
a map) of bundle response entries with warnings. It used to construct an
entire response bundle.

2. FHIRRestHelper.processEntriesByMethod replaces
FHIRRestHelper.processEntriesForMethod and iterates the request entries
a single time, instead of once-per-method.

3. Entries are collected into an array instead of "modifying" the
responseBundle with each one (which forced lots of needless copying
since our model is now immutable).

4. Stop duplicating custom operation results in the
`Bundle.entry.response.outcome` field of the response. We already copy
the operation's result in the `Bundle.entry.resource` and that should be
sufficient.

I also made significant formatting changes in FHIRRestHelper.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@prb112 prb112 added the enhancement New feature or request label Apr 26, 2021
@d0roppe
Copy link
Collaborator

d0roppe commented Apr 30, 2021

Verified that this is now working as described for the different options listed.

@d0roppe d0roppe closed this as completed Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants