fix(router): use route registry to generate uris (#1724)

Co-authored-by: Brent Roose <[email protected]>

Author: innocenzi

packages/router/src/Exceptions/ControllerActionDoesNotExist.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Router\Exceptions;
+
+use Exception;
+
+final class ControllerActionDoesNotExist extends Exception implements RouterException
+{
+    public static function controllerNotFound(string $controller): self
+    {
+        return new self("The controller class `{$controller}` does not exist.");
+    }
+
+    public static function actionNotFound(string $controller, string $method): self
+    {
+        return new self("The method `{$method}()` does not exist in controller class `{$controller}`.");
+    }
+}

packages/router/src/Exceptions/ControllerMethodHadNoRoute.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Router\Exceptions;
+
+use Exception;
+
+final class ControllerMethodHadNoRoute extends Exception implements RouterException
+{
+    public function __construct(string $controllerClass, string $controllerMethod)
+    {
+        parent::__construct("No route found for `{$controllerClass}::{$controllerMethod}()`. Did you forget to add a `Route` attribute?");
+    }
+}

packages/router/src/Exceptions/ControllerMethodHadNoRouteAttribute.php

@@ -18,6 +18,9 @@ public function __construct(
         /** @var array<string,\Tempest\Router\Routing\Matching\MatchingRegex> */
         public array $matchingRegexes = [],
 
+        /** @var array<string,string[]> */
+        public array $handlerIndex = [],
+
         /** @var class-string<\Tempest\Router\ResponseProcessor>[] */
         public array $responseProcessors = [],
 
@@ -37,6 +40,7 @@ public function apply(RouteConfig $newConfig): void
         $this->staticRoutes = $newConfig->staticRoutes;
         $this->dynamicRoutes = $newConfig->dynamicRoutes;
         $this->matchingRegexes = $newConfig->matchingRegexes;
+        $this->handlerIndex = $newConfig->handlerIndex;
     }
 
     public function addResponseProcessor(string $responseProcessor): void

packages/router/src/RouteConfig.php

@@ -27,8 +27,8 @@ public function discover(DiscoveryLocation $location, ClassReflector $class): vo
 
             foreach ($routeAttributes as $routeAttribute) {
                 $decorators = [
-                    ...$method->getDeclaringClass()->getAttributes(RouteDecorator::class),
                     ...$method->getAttributes(RouteDecorator::class),
+                    ...$method->getDeclaringClass()->getAttributes(RouteDecorator::class),
                 ];
 
                 $route = DiscoveredRoute::fromRoute($routeAttribute, $decorators, $method);

packages/router/src/RouteDiscovery.php

@@ -22,6 +22,9 @@ final class RouteConfigurator
     /** @var array<string,array<string,\Tempest\Router\Routing\Construction\DiscoveredRoute>> */
     private array $dynamicRoutes = [];
 
+    /** @var array<string,string[]> */
+    private array $handlerIndex = [];
+
     private bool $isDirty = false;
 
     private RoutingTree $routingTree;
@@ -35,6 +38,10 @@ public function addRoute(DiscoveredRoute $route): void
     {
         $this->isDirty = true;
 
+        $handler = $route->handler->getDeclaringClass()->getName() . '::' . $route->handler->getName();
+        $this->handlerIndex[$handler] ??= [];
+        $this->handlerIndex[$handler][] = $route->uri;
+
         if ($route->isDynamic) {
             $this->addDynamicRoute($route);
         } else {
@@ -76,6 +83,7 @@ public function toRouteConfig(): RouteConfig
             $this->staticRoutes,
             $this->dynamicRoutes,
             $this->routingTree->toMatchingRegexes(),
+            $this->handlerIndex,
         );
     }
 

packages/router/src/Routing/Construction/RouteConfigurator.php

@@ -13,7 +13,8 @@
 use Tempest\Http\Request;
 use Tempest\Reflection\ClassReflector;
 use Tempest\Reflection\MethodReflector;
-use Tempest\Router\Exceptions\ControllerMethodHadNoRouteAttribute;
+use Tempest\Router\Exceptions\ControllerActionDoesNotExist;
+use Tempest\Router\Exceptions\ControllerMethodHadNoRoute;
 use Tempest\Router\Routing\Construction\DiscoveredRoute;
 use Tempest\Support\Arr;
 use Tempest\Support\Regex;
@@ -25,6 +26,7 @@ final class UriGenerator
 {
     public function __construct(
         private AppConfig $appConfig,
+        private RouteConfig $routeConfig,
         private Signer $signer,
         private Container $container,
     ) {}
@@ -178,7 +180,7 @@ public function isCurrentUri(array|string|MethodReflector $action, mixed ...$par
 
         $matchedRoute = $this->container->get(MatchedRoute::class);
         $candidateUri = $this->createUri($action, ...[...$matchedRoute->params, ...$params]);
-        $currentUri = $this->createUri([$matchedRoute->route->handler->getDeclaringClass(), $matchedRoute->route->handler->getName()]);
+        $currentUri = $this->createUri([$matchedRoute->route->handler->getDeclaringClass()->getName(), $matchedRoute->route->handler->getName()]);
 
         foreach ($matchedRoute->params as $key => $value) {
             if ($value instanceof BackedEnum) {
@@ -206,14 +208,20 @@ private function normalizeActionToUri(array|string|MethodReflector $action): str
 
         [$controllerClass, $controllerMethod] = is_array($action) ? $action : [$action, '__invoke'];
 
-        $routeAttribute = new ClassReflector($controllerClass)
-            ->getMethod($controllerMethod)
-            ->getAttribute(Route::class);
+        $routes = array_unique($this->routeConfig->handlerIndex[$controllerClass . '::' . $controllerMethod] ?? []);
 
-        if ($routeAttribute === null) {
-            throw new ControllerMethodHadNoRouteAttribute($controllerClass, $controllerMethod);
+        if ($routes === []) {
+            if (! class_exists($controllerClass)) {
+                throw ControllerActionDoesNotExist::controllerNotFound($controllerClass, $controllerMethod);
+            }
+
+            if (! method_exists($controllerClass, $controllerMethod)) {
+                throw ControllerActionDoesNotExist::actionNotFound($controllerClass, $controllerMethod);
+            }
+
+            throw new ControllerMethodHadNoRoute($controllerClass, $controllerMethod);
         }
 
-        return Str\ensure_starts_with($routeAttribute->uri, '/');
+        return Str\ensure_starts_with($routes[0], '/');
     }
 }

packages/router/src/UriGenerator.php

@@ -5,13 +5,11 @@
 use Rector\Arguments\Rector\ClassMethod\ArgumentAdderRector;
 use Rector\Caching\ValueObject\Storage\FileCacheStorage;
 use Rector\CodingStyle\Rector\Encapsed\EncapsedStringsToSprintfRector;
-use Rector\CodingStyle\Rector\Stmt\NewlineAfterStatementRector;
 use Rector\Config\RectorConfig;
 use Rector\DeadCode\Rector\PropertyProperty\RemoveNullPropertyInitializationRector;
 use Rector\Php70\Rector\StaticCall\StaticCallOnNonStaticToInstanceCallRector;
 use Rector\Php74\Rector\Closure\ClosureToArrowFunctionRector;
 use Rector\Php74\Rector\Property\RestoreDefaultNullToNullableTypePropertyRector;
-use Rector\Php74\Rector\Ternary\ParenthesizeNestedTernaryRector;
 use Rector\Php81\Rector\Array_\FirstClassCallableRector;
 use Rector\Php81\Rector\FuncCall\NullToStrictStringFuncCallArgRector;
 use Rector\Php81\Rector\Property\ReadOnlyPropertyRector;
@@ -21,8 +19,8 @@
 use Rector\Php84\Rector\Param\ExplicitNullableParamTypeRector;
 use Rector\Privatization\Rector\ClassMethod\PrivatizeFinalClassMethodRector;
 use Rector\TypeDeclaration\Rector\ArrowFunction\AddArrowFunctionReturnTypeRector;
+use Rector\TypeDeclaration\Rector\ClassMethod\NarrowObjectReturnTypeRector;
 use Rector\TypeDeclaration\Rector\ClassMethod\ReturnNeverTypeRector;
-use Rector\TypeDeclaration\Rector\Closure\ClosureReturnTypeRector;
 use Rector\TypeDeclaration\Rector\Empty_\EmptyOnNullableObjectToInstanceOfRector;
 
 return RectorConfig::configure()
@@ -54,10 +52,10 @@
         RestoreDefaultNullToNullableTypePropertyRector::class,
         ReturnNeverTypeRector::class,
         StaticCallOnNonStaticToInstanceCallRector::class,
-        ClosureReturnTypeRector::class,
         EncapsedStringsToSprintfRector::class,
         AddArrowFunctionReturnTypeRector::class,
         PrivatizeFinalClassMethodRector::class,
+        NarrowObjectReturnTypeRector::class,
     ])
     ->withParallel(300, 10, 10)
     ->withPreparedSets(

rector.php

@@ -17,6 +17,7 @@
     #[Get('/repeated/d')]
     #[Post('/repeated/e')]
     #[Post('/repeated/f')]
+    #[Get('/repeated/f')]
     public function __invoke(): Response
     {
         return new Ok();

tests/Fixtures/Controllers/ControllerWithRepeatedRoutes.php

@@ -9,7 +9,7 @@
 #[Prefix('/prefix')]
 final class PrefixController
 {
-    #[Get('/endpoint')]
+    #[Prefix('/method'), Get('/endpoint')]
     public function __invoke(): Ok
     {
         return new Ok();