From 497757501b879b3404c114af3938cda905c46c7c Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 17 Apr 2020 09:28:33 -0400 Subject: [PATCH] Improve search method binding priority (#1802) * Work on search method binding priority * Work on method priority * Work on binding priority * Test fixes * Add changelog * Test fixes * compile fix * One more comple fix * Test cleanup * Test fix --- .../ca/uhn/fhir/rest/param/ParameterUtil.java | 13 + .../java/ca/uhn/fhir/util/ReflectionUtil.java | 48 +++- .../ca/uhn/fhir/util/ReflectionUtilTest.java | 9 + ...mprove-search-binding-method-priority.yaml | 8 + .../ca/uhn/fhir/jpa/dao/BaseStorageDao.java | 2 +- .../r4/FhirResourceDaoR4InterceptorTest.java | 14 +- .../uhn/fhir/rest/server/ResourceBinding.java | 34 ++- .../uhn/fhir/rest/server/RestfulServer.java | 18 +- .../rest/server/method/BaseMethodBinding.java | 30 ++- .../BaseOutcomeReturningMethodBinding.java | 12 +- .../server/method/BaseQueryParameter.java | 2 + .../method/ConformanceMethodBinding.java | 10 +- .../server/method/GraphQLMethodBinding.java | 8 +- .../server/method/HistoryMethodBinding.java | 26 +- .../rest/server/method/IncludeParameter.java | 5 + .../rest/server/method/MethodMatchEnum.java | 19 ++ .../fhir/rest/server/method/MethodUtil.java | 7 +- .../server/method/OperationMethodBinding.java | 18 +- .../rest/server/method/PageMethodBinding.java | 10 +- .../server/method/PatchMethodBinding.java | 6 +- ...sParmeter.java => RawParamsParameter.java} | 8 +- .../rest/server/method/ReadMethodBinding.java | 22 +- .../server/method/SearchMethodBinding.java | 246 ++++++++---------- .../rest/server/method/SearchParameter.java | 69 ++--- .../method/TransactionMethodBinding.java | 14 +- .../server/method/MethodMatchEnumTest.java | 16 ++ .../server/method/ReadMethodBindingTest.java | 19 +- .../method/SearchMethodBindingTest.java | 22 +- .../rest/server/ServerSearchDstu2Test.java | 12 +- .../server/SearchCountParamDstu3Test.java | 2 - .../rest/server/SearchMethodPriorityTest.java | 221 ++++++++++++++++ .../utilities/server/RestfulServerRule.java | 41 ++- 32 files changed, 691 insertions(+), 300 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1802-improve-search-binding-method-priority.yaml create mode 100644 hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/MethodMatchEnum.java rename hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/{RawParamsParmeter.java => RawParamsParameter.java} (90%) create mode 100644 hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/MethodMatchEnumTest.java create mode 100644 hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchMethodPriorityTest.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ParameterUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ParameterUtil.java index 5407ac8bd14..086c6d9e624 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ParameterUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/ParameterUtil.java @@ -118,6 +118,19 @@ public class ParameterUtil { return parseQueryParams(theContext, paramType, theUnqualifiedParamName, theParameters); } + /** + * Removes :modifiers and .chains from URL parameter names + */ + public static String stripModifierPart(String theParam) { + for (int i = 0; i < theParam.length(); i++) { + char nextChar = theParam.charAt(i); + if (nextChar == ':' || nextChar == '.') { + return theParam.substring(0, i); + } + } + return theParam; + } + /** * Escapes a string according to the rules for parameter escaping specified in the FHIR Specification Escaping * Section diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ReflectionUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ReflectionUtil.java index c33524dfddf..df364b9f143 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ReflectionUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ReflectionUtil.java @@ -31,31 +31,63 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; import java.lang.reflect.WildcardType; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.concurrent.ConcurrentHashMap; public class ReflectionUtil { - private static final ConcurrentHashMap ourFhirServerVersions = new ConcurrentHashMap<>(); - - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ReflectionUtil.class); public static final Object[] EMPTY_OBJECT_ARRAY = new Object[0]; public static final Class[] EMPTY_CLASS_ARRAY = new Class[0]; + private static final ConcurrentHashMap ourFhirServerVersions = new ConcurrentHashMap<>(); + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ReflectionUtil.class); - public static LinkedHashSet getDeclaredMethods(Class theClazz) { - LinkedHashSet retVal = new LinkedHashSet<>(); - for (Method next : theClazz.getDeclaredMethods()) { + /** + * Returns all methods declared against {@literal theClazz}. This method returns a predictable order, which is + * sorted by method name and then by parameters. + */ + public static List getDeclaredMethods(Class theClazz) { + HashSet foundMethods = new LinkedHashSet<>(); + Method[] declaredMethods = theClazz.getDeclaredMethods(); + for (Method next : declaredMethods) { try { Method method = theClazz.getMethod(next.getName(), next.getParameterTypes()); - retVal.add(method); + foundMethods.add(method); } catch (NoSuchMethodException | SecurityException e) { - retVal.add(next); + foundMethods.add(next); } } + + List retVal = new ArrayList<>(foundMethods); + retVal.sort((Comparator.comparing(ReflectionUtil::describeMethodInSortFriendlyWay))); return retVal; } + /** + * Returns a description like startsWith params(java.lang.String, int) returns(boolean). + * The format is chosen in order to provide a predictable and useful sorting order. + */ + public static String describeMethodInSortFriendlyWay(Method theMethod) { + StringBuilder b = new StringBuilder(); + b.append(theMethod.getName()); + b.append(" returns("); + b.append(theMethod.getReturnType().getName()); + b.append(") params("); + Class[] parameterTypes = theMethod.getParameterTypes(); + for (int i = 0, parameterTypesLength = parameterTypes.length; i < parameterTypesLength; i++) { + if (i > 0) { + b.append(", "); + } + Class next = parameterTypes[i]; + b.append(next.getName()); + } + b.append(")"); + return b.toString(); + } + public static Class getGenericCollectionTypeOfField(Field next) { ParameterizedType collectionType = (ParameterizedType) next.getGenericType(); return getGenericCollectionTypeOf(collectionType.getActualTypeArguments()[0]); diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/ReflectionUtilTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/ReflectionUtilTest.java index 4ec95c037cc..3239dcc9420 100644 --- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/ReflectionUtilTest.java +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/ReflectionUtilTest.java @@ -2,6 +2,7 @@ package ca.uhn.fhir.util; import static org.junit.Assert.*; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.List; @@ -51,4 +52,12 @@ public class ReflectionUtilTest { assertFalse(ReflectionUtil.typeExists("ca.Foo")); assertTrue(ReflectionUtil.typeExists(String.class.getName())); } + + @Test + public void testDescribeMethod() throws NoSuchMethodException { + Method method = String.class.getMethod("startsWith", String.class, int.class); + String description = ReflectionUtil.describeMethodInSortFriendlyWay(method); + assertEquals("startsWith returns(boolean) params(java.lang.String, int)", description); + } + } diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1802-improve-search-binding-method-priority.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1802-improve-search-binding-method-priority.yaml new file mode 100644 index 00000000000..180ade87229 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_3_0/1802-improve-search-binding-method-priority.yaml @@ -0,0 +1,8 @@ +--- +type: add +issue: 1802 +title: "In a plain server, if a Resource Provider class had two methods with the same parameter names + (as specified in the @OptionalParam or @RequiredParam) but different cardinalities, the server could + sometimes pick the incorrect method to execute. The selection algorithm has been improved to no longer + have this issue, and to be more consistent and predictable in terms of which resource provider + method is selected when the choice is somewhat ambiguous." diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java index 92d3977aba9..1df60039bff 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseStorageDao.java @@ -224,7 +224,7 @@ public abstract class BaseStorageDao { Set paramNames = theSource.keySet(); for (String nextParamName : paramNames) { - QualifierDetails qualifiedParamName = SearchMethodBinding.extractQualifiersFromParameterName(nextParamName); + QualifierDetails qualifiedParamName = QualifierDetails.extractQualifiersFromParameterName(nextParamName); RuntimeSearchParam param = searchParams.get(qualifiedParamName.getParamName()); if (param == null) { String msg = getContext().getLocalizer().getMessageSanitized(BaseHapiFhirResourceDao.class, "invalidSearchParameter", qualifiedParamName.getParamName(), new TreeSet<>(searchParams.keySet())); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4InterceptorTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4InterceptorTest.java index 43811b86857..54f08074aba 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4InterceptorTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4InterceptorTest.java @@ -29,10 +29,18 @@ import java.util.ArrayList; import java.util.List; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.in; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; import static org.mockito.Matchers.any; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.nullable; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; public class FhirResourceDaoR4InterceptorTest extends BaseJpaR4Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4InterceptorTest.class); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/ResourceBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/ResourceBinding.java index 0d264258bfe..04695bc0957 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/ResourceBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/ResourceBinding.java @@ -25,9 +25,10 @@ import java.util.List; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.method.BaseMethodBinding; +import ca.uhn.fhir.rest.server.method.MethodMatchEnum; /** - * Created by dsotnikov on 2/25/2014. + * Holds all method bindings for an individual resource type */ public class ResourceBinding { @@ -36,9 +37,16 @@ public class ResourceBinding { private String resourceName; private List> myMethodBindings = new ArrayList<>(); + /** + * Constructor + */ public ResourceBinding() { + super(); } + /** + * Constructor + */ public ResourceBinding(String resourceName, List> methods) { this.resourceName = resourceName; this.myMethodBindings = methods; @@ -51,14 +59,28 @@ public class ResourceBinding { } ourLog.debug("Looking for a handler for {}", theRequest); + + /* + * Look for the method with the highest match strength + */ + + BaseMethodBinding matchedMethod = null; + MethodMatchEnum matchedMethodStrength = null; + for (BaseMethodBinding rm : myMethodBindings) { - if (rm.incomingServerRequestMatchesMethod(theRequest)) { - ourLog.debug("Handler {} matches", rm); - return rm; + MethodMatchEnum nextMethodMatch = rm.incomingServerRequestMatchesMethod(theRequest); + if (nextMethodMatch != MethodMatchEnum.NONE) { + if (matchedMethodStrength == null || matchedMethodStrength.ordinal() < nextMethodMatch.ordinal()) { + matchedMethod = rm; + matchedMethodStrength = nextMethodMatch; + } + if (matchedMethodStrength == MethodMatchEnum.EXACT) { + break; + } } - ourLog.trace("Handler {} does not match", rm); } - return null; + + return matchedMethod; } public String getResourceName() { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java index 3c32507105a..b4361907e96 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java @@ -45,6 +45,7 @@ import ca.uhn.fhir.rest.server.interceptor.ExceptionHandlingInterceptor; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor; import ca.uhn.fhir.rest.server.method.BaseMethodBinding; import ca.uhn.fhir.rest.server.method.ConformanceMethodBinding; +import ca.uhn.fhir.rest.server.method.MethodMatchEnum; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.rest.server.tenant.ITenantIdentificationStrategy; import ca.uhn.fhir.util.*; @@ -298,7 +299,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer resourceMethod = null; String resourceName = requestDetails.getResourceName(); - if (myServerConformanceMethod.incomingServerRequestMatchesMethod(requestDetails)) { + if (myServerConformanceMethod.incomingServerRequestMatchesMethod(requestDetails) != MethodMatchEnum.NONE) { resourceMethod = myServerConformanceMethod; } else if (resourceName == null) { resourceBinding = myServerBinding; @@ -1785,6 +1786,21 @@ public class RestfulServer extends HttpServlet implements IRestfulServer theProviders) { + while (theProviders.size() > 0) { + unregisterProvider(theProviders.get(0)); + } + } + + private void writeExceptionToResponse(HttpServletResponse theResponse, BaseServerResponseException theException) throws IOException { theResponse.setStatus(theException.getStatusCode()); addHeadersToResponse(theResponse); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseMethodBinding.java index 19ec2bcced3..c346050a0dd 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseMethodBinding.java @@ -26,47 +26,44 @@ import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.api.Include; -import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.rest.annotation.*; -import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.api.EncodingEnum; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.IRestfulServer; import ca.uhn.fhir.rest.api.server.RequestDetails; -import ca.uhn.fhir.rest.client.exceptions.NonFhirResponseException; import ca.uhn.fhir.rest.server.BundleProviders; import ca.uhn.fhir.rest.server.IResourceProvider; import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; -import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.interceptor.IServerInterceptor.ActionRequestDetails; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.util.ReflectionUtil; -import org.apache.commons.io.IOUtils; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseResource; import javax.annotation.Nonnull; import java.io.IOException; -import java.io.Reader; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.util.*; - -import static org.apache.commons.lang3.StringUtils.isBlank; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; public abstract class BaseMethodBinding { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseMethodBinding.class); + private final List myQueryParameters; private FhirContext myContext; private Method myMethod; private List myParameters; private Object myProvider; private boolean mySupportsConditional; private boolean mySupportsConditionalMultiple; - public BaseMethodBinding(Method theMethod, FhirContext theContext, Object theProvider) { assert theMethod != null; assert theContext != null; @@ -75,6 +72,11 @@ public abstract class BaseMethodBinding { myContext = theContext; myProvider = theProvider; myParameters = MethodUtil.getResourceParameters(theContext, theMethod, theProvider, getRestOperationType()); + myQueryParameters = myParameters + .stream() + .filter(t -> t instanceof BaseQueryParameter) + .map(t -> (BaseQueryParameter) t) + .collect(Collectors.toList()); for (IParameter next : myParameters) { if (next instanceof ConditionalParamBinder) { @@ -90,6 +92,10 @@ public abstract class BaseMethodBinding { myMethod.setAccessible(true); } + protected List getQueryParameters() { + return myQueryParameters; + } + protected Object[] createMethodParams(RequestDetails theRequest) { Object[] params = new Object[getParameters().size()]; for (int i = 0; i < getParameters().size(); i++) { @@ -211,7 +217,7 @@ public abstract class BaseMethodBinding { return getRestOperationType(); } - public abstract boolean incomingServerRequestMatchesMethod(RequestDetails theRequest); + public abstract MethodMatchEnum incomingServerRequestMatchesMethod(RequestDetails theRequest); public abstract Object invokeServer(IRestfulServer theServer, RequestDetails theRequest) throws BaseServerResponseException, IOException; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java index ba9fe65d7c4..aaed5731a6a 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseOutcomeReturningMethodBinding.java @@ -111,20 +111,20 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding allowableRequestTypes = provideAllowableRequestTypes(); RequestTypeEnum requestType = theRequest.getRequestType(); if (!allowableRequestTypes.contains(requestType)) { - return false; + return MethodMatchEnum.NONE; } if (!getResourceName().equals(theRequest.getResourceName())) { - return false; + return MethodMatchEnum.NONE; } if (getMatchingOperation() == null && StringUtils.isNotBlank(theRequest.getOperation())) { - return false; + return MethodMatchEnum.NONE; } if (getMatchingOperation() != null && !getMatchingOperation().equals(theRequest.getOperation())) { - return false; + return MethodMatchEnum.NONE; } /* @@ -137,7 +137,7 @@ abstract class BaseOutcomeReturningMethodBinding extends BaseMethodBinding { } @Override - public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) { + public MethodMatchEnum incomingServerRequestMatchesMethod(RequestDetails theRequest) { if (Constants.OPERATION_NAME_GRAPHQL.equals(theRequest.getOperation())) { - return true; + return MethodMatchEnum.EXACT; } - return false; + return MethodMatchEnum.NONE; } @Override diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/HistoryMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/HistoryMethodBinding.java index f378ec57128..9a92fc2e163 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/HistoryMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/HistoryMethodBinding.java @@ -51,7 +51,7 @@ public class HistoryMethodBinding extends BaseResourceReturningMethodBinding { private final Integer myIdParamIndex; private final RestOperationTypeEnum myResourceOperationType; - private String myResourceName; + private final String myResourceName; public HistoryMethodBinding(Method theMethod, FhirContext theContext, Object theProvider) { super(toReturnType(theMethod, theProvider), theMethod, theContext, theProvider); @@ -105,30 +105,36 @@ public class HistoryMethodBinding extends BaseResourceReturningMethodBinding { // ObjectUtils.equals is replaced by a JDK7 method.. @Override - public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) { + public MethodMatchEnum incomingServerRequestMatchesMethod(RequestDetails theRequest) { if (!Constants.PARAM_HISTORY.equals(theRequest.getOperation())) { - return false; + return MethodMatchEnum.NONE; } if (theRequest.getResourceName() == null) { - return myResourceOperationType == RestOperationTypeEnum.HISTORY_SYSTEM; + if (myResourceOperationType == RestOperationTypeEnum.HISTORY_SYSTEM) { + return MethodMatchEnum.EXACT; + } else { + return MethodMatchEnum.NONE; + } } if (!StringUtils.equals(theRequest.getResourceName(), myResourceName)) { - return false; + return MethodMatchEnum.NONE; } boolean haveIdParam = theRequest.getId() != null && !theRequest.getId().isEmpty(); boolean wantIdParam = myIdParamIndex != null; if (haveIdParam != wantIdParam) { - return false; + return MethodMatchEnum.NONE; } if (theRequest.getId() == null) { - return myResourceOperationType == RestOperationTypeEnum.HISTORY_TYPE; + if (myResourceOperationType != RestOperationTypeEnum.HISTORY_TYPE) { + return MethodMatchEnum.NONE; + } } else if (theRequest.getId().hasVersionIdPart()) { - return false; + return MethodMatchEnum.NONE; } - return true; + return MethodMatchEnum.EXACT; } @@ -179,7 +185,7 @@ public class HistoryMethodBinding extends BaseResourceReturningMethodBinding { } if (isBlank(nextResource.getIdElement().getVersionIdPart()) && nextResource instanceof IResource) { //TODO: Use of a deprecated method should be resolved. - IdDt versionId = (IdDt) ResourceMetadataKeyEnum.VERSION_ID.get((IResource) nextResource); + IdDt versionId = ResourceMetadataKeyEnum.VERSION_ID.get((IResource) nextResource); if (versionId == null || versionId.isEmpty()) { throw new InternalErrorException("Server provided resource at index " + index + " with no Version ID set (using IResource#setId(IdDt))"); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/IncludeParameter.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/IncludeParameter.java index 6e3cb7fa1fe..2dfc38d4fc7 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/IncludeParameter.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/IncludeParameter.java @@ -106,6 +106,11 @@ class IncludeParameter extends BaseQueryParameter { return null; } + @Override + protected boolean supportsRepetition() { + return myInstantiableCollectionType != null; + } + @Override public boolean handlesMissing() { return true; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/MethodMatchEnum.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/MethodMatchEnum.java new file mode 100644 index 00000000000..335b38bf709 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/MethodMatchEnum.java @@ -0,0 +1,19 @@ +package ca.uhn.fhir.rest.server.method; + +public enum MethodMatchEnum { + + // Order these from worst to best! + + NONE, + APPROXIMATE, + EXACT; + + public MethodMatchEnum weakerOf(MethodMatchEnum theOther) { + if (this.ordinal() < theOther.ordinal()) { + return this; + } else { + return theOther; + } + } + +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/MethodUtil.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/MethodUtil.java index 87b648840bf..13a8ff92215 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/MethodUtil.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/MethodUtil.java @@ -36,7 +36,6 @@ import ca.uhn.fhir.rest.server.method.ResourceParameter.Mode; import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.util.ParametersUtil; import ca.uhn.fhir.util.ReflectionUtil; -import ca.uhn.fhir.util.ValidateUtil; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IPrimitiveType; @@ -144,7 +143,7 @@ public class MethodUtil { parameter.setRequired(true); parameter.setDeclaredTypes(((RequiredParam) nextAnnotation).targetTypes()); parameter.setCompositeTypes(((RequiredParam) nextAnnotation).compositeTypes()); - parameter.setChainlists(((RequiredParam) nextAnnotation).chainWhitelist(), ((RequiredParam) nextAnnotation).chainBlacklist()); + parameter.setChainLists(((RequiredParam) nextAnnotation).chainWhitelist(), ((RequiredParam) nextAnnotation).chainBlacklist()); parameter.setType(theContext, parameterType, innerCollectionType, outerCollectionType); MethodUtil.extractDescription(parameter, annotations); param = parameter; @@ -154,12 +153,12 @@ public class MethodUtil { parameter.setRequired(false); parameter.setDeclaredTypes(((OptionalParam) nextAnnotation).targetTypes()); parameter.setCompositeTypes(((OptionalParam) nextAnnotation).compositeTypes()); - parameter.setChainlists(((OptionalParam) nextAnnotation).chainWhitelist(), ((OptionalParam) nextAnnotation).chainBlacklist()); + parameter.setChainLists(((OptionalParam) nextAnnotation).chainWhitelist(), ((OptionalParam) nextAnnotation).chainBlacklist()); parameter.setType(theContext, parameterType, innerCollectionType, outerCollectionType); MethodUtil.extractDescription(parameter, annotations); param = parameter; } else if (nextAnnotation instanceof RawParam) { - param = new RawParamsParmeter(parameters); + param = new RawParamsParameter(parameters); } else if (nextAnnotation instanceof IncludeParam) { Class> instantiableCollectionType; Class specType; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/OperationMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/OperationMethodBinding.java index aa4b536bd1f..ee5f810d031 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/OperationMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/OperationMethodBinding.java @@ -226,43 +226,43 @@ public class OperationMethodBinding extends BaseResourceReturningMethodBinding { } @Override - public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) { + public MethodMatchEnum incomingServerRequestMatchesMethod(RequestDetails theRequest) { if (isBlank(theRequest.getOperation())) { - return false; + return MethodMatchEnum.NONE; } if (!myName.equals(theRequest.getOperation())) { if (!myName.equals(WILDCARD_NAME)) { - return false; + return MethodMatchEnum.NONE; } } if (getResourceName() == null) { if (isNotBlank(theRequest.getResourceName())) { if (!isGlobalMethod()) { - return false; + return MethodMatchEnum.NONE; } } } if (getResourceName() != null && !getResourceName().equals(theRequest.getResourceName())) { - return false; + return MethodMatchEnum.NONE; } RequestTypeEnum requestType = theRequest.getRequestType(); if (requestType != RequestTypeEnum.GET && requestType != RequestTypeEnum.POST) { // Operations can only be invoked with GET and POST - return false; + return MethodMatchEnum.NONE; } boolean requestHasId = theRequest.getId() != null; if (requestHasId) { - return myCanOperateAtInstanceLevel; + return myCanOperateAtInstanceLevel ? MethodMatchEnum.EXACT : MethodMatchEnum.NONE; } if (isNotBlank(theRequest.getResourceName())) { - return myCanOperateAtTypeLevel; + return myCanOperateAtTypeLevel ? MethodMatchEnum.EXACT : MethodMatchEnum.NONE; } - return myCanOperateAtServerLevel; + return myCanOperateAtServerLevel ? MethodMatchEnum.EXACT : MethodMatchEnum.NONE; } @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 5b0bd9355af..7bcf3756694 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 @@ -174,12 +174,16 @@ public class PageMethodBinding extends BaseResourceReturningMethodBinding { } @Override - public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) { + public MethodMatchEnum incomingServerRequestMatchesMethod(RequestDetails theRequest) { String[] pageId = theRequest.getParameters().get(Constants.PARAM_PAGINGACTION); if (pageId == null || pageId.length == 0 || isBlank(pageId[0])) { - return false; + return MethodMatchEnum.NONE; } - return theRequest.getRequestType() == RequestTypeEnum.GET; + if (theRequest.getRequestType() != RequestTypeEnum.GET) { + return MethodMatchEnum.NONE; + } + + return MethodMatchEnum.EXACT; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PatchMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PatchMethodBinding.java index e68351bd167..c04e01a7ee9 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PatchMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PatchMethodBinding.java @@ -80,9 +80,9 @@ public class PatchMethodBinding extends BaseOutcomeReturningMethodBindingWithRes } @Override - public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) { - boolean retVal = super.incomingServerRequestMatchesMethod(theRequest); - if (retVal) { + public MethodMatchEnum incomingServerRequestMatchesMethod(RequestDetails theRequest) { + MethodMatchEnum retVal = super.incomingServerRequestMatchesMethod(theRequest); + if (retVal.ordinal() > MethodMatchEnum.NONE.ordinal()) { PatchTypeParameter.getTypeForRequestOrThrowInvalidRequestException(theRequest); } return retVal; diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/RawParamsParmeter.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/RawParamsParameter.java similarity index 90% rename from hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/RawParamsParmeter.java rename to hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/RawParamsParameter.java index 1d8e992cad7..a74dba97398 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/RawParamsParmeter.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/RawParamsParameter.java @@ -35,11 +35,11 @@ import ca.uhn.fhir.rest.param.QualifierDetails; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -public class RawParamsParmeter implements IParameter { +public class RawParamsParameter implements IParameter { private final List myAllMethodParameters; - public RawParamsParmeter(List theParameters) { + public RawParamsParameter(List theParameters) { myAllMethodParameters = theParameters; } @@ -53,7 +53,7 @@ public class RawParamsParmeter implements IParameter { continue; } - QualifierDetails qualifiers = SearchMethodBinding.extractQualifiersFromParameterName(nextName); + QualifierDetails qualifiers = QualifierDetails.extractQualifiersFromParameterName(nextName); boolean alreadyCaptured = false; for (IParameter nextParameter : myAllMethodParameters) { @@ -70,7 +70,7 @@ public class RawParamsParmeter implements IParameter { if (!alreadyCaptured) { if (retVal == null) { - retVal = new HashMap>(); + retVal = new HashMap<>(); } retVal.put(nextName, Arrays.asList(theRequest.getParameters().get(nextName))); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ReadMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ReadMethodBinding.java index 538b8652c52..c0265121e70 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ReadMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ReadMethodBinding.java @@ -114,40 +114,40 @@ public class ReadMethodBinding extends BaseResourceReturningMethodBinding { } @Override - public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) { + public MethodMatchEnum incomingServerRequestMatchesMethod(RequestDetails theRequest) { if (!theRequest.getResourceName().equals(getResourceName())) { - return false; + return MethodMatchEnum.NONE; } for (String next : theRequest.getParameters().keySet()) { if (!next.startsWith("_")) { - return false; + return MethodMatchEnum.NONE; } } if (theRequest.getId() == null) { - return false; + return MethodMatchEnum.NONE; } if (mySupportsVersion == false) { if (theRequest.getId().hasVersionIdPart()) { - return false; + return MethodMatchEnum.NONE; } } if (isNotBlank(theRequest.getCompartmentName())) { - return false; + return MethodMatchEnum.NONE; } if (theRequest.getRequestType() != RequestTypeEnum.GET && theRequest.getRequestType() != RequestTypeEnum.HEAD ) { ourLog.trace("Method {} doesn't match because request type is not GET or HEAD: {}", theRequest.getId(), theRequest.getRequestType()); - return false; + return MethodMatchEnum.NONE; } if (Constants.PARAM_HISTORY.equals(theRequest.getOperation())) { if (mySupportsVersion == false) { - return false; + return MethodMatchEnum.NONE; } else if (theRequest.getId().hasVersionIdPart() == false) { - return false; + return MethodMatchEnum.NONE; } } else if (!StringUtils.isBlank(theRequest.getOperation())) { - return false; + return MethodMatchEnum.NONE; } - return true; + return MethodMatchEnum.EXACT; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java index b5692b75463..7d54efd256d 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchMethodBinding.java @@ -41,7 +41,11 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import javax.annotation.Nonnull; import java.lang.reflect.Method; -import java.util.*; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.isBlank; import static org.apache.commons.lang3.StringUtils.isNotBlank; @@ -61,11 +65,13 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { } private final String myResourceProviderResourceName; - private String myCompartmentName; + private final List myRequiredParamNames; + private final List myOptionalParamNames; + private final String myCompartmentName; private String myDescription; - private Integer myIdParamIndex; - private String myQueryName; - private boolean myAllowUnknownParams; + private final Integer myIdParamIndex; + private final String myQueryName; + private final boolean myAllowUnknownParams; public SearchMethodBinding(Class theReturnResourceType, Class theResourceProviderResourceType, Method theMethod, FhirContext theContext, Object theProvider) { super(theReturnResourceType, theMethod, theContext, theProvider); @@ -98,6 +104,17 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { this.myResourceProviderResourceName = null; } + myRequiredParamNames = getQueryParameters() + .stream() + .filter(t -> t.isRequired()) + .map(t -> t.getName()) + .collect(Collectors.toList()); + myOptionalParamNames = getQueryParameters() + .stream() + .filter(t -> !t.isRequired()) + .map(t -> t.getName()) + .collect(Collectors.toList()); + } public String getDescription() { @@ -129,122 +146,129 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { } @Override - public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) { + public MethodMatchEnum incomingServerRequestMatchesMethod(RequestDetails theRequest) { if (theRequest.getId() != null && myIdParamIndex == null) { ourLog.trace("Method {} doesn't match because ID is not null: {}", getMethod(), theRequest.getId()); - return false; + return MethodMatchEnum.NONE; } if (theRequest.getRequestType() == RequestTypeEnum.GET && theRequest.getOperation() != null && !Constants.PARAM_SEARCH.equals(theRequest.getOperation())) { ourLog.trace("Method {} doesn't match because request type is GET but operation is not null: {}", theRequest.getId(), theRequest.getOperation()); - return false; + return MethodMatchEnum.NONE; } if (theRequest.getRequestType() == RequestTypeEnum.POST && !Constants.PARAM_SEARCH.equals(theRequest.getOperation())) { ourLog.trace("Method {} doesn't match because request type is POST but operation is not _search: {}", theRequest.getId(), theRequest.getOperation()); - return false; + return MethodMatchEnum.NONE; } if (theRequest.getRequestType() != RequestTypeEnum.GET && theRequest.getRequestType() != RequestTypeEnum.POST) { ourLog.trace("Method {} doesn't match because request type is {}", getMethod(), theRequest.getRequestType()); - return false; + return MethodMatchEnum.NONE; } if (!StringUtils.equals(myCompartmentName, theRequest.getCompartmentName())) { ourLog.trace("Method {} doesn't match because it is for compartment {} but request is compartment {}", getMethod(), myCompartmentName, theRequest.getCompartmentName()); - return false; + return MethodMatchEnum.NONE; } if (theRequest.getParameters().get(Constants.PARAM_PAGINGACTION) != null) { - return false; + return MethodMatchEnum.NONE; } - // This is used to track all the parameters so we can reject queries that - // have additional params we don't understand - Set methodParamsTemp = new HashSet<>(); - - Set unqualifiedNames = theRequest.getUnqualifiedToQualifiedNames().keySet(); - Set qualifiedParamNames = theRequest.getParameters().keySet(); - for (IParameter nextParameter : getParameters()) { - if (!(nextParameter instanceof BaseQueryParameter)) { - continue; - } - BaseQueryParameter nextQueryParameter = (BaseQueryParameter) nextParameter; - String name = nextQueryParameter.getName(); - if (nextQueryParameter.isRequired()) { - - if (qualifiedParamNames.contains(name)) { - QualifierDetails qualifiers = extractQualifiersFromParameterName(name); - if (qualifiers.passes(nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist())) { - methodParamsTemp.add(name); - } - } - if (unqualifiedNames.contains(name)) { - List qualifiedNames = theRequest.getUnqualifiedToQualifiedNames().get(name); - qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist()); - methodParamsTemp.addAll(qualifiedNames); - } - if (!qualifiedParamNames.contains(name) && !unqualifiedNames.contains(name)) { - ourLog.trace("Method {} doesn't match param '{}' is not present", getMethod().getName(), name); - return false; - } - - } else { - if (qualifiedParamNames.contains(name)) { - QualifierDetails qualifiers = extractQualifiersFromParameterName(name); - if (qualifiers.passes(nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist())) { - methodParamsTemp.add(name); - } - } - if (unqualifiedNames.contains(name)) { - List qualifiedNames = theRequest.getUnqualifiedToQualifiedNames().get(name); - qualifiedNames = processWhitelistAndBlacklist(qualifiedNames, nextQueryParameter.getQualifierWhitelist(), nextQueryParameter.getQualifierBlacklist()); - methodParamsTemp.addAll(qualifiedNames); - } - if (!qualifiedParamNames.contains(name)) { - methodParamsTemp.add(name); - } - } - } if (myQueryName != null) { String[] queryNameValues = theRequest.getParameters().get(Constants.PARAM_QUERY); if (queryNameValues != null && StringUtils.isNotBlank(queryNameValues[0])) { String queryName = queryNameValues[0]; if (!myQueryName.equals(queryName)) { ourLog.trace("Query name does not match {}", myQueryName); - return false; + return MethodMatchEnum.NONE; } - methodParamsTemp.add(Constants.PARAM_QUERY); } else { ourLog.trace("Query name does not match {}", myQueryName); - return false; + return MethodMatchEnum.NONE; } } else { String[] queryNameValues = theRequest.getParameters().get(Constants.PARAM_QUERY); if (queryNameValues != null && StringUtils.isNotBlank(queryNameValues[0])) { ourLog.trace("Query has name"); - return false; + return MethodMatchEnum.NONE; } } - for (String next : theRequest.getParameters().keySet()) { - if (next.startsWith("_") && !SPECIAL_SEARCH_PARAMS.contains(truncModifierPart(next))) { - methodParamsTemp.add(next); - } - } - Set keySet = theRequest.getParameters().keySet(); - if (myAllowUnknownParams == false) { - for (String next : keySet) { - if (!methodParamsTemp.contains(next)) { - return false; + Set unqualifiedNames = theRequest.getUnqualifiedToQualifiedNames().keySet(); + Set qualifiedParamNames = theRequest.getParameters().keySet(); + + MethodMatchEnum retVal = MethodMatchEnum.EXACT; + for (String nextRequestParam : theRequest.getParameters().keySet()) { + String nextUnqualifiedRequestParam = ParameterUtil.stripModifierPart(nextRequestParam); + if (nextRequestParam.startsWith("_") && !SPECIAL_SEARCH_PARAMS.contains(nextUnqualifiedRequestParam)) { + continue; + } + + boolean parameterMatches = false; + boolean approx = false; + for (BaseQueryParameter nextMethodParam : getQueryParameters()) { + + if (nextRequestParam.equals(nextMethodParam.getName())) { + QualifierDetails qualifiers = QualifierDetails.extractQualifiersFromParameterName(nextRequestParam); + if (qualifiers.passes(nextMethodParam.getQualifierWhitelist(), nextMethodParam.getQualifierBlacklist())) { + parameterMatches = true; + } + } else if (nextUnqualifiedRequestParam.equals(nextMethodParam.getName())) { + List qualifiedNames = theRequest.getUnqualifiedToQualifiedNames().get(nextUnqualifiedRequestParam); + if (passesWhitelistAndBlacklist(qualifiedNames, nextMethodParam.getQualifierWhitelist(), nextMethodParam.getQualifierBlacklist())) { + parameterMatches = true; + } + } + + // Repetitions supplied by URL but not supported by this parameter + if (theRequest.getParameters().get(nextRequestParam).length > 1 != nextMethodParam.supportsRepetition()) { + approx = true; + } + + } + + + if (parameterMatches) { + + if (approx) { + retVal = retVal.weakerOf(MethodMatchEnum.APPROXIMATE); + } + + } else { + + if (myAllowUnknownParams) { + retVal = retVal.weakerOf(MethodMatchEnum.APPROXIMATE); + } else { + retVal = retVal.weakerOf(MethodMatchEnum.NONE); + } + + } + + if (retVal == MethodMatchEnum.NONE) { + break; + } + + } + + if (retVal != MethodMatchEnum.NONE) { + for (String nextRequiredParamName : myRequiredParamNames) { + if (!qualifiedParamNames.contains(nextRequiredParamName)) { + if (!unqualifiedNames.contains(nextRequiredParamName)) { + retVal = MethodMatchEnum.NONE; + break; + } } } } - return true; - } - - private String truncModifierPart(String param) { - int indexOfSeparator = param.indexOf(":"); - if (indexOfSeparator != -1) { - return param.substring(0, indexOfSeparator); + if (retVal != MethodMatchEnum.NONE) { + for (String nextRequiredParamName : myOptionalParamNames) { + if (!qualifiedParamNames.contains(nextRequiredParamName)) { + if (!unqualifiedNames.contains(nextRequiredParamName)) { + retVal = retVal.weakerOf(MethodMatchEnum.APPROXIMATE); + } + } + } } - return param; + + return retVal; } @Override @@ -264,19 +288,18 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { return false; } - private List processWhitelistAndBlacklist(List theQualifiedNames, Set theQualifierWhitelist, Set theQualifierBlacklist) { + + private boolean passesWhitelistAndBlacklist(List theQualifiedNames, Set theQualifierWhitelist, Set theQualifierBlacklist) { if (theQualifierWhitelist == null && theQualifierBlacklist == null) { - return theQualifiedNames; + return true; } - ArrayList retVal = new ArrayList<>(theQualifiedNames.size()); for (String next : theQualifiedNames) { - QualifierDetails qualifiers = extractQualifiersFromParameterName(next); + QualifierDetails qualifiers = QualifierDetails.extractQualifiersFromParameterName(next); if (!qualifiers.passes(theQualifierWhitelist, theQualifierBlacklist)) { - continue; + return false; } - retVal.add(next); } - return retVal; + return true; } @Override @@ -284,52 +307,5 @@ public class SearchMethodBinding extends BaseResourceReturningMethodBinding { return getMethod().toString(); } - 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)); - retVal.setParamName(theParamName.substring(0, dotIdx)); - retVal.setWholeQualifier(theParamName.substring(dotIdx)); - } else { - retVal.setColonQualifier(theParamName.substring(colonIdx, dotIdx)); - retVal.setDotQualifier(theParamName.substring(dotIdx)); - retVal.setParamName(theParamName.substring(0, colonIdx)); - retVal.setWholeQualifier(theParamName.substring(colonIdx)); - } - } else if (dotIdx != -1) { - retVal.setDotQualifier(theParamName.substring(dotIdx)); - retVal.setParamName(theParamName.substring(0, dotIdx)); - retVal.setWholeQualifier(theParamName.substring(dotIdx)); - } else if (colonIdx != -1) { - retVal.setColonQualifier(theParamName.substring(colonIdx)); - retVal.setParamName(theParamName.substring(0, colonIdx)); - retVal.setWholeQualifier(theParamName.substring(colonIdx)); - } else { - retVal.setParamName(theParamName); - retVal.setColonQualifier(null); - retVal.setDotQualifier(null); - retVal.setWholeQualifier(null); - } - - return retVal; - } - } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchParameter.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchParameter.java index 550fd5f15de..269f21b3047 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchParameter.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/SearchParameter.java @@ -42,8 +42,8 @@ import ca.uhn.fhir.util.ReflectionUtil; public class SearchParameter extends BaseQueryParameter { private static final String EMPTY_STRING = ""; - private static HashMap> ourParamQualifiers; - private static HashMap, RestSearchParameterTypeEnum> ourParamTypes; + private static final HashMap> ourParamQualifiers; + private static final HashMap, RestSearchParameterTypeEnum> ourParamTypes; static final String QUALIFIER_ANY_TYPE = ":*"; static { @@ -114,6 +114,7 @@ public class SearchParameter extends BaseQueryParameter { private Set myQualifierWhitelist; private boolean myRequired; private Class myType; + private boolean mySupportsRepetition = false; public SearchParameter() { } @@ -202,17 +203,17 @@ public class SearchParameter extends BaseQueryParameter { return myParamBinder.parse(theContext, getName(), theString); } - public void setChainlists(String[] theChainWhitelist, String[] theChainBlacklist) { + public void setChainLists(String[] theChainWhitelist, String[] theChainBlacklist) { myQualifierWhitelist = new HashSet<>(theChainWhitelist.length); myQualifierWhitelist.add(QUALIFIER_ANY_TYPE); - for (int i = 0; i < theChainWhitelist.length; i++) { - if (theChainWhitelist[i].equals(OptionalParam.ALLOW_CHAIN_ANY)) { + for (String nextChain : theChainWhitelist) { + if (nextChain.equals(OptionalParam.ALLOW_CHAIN_ANY)) { myQualifierWhitelist.add('.' + OptionalParam.ALLOW_CHAIN_ANY); - } else if (theChainWhitelist[i].equals(EMPTY_STRING)) { + } else if (nextChain.equals(EMPTY_STRING)) { myQualifierWhitelist.add("."); } else { - myQualifierWhitelist.add('.' + theChainWhitelist[i]); + myQualifierWhitelist.add('.' + nextChain); } } @@ -248,42 +249,48 @@ public class SearchParameter extends BaseQueryParameter { this.myRequired = required; } + @Override + protected boolean supportsRepetition() { + return mySupportsRepetition; + } + @SuppressWarnings("unchecked") - public void setType(FhirContext theContext, final Class type, Class> theInnerCollectionType, Class> theOuterCollectionType) { + public void setType(FhirContext theContext, final Class theType, Class> theInnerCollectionType, Class> theOuterCollectionType) { - this.myType = type; - if (IQueryParameterType.class.isAssignableFrom(type)) { - myParamBinder = new QueryParameterTypeBinder((Class) type, myCompositeTypes); - } else if (IQueryParameterOr.class.isAssignableFrom(type)) { - myParamBinder = new QueryParameterOrBinder((Class>) type, myCompositeTypes); - } else if (IQueryParameterAnd.class.isAssignableFrom(type)) { - myParamBinder = new QueryParameterAndBinder((Class>) type, myCompositeTypes); - } else if (String.class.equals(type)) { + this.myType = theType; + if (IQueryParameterType.class.isAssignableFrom(theType)) { + myParamBinder = new QueryParameterTypeBinder((Class) theType, myCompositeTypes); + } else if (IQueryParameterOr.class.isAssignableFrom(theType)) { + myParamBinder = new QueryParameterOrBinder((Class>) theType, myCompositeTypes); + } else if (IQueryParameterAnd.class.isAssignableFrom(theType)) { + myParamBinder = new QueryParameterAndBinder((Class>) theType, myCompositeTypes); + mySupportsRepetition = true; + } else if (String.class.equals(theType)) { myParamBinder = new StringBinder(); myParamType = RestSearchParameterTypeEnum.STRING; - } else if (Date.class.equals(type)) { + } else if (Date.class.equals(theType)) { myParamBinder = new DateBinder(); myParamType = RestSearchParameterTypeEnum.DATE; - } else if (Calendar.class.equals(type)) { + } else if (Calendar.class.equals(theType)) { myParamBinder = new CalendarBinder(); myParamType = RestSearchParameterTypeEnum.DATE; - } else if (IPrimitiveType.class.isAssignableFrom(type) && ReflectionUtil.isInstantiable(type)) { - RuntimePrimitiveDatatypeDefinition def = (RuntimePrimitiveDatatypeDefinition) theContext.getElementDefinition((Class>) type); + } else if (IPrimitiveType.class.isAssignableFrom(theType) && ReflectionUtil.isInstantiable(theType)) { + RuntimePrimitiveDatatypeDefinition def = (RuntimePrimitiveDatatypeDefinition) theContext.getElementDefinition((Class>) theType); if (def.getNativeType() != null) { if (def.getNativeType().equals(Date.class)) { - myParamBinder = new FhirPrimitiveBinder((Class>) type); + myParamBinder = new FhirPrimitiveBinder((Class>) theType); myParamType = RestSearchParameterTypeEnum.DATE; } else if (def.getNativeType().equals(String.class)) { - myParamBinder = new FhirPrimitiveBinder((Class>) type); + myParamBinder = new FhirPrimitiveBinder((Class>) theType); myParamType = RestSearchParameterTypeEnum.STRING; } } } else { - throw new ConfigurationException("Unsupported data type for parameter: " + type.getCanonicalName()); + throw new ConfigurationException("Unsupported data type for parameter: " + theType.getCanonicalName()); } - RestSearchParameterTypeEnum typeEnum = ourParamTypes.get(type); + RestSearchParameterTypeEnum typeEnum = ourParamTypes.get(theType); if (typeEnum != null) { Set builtInQualifiers = ourParamQualifiers.get(typeEnum); if (builtInQualifiers != null) { @@ -304,22 +311,22 @@ public class SearchParameter extends BaseQueryParameter { if (myParamType != null) { // ok - } else if (StringDt.class.isAssignableFrom(type)) { + } else if (StringDt.class.isAssignableFrom(theType)) { myParamType = RestSearchParameterTypeEnum.STRING; - } else if (BaseIdentifierDt.class.isAssignableFrom(type)) { + } else if (BaseIdentifierDt.class.isAssignableFrom(theType)) { myParamType = RestSearchParameterTypeEnum.TOKEN; - } else if (BaseQuantityDt.class.isAssignableFrom(type)) { + } else if (BaseQuantityDt.class.isAssignableFrom(theType)) { myParamType = RestSearchParameterTypeEnum.QUANTITY; - } else if (ReferenceParam.class.isAssignableFrom(type)) { + } else if (ReferenceParam.class.isAssignableFrom(theType)) { myParamType = RestSearchParameterTypeEnum.REFERENCE; - } else if (HasParam.class.isAssignableFrom(type)) { + } else if (HasParam.class.isAssignableFrom(theType)) { myParamType = RestSearchParameterTypeEnum.STRING; } else { - throw new ConfigurationException("Unknown search parameter type: " + type); + throw new ConfigurationException("Unknown search parameter theType: " + theType); } // NB: Once this is enabled, we should return true from handlesMissing if - // it's a collection type + // it's a collection theType // if (theInnerCollectionType != null) { // this.parser = new CollectionBinder(this.parser, theInnerCollectionType); // } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/TransactionMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/TransactionMethodBinding.java index 238ba0af398..7aca1821a09 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/TransactionMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/TransactionMethodBinding.java @@ -22,17 +22,13 @@ package ca.uhn.fhir.rest.server.method; import static org.apache.commons.lang3.StringUtils.isNotBlank; import java.lang.reflect.Method; -import java.util.IdentityHashMap; import java.util.List; import org.hl7.fhir.instance.model.api.IBaseResource; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.model.api.IResource; -import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; import ca.uhn.fhir.model.base.resource.BaseOperationOutcome; -import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.valueset.BundleTypeEnum; import ca.uhn.fhir.rest.annotation.Transaction; import ca.uhn.fhir.rest.annotation.TransactionParam; @@ -92,17 +88,17 @@ public class TransactionMethodBinding extends BaseResourceReturningMethodBinding } @Override - public boolean incomingServerRequestMatchesMethod(RequestDetails theRequest) { + public MethodMatchEnum incomingServerRequestMatchesMethod(RequestDetails theRequest) { if (theRequest.getRequestType() != RequestTypeEnum.POST) { - return false; + return MethodMatchEnum.NONE; } if (isNotBlank(theRequest.getOperation())) { - return false; + return MethodMatchEnum.NONE; } if (isNotBlank(theRequest.getResourceName())) { - return false; + return MethodMatchEnum.NONE; } - return true; + return MethodMatchEnum.EXACT; } @SuppressWarnings("unchecked") diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/MethodMatchEnumTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/MethodMatchEnumTest.java new file mode 100644 index 00000000000..d234922418c --- /dev/null +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/MethodMatchEnumTest.java @@ -0,0 +1,16 @@ +package ca.uhn.fhir.rest.server.method; + +import org.junit.Test; + +import static org.junit.Assert.*; + +public class MethodMatchEnumTest { + + @Test + public void testOrder() { + assertEquals(0, MethodMatchEnum.NONE.ordinal()); + assertEquals(1, MethodMatchEnum.APPROXIMATE.ordinal()); + assertEquals(2, MethodMatchEnum.EXACT.ordinal()); + } + +} diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/ReadMethodBindingTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/ReadMethodBindingTest.java index ba4ee6476ce..69aceeae851 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/ReadMethodBindingTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/ReadMethodBindingTest.java @@ -20,6 +20,7 @@ import org.mockito.junit.MockitoJUnitRunner; import java.lang.reflect.Method; import java.util.List; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -56,17 +57,17 @@ public class ReadMethodBindingTest { // Read ReadMethodBinding binding = createBinding(new MyProvider()); when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123")); - assertTrue(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + assertEquals(MethodMatchEnum.EXACT, binding.incomingServerRequestMatchesMethod(myRequestDetails)); // VRead when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123/_history/123")); - assertFalse(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + assertEquals(MethodMatchEnum.NONE, binding.incomingServerRequestMatchesMethod(myRequestDetails)); // Type history when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123")); when(myRequestDetails.getResourceName()).thenReturn("Patient"); when(myRequestDetails.getOperation()).thenReturn("_history"); - assertFalse(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + assertEquals(MethodMatchEnum.NONE, binding.incomingServerRequestMatchesMethod(myRequestDetails)); } @@ -89,27 +90,27 @@ public class ReadMethodBindingTest { ReadMethodBinding binding = createBinding(new MyProvider()); when(myRequestDetails.getResourceName()).thenReturn("Observation"); when(myRequestDetails.getId()).thenReturn(new IdDt("Observation/123")); - assertFalse(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + assertEquals(MethodMatchEnum.NONE, binding.incomingServerRequestMatchesMethod(myRequestDetails)); // Read when(myRequestDetails.getResourceName()).thenReturn("Patient"); when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123")); - assertTrue(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + assertEquals(MethodMatchEnum.EXACT, binding.incomingServerRequestMatchesMethod(myRequestDetails)); // VRead when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123/_history/123")); when(myRequestDetails.getOperation()).thenReturn("_history"); - assertTrue(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + assertEquals(MethodMatchEnum.EXACT, binding.incomingServerRequestMatchesMethod(myRequestDetails)); // Some other operation when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123/_history/123")); when(myRequestDetails.getOperation()).thenReturn("$foo"); - assertFalse(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + assertEquals(MethodMatchEnum.NONE, binding.incomingServerRequestMatchesMethod(myRequestDetails)); // History operation when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123")); when(myRequestDetails.getOperation()).thenReturn("_history"); - assertFalse(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + assertEquals(MethodMatchEnum.NONE, binding.incomingServerRequestMatchesMethod(myRequestDetails)); } @@ -130,7 +131,7 @@ public class ReadMethodBindingTest { when(myRequestDetails.getId()).thenReturn(new IdDt("Patient/123")); ReadMethodBinding binding = createBinding(new MyProvider()); - assertFalse(binding.incomingServerRequestMatchesMethod(myRequestDetails)); + assertEquals(MethodMatchEnum.NONE, binding.incomingServerRequestMatchesMethod(myRequestDetails)); } public ReadMethodBinding createBinding(Object theProvider) throws NoSuchMethodException { diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java index 30c722bf5ed..e97f416e462 100644 --- a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/method/SearchMethodBindingTest.java @@ -42,39 +42,39 @@ public class SearchMethodBindingTest { public void methodShouldNotMatchWhenUnderscoreQueryParameter() throws NoSuchMethodException { Assert.assertThat(getBinding("param", String.class).incomingServerRequestMatchesMethod( mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_include", new String[]{"test"}))), - Matchers.is(false)); + Matchers.is(MethodMatchEnum.NONE)); Assert.assertThat(getBinding("paramAndTest", String.class, String.class).incomingServerRequestMatchesMethod( mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_include", new String[]{"test"}))), - Matchers.is(false)); + Matchers.is(MethodMatchEnum.NONE)); Assert.assertThat(getBinding("paramAndUnderscoreTest", String.class, String.class).incomingServerRequestMatchesMethod( mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_include", new String[]{"test"}))), - Matchers.is(false)); + Matchers.is(MethodMatchEnum.NONE)); } @Test public void methodShouldNotMatchWhenExtraQueryParameter() throws NoSuchMethodException { Assert.assertThat(getBinding("param", String.class).incomingServerRequestMatchesMethod( mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "extra", new String[]{"test"}))), - Matchers.is(false)); + Matchers.is(MethodMatchEnum.NONE)); Assert.assertThat(getBinding("paramAndTest", String.class, String.class).incomingServerRequestMatchesMethod( mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "extra", new String[]{"test"}))), - Matchers.is(false)); + Matchers.is(MethodMatchEnum.NONE)); Assert.assertThat(getBinding("paramAndUnderscoreTest", String.class, String.class).incomingServerRequestMatchesMethod( mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "extra", new String[]{"test"}))), - Matchers.is(false)); + Matchers.is(MethodMatchEnum.NONE)); } @Test public void methodMatchesOwnParams() throws NoSuchMethodException { Assert.assertThat(getBinding("param", String.class).incomingServerRequestMatchesMethod( mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}))), - Matchers.is(true)); + Matchers.is(MethodMatchEnum.EXACT)); Assert.assertThat(getBinding("paramAndTest", String.class, String.class).incomingServerRequestMatchesMethod( mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "test", new String[]{"test"}))), - Matchers.is(true)); + Matchers.is(MethodMatchEnum.EXACT)); Assert.assertThat(getBinding("paramAndUnderscoreTest", String.class, String.class).incomingServerRequestMatchesMethod( mockSearchRequest(ImmutableMap.of("param", new String[]{"value"}, "_test", new String[]{"test"}))), - Matchers.is(true)); + Matchers.is(MethodMatchEnum.EXACT)); } @Test @@ -83,10 +83,10 @@ public class SearchMethodBindingTest { ourLog.info("Testing binding: {}", binding); Assert.assertThat(binding.incomingServerRequestMatchesMethod( mockSearchRequest(ImmutableMap.of("refChainBlacklist.badChain", new String[]{"foo"}))), - Matchers.is(false)); + Matchers.is(MethodMatchEnum.NONE)); Assert.assertThat(binding.incomingServerRequestMatchesMethod( mockSearchRequest(ImmutableMap.of("refChainBlacklist.goodChain", new String[]{"foo"}))), - Matchers.is(true)); + Matchers.is(MethodMatchEnum.EXACT)); } private SearchMethodBinding getBinding(String name, Class... parameters) throws NoSuchMethodException { diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/ServerSearchDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/ServerSearchDstu2Test.java index bde195f19d0..700221dc940 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/ServerSearchDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/ServerSearchDstu2Test.java @@ -156,53 +156,47 @@ public class ServerSearchDstu2Test { public static class DummyPatientResourceProvider { - //@formatter:off @Search(allowUnknownParams=true) public List searchParam1( @RequiredParam(name = "param1") StringParam theParam) { ourLastMethod = "searchParam1"; ourLastRef = theParam; - List retVal = new ArrayList(); + List retVal = new ArrayList<>(); Patient patient = new Patient(); patient.setId("123"); patient.addName().addGiven("GIVEN"); retVal.add(patient); return retVal; } - //@formatter:on - //@formatter:off @Search(allowUnknownParams=true) public List searchParam2( @RequiredParam(name = "param2") StringParam theParam) { ourLastMethod = "searchParam2"; ourLastRef = theParam; - List retVal = new ArrayList(); + List retVal = new ArrayList<>(); Patient patient = new Patient(); patient.setId("123"); patient.addName().addGiven("GIVEN"); retVal.add(patient); return retVal; } - //@formatter:on - //@formatter:off @Search(allowUnknownParams=true) public List searchParam3( @RequiredParam(name = "param3") ReferenceParam theParam) { ourLastMethod = "searchParam3"; ourLastRef2 = theParam; - List retVal = new ArrayList(); + List retVal = new ArrayList<>(); Patient patient = new Patient(); patient.setId("123"); patient.addName().addGiven("GIVEN"); retVal.add(patient); return retVal; } - //@formatter:on } 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 867fa36ed75..c62b872f10c 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 @@ -112,7 +112,6 @@ public class SearchCountParamDstu3Test { assertEquals("searchWithNoCountParam", ourLastMethod); assertEquals(null, ourLastParam); - //@formatter:off assertThat(responseContent, stringContainsInOrder( "", "", @@ -122,7 +121,6 @@ public class SearchCountParamDstu3Test { "", "", "")); - //@formatter:on } finally { IOUtils.closeQuietly(status.getEntity().getContent()); diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchMethodPriorityTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchMethodPriorityTest.java new file mode 100644 index 00000000000..abc313106ee --- /dev/null +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchMethodPriorityTest.java @@ -0,0 +1,221 @@ +package ca.uhn.fhir.rest.server; + +import ca.uhn.fhir.context.FhirVersionEnum; +import ca.uhn.fhir.rest.annotation.OptionalParam; +import ca.uhn.fhir.rest.annotation.RequiredParam; +import ca.uhn.fhir.rest.annotation.Search; +import ca.uhn.fhir.rest.client.api.IGenericClient; +import ca.uhn.fhir.rest.param.DateParam; +import ca.uhn.fhir.rest.param.DateRangeParam; +import ca.uhn.fhir.rest.param.StringAndListParam; +import ca.uhn.fhir.test.utilities.server.RestfulServerRule; +import com.google.common.collect.Lists; +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.Patient; +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; + +import java.util.List; + +import static org.junit.Assert.assertEquals; + +public class SearchMethodPriorityTest { + + @ClassRule + public static RestfulServerRule ourServerRule = new RestfulServerRule(FhirVersionEnum.R4); + + private String myLastMethod; + private IGenericClient myClient; + + @Before + public void before() { + myLastMethod = null; + myClient = ourServerRule.getFhirClient(); + } + + @After + public void after() { + ourServerRule.getRestfulServer().unregisterAllProviders(); + } + + @Test + public void testDateRangeSelectedWhenMultipleParametersProvided() { + ourServerRule.getRestfulServer().registerProviders(new DateStrengthsWithRequiredResourceProvider()); + + myClient + .search() + .forResource("Patient") + .where(Patient.BIRTHDATE.after().day("2001-01-01")) + .and(Patient.BIRTHDATE.before().day("2002-01-01")) + .returnBundle(Bundle.class) + .execute(); + + assertEquals("findDateRangeParam", myLastMethod); + } + + @Test + public void testDateRangeNotSelectedWhenSingleParameterProvided() { + ourServerRule.getRestfulServer().registerProviders(new DateStrengthsWithRequiredResourceProvider()); + + myClient + .search() + .forResource("Patient") + .where(Patient.BIRTHDATE.after().day("2001-01-01")) + .returnBundle(Bundle.class) + .execute(); + + assertEquals("findDateParam", myLastMethod); + } + + @Test + public void testEmptyDateSearchProvidedWithNoParameters() { + ourServerRule.getRestfulServer().registerProviders(new DateStrengthsWithRequiredResourceProvider()); + + myClient + .search() + .forResource("Patient") + .returnBundle(Bundle.class) + .execute(); + + assertEquals("find", myLastMethod); + } + + @Test + public void testStringAndListSelectedWhenMultipleParametersProvided() { + ourServerRule.getRestfulServer().registerProviders(new StringStrengthsWithOptionalResourceProvider()); + + myClient + .search() + .forResource("Patient") + .where(Patient.NAME.matches().value("hello")) + .and(Patient.NAME.matches().value("goodbye")) + .returnBundle(Bundle.class) + .execute(); + + assertEquals("findStringAndListParam", myLastMethod); + } + + @Test + public void testStringAndListNotSelectedWhenSingleParameterProvided() { + ourServerRule.getRestfulServer().registerProviders(new StringStrengthsWithOptionalResourceProvider()); + + myClient + .search() + .forResource("Patient") + .where(Patient.NAME.matches().value("hello")) + .returnBundle(Bundle.class) + .execute(); + + assertEquals("findString", myLastMethod); + } + + @Test + public void testEmptyStringSearchProvidedWithNoParameters() { + ourServerRule.getRestfulServer().registerProviders(new StringStrengthsWithOptionalResourceProvider()); + + myClient + .search() + .forResource("Patient") + .returnBundle(Bundle.class) + .execute(); + + assertEquals("find", myLastMethod); + } + + @Test + public void testEmptyStringSearchProvidedWithNoParameters2() { + ourServerRule.getRestfulServer().registerProviders(new StringStrengthsWithOptionalResourceProviderReverseOrder()); + + myClient + .search() + .forResource("Patient") + .returnBundle(Bundle.class) + .execute(); + + assertEquals("find", myLastMethod); + } + + public class DateStrengthsWithRequiredResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Patient.class; + } + + @Search + public List find() { + myLastMethod = "find"; + return Lists.newArrayList(); + } + + @Search() + public List findDateParam( + @RequiredParam(name = Patient.SP_BIRTHDATE) DateParam theDate) { + myLastMethod = "findDateParam"; + return Lists.newArrayList(); + } + + @Search() + public List findDateRangeParam( + @RequiredParam(name = Patient.SP_BIRTHDATE) DateRangeParam theRange) { + myLastMethod = "findDateRangeParam"; + return Lists.newArrayList(); + } + + } + + public class StringStrengthsWithOptionalResourceProvider implements IResourceProvider { + + @Override + public Class getResourceType() { + return Patient.class; + } + + @Search() + public List findString( + @OptionalParam(name = Patient.SP_NAME) String theDate) { + myLastMethod = "findString"; + return Lists.newArrayList(); + } + + @Search() + public List findStringAndListParam( + @OptionalParam(name = Patient.SP_NAME) StringAndListParam theRange) { + myLastMethod = "findStringAndListParam"; + return Lists.newArrayList(); + } + + @Search + public List find() { + myLastMethod = "find"; + return Lists.newArrayList(); + } + + } + + + public class StringStrengthsWithOptionalResourceProviderReverseOrder implements IResourceProvider { + + @Override + public Class getResourceType() { + return Patient.class; + } + + @Search() + public List findA( + @OptionalParam(name = Patient.SP_NAME) String theDate) { + myLastMethod = "findString"; + return Lists.newArrayList(); + } + + @Search + public List findB() { + myLastMethod = "find"; + return Lists.newArrayList(); + } + + } + +} diff --git a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/server/RestfulServerRule.java b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/server/RestfulServerRule.java index cfeb6487628..3922e8f0ab9 100644 --- a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/server/RestfulServerRule.java +++ b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/server/RestfulServerRule.java @@ -21,10 +21,12 @@ package ca.uhn.fhir.test.utilities.server; */ import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.rest.client.api.IGenericClient; import ca.uhn.fhir.rest.client.api.ServerValidationModeEnum; import ca.uhn.fhir.rest.server.RestfulServer; import ca.uhn.fhir.test.utilities.JettyUtil; +import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.time.DateUtils; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; @@ -43,35 +45,66 @@ import java.util.concurrent.TimeUnit; public class RestfulServerRule implements TestRule { private static final Logger ourLog = LoggerFactory.getLogger(RestfulServerRule.class); - private final FhirContext myFhirContext; - private final Object[] myProviders; + private FhirContext myFhirContext; + private Object[] myProviders; + private FhirVersionEnum myFhirVersion; private Server myServer; private RestfulServer myServlet; private int myPort; private CloseableHttpClient myHttpClient; private IGenericClient myFhirClient; + /** + * Constructor + */ public RestfulServerRule(FhirContext theFhirContext, Object... theProviders) { + Validate.notNull(theFhirContext); myFhirContext = theFhirContext; myProviders = theProviders; } + /** + * Constructor: If this is used, it will create and tear down a FhirContext which is good for memory + */ + public RestfulServerRule(FhirVersionEnum theFhirVersionEnum) { + Validate.notNull(theFhirVersionEnum); + myFhirVersion = theFhirVersionEnum; + } + @Override public Statement apply(Statement theBase, Description theDescription) { return new Statement() { @Override public void evaluate() throws Throwable { + createContextIfNeeded(); startServer(); theBase.evaluate(); stopServer(); + destroyContextIfWeCreatedIt(); } }; } + private void createContextIfNeeded() { + if (myFhirVersion != null) { + myFhirContext = new FhirContext(myFhirVersion); + } + } + + private void destroyContextIfWeCreatedIt() { + if (myFhirVersion != null) { + myFhirContext = null; + } + } + private void stopServer() throws Exception { JettyUtil.closeServer(myServer); + myServer = null; + myFhirClient = null; + myHttpClient.close(); + myHttpClient = null; } private void startServer() throws Exception { @@ -80,7 +113,9 @@ public class RestfulServerRule implements TestRule { ServletHandler servletHandler = new ServletHandler(); myServlet = new RestfulServer(myFhirContext); myServlet.setDefaultPrettyPrint(true); - myServlet.registerProviders(myProviders); + if (myProviders != null) { + myServlet.registerProviders(myProviders); + } ServletHolder servletHolder = new ServletHolder(myServlet); servletHandler.addServletWithMapping(servletHolder, "/*");