From 9265e1b8b79f8774e631bc09cb00997e4aecbcd7 Mon Sep 17 00:00:00 2001 From: Jelle van Oosterbosch Date: Mon, 15 Jun 2026 20:43:06 +0200 Subject: [PATCH 1/2] Order command routes most-specific-first so literal paths win over placeholders The command-route directory loader registered routes in filename-sort order, and CommandRouteClassLoader builds every route with empty requirements, so a placeholder segment such as {id} compiles to [^/]++ and also matches a sibling literal segment (for example a /resource/{id} route swallowing /resource/current). Resolution then depends on registration order rather than path specificity. RouteSpecificitySorter reorders the scanned command routes most-specific-first (fewer placeholders, then longer static prefix, stable otherwise) and is applied in AttributeDirectoryLoaderDecorator before the routes are merged, so concrete paths are matched before templated ones per the OpenAPI path-precedence rule. Per-class $routes->import(...) ordering is unchanged. --- .../AttributeDirectoryLoaderDecorator.php | 3 +- src/Routing/RouteSpecificitySorter.php | 72 +++++++++++++++++ .../Routing/src/CollectionItemCommand.php | 21 +++++ .../Routing/src/CollectionLiteralCommand.php | 21 +++++ .../AttributeDirectoryLoaderDecoratorTest.php | 18 +++++ .../Routing/RouteSpecificitySorterTest.php | 79 +++++++++++++++++++ 6 files changed, 213 insertions(+), 1 deletion(-) create mode 100644 src/Routing/RouteSpecificitySorter.php create mode 100644 tests/Mock/Routing/src/CollectionItemCommand.php create mode 100644 tests/Mock/Routing/src/CollectionLiteralCommand.php create mode 100644 tests/Unit/Routing/RouteSpecificitySorterTest.php diff --git a/src/Routing/Loader/AttributeDirectoryLoaderDecorator.php b/src/Routing/Loader/AttributeDirectoryLoaderDecorator.php index d7be38f..fc10560 100644 --- a/src/Routing/Loader/AttributeDirectoryLoaderDecorator.php +++ b/src/Routing/Loader/AttributeDirectoryLoaderDecorator.php @@ -13,6 +13,7 @@ namespace Stixx\OpenApiCommandBundle\Routing\Loader; +use Stixx\OpenApiCommandBundle\Routing\RouteSpecificitySorter; use Symfony\Component\Config\FileLocatorInterface; use Symfony\Component\Config\Loader\Loader; use Symfony\Component\Routing\Loader\AttributeDirectoryLoader; @@ -49,7 +50,7 @@ public function load(mixed $resource, ?string $type = null): RouteCollection $commands = $commandDirLoader->load($projectDirectory, 'attribute'); if ($commands instanceof RouteCollection) { - $collection->addCollection($commands); + $collection->addCollection((new RouteSpecificitySorter())->sort($commands)); } return $collection; diff --git a/src/Routing/RouteSpecificitySorter.php b/src/Routing/RouteSpecificitySorter.php new file mode 100644 index 0000000..ee9fde6 --- /dev/null +++ b/src/Routing/RouteSpecificitySorter.php @@ -0,0 +1,72 @@ +all(); + + $positions = []; + $position = 0; + foreach ($all as $name => $route) { + $positions[$name] = $position; + ++$position; + } + + $names = array_keys($all); + usort($names, function (string $left, string $right) use ($all, $positions): int { + $byPlaceholders = $this->placeholderCount($all[$left]) <=> $this->placeholderCount($all[$right]); + if ($byPlaceholders !== 0) { + return $byPlaceholders; + } + + $byStaticLength = $this->staticLength($all[$right]) <=> $this->staticLength($all[$left]); + if ($byStaticLength !== 0) { + return $byStaticLength; + } + + return $positions[$left] <=> $positions[$right]; + }); + + $sorted = new RouteCollection(); + foreach ($names as $name) { + $sorted->add($name, $all[$name]); + } + + return $sorted; + } + + private function placeholderCount(Route $route): int + { + return substr_count($route->getPath(), '{'); + } + + private function staticLength(Route $route): int + { + return strlen((string) preg_replace('/\{[^}]*\}/', '', $route->getPath())); + } +} diff --git a/tests/Mock/Routing/src/CollectionItemCommand.php b/tests/Mock/Routing/src/CollectionItemCommand.php new file mode 100644 index 0000000..f5477a5 --- /dev/null +++ b/tests/Mock/Routing/src/CollectionItemCommand.php @@ -0,0 +1,21 @@ +all(), 'Second load returns inner collection without augmentation'); } + public function testCommandRoutesAreOrderedMostSpecificFirst(): void + { + // Arrange — the fixture filenames scan as CollectionItemCommand (/api/items/{id}) before + // CollectionLiteralCommand (/api/items/featured), so without specificity ordering the + // placeholder route would be registered first and swallow the literal one. + $this->inner->method('load')->willReturn(new RouteCollection()); + + $locator = new FileLocator([$this->projectDir]); + $decorator = new AttributeDirectoryLoaderDecorator($this->inner, $locator, new CommandRouteClassLoader(), $this->projectDir); + + // Act + $names = array_keys($decorator->load('ignored')->all()); + $itemRoutes = array_values(array_filter($names, static fn (string $name): bool => str_starts_with($name, 'items_'))); + + // Assert + self::assertSame(['items_featured', 'items_item'], $itemRoutes); + } + public function testSupportsDelegatesToInner(): void { // Arrange diff --git a/tests/Unit/Routing/RouteSpecificitySorterTest.php b/tests/Unit/Routing/RouteSpecificitySorterTest.php new file mode 100644 index 0000000..145ce3f --- /dev/null +++ b/tests/Unit/Routing/RouteSpecificitySorterTest.php @@ -0,0 +1,79 @@ +add('books_get_one', new Route('/api/books/{id}')); + $routes->add('books_featured', new Route('/api/books/featured')); + + // Act + $sorted = array_keys((new RouteSpecificitySorter())->sort($routes)->all()); + + // Assert + self::assertSame(['books_featured', 'books_get_one'], $sorted); + } + + public function testFewerPlaceholdersAreOrderedFirstAcrossDepth(): void + { + // Arrange + $routes = new RouteCollection(); + $routes->add('two', new Route('/api/books/{id}/reviews/{reviewId}')); + $routes->add('one', new Route('/api/books/{id}/reviews')); + $routes->add('zero', new Route('/api/books/reviews')); + + // Act + $sorted = array_keys((new RouteSpecificitySorter())->sort($routes)->all()); + + // Assert + self::assertSame(['zero', 'one', 'two'], $sorted); + } + + public function testLongerStaticPrefixWinsAmongEqualPlaceholderCounts(): void + { + // Arrange — both carry one placeholder; the longer literal prefix is more specific. + $routes = new RouteCollection(); + $routes->add('short', new Route('/api/{id}')); + $routes->add('long', new Route('/api/books/{id}')); + + // Act + $sorted = array_keys((new RouteSpecificitySorter())->sort($routes)->all()); + + // Assert + self::assertSame(['long', 'short'], $sorted); + } + + public function testEquallySpecificRoutesKeepTheirOriginalOrder(): void + { + // Arrange — same placeholder count and static length, so the original order must be preserved. + $routes = new RouteCollection(); + $routes->add('alpha', new Route('/api/aaa/{id}')); + $routes->add('beta', new Route('/api/bbb/{id}')); + + // Act + $sorted = array_keys((new RouteSpecificitySorter())->sort($routes)->all()); + + // Assert + self::assertSame(['alpha', 'beta'], $sorted); + } +} From 09f1787371870e16a0065e2d13250b9013de8cce Mon Sep 17 00:00:00 2001 From: Jelle van Oosterbosch Date: Mon, 15 Jun 2026 20:52:52 +0200 Subject: [PATCH 2/2] Preserve route priorities when reordering for specificity RouteCollection::add() carries a route's priority as its third argument; the specificity rebuild now forwards RouteCollection::getPriority() so an explicit non-default priority survives the sort. Behaviour is unchanged for the default priority 0 (not stored, so the collection stays in specificity order); when a priority is set it dominates, with specificity as the stable tiebreak. --- src/Routing/RouteSpecificitySorter.php | 2 +- tests/Unit/Routing/RouteSpecificitySorterTest.php | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/Routing/RouteSpecificitySorter.php b/src/Routing/RouteSpecificitySorter.php index ee9fde6..097fe54 100644 --- a/src/Routing/RouteSpecificitySorter.php +++ b/src/Routing/RouteSpecificitySorter.php @@ -54,7 +54,7 @@ public function sort(RouteCollection $routes): RouteCollection $sorted = new RouteCollection(); foreach ($names as $name) { - $sorted->add($name, $all[$name]); + $sorted->add($name, $all[$name], $routes->getPriority($name) ?? 0); } return $sorted; diff --git a/tests/Unit/Routing/RouteSpecificitySorterTest.php b/tests/Unit/Routing/RouteSpecificitySorterTest.php index 145ce3f..259c651 100644 --- a/tests/Unit/Routing/RouteSpecificitySorterTest.php +++ b/tests/Unit/Routing/RouteSpecificitySorterTest.php @@ -76,4 +76,19 @@ public function testEquallySpecificRoutesKeepTheirOriginalOrder(): void // Assert self::assertSame(['alpha', 'beta'], $sorted); } + + public function testItPreservesExistingRoutePriorities(): void + { + // Arrange — a non-default priority must survive the rebuild into the sorted collection. + $routes = new RouteCollection(); + $routes->add('low', new Route('/api/books/{id}')); + $routes->add('high', new Route('/api/books/{id}/reviews'), 10); + + // Act + $sorted = (new RouteSpecificitySorter())->sort($routes); + + // Assert + self::assertSame(10, $sorted->getPriority('high')); + self::assertNull($sorted->getPriority('low')); + } }