Simplify dispatching of sync/async handlers

core/src/main/java/uk/gov/justice/services/core/dispatcher/Dispatcher.java

@@ -1,13 +1,9 @@
 package uk.gov.justice.services.core.dispatcher;
 
-import uk.gov.justice.services.core.handler.AsynchronousHandlerMethod;
-import uk.gov.justice.services.core.handler.SynchronousHandlerMethod;
-import uk.gov.justice.services.core.handler.exception.MissingHandlerException;
+import uk.gov.justice.services.core.handler.HandlerMethod;
 import uk.gov.justice.services.core.handler.registry.HandlerRegistry;
 import uk.gov.justice.services.messaging.Envelope;
 
-import javax.enterprise.inject.Alternative;
-
 /**
  * Dispatches messages synchronously or asynchronously to their corresponding handlers, which could
  * be a command handler, command controller, event processor, etc.
@@ -29,10 +25,14 @@ class Dispatcher {
      * Asynchronously dispatch message to its corresponding handler, which could be a command
      * handler, command controller, event processor, etc.
      *
+     * The underlying {@link HandlerMethod} will have returned a null {@link Void}, which we throw
+     * away at this point to provide void method that can be exposed via the
+     * {@link AsynchronousDispatcher} interface.
+     *
      * @param envelope the envelope to dispatch to a handler
      */
     void asynchronousDispatch(final Envelope envelope) {
-        getAsynchronousMethod(envelope).execute(envelope);
+        getMethod(envelope, false).execute(envelope);
     }
 
     /**
@@ -43,7 +43,7 @@ void asynchronousDispatch(final Envelope envelope) {
      * @return the envelope returned by the handler method
      */
     Envelope synchronousDispatch(final Envelope envelope) {
-        return getSynchronousMethod(envelope).execute(envelope);
+        return (Envelope) getMethod(envelope, true).execute(envelope);
     }
 
     /**
@@ -63,31 +63,8 @@ void register(final Object handler) {
      * @param envelope the envelope to be handled
      * @return the handler method
      */
-    private AsynchronousHandlerMethod getAsynchronousMethod(final Envelope envelope) {
-
+    private HandlerMethod getMethod(final Envelope envelope, final boolean isSynchronous) {
         final String name = envelope.metadata().name();
-
-        if (handlerRegistry.canHandleAsynchronous(name)) {
-            return handlerRegistry.getAsynchronous(name);
-        } else {
-            throw new MissingHandlerException("No handler registered to handle action: " + name);
-        }
-    }
-
-    /**
-     * Get the handler method for handling this envelope or throw an exception.
-     *
-     * @param envelope the envelope to be handled
-     * @return the handler method
-     */
-    private SynchronousHandlerMethod getSynchronousMethod(final Envelope envelope) {
-
-        final String name = envelope.metadata().name();
-
-        if (handlerRegistry.canHandleSynchronous(name)) {
-            return handlerRegistry.getSynchronous(name);
-        } else {
-            throw new MissingHandlerException("No handler registered to handle action: " + name);
-        }
+        return handlerRegistry.get(name, isSynchronous);
     }
 }

core/src/main/java/uk/gov/justice/services/core/handler/HandlerMethod.java

@@ -2,52 +2,107 @@
 
 import static java.lang.String.format;
 
+import uk.gov.justice.services.core.handler.exception.HandlerExecutionException;
 import uk.gov.justice.services.core.handler.registry.exception.InvalidHandlerException;
 import uk.gov.justice.services.messaging.Envelope;
 
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 
 /**
- * Base class for synchronous and asynchronous handler methods.
+ * Encapsulates a handler class instance and a handler method.
+ *
+ * Asynchronous handler methods will return a null {@link Void} whereas synchronous handler methods
+ * must return an {@link Envelope}.
  */
-public abstract class HandlerMethod {
+public class HandlerMethod {
 
-    protected final Object handlerInstance;
-    protected final Method handlerMethod;
+    private final Object handlerInstance;
+    private final Method handlerMethod;
+
+    private final boolean isSynchronous;
 
     /**
-     * Constructor with common handler method validation.
+     * Constructor with handler method validation.
      *
-     * @param handlerInstance the instance of the handler object
-     * @param handlerMethod   the method on the handler object
+     * @param object             the instance of the handler object
+     * @param method             the method on the handler object
+     * @param expectedReturnType the expected return type for the method
      */
-    HandlerMethod(final Object handlerInstance, final Method handlerMethod) {
+    public HandlerMethod(final Object object, final Method method, final Class<?> expectedReturnType) {
 
-        if (handlerInstance == null) {
+        if (object == null) {
             throw new IllegalArgumentException("Handler instance cannot be null");
         }
 
-        if (handlerMethod == null) {
+        if (method == null) {
             throw new IllegalArgumentException("Handler method cannot be null");
         }
 
-        if (handlerMethod.getParameterTypes().length != 1) {
-            throw new InvalidHandlerException("Handles method must have exactly one parameter. Found " + handlerMethod.getParameterTypes().length);
+        if (method.getParameterTypes().length != 1) {
+            throw new InvalidHandlerException(
+                    format("Handles method must have exactly one parameter; found %d",
+                            method.getParameterTypes().length));
+        }
+
+        final Class<?> paramClass = method.getParameterTypes()[0];
+        if (paramClass != Envelope.class) {
+            throw new IllegalArgumentException("Handler methods must receive Envelope as an argument");
+        }
+
+        if (isVoid(expectedReturnType) && !isVoid(method.getReturnType())) {
+            throw new InvalidHandlerException("Asynchronous handler must return void");
         }
+        if (!isVoid(expectedReturnType) && !isEnvelope(expectedReturnType)) {
+            throw new IllegalArgumentException("Synchronous handler method must handle envelopes");
+        }
+        if (!isVoid(expectedReturnType) && !isEnvelope(method.getReturnType())) {
+            throw new InvalidHandlerException("Synchronous handler must return an envelope");
+        }
+
+        this.handlerInstance = object;
+        this.handlerMethod = method;
+        this.isSynchronous = !isVoid(expectedReturnType);
+    }
 
-        final Class<?> clazz = handlerMethod.getParameterTypes()[0];
-        if (clazz != Envelope.class) {
-            throw new IllegalArgumentException("Handler methods must receive Envelope as an argument.");
+    /**
+     * Invokes the handler method passing the <code>envelope</code> to it.
+     *
+     * @param envelope the envelope that is passed to the handler method
+     * @return the result of invoking the handler, which will either be an {@link Envelope} or a
+     * null {@link Void}
+     */
+    @SuppressWarnings("unchecked")
+    public Object execute(final Envelope envelope) {
+        try {
+            return handlerMethod.invoke(handlerInstance, envelope);
+        } catch (IllegalAccessException | InvocationTargetException ex) {
+            throw new HandlerExecutionException(
+                    format("Error while invoking command handler method %s with parameter %s",
+                            handlerMethod, envelope), ex);
         }
+    }
 
-        this.handlerInstance = handlerInstance;
-        this.handlerMethod = handlerMethod;
+    /**
+     * Check if this handler method is synchronous.
+     * @return true if the method returns a value
+     */
+    public boolean isSynchronous() {
+        return isSynchronous;
     }
 
     @Override
     public String toString() {
-        return format("HandlerInstanceAndMethod[ Class:%s method:%s]", handlerInstance != null ? handlerInstance.getClass().getName() : null,
+        return format("HandlerMethod[ Class: %s method: %s]",
+                handlerInstance != null ? handlerInstance.getClass().getName() : null,
                 handlerMethod != null ? handlerMethod.getName() : null);
     }
 
+    private static boolean isVoid(final Class<?> clazz) {
+        return Void.TYPE.equals(clazz);
+    }
+
+    private static boolean isEnvelope(final Class<?> clazz) {
+        return Envelope.class.equals(clazz);
+    }
 }

core/src/main/java/uk/gov/justice/services/core/handler/registry/HandlerRegistry.java

@@ -1,47 +1,37 @@
 package uk.gov.justice.services.core.handler.registry;
 
+import static java.lang.String.format;
 import static uk.gov.justice.services.core.handler.HandlerUtil.findHandlerMethods;
 
 import uk.gov.justice.services.core.annotation.Handles;
-import uk.gov.justice.services.core.handler.AsynchronousHandlerMethod;
 import uk.gov.justice.services.core.handler.HandlerMethod;
-import uk.gov.justice.services.core.handler.SynchronousHandlerMethod;
+import uk.gov.justice.services.core.handler.exception.MissingHandlerException;
 import uk.gov.justice.services.core.handler.registry.exception.DuplicateHandlerException;
 
 import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.function.BiFunction;
 
 /**
  * Service for storing a map of which command handlers handle which commands.
  */
 public class HandlerRegistry {
 
-    private final Map<String, AsynchronousHandlerMethod> asynchronousMethods = new HashMap<>();
-    private final Map<String, SynchronousHandlerMethod> synchronousMethods = new HashMap<>();
+    private final Map<String, HandlerMethod> handlerMethods = new HashMap<>();
 
-    public AsynchronousHandlerMethod getAsynchronous(final String name) {
-        return asynchronousMethods.get(name);
-    }
-
-    public SynchronousHandlerMethod getSynchronous(final String name) {
-        return synchronousMethods.get(name);
-    }
-
-    public boolean canHandleAsynchronous(final String name) {
-        return asynchronousMethods.containsKey(name);
-    }
-
-    public boolean canHandleSynchronous(final String name) {
-        return synchronousMethods.containsKey(name);
+    public HandlerMethod get(final String name, final boolean isSynchronous) {
+        if (canHandle(name, isSynchronous)) {
+            return handlerMethods.get(name);
+        } else {
+            throw new MissingHandlerException("No handler registered to handle action: " + name);
+        }
     }
 
     /**
      * Registers a handler instance.
      *
-     * @param handlerInstance Handler instance to be registered.
+     * @param handlerInstance handler instance to be registered.
      */
     public void register(final Object handlerInstance) {
         final List<Method> handlerMethods = findHandlerMethods(handlerInstance.getClass(), Handles.class);
@@ -55,29 +45,22 @@ public void register(final Object handlerInstance) {
      * Registers handler instance and method.
      *
      * @param handler handler instance to register.
-     * @param method handler method to register.
+     * @param method  handler method to register.
      */
     private void register(final Object handler, final Method method) {
 
-        if (Void.TYPE.equals(method.getReturnType())) {
-            put(handler, method, AsynchronousHandlerMethod::new, asynchronousMethods);
-        } else {
-            put(handler, method, SynchronousHandlerMethod::new, synchronousMethods);
-        }
-    }
-
-    private <T extends HandlerMethod> void put(final Object handler,
-                                               final Method method,
-                                               final BiFunction<Object, Method, T> constructor,
-                                               final Map<String, T> map) {
-
-        T handlerMethod = constructor.apply(handler, method);
+        HandlerMethod handlerMethod = new HandlerMethod(handler, method, method.getReturnType());
         String name = method.getAnnotation(Handles.class).value();
-        if (map.containsKey(name)) {
-            throw new DuplicateHandlerException("Can't register " + handlerMethod + ", because a command handler method "
-                    + map.get(name) + " has already been registered for " + name);
+        if (handlerMethods.containsKey(name)) {
+            throw new DuplicateHandlerException(
+                    format("Can't register %s because a command handler method %s has " +
+                            "already been registered for %s ", handlerMethod, handlerMethods.get(name), name));
         }
 
-        map.put(name, handlerMethod);
+        handlerMethods.put(name, handlerMethod);
+    }
+
+    private boolean canHandle(final String name, final boolean isSynchronous) {
+        return handlerMethods.containsKey(name) && isSynchronous == handlerMethods.get(name).isSynchronous();
     }
 }