diff --git a/hapi-fhir-base/pom.xml b/hapi-fhir-base/pom.xml index a863cf5c890..45f4e8fab48 100644 --- a/hapi-fhir-base/pom.xml +++ b/hapi-fhir-base/pom.xml @@ -585,12 +585,12 @@ org.apache.maven.plugins maven-surefire-report-plugin - 2.16 + ${maven_surefire_plugin_version} org.apache.maven.plugins maven-javadoc-plugin - 2.9.1 + ${maven_javadoc_plugin_version} http://docs.oracle.com/javaee/7/api diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchMethodBinding.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchMethodBinding.java index 001f33aa4d5..46bb981d96b 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchMethodBinding.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchMethodBinding.java @@ -20,7 +20,8 @@ package ca.uhn.fhir.rest.method; * #L% */ -import static org.apache.commons.lang3.StringUtils.*; +import static org.apache.commons.lang3.StringUtils.isBlank; +import static org.apache.commons.lang3.StringUtils.isNotBlank; import java.lang.reflect.Method; import java.util.ArrayList; @@ -29,8 +30,8 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.Map.Entry; +import java.util.Set; import org.apache.commons.lang3.StringUtils; @@ -41,7 +42,6 @@ import ca.uhn.fhir.model.api.annotation.Description; import ca.uhn.fhir.model.dstu.valueset.RestfulOperationSystemEnum; import ca.uhn.fhir.model.dstu.valueset.RestfulOperationTypeEnum; import ca.uhn.fhir.model.primitive.IdDt; -import ca.uhn.fhir.rest.annotation.OptionalParam; import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.client.BaseHttpClientInvocation; import ca.uhn.fhir.rest.param.BaseQueryParameter; @@ -95,7 +95,8 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { SearchParameter sp = (SearchParameter) next; if (sp.getName().startsWith("_")) { if (ALLOWED_PARAMS.contains(sp.getName())) { - String msg = getContext().getLocalizer().getMessage(getClass().getName() + ".invalidSpecialParamName", theMethod.getName(), theMethod.getDeclaringClass().getSimpleName(), sp.getName()); + String msg = getContext().getLocalizer().getMessage(getClass().getName() + ".invalidSpecialParamName", theMethod.getName(), theMethod.getDeclaringClass().getSimpleName(), + sp.getName()); throw new ConfigurationException(msg); } } @@ -117,6 +118,40 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { } + public static QualifierDetails extractQualifiersFromParameterName(String theParamName) { + QualifierDetails retVal = new QualifierDetails(); + if (theParamName == null || theParamName.length() == 0) { + return retVal; + } + + int dotIdx = -1; + int colonIdx = -1; + for (int idx = 0; idx < theParamName.length(); idx++) { + char nextChar = theParamName.charAt(idx); + if (nextChar == '.' && dotIdx == -1) { + dotIdx = idx; + } else if (nextChar == ':' && colonIdx == -1) { + colonIdx = idx; + } + } + + if (dotIdx != -1 && colonIdx != -1) { + if (dotIdx < colonIdx) { + retVal.setDotQualifier(theParamName.substring(dotIdx, colonIdx)); + retVal.setColonQualifier(theParamName.substring(colonIdx)); + } else { + retVal.setColonQualifier(theParamName.substring(colonIdx, dotIdx)); + retVal.setDotQualifier(theParamName.substring(dotIdx)); + } + } else if (dotIdx != -1) { + retVal.setDotQualifier(theParamName.substring(dotIdx)); + } else if (colonIdx != -1) { + retVal.setColonQualifier(theParamName.substring(colonIdx)); + } + + return retVal; + } + public Class getDeclaredResourceType() { return myDeclaredResourceType; } @@ -181,7 +216,10 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { if (temp.isRequired()) { if (qualifiedParamNames.contains(name)) { - methodParamsTemp.add(name); + QualifierDetails qualifiers = extractQualifiersFromParameterName(name); + if (qualifiers.passes(temp.getQualifierWhitelist(), temp.getQualifierBlacklist())) { + methodParamsTemp.add(name); + } } else if (unqualifiedNames.contains(name)) { List qualifiedNames = theRequest.getUnqualifiedToQualifiedNames().get(name); qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, temp.getQualifierWhitelist(), temp.getQualifierBlacklist()); @@ -193,7 +231,10 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { } else { if (qualifiedParamNames.contains(name)) { - methodParamsTemp.add(name); + QualifierDetails qualifiers = extractQualifiersFromParameterName(name); + if (qualifiers.passes(temp.getQualifierWhitelist(), temp.getQualifierBlacklist())) { + methodParamsTemp.add(name); + } } else if (unqualifiedNames.contains(name)) { List qualifiedNames = theRequest.getUnqualifiedToQualifiedNames().get(name); qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, temp.getQualifierWhitelist(), temp.getQualifierBlacklist()); @@ -281,67 +322,27 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { } - public void setResourceType(Class resourceType) { - this.myDeclaredResourceType = resourceType; - } - private List processWhitelistAndBlacklist(List theQualifiedNames, Set theQualifierWhitelist, Set theQualifierBlacklist) { if (theQualifierWhitelist == null && theQualifierBlacklist == null) { return theQualifiedNames; } ArrayList retVal = new ArrayList(theQualifiedNames.size()); for (String next : theQualifiedNames) { - String qualifier = ""; - - int start = -1; - int end = -1; - for (int idx = 0; idx < next.length(); idx++) { - char nextChar = next.charAt(idx); - if (nextChar == '.' || nextChar == ':') { - if (start == -1) { - start = idx; - } else { - end = idx; - break; - } - } - } - - if (start != -1) { - if (end != -1) { - qualifier = next.substring(start, end); - } else { - qualifier = next.substring(start); - } - } - - if (theQualifierWhitelist != null) { - if (qualifier != null) { - if (!theQualifierWhitelist.contains(qualifier)) { - if (qualifier.charAt(0) == '.') { - if (!theQualifierWhitelist.contains(".*")) { - continue; - } - } - if (qualifier.charAt(0) == ':') { - if (!theQualifierWhitelist.contains(":*")) { - continue; - } - } - } - } - } - if (theQualifierBlacklist != null) { - if (theQualifierBlacklist.contains(qualifier)) { - continue; - } + QualifierDetails qualifiers = extractQualifiersFromParameterName(next); + if (!qualifiers.passes(theQualifierWhitelist, theQualifierBlacklist)) { + continue; } retVal.add(next); } return retVal; } - public static BaseHttpClientInvocation createSearchInvocation(FhirContext theContext, String theResourceName, Map> theParameters, IdDt theId, String theCompartmentName, SearchStyleEnum theSearchStyle) { + public void setResourceType(Class resourceType) { + this.myDeclaredResourceType = resourceType; + } + + public static BaseHttpClientInvocation createSearchInvocation(FhirContext theContext, String theResourceName, Map> theParameters, IdDt theId, String theCompartmentName, + SearchStyleEnum theSearchStyle) { SearchStyleEnum searchStyle = theSearchStyle; if (searchStyle == null) { int length = 0; @@ -372,8 +373,7 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { } /* - * Are we doing a get (GET [base]/Patient?name=foo) or a get with search (GET [base]/Patient/_search?name=foo) - * or a post (POST [base]/Patient with parameters in the POST body) + * Are we doing a get (GET [base]/Patient?name=foo) or a get with search (GET [base]/Patient/_search?name=foo) or a post (POST [base]/Patient with parameters in the POST body) */ switch (searchStyle) { case GET: @@ -402,6 +402,76 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { return invocation; } + public static class QualifierDetails { + + private String myColonQualifier; + + private String myDotQualifier; + + public String getColonQualifier() { + return myColonQualifier; + } + + public String getDotQualifier() { + return myDotQualifier; + } + + public boolean passes(Set theQualifierWhitelist, Set theQualifierBlacklist) { + if (theQualifierWhitelist != null) { + if (!theQualifierWhitelist.contains(".*")) { + if (myDotQualifier != null) { + if (!theQualifierWhitelist.contains(myDotQualifier)) { + return false; + } + } else { + if (!theQualifierWhitelist.contains(".")) { + return false; + } + } + } + if (!theQualifierWhitelist.contains(":*")) { + if (myColonQualifier != null) { + if (!theQualifierWhitelist.contains(myColonQualifier)) { + return false; + } + } else { + if (!theQualifierWhitelist.contains(":")) { + return false; + } + } + } + } + if (theQualifierBlacklist != null) { + if (myDotQualifier != null) { + if (theQualifierBlacklist.contains(myDotQualifier)) { + return false; + } + } + if (myColonQualifier != null) { + if (theQualifierBlacklist.contains(myColonQualifier)) { + return false; + } + } + } + + return true; + } + + public void setColonQualifier(String theColonQualifier) { + myColonQualifier = theColonQualifier; + } + + public void setDotQualifier(String theDotQualifier) { + myDotQualifier = theDotQualifier; + } + + } + + @Override + public String toString() { + return getMethod().toString(); + } + public static enum RequestType { DELETE, GET, OPTIONS, POST, PUT } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchParameter.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchParameter.java index 21dd06c031f..d20fa0a4ae3 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchParameter.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/method/SearchParameter.java @@ -117,20 +117,15 @@ public class SearchParameter extends BaseQueryParameter { ourParamTypes.put(ReferenceParam.class, SearchParamTypeEnum.REFERENCE); ourParamTypes.put(ReferenceOrListParam.class, SearchParamTypeEnum.REFERENCE); ourParamTypes.put(ReferenceAndListParam.class, SearchParamTypeEnum.REFERENCE); - ourParamQualifiers.put(SearchParamTypeEnum.REFERENCE, CollectionUtil.newSet(Constants.PARAMQUALIFIER_MISSING)); // no - // empty - // because - // that - // gets - // added - // from - // OptionalParam#chainWhitelist + // --vvvv-- no empty because that gets added from OptionalParam#chainWhitelist + ourParamQualifiers.put(SearchParamTypeEnum.REFERENCE, CollectionUtil.newSet(Constants.PARAMQUALIFIER_MISSING)); ourParamTypes.put(CompositeParam.class, SearchParamTypeEnum.COMPOSITE); ourParamTypes.put(CompositeOrListParam.class, SearchParamTypeEnum.COMPOSITE); ourParamTypes.put(CompositeAndListParam.class, SearchParamTypeEnum.COMPOSITE); ourParamQualifiers.put(SearchParamTypeEnum.COMPOSITE, CollectionUtil.newSet(Constants.PARAMQUALIFIER_MISSING, EMPTY_STRING)); } + private List> myCompositeTypes; private List> myDeclaredTypes; private String myDescription; @@ -230,9 +225,9 @@ public class SearchParameter extends BaseQueryParameter { for (int i = 0; i < theChainWhitelist.length; i++) { if (theChainWhitelist[i].equals(OptionalParam.ALLOW_CHAIN_ANY)) { - myQualifierWhitelist.add(OptionalParam.ALLOW_CHAIN_ANY); + myQualifierWhitelist.add('.' + OptionalParam.ALLOW_CHAIN_ANY); } else if (theChainWhitelist[i].equals(EMPTY_STRING)) { - myQualifierWhitelist.add(""); + myQualifierWhitelist.add("."); } else { myQualifierWhitelist.add('.' + theChainWhitelist[i]); } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseQueryParameter.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseQueryParameter.java index 26185a2ebb1..858493e6e2d 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseQueryParameter.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/BaseQueryParameter.java @@ -20,8 +20,6 @@ package ca.uhn.fhir.rest.param; * #L% */ -import static org.apache.commons.lang3.StringUtils.*; - import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; @@ -33,12 +31,13 @@ import org.apache.commons.lang3.StringUtils; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.model.dstu.valueset.SearchParamTypeEnum; -import ca.uhn.fhir.rest.annotation.OptionalParam; import ca.uhn.fhir.rest.client.BaseHttpClientInvocation; import ca.uhn.fhir.rest.method.IParameter; import ca.uhn.fhir.rest.method.QualifiedParamList; import ca.uhn.fhir.rest.method.Request; import ca.uhn.fhir.rest.method.RequestDetails; +import ca.uhn.fhir.rest.method.SearchMethodBinding; +import ca.uhn.fhir.rest.method.SearchMethodBinding.QualifierDetails; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; @@ -89,7 +88,7 @@ public abstract class BaseQueryParameter implements IParameter { theTargetQueryArguments.put(getName() + StringUtils.defaultString(qualifier), paramValues); } } - +private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseQueryParameter.class); @Override public Object translateQueryParametersIntoServerArgument(Request theRequest, Object theRequestContents) throws InternalErrorException, InvalidRequestException { @@ -105,6 +104,9 @@ public abstract class BaseQueryParameter implements IParameter { } if (paramList.isEmpty()) { + + ourLog.debug("No value for parameter '{}' - Qualified names {} and qualifier whitelist {}", getName(), qualified, getQualifierWhitelist()); + if (handlesMissing()) { return parse(paramList); } else { @@ -117,28 +119,9 @@ public abstract class BaseQueryParameter implements IParameter { } private void parseParams(RequestDetails theRequest, List paramList, String theQualifiedParamName, String theQualifier) { - if (getQualifierWhitelist() != null) { - if (!getQualifierWhitelist().contains(defaultString(theQualifier))) { - if (theQualifier == null) { - return; - } - if (theQualifier.charAt(0) == '.') { - if (!getQualifierWhitelist().contains(".*")) { - return; - } - } else if (theQualifier.charAt(0) == ':') { - if (!getQualifierWhitelist().contains(":*")) { - return; - } - } else { - return; - } - } - } - if (getQualifierBlacklist() != null) { - if (getQualifierBlacklist().contains(defaultString(theQualifier))) { - return; - } + QualifierDetails qualifiers = SearchMethodBinding.extractQualifiersFromParameterName(theQualifier); + if (!qualifiers.passes(getQualifierWhitelist(), getQualifierBlacklist())) { + return; } String[] value = theRequest.getParameters().get(theQualifiedParamName); diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/client/InterceptorTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/client/InterceptorTest.java index 19be04260fe..457eb426b95 100644 --- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/client/InterceptorTest.java +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/client/InterceptorTest.java @@ -4,6 +4,8 @@ import static org.junit.Assert.*; import static org.mockito.Matchers.*; import static org.mockito.Mockito.*; +import java.util.ResourceBundle; + import org.eclipse.jetty.server.Server; import org.eclipse.jetty.servlet.ServletHandler; import org.eclipse.jetty.servlet.ServletHolder; @@ -21,6 +23,7 @@ import ca.uhn.fhir.rest.annotation.IdParam; import ca.uhn.fhir.rest.annotation.Read; import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor; import ca.uhn.fhir.rest.server.IResourceProvider; +import ca.uhn.fhir.rest.server.ResourceBinding; import ca.uhn.fhir.rest.server.RestfulServer; import ca.uhn.fhir.testutil.RandomServerPortProvider; import ch.qos.logback.classic.spi.ILoggingEvent; @@ -53,7 +56,12 @@ public class InterceptorTest { Patient patient = client.read(Patient.class, "1"); assertFalse(patient.getIdentifierFirstRep().isEmpty()); - verify(mockAppender).doAppend(argThat(new ArgumentMatcher() { + int times = 1; + if (LoggerFactory.getLogger(ResourceBinding.class).isDebugEnabled()) { + times = 3; + } + + verify(mockAppender, times(times)).doAppend(argThat(new ArgumentMatcher() { @Override public boolean matches(final Object argument) { return ((LoggingEvent) argument).getFormattedMessage().contains("Content-Type: application/xml+fhir; charset=UTF-8"); diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/server/ReferenceParameterTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/server/ReferenceParameterTest.java index 69f20c955f1..2d06a4706b0 100644 --- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/server/ReferenceParameterTest.java +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/rest/server/ReferenceParameterTest.java @@ -28,6 +28,7 @@ import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.dstu.resource.Conformance; import ca.uhn.fhir.model.dstu.resource.Conformance.RestResource; import ca.uhn.fhir.model.dstu.resource.Conformance.RestResourceSearchParam; +import ca.uhn.fhir.model.dstu.resource.Location; import ca.uhn.fhir.model.dstu.resource.Organization; import ca.uhn.fhir.model.dstu.resource.Patient; import ca.uhn.fhir.model.dstu.valueset.ResourceTypeEnum; @@ -35,6 +36,7 @@ import ca.uhn.fhir.rest.annotation.OptionalParam; import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.param.ReferenceParam; import ca.uhn.fhir.rest.param.StringParam; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.testutil.RandomServerPortProvider; /** @@ -156,7 +158,7 @@ public class ReferenceParameterTest { String responseContent = IOUtils.toString(status.getEntity().getContent()); IOUtils.closeQuietly(status.getEntity().getContent()); assertEquals(200, status.getStatusLine().getStatusCode()); - assertThat(responseContent, containsString("value=\"theBarId po123 bar\"")); + assertThat(responseContent, containsString("value=\"theBarId Organization/po123 bar\"")); } @Test @@ -166,7 +168,15 @@ public class ReferenceParameterTest { String responseContent = IOUtils.toString(status.getEntity().getContent()); IOUtils.closeQuietly(status.getEntity().getContent()); assertEquals(200, status.getStatusLine().getStatusCode()); - assertThat(responseContent, containsString("value=\"thePartOfId po123 null\"")); + assertThat(responseContent, containsString("value=\"thePartOfId Organization/po123 null\"")); + } + + @Test + public void testSearchWithMultipleParamsOfTheSameName8() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Location?partof=po123"); + HttpResponse status = ourClient.execute(httpGet); + IOUtils.closeQuietly(status.getEntity().getContent()); + assertEquals(400, status.getStatusLine().getStatusCode()); } @@ -202,7 +212,7 @@ public class ReferenceParameterTest { assertEquals(200, status.getStatusLine().getStatusCode()); Conformance conf = ourCtx.newXmlParser().parseResource(Conformance.class, responseContent); - RestResource res = conf.getRestFirstRep().getResource().get(1); + RestResource res = conf.getRestFirstRep().getResource().get(2); assertEquals("Patient", res.getType().getValue()); RestResourceSearchParam param = res.getSearchParamFirstRep(); @@ -227,7 +237,7 @@ public class ReferenceParameterTest { ServletHandler proxyHandler = new ServletHandler(); RestfulServer servlet = new RestfulServer(); ourCtx = servlet.getFhirContext(); - servlet.setResourceProviders(patientProvider, new DummyOrganizationResourceProvider()); + servlet.setResourceProviders(patientProvider, new DummyOrganizationResourceProvider(), new DummyLocationResourceProvider()); ServletHolder servletHolder = new ServletHolder(servlet); proxyHandler.addServletWithMapping(servletHolder, "/*"); ourServer.setHandler(proxyHandler); @@ -272,10 +282,8 @@ public class ReferenceParameterTest { retVal.add(org); } if (retVal.isEmpty()) { - Organization org = new Organization(); - org.setId("0"); - org.getName().setValue("none"); - retVal.add(org); + ourLog.info("No values for foo - Going to fail"); + throw new InternalErrorException("No Values for foo"); } return retVal; @@ -295,16 +303,46 @@ public class ReferenceParameterTest { retVal.add(org); } if (retVal.isEmpty()) { - Organization org = new Organization(); - org.setId("0"); - org.getName().setValue("none"); - retVal.add(org); + ourLog.info("No values for bar - Going to fail"); + throw new InternalErrorException("No Values for bar"); } return retVal; } } + + + public static class DummyLocationResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Location.class; + } + + //@formatter:off + @Search + public List searchByNameWithDifferentChain( + @OptionalParam(name = "partof", chainWhitelist= {"bar"}) ReferenceParam theBarId) { + //@formatter:on + + ArrayList retVal = new ArrayList(); + if (theBarId != null) { + Location loc = new Location(); + loc.setId("1"); + loc.getName().setValue("theBarId " + theBarId.getValue() + " " + theBarId.getChain()); + retVal.add(loc); + } + if (retVal.isEmpty()) { + ourLog.info("No values for bar - Going to fail"); + throw new InternalErrorException("No Values for bar"); + } + + return retVal; + } + + } + /** * Created by dsotnikov on 2/25/2014. diff --git a/hapi-fhir-base/src/test/resources/logback-test.xml b/hapi-fhir-base/src/test/resources/logback-test.xml index ba6af5fea46..a4701dc31a8 100644 --- a/hapi-fhir-base/src/test/resources/logback-test.xml +++ b/hapi-fhir-base/src/test/resources/logback-test.xml @@ -18,7 +18,7 @@ --> - + diff --git a/pom.xml b/pom.xml index 3af9aa4e022..7713d5c7198 100644 --- a/pom.xml +++ b/pom.xml @@ -91,6 +91,7 @@ 4.11 1.1.1 2.9.1 + 2.17 3.4 1.1.8 1.9.5 @@ -115,6 +116,14 @@ 1.6 + + org.apache.maven.plugins + maven-surefire-plugin + ${maven_surefire_plugin_version} + + true + + org.apache.maven.plugins maven-site-plugin