Fix Patch in bundle to return Response Entry (#3678)

* Refactor test to cause failure

* Handle patch for entry response building

* add changelog

* Update hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java

Co-authored-by: Ken Stevens <khstevens@gmail.com>

Co-authored-by: Ken Stevens <khstevens@gmail.com>

hapi-fhir-base/src/main/java/ca/uhn/fhir/model/primitive/IdDt.java

@@ -15,6 +15,7 @@ import org.hl7.fhir.instance.model.api.IAnyResource;
 import org.hl7.fhir.instance.model.api.IBaseResource;
 import org.hl7.fhir.instance.model.api.IIdType;
 
+import javax.annotation.Nonnull;
 import java.math.BigDecimal;
 import java.util.UUID;
 
@@ -286,6 +287,7 @@ public class IdDt extends UriDt implements /*IPrimitiveDatatype<String>, */IIdTy
 		return myResourceType;
 	}
 
+
 	/**
 	 * Returns the value of this ID. Note that this value may be a fully qualified URL, a relative/partial URL, or a simple ID. Use {@link #getIdPart()} to get just the ID portion.
 	 *

hapi-fhir-base/src/main/java/org/hl7/fhir/instance/model/api/IIdType.java

@@ -59,6 +59,7 @@ public interface IIdType extends IPrimitiveType<String> {
 
 	String getResourceType();
 
+
 	/**
 	 * Returns the value of this ID. Note that this value may be a fully qualified URL, a relative/partial URL, or a simple ID. Use {@link #getIdPart()} to get just the ID portion.
 	 * 

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3574-patch-bundle-entry-response.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 3574
+title: "Previously, PATCH operations that were contained in a transaction bundle would not return response entries. This has been corrected."
+

hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderTransactionSearchDstu3Test.java

@@ -42,11 +42,15 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
+import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.nullValue;
 import static org.hamcrest.Matchers.startsWith;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -208,11 +212,11 @@ public class SystemProviderTransactionSearchDstu3Test extends BaseJpaDstu3Test {
 		}
 
 		String patchString = "[ { \"op\":\"replace\", \"path\":\"/active\", \"value\":false } ]";
+
 		Binary patch = new Binary();
 		patch.setContentType(ca.uhn.fhir.rest.api.Constants.CT_JSON_PATCH);
 		patch.setContent(patchString.getBytes(Charsets.UTF_8));
 
-		// Note that we don't set the type
 		Bundle input = new Bundle();
 		input.setType(Bundle.BundleType.TRANSACTION);
 		input.addEntry()
@@ -220,16 +224,23 @@ public class SystemProviderTransactionSearchDstu3Test extends BaseJpaDstu3Test {
 			.setResource(patch)
 			.getRequest().setUrl(pid1.getValue());
 
-		HttpPost post = new HttpPost(ourServerBase);
+		Bundle putBundle = new Bundle();
-		String encodedRequest = myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(input);
+		putBundle.setType(Bundle.BundleType.TRANSACTION);
-		ourLog.info("Requet:\n{}", encodedRequest);
+		putBundle.addEntry()
-		post.setEntity(new StringEntity(encodedRequest, ContentType.parse(ca.uhn.fhir.rest.api.Constants.CT_FHIR_JSON_NEW+ Constants.CHARSET_UTF8_CTSUFFIX)));
+			.setFullUrl(pid1.getValue())
-		try (CloseableHttpResponse response = ourHttpClient.execute(post)) {
+			.setResource(new Patient().setId(pid1.getIdPart()))
-			String responseString = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8);
+			.getRequest().setUrl(pid1.getValue()).setMethod(HTTPVerb.PUT);
-			ourLog.info(responseString);
+
-			assertEquals(200, response.getStatusLine().getStatusCode());
+		Bundle bundle = ourClient.transaction().withBundle(input).execute();
-			assertThat(responseString, containsString("\"resourceType\":\"Bundle\""));
+
-		}
+		//Validate over all bundle response entry contents.
+		assertThat(bundle.getType(), is(equalTo(Bundle.BundleType.TRANSACTIONRESPONSE)));
+		assertThat(bundle.getEntry(), hasSize(1));
+		Bundle.BundleEntryResponseComponent response = bundle.getEntry().get(0).getResponse();
+		assertThat(response.getStatus(), is(equalTo("200 OK")));
+		assertThat(response.getEtag(), is(notNullValue()));
+		assertThat(response.getLastModified(), is(notNullValue()));
+		assertThat(response.getLocation(), is(equalTo(pid1.getValue() + "/_history/2")));
 
 		Patient newPt = ourClient.read().resource(Patient.class).withId(pid1.getIdPart()).execute();
 		assertEquals("2", newPt.getIdElement().getVersionIdPart());

hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java

@@ -282,13 +282,18 @@ public abstract class BaseTransactionProcessor {
 
 		populateIdToPersistedOutcomeMap(idToPersistedOutcome, newId, outcome);
 
+		if(shouldSwapBinaryToActualResource(theRes, theResourceType, nextResourceId)) {
+			theRes = idToPersistedOutcome.get(newId).getResource();
+			theResourceType = idToPersistedOutcome.get(newId).getResource().fhirType();
+		}
+
 		if (outcome.getCreated()) {
 			myVersionAdapter.setResponseStatus(newEntry, toStatusString(Constants.STATUS_HTTP_201_CREATED));
 		} else {
 			myVersionAdapter.setResponseStatus(newEntry, toStatusString(Constants.STATUS_HTTP_200_OK));
 		}
-		Date lastModifier = getLastModified(theRes);
+		Date lastModified = getLastModified(theRes);
-		myVersionAdapter.setResponseLastModified(newEntry, lastModifier);
+		myVersionAdapter.setResponseLastModified(newEntry, lastModified);
 
 		if (theRequestDetails != null) {
 			String prefer = theRequestDetails.getHeader(Constants.HEADER_PREFER);
@@ -1089,6 +1094,10 @@ public abstract class BaseTransactionProcessor {
 						if (outcome.getResource() != null) {
 							updatedResources.add(outcome.getResource());
 						}
+						if (nextResourceId != null) {
+							handleTransactionCreateOrUpdateOutcome(theIdSubstitutions, theIdToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequest);
+						}
+						entriesToProcess.put(nextRespEntry, outcome.getId());
 
 						break;
 					}
@@ -1188,6 +1197,14 @@ public abstract class BaseTransactionProcessor {
 		}
 	}
 
+	private boolean shouldSwapBinaryToActualResource(IBaseResource theResource, String theResourceType, IIdType theNextResourceId) {
+		if ("Binary".equalsIgnoreCase(theResourceType) && theNextResourceId.getResourceType() != null && !theNextResourceId.getResourceType().equalsIgnoreCase("Binary")) {
+			return true;
+		} else {
+			return false;
+		}
+	}
+
 	private void setConditionalUrlToBeValidatedLater(Map<String, IIdType> theConditionalUrlToIdMap, String theMatchUrl, IIdType theId) {
 		if (!StringUtils.isBlank(theMatchUrl)) {
 			theConditionalUrlToIdMap.put(theMatchUrl, theId);