refactor(routing): split route construction (#666)

src/Tempest/Http/src/RouteConfig.php

@@ -4,81 +4,22 @@
 
 namespace Tempest\Http;
 
-use Tempest\Http\Routing\Construction\MarkedRoute;
-use Tempest\Http\Routing\Construction\RoutingTree;
-use Tempest\Reflection\MethodReflector;
-
 final class RouteConfig
 {
-    /** @var string The mark to give the next route in the matching Regex */
-    private string $regexMark = 'a';
-
-    /** @var array<string, string> */
-    public array $matchingRegexes = [];
-
-    public RoutingTree $routingTree;
-
     public function __construct(
-        /** @var array<string, array<string, \Tempest\Http\Route>> */
+        /** @var array<string, array<string, Route>> */
         public array $staticRoutes = [],
-        /** @var array<string, array<string, \Tempest\Http\Route>> */
+        /** @var array<string, array<string, Route>> */
         public array $dynamicRoutes = [],
+        /** @var array<string, string> */
+        public array $matchingRegexes = [],
     ) {
-        $this->routingTree = new RoutingTree();
-    }
-
-    public function addRoute(MethodReflector $handler, Route $route): self
-    {
-        $route->setHandler($handler);
-
-        if ($route->isDynamic) {
-            $this->regexMark = str_increment($this->regexMark);
-            $this->dynamicRoutes[$route->method->value][$this->regexMark] = $route;
-
-            $this->routingTree->add(
-                new MarkedRoute(
-                    mark: $this->regexMark,
-                    route: $route,
-                )
-            );
-
-        } else {
-            $uriWithTrailingSlash = rtrim($route->uri, '/');
-
-            $this->staticRoutes[$route->method->value][$uriWithTrailingSlash] = $route;
-            $this->staticRoutes[$route->method->value][$uriWithTrailingSlash . '/'] = $route;
-        }
-
-        return $this;
-    }
-
-    public function prepareMatchingRegexes(): void
-    {
-        if (! empty($this->matchingRegexes)) {
-            return;
-        }
-
-        $this->matchingRegexes = $this->routingTree->toMatchingRegexes();
-    }
-
-    /**
-     * __sleep is called before serialize and returns the public properties to serialize. We do not want the routingTree
-     * to be serialized, but we do want the result to be serialized. Thus prepareMatchingRegexes is called and the
-     * resulting matchingRegexes are stored.
-     */
-    public function __sleep(): array
-    {
-        $this->prepareMatchingRegexes();
-
-        return ['staticRoutes', 'dynamicRoutes', 'matchingRegexes'];
     }
 
-    /**
-     * __wakeup is called after unserialize. We do not serialize the routingTree thus we need to provide some default
-     *  for it. Otherwise, it will be uninitialized and cause issues when tempest expects it to be defined.
-     */
-    public function __wakeup(): void
+    public function apply(RouteConfig $newConfig): void
     {
-        $this->routingTree = new RoutingTree();
+        $this->staticRoutes = $newConfig->staticRoutes;
+        $this->dynamicRoutes = $newConfig->dynamicRoutes;
+        $this->matchingRegexes = $newConfig->matchingRegexes;
     }
 }

src/Tempest/Http/src/RouteDiscovery.php

@@ -6,11 +6,15 @@
 
 use Tempest\Container\Container;
 use Tempest\Core\Discovery;
+use Tempest\Core\KernelEvent;
+use Tempest\EventBus\EventHandler;
+use Tempest\Http\Routing\Construction\RouteConfigurator;
 use Tempest\Reflection\ClassReflector;
 
 final readonly class RouteDiscovery implements Discovery
 {
     public function __construct(
+        private RouteConfigurator $configurator,
         private RouteConfig $routeConfig,
     ) {
     }
@@ -21,22 +25,32 @@ public function discover(ClassReflector $class): void
             $routeAttributes = $method->getAttributes(Route::class);
 
             foreach ($routeAttributes as $routeAttribute) {
-                $this->routeConfig->addRoute($method, $routeAttribute);
+                $routeAttribute->setHandler($method);
+
+                $this->configurator->addRoute($routeAttribute);
             }
         }
     }
 
+    #[EventHandler(KernelEvent::BOOTED)]
+    public function finishDiscovery(): void
+    {
+        if ($this->configurator->isDirty()) {
+            $this->routeConfig->apply($this->configurator->toRouteConfig());
+        }
+    }
+
     public function createCachePayload(): string
     {
+        $this->finishDiscovery();
+
         return serialize($this->routeConfig);
     }
 
     public function restoreCachePayload(Container $container, string $payload): void
     {
-        $routeConfig = unserialize($payload);
+        $routeConfig = unserialize($payload, [ 'allowed_classes' => true ]);
 
-        $this->routeConfig->staticRoutes = $routeConfig->staticRoutes;
-        $this->routeConfig->dynamicRoutes = $routeConfig->dynamicRoutes;
-        $this->routeConfig->matchingRegexes = $routeConfig->matchingRegexes;
+        $this->routeConfig->apply($routeConfig);
     }
 }

src/Tempest/Http/src/Routing/Construction/RouteConfigurator.php

@@ -0,0 +1,88 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Http\Routing\Construction;
+
+use Tempest\Container\Singleton;
+use Tempest\Http\Route;
+use Tempest\Http\RouteConfig;
+
+/**
+ * @internal
+ */
+#[Singleton]
+final class RouteConfigurator
+{
+    /** @var string The mark to give the next route in the matching Regex */
+    private string $regexMark = 'a';
+
+    /** @var array<string, array<string, Route>> */
+    private array $staticRoutes = [];
+
+    /** @var array<string, array<string, Route>> */
+    private array $dynamicRoutes = [];
+
+    private bool $isDirty = false;
+
+    private RoutingTree $routingTree;
+
+    public function __construct()
+    {
+
+        $this->routingTree = new RoutingTree();
+    }
+
+    public function addRoute(Route $route): void
+    {
+        $this->isDirty = true;
+
+        if ($route->isDynamic) {
+            $this->addDynamicRoute($route);
+        } else {
+            $this->addStaticRoute($route);
+        }
+    }
+
+    private function addDynamicRoute(Route $route): void
+    {
+        $markedRoute = $this->markRoute($route);
+        $this->dynamicRoutes[$route->method->value][$markedRoute->mark] = $route;
+
+        $this->routingTree->add($markedRoute);
+    }
+
+    private function addStaticRoute(Route $route): void
+    {
+        $uriWithTrailingSlash = rtrim($route->uri, '/');
+
+        $this->staticRoutes[$route->method->value][$uriWithTrailingSlash] = $route;
+        $this->staticRoutes[$route->method->value][$uriWithTrailingSlash . '/'] = $route;
+    }
+
+    private function markRoute(Route $route): MarkedRoute
+    {
+        $this->regexMark = str_increment($this->regexMark);
+
+        return new MarkedRoute(
+            mark: $this->regexMark,
+            route: $route,
+        );
+    }
+
+    public function toRouteConfig(): RouteConfig
+    {
+        $this->isDirty = false;
+
+        return new RouteConfig(
+            $this->staticRoutes,
+            $this->dynamicRoutes,
+            $this->routingTree->toMatchingRegexes(),
+        );
+    }
+
+    public function isDirty(): bool
+    {
+        return $this->isDirty;
+    }
+}

src/Tempest/Http/src/Routing/Matching/GenericRouteMatcher.php

@@ -46,9 +46,6 @@ private function matchDynamicRoute(PsrRequest $request): ?MatchedRoute
             return null;
         }
 
-        // Ensures matching regexes are available
-        $this->routeConfig->prepareMatchingRegexes();
-
         // Get matching regex for route
         $matchingRegexForMethod = $this->routeConfig->matchingRegexes[$request->getMethod()];
 

src/Tempest/Http/tests/RouteConfigTest.php

@@ -5,126 +5,33 @@
 namespace Tempest\Http\Tests;
 
 use PHPUnit\Framework\TestCase;
-use ReflectionMethod;
 use Tempest\Http\Method;
-use Tempest\Http\Response;
-use Tempest\Http\Responses\Ok;
 use Tempest\Http\Route;
 use Tempest\Http\RouteConfig;
-use Tempest\Reflection\MethodReflector;
 
 /**
  * @internal
  */
 final class RouteConfigTest extends TestCase
 {
-    private RouteConfig $subject;
-
-    private MethodReflector $dummyMethod;
-
-    protected function setUp(): void
-    {
-        parent::setUp();
-
-        $this->subject = new RouteConfig();
-        $this->dummyMethod = new MethodReflector(new ReflectionMethod($this, 'dummyMethod'));
-    }
-
-    public function test_empty(): void
-    {
-        $this->assertEquals([], $this->subject->dynamicRoutes);
-        $this->assertEquals([], $this->subject->matchingRegexes);
-        $this->assertEquals([], $this->subject->staticRoutes);
-    }
-
-    public function test_matching_regexes_is_updated_using_prepare_method(): void
-    {
-        $this->subject->addRoute($this->dummyMethod, new Route('/{id}', Method::GET));
-
-        $this->assertEquals([], $this->subject->matchingRegexes);
-        $this->subject->prepareMatchingRegexes();
-        $this->assertNotEquals([], $this->subject->matchingRegexes);
-    }
-
-    public function test_adding_static_routes(): void
-    {
-        $routes = [
-            new Route('/1', Method::GET),
-            new Route('/2', Method::POST),
-            new Route('/3', Method::GET),
-        ];
-
-        $this->subject->addRoute($this->dummyMethod, $routes[0]);
-        $this->subject->addRoute($this->dummyMethod, $routes[1]);
-        $this->subject->addRoute($this->dummyMethod, $routes[2]);
-
-        $this->assertEquals([
-            'GET' => [
-                '/1' => $routes[0],
-                '/1/' => $routes[0],
-                '/3' => $routes[2],
-                '/3/' => $routes[2],
-            ],
-            'POST' => [
-                '/2' => $routes[1],
-                '/2/' => $routes[1],
-            ],
-        ], $this->subject->staticRoutes);
-    }
-
-    public function test_adding_dynamic_routes(): void
+    public function test_serialization(): void
     {
-        $routes = [
-            new Route('/{id}/1', Method::GET),
-            new Route('/{id}/2', Method::POST),
-            new Route('/{id}/3', Method::GET),
-        ];
-
-        $this->subject->addRoute($this->dummyMethod, $routes[0]);
-        $this->subject->addRoute($this->dummyMethod, $routes[1]);
-        $this->subject->addRoute($this->dummyMethod, $routes[2]);
-
-        $this->subject->prepareMatchingRegexes();
-
-        $this->assertEquals([
-            'GET' => [
-                'b' => $routes[0],
-                'd' => $routes[2],
+        $original = new RouteConfig(
+            [
+                'POST' => ['/a' => new Route('/', Method::POST)],
             ],
-            'POST' => [
-                'c' => $routes[1],
+            [
+                'POST' => ['b' => new Route('/', Method::POST)],
             ],
-        ], $this->subject->dynamicRoutes);
-
-        $this->assertEquals([
-            'GET' => '#^(?|/([^/]++)(?|/1\/?$(*MARK:b)|/3\/?$(*MARK:d)))#',
-            'POST' => '#^(?|/([^/]++)(?|/2\/?$(*MARK:c)))#',
-        ], $this->subject->matchingRegexes);
-    }
-
-    public function test_serialization(): void
-    {
-        $routes = [
-            new Route('/{id}/1', Method::GET),
-            new Route('/{id}/2', Method::POST),
-            new Route('/3', Method::GET),
-        ];
+            [
+                'POST' => '#^(?|/([^/]++)(?|/1\/?$(*MARK:b)|/3\/?$(*MARK:d)))#',
+            ]
+        );
 
-        $this->subject->addRoute($this->dummyMethod, $routes[0]);
-        $this->subject->addRoute($this->dummyMethod, $routes[1]);
-        $this->subject->addRoute($this->dummyMethod, $routes[2]);
-
-        $serialized = serialize($this->subject);
+        $serialized = serialize($original);
         /** @var RouteConfig $deserialized */
         $deserialized = unserialize($serialized);
 
-        $this->assertEquals($this->subject->matchingRegexes, $deserialized->matchingRegexes);
-        $this->assertEquals($this->subject->dynamicRoutes, $deserialized->dynamicRoutes);
-        $this->assertEquals($this->subject->staticRoutes, $deserialized->staticRoutes);
-    }
-
-    public function dummyMethod(): Response
-    {
-        return new Ok();
+        $this->assertEquals($original, $deserialized);
     }
 }