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

FHIRPath Patch #679

Closed
lmsurpre opened this issue Feb 4, 2020 · 2 comments
Closed

FHIRPath Patch #679

lmsurpre opened this issue Feb 4, 2020 · 2 comments
Assignees
Milestone

Comments

@lmsurpre
Copy link
Member

lmsurpre commented Feb 4, 2020

Is your feature request related to a problem? Please describe.
Support for https://www.hl7.org/fhir/fhirpatch.html

Describe the solution you'd like
In addition to the HTTP interface, the solution should have a Java API that can be used directly.

Additional context
Not a high priority, but #666 adds the base support, so exposing that from the REST layer shouldn't be too bad.

@lmsurpre lmsurpre added this to the Sprint 8 milestone Feb 4, 2020
@lmsurpre lmsurpre self-assigned this Feb 5, 2020
lmsurpre added a commit that referenced this issue Feb 5, 2020
TODO:
1. Hook up the tests provided with the spec
2. Update our CapabilityStatement to indicate we support FHIRPath patch

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Feb 5, 2020
1. Added FHIRPathSpecTest for executing the spec-provided
FHIRPath Patch test cases. I needed to tweak the test file to work
around our strict interpretation of `ele-1`
which makes empty resources invalid.
I also removed a single fhirpath patch operation from the
`Full Resource` testcase...we currently cannot handle replace of
DomainResource.text.div

2. Added logic to throw UnsupportedOperationException for patches
with nested part values and added corresponding skip logic from the test

3. Added skip logic for 2 other test cases, both with `mode=forwards`
* `Delete Nested Primitive #2`: this test expects that the server will
automatically remove a parent elements when the last element from within
one is removed. We don't do that.
* `Reorder List #4`: on this one, I disagree with the expected output

4. Found and fixed an issue with the original implementation;
patch values for code subtypes come is as basic Code types whereas the
setters expect a subtype, so I needed to add logic to convert the Code
to the appropriate subtype before calling any setters.

Finally, I added the FHIR mimetypes to the patchFormat element of our
CapabilityStatement to indicate that we now support FHIRPath patch.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Feb 5, 2020
1. Added FHIRPathSpecTest for executing the spec-provided
FHIRPath Patch test cases. I needed to tweak the test file to work
around our strict interpretation of `ele-1`
which makes empty resources invalid.
I also removed a single fhirpath patch operation from the
`Full Resource` testcase...we currently cannot handle replace of
DomainResource.text.div

2. Added logic to throw UnsupportedOperationException for patches
with nested part values and added corresponding skip logic from the test

3. Added skip logic for 2 other test cases, both with `mode=forwards`
* `Delete Nested Primitive #2`: this test expects that the server will
automatically remove a parent elements when the last element from within
one is removed. We don't do that.
* `Reorder List #4`: on this one, I disagree with the expected output

4. Found and fixed an issue with the original implementation;
patch values for code subtypes come is as basic Code types whereas the
setters expect a subtype, so I needed to add logic to convert the Code
to the appropriate subtype before calling any setters.

Finally, I added the FHIR mimetypes to the patchFormat element of our
CapabilityStatement to indicate that we now support FHIRPath patch.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Feb 5, 2020
1. Added FHIRPathSpecTest for executing the spec-provided
FHIRPath Patch test cases. I needed to tweak the test file to work
around our strict interpretation of `ele-1`
which makes empty resources invalid.
I also removed a single fhirpath patch operation from the
`Full Resource` testcase...we currently cannot handle replace of
DomainResource.text.div

2. Added logic to throw UnsupportedOperationException for patches
with nested part values and added corresponding skip logic from the test

3. Added skip logic for 2 other test cases, both with `mode=forwards`
* `Delete Nested Primitive #2`: this test expects that the server will
automatically remove a parent elements when the last element from within
one is removed. We don't do that.
* `Reorder List #4`: on this one, I disagree with the expected output

4. Found and fixed an issue with the original implementation;
patch values for code subtypes come is as basic Code types whereas the
setters expect a subtype, so I needed to add logic to convert the Code
to the appropriate subtype before calling any setters.

Finally, I added the FHIR mimetypes to the patchFormat element of our
CapabilityStatement to indicate that we now support FHIRPath patch.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Feb 5, 2020
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Feb 5, 2020
lmsurpre added a commit that referenced this issue Feb 8, 2020
The previous approach failed in certain cases when the Visitable tree
contains the exact same object in multiple different places (e.g. adding
to a name when that same name object is repeated twice in teh same
list).

With this change, the CopyingVisitor now keeps its own pathStack and we
use the normalized path of FHIRPath nodes and/or their parent to ensure
we're only modifying the proper paths in the tree.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Feb 8, 2020
The previous approach failed in certain cases when the Visitable tree
contains the exact same object in multiple different places (e.g. adding
to a name when that same name object is repeated twice in teh same
list).

With this change, the CopyingVisitor now keeps its own pathStack and we
use the normalized path of FHIRPath nodes and/or their parent to ensure
we're only modifying the proper paths in the tree.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre
Copy link
Member Author

  1. bring up failing test on fhir chat
  2. open placeholder issue for nested parameters

lmsurpre added a commit that referenced this issue Feb 18, 2020
Somehow I left these in a broken state. I had renamed the test file and
added some stuff, but because the test file never loaded properly it
never generated any tests (which would have failed). This change fixes
that all.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Feb 18, 2020
Somehow I left these in a broken state. I had renamed the test file and
added some stuff, but because the test file never loaded properly it
never generated any tests (which would have failed). This change fixes
that all.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Feb 19, 2020
issue #679 - fix the fhirpathspectests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant