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..097fe54 --- /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], $routes->getPriority($name) ?? 0); + } + + 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..259c651 --- /dev/null +++ b/tests/Unit/Routing/RouteSpecificitySorterTest.php @@ -0,0 +1,94 @@ +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); + } + + 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')); + } +}