From 1eeb6112f67aa791d67dc7d92fbd11e3e7c9a7c9 Mon Sep 17 00:00:00 2001 From: Tadgh Date: Mon, 13 Jun 2022 15:11:18 -0400 Subject: [PATCH] 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 Co-authored-by: Ken Stevens --- .../ca/uhn/fhir/model/primitive/IdDt.java | 2 ++ .../hl7/fhir/instance/model/api/IIdType.java | 1 + .../3574-patch-bundle-entry-response.yaml | 5 +++ ...temProviderTransactionSearchDstu3Test.java | 33 ++++++++++++------- .../jpa/dao/BaseTransactionProcessor.java | 21 ++++++++++-- 5 files changed, 49 insertions(+), 13 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3574-patch-bundle-entry-response.yaml diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/primitive/IdDt.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/primitive/IdDt.java index 8b1c5a9843e..a0c10fcc367 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/primitive/IdDt.java +++ b/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, */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. * diff --git a/hapi-fhir-base/src/main/java/org/hl7/fhir/instance/model/api/IIdType.java b/hapi-fhir-base/src/main/java/org/hl7/fhir/instance/model/api/IIdType.java index 538d45b17a5..2cdaa447467 100644 --- a/hapi-fhir-base/src/main/java/org/hl7/fhir/instance/model/api/IIdType.java +++ b/hapi-fhir-base/src/main/java/org/hl7/fhir/instance/model/api/IIdType.java @@ -59,6 +59,7 @@ public interface IIdType extends IPrimitiveType { 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. * diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3574-patch-bundle-entry-response.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_1_0/3574-patch-bundle-entry-response.yaml new file mode 100644 index 00000000000..d2e8b496373 --- /dev/null +++ b/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." + diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderTransactionSearchDstu3Test.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderTransactionSearchDstu3Test.java index d1d0d87d4a5..9a64177a421 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderTransactionSearchDstu3Test.java +++ b/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); - String encodedRequest = myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(input); - ourLog.info("Requet:\n{}", encodedRequest); - post.setEntity(new StringEntity(encodedRequest, ContentType.parse(ca.uhn.fhir.rest.api.Constants.CT_FHIR_JSON_NEW+ Constants.CHARSET_UTF8_CTSUFFIX))); - try (CloseableHttpResponse response = ourHttpClient.execute(post)) { - String responseString = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); - ourLog.info(responseString); - assertEquals(200, response.getStatusLine().getStatusCode()); - assertThat(responseString, containsString("\"resourceType\":\"Bundle\"")); - } + Bundle putBundle = new Bundle(); + putBundle.setType(Bundle.BundleType.TRANSACTION); + putBundle.addEntry() + .setFullUrl(pid1.getValue()) + .setResource(new Patient().setId(pid1.getIdPart())) + .getRequest().setUrl(pid1.getValue()).setMethod(HTTPVerb.PUT); + + Bundle bundle = ourClient.transaction().withBundle(input).execute(); + + //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()); diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java index 09b4595a077..fa5481c676a 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/dao/BaseTransactionProcessor.java +++ b/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); - myVersionAdapter.setResponseLastModified(newEntry, lastModifier); + Date lastModified = getLastModified(theRes); + 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 theConditionalUrlToIdMap, String theMatchUrl, IIdType theId) { if (!StringUtils.isBlank(theMatchUrl)) { theConditionalUrlToIdMap.put(theMatchUrl, theId);