reversed order matching operation bindings are searched so custom ope… (#3568)

* 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

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."

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<BaseMethodBinding<?>> myMethodBindings = new ArrayList<>();
+	private LinkedList<BaseMethodBinding<?>> myMethodBindings = new LinkedList<>();
 
 	/**
 	 * Constructor
@@ -44,14 +44,6 @@ public class ResourceBinding {
 		super();
 	}
 
-	/**
-	 * Constructor
-	 */
-	public ResourceBinding(String resourceName, List<BaseMethodBinding<?>> 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<BaseMethodBinding<?>> methods) {
-		this.myMethodBindings = methods;
-	}
-
 	public void addMethod(BaseMethodBinding<?> method) {
-		this.myMethodBindings.add(method);
+		this.myMethodBindings.push(method);
 	}
 
 	@Override

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<Servlet
 		Class<?> 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<Servlet
 	private int findResourceMethodsOnInterfaces(Object theProvider, Class<?>[] interfaces) {
 		int count = 0;
 		for (Class<?> anInterface : interfaces) {
-			count += findResourceMethods(theProvider, anInterface);
 			count += findResourceMethodsOnInterfaces(theProvider, anInterface.getInterfaces());
+			count += findResourceMethods(theProvider, anInterface);
 		}
 		return count;
 	}

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<BaseMethodBinding<?>> list = myResourceBinding.getMethodBindings();
+		assertNotEquals(first, second);
+		assertEquals(second, list.get(0));
+		assertEquals(first, list.get(1));
+	}
+}

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();