Merge branch 'client_enhancements'

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/exceptions/ResourceGoneException.java

@@ -19,11 +19,13 @@ package ca.uhn.fhir.rest.server.exceptions;
  * limitations under the License.
  * #L%
  */
-import org.hl7.fhir.instance.model.api.*;
 
 import ca.uhn.fhir.model.base.composite.BaseIdentifierDt;
 import ca.uhn.fhir.rest.api.Constants;
 import ca.uhn.fhir.util.CoverageIgnore;
+import org.hl7.fhir.instance.model.api.IBaseOperationOutcome;
+import org.hl7.fhir.instance.model.api.IBaseResource;
+import org.hl7.fhir.instance.model.api.IIdType;
 
 /**
  * Represents an <b>HTTP 410 Resource Gone</b> response, which geenerally
@@ -33,12 +35,12 @@ import ca.uhn.fhir.util.CoverageIgnore;
 public class ResourceGoneException extends BaseServerResponseException {
 
 	public static final int STATUS_CODE = Constants.STATUS_HTTP_410_GONE;
+	private static final long serialVersionUID = 1L;
 
 	/**
 	 * Constructor which creates an error message based on a given resource ID
 	 *
-	 * @param theResourceId
+	 * @param theResourceId The ID of the resource that could not be found
-	 *           The ID of the resource that could not be found
 	 */
 	public ResourceGoneException(IIdType theResourceId) {
 		super(STATUS_CODE, "Resource " + (theResourceId != null ? theResourceId.getValue() : "") + " is gone/deleted");
@@ -56,10 +58,8 @@ public class ResourceGoneException extends BaseServerResponseException {
 	/**
 	 * Constructor which creates an error message based on a given resource ID
 	 *
-	 * @param theClass
+	 * @param theClass      The type of resource that could not be found
-	 *           The type of resource that could not be found
+	 * @param theResourceId The ID of the resource that could not be found
-	 * @param theResourceId
-	 *           The ID of the resource that could not be found
 	 */
 	public ResourceGoneException(Class<? extends IBaseResource> theClass, IIdType theResourceId) {
 		super(STATUS_CODE, "Resource of type " + theClass.getSimpleName() + " with ID " + theResourceId + " is gone/deleted");
@@ -68,19 +68,20 @@ public class ResourceGoneException extends BaseServerResponseException {
 	/**
 	 * Constructor
 	 *
-	 * @param theMessage
+	 * @param theMessage          The message
-	 *           The message
+	 * @param theOperationOutcome The OperationOutcome resource to return to the client
-	 * @param theOperationOutcome
-	 *           The OperationOutcome resource to return to the client
 	 */
 	public ResourceGoneException(String theMessage, IBaseOperationOutcome theOperationOutcome) {
 		super(STATUS_CODE, theMessage, theOperationOutcome);
 	}
 
+	/**
+	 * Constructor
+	 *
+	 * @param theMessage The message
+	 */
 	public ResourceGoneException(String theMessage) {
 		super(STATUS_CODE, theMessage);
 	}
 
-	private static final long serialVersionUID = 1L;
-
 }

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/FifoMemoryPagingProvider.java

@@ -36,7 +36,7 @@ public class FifoMemoryPagingProvider extends BasePagingProvider implements IPag
 		Validate.isTrue(theSize > 0, "theSize must be greater than 0");
 
 		mySize = theSize;
-		myBundleProviders = new LinkedHashMap<String, IBundleProvider>(mySize);
+		myBundleProviders = new LinkedHashMap<>(mySize);
 	}
 
 	@Override

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PageMethodBinding.java

@@ -98,12 +98,16 @@ public class PageMethodBinding extends BaseResourceReturningMethodBinding {
 		}
 
 		if (pageId != null) {
+			// This is a page request by Search ID and Page ID
 
 			resultList = pagingProvider.retrieveResultList(thePagingAction, pageId);
+			validateHaveBundleProvider(thePagingAction, resultList);
 
 		} else {
+			// This is a page request by Search ID and Offset
 
 			resultList = pagingProvider.retrieveResultList(thePagingAction);
+			validateHaveBundleProvider(thePagingAction, resultList);
 
 			offsetI = RestfulServerUtils.tryToExtractNamedParameter(theRequest, Constants.PARAM_PAGINGOFFSET);
 			if (offsetI == null || offsetI < 0) {
@@ -117,13 +121,6 @@ public class PageMethodBinding extends BaseResourceReturningMethodBinding {
 			}
 		}
 
-		// Return an HTTP 409 if the search is not known
-		if (resultList == null) {
-			ourLog.info("Client requested unknown paging ID[{}]", thePagingAction);
-			String msg = getContext().getLocalizer().getMessage(PageMethodBinding.class, "unknownSearchId", thePagingAction);
-			throw new ResourceGoneException(msg);
-		}
-
 		ResponseEncoding responseEncoding = RestfulServerUtils.determineResponseEncodingNoDefault(theRequest, theServer.getDefaultResponseEncoding());
 
 		Set<Include> includes = new HashSet<>();
@@ -160,6 +157,15 @@ public class PageMethodBinding extends BaseResourceReturningMethodBinding {
 		return createBundleFromBundleProvider(theServer, theRequest, count, linkSelf, includes, resultList, start, bundleType, encodingEnum, thePagingAction);
 	}
 
+	private void validateHaveBundleProvider(String thePagingAction, IBundleProvider theBundleProvider) {
+		// Return an HTTP 410 if the search is not known
+		if (theBundleProvider == null) {
+			ourLog.info("Client requested unknown paging ID[{}]", thePagingAction);
+			String msg = getContext().getLocalizer().getMessage(PageMethodBinding.class, "unknownSearchId", thePagingAction);
+			throw new ResourceGoneException(msg);
+		}
+	}
+
 	@Override
 	public RestOperationTypeEnum getRestOperationType() {
 		return RestOperationTypeEnum.GET_PAGE;

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/PagingUsingNamedPagesR4Test.java

@@ -37,6 +37,7 @@ import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.*;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.ArgumentMatchers.nullable;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -49,6 +50,7 @@ public class PagingUsingNamedPagesR4Test {
 
 	private static Server ourServer;
 	private static RestfulServer servlet;
+	private static IBundleProvider ourNextBundleProvider;
 	private IPagingProvider myPagingProvider;
 
 	@Before
@@ -58,6 +60,17 @@ public class PagingUsingNamedPagesR4Test {
 		ourNextBundleProvider = null;
 	}
 
+	private List<IBaseResource> createPatients(int theLow, int theHigh) {
+		List<IBaseResource> patients = new ArrayList<>();
+		for (int id = theLow; id <= theHigh; id++) {
+			Patient pt = new Patient();
+			pt.setId("Patient/" + id);
+			pt.addName().setFamily("FAM" + id);
+			patients.add(pt);
+		}
+		return patients;
+	}
+
 	private Bundle executeAndReturnBundle(HttpGet httpGet, EncodingEnum theExpectEncoding) throws IOException {
 		Bundle bundle;
 		try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
@@ -73,32 +86,6 @@ public class PagingUsingNamedPagesR4Test {
 		return bundle;
 	}
 
-
-	@Test
-	public void testPagingLinksSanitizeBundleType() throws Exception {
-
-		List<IBaseResource> patients0 = createPatients(0, 9);
-		BundleProviderWithNamedPages provider0 = new BundleProviderWithNamedPages(patients0, "SEARCHID0", "PAGEID0", 1000);
-		provider0.setNextPageId("PAGEID1");
-		when(myPagingProvider.retrieveResultList(eq("SEARCHID0"), eq("PAGEID0"))).thenReturn(provider0);
-
-		// Initial search
-		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "?_getpages=SEARCHID0&pageId=PAGEID0&_format=xml&_bundletype=FOO" + UrlUtil.escapeUrlParam("\""));
-		try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
-			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
-			ourLog.info(responseContent);
-			assertThat(responseContent, not(containsString("FOO\"")));
-			assertEquals(200, status.getStatusLine().getStatusCode());
-			EncodingEnum ct = EncodingEnum.forContentType(status.getEntity().getContentType().getValue().replaceAll(";.*", "").trim());
-			assert ct != null;
-			Bundle bundle = EncodingEnum.XML.newParser(ourCtx).parseResource(Bundle.class, responseContent);
-			assertEquals(10, bundle.getEntry().size());
-		}
-
-	}
-
-
-
 	@Test
 	public void testPaging() throws Exception {
 
@@ -132,41 +119,77 @@ public class PagingUsingNamedPagesR4Test {
 		linkSelf = bundle.getLink(Constants.LINK_SELF).getUrl();
 		assertEquals("http://localhost:" + ourPort + "/Patient?_format=xml", linkSelf);
 		linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
-		assertEquals("http://localhost:"+ourPort+"?_getpages=SEARCHID0&pageId=PAGEID1&_format=xml&_bundletype=searchset", linkNext);
+		assertEquals("http://localhost:" + ourPort + "?_getpages=SEARCHID0&_pageId=PAGEID1&_format=xml&_bundletype=searchset", linkNext);
 		assertNull(bundle.getLink(Constants.LINK_PREVIOUS));
 
 		// Fetch the next page
 		httpGet = new HttpGet(linkNext);
 		bundle = executeAndReturnBundle(httpGet, EncodingEnum.XML);
 		linkSelf = bundle.getLink(Constants.LINK_SELF).getUrl();
-		assertEquals("http://localhost:"+ourPort+"?_getpages=SEARCHID0&pageId=PAGEID1&_format=xml&_bundletype=searchset", linkSelf);
+		assertEquals("http://localhost:" + ourPort + "?_getpages=SEARCHID0&_pageId=PAGEID1&_format=xml&_bundletype=searchset", linkSelf);
 		linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
-		assertEquals("http://localhost:"+ourPort+"?_getpages=SEARCHID0&pageId=PAGEID2&_format=xml&_bundletype=searchset", linkNext);
+		assertEquals("http://localhost:" + ourPort + "?_getpages=SEARCHID0&_pageId=PAGEID2&_format=xml&_bundletype=searchset", linkNext);
 		linkPrev = bundle.getLink(Constants.LINK_PREVIOUS).getUrl();
-		assertEquals("http://localhost:"+ourPort+"?_getpages=SEARCHID0&pageId=PAGEID0&_format=xml&_bundletype=searchset", linkPrev);
+		assertEquals("http://localhost:" + ourPort + "?_getpages=SEARCHID0&_pageId=PAGEID0&_format=xml&_bundletype=searchset", linkPrev);
 
 		// Fetch the next page
 		httpGet = new HttpGet(linkNext);
 		bundle = executeAndReturnBundle(httpGet, EncodingEnum.XML);
 		linkSelf = bundle.getLink(Constants.LINK_SELF).getUrl();
-		assertEquals("http://localhost:"+ourPort+"?_getpages=SEARCHID0&pageId=PAGEID2&_format=xml&_bundletype=searchset", linkSelf);
+		assertEquals("http://localhost:" + ourPort + "?_getpages=SEARCHID0&_pageId=PAGEID2&_format=xml&_bundletype=searchset", linkSelf);
 		assertNull(bundle.getLink(Constants.LINK_NEXT));
 		linkPrev = bundle.getLink(Constants.LINK_PREVIOUS).getUrl();
-		assertEquals("http://localhost:"+ourPort+"?_getpages=SEARCHID0&pageId=PAGEID1&_format=xml&_bundletype=searchset", linkPrev);
+		assertEquals("http://localhost:" + ourPort + "?_getpages=SEARCHID0&_pageId=PAGEID1&_format=xml&_bundletype=searchset", linkPrev);
 	}
 
-	private List<IBaseResource> createPatients(int theLow, int theHigh) {
+	@Test
-		List<IBaseResource> patients = new ArrayList<>();
+	public void testPagingLinkUnknownPage() throws Exception {
-		for (int id = theLow; id <= theHigh; id++) {
+
-			Patient pt = new Patient();
+		when(myPagingProvider.retrieveResultList(nullable(String.class))).thenReturn(null);
-			pt.setId("Patient/" + id);
+		when(myPagingProvider.retrieveResultList(nullable(String.class), nullable(String.class))).thenReturn(null);
-			pt.addName().setFamily("FAM" + id);
+
-			patients.add(pt);
+		// With ID
-		}
+		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "?_getpages=SEARCHID0&_pageId=PAGEID0&_format=xml&_bundletype=FOO" + UrlUtil.escapeUrlParam("\""));
-		return patients;
+		try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
+			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
+			ourLog.info(responseContent);
+			assertThat(responseContent, not(containsString("FOO\"")));
+			assertEquals(410, status.getStatusLine().getStatusCode());
 		}
 
+		// Without ID
+		httpGet = new HttpGet("http://localhost:" + ourPort + "?_getpages=SEARCHID0&_format=xml&_bundletype=FOO" + UrlUtil.escapeUrlParam("\""));
+		try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
+			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
+			ourLog.info(responseContent);
+			assertThat(responseContent, not(containsString("FOO\"")));
+			assertEquals(410, status.getStatusLine().getStatusCode());
+		}
 
+	}
+
+	@Test
+	public void testPagingLinksSanitizeBundleType() throws Exception {
+
+		List<IBaseResource> patients0 = createPatients(0, 9);
+		BundleProviderWithNamedPages provider0 = new BundleProviderWithNamedPages(patients0, "SEARCHID0", "PAGEID0", 1000);
+		provider0.setNextPageId("PAGEID1");
+		when(myPagingProvider.retrieveResultList(eq("SEARCHID0"), eq("PAGEID0"))).thenReturn(provider0);
+
+		// Initial search
+		HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "?_getpages=SEARCHID0&_pageId=PAGEID0&_format=xml&_bundletype=FOO" + UrlUtil.escapeUrlParam("\""));
+		try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
+			String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
+			ourLog.info(responseContent);
+			assertThat(responseContent, not(containsString("FOO\"")));
+			assertEquals(200, status.getStatusLine().getStatusCode());
+			EncodingEnum ct = EncodingEnum.forContentType(status.getEntity().getContentType().getValue().replaceAll(";.*", "").trim());
+			assert ct != null;
+			Bundle bundle = EncodingEnum.XML.newParser(ourCtx).parseResource(Bundle.class, responseContent);
+			assertEquals(10, bundle.getEntry().size());
+		}
+
+	}
 
 	@AfterClass
 	public static void afterClassClearContext() throws Exception {
@@ -200,7 +223,6 @@ public class PagingUsingNamedPagesR4Test {
 
 	}
 
-	private static IBundleProvider ourNextBundleProvider;
 	public static class DummyPatientResourceProvider implements IResourceProvider {
 
 
@@ -221,5 +243,4 @@ public class PagingUsingNamedPagesR4Test {
 	}
 
 
-
 }

pom.xml

@@ -96,10 +96,6 @@
 		</dependency>
 	</dependencies>
 
-	<prerequisites>
-		<maven>3.2</maven>
-	</prerequisites>
-
 	<developers>
 		<developer>
 			<id>jamesagnew</id>
@@ -1623,22 +1619,19 @@
 			<plugin>
 				<groupId>org.apache.maven.plugins</groupId>
 				<artifactId>maven-enforcer-plugin</artifactId>
-				<version>1.4.1</version>
+				<version>3.0.0-M2</version>
 				<executions>
 					<execution>
-						<id>enforce-java</id>
+						<id>enforce-maven</id>
 						<goals>
 							<goal>enforce</goal>
 						</goals>
-					</execution>
-				</executions>
 						<configuration>
 							<rules>
 								<requireMavenVersion>
-							<version>3.3</version>
+									<version>3.3.1</version>
 								</requireMavenVersion>
 								<requireJavaVersion>
-							<!--<version>[1.8,)</version>-->
 									<version>1.8</version>
 									<message>
 										The hapi-fhir Maven build requires JDK version 1.8.x.
@@ -1646,6 +1639,8 @@
 								</requireJavaVersion>
 							</rules>
 						</configuration>
+					</execution>
+				</executions>
 			</plugin>
 			<plugin>
 				<groupId>org.codehaus.mojo</groupId>