Merge commit from fork

Preserve before/after filters when remapping an event

flight/core/Dispatcher.php

@@ -170,7 +170,17 @@ class Dispatcher
     public function set(string $name, callable $callback): self
     {
         $this->events[$name] = $callback;
-        $this->namedCallables[$name] = new FilteredCallable($callback);
+        $filteredCallable = new FilteredCallable($callback);
+
+        foreach ($this->filters[$name][self::FILTER_BEFORE] ?? [] as $filter) {
+            $filteredCallable->pushBeforeFilter($filter);
+        }
+
+        foreach ($this->filters[$name][self::FILTER_AFTER] ?? [] as $filter) {
+            $filteredCallable->pushAfterFilter($filter);
+        }
+
+        $this->namedCallables[$name] = $filteredCallable;
 
         return $this;
     }

tests/DispatcherTest.php

@@ -130,6 +130,75 @@ class DispatcherTest extends TestCase
         $this->assertSame('Hello, Fred! Have a nice day!', $result);
     }
 
+    public function testSetKeepsFiltersHookedBeforeIt(): void
+    {
+        $calls = [];
+
+        $this->dispatcher->set('hello', function () use (&$calls): string {
+            $calls[] = 'original';
+
+            return 'original';
+        });
+
+        $this->dispatcher
+            ->hook('hello', Dispatcher::FILTER_BEFORE, function () use (&$calls): void {
+                $calls[] = 'before';
+            })
+            ->hook('hello', Dispatcher::FILTER_AFTER, function (array &$params, &$output) use (&$calls): void {
+                $calls[] = 'after';
+            });
+
+        // Re-mapping the same name must keep the filters that were already registered.
+        $this->dispatcher->set('hello', function () use (&$calls): string {
+            $calls[] = 'replacement';
+
+            return 'replacement';
+        });
+
+        $result = $this->dispatcher->run('hello');
+
+        $this->assertSame('replacement', $result);
+        $this->assertSame(['before', 'replacement', 'after'], $calls);
+    }
+
+    public function testSetKeepsFiltersInRegistrationOrder(): void
+    {
+        $calls = [];
+
+        $this->dispatcher->set('hello', fn(): string => 'original');
+
+        $this->dispatcher
+            ->hook('hello', Dispatcher::FILTER_BEFORE, function () use (&$calls): void {
+                $calls[] = 'before1';
+            })
+            ->hook('hello', Dispatcher::FILTER_BEFORE, function () use (&$calls): void {
+                $calls[] = 'before2';
+            })
+            ->hook('hello', Dispatcher::FILTER_AFTER, function (array &$params, &$output) use (&$calls): void {
+                $calls[] = 'after1';
+            })
+            ->hook('hello', Dispatcher::FILTER_AFTER, function (&$output) use (&$calls): void {
+                $calls[] = 'after2';
+            });
+
+        $this->dispatcher->set('hello', fn(): string => 'replacement');
+
+        $this->assertSame('replacement', $this->dispatcher->run('hello'));
+        $this->assertSame(['before1', 'before2', 'after1', 'after2'], $calls);
+    }
+
+    public function testSetKeepsAfterFilterHookedBeforeIt(): void
+    {
+        $this->dispatcher->set('hello', fn(): string => 'Hello');
+        $this->dispatcher->hook('hello', Dispatcher::FILTER_AFTER, function (array &$params, &$output): void {
+            $output .= ' there';
+        });
+
+        $this->dispatcher->set('hello', fn(): string => 'Hi');
+
+        $this->assertSame('Hi there', $this->dispatcher->run('hello'));
+    }
+
     public function testBeforeAndAfterWithShortAfterFilterSyntax(): void
     {
         $this->dispatcher->set('hello', fn(string $name): string => "Hello, $name!");

tests/EngineTest.php

@@ -1381,6 +1381,74 @@ class EngineTest extends TestCase
         $this->assertSame('HEAD', $engine->request()->method);
     }
 
+    /**
+     * The startup configuration is registered as a "before" filter on start. Remapping
+     * start after construction must not drop it.
+     */
+    public function testMapStartKeepsStartupConfiguration(): void
+    {
+        $_SERVER['REQUEST_METHOD'] = 'GET';
+        $_SERVER['REQUEST_URI'] = '/test';
+        $_SERVER['SCRIPT_NAME'] = '/index.php';
+        $_SERVER['HTTP_X_HTTP_METHOD_OVERRIDE'] = 'DELETE';
+        $_SERVER['REMOTE_ADDR'] = '127.0.0.1';
+        $_SERVER['SERVER_NAME'] = 'localhost';
+        $_SERVER['HTTP_HOST'] = 'localhost';
+
+        $engine = new Engine();
+        $engine->set('flight.allow_method_override', false);
+        $engine->set('flight.case_sensitive', true);
+
+        $ran = false;
+        $engine->map('start', function () use (&$ran) {
+            $ran = true;
+        });
+
+        $engine->start();
+
+        // set_*_handler() returns the handler that was installed before the call.
+        $errorHandler = set_error_handler(null);
+        restore_error_handler();
+        $exceptionHandler = set_exception_handler(null);
+        restore_exception_handler();
+
+        // Remove the handlers installed by the startup filter so they do not leak into other tests.
+        if ($errorHandler === [$engine, 'handleError']) {
+            restore_error_handler();
+        }
+        if ($exceptionHandler === [$engine, 'handleException']) {
+            restore_exception_handler();
+        }
+
+        $this->assertTrue($ran);
+        $this->assertFalse(Request::$allowMethodOverride);
+        $this->assertSame('GET', $engine->request()->method);
+        $this->assertTrue($engine->router()->caseSensitive);
+        $this->assertSame([$engine, 'handleError'], $errorHandler);
+        $this->assertSame([$engine, 'handleException'], $exceptionHandler);
+    }
+
+    public function testMapStartKeepsUserFiltersRegisteredBeforeIt(): void
+    {
+        $engine = new Engine();
+        $engine->set('flight.handle_errors', false);
+
+        $calls = [];
+        $engine->before('start', function () use (&$calls) {
+            $calls[] = 'before';
+        });
+        $engine->after('start', function () use (&$calls) {
+            $calls[] = 'after';
+        });
+
+        $engine->map('start', function () use (&$calls) {
+            $calls[] = 'start';
+        });
+        $engine->start();
+
+        $this->assertSame(['before', 'start', 'after'], $calls);
+    }
+
     public function testViewsRestrictToPathConfigIsAppliedToView(): void
     {
         $engine = new Engine();