From b59f2d05a7d0fd10c7b03bb6f0ebf97757172a71 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Sat, 30 Apr 2022 16:46:37 -0400 Subject: [PATCH] =?UTF-8?q?reversed=20order=20matching=20operation=20bindi?= =?UTF-8?q?ngs=20are=20searched=20so=20custom=20ope=E2=80=A6=20(#3568)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * reversed order matching operation bindings are searched so custom operations override builtin ones * change log * add test * reverse finding operations in interfaces and then classes so last-in-wins finds the right provider method --- .../6_0_0/3568-reverse-operation-order.yaml | 4 ++ .../uhn/fhir/rest/server/ResourceBinding.java | 22 +++------- .../uhn/fhir/rest/server/RestfulServer.java | 8 ++-- .../fhir/rest/server/ResourceBindingTest.java | 41 +++++++++++++++++++ .../rest/server/OperationServerDstu3Test.java | 4 +- 5 files changed, 56 insertions(+), 23 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3568-reverse-operation-order.yaml create mode 100644 hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/ResourceBindingTest.java diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3568-reverse-operation-order.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3568-reverse-operation-order.yaml new file mode 100644 index 00000000000..03c2f7110b3 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3568-reverse-operation-order.yaml @@ -0,0 +1,4 @@ +type: change +issue: 3568 +title: "Modified operation method binding so that later registered custom providers take precedence over earlier registered ones. +In particular, this means that custom operations can now override built-in operations." 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 7039e449749..d569f3cc360 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 @@ -20,13 +20,13 @@ package ca.uhn.fhir.rest.server; * #L% */ -import java.util.ArrayList; -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; +import java.util.LinkedList; +import java.util.List; + /** * Holds all method bindings for an individual resource type */ @@ -35,7 +35,7 @@ public class ResourceBinding { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ResourceBinding.class); private String resourceName; - private List> myMethodBindings = new ArrayList<>(); + private LinkedList> myMethodBindings = new LinkedList<>(); /** * Constructor @@ -44,14 +44,6 @@ public class ResourceBinding { super(); } - /** - * Constructor - */ - public ResourceBinding(String resourceName, List> methods) { - this.resourceName = resourceName; - this.myMethodBindings = methods; - } - public BaseMethodBinding getMethod(RequestDetails theRequest) { if (null == myMethodBindings) { ourLog.warn("No methods exist for resource: {}", resourceName); @@ -95,12 +87,8 @@ public class ResourceBinding { return myMethodBindings; } - public void setMethods(List> methods) { - this.myMethodBindings = methods; - } - public void addMethod(BaseMethodBinding method) { - this.myMethodBindings.add(method); + this.myMethodBindings.push(method); } @Override 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 b79a61d77a5..bd7ff66546e 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 @@ -20,12 +20,12 @@ package ca.uhn.fhir.rest.server; * #L% */ -import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.context.api.AddProfileTagEnum; import ca.uhn.fhir.context.api.BundleInclusionRule; +import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.interceptor.api.HookParams; import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.interceptor.api.Pointcut; @@ -437,14 +437,14 @@ public class RestfulServer extends HttpServlet implements IRestfulServer clazz = theProvider.getClass(); Class supertype = clazz.getSuperclass(); while (!Object.class.equals(supertype)) { - count += findResourceMethods(theProvider, supertype); count += findResourceMethodsOnInterfaces(theProvider, supertype.getInterfaces()); + count += findResourceMethods(theProvider, supertype); supertype = supertype.getSuperclass(); } try { - count += findResourceMethods(theProvider, clazz); count += findResourceMethodsOnInterfaces(theProvider, clazz.getInterfaces()); + count += findResourceMethods(theProvider, clazz); } catch (ConfigurationException e) { throw new ConfigurationException(Msg.code(288) + "Failure scanning class " + clazz.getSimpleName() + ": " + e.getMessage(), e); } @@ -456,8 +456,8 @@ public class RestfulServer extends HttpServlet implements IRestfulServer[] interfaces) { int count = 0; for (Class anInterface : interfaces) { - count += findResourceMethods(theProvider, anInterface); count += findResourceMethodsOnInterfaces(theProvider, anInterface.getInterfaces()); + count += findResourceMethods(theProvider, anInterface); } return count; } diff --git a/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/ResourceBindingTest.java b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/ResourceBindingTest.java new file mode 100644 index 00000000000..5a1d4e369d6 --- /dev/null +++ b/hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/ResourceBindingTest.java @@ -0,0 +1,41 @@ +package ca.uhn.fhir.rest.server; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.rest.server.method.BaseMethodBinding; +import ca.uhn.fhir.rest.server.method.PageMethodBinding; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.lang.reflect.Method; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +@ExtendWith(MockitoExtension.class) +class ResourceBindingTest { + @Mock + FhirContext ourFhirContext; + + ResourceBinding myResourceBinding = new ResourceBinding(); + + @Test + public void testFILO() throws NoSuchMethodException { + // setup + Method method = ResourceBindingTest.class.getMethod("testFILO"); + BaseMethodBinding first = new PageMethodBinding(ourFhirContext, method); + BaseMethodBinding second = new PageMethodBinding(ourFhirContext, method);; + + // execute + myResourceBinding.addMethod(first); + myResourceBinding.addMethod(second); + + // verify + List> list = myResourceBinding.getMethodBindings(); + assertNotEquals(first, second); + assertEquals(second, list.get(0)); + assertEquals(first, list.get(1)); + } +} diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/OperationServerDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/OperationServerDstu3Test.java index 75e16a9de2e..b9f98d0e72f 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/OperationServerDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/server/OperationServerDstu3Test.java @@ -559,7 +559,7 @@ public class OperationServerDstu3Test { servlet.setFhirContext(ourCtx); servlet.setResourceProviders(new PatientProvider()); - servlet.setPlainProviders(new PlainProvider()); + servlet.registerProvider(new PlainProvider()); ServletHolder servletHolder = new ServletHolder(servlet); proxyHandler.addServletWithMapping(servletHolder, "/*"); ourServer.setHandler(proxyHandler); @@ -676,7 +676,7 @@ public class OperationServerDstu3Test { ) { //@formatter:on - ourLastMethod = "$OP_TYPE"; + ourLastMethod = "$OP_TYPE_ONLY_STRING"; ourLastParam1 = theParam1; Parameters retVal = new Parameters();