Merge pull request #540 from flightphp/dispatcher-refactor

Dispatcher refactor

composer.json

@@ -55,6 +55,7 @@
     "scripts": {
         "test": "phpunit",
         "test-coverage": "rm clover.xml && XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-html=coverage --coverage-clover=clover.xml && vendor/bin/coverage-check clover.xml 100",
+        "test-coverage:win": "del clover.xml && phpunit --coverage-html=coverage --coverage-clover=clover.xml && coverage-check clover.xml 100",
         "lint": "phpstan --no-progress -cphpstan.neon",
         "beautify": "phpcbf --standard=phpcs.xml",
         "phpcs": "phpcs --standard=phpcs.xml -n"

flight.sublime-project

@@ -53,6 +53,11 @@
             "name": "Format",
             "quiet": true,
             "shell_cmd": "composer beautify -- --no-colors"
+        },
+        {
+            "name": "Test coverage",
+            "quiet": true,
+            "shell_cmd": "composer test-coverage:win -- --no-colors"
         }
     ],
 }

flight/core/Dispatcher.php

@@ -46,29 +46,61 @@ class Dispatcher
      */
     public function run(string $name, array $params = [])
     {
-        $output = '';
+        $this->runPreFilters($name, $params);
+        $output = $this->runEvent($name, $params);
 
-        // Run pre-filters
+        return $this->runPostFilters($name, $output);
-        $thereAreBeforeFilters = !empty($this->filters[$name]['before']);
+    }
+
+    /**
+     * @param array<int, mixed> &$params
+     *
+     * @return $this
+     * @throws Exception
+     */
+    protected function runPreFilters(string $eventName, array &$params): self
+    {
+        $thereAreBeforeFilters = !empty($this->filters[$eventName][self::FILTER_BEFORE]);
 
         if ($thereAreBeforeFilters) {
-            $this->filter($this->filters[$name]['before'], $params, $output);
+            $this->filter($this->filters[$eventName][self::FILTER_BEFORE], $params, $output);
         }
 
-        // Run requested method
+        return $this;
-        $callback = $this->get($name);
+    }
+
+    /**
+     * @param array<int, mixed> &$params
+     * @param mixed &$output
+     *
+     * @return void|mixed
+     * @throws Exception
+     */
+    protected function runEvent(string $eventName, array &$params)
+    {
+        $requestedMethod = $this->get($eventName);
 
-        if ($callback === null) {
+        if ($requestedMethod === null) {
-            throw new Exception("Event '$name' isn't found.");
+            throw new Exception("Event '$eventName' isn't found.");
         }
 
-        $output = $callback(...$params);
+        return $requestedMethod(...$params);
+    }
+
+    /**
+     * @param mixed &$output
+     *
+     * @return mixed
+     * @throws Exception
+     */
+    protected function runPostFilters(string $eventName, &$output)
+    {
+        static $params = [];
 
-        // Run post-filters
+        $thereAreAfterFilters = !empty($this->filters[$eventName][self::FILTER_AFTER]);
-        $thereAreAfterFilters = !empty($this->filters[$name]['after']);
 
         if ($thereAreAfterFilters) {
-            $this->filter($this->filters[$name]['after'], $params, $output);
+            $this->filter($this->filters[$eventName][self::FILTER_AFTER], $params, $output);
         }
 
         return $output;
@@ -118,7 +150,7 @@ class Dispatcher
     }
 
     /**
-     * Clears an event. If no name is given, all events are removed.
+     * Clears an event. If no name is given, all events will be removed.
      *
      * @param ?string $name Event name
      */
@@ -165,7 +197,7 @@ class Dispatcher
      * @param array<int, mixed> $params Method parameters
      * @param mixed $output Method output
      *
-     * @throws Exception If an event throws an `Exception`.
+     * @throws Exception If an event throws an `Exception` or if `$filters` contains an invalid filter.
      */
     public static function filter(array $filters, array &$params, &$output): void
     {
@@ -190,7 +222,7 @@ class Dispatcher
      * @param array<int, mixed> $params Function parameters
      *
      * @return mixed Function results
-     * @throws Exception
+     * @throws Exception If `$callback` also throws an `Exception`.
      */
     public static function execute($callback, array &$params = [])
     {

tests/DispatcherTest.php

@@ -7,9 +7,11 @@ namespace tests;
 use Closure;
 use Exception;
 use flight\core\Dispatcher;
+use InvalidArgumentException;
 use PharIo\Manifest\InvalidEmailException;
 use tests\classes\Hello;
 use PHPUnit\Framework\TestCase;
+use tests\classes\TesterClass;
 use TypeError;
 
 class DispatcherTest extends TestCase
@@ -126,6 +128,28 @@ class DispatcherTest extends TestCase
         Dispatcher::execute(['NonExistentClass', 'nonExistentMethod']);
     }
 
+    public function testInvalidCallableString(): void
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('Invalid callback specified.');
+
+        Dispatcher::execute('inexistentGlobalFunction');
+    }
+
+    public function testInvalidCallbackBecauseConstructorParameters(): void
+    {
+        $class = TesterClass::class;
+        $method = 'instanceMethod';
+        $exceptionMessage = "Method '$class::$method' cannot be called statically. ";
+        $exceptionMessage .= "$class::__construct require 6 parameters";
+
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage($exceptionMessage);
+
+        static $params = [];
+        Dispatcher::invokeMethod([$class, $method], $params);
+    }
+
     // It will be useful for executing instance Controller methods statically
     public function testCanExecuteAnNonStaticMethodStatically(): void
     {
@@ -193,7 +217,7 @@ class DispatcherTest extends TestCase
             ->set('myMethod', function (): string {
                 return 'Original';
             })
-            ->hook('myMethod', 'invalid', function (array &$params, $output): void {
+            ->hook('myMethod', 'invalid', function (array &$params, &$output): void {
                 $output = 'Overriden';
             });
 
@@ -208,35 +232,12 @@ class DispatcherTest extends TestCase
 
         $params = [];
         $output = '';
-        $validCallable = function (): void {
-        };
         $invalidCallable = 'invalidGlobalFunction';
 
-        Dispatcher::filter([$validCallable, $invalidCallable], $params, $output);
+        $validCallable = function (): void {
-    }
-
-    public function testCallFunction4Params(): void
-    {
-        $myFunction = function ($param1, $param2, $param3, $param4) {
-            return "hello{$param1}{$param2}{$param3}{$param4}";
-        };
-
-        $params = ['param1', 'param2', 'param3', 'param4'];
-        $result = Dispatcher::callFunction($myFunction, $params);
-
-        $this->assertSame('helloparam1param2param3param4', $result);
-    }
-
-    public function testCallFunction5Params(): void
-    {
-        $myFunction = function ($param1, $param2, $param3, $param4, $param5) {
-            return "hello{$param1}{$param2}{$param3}{$param4}{$param5}";
         };
 
-        $params = ['param1', 'param2', 'param3', 'param4', 'param5'];
+        Dispatcher::filter([$validCallable, $invalidCallable], $params, $output);
-        $result = Dispatcher::callFunction($myFunction, $params);
-
-        $this->assertSame('helloparam1param2param3param4param5', $result);
     }
 
     public function testCallFunction6Params(): void

tests/classes/TesterClass.php

@@ -22,4 +22,9 @@ class TesterClass
         $this->param5 = $param5;
         $this->param6 = $param6;
     }
+
+    public function instanceMethod(): void
+    {
+        $this->param2 = $this->param1;
+    }
 }