diff --git a/README.md b/README.md index 11d1c8b..64e27d2 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,8 @@ stixx_openapi_command: enabled: true groups: ['Default'] cache_control: 'no-store' # Any valid Cache-Control directives, or null to disable + command_paths: # Directories scanned for command DTOs; [] disables discovery + - '%kernel.project_dir%/src' openapi: problem_details: true # Enable RFC 7807 problem details for errors ``` diff --git a/config/routing.php b/config/routing.php index 7641239..98346dc 100644 --- a/config/routing.php +++ b/config/routing.php @@ -2,9 +2,12 @@ declare(strict_types=1); -use Stixx\OpenApiCommandBundle\Routing\Loader\AttributeDirectoryLoaderDecorator; +use Stixx\OpenApiCommandBundle\Routing\CommandRouteDiscovery; use Stixx\OpenApiCommandBundle\Routing\Loader\CommandRouteClassLoader; +use Stixx\OpenApiCommandBundle\Routing\Loader\CommandRouteDirectoryLoader; +use Stixx\OpenApiCommandBundle\Routing\Loader\RouterLoaderDecorator; use Stixx\OpenApiCommandBundle\Routing\NelmioAreaRoutesChecker; +use Stixx\OpenApiCommandBundle\Routing\RouteSpecificitySorter; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; use function Symfony\Component\DependencyInjection\Loader\Configurator\param; use function Symfony\Component\DependencyInjection\Loader\Configurator\service; @@ -19,17 +22,32 @@ ->arg('$routesLocator', service('stixx_openapi_command.nelmio.routes_locator')) ->arg('$pathPatterns', param('stixx_openapi_command.nelmio.path_patterns')); + $services + ->set(RouteSpecificitySorter::class); + $services ->set(CommandRouteClassLoader::class) - ->arg('$env', param('kernel.environment')) - ->arg('$controllerClasses', param('stixx_openapi_command.controller_classes')) - ->tag('routing.loader'); + ->arg('$env', param('kernel.environment')) + ->arg('$controllerClasses', param('stixx_openapi_command.controller_classes')) + // Supports $routes->import(SomeCommand::class, 'attribute'). + ->tag('routing.loader'); $services - ->set(AttributeDirectoryLoaderDecorator::class) - ->decorate('routing.loader.attribute.directory') - ->arg('$inner', service('.inner')) + ->set(CommandRouteDirectoryLoader::class) ->arg('$locator', service('file_locator')) - ->arg('$commandAttributeLoader', service(CommandRouteClassLoader::class)) - ->arg('$projectDir', param('kernel.project_dir')); + ->arg('$loader', service(CommandRouteClassLoader::class)) + // Supports $routes->import('../src/Command', 'stixx_openapi_command.command_attributes'). + ->tag('routing.loader'); + + $services + ->set(CommandRouteDiscovery::class) + ->arg('$directoryLoader', service(CommandRouteDirectoryLoader::class)) + ->arg('$commandPaths', param('stixx_openapi_command.command_paths')) + ->arg('$sorter', service(RouteSpecificitySorter::class)); + + $services + ->set(RouterLoaderDecorator::class) + ->decorate('routing.loader') + ->arg('$inner', service('.inner')) + ->arg('$discovery', service(CommandRouteDiscovery::class)); }; diff --git a/docs/command-routing.md b/docs/command-routing.md index 7b6d6bd..8baf29f 100644 --- a/docs/command-routing.md +++ b/docs/command-routing.md @@ -119,16 +119,49 @@ sequenceDiagram Starting with this version, you do not need to add any custom route import for command DTOs. How it works -- The bundle decorates Symfony’s `AttributeDirectoryLoader` (the same mechanism used to load controller routes from attributes). -- During the normal route-building process, we automatically scan your project’s `%kernel.project_dir%/src` directory and add routes for command classes that meet the criteria: have class-level OpenAPI operation attributes (e.g., `#[OA\Post]`, `#[OA\Get]`, …) and are not controllers. -- This happens once per router build and coexists with your existing controller routes and any manually configured routes. - +- The bundle decorates `routing.loader`, the loader the router asks for when it builds its route collection. It runs once per router build, for the root routing resource, so command routes are added no matter how your application declares its own routes — or whether it declares any at all. +- During that build, the bundle scans the configured `command_paths` and adds routes for command classes that meet the criteria: class-level OpenAPI operation attributes (e.g., `#[OA\Post]`, `#[OA\Get]`, …) and not a controller. +- Discovered routes coexist with your existing controller routes and any manually configured routes. Notes -- No additional routing import is necessary. The bundle augments the standard attribute route loading automatically. -- The scan is recursive and limited to `%kernel.project_dir%/src`. +- No additional routing import is necessary. +- The scan is recursive and covers the directories listed under `command_paths`, which defaults to `%kernel.project_dir%/src`: + + ```yaml + stixx_openapi_command: + command_paths: + - '%kernel.project_dir%/src/Command' + - '%kernel.project_dir%/lib/Billing/Command' + ``` + + Configured paths that do not exist are skipped, so listing a directory that only some environments have is safe. - Only classes that are annotated with OpenAPI operation attributes (e.g., `#[OA\Post]`) at class level and are not recognized controllers (`AbstractController`, `#[AsController]`, or having method-level `#[Route]`) will produce routes. - Because of this, ensure your commands are plain DTOs and do not extend `AbstractController`, do not use `#[AsController]`, and do not declare method-level `#[Route]` attributes. +- If a route name is already present in the collection — because you imported the command explicitly — the bundle leaves your route alone rather than replacing it. +- Two different command classes resolving to the same route name is an error, and the container fails to compile naming both classes. Without `operationId` the name is derived from the class short name, so `Billing\CreateInvoiceCommand` and `Sales\CreateInvoiceCommand` both resolve to `command_createinvoicecommand`; give at least one of them an explicit `operationId`. Loading the same class twice — via discovery and an explicit import — is not a conflict. + +### Declaring command routes explicitly + +Discovery is optional. To control exactly what gets routed, set `command_paths: []` and import commands from your routing config, either one class at a time: + +```php +// config/routes/commands.php +use App\Command\CreateProjectCommand; +use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator; + +return static function (RoutingConfigurator $routes): void { + $routes->import(CreateProjectCommand::class, 'attribute'); +}; +``` + +or a directory at a time, using the bundle's routing type: + +```yaml +# config/routes/commands.yaml +commands: + resource: '../../src/Command' + type: stixx_openapi_command.command_attributes +``` ## Use OpenAPI attributes on command classes (no Symfony #[Route]) diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 1f48bd3..c8d47e3 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -45,6 +45,11 @@ public function getConfigTreeBuilder(): TreeBuilder ->scalarNode('cache_control') ->defaultValue('no-store') ->end() + ->arrayNode('command_paths') + ->info('Directories scanned for command DTOs carrying OpenAPI operation attributes.') + ->scalarPrototype()->end() + ->defaultValue(['%kernel.project_dir%/src']) + ->end() ->arrayNode('openapi') ->addDefaultsIfNotSet() ->children() diff --git a/src/DependencyInjection/StixxOpenApiCommandExtension.php b/src/DependencyInjection/StixxOpenApiCommandExtension.php index 615d38e..68aff14 100644 --- a/src/DependencyInjection/StixxOpenApiCommandExtension.php +++ b/src/DependencyInjection/StixxOpenApiCommandExtension.php @@ -87,6 +87,10 @@ public function load(array $configs, ContainerBuilder $container): void $cacheControl = $config['cache_control']; $container->setParameter('stixx_openapi_command.cache_control', $cacheControl); + /** @var list $commandPaths */ + $commandPaths = $config['command_paths']; + $container->setParameter('stixx_openapi_command.command_paths', $commandPaths); + $container ->registerForAutoconfiguration(ResponderInterface::class) ->addTag(ResponderInterface::TAG_NAME); diff --git a/src/Routing/CommandRouteDiscovery.php b/src/Routing/CommandRouteDiscovery.php new file mode 100644 index 0000000..57a3d56 --- /dev/null +++ b/src/Routing/CommandRouteDiscovery.php @@ -0,0 +1,64 @@ + $commandPaths + */ + public function __construct( + private readonly CommandRouteDirectoryLoader $directoryLoader, + private readonly array $commandPaths, + private readonly RouteSpecificitySorter $sorter = new RouteSpecificitySorter(), + ) { + } + + /** + * Scans every configured path once, sorting the result so concrete paths are matched before templated ones. + */ + public function discover(): RouteCollection + { + return $this->collection ??= $this->scan(); + } + + private function scan(): RouteCollection + { + $discovered = new RouteCollection(); + + foreach ($this->commandPaths as $path) { + // Configured paths may legitimately be absent, including the default %kernel.project_dir%/src. + if (!is_dir($path)) { + continue; + } + + $routes = $this->directoryLoader->load($path, CommandRouteDirectoryLoader::TYPE); + if ($routes instanceof RouteCollection) { + $discovered->addCollection($routes); + } + } + + return $this->sorter->sort($discovered); + } +} diff --git a/src/Routing/Loader/AttributeDirectoryLoaderDecorator.php b/src/Routing/Loader/AttributeDirectoryLoaderDecorator.php deleted file mode 100644 index fc10560..0000000 --- a/src/Routing/Loader/AttributeDirectoryLoaderDecorator.php +++ /dev/null @@ -1,63 +0,0 @@ -inner->load($resource, $type); - if (!$collection instanceof RouteCollection) { - $collection = new RouteCollection(); - } - - if ($this->augmented) { - return $collection; - } - - $this->augmented = true; - $projectDirectory = rtrim($this->projectDir, '/').'/src'; - $commandDirLoader = new AttributeDirectoryLoader($this->locator, $this->commandAttributeLoader); - - $commands = $commandDirLoader->load($projectDirectory, 'attribute'); - if ($commands instanceof RouteCollection) { - $collection->addCollection((new RouteSpecificitySorter())->sort($commands)); - } - - return $collection; - } - - public function supports(mixed $resource, ?string $type = null): bool - { - return $this->inner->supports($resource, $type); - } -} diff --git a/src/Routing/Loader/CommandRouteClassLoader.php b/src/Routing/Loader/CommandRouteClassLoader.php index 7fb7a0c..a9cd2b6 100644 --- a/src/Routing/Loader/CommandRouteClassLoader.php +++ b/src/Routing/Loader/CommandRouteClassLoader.php @@ -13,6 +13,7 @@ namespace Stixx\OpenApiCommandBundle\Routing\Loader; +use LogicException; use OpenApi\Annotations\Operation; use OpenApi\Attributes as OA; use OpenApi\Generator; @@ -30,6 +31,13 @@ */ final class CommandRouteClassLoader extends AttributeClassLoader { + /** + * Route name => the command class that claimed it, across every class this loader has seen. + * + * @var array + */ + private array $routeOwners = []; + /** * @param array $controllerClasses */ @@ -106,6 +114,7 @@ public function load(mixed $class, ?string $type = null): RouteCollection $name = $this->routeNameFromOperation($operation, $reflectionClass); $finalName = $this->ensureUniqueName($collection, $name); + $this->assertNameIsNotTakenByAnotherCommand($finalName, $class); $collection->add($finalName, $route); } @@ -130,6 +139,26 @@ private function defaultNameFromClass(ReflectionClass $class): string return 'command_'.$base; } + /** + * Route names are unique per class via {@see ensureUniqueName()}, but each class is loaded into its own + * collection and those are merged with addCollection(), which overwrites by name. Two commands deriving + * the same name would silently cost one of them its endpoint, so fail loudly instead. + * + * Loading the same class twice — discovery plus an explicit import — is not a conflict. + * + * @param class-string $class + */ + private function assertNameIsNotTakenByAnotherCommand(string $name, string $class): void + { + $owner = $this->routeOwners[$name] ?? null; + + if ($owner !== null && $owner !== $class) { + throw new LogicException(sprintf('Command classes "%s" and "%s" both produce the route name "%s". Set a distinct operationId on one of their OpenAPI operation attributes.', $owner, $class, $name)); + } + + $this->routeOwners[$name] = $class; + } + private function ensureUniqueName(RouteCollection $collection, string $name): string { if ($collection->get($name) === null) { diff --git a/src/Routing/Loader/RouterLoaderDecorator.php b/src/Routing/Loader/RouterLoaderDecorator.php new file mode 100644 index 0000000..59c1dc2 --- /dev/null +++ b/src/Routing/Loader/RouterLoaderDecorator.php @@ -0,0 +1,94 @@ +inner->load($resource, $type); + if (!$collection instanceof RouteCollection) { + return $collection; + } + + $discovered = $this->discovery->discover(); + + foreach ($discovered->all() as $name => $route) { + // Keep a route the application already declared: re-adding moves it to the end of the + // collection, changing which route matches first. + if ($collection->get($name) !== null) { + continue; + } + + $collection->add($name, $route, $discovered->getPriority($name) ?? 0); + } + + // Without these the router cache never sees command files change, so new or edited commands + // do not show up until the cache is cleared by hand. + foreach ($discovered->getResources() as $resource) { + $collection->addResource($resource); + } + + return $collection; + } + + public function supports(mixed $resource, ?string $type = null): bool + { + return $this->inner->supports($resource, $type); + } + + public function getResolver(): LoaderResolverInterface + { + return $this->inner->getResolver(); + } + + public function setResolver(LoaderResolverInterface $resolver): void + { + $this->inner->setResolver($resolver); + } + + /** + * Nothing in the framework calls import() on `routing.loader`, but it is public on the decorated loader, + * so it is delegated rather than dropped: decorating a service should not shrink its surface. + */ + public function import(mixed $resource, ?string $type = null): mixed + { + return $this->inner->import($resource, $type); + } +} diff --git a/src/Routing/RouteSpecificitySorter.php b/src/Routing/RouteSpecificitySorter.php index 097fe54..a46613f 100644 --- a/src/Routing/RouteSpecificitySorter.php +++ b/src/Routing/RouteSpecificitySorter.php @@ -57,6 +57,12 @@ public function sort(RouteCollection $routes): RouteCollection $sorted->add($name, $all[$name], $routes->getPriority($name) ?? 0); } + // Carry the loader's resources over: they are what invalidates the router cache when a command + // file changes, and building a fresh collection would otherwise drop them. + foreach ($routes->getResources() as $resource) { + $sorted->addResource($resource); + } + return $sorted; } diff --git a/tests/Functional/App/DiscoveryKernel.php b/tests/Functional/App/DiscoveryKernel.php new file mode 100644 index 0000000..cffe42d --- /dev/null +++ b/tests/Functional/App/DiscoveryKernel.php @@ -0,0 +1,52 @@ +addTestBundle(FrameworkBundle::class); + $this->addTestBundle(NelmioApiDocBundle::class); + $this->addTestBundle(StixxOpenApiCommandBundle::class); + $this->addTestConfig(__DIR__.'/../Resources/config/discovery.php'); + } + + public function getProjectDir(): string + { + return __DIR__.'/../'; + } + + /** + * @param RoutingConfigurator $routes + */ + protected function configureRoutes($routes): void + { + parent::configureRoutes($routes); + + // Deliberately imports nothing. + } +} diff --git a/tests/Functional/BundleInitializationTest.php b/tests/Functional/BundleInitializationTest.php index b5a1dc3..e1cedd5 100644 --- a/tests/Functional/BundleInitializationTest.php +++ b/tests/Functional/BundleInitializationTest.php @@ -21,7 +21,8 @@ use Stixx\OpenApiCommandBundle\Responder\{JsonResponder, JsonSerializedResponder, NullableResponder, ResponderChain, ResponderInterface}; use Stixx\OpenApiCommandBundle\Response\{ResponseStatusResolver, StatusResolverInterface}; use Stixx\OpenApiCommandBundle\RouteDescriber\CommandRouteDescriber; -use Stixx\OpenApiCommandBundle\Routing\Loader\{AttributeDirectoryLoaderDecorator, CommandRouteClassLoader}; +use Stixx\OpenApiCommandBundle\Routing\CommandRouteDiscovery; +use Stixx\OpenApiCommandBundle\Routing\Loader\{CommandRouteClassLoader, CommandRouteDirectoryLoader, RouterLoaderDecorator}; use Stixx\OpenApiCommandBundle\Routing\NelmioAreaRoutesChecker; use Stixx\OpenApiCommandBundle\Serializer\Normalizer\{ApiProblemNormalizer, ConstraintViolationListNormalizer, ConstraintViolationNormalizer}; use Stixx\OpenApiCommandBundle\Tests\Functional\App\Kernel; @@ -70,7 +71,9 @@ public function testInitBundle(): void // Routing $this->assertTrue($container->has(NelmioAreaRoutesChecker::class)); $this->assertTrue($container->has(CommandRouteClassLoader::class)); - $this->assertTrue($container->has(AttributeDirectoryLoaderDecorator::class)); + $this->assertTrue($container->has(CommandRouteDirectoryLoader::class)); + $this->assertTrue($container->has(CommandRouteDiscovery::class)); + $this->assertTrue($container->has(RouterLoaderDecorator::class)); // Validators $this->assertTrue($container->has(RequestValidatorChain::class)); diff --git a/tests/Functional/Resources/config/discovery.php b/tests/Functional/Resources/config/discovery.php new file mode 100644 index 0000000..7517071 --- /dev/null +++ b/tests/Functional/Resources/config/discovery.php @@ -0,0 +1,50 @@ +extension('framework', [ + 'test' => true, + 'messenger' => [ + 'enabled' => true, + ], + 'serializer' => [ + 'enabled' => true, + ], + 'validation' => [ + 'enabled' => true, + ], + 'http_method_override' => false, + 'php_errors' => [ + 'log' => false, + ], + ]); + + $container->extension('nelmio_api_doc', [ + 'areas' => [ + 'default' => [ + 'path_patterns' => ['^/api'], + ], + ], + ]); + + $container->extension('stixx_openapi_command', [ + 'validation' => [ + 'enabled' => true, + ], + 'command_paths' => ['%kernel.project_dir%/App/Command'], + ]); + + $container->parameters()->set('validator.translation_domain', 'validators'); +}; diff --git a/tests/Functional/RouteDiscoveryTest.php b/tests/Functional/RouteDiscoveryTest.php new file mode 100644 index 0000000..431a21f --- /dev/null +++ b/tests/Functional/RouteDiscoveryTest.php @@ -0,0 +1,95 @@ +bootDiscoveryKernel(); + + // Act + $router = $kernel->getContainer()->get('router'); + self::assertInstanceOf(RouterInterface::class, $router); + $routes = $router->getRouteCollection(); + + // Assert + $createBook = $routes->get('command_createbookcommand'); + self::assertNotNull($createBook, 'Expected the command route to be discovered'); + self::assertSame('/api/books', $createBook->getPath()); + self::assertSame(['POST'], $createBook->getMethods()); + + self::assertNotNull($routes->get('command_updatebookcommand')); + self::assertNotNull($routes->get('command_deletebookcommand')); + + // The command directory must be among the router's cache resources, or editing a command + // will not invalidate the route cache. + $resources = array_map(strval(...), $routes->getResources()); + self::assertNotEmpty( + array_filter($resources, static fn (string $r): bool => str_contains($r, 'App/Command')), + 'Expected a cache resource covering the scanned command directory' + ); + } + + #[WithoutErrorHandler] + public function testDiscoveredRoutesServeRequests(): void + { + // Arrange + $kernel = $this->bootDiscoveryKernel(); + + $request = Request::create( + uri: '/api/books', + method: 'POST', + content: json_encode(['title' => 'Refactoring', 'author' => 'Martin Fowler'], JSON_THROW_ON_ERROR) + ); + $request->headers->set('Content-Type', 'application/json'); + + // Act + $response = $kernel->handle($request); + + // Assert + self::assertSame(201, $response->getStatusCode()); + $data = json_decode($response->getContent() ?: 'null', true, 512, JSON_THROW_ON_ERROR); + self::assertIsArray($data); + self::assertSame('Refactoring', $data['title'] ?? null); + } + + private function bootDiscoveryKernel(): DiscoveryKernel + { + $kernel = new DiscoveryKernel('test', true); + $kernel->addTestConfig(__DIR__.'/Resources/config/scenario.php'); + $kernel->boot(); + + return $kernel; + } +} diff --git a/tests/Mock/Routing/duplicates/DuplicateAlphaCommand.php b/tests/Mock/Routing/duplicates/DuplicateAlphaCommand.php new file mode 100644 index 0000000..9ee4cee --- /dev/null +++ b/tests/Mock/Routing/duplicates/DuplicateAlphaCommand.php @@ -0,0 +1,21 @@ + ['Default'], ], 'cache_control' => 'no-store', + 'command_paths' => ['%kernel.project_dir%/src'], 'openapi' => [ 'problem_details' => true, ], @@ -42,6 +43,19 @@ public function testDefaultConfig(): void self::assertSame($expected, $config); } + public function testCommandPathsCanBeEmptiedToDisableDiscovery(): void + { + // Arrange + $configuration = new Configuration(); + $processor = new Processor(); + + // Act + $config = $processor->processConfiguration($configuration, [['command_paths' => []]]); + + // Assert + self::assertSame([], $config['command_paths']); + } + public function testCustomConfig(): void { // Arrange @@ -64,6 +78,7 @@ public function testCustomConfig(): void 'groups' => ['Custom', 'Special'], ], 'cache_control' => 'no-store', + 'command_paths' => ['%kernel.project_dir%/src'], 'openapi' => [ 'problem_details' => true, ], diff --git a/tests/Unit/Routing/CommandRouteDiscoveryTest.php b/tests/Unit/Routing/CommandRouteDiscoveryTest.php new file mode 100644 index 0000000..895cf47 --- /dev/null +++ b/tests/Unit/Routing/CommandRouteDiscoveryTest.php @@ -0,0 +1,101 @@ +commandDir = dirname(__DIR__, 2).'/Mock/Routing/src'; + } + + public function testDiscoversCommandsInConfiguredPaths(): void + { + // Act + $names = array_keys($this->createDiscovery([$this->commandDir])->discover()->all()); + + // Assert + self::assertContains('api_test', $names); + } + + 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. + + // Act + $names = array_keys($this->createDiscovery([$this->commandDir])->discover()->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 testDiscoveredRoutesKeepTheirCacheResources(): void + { + // Arrange — resources are what invalidate the router cache when a command file changes. + + // Act + $resources = $this->createDiscovery([$this->commandDir])->discover()->getResources(); + + // Assert + self::assertNotEmpty($resources, 'Sorting must not drop the loader resources'); + } + + public function testNonExistentPathsAreSkipped(): void + { + // Act + $collection = $this->createDiscovery([$this->commandDir.'/does-not-exist'])->discover(); + + // Assert + self::assertCount(0, $collection->all()); + } + + public function testScanHappensOnlyOnce(): void + { + // Arrange + $discovery = $this->createDiscovery([$this->commandDir]); + + // Act + $first = $discovery->discover(); + $second = $discovery->discover(); + + // Assert + self::assertSame($first, $second); + } + + /** + * @param list $paths + */ + private function createDiscovery(array $paths): CommandRouteDiscovery + { + $directoryLoader = new CommandRouteDirectoryLoader( + new FileLocator([$this->commandDir]), + new CommandRouteClassLoader(), + ); + + return new CommandRouteDiscovery($directoryLoader, $paths); + } +} diff --git a/tests/Unit/Routing/Loader/AttributeDirectoryLoaderDecoratorTest.php b/tests/Unit/Routing/Loader/AttributeDirectoryLoaderDecoratorTest.php deleted file mode 100644 index feeac81..0000000 --- a/tests/Unit/Routing/Loader/AttributeDirectoryLoaderDecoratorTest.php +++ /dev/null @@ -1,90 +0,0 @@ -projectDir = dirname(__DIR__, 3).'/Mock/Routing'; - $this->inner = $this->createMock(AttributeDirectoryLoader::class); - } - - public function testLoadAugmentsOnlyOnceByScanningSrc(): void - { - // Arrange - $this->inner->expects(self::exactly(2)) - ->method('load') - ->with($this->anything(), $this->anything()) - ->willReturnOnConsecutiveCalls(new RouteCollection(), new RouteCollection()); - - $locator = new FileLocator([$this->projectDir]); - $commandClassLoader = new CommandRouteClassLoader(); - $decorator = new AttributeDirectoryLoaderDecorator($this->inner, $locator, $commandClassLoader, $this->projectDir); - - // Act & Assert - $first = $decorator->load('ignored'); - $route = array_keys($first->all()); - self::assertContains('api_test', $route, 'Expected route from AnnotatedCommand to be merged'); - - $second = $decorator->load('ignored'); - self::assertCount(0, $second->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 - $this->inner->expects(self::once()) - ->method('supports') - ->with('resource', 'attribute') - ->willReturn(true); - - $locator = new FileLocator([$this->projectDir]); - $decorator = new AttributeDirectoryLoaderDecorator($this->inner, $locator, new CommandRouteClassLoader(), $this->projectDir); - - // Assert - self::assertTrue($decorator->supports('resource', 'attribute')); - } -} diff --git a/tests/Unit/Routing/Loader/CommandRouteClassLoaderTest.php b/tests/Unit/Routing/Loader/CommandRouteClassLoaderTest.php index df50264..c0d2a70 100644 --- a/tests/Unit/Routing/Loader/CommandRouteClassLoaderTest.php +++ b/tests/Unit/Routing/Loader/CommandRouteClassLoaderTest.php @@ -13,10 +13,12 @@ namespace Stixx\OpenApiCommandBundle\Tests\Unit\Routing\Loader; +use LogicException; use PHPUnit\Framework\TestCase; use Stixx\OpenApiCommandBundle\Attribute\CommandObject; use Stixx\OpenApiCommandBundle\Controller\CommandController; use Stixx\OpenApiCommandBundle\Routing\Loader\CommandRouteClassLoader; +use Stixx\OpenApiCommandBundle\Tests\Mock\Routing\duplicates\{DuplicateAlphaCommand, DuplicateBetaCommand}; /** * Tests for CommandRouteClassLoader. @@ -243,6 +245,34 @@ final class WithOperationId {} self::assertSame(['my_op'], $names); } + public function testTwoCommandsClaimingTheSameRouteNameFail(): void + { + // Arrange — per-class collections are merged with addCollection(), which would otherwise let one + // command silently overwrite the other's endpoint. + $loader = new CommandRouteClassLoader(); + $loader->load(DuplicateAlphaCommand::class); + + // Assert + $this->expectException(LogicException::class); + $this->expectExceptionMessage('both produce the route name "duplicate_operation"'); + + // Act + $loader->load(DuplicateBetaCommand::class); + } + + public function testLoadingTheSameCommandTwiceIsNotAConflict(): void + { + // Arrange — discovery and an explicit route import can both reach the same class. + $loader = new CommandRouteClassLoader(); + $loader->load(DuplicateAlphaCommand::class); + + // Act + $collection = $loader->load(DuplicateAlphaCommand::class); + + // Assert + self::assertSame(['duplicate_operation'], array_keys($collection->all())); + } + private static function classNamespace(string $short): string { return __NAMESPACE__.'\\'.$short; diff --git a/tests/Unit/Routing/Loader/RouterLoaderDecoratorTest.php b/tests/Unit/Routing/Loader/RouterLoaderDecoratorTest.php new file mode 100644 index 0000000..286353a --- /dev/null +++ b/tests/Unit/Routing/Loader/RouterLoaderDecoratorTest.php @@ -0,0 +1,119 @@ +commandDir = dirname(__DIR__, 3).'/Mock/Routing/src'; + $this->inner = $this->createMock(Loader::class); + } + + public function testCommandRoutesAreAddedToTheApplicationCollection(): void + { + // Arrange — an application collection that knows nothing about commands. + $appRoutes = new RouteCollection(); + $appRoutes->add('app_home', new Route('/')); + $this->inner->method('load')->willReturn($appRoutes); + + // Act + $collection = $this->createDecorator()->load('routing.yaml'); + + // Assert + self::assertInstanceOf(RouteCollection::class, $collection); + $names = array_keys($collection->all()); + self::assertContains('app_home', $names); + self::assertContains('api_test', $names, 'Expected discovered command routes to be added'); + } + + public function testRoutesTheApplicationAlreadyDeclaredArePreserved(): void + { + // Arrange — the application imported a command explicitly, so the name is already taken. + $explicit = new Route('/explicitly/imported'); + $appRoutes = new RouteCollection(); + $appRoutes->add('api_test', $explicit); + $this->inner->method('load')->willReturn($appRoutes); + + // Act + $collection = $this->createDecorator()->load('routing.yaml'); + + // Assert + self::assertInstanceOf(RouteCollection::class, $collection); + self::assertSame($explicit, $collection->get('api_test')); + } + + public function testCacheResourcesAreAddedToTheApplicationCollection(): void + { + // Arrange + $this->inner->method('load')->willReturn(new RouteCollection()); + + // Act + $collection = $this->createDecorator()->load('routing.yaml'); + + // Assert — without these the router cache never notices a command file changing. + self::assertInstanceOf(RouteCollection::class, $collection); + self::assertNotEmpty($collection->getResources()); + } + + public function testNonRouteCollectionResultsPassThroughUntouched(): void + { + // Arrange + $this->inner->method('load')->willReturn(null); + + // Act & Assert + self::assertNull($this->createDecorator()->load('routing.yaml')); + } + + public function testSupportsDelegatesToInner(): void + { + // Arrange + $this->inner->expects(self::once()) + ->method('supports') + ->with('resource', 'attribute') + ->willReturn(true); + + // Assert + self::assertTrue($this->createDecorator()->supports('resource', 'attribute')); + } + + private function createDecorator(): RouterLoaderDecorator + { + $directoryLoader = new CommandRouteDirectoryLoader( + new FileLocator([$this->commandDir]), + new CommandRouteClassLoader(), + ); + + return new RouterLoaderDecorator( + $this->inner, + new CommandRouteDiscovery($directoryLoader, [$this->commandDir]), + ); + } +} diff --git a/tests/Unit/Routing/RouteSpecificitySorterTest.php b/tests/Unit/Routing/RouteSpecificitySorterTest.php index 259c651..a4faafd 100644 --- a/tests/Unit/Routing/RouteSpecificitySorterTest.php +++ b/tests/Unit/Routing/RouteSpecificitySorterTest.php @@ -15,6 +15,7 @@ use PHPUnit\Framework\TestCase; use Stixx\OpenApiCommandBundle\Routing\RouteSpecificitySorter; +use Symfony\Component\Config\Resource\FileResource; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; @@ -91,4 +92,18 @@ public function testItPreservesExistingRoutePriorities(): void self::assertSame(10, $sorted->getPriority('high')); self::assertNull($sorted->getPriority('low')); } + + public function testItPreservesCacheResources(): void + { + // Arrange — resources drive router cache invalidation, so the rebuild must not drop them. + $routes = new RouteCollection(); + $routes->add('alpha', new Route('/api/books')); + $routes->addResource(new FileResource(__FILE__)); + + // Act + $sorted = (new RouteSpecificitySorter())->sort($routes); + + // Assert + self::assertEquals([new FileResource(__FILE__)], $sorted->getResources()); + } }