3788 FHIR patch on soft deleted resource undeletes resource (#3789)

* failing test

* fix

* refactor

* refactor

* changelog

* changelog

* Update hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3788-fix-fhir-patch-on-soft-deleted-resources.yaml

Co-authored-by: jmarchionatto <60409882+jmarchionatto@users.noreply.github.com>

Co-authored-by: Justin_Dar <justin.dar@smilecdr.com>
Co-authored-by: jmarchionatto <60409882+jmarchionatto@users.noreply.github.com>
This commit is contained in:
jdar8 2022-07-14 11:41:36 -07:00 committed by GitHub
parent 05ebf0286d
commit 5575e722d4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 71 additions and 5 deletions

View File

@ -0,0 +1,7 @@
---
type: fix
issue: 3788
jira: SMILE-4321
title: "Previously, if a FHIR patch was performed on a soft deleted resource, the patch was successfully applied
and the resource was undeleted and populated with only the data contained in the patch. This has been fixed
so that patch on deleted resources now issues a HTTP 410 response."

View File

@ -550,7 +550,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
}
// Don't delete again if it's already deleted
if (entity.getDeleted() != null) {
if (isDeleted(entity)) {
DaoMethodOutcome outcome = createMethodOutcomeForDelete(entity.getIdDt().getValue());
// used to exist, so we'll set the persistent id
@ -1145,6 +1145,10 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
validateResourceType(entityToUpdate);
if (isDeleted(entityToUpdate)) {
throw createResourceGoneException(entityToUpdate);
}
IBaseResource resourceToUpdate = toResource(entityToUpdate, false);
IBaseResource destination;
switch (thePatchType) {
@ -1168,6 +1172,10 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
return update(destinationCasted, null, true, theRequest);
}
private boolean isDeleted(BaseHasResource entityToUpdate) {
return entityToUpdate.getDeleted() != null;
}
@PostConstruct
@Override
public void start() {
@ -1208,7 +1216,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
if (!entity.isPresent()) {
throw new ResourceNotFoundException(Msg.code(975) + "No resource found with PID " + thePid);
}
if (entity.get().getDeleted() != null && !theDeletedOk) {
if (isDeleted(entity.get()) && !theDeletedOk) {
throw createResourceGoneException(entity.get());
}
@ -1253,7 +1261,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
T retVal = toResource(myResourceType, entity, null, false);
if (theDeletedOk == false) {
if (entity.getDeleted() != null) {
if (isDeleted(entity)) {
throw createResourceGoneException(entity);
}
}
@ -1808,7 +1816,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
* See SystemProviderR4Test#testTransactionReSavesPreviouslyDeletedResources
* for a test that needs this.
*/
boolean wasDeleted = entity.getDeleted() != null;
boolean wasDeleted = isDeleted(entity);
entity.setDeleted(null);
/*
@ -1888,7 +1896,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
}
assert resourceId.hasVersionIdPart();
boolean wasDeleted = entity.getDeleted() != null;
boolean wasDeleted = isDeleted(entity);
entity.setDeleted(null);
boolean isUpdatingCurrent = resourceId.hasVersionIdPart() && Long.parseLong(resourceId.getVersionIdPart()) == currentEntity.getVersion();
IBasePersistedResource savedEntity = updateHistoryEntity(theRequest, theResource, currentEntity, entity, resourceId, theTransactionDetails, isUpdatingCurrent);

View File

@ -3,6 +3,8 @@ package ca.uhn.fhir.jpa.provider.r4;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import com.google.common.base.Charsets;
import org.apache.commons.io.IOUtils;
import org.apache.http.client.methods.CloseableHttpResponse;
@ -10,13 +12,16 @@ import org.apache.http.client.methods.HttpPatch;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.Binary;
import org.hl7.fhir.r4.model.BooleanType;
import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.CodeType;
import org.hl7.fhir.r4.model.IdType;
import org.hl7.fhir.r4.model.Media;
import org.hl7.fhir.r4.model.Observation;
import org.hl7.fhir.r4.model.OperationOutcome;
import org.hl7.fhir.r4.model.Parameters;
import org.hl7.fhir.r4.model.Patient;
import org.hl7.fhir.r4.model.StringType;
@ -30,6 +35,9 @@ import java.nio.charset.StandardCharsets;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
public class PatchProviderR4Test extends BaseResourceProviderR4Test {
@ -143,6 +151,49 @@ public class PatchProviderR4Test extends BaseResourceProviderR4Test {
assertEquals(false, newPt.getActive());
}
@Test
public void testFhirPatch_AfterDelete_Returns410() {
Patient patient = new Patient();
patient.setActive(true);
patient.addIdentifier().addExtension("http://foo", new StringType("abc"));
patient.addIdentifier().setSystem("sys").setValue("val");
IIdType id = myClient.create().resource(patient).execute().getId().toUnqualifiedVersionless();
OperationOutcome delOutcome = (OperationOutcome) myClient.delete().resourceById(id).execute().getOperationOutcome();
assertTrue(delOutcome.getIssue().get(0).getDiagnostics().contains("Successfully deleted"));
Parameters patch = new Parameters();
Parameters.ParametersParameterComponent operation = patch.addParameter();
operation.setName("operation");
operation
.addPart()
.setName("type")
.setValue(new CodeType("replace"));
operation
.addPart()
.setName("path")
.setValue(new StringType("Patient.active"));
operation
.addPart()
.setName("value")
.setValue(new BooleanType(false));
try {
myClient.patch().withFhirPatch(patch).withId(id).execute();
fail();
} catch (ResourceGoneException e) {
assertEquals(Constants.STATUS_HTTP_410_GONE, e.getStatusCode());
}
try {
myClient.read().resource(Patient.class).withId(id).execute();
fail();
} catch (ResourceGoneException e) {
assertEquals(Constants.STATUS_HTTP_410_GONE, e.getStatusCode());
}
}
@Test
public void testPatchAddArray() throws IOException {
IIdType id;