diff --git a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties index 14644b67b34..d3c97083bf9 100644 --- a/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties +++ b/hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties @@ -114,3 +114,5 @@ ca.uhn.fhir.jpa.dao.dstu3.FhirResourceDaoConceptMapDstu3.matchesFound=Matches fo ca.uhn.fhir.jpa.dao.dstu3.FhirResourceDaoConceptMapDstu3.noMatchesFound=No matches found! ca.uhn.fhir.jpa.dao.r4.FhirResourceDaoConceptMapR4.matchesFound=Matches found! ca.uhn.fhir.jpa.dao.r4.FhirResourceDaoConceptMapR4.noMatchesFound=No matches found! + +ca.uhn.fhir.jpa.util.jsonpatch.JsonPatchUtils.failedToApplyPatch=Failed to apply JSON patch to {0}: {1} diff --git a/hapi-fhir-jpaserver-base/pom.xml b/hapi-fhir-jpaserver-base/pom.xml index 947ea4de6e8..23a633eb21f 100644 --- a/hapi-fhir-jpaserver-base/pom.xml +++ b/hapi-fhir-jpaserver-base/pom.xml @@ -529,6 +529,12 @@ guava-testlib test + + org.jetbrains + annotations + 16.0.3 + compile + diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/jsonpatch/JsonPatchUtils.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/jsonpatch/JsonPatchUtils.java index 8a1c0a2e64d..d1125eca149 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/jsonpatch/JsonPatchUtils.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/jsonpatch/JsonPatchUtils.java @@ -21,6 +21,9 @@ package ca.uhn.fhir.jpa.util.jsonpatch; */ import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.parser.DataFormatException; +import ca.uhn.fhir.parser.IParser; +import ca.uhn.fhir.parser.StrictErrorHandler; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonParser; @@ -29,13 +32,15 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.github.fge.jsonpatch.JsonPatch; import com.github.fge.jsonpatch.JsonPatchException; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.intellij.lang.annotations.Language; import java.io.IOException; -import java.io.StringReader; + +import static org.apache.commons.lang3.StringUtils.defaultString; public class JsonPatchUtils { - public static T apply(FhirContext theCtx, T theResourceToUpdate, String thePatchBody) { + public static T apply(FhirContext theCtx, T theResourceToUpdate, @Language("JSON") String thePatchBody) { // Parse the patch ObjectMapper mapper = new ObjectMapper(); mapper.configure(JsonParser.Feature.INCLUDE_SOURCE_IN_LOCATION, false); @@ -54,7 +59,21 @@ public class JsonPatchUtils { @SuppressWarnings("unchecked") Class clazz = (Class) theResourceToUpdate.getClass(); - T retVal = theCtx.newJsonParser().parseResource(clazz, mapper.writeValueAsString(after)); + String postPatchedContent = mapper.writeValueAsString(after); + + IParser fhirJsonParser = theCtx.newJsonParser(); + fhirJsonParser.setParserErrorHandler(new StrictErrorHandler()); + + T retVal; + try { + retVal = fhirJsonParser.parseResource(clazz, postPatchedContent); + } catch (DataFormatException e) { + String resourceId = theResourceToUpdate.getIdElement().toUnqualifiedVersionless().getValue(); + String resourceType = theCtx.getResourceDefinition(theResourceToUpdate).getName(); + resourceId = defaultString(resourceId, resourceType); + String msg = theCtx.getLocalizer().getMessage(JsonPatchUtils.class, "failedToApplyPatch", resourceId, e.getMessage()); + throw new InvalidRequestException(msg); + } return retVal; } catch (IOException | JsonPatchException theE) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatchProviderR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatchProviderR4Test.java index fd7c70dddcd..6311dd10e29 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatchProviderR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/PatchProviderR4Test.java @@ -7,10 +7,14 @@ import org.apache.http.client.methods.HttpPatch; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.Media; import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.Patient; import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.io.IOException; import java.nio.charset.StandardCharsets; import static org.hamcrest.CoreMatchers.containsString; @@ -19,6 +23,51 @@ import static org.junit.Assert.assertThat; public class PatchProviderR4Test extends BaseResourceProviderR4Test { + + private static final Logger ourLog = LoggerFactory.getLogger(PatchProviderR4Test.class); + + @Test + public void testPatchAddArray() throws IOException { + IIdType id; + { + Media media = new Media(); + media.setId("465eb73a-bce3-423a-b86e-5d0d267638f4"); + media.setDuration(100L); + myMediaDao.update(media); + + Observation obs = new Observation(); + obs.addIdentifier().setSystem("urn:system").setValue("0"); + id = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless(); + } + + + String patchText = "[ " + + " {" + + " \"op\": \"add\"," + + " \"path\": \"/derivedFrom\"," + + " \"value\": [" + + " {\"reference\": \"/Media/465eb73a-bce3-423a-b86e-5d0d267638f4\"}" + + " ]" + + " } " + + "]"; + + HttpPatch patch = new HttpPatch(ourServerBase + "/Observation/" + id.getIdPart()); + patch.setEntity(new StringEntity(patchText, ContentType.parse(Constants.CT_JSON_PATCH + Constants.CHARSET_UTF8_CTSUFFIX))); + patch.addHeader(Constants.HEADER_PREFER, Constants.HEADER_PREFER_RETURN + "=" + Constants.HEADER_PREFER_RETURN_REPRESENTATION); + patch.addHeader(Constants.HEADER_ACCEPT, Constants.CT_FHIR_JSON); + + CloseableHttpResponse response = ourHttpClient.execute(patch); + try { + assertEquals(200, response.getStatusLine().getStatusCode()); + String responseString = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + ourLog.info("Response:\n{}", responseString); + assertThat(responseString, containsString("\"derivedFrom\":[{\"reference\":\"Media/465eb73a-bce3-423a-b86e-5d0d267638f4\"}]")); + } finally { + response.close(); + } + + } + @Test public void testPatchUsingJsonPatch() throws Exception { String methodName = "testPatchUsingJsonPatch"; @@ -186,5 +235,4 @@ public class PatchProviderR4Test extends BaseResourceProviderR4Test { } - } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/jsonpatch/JsonPatchUtilsTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/jsonpatch/JsonPatchUtilsTest.java index 96c0b92bf4f..a09fbcb2dc9 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/jsonpatch/JsonPatchUtilsTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/util/jsonpatch/JsonPatchUtilsTest.java @@ -11,8 +11,7 @@ import org.springframework.transaction.PlatformTransactionManager; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; public class JsonPatchUtilsTest extends BaseJpaTest { @@ -48,12 +47,12 @@ public class JsonPatchUtilsTest extends BaseJpaTest { public void testInvalidPatchSyntaxError() { // Quotes are incorrect in the "value" body - String patchText = "[ {\n" + - " \"comment\": \"add image to examination\",\n" + - " \"patch\": [ {\n" + - " \"op\": \"foo\",\n" + - " \"path\": \"/derivedFrom/-\",\n" + - " \"value\": [{\"reference\": \"/Media/465eb73a-bce3-423a-b86e-5d0d267638f4\"}]\n" + + String patchText = "[ {" + + " \"comment\": \"add image to examination\"," + + " \"patch\": [ {" + + " \"op\": \"foo\"," + + " \"path\": \"/derivedFrom/-\"," + + " \"value\": [{\"reference\": \"/Media/465eb73a-bce3-423a-b86e-5d0d267638f4\"}]" + " } ]\n" + " } ]"; @@ -69,6 +68,52 @@ public class JsonPatchUtilsTest extends BaseJpaTest { } + + @Test + public void testPatchAddArray() { + + String patchText = "[ " + + " {" + + " \"op\": \"add\"," + + " \"path\": \"/derivedFrom\"," + + " \"value\": [" + + " {\"reference\": \"/Media/465eb73a-bce3-423a-b86e-5d0d267638f4\"}" + + " ]" + + " } " + + "]"; + + Observation toUpdate = new Observation(); + toUpdate = JsonPatchUtils.apply(ourCtx, toUpdate, patchText); + + String outcome = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(toUpdate); + ourLog.info(outcome); + + assertThat(outcome, containsString("\"reference\": \"Media/465eb73a-bce3-423a-b86e-5d0d267638f4\"")); + } + + @Test + public void testPatchAddInvalidElement() { + + String patchText = "[ " + + " {" + + " \"op\": \"add\"," + + " \"path\": \"/derivedFromXXX\"," + + " \"value\": [" + + " {\"reference\": \"/Media/465eb73a-bce3-423a-b86e-5d0d267638f4\"}" + + " ]" + + " } " + + "]"; + + Observation toUpdate = new Observation(); + try { + JsonPatchUtils.apply(ourCtx, toUpdate, patchText); + fail(); + } catch (InvalidRequestException e) { + assertEquals("Failed to apply JSON patch to Observation: Unknown element 'derivedFromXXX' found during parse", e.getMessage()); + } + + } + @Override protected FhirContext getContext() { return ourCtx; diff --git a/pom.xml b/pom.xml index f468c8fe811..cb312c07a82 100755 --- a/pom.xml +++ b/pom.xml @@ -544,7 +544,8 @@ 2.3.0.1 2.3.1 2.25.1 - 9.4.17.v20190418 + + 9.4.14.v20181114 3.0.2 5.4.2.Final diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 446454c1afd..53b90e50369 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -19,7 +19,7 @@
  • Spring-Data (JPA): 2.1.3.RELEASE -> 2.1.6.RELEASE
  • Caffeine (JPA): 2.6.2 -> 2.7.0
  • JANSI (CLI): 1.16 -> 1.17.1
  • -
  • Jetty (CLI): 9.4.14.v20181114 -> 9.4.17.v20190418
  • + ]]> @@ -181,6 +181,11 @@ Ensure that database cursors are closed immediately after performing a FHIR search. + + When performing a JSON Patch in JPA server, the post-patched document is now validated to + ensure that the patch was valid for the candidate resource. This means that invalid patches + are caught and not just silently ignored. +