fix: keep the travel date code to a single path segment
The dateCode requirement added with the export route was `.+`, which
replaces Symfony's default `[^/]+` and lets the segment span slashes.
`/api/travels/{dateId}/{hotelId}/{dateTo}/availability` was then matched
by byCode() -- declared above it -- with dateCode swallowing all four
segments, so availability answered 404 "Not found" instead of reaching
the upstream call.
Adds a routing test that reflects the controller's own attributes, so
both the declaration order and the segment class stay guarded. Neither
fails loudly on its own: a path falling through to the wrong route reads
like an endpoint that exists and holds no data.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
1 parent
ca1ab17595
commit
b27639c7fa
2 files changed
+100
-1
No files matched your search
@@ -0,0 +1,97 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Controller\Api;
|
||||
|
||||
use App\Controller\Api\TravelController;
|
||||
use PHPUnit\Framework\Attributes\DataProvider;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
use Symfony\Component\Routing\Attribute\Route as RouteAttribute;
|
||||
use Symfony\Component\Routing\Matcher\UrlMatcher;
|
||||
use Symfony\Component\Routing\RequestContext;
|
||||
use Symfony\Component\Routing\Route;
|
||||
use Symfony\Component\Routing\RouteCollection;
|
||||
|
||||
/**
|
||||
* Guards the ordering and the segment requirements of the /api/travels routes.
|
||||
*
|
||||
* Both are load-bearing and neither fails loudly: a path that falls through to the wrong
|
||||
* route answers 404 with a body that reads like an endpoint holding no data. The collection
|
||||
* is built by reflecting over the real attributes, so this breaks if the declarations move.
|
||||
*/
|
||||
class TravelControllerRoutingTest extends TestCase
|
||||
{
|
||||
/**
|
||||
* @return array<string, array{string, string}>
|
||||
*/
|
||||
public static function pathProvider(): array
|
||||
{
|
||||
return [
|
||||
'the export route wins over the code lookup' => ['/api/travels/export', 'api_travel_export'],
|
||||
// Regression: a dateCode requirement of `.+` instead of `[^/]+` widens the segment
|
||||
// across slashes, and this four-segment path is then swallowed by byCode().
|
||||
'availability is not shadowed by the code lookup' => [
|
||||
'/api/travels/12794/225444/2026-12-30/availability',
|
||||
'api_travel_hotel_availability',
|
||||
],
|
||||
'numeric ids still reach byId' => ['/api/travels/12794/225444', 'api_travel_single_id'],
|
||||
'a date code alone reaches byCode' => ['/api/travels/SBW261226', 'api_travel_single_code'],
|
||||
'a date and hotel code reach byCode' => ['/api/travels/SBW261226/H1', 'api_travel_single_code'],
|
||||
'the mapping index is untouched' => ['/api/travels', 'api_travel_mapping'],
|
||||
];
|
||||
}
|
||||
|
||||
#[DataProvider('pathProvider')]
|
||||
public function testPathResolvesToTheIntendedRoute(string $path, string $expected): void
|
||||
{
|
||||
$matcher = new UrlMatcher($this->collection(), new RequestContext('', 'GET'));
|
||||
|
||||
self::assertSame($expected, $matcher->match($path)['_route']);
|
||||
}
|
||||
|
||||
public function testDateCodeNeverSpansSlashes(): void
|
||||
{
|
||||
$route = $this->collection()->get('api_travel_single_code');
|
||||
|
||||
self::assertNotNull($route);
|
||||
self::assertStringNotContainsString(
|
||||
'.+',
|
||||
(string) $route->getRequirement('dateCode'),
|
||||
'dateCode must keep the single-segment class, or it shadows the routes below it',
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reflects the controller's own #[Route] attributes, in declaration order.
|
||||
*/
|
||||
private function collection(): RouteCollection
|
||||
{
|
||||
$class = new \ReflectionClass(TravelController::class);
|
||||
|
||||
$prefix = '';
|
||||
foreach ($class->getAttributes(RouteAttribute::class) as $attribute) {
|
||||
$prefix = (string) $attribute->newInstance()->path;
|
||||
}
|
||||
|
||||
$collection = new RouteCollection();
|
||||
|
||||
foreach ($class->getMethods(\ReflectionMethod::IS_PUBLIC) as $method) {
|
||||
foreach ($method->getAttributes(RouteAttribute::class) as $attribute) {
|
||||
$route = $attribute->newInstance();
|
||||
|
||||
$collection->add((string) $route->name, new Route(
|
||||
$prefix.$route->path,
|
||||
$route->defaults,
|
||||
$route->requirements,
|
||||
[],
|
||||
'',
|
||||
[],
|
||||
$route->methods,
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
return $collection;
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user