Merge branch 'master' of github.com:jamesagnew/hapi-fhir

hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java

@@ -11,9 +11,9 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank;
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
- * 
+ *
  *      http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -24,6 +24,7 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 
 import org.apache.commons.lang3.tuple.Pair;
 import org.hl7.fhir.instance.model.api.*;
@@ -50,7 +51,7 @@ public class BundleUtil {
 			BaseRuntimeChildDefinition relChild = relDef.getChildByName("relation");
 			List<IBase> relValues = relChild.getAccessor().getValues(nextLink);
 			for (IBase next : relValues) {
-				IPrimitiveType<?> nextValue = (IPrimitiveType<?>)next;
+				IPrimitiveType<?> nextValue = (IPrimitiveType<?>) next;
 				if (theLinkRelation.equals(nextValue.getValueAsString())) {
 					isRightRel = true;
 				}
@@ -64,7 +65,7 @@ public class BundleUtil {
 			BaseRuntimeChildDefinition urlChild = linkDef.getChildByName("url");
 			List<IBase> values = urlChild.getAccessor().getValues(nextLink);
 			for (IBase nextUrl : values) {
-				IPrimitiveType<?> nextValue = (IPrimitiveType<?>)nextUrl;
+				IPrimitiveType<?> nextValue = (IPrimitiveType<?>) nextUrl;
 				if (isNotBlank(nextValue.getValueAsString())) {
 					return nextValue.getValueAsString();
 				}
@@ -83,35 +84,35 @@ public class BundleUtil {
 
 		BaseRuntimeElementCompositeDefinition<?> entryChildElem = (BaseRuntimeElementCompositeDefinition<?>) entryChild.getChildByName("entry");
 		BaseRuntimeChildDefinition resourceChild = entryChildElem.getChildByName("resource");
-		
+
 		BaseRuntimeChildDefinition requestChild = entryChildElem.getChildByName("request");
 		BaseRuntimeElementCompositeDefinition<?> requestDef = (BaseRuntimeElementCompositeDefinition<?>) requestChild.getChildByName("request");
-		
+
 		BaseRuntimeChildDefinition urlChild = requestDef.getChildByName("url");
 
 		List<Pair<String, IBaseResource>> retVal = new ArrayList<>(entries.size());
 		for (IBase nextEntry : entries) {
-			
+
 			String url = null;
 			IBaseResource resource = null;
-			
+
 			for (IBase nextEntryValue : requestChild.getAccessor().getValues(nextEntry)) {
 				for (IBase nextUrlValue : urlChild.getAccessor().getValues(nextEntryValue)) {
-					url = ((IPrimitiveType<String>)nextUrlValue).getValue();
+					url = ((IPrimitiveType<String>) nextUrlValue).getValue();
 				}
 			}
-			
+
 			// Should return 0..1 only
 			for (IBase nextValue : resourceChild.getAccessor().getValues(nextEntry)) {
 				resource = (IBaseResource) nextValue;
 			}
-			
+
 			retVal.add(Pair.of(url, resource));
 		}
-		
+
-		return retVal;		
+		return retVal;
 	}
-	
+
 	public static String getBundleType(FhirContext theContext, IBaseBundle theBundle) {
 		RuntimeResourceDefinition def = theContext.getResourceDefinition(theBundle);
 		BaseRuntimeChildDefinition entryChild = def.getChildByName("type");
@@ -147,13 +148,13 @@ public class BundleUtil {
 		List<IBase> entries = entryChild.getAccessor().getValues(theBundle);
 
 		BaseRuntimeElementCompositeDefinition<?> entryChildElem = (BaseRuntimeElementCompositeDefinition<?>) entryChild.getChildByName("entry");
-		
+
 		BaseRuntimeChildDefinition resourceChild = entryChildElem.getChildByName("resource");
 		BaseRuntimeChildDefinition requestChild = entryChildElem.getChildByName("request");
-		BaseRuntimeElementCompositeDefinition<?>  requestElem = (BaseRuntimeElementCompositeDefinition<?>) requestChild.getChildByName("request");
+		BaseRuntimeElementCompositeDefinition<?> requestElem = (BaseRuntimeElementCompositeDefinition<?>) requestChild.getChildByName("request");
 		BaseRuntimeChildDefinition urlChild = requestElem.getChildByName("url");
 		BaseRuntimeChildDefinition methodChild = requestElem.getChildByName("method");
-		
+
 		for (IBase nextEntry : entries) {
 			IBaseResource resource = null;
 			String url = null;
@@ -164,39 +165,40 @@ public class BundleUtil {
 			}
 			for (IBase nextRequest : requestChild.getAccessor().getValues(nextEntry)) {
 				for (IBase nextUrl : urlChild.getAccessor().getValues(nextRequest)) {
-					url = ((IPrimitiveType<?>)nextUrl).getValueAsString();
+					url = ((IPrimitiveType<?>) nextUrl).getValueAsString();
 				}
 				for (IBase nextUrl : methodChild.getAccessor().getValues(nextRequest)) {
-					String methodString = ((IPrimitiveType<?>)nextUrl).getValueAsString();
+					String methodString = ((IPrimitiveType<?>) nextUrl).getValueAsString();
 					if (isNotBlank(methodString)) {
 						requestType = RequestTypeEnum.valueOf(methodString);
 					}
 				}
 			}
 
-			/* 
+			/*
 			 * All 3 might be null - That's ok because we still want to know the
 			 * order in the original bundle.
 			 */
 			retVal.add(new BundleEntryParts(requestType, url, resource));
 		}
 
-		
+
 		return retVal;
 	}
-	
+
 	/**
 	 * Extract all of the resources from a given bundle
 	 */
 	public static List<IBaseResource> toListOfResources(FhirContext theContext, IBaseBundle theBundle) {
-		return toListOfResourcesOfType(theContext, theBundle, null);
+		return toListOfResourcesOfType(theContext, theBundle, IBaseResource.class);
 	}
 
 	/**
-	 * Extract all of the resources of a given type from a given bundle 
+	 * Extract all of the resources of a given type from a given bundle
 	 */
 	@SuppressWarnings("unchecked")
 	public static <T extends IBaseResource> List<T> toListOfResourcesOfType(FhirContext theContext, IBaseBundle theBundle, Class<T> theTypeToInclude) {
+		Objects.requireNonNull(theTypeToInclude, "ResourceType must not be null");
 		List<T> retVal = new ArrayList<>();
 
 		RuntimeResourceDefinition def = theContext.getResourceDefinition(theBundle);
@@ -207,36 +209,36 @@ public class BundleUtil {
 		BaseRuntimeChildDefinition resourceChild = entryChildElem.getChildByName("resource");
 		for (IBase nextEntry : entries) {
 			for (IBase next : resourceChild.getAccessor().getValues(nextEntry)) {
-				if (theTypeToInclude != null && !theTypeToInclude.isAssignableFrom(next.getClass())) {
+				if (theTypeToInclude.isAssignableFrom(next.getClass())) {
-					continue;
+					retVal.add((T) next);
 				}
-				retVal.add((T) next);
 			}
 		}
-
 		return retVal;
 	}
 
-	public static class BundleEntryParts
+	public static class BundleEntryParts {
-	{
 		private final RequestTypeEnum myRequestType;
 		private final IBaseResource myResource;
 		private final String myUrl;
+
 		BundleEntryParts(RequestTypeEnum theRequestType, String theUrl, IBaseResource theResource) {
 			super();
 			myRequestType = theRequestType;
 			myUrl = theUrl;
 			myResource = theResource;
 		}
+
 		public RequestTypeEnum getRequestType() {
 			return myRequestType;
 		}
+
 		public IBaseResource getResource() {
 			return myResource;
 		}
+
 		public String getUrl() {
 			return myUrl;
 		}
 	}
-
 }

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}

hapi-fhir-jpaserver-base/pom.xml

@@ -529,6 +529,12 @@
 			<artifactId>guava-testlib</artifactId>
 			<scope>test</scope>
 		</dependency>
+		<dependency>
+			<groupId>org.jetbrains</groupId>
+			<artifactId>annotations</artifactId>
+			<version>16.0.3</version>
+			<scope>compile</scope>
+		</dependency>
 
 	</dependencies>
 

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 extends IBaseResource> T apply(FhirContext theCtx, T theResourceToUpdate, String thePatchBody) {
+	public static <T extends IBaseResource> 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<T> clazz = (Class<T>) 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) {

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 {
 	}
 
 
-
 }

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.*;
-import static org.junit.Assert.fail;
 
 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" +
+		String patchText = "[ {" +
-			"        \"comment\": \"add image to examination\",\n" +
+			"        \"comment\": \"add image to examination\"," +
-			"        \"patch\": [ {\n" +
+			"        \"patch\": [ {" +
-			"            \"op\": \"foo\",\n" +
+			"            \"op\": \"foo\"," +
-			"            \"path\": \"/derivedFrom/-\",\n" +
+			"            \"path\": \"/derivedFrom/-\"," +
-			"            \"value\": [{\"reference\": \"/Media/465eb73a-bce3-423a-b86e-5d0d267638f4\"}]\n" +
+			"            \"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;

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/BundleUtilTest.java

@@ -2,10 +2,13 @@ package ca.uhn.fhir.util;
 
 import ca.uhn.fhir.context.FhirContext;
 import org.hl7.fhir.r4.model.Bundle;
+import org.hl7.fhir.r4.model.Patient;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.List;
+
 public class BundleUtilTest {
 
 	private static FhirContext ourCtx = FhirContext.forR4();
@@ -38,6 +41,17 @@ public class BundleUtilTest {
 		Assert.assertEquals(null, BundleUtil.getTotal(ourCtx, b));
 	}
 
+	@Test
+	public void toListOfResourcesOfTypeTest() {
+		Bundle bundle = new Bundle();
+		for (int i = 0; i < 5; i++) {
+			bundle.addEntry(new Bundle.BundleEntryComponent().setResource(new Patient()));
+		}
+		List<Patient> list = BundleUtil.toListOfResourcesOfType(ourCtx, bundle, Patient.class);
+		Assert.assertEquals(5, list.size());
+	}
+
+
 	@AfterClass
 	public static void afterClassClearContext() {
 		TestUtil.clearAllStaticFieldsForUnitTest();

pom.xml

@@ -544,7 +544,8 @@
 		<jaxb_core_version>2.3.0.1</jaxb_core_version>
 		<jaxb_runtime_version>2.3.1</jaxb_runtime_version>
 		<jersey_version>2.25.1</jersey_version>
-		<jetty_version>9.4.17.v20190418</jetty_version>
+		<!-- 9.4.17 seems to have issues -->
+		<jetty_version>9.4.14.v20181114</jetty_version>
 		<jsr305_version>3.0.2</jsr305_version>
 		<!--<hibernate_version>5.2.10.Final</hibernate_version>-->
 		<hibernate_version>5.4.2.Final</hibernate_version>

src/changes/changes.xml

@@ -19,7 +19,7 @@
 					<li>Spring-Data (JPA): 2.1.3.RELEASE -&gt; 2.1.6.RELEASE</li>
 					<li>Caffeine (JPA): 2.6.2 -&gt; 2.7.0</li>
 					<li>JANSI (CLI): 1.16 -&gt; 1.17.1</li>
-					<li>Jetty (CLI): 9.4.14.v20181114 -&gt; 9.4.17.v20190418</li>
+					<!--<li>Jetty (CLI): 9.4.14.v20181114 -&gt; 9.4.17.v20190418</li>-->
 				</ul>
 				]]>
 			</action>
@@ -181,6 +181,11 @@
 			<action type="fix">
 				Ensure that database cursors are closed immediately after performing a FHIR search.
 			</action>
+			<action type="add">
+				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.
+			</action>
 			<action type="add">
 				Expunges are now done in batches in multiple threads.  Both the number of expunge threads and batch size are configurable
 				in DaoConfig.