3788 FHIR patch on soft deleted resource undeletes resource (hapifhir#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 <[email protected]>

Co-authored-by: Justin_Dar <[email protected]>
Co-authored-by: jmarchionatto <[email protected]>

Author: 3 people

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

@@ -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."

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java

@@ -550,7 +550,7 @@ public DaoMethodOutcome delete(IIdType theId,
 		}
 
 		// 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 @@ private DaoMethodOutcome doPatch(IIdType theId, String theConditionalUrl, PatchT
 
 		validateResourceType(entityToUpdate);
 
+		if (isDeleted(entityToUpdate)) {
+			throw createResourceGoneException(entityToUpdate);
+		}
+
 		IBaseResource resourceToUpdate = toResource(entityToUpdate, false);
 		IBaseResource destination;
 		switch (thePatchType) {
@@ -1168,6 +1172,10 @@ private DaoMethodOutcome doPatch(IIdType theId, String theConditionalUrl, PatchT
 		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 T readByPid(ResourcePersistentId thePid, boolean theDeletedOk) {
 		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 T doRead(IIdType theId, RequestDetails theRequest, boolean theDeletedOk)
 		T retVal = toResource(myResourceType, entity, null, false);
 
 		if (theDeletedOk == false) {
-			if (entity.getDeleted() != null) {
+			if (isDeleted(entity)) {
 				throw createResourceGoneException(entity);
 			}
 		}
@@ -1808,7 +1816,7 @@ private DaoMethodOutcome doUpdate(T theResource, String theMatchUrl, boolean the
 		 * 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 @@ private DaoMethodOutcome doUpdateWithHistoryRewrite(T theResource, RequestDetail
 		}
 		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);

hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatchProviderR4Test.java

@@ -3,20 +3,25 @@
 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;
 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 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 void testFhirPatch_TransactionWithSearchParameter() throws Exception {
 		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;