From 46bcb9b1b4224e996341e937d9601ad457eb804e Mon Sep 17 00:00:00 2001 From: Jelle van Oosterbosch Date: Sun, 30 Aug 2026 17:28:05 +0200 Subject: [PATCH 1/4] Register command routes independently of the application's routing style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Command route discovery hung off a decorator on `routing.loader.attribute.directory`, so it only ran when the application happened to load routes through that loader. The current Symfony skeleton's config/routes.yaml sets a `namespace`, which routes through Psr4DirectoryLoader instead, so the decorator was never invoked and command routes silently did not exist — no error, just 404s. Decorate `routing.loader` instead. That is the loader the router resolves to build its collection, and it is called exactly once for the root routing resource, so discovery runs regardless of how (or whether) the application declares its own routes. DelegatingLoader is not itself tagged `routing.loader`, so it is absent from the resolver nested imports go through and the decoration cannot recurse. Discovery moves out of the decorator into CommandRouteDiscovery, which scans the new `command_paths` setting (default `%kernel.project_dir%/src`, matching the previous hardcoded path) and skips paths that do not exist. Setting it to [] disables discovery for applications that prefer to import commands explicitly. CommandRouteDirectoryLoader existed and was unit-tested but was never registered as a service; it is now the directory-scanning loader behind discovery and is registered so `type: stixx_openapi_command.command_attributes` works from an application's routing config. The functional kernel imports every command explicitly, so it exercised the explicit-import path and never covered discovery. RouteDiscoveryTest adds a kernel that declares no routes at all; both of its tests fail against the old decoration target. --- README.md | 2 + config/routing.php | 40 +++++-- docs/command-routing.md | 44 +++++++- src/DependencyInjection/Configuration.php | 5 + .../StixxOpenApiCommandExtension.php | 4 + src/Routing/CommandRouteDiscovery.php | 65 +++++++++++ .../AttributeDirectoryLoaderDecorator.php | 63 ----------- src/Routing/Loader/RouterLoaderDecorator.php | 94 ++++++++++++++++ tests/Functional/App/DiscoveryKernel.php | 55 +++++++++ tests/Functional/BundleInitializationTest.php | 7 +- .../Functional/Resources/config/discovery.php | 51 +++++++++ tests/Functional/RouteDiscoveryTest.php | 90 +++++++++++++++ .../DependencyInjection/ConfigurationTest.php | 15 +++ .../Routing/CommandRouteDiscoveryTest.php | 90 +++++++++++++++ .../AttributeDirectoryLoaderDecoratorTest.php | 90 --------------- .../Loader/RouterLoaderDecoratorTest.php | 106 ++++++++++++++++++ 16 files changed, 651 insertions(+), 170 deletions(-) create mode 100644 src/Routing/CommandRouteDiscovery.php delete mode 100644 src/Routing/Loader/AttributeDirectoryLoaderDecorator.php create mode 100644 src/Routing/Loader/RouterLoaderDecorator.php create mode 100644 tests/Functional/App/DiscoveryKernel.php create mode 100644 tests/Functional/Resources/config/discovery.php create mode 100644 tests/Functional/RouteDiscoveryTest.php create mode 100644 tests/Unit/Routing/CommandRouteDiscoveryTest.php delete mode 100644 tests/Unit/Routing/Loader/AttributeDirectoryLoaderDecoratorTest.php create mode 100644 tests/Unit/Routing/Loader/RouterLoaderDecoratorTest.php diff --git a/README.md b/README.md index 68f9cb8..52a68ba 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,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..667255b 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,36 @@ ->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')) + // Keeps `$routes->import(SomeCommand::class, 'attribute')` working for applications that prefer + // to declare command routes explicitly instead of relying on discovery. + ->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)) + // Allows `$routes->import('../src/Command', 'stixx_openapi_command.command_attributes')` for + // applications that want to control which directories are scanned from their routing config. + ->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)); + + // Decorates `routing.loader` (the DelegatingLoader the router asks for) rather than one of the loaders + // behind it, so command routes are added regardless of how the application declares its own routes. + $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..43a078b 100644 --- a/docs/command-routing.md +++ b/docs/command-routing.md @@ -119,16 +119,48 @@ 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. + +### 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..b8be953 --- /dev/null +++ b/src/Routing/CommandRouteDiscovery.php @@ -0,0 +1,65 @@ + $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) { + // A configured path is allowed not to exist: a project may keep commands in only some of them, and + // the default `%kernel.project_dir%/src` is absent in a few project layouts. + 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/RouterLoaderDecorator.php b/src/Routing/Loader/RouterLoaderDecorator.php new file mode 100644 index 0000000..d59ce4e --- /dev/null +++ b/src/Routing/Loader/RouterLoaderDecorator.php @@ -0,0 +1,94 @@ +inner->load($resource, $type); + if (!$collection instanceof RouteCollection) { + return $collection; + } + + foreach ($this->discovery->discover()->all() as $name => $route) { + // An application may already have imported a command explicitly. Keep its route: re-adding would + // move the route to the end of the collection and change which one matches first. + if ($collection->get($name) !== null) { + continue; + } + + $collection->add($name, $route); + } + + 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); + } + + public function import(mixed $resource, ?string $type = null): mixed + { + return $this->inner->import($resource, $type); + } +} diff --git a/tests/Functional/App/DiscoveryKernel.php b/tests/Functional/App/DiscoveryKernel.php new file mode 100644 index 0000000..a6f68c8 --- /dev/null +++ b/tests/Functional/App/DiscoveryKernel.php @@ -0,0 +1,55 @@ +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 empty: no command is imported here. + } +} 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..0f43c63 --- /dev/null +++ b/tests/Functional/Resources/config/discovery.php @@ -0,0 +1,51 @@ +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, + ], + // The test application keeps its commands in App/Command rather than the default src/. + '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..d082f50 --- /dev/null +++ b/tests/Functional/RouteDiscoveryTest.php @@ -0,0 +1,90 @@ +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')); + } + + #[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/Unit/DependencyInjection/ConfigurationTest.php b/tests/Unit/DependencyInjection/ConfigurationTest.php index 48dfc38..c8c0425 100644 --- a/tests/Unit/DependencyInjection/ConfigurationTest.php +++ b/tests/Unit/DependencyInjection/ConfigurationTest.php @@ -35,6 +35,7 @@ public function testDefaultConfig(): void 'groups' => ['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..549f4fe --- /dev/null +++ b/tests/Unit/Routing/CommandRouteDiscoveryTest.php @@ -0,0 +1,90 @@ +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 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/RouterLoaderDecoratorTest.php b/tests/Unit/Routing/Loader/RouterLoaderDecoratorTest.php new file mode 100644 index 0000000..b78accf --- /dev/null +++ b/tests/Unit/Routing/Loader/RouterLoaderDecoratorTest.php @@ -0,0 +1,106 @@ +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 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]), + ); + } +} From 8485412e99de821dc967caa8650467584d3521fc Mon Sep 17 00:00:00 2001 From: Jelle van Oosterbosch Date: Sun, 30 Aug 2026 17:36:19 +0200 Subject: [PATCH 2/4] Trim comments to match the project style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep the reasoning that is not recoverable from the code — why routing.loader is the decoration point, why $inner is typed as Loader — and drop the rest. --- config/routing.php | 8 ++--- src/Routing/CommandRouteDiscovery.php | 3 +- src/Routing/Loader/RouterLoaderDecorator.php | 29 +++++-------------- tests/Functional/App/DiscoveryKernel.php | 9 ++---- .../Functional/Resources/config/discovery.php | 1 - tests/Functional/RouteDiscoveryTest.php | 9 ++---- 6 files changed, 17 insertions(+), 42 deletions(-) diff --git a/config/routing.php b/config/routing.php index 667255b..98346dc 100644 --- a/config/routing.php +++ b/config/routing.php @@ -29,16 +29,14 @@ ->set(CommandRouteClassLoader::class) ->arg('$env', param('kernel.environment')) ->arg('$controllerClasses', param('stixx_openapi_command.controller_classes')) - // Keeps `$routes->import(SomeCommand::class, 'attribute')` working for applications that prefer - // to declare command routes explicitly instead of relying on discovery. + // Supports $routes->import(SomeCommand::class, 'attribute'). ->tag('routing.loader'); $services ->set(CommandRouteDirectoryLoader::class) ->arg('$locator', service('file_locator')) ->arg('$loader', service(CommandRouteClassLoader::class)) - // Allows `$routes->import('../src/Command', 'stixx_openapi_command.command_attributes')` for - // applications that want to control which directories are scanned from their routing config. + // Supports $routes->import('../src/Command', 'stixx_openapi_command.command_attributes'). ->tag('routing.loader'); $services @@ -47,8 +45,6 @@ ->arg('$commandPaths', param('stixx_openapi_command.command_paths')) ->arg('$sorter', service(RouteSpecificitySorter::class)); - // Decorates `routing.loader` (the DelegatingLoader the router asks for) rather than one of the loaders - // behind it, so command routes are added regardless of how the application declares its own routes. $services ->set(RouterLoaderDecorator::class) ->decorate('routing.loader') diff --git a/src/Routing/CommandRouteDiscovery.php b/src/Routing/CommandRouteDiscovery.php index b8be953..57a3d56 100644 --- a/src/Routing/CommandRouteDiscovery.php +++ b/src/Routing/CommandRouteDiscovery.php @@ -48,8 +48,7 @@ private function scan(): RouteCollection $discovered = new RouteCollection(); foreach ($this->commandPaths as $path) { - // A configured path is allowed not to exist: a project may keep commands in only some of them, and - // the default `%kernel.project_dir%/src` is absent in a few project layouts. + // Configured paths may legitimately be absent, including the default %kernel.project_dir%/src. if (!is_dir($path)) { continue; } diff --git a/src/Routing/Loader/RouterLoaderDecorator.php b/src/Routing/Loader/RouterLoaderDecorator.php index d59ce4e..cc4bfd9 100644 --- a/src/Routing/Loader/RouterLoaderDecorator.php +++ b/src/Routing/Loader/RouterLoaderDecorator.php @@ -22,30 +22,17 @@ /** * Adds the discovered command routes to the application's route collection. * - * @see CommandRouteDiscovery - * - * This decorates `routing.loader` (FrameworkBundle's `DelegatingLoader`) rather than one of the individual - * loaders behind it. The router resolves `routing.loader` to build its collection and calls it exactly once, - * for the root routing resource, so command routes are registered no matter how the application declares its - * own routes — or whether it declares any at all. - * - * Decorating a specific loader instead makes discovery conditional on the application happening to use it. The - * previous implementation decorated `routing.loader.attribute.directory`, which the current Symfony skeleton - * never invokes: its `config/routes.yaml` sets a `namespace`, so routes load through `Psr4DirectoryLoader`, and - * command routes silently disappeared. - * - * `DelegatingLoader` is not itself tagged `routing.loader`, so it is absent from the resolver that nested - * imports go through. Decorating it therefore cannot recurse. + * Decorates `routing.loader`, which the router calls exactly once for the root routing resource, so discovery + * runs whichever loader the application's own routes happen to use. That service is FrameworkBundle's + * DelegatingLoader, which is not itself tagged `routing.loader`, so the decoration cannot recurse through + * nested imports. * * @internal */ final class RouterLoaderDecorator implements LoaderInterface { - /** - * `$inner` is typed as the abstract Loader rather than LoaderInterface because MicroKernelTrait hands the - * decorated service to `configureRoutes()`, and RoutingConfigurator::import() calls import() on it — a - * method Loader declares but LoaderInterface does not. - */ + // $inner is the abstract Loader, not LoaderInterface: MicroKernelTrait passes this service to + // configureRoutes(), where RoutingConfigurator::import() calls import() — declared only on Loader. public function __construct( private readonly Loader $inner, private readonly CommandRouteDiscovery $discovery, @@ -60,8 +47,8 @@ public function load(mixed $resource, ?string $type = null): mixed } foreach ($this->discovery->discover()->all() as $name => $route) { - // An application may already have imported a command explicitly. Keep its route: re-adding would - // move the route to the end of the collection and change which one matches first. + // 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; } diff --git a/tests/Functional/App/DiscoveryKernel.php b/tests/Functional/App/DiscoveryKernel.php index a6f68c8..cffe42d 100644 --- a/tests/Functional/App/DiscoveryKernel.php +++ b/tests/Functional/App/DiscoveryKernel.php @@ -20,11 +20,8 @@ use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator; /** - * A kernel that declares no routes of its own. - * - * {@see Kernel} imports every command explicitly, which means it exercises the explicit-import path rather - * than discovery. This kernel covers the case an application actually hits: command routes have to appear - * without the application importing anything. + * A kernel that declares no routes of its own, unlike {@see Kernel}, which imports every command explicitly + * and so only covers the explicit-import path. */ class DiscoveryKernel extends TestKernel { @@ -50,6 +47,6 @@ protected function configureRoutes($routes): void { parent::configureRoutes($routes); - // Deliberately empty: no command is imported here. + // Deliberately imports nothing. } } diff --git a/tests/Functional/Resources/config/discovery.php b/tests/Functional/Resources/config/discovery.php index 0f43c63..7517071 100644 --- a/tests/Functional/Resources/config/discovery.php +++ b/tests/Functional/Resources/config/discovery.php @@ -43,7 +43,6 @@ 'validation' => [ 'enabled' => true, ], - // The test application keeps its commands in App/Command rather than the default src/. 'command_paths' => ['%kernel.project_dir%/App/Command'], ]); diff --git a/tests/Functional/RouteDiscoveryTest.php b/tests/Functional/RouteDiscoveryTest.php index d082f50..bf91bf6 100644 --- a/tests/Functional/RouteDiscoveryTest.php +++ b/tests/Functional/RouteDiscoveryTest.php @@ -19,12 +19,9 @@ use Symfony\Component\Routing\RouterInterface; /** - * Command routes must be registered without the application importing anything. - * - * Route discovery used to hang off a decorator on `routing.loader.attribute.directory`, so it only ran when - * the application happened to load routes through that loader. An application using the current Symfony - * skeleton's `config/routes.yaml` — which sets a `namespace` and therefore loads via `Psr4DirectoryLoader` — - * got no command routes at all, with no error to explain why. + * Command routes must be registered without the application importing anything. Discovery used to hang off a + * decorator on `routing.loader.attribute.directory`, so it never ran for applications whose routes load + * through another loader — the skeleton's `namespace` key routes through Psr4DirectoryLoader. */ final class RouteDiscoveryTest extends AbstractKernelTestCase { From 9207b65295b4655b6ee53d3261d0ee21c6b5bb7f Mon Sep 17 00:00:00 2001 From: Jelle van Oosterbosch Date: Sun, 30 Aug 2026 17:43:17 +0200 Subject: [PATCH 3/4] Keep cache resources on discovered command routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RouteSpecificitySorter rebuilt the collection without copying resources, and the decorator added routes without them, so the router cache had nothing covering the scanned command directories. In debug mode a new or edited command DTO did not invalidate the route cache and stayed invisible until a manual cache:clear. The previous decorator lost resources the same way, but it only ran while the application's own AttributeDirectoryLoader was scanning src/, whose collection carried a resource for those files. Nothing scans the command paths now, so the resource has to come from discovery itself. Also carry route priorities through the decorator's copy, and correct the note on the $inner type: MicroKernelTrait calls getResolver() on this service, not import() — RoutingConfigurator gets a RoutingPhpFileLoader, not the decorator. --- src/Routing/Loader/RouterLoaderDecorator.php | 17 +++++++++++++---- src/Routing/RouteSpecificitySorter.php | 6 ++++++ tests/Functional/RouteDiscoveryTest.php | 8 ++++++++ .../Unit/Routing/CommandRouteDiscoveryTest.php | 11 +++++++++++ .../Loader/RouterLoaderDecoratorTest.php | 13 +++++++++++++ .../Unit/Routing/RouteSpecificitySorterTest.php | 15 +++++++++++++++ 6 files changed, 66 insertions(+), 4 deletions(-) diff --git a/src/Routing/Loader/RouterLoaderDecorator.php b/src/Routing/Loader/RouterLoaderDecorator.php index cc4bfd9..725ed00 100644 --- a/src/Routing/Loader/RouterLoaderDecorator.php +++ b/src/Routing/Loader/RouterLoaderDecorator.php @@ -31,8 +31,9 @@ */ final class RouterLoaderDecorator implements LoaderInterface { - // $inner is the abstract Loader, not LoaderInterface: MicroKernelTrait passes this service to - // configureRoutes(), where RoutingConfigurator::import() calls import() — declared only on Loader. + // getResolver() is load-bearing: MicroKernelTrait::loadRoutes() calls it on this service to resolve the + // kernel's own loader. $inner is the abstract Loader so import() can be delegated too, which the + // interface does not declare. public function __construct( private readonly Loader $inner, private readonly CommandRouteDiscovery $discovery, @@ -46,14 +47,22 @@ public function load(mixed $resource, ?string $type = null): mixed return $collection; } - foreach ($this->discovery->discover()->all() as $name => $route) { + $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); + $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; 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/RouteDiscoveryTest.php b/tests/Functional/RouteDiscoveryTest.php index bf91bf6..431a21f 100644 --- a/tests/Functional/RouteDiscoveryTest.php +++ b/tests/Functional/RouteDiscoveryTest.php @@ -51,6 +51,14 @@ public function testCommandRoutesAreRegisteredWithoutAnyRouteImports(): void 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] diff --git a/tests/Unit/Routing/CommandRouteDiscoveryTest.php b/tests/Unit/Routing/CommandRouteDiscoveryTest.php index 549f4fe..895cf47 100644 --- a/tests/Unit/Routing/CommandRouteDiscoveryTest.php +++ b/tests/Unit/Routing/CommandRouteDiscoveryTest.php @@ -53,6 +53,17 @@ public function testCommandRoutesAreOrderedMostSpecificFirst(): void 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 diff --git a/tests/Unit/Routing/Loader/RouterLoaderDecoratorTest.php b/tests/Unit/Routing/Loader/RouterLoaderDecoratorTest.php index b78accf..286353a 100644 --- a/tests/Unit/Routing/Loader/RouterLoaderDecoratorTest.php +++ b/tests/Unit/Routing/Loader/RouterLoaderDecoratorTest.php @@ -70,6 +70,19 @@ public function testRoutesTheApplicationAlreadyDeclaredArePreserved(): void 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 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()); + } } From 97e34c111206ba2a76f86fb655546577e6d19a0b Mon Sep 17 00:00:00 2001 From: Jelle van Oosterbosch Date: Sun, 30 Aug 2026 17:48:11 +0200 Subject: [PATCH 4/4] Fail when two commands claim the same route name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each command class is loaded into its own collection and those are merged with addCollection(), which overwrites by name. Two commands resolving to the same route name therefore cost one of them its endpoint, silently — the same class of failure this branch set out to remove. Without an operationId the name derives from the class short name, so Billing\CreateInvoiceCommand and Sales\CreateInvoiceCommand both resolve to command_createinvoicecommand. Track the owning class per route name in the class loader and throw, naming both classes. Loading the same class twice, via discovery and an explicit import, stays valid. Detection lives in the class loader rather than in discovery because AttributeDirectoryLoader already merges per-class collections, so a collision within one directory is gone before discovery sees it. Also record why import() is delegated rather than dropped. --- docs/command-routing.md | 1 + .../Loader/CommandRouteClassLoader.php | 29 ++++++++++++++++++ src/Routing/Loader/RouterLoaderDecorator.php | 4 +++ .../duplicates/DuplicateAlphaCommand.php | 21 +++++++++++++ .../duplicates/DuplicateBetaCommand.php | 21 +++++++++++++ .../Loader/CommandRouteClassLoaderTest.php | 30 +++++++++++++++++++ 6 files changed, 106 insertions(+) create mode 100644 tests/Mock/Routing/duplicates/DuplicateAlphaCommand.php create mode 100644 tests/Mock/Routing/duplicates/DuplicateBetaCommand.php diff --git a/docs/command-routing.md b/docs/command-routing.md index 43a078b..8baf29f 100644 --- a/docs/command-routing.md +++ b/docs/command-routing.md @@ -138,6 +138,7 @@ Notes - 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 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 index 725ed00..59c1dc2 100644 --- a/src/Routing/Loader/RouterLoaderDecorator.php +++ b/src/Routing/Loader/RouterLoaderDecorator.php @@ -83,6 +83,10 @@ 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/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 @@ +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;