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

Allow persistence interceptors to modify the incoming resource #2369

Closed
lmsurpre opened this issue May 17, 2021 · 2 comments
Closed

Allow persistence interceptors to modify the incoming resource #2369

lmsurpre opened this issue May 17, 2021 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@lmsurpre
Copy link
Member

lmsurpre commented May 17, 2021

Is your feature request related to a problem? Please describe.
IBM FHIR Server supports persistence interceptors which can fire arbitrary business logic before or after an interaction for a given resource type.
In some cases, it might be helpful to add or remove fields to the incoming resources (during create or update).
For example, to add some tag.

Unfortunately, the current implementation allows an interceptor to modify the fhirResource in the persistence event, but it uses the original resource for the create/update instead of the updated one in the event.

Describe the solution you'd like
the doCreate and doUpdate methods should use the resource from the event instead of passing in the original payload

Describe alternatives you've considered
the persistencecontext already includes the event, so we could potentially update the Persistence interface to stop duplicating the resources in its signature. that would be a breaking change and it would make implementing persistence layers a bit more annoying, but would prevent the possibility of a resource that is out-of-sync with the corresponding event.

Acceptance Criteria

  1. GIVEN a persistence interceptor that modifies the incoming resources
    WHEN you create a resource
    THEN the saved resource is the modified one from the interceptor

  2. GIVEN a persistence interceptor that modifies the incoming resources
    WHEN you update a resource
    THEN the saved resource is the modified one from the interceptor

Additional context
I think this used to work back when our model was mutable...the interceptor would just change the actual object in the event. Now that we are immutable it needs to create a new one and attach that new one to the event.

@lmsurpre lmsurpre self-assigned this May 17, 2021
@lmsurpre lmsurpre added this to the Sprint 2021-07 milestone May 17, 2021
@lmsurpre
Copy link
Member Author

lmsurpre commented May 18, 2021

Robin pointed out that, when we allow this, there will be an interesting ordering dependency between the interceptors and the "update-only-if-modified" feature from #2263. Which should come first?

In my opinion, I think it would be best for the "update-only-if-modified" check to come first. This way, we will avoid calling the beforeUpdate hooks for updates that won't actually happen.

This means that, in cases where the interceptors modify the incoming payload in some way, we might end up performing an unnecessary update (if the incoming resources didn't match the saved resource before the interceptors but does match afterward).

It also means that if you have some resources saved and then introduce an interceptor that modifies incoming resources and then perform the update with X-FHIR-UPDATE-IF-MODIFIED set to true; the result will be that the update gets skipped even though the resources wouldn't have matched after the interceptor did its thing.

lmsurpre added a commit that referenced this issue May 18, 2021
This allows the interceptors to modify the incoming resource.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@prb112 prb112 added the enhancement New feature or request label May 18, 2021
lmsurpre added a commit that referenced this issue May 18, 2021
This allows the interceptors to modify the incoming resource.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue May 19, 2021
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue May 19, 2021
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@prb112
Copy link
Contributor

prb112 commented May 19, 2021

QA:
used the tagginginterceptor
created a Patient (Prefer=representation)
Confirmed the tags

{
    "resourceType": "Patient",
    "id": "17985d2db64-ec4f577a-d081-4d14-bd49-90b1b568d91b",
    "meta": {
        "versionId": "1",
        "lastUpdated": "2021-05-19T18:10:59.044464Z",
        "tag": [
            {
                "system": "http://ibm.com/fhir/test",
                "code": "test"
            }
        ]
    },

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

2 participants