NullPointerException when deleting resource with fhir-smart enabled.

[jornvanwier]

Describe the bug

When the fhir-smart interceptor is enabled, deleting a resource gives a NullPointerException.

I did some digging, and I see that in the AuthzPolicyEnforcementPersistenceInterceptor it uses event.getPrevFhirResource to determine to resource being deleted, but in FHIRRestHelper only event.setFhirResource is called, not event.setPrevFhirResource.

I was able to successfully delete a resource after modifying the code to call event.setPrevFhirResource in the doDelete method of FHIRRestHelper. Alternatively, I believe the beforeDelete method in AuthzPolicyEnforcementPersistenceInterceptor could also use event.getFhirResource instead of event.getPrevFhirResource.

Environment

4.10.2

To Reproduce

Steps to reproduce the behavior:

1. Have a patient with a resource (e.g. Observation).
2. perform a DELETE request for that resource, authenticated with a JWT containing the corresponding patient id and at least the patient/Observation.* scope.
3. The server responds with a status code of 500 and body of {"resourceType":"OperationOutcome","issue":[{"severity":"fatal","code":"exception","details":{"text":"NullPointerException: resource"}}]}

Expected behavior

The resource should be deleted, and no NullPointerException should occur.

[jornvanwier]

I have created a PR with possible fix: #3292

[lmsurpre]

Thanks @jornvanwier for the good bug report.
We discussed this one as a team today and we're leaning toward the opposite fix: an update the FHIRRestHelper that sets the previous resource (the version thats in the db prior to the delete) in the event.
Specifically, the event's prevResource should match exactly what we read from the db.
We should also be setting the event's resource to be the updated version (exactly the same but with an updated meta.versionId and meta.lastUpdated).

We'd like to get this in for our next release. If you'd like to take a stab at the implementation, we can provide some guidance as needed. Otherwise, we can take this one. Let us know.

P.S. There's been a number of changes in this area recently, so be sure to work from the latest on main.

[jornvanwier]

Hi Lee, I'd like to give the implementation a try. Currently, a resource with the new versionId and lastUpdated is created after the beforeDelete event is fired, am I correct in assuming I can just move that code up a bit and add that resource to the event?

[punktilious]

Hi @jornvanwier, yes I think that will work. Something like:

                    // For soft-delete we store a new version of the resource with the deleted
                    // flag set. Update the resource meta so that it has the correct version id
                    final String resourcePayloadKey = UUID.randomUUID().toString();
                    final com.ibm.fhir.model.type.Instant lastUpdated = getCurrentInstant();
                    final int newVersionNumber = FHIRPersistenceSupport.getMetaVersionId(resourceToDelete) + 1;
                    final Resource resource = FHIRPersistenceUtil.copyAndSetResourceMetaFields(resourceToDelete, id, newVersionNumber, lastUpdated);

                    event.setPrevFhirResource(resourceToDelete);
                    event.setFhirResource(resource);
                    getInterceptorMgr().fireBeforeDeleteEvent(event);
...
                    // Invoke the 'afterDelete' interceptor methods.
                    // event.setFhirResource(resource); <== this can also be removed now
                    getInterceptorMgr().fireAfterDeleteEvent(event);

[kmbarton423]

Confirmed successful deletion of resources with fhir-smart enabled.