Skip to content

Commit 330b60e

Browse files
committed
feat(router): throw exception when generating an uri to a controller action that has multiple different routes
1 parent 6d662fb commit 330b60e

File tree

6 files changed

+58
-5
lines changed

6 files changed

+58
-5
lines changed
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tempest\Router\Exceptions;
6+
7+
use Exception;
8+
use Tempest\Support\Arr;
9+
10+
final class ControllerMethodHasMultipleRoutes extends Exception implements RouterException
11+
{
12+
/** @param string[] $routes */
13+
public function __construct(string $controllerClass, string $controllerMethod, array $routes)
14+
{
15+
parent::__construct(vsprintf(
16+
format: "Controller method `%s::%s()` has multiple different routes: \"%s\". Please use the route path directly.",
17+
values: [
18+
$controllerClass,
19+
$controllerMethod,
20+
Arr\join($routes),
21+
],
22+
));
23+
}
24+
}

packages/router/src/RouteConfig.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public function __construct(
1818
/** @var array<string,\Tempest\Router\Routing\Matching\MatchingRegex> */
1919
public array $matchingRegexes = [],
2020

21-
/** @var array<string,string> */
21+
/** @var array<string,string[]> */
2222
public array $handlerIndex = [],
2323

2424
/** @var class-string<\Tempest\Router\ResponseProcessor>[] */

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,10 @@ public function __construct()
3737
public function addRoute(DiscoveredRoute $route): void
3838
{
3939
$this->isDirty = true;
40-
$this->handlerIndex[$route->handler->getDeclaringClass()->getName() . '::' . $route->handler->getName()] = $route->uri;
40+
41+
$handler = $route->handler->getDeclaringClass()->getName() . '::' . $route->handler->getName();
42+
$this->handlerIndex[$handler] ??= [];
43+
$this->handlerIndex[$handler][] = $route->uri;
4144

4245
if ($route->isDynamic) {
4346
$this->addDynamicRoute($route);

packages/router/src/UriGenerator.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Tempest\Reflection\MethodReflector;
1616
use Tempest\Router\Exceptions\ControllerActionDoesNotExist;
1717
use Tempest\Router\Exceptions\ControllerMethodHadNoRoute;
18+
use Tempest\Router\Exceptions\ControllerMethodHasMultipleRoutes;
1819
use Tempest\Router\Routing\Construction\DiscoveredRoute;
1920
use Tempest\Support\Arr;
2021
use Tempest\Support\Regex;
@@ -208,9 +209,9 @@ private function normalizeActionToUri(array|string|MethodReflector $action): str
208209

209210
[$controllerClass, $controllerMethod] = is_array($action) ? $action : [$action, '__invoke'];
210211

211-
$uri = $this->routeConfig->handlerIndex[$controllerClass . '::' . $controllerMethod] ?? null;
212+
$routes = array_unique($this->routeConfig->handlerIndex[$controllerClass . '::' . $controllerMethod] ?? []);
212213

213-
if (! $uri) {
214+
if ($routes === []) {
214215
if (! class_exists($controllerClass)) {
215216
throw ControllerActionDoesNotExist::controllerNotFound($controllerClass, $controllerMethod);
216217
}
@@ -222,6 +223,10 @@ private function normalizeActionToUri(array|string|MethodReflector $action): str
222223
throw new ControllerMethodHadNoRoute($controllerClass, $controllerMethod);
223224
}
224225

225-
return Str\ensure_starts_with($uri, '/');
226+
if (count($routes) > 1) {
227+
throw new ControllerMethodHasMultipleRoutes($controllerClass, $controllerMethod, $routes);
228+
}
229+
230+
return Str\ensure_starts_with($routes[0], '/');
226231
}
227232
}

tests/Fixtures/Controllers/ControllerWithRepeatedRoutes.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#[Get('/repeated/d')]
1818
#[Post('/repeated/e')]
1919
#[Post('/repeated/f')]
20+
#[Get('/repeated/f')]
2021
public function __invoke(): Response
2122
{
2223
return new Ok();

tests/Integration/Route/UriGeneratorTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@
1010
use Tempest\Http\GenericRequest;
1111
use Tempest\Http\Method;
1212
use Tempest\Router\Exceptions\ControllerActionDoesNotExist;
13+
use Tempest\Router\Exceptions\ControllerMethodHasMultipleRoutes;
1314
use Tempest\Router\UriGenerator;
1415
use Tests\Tempest\Fixtures\Controllers\ControllerWithEnumBinding;
16+
use Tests\Tempest\Fixtures\Controllers\ControllerWithRepeatedRoutes;
1517
use Tests\Tempest\Fixtures\Controllers\EnumForController;
1618
use Tests\Tempest\Fixtures\Controllers\PrefixController;
1719
use Tests\Tempest\Fixtures\Controllers\TestController;
@@ -282,4 +284,22 @@ public function is_current_uri_with_prefix_decorator(): void
282284

283285
$this->assertTrue($this->generator->isCurrentUri(PrefixController::class));
284286
}
287+
288+
#[Test]
289+
public function controller_with_multiple_routes(): void
290+
{
291+
$this->expectException(ControllerMethodHasMultipleRoutes::class);
292+
$this->expectExceptionMessage('Controller method `' . ControllerWithRepeatedRoutes::class . '::__invoke()` has multiple different routes');
293+
294+
$this->generator->createUri(ControllerWithRepeatedRoutes::class);
295+
}
296+
297+
#[Test]
298+
public function uri_to_controller_with_multiple_routes(): void
299+
{
300+
$this->assertSame(
301+
'/repeated/a',
302+
$this->generator->createUri('/repeated/a'),
303+
);
304+
}
285305
}

0 commit comments

Comments
 (0)