Improved error reporting in Dispatcher

flight/core/Dispatcher.php

@@ -22,45 +22,49 @@ class Dispatcher
     public const FILTER_BEFORE = 'before';
     public const FILTER_AFTER = 'after';
 
-    /**
+    /** @var array<string, Closure(): (void|mixed)> Mapped events. */
-     * Mapped events.
-     *
-     * @var array<string, callable>
-     */
     protected array $events = [];
 
     /**
      * Method filters.
      *
-     * @var array<string, array<'before'|'after', array<int, callable>>>
+     * @var array<string, array<'before'|'after', array<int, Closure(array<int, mixed> &$params, mixed &$output): (void|false)>>>
      */
     protected array $filters = [];
 
     /**
      * Dispatches an event.
      *
-     * @param string $name   Event name
+     * @param string $name Event name
-     * @param array<int, mixed>  $params Callback parameters
+     * @param array<int, mixed> $params Callback parameters.
-     *
-     * @throws Exception
      *
      * @return mixed Output of callback
+     * @throws Exception If event name isn't found or if event throws an `Exception`
      */
     public function run(string $name, array $params = [])
     {
         $output = '';
 
         // Run pre-filters
-        if (!empty($this->filters[$name]['before'])) {
+        $thereAreBeforeFilters = !empty($this->filters[$name]['before']);
+
+        if ($thereAreBeforeFilters) {
             $this->filter($this->filters[$name]['before'], $params, $output);
         }
 
         // Run requested method
         $callback = $this->get($name);
+
+        if ($callback === null) {
+            throw new Exception("Event '$name' isn't found.");
+        }
+
         $output = $callback(...$params);
 
         // Run post-filters
-        if (!empty($this->filters[$name]['after'])) {
+        $thereAreAfterFilters = !empty($this->filters[$name]['after']);
+
+        if ($thereAreAfterFilters) {
             $this->filter($this->filters[$name]['after'], $params, $output);
         }
 
@@ -77,6 +81,10 @@ class Dispatcher
      */
     public function set(string $name, callable $callback): self
     {
+        if ($this->get($name) !== null) {
+            trigger_error("Event '$name' has been overriden!", E_USER_NOTICE);
+        }
+
         $this->events[$name] = $callback;
 
         return $this;
@@ -87,7 +95,7 @@ class Dispatcher
      *
      * @param string $name Event name
      *
-     * @return ?callable $callback Callback function
+     * @return null|(Closure(): (void|mixed)) $callback Callback function
      */
     public function get(string $name): ?callable
     {
@@ -141,18 +149,19 @@ class Dispatcher
     /**
      * Executes a chain of method filters.
      *
-     * @param array<int, callable> $filters Chain of filters
+     * @param array<int, Closure(array<int, mixed> &$params, mixed &$output): (void|false)> $filters
-     * @param array<int, mixed> $params  Method parameters
+     * Chain of filters-
-     * @param mixed $output  Method output
+     * @param array<int, mixed> $params Method parameters
+     * @param mixed $output Method output
      *
-     * @throws Exception
+     * @throws Exception If an event throws an `Exception`.
      */
     public function filter(array $filters, array &$params, &$output): void
     {
-        $args = [&$params, &$output];
         foreach ($filters as $callback) {
-            $continue = $callback(...$args);
+            $continue = $callback($params, $output);
-            if (false === $continue) {
+
+            if ($continue === false) {
                 break;
             }
         }
@@ -161,33 +170,33 @@ class Dispatcher
     /**
      * Executes a callback function.
      *
-     * @param callable|array<class-string|object, string> $callback Callback function
+     * @param callable-string|(Closure(): mixed)|array{class-string|object, string} $callback
-     * @param array<int, mixed>          $params   Function parameters
+     * Callback function
-     *
+     * @param array<int, mixed> $params Function parameters
-     * @throws Exception
      *
      * @return mixed Function results
+     * @throws Exception
      */
     public static function execute($callback, array &$params = [])
     {
-        if (\is_callable($callback)) {
+        if (!\is_callable($callback)) {
-            return \is_array($callback) ?
+            throw new InvalidArgumentException('Invalid callback specified.');
-                self::invokeMethod($callback, $params) :
-                self::callFunction($callback, $params);
         }
 
-        throw new InvalidArgumentException('Invalid callback specified.');
+        return \is_array($callback)
+            ? self::invokeMethod($callback, $params)
+            : self::callFunction($callback, $params);
     }
 
     /**
      * Calls a function.
      *
-     * @param callable|string $func   Name of function to call
+     * @param callable $func Name of function to call
-     * @param array<int, mixed>           $params Function parameters
+     * @param array<int, mixed> &$params Function parameters
      *
      * @return mixed Function results
      */
-    public static function callFunction($func, array &$params = [])
+    public static function callFunction(callable $func, array &$params = [])
     {
         return call_user_func_array($func, $params);
     }

tests/DispatcherTest.php

@@ -7,6 +7,7 @@ namespace tests;
 use Closure;
 use Exception;
 use flight\core\Dispatcher;
+use PharIo\Manifest\InvalidEmailException;
 use tests\classes\Hello;
 use PHPUnit\Framework\TestCase;
 
@@ -117,41 +118,96 @@ class DispatcherTest extends TestCase
         $this->assertSame('Hello, Fred! Have a nice day!', $result);
     }
 
-    // Test an invalid callback
+    public function testInvalidCallback(): void
-    public function testInvalidCallback()
     {
         $this->expectException(Exception::class);
 
-        $this->dispatcher->execute(['NonExistentClass', 'nonExistentMethod']);
+        Dispatcher::execute(['NonExistentClass', 'nonExistentMethod']);
     }
 
-    public function testCallFunction4Params()
+    public function testItThrowsAnExceptionWhenRunAnUnregistedEventName(): void
     {
-        $closure = function ($param1, $params2, $params3, $param4) {
+        $this->expectException(Exception::class);
-            return 'hello' . $param1 . $params2 . $params3 . $param4;
+
+        $this->dispatcher->run('nonExistentEvent');
+    }
+
+    public function testWhenAnEventThrowsAnExceptionItPersistUntilNextCatchBlock(): void
+    {
+        $this->dispatcher->set('myMethod', function (): void {
+            throw new Exception('myMethod Exception');
+        });
+
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('myMethod Exception');
+
+        $this->dispatcher->run('myMethod');
+    }
+
+    public function testWhenAnEventThrowsCustomExceptionItPersistUntilNextCatchBlock(): void
+    {
+        $this->dispatcher->set('checkEmail', function (string $email): void {
+            throw new InvalidEmailException("Invalid email $email");
+        });
+
+        $this->expectException(InvalidEmailException::class);
+        $this->expectExceptionMessage('Invalid email mail@mail,com');
+
+        $this->dispatcher->run('checkEmail', ['mail@mail,com']);
+    }
+
+    public function testItThrowsNoticeForOverrideRegisteredEvents(): void
+    {
+        set_error_handler(function (int $errno, string $errstr): void {
+            $this->assertSame(E_USER_NOTICE, $errno);
+            $this->assertSame("Event 'myMethod' has been overriden!", $errstr);
+        });
+
+        $this->dispatcher->set('myMethod', function (): string {
+            return 'Original';
+        });
+
+        $this->dispatcher->set('myMethod', function (): string {
+            return 'Overriden';
+        });
+
+        $this->assertSame('Overriden', $this->dispatcher->run('myMethod'));
+        restore_error_handler();
+    }
+
+    public function testCallFunction4Params(): void
+    {
+        $myFunction = function ($param1, $param2, $param3, $param4) {
+            return "hello{$param1}{$param2}{$param3}{$param4}";
         };
+
         $params = ['param1', 'param2', 'param3', 'param4'];
-        $result = $this->dispatcher->callFunction($closure, $params);
+        $result = Dispatcher::callFunction($myFunction, $params);
-        $this->assertEquals('helloparam1param2param3param4', $result);
+
+        $this->assertSame('helloparam1param2param3param4', $result);
     }
 
-    public function testCallFunction5Params()
+    public function testCallFunction5Params(): void
     {
-        $closure = function ($param1, $params2, $params3, $param4, $param5) {
+        $myFunction = function ($param1, $param2, $param3, $param4, $param5) {
-            return 'hello' . $param1 . $params2 . $params3 . $param4 . $param5;
+            return "hello{$param1}{$param2}{$param3}{$param4}{$param5}";
         };
+
         $params = ['param1', 'param2', 'param3', 'param4', 'param5'];
-        $result = $this->dispatcher->callFunction($closure, $params);
+        $result = Dispatcher::callFunction($myFunction, $params);
-        $this->assertEquals('helloparam1param2param3param4param5', $result);
+
+        $this->assertSame('helloparam1param2param3param4param5', $result);
     }
 
-    public function testCallFunction6Params()
+    public function testCallFunction6Params(): void
     {
-        $closure = function ($param1, $params2, $params3, $param4, $param5, $param6) {
+        $func = function ($param1, $param2, $param3, $param4, $param5, $param6) {
-            return 'hello' . $param1 . $params2 . $params3 . $param4 . $param5 . $param6;
+            return "hello{$param1}{$param2}{$param3}{$param4}{$param5}{$param6}";
         };
+
         $params = ['param1', 'param2', 'param3', 'param4', 'param5', 'param6'];
-        $result = $this->dispatcher->callFunction($closure, $params);
+        $result = Dispatcher::callFunction($func, $params);
-        $this->assertEquals('helloparam1param2param3param4param5param6', $result);
+
+        $this->assertSame('helloparam1param2param3param4param5param6', $result);
     }
 }