From fc9f2ec0fbedc18b433e09d91b9af43e8a32f36a Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sun, 29 Jul 2018 18:20:09 -0400 Subject: [PATCH] FIx test issue --- .../exceptions/ResourceGoneException.java | 39 ++++--- .../rest/server/FifoMemoryPagingProvider.java | 2 +- .../rest/server/method/PageMethodBinding.java | 20 ++-- .../server/PagingUsingNamedPagesR4Test.java | 108 +++++++++++------- pom.xml | 35 +++--- 5 files changed, 113 insertions(+), 91 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/exceptions/ResourceGoneException.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/exceptions/ResourceGoneException.java index 93d6a62a6fa..f9494cdb9e5 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/exceptions/ResourceGoneException.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/exceptions/ResourceGoneException.java @@ -9,9 +9,9 @@ package ca.uhn.fhir.rest.server.exceptions; * 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. @@ -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 HTTP 410 Resource Gone 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 - * The ID of the resource that could not be found + * + * @param theResourceId 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"); @@ -46,7 +48,7 @@ public class ResourceGoneException extends BaseServerResponseException { /** * @deprecated This constructor has a dependency on a specific model version and will be removed. Deprecated in HAPI - * 1.6 - 2016-07-02 + * 1.6 - 2016-07-02 */ @Deprecated public ResourceGoneException(Class theClass, BaseIdentifierDt thePatientId) { @@ -55,11 +57,9 @@ public class ResourceGoneException extends BaseServerResponseException { /** * Constructor which creates an error message based on a given resource ID - * - * @param theClass - * The type of resource that could not be found - * @param theResourceId - * The ID of the resource that could not be found + * + * @param theClass The type of resource that could not be found + * @param theResourceId The ID of the resource that could not be found */ public ResourceGoneException(Class theClass, IIdType theResourceId) { super(STATUS_CODE, "Resource of type " + theClass.getSimpleName() + " with ID " + theResourceId + " is gone/deleted"); @@ -67,20 +67,21 @@ public class ResourceGoneException extends BaseServerResponseException { /** * Constructor - * - * @param theMessage - * The message - * @param theOperationOutcome - * The OperationOutcome resource to return to the client + * + * @param theMessage The message + * @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; - } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/FifoMemoryPagingProvider.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/FifoMemoryPagingProvider.java index f7c1a907d48..8e4ee4dacb7 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/FifoMemoryPagingProvider.java +++ b/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(mySize); + myBundleProviders = new LinkedHashMap<>(mySize); } @Override diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PageMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PageMethodBinding.java index 22e197fbb81..d7ce7a2b198 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PageMethodBinding.java +++ b/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 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; diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/PagingUsingNamedPagesR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/PagingUsingNamedPagesR4Test.java index e06c82128f3..2249738da8e 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/PagingUsingNamedPagesR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/PagingUsingNamedPagesR4Test.java @@ -36,8 +36,8 @@ import java.util.concurrent.TimeUnit; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; import static org.junit.Assert.*; -import static org.mockito.ArgumentMatchers.contains; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -50,6 +50,7 @@ public class PagingUsingNamedPagesR4Test { private static Server ourServer; private static RestfulServer servlet; + private static IBundleProvider ourNextBundleProvider; private IPagingProvider myPagingProvider; @Before @@ -59,6 +60,17 @@ public class PagingUsingNamedPagesR4Test { ourNextBundleProvider = null; } + private List createPatients(int theLow, int theHigh) { + List 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)) { @@ -74,32 +86,6 @@ public class PagingUsingNamedPagesR4Test { return bundle; } - - @Test - public void testPagingLinksSanitizeBundleType() throws Exception { - - List 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 { @@ -131,43 +117,79 @@ public class PagingUsingNamedPagesR4Test { httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_format=xml"); bundle = executeAndReturnBundle(httpGet, EncodingEnum.XML); linkSelf = bundle.getLink(Constants.LINK_SELF).getUrl(); - assertEquals("http://localhost:"+ourPort+"/Patient?_format=xml", linkSelf); + 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 createPatients(int theLow, int theHigh) { - List 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); + @Test + public void testPagingLinkUnknownPage() throws Exception { + + when(myPagingProvider.retrieveResultList(nullable(String.class))).thenReturn(null); + when(myPagingProvider.retrieveResultList(nullable(String.class), nullable(String.class))).thenReturn(null); + + // With ID + 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(410, status.getStatusLine().getStatusCode()); } - return patients; + + // 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 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 { @@ -201,7 +223,6 @@ public class PagingUsingNamedPagesR4Test { } - private static IBundleProvider ourNextBundleProvider; public static class DummyPatientResourceProvider implements IResourceProvider { @@ -222,5 +243,4 @@ public class PagingUsingNamedPagesR4Test { } - } diff --git a/pom.xml b/pom.xml index 4e4402a2b77..605fde4e2e4 100644 --- a/pom.xml +++ b/pom.xml @@ -96,10 +96,6 @@ - - 3.2 - - jamesagnew @@ -1615,29 +1611,28 @@ org.apache.maven.plugins maven-enforcer-plugin - 1.4.1 + 3.0.0-M2 - enforce-java + enforce-maven enforce + + + + 3.3.1 + + + 1.8 + + The hapi-fhir Maven build requires JDK version 1.8.x. + + + + - - - - 3.3 - - - - 1.8 - - The hapi-fhir Maven build requires JDK version 1.8.x. - - - - org.codehaus.mojo