diff --git a/.gitignore b/.gitignore index 88f1b76..a038b4b 100644 --- a/.gitignore +++ b/.gitignore @@ -60,3 +60,4 @@ Thumbs.db # Quickstart example's file-backed JSON store (created at request time) /var/ +/.claude/ diff --git a/CHANGELOG.md b/CHANGELOG.md index c38a4d8..9f3dcd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,81 @@ read alongside the wins. ## Unreleased +### `#[Version]` attribute primitive (`feat/version-attribute`) + +Recovers what the convention router's `/v{N}/{controller}/{action}` +URL shape gave us — automatic URL-versioned routing — without the +convention router. Apps now declare versions via attribute: + +```php +class ProductsController +{ + #[Route('GET', '/products/{id:int}')] + #[Version('v1')] + public function showV1(int $id): array { /* … */ } + + #[Route('GET', '/products/{id:int}')] + #[Version('v2')] + public function showV2(int $id): array { /* … */ } +} +``` + +The `Scanner` registers each at `/v1/products/{id:int}` and +`/v2/products/{id:int}`. Class-level `#[Version]` applies to +every route in the class; method-level wins when both are +present. `routes:check` sees `/v1/...` and `/v2/...` as +distinct paths and won't flag them as conflicts. + +#### Added + +- **`Rxn\Framework\Http\Attribute\Version`** — value object + attribute. `version` (label, e.g. `'v1'` / `'1.0'` / + `'2025-10-15'`), optional `deprecatedAt` / `sunsetAt` (any + `DateTimeImmutable`-parseable date string). +- **`Rxn\Framework\Http\Versioning\Deprecation`** — PSR-15 + middleware that emits RFC 8594 `Deprecation:` / `Sunset:` + headers. Both as IMF-fixdate (`Thu, 01 Jan 2026 00:00:00 GMT`). + Unparseable date inputs are silently dropped — the contract is + best-effort signalling, not "die on bad config." +- **Scanner integration** — when a route's effective `#[Version]` + carries `deprecatedAt` / `sunsetAt`, the Scanner auto-attaches + the `Deprecation` middleware to that route. Apps don't write + per-handler header boilerplate. +- **Path-prefix idempotence** — Scanner detects routes that + already start with the version prefix and doesn't double-prefix + them. So a hand-written `'/v1/old'` + `#[Version('v1')]` lands + at `/v1/old`, not `/v1/v1/old`. +- **`Version::applyTo()`** — single source of truth for "what URL + does this versioned route actually register at." Both the + Scanner and `Routing\ConflictDetector` call it, so the runtime + registration and the static-analysis detection stay in lockstep. +- **`Routing\ConflictDetector` honours `#[Version]`** — `collect()` + now applies the same prefix the Scanner would, so `routes:check` + treats `/v1/widgets/{id}` and `/v2/widgets/{id}` as distinct + paths (no false-positive conflict). Same-version same-pattern + is still flagged as the real ambiguity it is. + +#### Tests + +- 13 Scanner integration tests (method-level prefixes, class-level + applies, method overrides class, unversioned routes stay + unprefixed, no cross-version conflicts, deprecation middleware + attached when needed and not otherwise, RFC 8594 headers + formatted correctly, deprecation middleware decorates + short-circuit responses (auth 401 / rate-limit 429), version- + label tolerance for stray slashes, empty-version rejection, + root-path produces no trailing slash, path-prefix idempotence). +- 8 Deprecation middleware unit tests (bare ISO date, full ISO + with timezone, UTC conversion, null args, unparseable dates, + deprecation-only / sunset-only, terminal response preservation). +- 3 ConflictDetector tests covering the version-aware shape + (collect applies the version prefix, cross-version routes + aren't flagged, same-version same-pattern still flags). + +Suite 618 → 642 / 1329 → 1391. + +--- + ### Strip convention router + legacy AR layer (`chore/strip-convention-router`) The convention router (`App::run()`, `Service\Api`, `Service\Stats`, diff --git a/README.md b/README.md index a57ce08..8fb1faf 100644 --- a/README.md +++ b/README.md @@ -112,6 +112,13 @@ Opinionated pieces worth naming: - **Typed route constraints** (`{id:int}`, `{slug:slug}`, `{id:uuid}`, custom) so `/users/foo` falls through to 404 instead of reaching a controller that has to validate and throw. +- **API versioning as a primitive** — `#[Version('v1')]` on a + method (or class) prefixes the route's path; `#[Version('v1', + deprecatedAt: '…', sunsetAt: '…')]` auto-attaches a middleware + that emits RFC 8594 `Deprecation:` / `Sunset:` headers. Multiple + versions of the same logical endpoint coexist as distinct paths; + `routes:check` knows the difference between "intentional + cross-version routes" and "real conflict." - **Compile-time route conflict detection.** `bin/rxn routes:check` flags ambiguous `#[Route]` patterns before they ship — `/items/{id:int}` vs `/items/{slug:slug}` (slug accepts @@ -229,7 +236,7 @@ cumulative scoreboard is in ```bash composer install -vendor/bin/phpunit # 618 tests, 1329 assertions +vendor/bin/phpunit # 642 tests, 1391 assertions bin/rxn help # CLI subcommands ``` @@ -281,7 +288,7 @@ CI runs lint + phpunit against PHP 8.2, 8.3, and 8.4 Test counts: -- **Rxn framework:** 618 tests / 1329 assertions (`vendor/bin/phpunit`). +- **Rxn framework:** 642 tests / 1391 assertions (`vendor/bin/phpunit`). - **[`davidwyly/rxn-orm`](https://github.com/davidwyly/rxn-orm)** (query builder): 68 tests / 132 assertions, run in that repo. - **[`davidwyly/rxn-observe`](https://github.com/davidwyly/rxn-observe)** diff --git a/docs/routing.md b/docs/routing.md index 9000c22..39a9fc9 100644 --- a/docs/routing.md +++ b/docs/routing.md @@ -142,6 +142,68 @@ $router->group('/api/v1', function (RouteGroup $g) use ($rateLimit, $auth) { `GET /api/v1/products/42` inherits `$rateLimit`; the `/admin/*` routes inherit `$rateLimit` + `$auth`. +### API versioning — `#[Version]` + +`#[Version('v1')]` on a controller method (or class) prefixes +its `#[Route]` paths with `/v1`. The `Scanner` registers each +version as a distinct path, so multiple versions of the same +logical endpoint coexist: + +```php +use Rxn\Framework\Http\Attribute\{Route, Version}; + +class ProductsController +{ + #[Route('GET', '/products/{id:int}')] + #[Version('v1')] + public function showV1(int $id): array { /* … */ } + + #[Route('GET', '/products/{id:int}')] + #[Version('v2')] + public function showV2(int $id): array { /* … */ } +} +``` + +After `Scanner::register()`: + +- `GET /v1/products/42` → `showV1` +- `GET /v2/products/42` → `showV2` + +Class-level `#[Version]` applies to every `#[Route]` in the +class. Method-level wins when both are present. + +`bin/rxn routes:check` won't flag cross-version routes as +conflicts — `/v1/products/{id:int}` and `/v2/products/{id:int}` +are different paths in the Router. + +#### Deprecation signals + +Pass `deprecatedAt` and/or `sunsetAt` (any `DateTimeImmutable`- +parseable date string) and the Scanner auto-attaches a +`Versioning\Deprecation` middleware that emits the matching +RFC 8594 response headers: + +```php +#[Route('GET', '/old/{id:int}')] +#[Version('v1', deprecatedAt: '2026-01-01', sunsetAt: '2026-12-31')] +public function show(int $id): array { /* … */ } +``` + +Outgoing responses gain: + +- `Deprecation: Thu, 01 Jan 2026 00:00:00 GMT` +- `Sunset: Thu, 31 Dec 2026 00:00:00 GMT` + +Both are advisory — they don't change response status, just +signal to API clients (and gateways) that the endpoint is on +its way out. Apps that prefer to wire the middleware by hand +can construct it directly: + +```php +$router->get('/products', $handler) + ->middleware(new \Rxn\Framework\Http\Versioning\Deprecation('2026-01-01', '2026-12-31')); +``` + ### Using matched routes with the pipeline `Router::match()` returns the matched route's `middlewares` alongside diff --git a/src/Rxn/Framework/Http/Attribute/Scanner.php b/src/Rxn/Framework/Http/Attribute/Scanner.php index aaaf621..3196ed2 100644 --- a/src/Rxn/Framework/Http/Attribute/Scanner.php +++ b/src/Rxn/Framework/Http/Attribute/Scanner.php @@ -5,22 +5,33 @@ use Psr\Http\Server\MiddlewareInterface; use Rxn\Framework\Container; use Rxn\Framework\Http\Router; +use Rxn\Framework\Http\Versioning\Deprecation; /** - * Turn #[Route] + #[Middleware] attributes on controller classes - * into live entries on a Router. Class-level #[Middleware] wraps - * every method's route; method-level #[Middleware] adds to the - * stack after class-level. Each middleware class is resolved - * through the container so autowired constructor deps work. + * Turn #[Route] + #[Middleware] + #[Version] attributes on + * controller classes into live entries on a Router. + * + * - **Class-level `#[Middleware]`** wraps every method's route; + * method-level `#[Middleware]` adds to the stack after + * class-level. + * - **Class-level `#[Version]`** prefixes every route in the + * class with `/$version`. Method-level `#[Version]` overrides + * class-level when both are present. + * - **`#[Version]` with `deprecatedAt` / `sunsetAt`** auto- + * attaches a `Versioning\Deprecation` middleware to the + * route — outgoing responses gain RFC 8594 `Deprecation:` / + * `Sunset:` headers without per-handler boilerplate. * * (new Scanner($container))->register( * $router, * [ProductsController::class, OrdersController::class], * ); * - * The handler stored on the route is `[ClassName, 'method']` — - * callers pick the dispatch strategy (container->get + invoke, - * direct call on a fresh instance, etc.). + * Each middleware class is resolved through the container so + * autowired constructor deps work. The handler stored on the + * route is `[ClassName, 'method']` — callers pick the dispatch + * strategy (container->get + invoke, direct call on a fresh + * instance, etc.). */ final class Scanner { @@ -44,6 +55,7 @@ private function registerOne(Router $router, string $class): void } $ref = new \ReflectionClass($class); $classMiddlewares = $this->resolveMiddlewareAttrs($ref->getAttributes(Middleware::class)); + $classVersion = $this->firstVersion($ref->getAttributes(Version::class)); foreach ($ref->getMethods(\ReflectionMethod::IS_PUBLIC) as $method) { if ($method->getDeclaringClass()->getName() !== $ref->getName()) { @@ -56,16 +68,51 @@ private function registerOne(Router $router, string $class): void $methodMiddlewares = $this->resolveMiddlewareAttrs($method->getAttributes(Middleware::class)); $stack = array_merge($classMiddlewares, $methodMiddlewares); + // Method-level #[Version] wins over class-level. The + // effective version applies to every #[Route] on this + // method (the same handler can serve multiple verbs / + // paths via repeated #[Route], but they all share the + // method's version annotation). + $methodVersion = $this->firstVersion($method->getAttributes(Version::class)); + $effectiveVersion = $methodVersion ?? $classVersion; + + // Auto-attach the deprecation middleware once per + // method, not per Route attribute, so the same response + // header doesn't get added twice for repeat-routed + // handlers. + // + // Prepend, not append: Pipeline runs middleware in + // registration order, so the first one wraps every + // later one. We need Deprecation OUTERMOST so its + // `process()` decorates whatever response comes back — + // including short-circuit responses from inner + // middleware (e.g. auth's 401, rate-limit's 429). If + // we appended, those failure paths would leave the + // route without the documented headers. + $perRouteStack = $stack; + if ($effectiveVersion !== null && self::hasDeprecation($effectiveVersion)) { + array_unshift( + $perRouteStack, + new Deprecation( + $effectiveVersion->deprecatedAt, + $effectiveVersion->sunsetAt, + ), + ); + } + foreach ($routeAttrs as $attr) { /** @var Route $route */ - $route = $attr->newInstance(); - $handle = [$class, $method->getName()]; - $entry = $router->add($route->method, $route->path, $handle); + $route = $attr->newInstance(); + $path = $effectiveVersion === null + ? $route->path + : $effectiveVersion->applyTo($route->path); + $handle = [$class, $method->getName()]; + $entry = $router->add($route->method, $path, $handle); if ($route->name !== null) { $entry->name($route->name); } - if ($stack !== []) { - $entry->middleware(...$stack); + if ($perRouteStack !== []) { + $entry->middleware(...$perRouteStack); } } } @@ -91,4 +138,27 @@ private function resolveMiddlewareAttrs(array $attrs): array } return $out; } + + /** + * Take the first `#[Version]` attribute on a method or class. + * `#[Version]` is not declared `IS_REPEATABLE`, so there's at + * most one, but reflection still hands us a list — we just + * pick element zero or null. + * + * @param list<\ReflectionAttribute> $attrs + */ + private function firstVersion(array $attrs): ?Version + { + if ($attrs === []) { + return null; + } + /** @var Version $v */ + $v = $attrs[0]->newInstance(); + return $v; + } + + private static function hasDeprecation(Version $version): bool + { + return $version->deprecatedAt !== null || $version->sunsetAt !== null; + } } diff --git a/src/Rxn/Framework/Http/Attribute/Version.php b/src/Rxn/Framework/Http/Attribute/Version.php new file mode 100644 index 0000000..aaa97c1 --- /dev/null +++ b/src/Rxn/Framework/Http/Attribute/Version.php @@ -0,0 +1,91 @@ +version, '/'); + if ($label === '') { + throw new \InvalidArgumentException( + "#[Version] label cannot be empty (got '{$this->version}')" + ); + } + $prefix = '/' . $label; + if (str_starts_with($path, $prefix . '/') || $path === $prefix) { + return $path; + } + // Root path special case: `'/'` (or empty) + version + // `'v1'` should yield `'/v1'`, not `'/v1/'`. Without + // this, the stored pattern carries a trailing slash that + // `Router::url()` and `ConflictDetector` reports would + // surface, even though `Router::compile()` would + // normalise it away during match. Better to keep one + // canonical form. + if ($path === '' || $path === '/') { + return $prefix; + } + // `$path` is conventionally rooted at `/` — concat is enough. + return $prefix . (str_starts_with($path, '/') ? $path : '/' . $path); + } +} diff --git a/src/Rxn/Framework/Http/Routing/ConflictDetector.php b/src/Rxn/Framework/Http/Routing/ConflictDetector.php index 130c8f7..f87402f 100644 --- a/src/Rxn/Framework/Http/Routing/ConflictDetector.php +++ b/src/Rxn/Framework/Http/Routing/ConflictDetector.php @@ -3,6 +3,7 @@ namespace Rxn\Framework\Http\Routing; use Rxn\Framework\Http\Attribute\Route; +use Rxn\Framework\Http\Attribute\Version; /** * Compile-time route conflict detector. Scans `#[Route]` attributes @@ -107,6 +108,17 @@ public function __construct(?array $constraints = null) * skipped — same rule the runtime Scanner applies, so detection * scope matches registration scope. * + * `#[Version]` is honoured here for the same reason: the + * detector's notion of "the route's path" has to match what + * the Scanner actually registers. Without this, two methods + * with identical `#[Route]` patterns but different + * `#[Version('v1')]` / `#[Version('v2')]` would still be + * flagged as conflicting — even though they end up at distinct + * `/v1/...` and `/v2/...` URLs at runtime. + * + * Method-level `#[Version]` overrides class-level (matches + * Scanner's resolution rule). + * * @param list $controllers * @return list */ @@ -117,17 +129,23 @@ public function collect(array $controllers): array if (!class_exists($class)) { continue; } - $ref = new \ReflectionClass($class); + $ref = new \ReflectionClass($class); + $classVersion = self::firstVersion($ref->getAttributes(Version::class)); foreach ($ref->getMethods(\ReflectionMethod::IS_PUBLIC) as $method) { if ($method->getDeclaringClass()->getName() !== $ref->getName()) { continue; } + $methodVersion = self::firstVersion($method->getAttributes(Version::class)); + $effectiveVersion = $methodVersion ?? $classVersion; foreach ($method->getAttributes(Route::class) as $attr) { /** @var Route $route */ - $route = $attr->newInstance(); + $route = $attr->newInstance(); + $pattern = $effectiveVersion === null + ? $route->path + : $effectiveVersion->applyTo($route->path); $entries[] = new RouteEntry( method: strtoupper($route->method), - pattern: self::normalisePattern($route->path), + pattern: self::normalisePattern($pattern), class: $class, methodName: $method->getName(), file: (string) $method->getFileName(), @@ -139,6 +157,23 @@ class: $class, return $entries; } + /** + * Pick the first `#[Version]` from a reflection-attribute list + * (or null when none). `#[Version]` isn't `IS_REPEATABLE`, so + * there's at most one — but reflection still hands us a list. + * + * @param list<\ReflectionAttribute> $attrs + */ + private static function firstVersion(array $attrs): ?Version + { + if ($attrs === []) { + return null; + } + /** @var Version $v */ + $v = $attrs[0]->newInstance(); + return $v; + } + /** * Find routes whose patterns reference unknown constraint types * or contain malformed placeholders. The runtime `Router::compile()` diff --git a/src/Rxn/Framework/Http/Versioning/Deprecation.php b/src/Rxn/Framework/Http/Versioning/Deprecation.php new file mode 100644 index 0000000..859d9aa --- /dev/null +++ b/src/Rxn/Framework/Http/Versioning/Deprecation.php @@ -0,0 +1,86 @@ +get('/products', $handler) + * ->middleware(new Deprecation('2026-01-01', '2026-12-31')); + * + * Headers emitted: + * + * - `Deprecation: ` when a deprecation date is + * set. Per RFC 8594 §2 the value is "the deprecation date + * of the resource version" formatted as + * `Sun, 06 Nov 1994 08:49:37 GMT`. + * - `Sunset: ` when a sunset date is set + * (RFC 8594 §3). + * + * Both are advisory headers — they don't change response status + * or behaviour, just signal to API clients (and intermediaries + * like API gateways) that the endpoint is on its way out. + */ +final class Deprecation implements MiddlewareInterface +{ + public function __construct( + private readonly ?string $deprecatedAt = null, + private readonly ?string $sunsetAt = null, + ) {} + + public function process( + ServerRequestInterface $request, + RequestHandlerInterface $handler, + ): ResponseInterface { + $response = $handler->handle($request); + if ($this->deprecatedAt !== null) { + $formatted = self::toImfFixdate($this->deprecatedAt); + if ($formatted !== null) { + $response = $response->withHeader('Deprecation', $formatted); + } + } + if ($this->sunsetAt !== null) { + $formatted = self::toImfFixdate($this->sunsetAt); + if ($formatted !== null) { + $response = $response->withHeader('Sunset', $formatted); + } + } + return $response; + } + + /** + * Turn an ISO 8601-ish input into an RFC 7231 IMF-fixdate + * (`Sun, 06 Nov 1994 08:49:37 GMT`). Inputs the framework + * accepts: + * + * - bare date `'2026-01-01'` → midnight UTC + * - full ISO `'2026-01-01T12:00:00+00:00'` + * - any string `\DateTimeImmutable::__construct` accepts + * + * Returns null on parse failure rather than throwing — the + * decorator's contract is "best-effort header signalling", + * not "fail the request." + */ + private static function toImfFixdate(string $input): ?string + { + if (trim($input) === '') { + return null; + } + try { + $dt = new \DateTimeImmutable($input, new \DateTimeZone('UTC')); + } catch (\Exception) { + return null; + } + // RFC 7231 §7.1.1.1 IMF-fixdate format. UTC required. + return $dt->setTimezone(new \DateTimeZone('UTC')) + ->format('D, d M Y H:i:s') . ' GMT'; + } +} diff --git a/src/Rxn/Framework/Tests/Http/Attribute/Fixture/AlwaysReject.php b/src/Rxn/Framework/Tests/Http/Attribute/Fixture/AlwaysReject.php new file mode 100644 index 0000000..3f945ff --- /dev/null +++ b/src/Rxn/Framework/Tests/Http/Attribute/Fixture/AlwaysReject.php @@ -0,0 +1,31 @@ +handle()`. Used by the + * Scanner's "Deprecation must be outermost" test to prove that + * the auto-attached Deprecation middleware decorates the + * short-circuit response — clients hitting a deprecated endpoint + * with a missing token still learn it's deprecated. + */ +final class AlwaysReject implements MiddlewareInterface +{ + public function process( + ServerRequestInterface $request, + RequestHandlerInterface $handler, + ): ResponseInterface { + return new Response( + 401, + ['Content-Type' => 'application/problem+json'], + '{"type":"about:blank","title":"Unauthorized","status":401}', + ); + } +} diff --git a/src/Rxn/Framework/Tests/Http/Attribute/Fixture/ClassVersionedController.php b/src/Rxn/Framework/Tests/Http/Attribute/Fixture/ClassVersionedController.php new file mode 100644 index 0000000..c72949e --- /dev/null +++ b/src/Rxn/Framework/Tests/Http/Attribute/Fixture/ClassVersionedController.php @@ -0,0 +1,37 @@ + $id]; + } + + // Method-level `#[Version]` — overrides the class-level v3 + // so this one route lands at /v9/sprockets/legacy. Useful for + // beta endpoints inside an otherwise-stable controller. + #[Route('GET', '/sprockets/legacy')] + #[Version('v9')] + public function legacy(): array + { + return []; + } +} diff --git a/src/Rxn/Framework/Tests/Http/Attribute/Fixture/DeprecatedVersionController.php b/src/Rxn/Framework/Tests/Http/Attribute/Fixture/DeprecatedVersionController.php new file mode 100644 index 0000000..5cf5fa1 --- /dev/null +++ b/src/Rxn/Framework/Tests/Http/Attribute/Fixture/DeprecatedVersionController.php @@ -0,0 +1,22 @@ + $id]; + } +} diff --git a/src/Rxn/Framework/Tests/Http/Attribute/Fixture/VersionedController.php b/src/Rxn/Framework/Tests/Http/Attribute/Fixture/VersionedController.php new file mode 100644 index 0000000..91d1a18 --- /dev/null +++ b/src/Rxn/Framework/Tests/Http/Attribute/Fixture/VersionedController.php @@ -0,0 +1,36 @@ + $id, 'version' => 'v1']; + } + + #[Route('GET', '/widgets/{id:int}', name: 'widgets.show.v2')] + #[Version('v2')] + public function showV2(int $id): array + { + return ['id' => $id, 'version' => 'v2']; + } + + // Undecorated route — registered as-is, no version prefix. + #[Route('GET', '/widgets/health', name: 'widgets.health')] + public function health(): array + { + return ['status' => 'ok']; + } +} diff --git a/src/Rxn/Framework/Tests/Http/Attribute/VersionScannerTest.php b/src/Rxn/Framework/Tests/Http/Attribute/VersionScannerTest.php new file mode 100644 index 0000000..1410606 --- /dev/null +++ b/src/Rxn/Framework/Tests/Http/Attribute/VersionScannerTest.php @@ -0,0 +1,321 @@ +scan([VersionedController::class]); + + // Both versions register at distinct paths and route to + // different methods on the same controller. + $v1 = $router->match('GET', '/v1/widgets/42'); + $v2 = $router->match('GET', '/v2/widgets/42'); + + $this->assertNotNull($v1); + $this->assertNotNull($v2); + $this->assertSame([VersionedController::class, 'showV1'], $v1['handler']); + $this->assertSame([VersionedController::class, 'showV2'], $v2['handler']); + } + + public function testUnversionedRoutesInVersionedControllerStayUnprefixed(): void + { + // The `health()` method in `VersionedController` has no + // `#[Version]` and the class has no class-level version. + // It must register at `/widgets/health`, not `/v1/...` or + // anything else inferred. + $router = $this->scan([VersionedController::class]); + + $this->assertNotNull($router->match('GET', '/widgets/health')); + $this->assertNull($router->match('GET', '/v1/widgets/health')); + } + + public function testClassLevelVersionAppliesToEveryRoute(): void + { + $router = $this->scan([ClassVersionedController::class]); + + $this->assertNotNull($router->match('GET', '/v3/sprockets')); + $this->assertNotNull($router->match('GET', '/v3/sprockets/42')); + } + + public function testMethodLevelVersionOverridesClassLevel(): void + { + $router = $this->scan([ClassVersionedController::class]); + + // `legacy()` is method-level v9 — class-level v3 is + // ignored for this route. Look it up at /v9/...; /v3/... + // must not match the legacy handler. + $hit = $router->match('GET', '/v9/sprockets/legacy'); + $this->assertNotNull($hit); + $this->assertSame([ClassVersionedController::class, 'legacy'], $hit['handler']); + + $this->assertNull( + $router->match('GET', '/v3/sprockets/legacy'), + 'class-level v3 must not register the legacy route at /v3/...', + ); + } + + public function testNoCrossVersionConflict(): void + { + // /v1/widgets/{id} and /v2/widgets/{id} are different + // paths — they must not interfere. The detector test + // (ConflictDetector) covers this from the static-analysis + // side; this test covers the runtime side. + $router = $this->scan([VersionedController::class]); + + $this->assertNotNull($router->match('GET', '/v1/widgets/1')); + $this->assertNotNull($router->match('GET', '/v2/widgets/1')); + } + + public function testDeprecatedVersionAttachesDeprecationMiddleware(): void + { + $router = $this->scan([DeprecatedVersionController::class]); + + $hit = $router->match('GET', '/v1/old/42'); + $this->assertNotNull($hit); + + // Exactly one middleware on the route — the auto-attached + // Deprecation. (The fixture has no other class-level or + // method-level middlewares.) + $this->assertCount(1, $hit['middlewares']); + $this->assertInstanceOf(Deprecation::class, $hit['middlewares'][0]); + } + + public function testDeprecationMiddlewareEmitsRfc8594Headers(): void + { + $router = $this->scan([DeprecatedVersionController::class]); + $hit = $router->match('GET', '/v1/old/42'); + + // Drive the middleware end-to-end via a stub terminal so + // the test asserts the actual response shape (header + // formatting, presence of both headers). + $terminal = new class implements RequestHandlerInterface { + public function handle(ServerRequestInterface $request): ResponseInterface + { + return new Response(200, ['Content-Type' => 'application/json'], '{"ok":true}'); + } + }; + + $mw = $hit['middlewares'][0]; + $response = $mw->process(new ServerRequest('GET', 'http://example.test/v1/old/42'), $terminal); + + // RFC 7231 IMF-fixdate: "Day, DD Mon YYYY HH:MM:SS GMT" + $this->assertSame('Thu, 01 Jan 2026 00:00:00 GMT', $response->getHeaderLine('Deprecation')); + $this->assertSame('Thu, 31 Dec 2026 00:00:00 GMT', $response->getHeaderLine('Sunset')); + } + + public function testNonDeprecatedVersionDoesNotAttachMiddleware(): void + { + // VersionedController has plain `#[Version('v1')]` / + // `#[Version('v2')]` — no deprecatedAt or sunsetAt. The + // Scanner must NOT attach a Deprecation middleware in + // that case (it'd be an empty no-op slot, but it'd still + // wrap a request with no signal value). + $router = $this->scan([VersionedController::class]); + $hit = $router->match('GET', '/v1/widgets/42'); + $this->assertSame([], $hit['middlewares']); + } + + public function testDeprecationMiddlewareIsOutermostInRouteStack(): void + { + // Real-world failure path: an auth middleware short- + // circuits with 401, never calling next. If Deprecation + // were innermost, the 401 response wouldn't get the + // RFC 8594 headers — clients hitting a deprecated + // endpoint with a missing token would never learn it's + // deprecated. So Deprecation must wrap the whole route + // stack: it sees the request before the auth middleware + // does, decorates whatever comes back (401 OR success), + // and forwards it. + $controller = new class { + #[\Rxn\Framework\Http\Attribute\Route('GET', '/auth-required')] + #[\Rxn\Framework\Http\Attribute\Middleware(Fixture\AlwaysReject::class)] + #[\Rxn\Framework\Http\Attribute\Version('v1', deprecatedAt: '2026-01-01', sunsetAt: '2026-12-31')] + public function show(): array { return []; } + }; + + $router = $this->scan([$controller::class]); + $hit = $router->match('GET', '/v1/auth-required'); + $this->assertNotNull($hit); + + // Two middlewares: Deprecation (auto-attached) and + // AlwaysReject (the auth-style short-circuit). The + // contract on trial is the ORDER — Deprecation first. + $this->assertCount(2, $hit['middlewares']); + $this->assertInstanceOf(Deprecation::class, $hit['middlewares'][0]); + + // End-to-end: run the route's middleware chain. The + // AlwaysReject middleware short-circuits with a 401, and + // the response should still emerge with Deprecation + + // Sunset headers attached because Deprecation wraps it. + $pipeline = new \Rxn\Framework\Http\Pipeline(); + foreach ($hit['middlewares'] as $mw) { + $pipeline->add($mw); + } + $response = $pipeline->run( + new ServerRequest('GET', 'http://example.test/v1/auth-required'), + new class implements RequestHandlerInterface { + public function handle(ServerRequestInterface $request): ResponseInterface + { + // Should never run — AlwaysReject short-circuits. + return new Response(200); + } + }, + ); + + $this->assertSame(401, $response->getStatusCode()); + $this->assertSame('Thu, 01 Jan 2026 00:00:00 GMT', $response->getHeaderLine('Deprecation')); + $this->assertSame('Thu, 31 Dec 2026 00:00:00 GMT', $response->getHeaderLine('Sunset')); + } + + public function testVersionLabelTolerantOfStraySlashes(): void + { + // `'v1'` / `'/v1'` / `'v1/'` / `'/v1/'` should all produce + // the same `/v1` prefix. Without trimming, a trailing + // slash on the version label would yield a double-slash + // path (`/v1//products`) which the Router can't match. + // + // Each variant gets its own eval'd controller — declared + // inside this test's namespace so `class_exists()` and + // `scan()` see the same class-string. Without the explicit + // `namespace ...;` inside the eval'd source, PHP would + // declare these in the global namespace, and passing the + // bare name to `scan()` would silently miss the route + // registrations (the bug Copilot flagged). + $namespace = __NAMESPACE__ . '\\StraySlashCases'; + $cases = ['v1', '/v1', 'v1/', '/v1/']; + foreach ($cases as $i => $label) { + $shortName = "VersionLabelCase$i"; + $fqcn = $namespace . '\\' . $shortName; + if (!class_exists($fqcn)) { + eval(sprintf( + 'namespace %s; class %s { + #[\Rxn\Framework\Http\Attribute\Route("GET", "/products")] + #[\Rxn\Framework\Http\Attribute\Version(%s)] + public function show(): array { return []; } + }', + $namespace, + $shortName, + var_export($label, true), + )); + } + // Sanity: the FQCN we hand to `scan()` actually + // resolves to something Reflection can introspect. + $this->assertTrue(class_exists($fqcn), "eval'd class $fqcn must be loadable"); + + $router = $this->scan([$fqcn]); + $hit = $router->match('GET', '/v1/products'); + $this->assertNotNull( + $hit, + "label '$label' must resolve to /v1/products", + ); + } + } + + public function testEmptyVersionLabelIsRejected(): void + { + // `#[Version('')]` is meaningless — the prefix would be + // bare `/`, which collides with every root route. Better + // to fail loud at scan time than silently produce + // surprising routing. + $controller = new class { + #[\Rxn\Framework\Http\Attribute\Route('GET', '/products')] + #[\Rxn\Framework\Http\Attribute\Version('')] + public function show(): array { return []; } + }; + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessageMatches('/Version.*empty/'); + $this->scan([$controller::class]); + } + + public function testRootPathDoesNotGetTrailingSlash(): void + { + // `#[Route('/')]` + `#[Version('v1')]` should land at + // `/v1`, not `/v1/`. The trailing-slash form would still + // match at runtime (Router::compile normalises it away), + // but the stored pattern would surface as `/v1/` in + // Router::url() output and ConflictDetector reports — + // two URLs for the same logical route is the kind of + // inconsistency that bites later (e.g. generated client + // SDKs picking the wrong form). + $controller = new class { + #[\Rxn\Framework\Http\Attribute\Route('GET', '/')] + #[\Rxn\Framework\Http\Attribute\Version('v1')] + public function root(): array { return []; } + }; + + $router = $this->scan([$controller::class]); + + // Direct match at /v1 (no trailing slash). + $this->assertNotNull( + $router->match('GET', '/v1'), + '#[Route("/")] + #[Version("v1")] must register at /v1', + ); + + // The STORED pattern matches what Router::url() would + // emit. Without the applyTo() special-case the registered + // pattern would be `/v1/`; this assertion is what catches + // the regression that runtime-match alone would miss. + $hit = $router->match('GET', '/v1'); + $this->assertSame('/v1', $hit['pattern']); + } + + public function testPathAlreadyPrefixedIsNotDoublePrefixed(): void + { + // Apps that hand-write `/v1/foo` in #[Route] AND mark the + // method `#[Version('v1')]` should still register at + // `/v1/foo`, not `/v1/v1/foo`. Defensive idempotence. + $controller = new class { + #[\Rxn\Framework\Http\Attribute\Route('GET', '/v1/already/{id:int}')] + #[\Rxn\Framework\Http\Attribute\Version('v1')] + public function show(int $id): array { return []; } + }; + + $router = $this->scan([$controller::class]); + $this->assertNotNull($router->match('GET', '/v1/already/42')); + $this->assertNull( + $router->match('GET', '/v1/v1/already/42'), + 'route path that already starts with /v1 must not be re-prefixed', + ); + } + + /** @param list $controllers */ + private function scan(array $controllers): Router + { + $router = new Router(); + (new Scanner(new Container()))->register($router, $controllers); + return $router; + } +} diff --git a/src/Rxn/Framework/Tests/Http/Routing/ConflictDetectorTest.php b/src/Rxn/Framework/Tests/Http/Routing/ConflictDetectorTest.php index ee6e1d8..0cc8c6a 100644 --- a/src/Rxn/Framework/Tests/Http/Routing/ConflictDetectorTest.php +++ b/src/Rxn/Framework/Tests/Http/Routing/ConflictDetectorTest.php @@ -311,6 +311,58 @@ public function testResultDescribeRendersBothInvalidAndConflicts(): void $this->assertStringContainsString("unknown constraint type 'nonsense'", $output); } + public function testCollectAppliesVersionPrefix(): void + { + $detector = new ConflictDetector(); + $entries = $detector->collect([Fixture\VersionedConflictController::class]); + $patterns = array_map(static fn ($e) => $e->pattern, $entries); + + // The detector's notion of "the route's path" matches what + // the runtime Scanner actually registers — version prefix + // applied at collect time. + $this->assertContains('/v1/widgets/{id:int}', $patterns); + $this->assertContains('/v2/widgets/{id:int}', $patterns); + // Class-level v1 applies to the un-overridden index method. + $this->assertContains('/v1/widgets', $patterns); + } + + public function testCrossVersionRoutesAreNotFlaggedAsConflict(): void + { + // /v1/widgets/{id:int} and /v2/widgets/{id:int} are + // distinct paths after version prefixing — same logical + // endpoint, different URLs at runtime. The detector must + // not block CI on these. + $detector = new ConflictDetector(); + $entries = $detector->collect([Fixture\VersionedConflictController::class]); + $conflicts = $detector->detect($entries); + + $crossVersion = array_filter( + $conflicts, + static fn ($c) => ($c->a->methodName === 'showV1' && $c->b->methodName === 'showV2') + || ($c->a->methodName === 'showV2' && $c->b->methodName === 'showV1'), + ); + $this->assertSame([], $crossVersion, 'cross-version routes must not be flagged as conflicts'); + } + + public function testSameVersionSamePatternStillFlagsConflict(): void + { + // showV1 + showV1Duplicate both end up at + // /v1/widgets/{id:int} — same pattern, same effective + // version, real ambiguity. The detector MUST flag this; + // version prefixing isn't an escape hatch from the + // detector's job. + $detector = new ConflictDetector(); + $entries = $detector->collect([Fixture\VersionedConflictController::class]); + $conflicts = $detector->detect($entries); + + $duplicate = array_filter( + $conflicts, + static fn ($c) => in_array('showV1', [$c->a->methodName, $c->b->methodName], true) + && in_array('showV1Duplicate', [$c->a->methodName, $c->b->methodName], true), + ); + $this->assertNotEmpty($duplicate, 'same-version same-pattern routes must be flagged'); + } + private static function entry(string $method, string $pattern): RouteEntry { return new RouteEntry( diff --git a/src/Rxn/Framework/Tests/Http/Routing/Fixture/VersionedConflictController.php b/src/Rxn/Framework/Tests/Http/Routing/Fixture/VersionedConflictController.php new file mode 100644 index 0000000..f97db6e --- /dev/null +++ b/src/Rxn/Framework/Tests/Http/Routing/Fixture/VersionedConflictController.php @@ -0,0 +1,35 @@ +process( + new ServerRequest('GET', 'http://example.test/'), + $this->terminal(), + ); + } + + public function testBareIsoDateBecomesImfFixdate(): void + { + $response = $this->exercise(new Deprecation('2026-01-01', '2026-12-31')); + $this->assertSame('Thu, 01 Jan 2026 00:00:00 GMT', $response->getHeaderLine('Deprecation')); + $this->assertSame('Thu, 31 Dec 2026 00:00:00 GMT', $response->getHeaderLine('Sunset')); + } + + public function testFullIsoDateRoundTripsToImfFixdate(): void + { + $response = $this->exercise(new Deprecation('2026-01-01T12:34:56+00:00', null)); + $this->assertSame('Thu, 01 Jan 2026 12:34:56 GMT', $response->getHeaderLine('Deprecation')); + $this->assertSame('', $response->getHeaderLine('Sunset')); + } + + public function testTimezonedDateConvertsToUtc(): void + { + // 2026-01-01 00:00:00 in +05:00 = 2025-12-31 19:00:00 UTC. + // The middleware emits the UTC equivalent. + $response = $this->exercise(new Deprecation('2026-01-01T00:00:00+05:00', null)); + $this->assertSame('Wed, 31 Dec 2025 19:00:00 GMT', $response->getHeaderLine('Deprecation')); + } + + public function testNullArgsEmitNoHeaders(): void + { + $response = $this->exercise(new Deprecation(null, null)); + $this->assertSame('', $response->getHeaderLine('Deprecation')); + $this->assertSame('', $response->getHeaderLine('Sunset')); + } + + public function testUnparseableDateEmitsNothingNotAFailure(): void + { + // Garbage date — the middleware silently drops it rather + // than 500ing the request. The contract is "best-effort + // header signalling", not "die on bad config." + $response = $this->exercise(new Deprecation('not-a-date', 'also-not')); + $this->assertSame('', $response->getHeaderLine('Deprecation')); + $this->assertSame('', $response->getHeaderLine('Sunset')); + $this->assertSame(200, $response->getStatusCode()); + } + + public function testDeprecationOnlyDoesNotAddSunset(): void + { + // Common case: "this version is deprecated, but we + // haven't decided when it'll be retired." Deprecation + // header fires, Sunset is left blank. + $response = $this->exercise(new Deprecation('2026-01-01', null)); + $this->assertNotSame('', $response->getHeaderLine('Deprecation')); + $this->assertSame('', $response->getHeaderLine('Sunset')); + } + + public function testSunsetOnlyDoesNotAddDeprecation(): void + { + // Less common, but valid per RFC 8594: an endpoint can be + // scheduled for sunset without a separate deprecation + // signal (the sunset itself implies the upcoming removal). + $response = $this->exercise(new Deprecation(null, '2026-12-31')); + $this->assertSame('', $response->getHeaderLine('Deprecation')); + $this->assertNotSame('', $response->getHeaderLine('Sunset')); + } + + public function testTerminalResponseIsPreserved(): void + { + // The middleware must NOT swallow the downstream + // response — only add headers to it. Body, status, + // existing headers all pass through. + $upstream = new Deprecation('2026-01-01', null); + $response = $upstream->process( + new ServerRequest('GET', 'http://example.test/'), + new class implements RequestHandlerInterface { + public function handle(ServerRequestInterface $request): ResponseInterface + { + return new Response( + 201, + ['Content-Type' => 'application/json', 'X-Custom' => 'preserved'], + '{"id":42}', + ); + } + }, + ); + + $this->assertSame(201, $response->getStatusCode()); + $this->assertSame('application/json', $response->getHeaderLine('Content-Type')); + $this->assertSame('preserved', $response->getHeaderLine('X-Custom')); + $this->assertSame('{"id":42}', (string) $response->getBody()); + } +}