Check if handler method exists in MethodToCall

By doing this in our value object instead of our middleware, we make
this more reusable for custom scenarios AND we simplify the middleware,
which should help encourage folks to write their own custom ones
instead.

src/Handler/CommandHandlerMiddleware.php

@@ -5,6 +5,7 @@
 namespace League\Tactician\Handler;
 
 use League\Tactician\Handler\Mapping\CommandToHandlerMapping;
+use League\Tactician\Handler\Mapping\MethodDoesNotExist;
 use League\Tactician\Middleware;
 use Psr\Container\ContainerInterface;
 use function get_class;
@@ -30,7 +31,7 @@ public function __construct(ContainerInterface $container, CommandToHandlerMappi
      * Executes a command and optionally returns a value
      *
      * @return mixed
-     * @throws CanNotInvokeHandler
+     * @throws MethodDoesNotExist
      */
     public function execute(object $command, callable $next)
     {
@@ -39,15 +40,6 @@ public function execute(object $command, callable $next)
         $handler = $this->container->get($handlerMethod->getClassName());
         $methodName = $handlerMethod->getMethodName();
 
-        // is_callable is used here instead of method_exists, as method_exists
-        // will fail when given a handler that relies on __call.
-        if (! \is_callable([$handler, $methodName])) {
-            throw CanNotInvokeHandler::forCommand(
-                $command,
-                'Method ' . $methodName . ' does not exist on handler'
-            );
-        }
-
         return $handler->{$methodName}($command);
     }
 }

src/Handler/Mapping/MethodDoesNotExist.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace League\Tactician\Handler\Mapping;
+
+use BadMethodCallException;
+use League\Tactician\Exception;
+
+/**
+ * Thrown when a specific handler object can not be used on a command object.
+ *
+ * The most common reason is the receiving method is missing or incorrectly
+ * named.
+ */
+final class MethodDoesNotExist extends BadMethodCallException implements Exception
+{
+    /** @var object */
+    private $command;
+
+    public static function on(string $className, string $methodName): self
+    {
+        return new self(
+            "The handler method {$className}::{$methodName} does not exist. Please check your Tactician mapping " .
+            "configuration or check verify that {$methodName} is actually declared in {$className}"
+        );
+    }
+}

src/Handler/Mapping/MethodToCall.php

@@ -3,7 +3,7 @@
 
 namespace League\Tactician\Handler\Mapping;
 
-use League\Tactician\Handler\CanNotInvokeHandler;
+use function method_exists;
 
 final class MethodToCall
 {
@@ -15,6 +15,13 @@ final class MethodToCall
 
     public function __construct(string $className, string $methodName)
     {
+        // If the method does not actually exist, we'll also check if __call exists (mainly for
+        // legacy purposes). That said, we won't rewrite the method name to __call because our
+        // static analysis checker might still be able to infer data from the original method name.
+        if (!method_exists($className, $methodName) && !method_exists($className, '__call')) {
+            throw MethodDoesNotExist::on($className, $methodName);
+        }
+
         $this->className = $className;
         $this->methodName = $methodName;
     }

tests/Fixtures/Handler/DynamicMethodsHandler.php

@@ -11,22 +11,10 @@
  */
 class DynamicMethodsHandler
 {
-    /** @var string[] */
-    private $methods = [];
-
-    /**
-     * @return string[]
-     */
-    public function getMethodsInvoked() : array
-    {
-        return $this->methods;
-    }
-
     /**
      * @param mixed[] $args
      */
     public function __call(string $methodName, array $args) : void
     {
-        $this->methods[] = $methodName;
     }
 }

tests/Handler/CommandHandlerMiddlewareTest.php

@@ -4,7 +4,7 @@
 
 namespace League\Tactician\Tests\Handler;
 
-use League\Tactician\Handler\CanNotInvokeHandler;
+use League\Tactician\Handler\Mapping\MethodDoesNotExist;
 use League\Tactician\Handler\CommandHandlerMiddleware;
 use League\Tactician\Handler\Mapping\CommandToHandlerMapping;
 use League\Tactician\Handler\Mapping\MethodToCall;
@@ -58,42 +58,6 @@ public function testHandlerIsExecuted(): void
         self::assertEquals('a-return-value', $this->middleware->execute($command, $this->mockNext()));
     }
 
-    public function testMissingMethodOnHandlerObjectIsDetected(): void
-    {
-        $command = new CompleteTaskCommand();
-
-        $this->container
-            ->method('get')
-            ->willReturn(new stdClass());
-
-        $this->mapping
-            ->method('mapCommandToHandler')
-            ->with(CompleteTaskCommand::class)
-            ->willReturn(new MethodToCall(CompleteTaskCommand::class, 'someMethodThatDoesNotExist'));
-
-        $this->expectException(CanNotInvokeHandler::class);
-        $this->middleware->execute($command, $this->mockNext());
-    }
-
-    public function testDynamicMethodNamesAreSupported(): void
-    {
-        $command = new CompleteTaskCommand();
-        $handler = new DynamicMethodsHandler();
-
-        $this->container
-            ->method('get')
-            ->willReturn($handler);
-
-        $this->mapping
-            ->method('mapCommandToHandler')
-            ->with(CompleteTaskCommand::class)
-            ->willReturn(new MethodToCall(CompleteTaskCommand::class, 'someHandlerMethod'));
-
-        $this->middleware->execute($command, $this->mockNext());
-
-        self::assertEquals(['someHandlerMethod'], $handler->getMethodsInvoked());
-    }
-
     protected function mockNext(): callable
     {
         return static function (): void {

tests/Handler/Mapping/MappingByNamingConventionTest.php

@@ -7,7 +7,7 @@
 use League\Tactician\Handler\Mapping\MapByNamingConvention;
 use League\Tactician\Handler\Mapping\MethodName\MethodNameInflector;
 use League\Tactician\Tests\Fixtures\Command\AddTaskCommand;
-use League\Tactician\Tests\Fixtures\Handler\ConcreteMethodsHandler;
+use League\Tactician\Tests\Fixtures\Handler\HandleMethodHandler;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -26,7 +26,7 @@ public function testMethodsAreDelegatedProperly(): void
             ->expects(self::once())
             ->method('getClassName')
             ->with(AddTaskCommand::class)
-            ->willReturn(ConcreteMethodsHandler::class);
+            ->willReturn(HandleMethodHandler::class);
 
         $methodName
             ->expects(self::once())
@@ -36,7 +36,7 @@ public function testMethodsAreDelegatedProperly(): void
 
         $handler = $mapping->mapCommandToHandler(AddTaskCommand::class);
 
-        self::assertEquals(ConcreteMethodsHandler::class, $handler->getClassName());
+        self::assertEquals(HandleMethodHandler::class, $handler->getClassName());
         self::assertEquals('handle', $handler->getMethodName());
     }
 }

tests/Handler/Mapping/MappingByStaticListTest.php

@@ -19,13 +19,13 @@ public function testSuccessfulMapping(): void
     {
         $mapping = new MapByStaticList(
             [
-                AddTaskCommand::class => [ConcreteMethodsHandler::class, 'handle'],
+                AddTaskCommand::class => [ConcreteMethodsHandler::class, 'handleTaskCompletedCommand'],
             ]
         );
 
         $handler = $mapping->mapCommandToHandler(AddTaskCommand::class);
         static::assertEquals(ConcreteMethodsHandler::class, $handler->getClassName());
-        static::assertEquals('handle', $handler->getMethodName());
+        static::assertEquals('handleTaskCompletedCommand', $handler->getMethodName());
     }
 
     public function testFailedMappingCommandToMethod(): void

tests/Handler/Mapping/MethodDoesNotExistTest.php

@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+namespace League\Tactician\Tests\Handler\Mapping;
+
+use League\Tactician\Exception;
+use League\Tactician\Handler\Mapping\MethodDoesNotExist;
+use League\Tactician\Tests\Fixtures\Handler\ConcreteMethodsHandler;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \League\Tactician\Handler\Mapping\MethodDoesNotExist
+ */
+class MethodDoesNotExistTest extends TestCase
+{
+    public function testExceptionContainsDebuggingInfo() : void
+    {
+        $exception = MethodDoesNotExist::on(ConcreteMethodsHandler::class, 'handleTaskCompletedCommand');
+
+        self::assertInstanceOf(Exception::class, $exception);
+        self::assertStringContainsString(ConcreteMethodsHandler::class, $exception->getMessage());
+        self::assertStringContainsString('handleTaskCompletedCommand', $exception->getMessage());
+    }
+}

tests/Handler/Mapping/MethodToCallTest.php

@@ -0,0 +1,36 @@
+<?php
+declare(strict_types=1);
+
+namespace League\Tactician\Tests\Handler\Mapping;
+
+use League\Tactician\Handler\Mapping\MethodDoesNotExist;
+use League\Tactician\Handler\Mapping\MethodToCall;
+use League\Tactician\Tests\Fixtures\Command\CompleteTaskCommand;
+use League\Tactician\Tests\Fixtures\Handler\ConcreteMethodsHandler;
+use League\Tactician\Tests\Fixtures\Handler\DynamicMethodsHandler;
+use PHPUnit\Framework\TestCase;
+
+final class MethodToCallTest extends TestCase
+{
+    public function testBoringGettersAndSetters(): void
+    {
+        $methodToCall = new MethodToCall(ConcreteMethodsHandler::class, 'handleTaskCompletedCommand');
+
+        self::assertEquals(ConcreteMethodsHandler::class, $methodToCall->getClassName());
+        self::assertEquals('handleTaskCompletedCommand', $methodToCall->getMethodName());
+    }
+
+    public function testMissingMethodOnHandlerObjectIsDetected(): void
+    {
+        $this->expectException(MethodDoesNotExist::class);
+        new MethodToCall(CompleteTaskCommand::class, 'someMethodThatDoesNotExist');
+    }
+
+    public function testDynamicMethodNamesAreSupported(): void
+    {
+        $methodToCall = new MethodToCall(DynamicMethodsHandler::class, 'someMethodThatDoesNotExist');
+
+        self::assertEquals(DynamicMethodsHandler::class, $methodToCall->getClassName());
+        self::assertEquals('someMethodThatDoesNotExist', $methodToCall->getMethodName());
+    }
+}