From aad5a30a3d559a78385a3eff25d355992210108e Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 15 Jan 2020 14:52:32 +0800 Subject: [PATCH] Support count=0 (#1670) * Support count=0 * Fix LGTM issue --- .../4_2_0/981-support-count-zero.yaml | 5 + .../jpa/search/SearchCoordinatorSvcImpl.java | 5 +- .../provider/r5/ResourceProviderR5Test.java | 21 ++ .../fhir/rest/server/RestfulServerUtils.java | 29 ++- .../rest/server/method/CountParameter.java | 26 ++- .../server/SearchCountParamDstu3Test.java | 179 ++++++++++-------- 6 files changed, 160 insertions(+), 105 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/981-support-count-zero.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/981-support-count-zero.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/981-support-count-zero.yaml new file mode 100644 index 00000000000..fb7869f56e1 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/981-support-count-zero.yaml @@ -0,0 +1,5 @@ +--- +type: add +issue: 981 +title: "Support has been added to the server (plain and JPA) for querying with + `_count=0` as a URL parameter." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index c848625194a..ca1e6de2187 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java @@ -97,6 +97,7 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchCoordinatorSvcImpl.class); public static final String UNIT_TEST_CAPTURE_STACK = "unit_test_capture_stack"; + public static final Integer INTEGER_0 = Integer.valueOf(0); private final ConcurrentHashMap myIdToSearchTask = new ConcurrentHashMap<>(); @Autowired private FhirContext myContext; @@ -957,7 +958,9 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { * * before doing anything else. */ - boolean wantOnlyCount = SummaryEnum.COUNT.equals(myParams.getSummaryMode()); + boolean wantOnlyCount = + SummaryEnum.COUNT.equals(myParams.getSummaryMode()) + | INTEGER_0.equals(myParams.getCount()); boolean wantCount = wantOnlyCount || SearchTotalModeEnum.ACCURATE.equals(myParams.getSearchTotalMode()) || diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5Test.java index 3c4ef4772af..6d4a4f4760b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r5/ResourceProviderR5Test.java @@ -222,6 +222,27 @@ public class ResourceProviderR5Test extends BaseResourceProviderR5Test { assertThat(ids, containsInAnyOrder(oid)); } + + @Test + public void testCount0() { + Observation observation = new Observation(); + observation.setEffective(new DateTimeType("1965-08-09")); + myObservationDao.create(observation).getId().toUnqualified(); + + observation = new Observation(); + observation.setEffective(new DateTimeType("1965-08-10")); + myObservationDao.create(observation).getId().toUnqualified(); + + Bundle output = ourClient + .search() + .byUrl("Observation?_count=0") + .returnBundle(Bundle.class) + .execute(); + + assertEquals(2, output.getTotal()); + assertEquals(0, output.getEntry().size()); + } + @AfterClass public static void afterClassClearContext() { TestUtil.clearAllStaticFieldsForUnitTest(); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java index 0b5a48c3f63..d21620f5c79 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java @@ -32,6 +32,7 @@ import ca.uhn.fhir.rest.api.*; import ca.uhn.fhir.rest.api.server.IRestfulResponse; import ca.uhn.fhir.rest.api.server.IRestfulServer; import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.param.ParameterUtil; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.method.ElementsParameter; @@ -129,7 +130,7 @@ public class RestfulServerUtils { // _elements Set elements = ElementsParameter.getElementsValueOrNull(theRequestDetails, false); - if (elements != null && summaryMode != null && !summaryMode.equals(Collections.singleton(SummaryEnum.FALSE))) { + if (elements != null && !summaryMode.equals(Collections.singleton(SummaryEnum.FALSE))) { throw new InvalidRequestException("Cannot combine the " + Constants.PARAM_SUMMARY + " and " + Constants.PARAM_ELEMENTS + " parameters"); } @@ -139,17 +140,24 @@ public class RestfulServerUtils { parser.setDontEncodeElements(elementsExclude); } - if (summaryMode != null) { - if (summaryMode.contains(SummaryEnum.COUNT) && summaryMode.size() == 1) { - parser.setEncodeElements(Collections.singleton("Bundle.total")); - } else if (summaryMode.contains(SummaryEnum.TEXT) && summaryMode.size() == 1) { - parser.setEncodeElements(TEXT_ENCODE_ELEMENTS); - parser.setEncodeElementsAppliesToChildResourcesOnly(true); - } else { - parser.setSuppressNarratives(summaryMode.contains(SummaryEnum.DATA)); - parser.setSummaryMode(summaryMode.contains(SummaryEnum.TRUE)); + boolean summaryModeCount = summaryMode.contains(SummaryEnum.COUNT) && summaryMode.size() == 1; + if (!summaryModeCount) { + String[] countParam = theRequestDetails.getParameters().get(Constants.PARAM_COUNT); + if (countParam != null && countParam.length > 0) { + summaryModeCount = "0".equalsIgnoreCase(countParam[0]); } } + + if (summaryModeCount) { + parser.setEncodeElements(Collections.singleton("Bundle.total")); + } else if (summaryMode.contains(SummaryEnum.TEXT) && summaryMode.size() == 1) { + parser.setEncodeElements(TEXT_ENCODE_ELEMENTS); + parser.setEncodeElementsAppliesToChildResourcesOnly(true); + } else { + parser.setSuppressNarratives(summaryMode.contains(SummaryEnum.DATA)); + parser.setSummaryMode(summaryMode.contains(SummaryEnum.TRUE)); + } + if (elements != null && elements.size() > 0) { String elementsAppliesTo = "*"; if (isNotBlank(theRequestDetails.getResourceName())) { @@ -502,6 +510,7 @@ public class RestfulServerUtils { return retVal; } + @Nonnull public static Set determineSummaryMode(RequestDetails theRequest) { Map requestParams = theRequest.getParameters(); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/CountParameter.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/CountParameter.java index 9d11cef440e..8793e1f3a92 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/CountParameter.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/CountParameter.java @@ -20,21 +20,19 @@ package ca.uhn.fhir.rest.server.method; * #L% */ -import java.lang.reflect.Method; -import java.util.Collection; - -import org.apache.commons.lang3.StringUtils; - import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.model.primitive.IntegerDt; import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.rest.annotation.Count; -import ca.uhn.fhir.rest.annotation.Since; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.param.ParameterUtil; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import org.apache.commons.lang3.StringUtils; + +import java.lang.reflect.Method; +import java.util.Collection; public class CountParameter implements IParameter { @@ -42,20 +40,20 @@ public class CountParameter implements IParameter { @Override public Object translateQueryParametersIntoServerArgument(RequestDetails theRequest, BaseMethodBinding theMethodBinding) throws InternalErrorException, InvalidRequestException { - String[] sinceParams = theRequest.getParameters().get(Constants.PARAM_COUNT); - if (sinceParams != null) { - if (sinceParams.length > 0) { - if (StringUtils.isNotBlank(sinceParams[0])) { + String[] countParam = theRequest.getParameters().get(Constants.PARAM_COUNT); + if (countParam != null) { + if (countParam.length > 0) { + if (StringUtils.isNotBlank(countParam[0])) { try { - IntegerDt since = new IntegerDt(sinceParams[0]); - return ParameterUtil.fromInteger(myType, since); + IntegerDt count = new IntegerDt(countParam[0]); + return ParameterUtil.fromInteger(myType, count); } catch (DataFormatException e) { - throw new InvalidRequestException("Invalid " + Constants.PARAM_COUNT + " value: " + sinceParams[0]); + throw new InvalidRequestException("Invalid " + Constants.PARAM_COUNT + " value: " + countParam[0]); } } } } - return ParameterUtil.fromInteger(myType, null); + return null; } @Override diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchCountParamDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchCountParamDstu3Test.java index b5351919be6..867fa36ed75 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchCountParamDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/SearchCountParamDstu3Test.java @@ -1,14 +1,13 @@ package ca.uhn.fhir.rest.server; -import static org.hamcrest.Matchers.stringContainsInOrder; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThat; - -import java.util.ArrayList; -import java.util.List; -import java.util.concurrent.TimeUnit; - +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.rest.annotation.Count; +import ca.uhn.fhir.rest.annotation.OptionalParam; +import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.api.EncodingEnum; +import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.test.utilities.JettyUtil; +import ca.uhn.fhir.util.TestUtil; import org.apache.commons.io.IOUtils; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; @@ -26,19 +25,21 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; -import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.rest.annotation.Count; -import ca.uhn.fhir.rest.annotation.OptionalParam; -import ca.uhn.fhir.rest.annotation.Search; -import ca.uhn.fhir.rest.param.TokenParam; -import ca.uhn.fhir.test.utilities.JettyUtil; -import ca.uhn.fhir.util.TestUtil; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.stringContainsInOrder; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; public class SearchCountParamDstu3Test { + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchCountParamDstu3Test.class); private static CloseableHttpClient ourClient; private static FhirContext ourCtx = FhirContext.forDstu3(); - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchCountParamDstu3Test.class); private static int ourPort; private static Server ourServer; private static String ourLastMethod; @@ -53,28 +54,46 @@ public class SearchCountParamDstu3Test { @Test public void testSearch() throws Exception { HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_count=2"); - CloseableHttpResponse status = ourClient.execute(httpGet); - try { + + try (CloseableHttpResponse status = ourClient.execute(httpGet)) { String responseContent = IOUtils.toString(status.getEntity().getContent()); ourLog.info(responseContent); assertEquals(200, status.getStatusLine().getStatusCode()); assertEquals("search", ourLastMethod); - assertEquals(new Integer(2), ourLastParam); - - //@formatter:off + assertEquals(Integer.valueOf(2), ourLastParam); + assertThat(responseContent, stringContainsInOrder( - "", - "", - "", - "", "", - "", - "", + "", + "", + "", + "", + "", + "", "")); - //@formatter:on - - } finally { - IOUtils.closeQuietly(status.getEntity().getContent()); + + } + + } + + + @Test + public void testSearchCount0() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_count=0&_pretty=true"); + + try (CloseableHttpResponse status = ourClient.execute(httpGet)) { + String responseContent = IOUtils.toString(status.getEntity().getContent()); + ourLog.info(responseContent); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertEquals("search", ourLastMethod); + assertEquals(Integer.valueOf(0), ourLastParam); + + assertThat(responseContent, stringContainsInOrder( + "", + "")); + assertThat(responseContent, not(containsString("entry"))); + } } @@ -92,25 +111,65 @@ public class SearchCountParamDstu3Test { assertEquals(200, status.getStatusLine().getStatusCode()); assertEquals("searchWithNoCountParam", ourLastMethod); assertEquals(null, ourLastParam); - + //@formatter:off assertThat(responseContent, stringContainsInOrder( - "", - "", - "", - "", "", - "", - "", + "", + "", + "", + "", + "", + "", "")); //@formatter:on - + } finally { IOUtils.closeQuietly(status.getEntity().getContent()); } } + public static class DummyPatientResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Patient.class; + } + + //@formatter:off + @SuppressWarnings("rawtypes") + @Search() + public List search( + @OptionalParam(name = Patient.SP_IDENTIFIER) TokenParam theIdentifier, + @Count() Integer theParam + ) { + ourLastMethod = "search"; + ourLastParam = theParam; + ArrayList retVal = new ArrayList(); + for (int i = 1; i < 100; i++) { + retVal.add((Patient) new Patient().addName(new HumanName().setFamily("FAMILY")).setId("" + i)); + } + return retVal; + } + //@formatter:on + + //@formatter:off + @SuppressWarnings("rawtypes") + @Search(queryName = "searchWithNoCountParam") + public List searchWithNoCountParam() { + ourLastMethod = "searchWithNoCountParam"; + ourLastParam = null; + ArrayList retVal = new ArrayList(); + for (int i = 1; i < 100; i++) { + retVal.add((Patient) new Patient().addName(new HumanName().setFamily("FAMILY")).setId("" + i)); + } + return retVal; + } + //@formatter:on + + } + @AfterClass public static void afterClassClearContext() throws Exception { JettyUtil.closeServer(ourServer); @@ -133,7 +192,7 @@ public class SearchCountParamDstu3Test { proxyHandler.addServletWithMapping(servletHolder, "/*"); ourServer.setHandler(proxyHandler); JettyUtil.startServer(ourServer); - ourPort = JettyUtil.getPortForStartedServer(ourServer); + ourPort = JettyUtil.getPortForStartedServer(ourServer); PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS); HttpClientBuilder builder = HttpClientBuilder.create(); @@ -142,44 +201,4 @@ public class SearchCountParamDstu3Test { } - public static class DummyPatientResourceProvider implements IResourceProvider { - - @Override - public Class getResourceType() { - return Patient.class; - } - - //@formatter:off - @SuppressWarnings("rawtypes") - @Search() - public List search( - @OptionalParam(name=Patient.SP_IDENTIFIER) TokenParam theIdentifier, - @Count() Integer theParam - ) { - ourLastMethod = "search"; - ourLastParam = theParam; - ArrayList retVal = new ArrayList(); - for (int i = 1; i < 100; i++) { - retVal.add((Patient) new Patient().addName(new HumanName().setFamily("FAMILY")).setId("" + i)); - } - return retVal; - } - //@formatter:on - - //@formatter:off - @SuppressWarnings("rawtypes") - @Search(queryName="searchWithNoCountParam") - public List searchWithNoCountParam() { - ourLastMethod = "searchWithNoCountParam"; - ourLastParam = null; - ArrayList retVal = new ArrayList(); - for (int i = 1; i < 100; i++) { - retVal.add((Patient) new Patient().addName(new HumanName().setFamily("FAMILY")).setId("" + i)); - } - return retVal; - } - //@formatter:on - - } - }