From afb27108526f34b8feab14b088c32665d79b25de Mon Sep 17 00:00:00 2001 From: Rodrigo Mammano Date: Thu, 28 May 2020 12:23:02 +0200 Subject: [PATCH 1/4] No longer allow registering multiple applications In order to simplify the container generation process, let's stop allowing the registration of multiple apps in a container and stick to a single one. A side effect of these changes is that we can statically create the XML files for the services created in the compiler passes. We looked into it but we couldn't do it in a time-efficient manner. --- src/Routing/Expressive/RegisterServices.php | 12 ++++++++- .../Expressive/RegisterServicesTest.php | 26 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/Routing/Expressive/RegisterServices.php b/src/Routing/Expressive/RegisterServices.php index 9d49bd15..9bdb484e 100644 --- a/src/Routing/Expressive/RegisterServices.php +++ b/src/Routing/Expressive/RegisterServices.php @@ -9,6 +9,7 @@ use Chimera\ExecuteQuery; use Chimera\IdentifierGenerator; use Chimera\MessageCreator; +use Chimera\Routing\Application as ApplicationInterface; use Chimera\Routing\Expressive\Application; use Chimera\Routing\Expressive\UriGenerator; use Chimera\Routing\Handler\CreateAndFetch; @@ -18,16 +19,17 @@ use Chimera\Routing\Handler\FetchOnly; use Chimera\Routing\MissingRouteDispatching; use Chimera\Routing\RouteParamsExtraction; +use InvalidArgumentException; use Fig\Http\Message\StatusCodeInterface as StatusCode; use Lcobucci\ContentNegotiation\ContentTypeMiddleware; use Lcobucci\ContentNegotiation\Formatter\Json; use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\StreamFactoryInterface; +use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; -use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Reference; use Zend\Diactoros\ServerRequestFactory; use Zend\Expressive\Application as Expressive; @@ -418,6 +420,14 @@ private function registerApplication( $app->setPublic(true); $container->setDefinition($this->applicationName . '.http', $app); + + // --- alias application + + if ($container->hasAlias(ApplicationInterface::class)) { + throw new InvalidArgumentException('There can only be one application registered.'); + } + + $container->setAlias(ApplicationInterface::class, new Alias($this->applicationName . '.http', true)); } private function generateReadAction(string $name, string $query, ContainerBuilder $container): Reference diff --git a/tests/Unit/Routing/Expressive/RegisterServicesTest.php b/tests/Unit/Routing/Expressive/RegisterServicesTest.php index 9eadccd0..a2c81c82 100644 --- a/tests/Unit/Routing/Expressive/RegisterServicesTest.php +++ b/tests/Unit/Routing/Expressive/RegisterServicesTest.php @@ -40,4 +40,30 @@ public function exceptionShouldBeRaisedWhenTryingToRegisterDuplicatedRoutes(): v ); $registerServices->process($builder); } + + /** + * @test + * + * @covers ::__construct + * @covers ::process + * @covers ::extractRoutes + */ + public function registeringServicesDoesNotAllowMultipleApplications(): void + { + $container = new ContainerBuilder(); + + $this->createRegisterServices('testing1')->process($container); + + $this->expectException(InvalidArgumentException::class); + $this->createRegisterServices('testing2')->process($container); + } + + private function createRegisterServices(string $applicationName): RegisterServices + { + return new RegisterServices( + $applicationName, + $applicationName . '.command_bus', + $applicationName . '.query_bus' + ); + } } From dfcc2ebf63a3a63a3463a86d19b169d02eeee8c8 Mon Sep 17 00:00:00 2001 From: Rafael Dohms Date: Fri, 19 Jun 2020 23:54:29 +0200 Subject: [PATCH 2/4] Replace service Ids All service Ids that use ApplicationName are now replaced with the proper class or interface, and an alias is set to the previous name. --- config/routing-expressive.xml | 42 ++++ src/Routing/Expressive/RegisterServices.php | 202 ++++++++---------- .../Expressive/RegisterServicesTest.php | 8 +- 3 files changed, 137 insertions(+), 115 deletions(-) diff --git a/config/routing-expressive.xml b/config/routing-expressive.xml index 5c68c260..84c53492 100644 --- a/config/routing-expressive.xml +++ b/config/routing-expressive.xml @@ -56,6 +56,46 @@ + + + + + + + + + + + + + createStream + + + + + + + + + + fromGlobals + + + + + + + + + + + + + + + + + @@ -63,5 +103,7 @@ + + diff --git a/src/Routing/Expressive/RegisterServices.php b/src/Routing/Expressive/RegisterServices.php index 9bdb484e..54fd5411 100644 --- a/src/Routing/Expressive/RegisterServices.php +++ b/src/Routing/Expressive/RegisterServices.php @@ -11,7 +11,6 @@ use Chimera\MessageCreator; use Chimera\Routing\Application as ApplicationInterface; use Chimera\Routing\Expressive\Application; -use Chimera\Routing\Expressive\UriGenerator; use Chimera\Routing\Handler\CreateAndFetch; use Chimera\Routing\Handler\CreateOnly; use Chimera\Routing\Handler\ExecuteAndFetch; @@ -19,25 +18,23 @@ use Chimera\Routing\Handler\FetchOnly; use Chimera\Routing\MissingRouteDispatching; use Chimera\Routing\RouteParamsExtraction; -use InvalidArgumentException; +use Chimera\Routing\UriGenerator as UriGeneratorInterface; use Fig\Http\Message\StatusCodeInterface as StatusCode; +use InvalidArgumentException; use Lcobucci\ContentNegotiation\ContentTypeMiddleware; use Lcobucci\ContentNegotiation\Formatter\Json; use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\StreamFactoryInterface; -use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; -use Zend\Diactoros\ServerRequestFactory; use Zend\Expressive\Application as Expressive; use Zend\Expressive\Helper\BodyParams\BodyParamsMiddleware; use Zend\Expressive\Middleware\LazyLoadingMiddleware; use Zend\Expressive\MiddlewareContainer; use Zend\Expressive\MiddlewareFactory; -use Zend\Expressive\Response\ServerRequestErrorResponseGenerator; use Zend\Expressive\Router\FastRouteRouter; use Zend\Expressive\Router\Middleware\DispatchMiddleware; use Zend\Expressive\Router\Middleware\ImplicitHeadMiddleware; @@ -45,7 +42,7 @@ use Zend\Expressive\Router\Middleware\MethodNotAllowedMiddleware; use Zend\Expressive\Router\Middleware\RouteMiddleware; use Zend\Expressive\Router\RouteCollector; -use Zend\HttpHandlerRunner\Emitter\EmitterInterface; +use Zend\Expressive\Router\RouterInterface; use Zend\HttpHandlerRunner\RequestHandlerRunner; use Zend\Stratigility\Middleware\PathMiddlewareDecorator; use Zend\Stratigility\MiddlewarePipe; @@ -97,13 +94,13 @@ public function process(ContainerBuilder $container): void $this->registerApplication( $container, - $routes[$this->applicationName] ?? [], - $this->prioritiseMiddleware($middlewareList[$this->applicationName] ?? []) + $routes ?? [], + $this->prioritiseMiddleware($middlewareList ?? []) ); } /** - * @return string[][][] + * @return string[][] * * @throws InvalidArgumentException */ @@ -137,12 +134,10 @@ private function extractRoutes(ContainerBuilder $container): array $tag['methods'] = explode(',', $tag['methods']); } - $tag['app'] ??= $this->applicationName; $tag['async'] = (bool) ($tag['async'] ?? false); $tag['serviceId'] = $serviceId; - $routes[$tag['app']] ??= []; - $routes[$tag['app']][] = $tag; + $routes[] = $tag; $names[$tag['route_name']] = $serviceId; } @@ -165,31 +160,26 @@ private function extractMiddlewareList(ContainerBuilder $container): array $priority = $tag['priority'] ?? 0; $path = $tag['path'] ?? '/'; - $tag['app'] ??= $this->applicationName; - - $list[$tag['app']][$priority][$path] ??= []; - $list[$tag['app']][$priority][$path][] = $serviceId; + $list[$priority][$path] ??= []; + $list[$priority][$path][] = $serviceId; } } - $list[$this->applicationName][Priorities::CONTENT_NEGOTIATION]['/'] ??= []; - $list[$this->applicationName][Priorities::BEFORE_CUSTOM]['/'] ??= []; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'] ??= []; + $list[Priorities::CONTENT_NEGOTIATION]['/'] ??= []; + $list[Priorities::BEFORE_CUSTOM]['/'] ??= []; + $list[Priorities::AFTER_CUSTOM]['/'] ??= []; - $list[$this->applicationName][Priorities::CONTENT_NEGOTIATION]['/'][] = $this->applicationName - . '.http.middleware.content_negotiation'; + $list[Priorities::CONTENT_NEGOTIATION]['/'][] = ContentTypeMiddleware::class; - $list[$this->applicationName][Priorities::BEFORE_CUSTOM]['/'][] = $this->applicationName - . '.http.middleware.route'; - $list[$this->applicationName][Priorities::BEFORE_CUSTOM]['/'][] = BodyParamsMiddleware::class; + $list[Priorities::BEFORE_CUSTOM]['/'][] = RouteMiddleware::class; + $list[Priorities::BEFORE_CUSTOM]['/'][] = BodyParamsMiddleware::class; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = $this->applicationName - . '.http.middleware.implicit_head'; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = ImplicitOptionsMiddleware::class; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = MethodNotAllowedMiddleware::class; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = RouteParamsExtraction::class; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = DispatchMiddleware::class; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = MissingRouteDispatching::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = ImplicitHeadMiddleware::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = ImplicitOptionsMiddleware::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = MethodNotAllowedMiddleware::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = RouteParamsExtraction::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = DispatchMiddleware::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = MissingRouteDispatching::class; return $list; } @@ -247,15 +237,23 @@ private function registerApplication( array $routes, array $middlewareList ): void { + if ($container->hasDefinition(ApplicationInterface::class)) { + throw new InvalidArgumentException('Registering multiple applications is deprecated.'); + } + $services = []; + $aliases = []; // for BC foreach ($routes as $route) { // @phpstan-ignore-next-line $services[] = $this->{self::BEHAVIORS[$route['behavior']]['callback']}( - $this->applicationName . '.http.route.' . $route['route_name'], + 'http.route.' . $route['route_name'], $route, $container ); + + $aliases[$this->applicationName . '.http.route.' . $route['route_name']] + = 'http.route.' . $route['route_name']; } $middleware = []; @@ -284,16 +282,12 @@ private function registerApplication( // -- middleware container $middlewareContainer = $this->createService(MiddlewareContainer::class, [$locator]); - $container->setDefinition($this->applicationName . '.http.middleware_container', $middlewareContainer); + $container->setDefinition(MiddlewareContainer::class, $middlewareContainer); + $aliases[$this->applicationName . '.http.middleware_container'] = MiddlewareContainer::class; // -- middleware factory - $middlewareFactory = $this->createService( - MiddlewareFactory::class, - [new Reference($this->applicationName . '.http.middleware_container')] - ); - - $container->setDefinition($this->applicationName . '.http.middleware_factory', $middlewareFactory); + $aliases[$this->applicationName . '.http.middleware_factory'] = MiddlewareFactory::class; // -- middleware pipeline @@ -303,28 +297,28 @@ private function registerApplication( $middlewarePipeline->addMethodCall('pipe', [new Reference($service)]); } - $container->setDefinition($this->applicationName . '.http.middleware_pipeline', $middlewarePipeline); + $container->setDefinition(MiddlewarePipe::class, $middlewarePipeline); + $aliases[$this->applicationName . '.http.middleware_pipeline'] = MiddlewarePipe::class; // -- routing - $appRouterConfig = $container->hasParameter($this->applicationName . '.router_config') - ? '%' . $this->applicationName . '.router_config%' - : []; - - $router = $this->createService(FastRouteRouter::class, [null, null, $appRouterConfig]); - - $container->setDefinition($this->applicationName . '.http.router', $router); - - $uriGenerator = $this->createService( - UriGenerator::class, - [new Reference($this->applicationName . '.http.router')] + $router = $this->createService( + FastRouteRouter::class, + [ + null, + null, + $this->readBCParameter($container, $this->applicationName . '.router_config', 'router_config', []), + ] ); - $container->setDefinition($this->applicationName . '.http.uri_generator', $uriGenerator); + $container->setDefinition(FastRouteRouter::class, $router); + $container->setAlias(RouterInterface::class, FastRouteRouter::class); + $aliases[$this->applicationName . '.http.router'] = FastRouteRouter::class; + $aliases[$this->applicationName . '.http.uri_generator'] = UriGeneratorInterface::class; $routeCollector = $this->createService( RouteCollector::class, - [new Reference($this->applicationName . '.http.router')] + [new Reference(FastRouteRouter::class)] ); foreach ($routes as $route) { @@ -332,31 +326,17 @@ private function registerApplication( 'route', [ $route['path'], - new Reference($this->applicationName . '.http.route.' . $route['route_name']), + new Reference('http.route.' . $route['route_name']), $route['methods'] ?? self::BEHAVIORS[$route['behavior']]['methods'], $route['route_name'], ] ); } - $container->setDefinition($this->applicationName . '.http.route_collector', $routeCollector); - - $routingMiddleware = $this->createService( - RouteMiddleware::class, - [new Reference($this->applicationName . '.http.router')] - ); - - $container->setDefinition($this->applicationName . '.http.middleware.route', $routingMiddleware); - - $implicitHeadMiddleware = $this->createService( - ImplicitHeadMiddleware::class, - [ - new Reference($this->applicationName . '.http.router'), - [new Reference(StreamFactoryInterface::class), 'createStream'], - ] - ); - - $container->setDefinition($this->applicationName . '.http.middleware.implicit_head', $implicitHeadMiddleware); + $container->setDefinition(RouteCollector::class, $routeCollector); + $aliases[$this->applicationName . '.http.route_collector'] = RouteCollector::class; + $aliases[$this->applicationName . '.http.middleware.route'] = RouteMiddleware::class; + $aliases[$this->applicationName . '.http.middleware.implicit_head'] = ImplicitHeadMiddleware::class; // -- content negotiation @@ -373,13 +353,15 @@ private function registerApplication( $formatters['application/problem+json'] = new Reference(Json::class); } - $applicationAllowedFormats = $this->applicationName . '.allowed_formats'; - $negotiator = $this->createService( ContentTypeMiddleware::class, [ - $container->hasParameter($applicationAllowedFormats) ? '%' . $applicationAllowedFormats . '%' - : '%chimera.default_allowed_formats%', + $this->readBCParameter( + $container, + $this->applicationName . '.allowed_formats', + 'allowed_formats', + '%chimera.default_allowed_formats%' + ), $formatters, new Reference(StreamFactoryInterface::class), ] @@ -387,47 +369,21 @@ private function registerApplication( $negotiator->setFactory([ContentTypeMiddleware::class, 'fromRecommendedSettings']); - $container->setDefinition($this->applicationName . '.http.middleware.content_negotiation', $negotiator); + $container->setDefinition(ContentTypeMiddleware::class, $negotiator); + $aliases[$this->applicationName . '.http.middleware.content_negotiation'] = ContentTypeMiddleware::class; + $aliases[$this->applicationName . '.http.request_handler_runner'] = RequestHandlerRunner::class; - // --- request handler runner - - $requestHandlerRunner = $this->createService( - RequestHandlerRunner::class, - [ - new Reference($this->applicationName . '.http.middleware_pipeline'), - new Reference(EmitterInterface::class), - [ServerRequestFactory::class, 'fromGlobals'], - new Reference(ServerRequestErrorResponseGenerator::class), - ] - ); - - $container->setDefinition($this->applicationName . '.http.request_handler_runner', $requestHandlerRunner); - - $container->setDefinition( - $this->applicationName . '.http_expressive', - new Definition( - Expressive::class, - [ - new Reference($this->applicationName . '.http.middleware_factory'), - new Reference($this->applicationName . '.http.middleware_pipeline'), - new Reference($this->applicationName . '.http.route_collector'), - new Reference($this->applicationName . '.http.request_handler_runner'), - ] - ) - ); - - $app = new Definition(Application::class, [new Reference($this->applicationName . '.http_expressive')]); + $app = new Definition(Application::class, [new Reference(Expressive::class)]); $app->setPublic(true); - $container->setDefinition($this->applicationName . '.http', $app); + $container->setDefinition(ApplicationInterface::class, $app); + $aliases[$this->applicationName . '.http'] = ApplicationInterface::class; - // --- alias application - - if ($container->hasAlias(ApplicationInterface::class)) { - throw new InvalidArgumentException('There can only be one application registered.'); + foreach ($aliases as $alias => $service) { + $container->setAlias($alias, $service); } - $container->setAlias(ApplicationInterface::class, new Alias($this->applicationName . '.http', true)); + $container->getAlias($this->applicationName . '.http')->setPublic(true); } private function generateReadAction(string $name, string $query, ContainerBuilder $container): Reference @@ -467,7 +423,7 @@ private function wrapHandler(string $name, ContainerBuilder $container): string $middleware = $this->createService( LazyLoadingMiddleware::class, [ - new Reference($this->applicationName . '.http.middleware_container'), + new Reference(MiddlewareContainer::class), $name . '.handler', ] ); @@ -502,7 +458,7 @@ public function createOnly(string $routeServiceId, array $route, ContainerBuilde $this->generateWriteAction($routeServiceId . '.action', $route['command'], $container), new Reference(ResponseFactoryInterface::class), $route['redirect_to'], - new Reference($this->applicationName . '.http.uri_generator'), + new Reference(UriGeneratorInterface::class), new Reference(IdentifierGenerator::class), $route['async'] === true ? StatusCode::STATUS_ACCEPTED : StatusCode::STATUS_CREATED, ] @@ -523,7 +479,7 @@ public function createAndFetch(string $routeServiceId, array $route, ContainerBu $this->generateReadAction($routeServiceId . '.read_action', $route['query'], $container), new Reference(ResponseFactoryInterface::class), $route['redirect_to'], - new Reference($this->applicationName . '.http.uri_generator'), + new Reference(UriGeneratorInterface::class), new Reference(IdentifierGenerator::class), ] ); @@ -574,4 +530,22 @@ public function noBehavior(string $routeServiceId, array $route, ContainerBuilde return $this->wrapHandler($routeServiceId, $container); } + + /** + * @param string|mixed[] $default + * + * @return mixed[]|string + */ + private function readBCParameter(ContainerBuilder $container, string $legacyName, string $parameterName, $default) + { + if ($container->hasParameter($legacyName)) { + return '%' . $legacyName . '%'; + } + + if ($container->hasParameter($parameterName)) { + return '%' . $parameterName . '%'; + } + + return $default; + } } diff --git a/tests/Unit/Routing/Expressive/RegisterServicesTest.php b/tests/Unit/Routing/Expressive/RegisterServicesTest.php index a2c81c82..a91172c4 100644 --- a/tests/Unit/Routing/Expressive/RegisterServicesTest.php +++ b/tests/Unit/Routing/Expressive/RegisterServicesTest.php @@ -45,8 +45,14 @@ public function exceptionShouldBeRaisedWhenTryingToRegisterDuplicatedRoutes(): v * @test * * @covers ::__construct - * @covers ::process + * @covers ::createService + * @covers ::extractMiddlewareList * @covers ::extractRoutes + * @covers ::prioritiseMiddleware + * @covers ::process + * @covers ::readBCParameter + * @covers ::registerApplication + * @covers ::registerServiceLocator */ public function registeringServicesDoesNotAllowMultipleApplications(): void { From 2e85edc293f66fa8b6784413d29e67d45924cb7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Tue, 30 Nov 2021 22:04:53 +0100 Subject: [PATCH 3/4] Apply changes to Mezzio Making sure everything still works as expected on the functional tests. --- config/routing-mezzio.xml | 42 ++++ src/Routing/Mezzio/RegisterServices.php | 202 ++++++++---------- src/ValidateApplicationComponents.php | 6 +- tests/Functional/ApplicationTestCase.php | 10 +- .../ValidateApplicationComponentsTest.php | 71 ++++++ 5 files changed, 216 insertions(+), 115 deletions(-) create mode 100644 tests/Unit/ValidateApplicationComponentsTest.php diff --git a/config/routing-mezzio.xml b/config/routing-mezzio.xml index b5079136..3e82f658 100644 --- a/config/routing-mezzio.xml +++ b/config/routing-mezzio.xml @@ -56,6 +56,48 @@ + + + + + + + + + + + + + createStream + + + + + + + + + + fromGlobals + + + + + + + + + + + + + + + + + + + diff --git a/src/Routing/Mezzio/RegisterServices.php b/src/Routing/Mezzio/RegisterServices.php index 3adbc42e..2413ad3f 100644 --- a/src/Routing/Mezzio/RegisterServices.php +++ b/src/Routing/Mezzio/RegisterServices.php @@ -9,29 +9,28 @@ use Chimera\ExecuteQuery; use Chimera\IdentifierGenerator; use Chimera\MessageCreator; +use Chimera\Routing\Application as ApplicationInterface; use Chimera\Routing\Handler\CreateAndFetch; use Chimera\Routing\Handler\CreateOnly; use Chimera\Routing\Handler\ExecuteAndFetch; use Chimera\Routing\Handler\ExecuteOnly; use Chimera\Routing\Handler\FetchOnly; use Chimera\Routing\Mezzio\Application; -use Chimera\Routing\Mezzio\UriGenerator; use Chimera\Routing\MissingRouteDispatching; use Chimera\Routing\RouteParamsExtraction; +use Chimera\Routing\UriGenerator as UriGeneratorInterface; use Fig\Http\Message\StatusCodeInterface as StatusCode; -use Laminas\Diactoros\ServerRequestFactory; -use Laminas\HttpHandlerRunner\Emitter\EmitterInterface; +use InvalidArgumentException; use Laminas\HttpHandlerRunner\RequestHandlerRunner; use Laminas\Stratigility\Middleware\PathMiddlewareDecorator; use Laminas\Stratigility\MiddlewarePipe; use Lcobucci\ContentNegotiation\ContentTypeMiddleware; use Lcobucci\ContentNegotiation\Formatter\Json; -use Mezzio\Application as Expressive; +use Mezzio\Application as Mezzio; use Mezzio\Helper\BodyParams\BodyParamsMiddleware; use Mezzio\Middleware\LazyLoadingMiddleware; use Mezzio\MiddlewareContainer; use Mezzio\MiddlewareFactory; -use Mezzio\Response\ServerRequestErrorResponseGenerator; use Mezzio\Router\FastRouteRouter; use Mezzio\Router\Middleware\DispatchMiddleware; use Mezzio\Router\Middleware\ImplicitHeadMiddleware; @@ -39,13 +38,13 @@ use Mezzio\Router\Middleware\MethodNotAllowedMiddleware; use Mezzio\Router\Middleware\RouteMiddleware; use Mezzio\Router\RouteCollector; +use Mezzio\Router\RouterInterface; use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\StreamFactoryInterface; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; -use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Reference; use function array_combine; @@ -95,13 +94,13 @@ public function process(ContainerBuilder $container): void $this->registerApplication( $container, - $routes[$this->applicationName] ?? [], - $this->prioritiseMiddleware($middlewareList[$this->applicationName] ?? []) + $routes ?? [], + $this->prioritiseMiddleware($middlewareList ?? []) ); } /** - * @return string[][][] + * @return string[][] * * @throws InvalidArgumentException */ @@ -135,12 +134,10 @@ private function extractRoutes(ContainerBuilder $container): array $tag['methods'] = explode(',', $tag['methods']); } - $tag['app'] ??= $this->applicationName; $tag['async'] = (bool) ($tag['async'] ?? false); $tag['serviceId'] = $serviceId; - $routes[$tag['app']] ??= []; - $routes[$tag['app']][] = $tag; + $routes[] = $tag; $names[$tag['route_name']] = $serviceId; } @@ -163,31 +160,26 @@ private function extractMiddlewareList(ContainerBuilder $container): array $priority = $tag['priority'] ?? 0; $path = $tag['path'] ?? '/'; - $tag['app'] ??= $this->applicationName; - - $list[$tag['app']][$priority][$path] ??= []; - $list[$tag['app']][$priority][$path][] = $serviceId; + $list[$priority][$path] ??= []; + $list[$priority][$path][] = $serviceId; } } - $list[$this->applicationName][Priorities::CONTENT_NEGOTIATION]['/'] ??= []; - $list[$this->applicationName][Priorities::BEFORE_CUSTOM]['/'] ??= []; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'] ??= []; + $list[Priorities::CONTENT_NEGOTIATION]['/'] ??= []; + $list[Priorities::BEFORE_CUSTOM]['/'] ??= []; + $list[Priorities::AFTER_CUSTOM]['/'] ??= []; - $list[$this->applicationName][Priorities::CONTENT_NEGOTIATION]['/'][] = $this->applicationName - . '.http.middleware.content_negotiation'; + $list[Priorities::CONTENT_NEGOTIATION]['/'][] = ContentTypeMiddleware::class; - $list[$this->applicationName][Priorities::BEFORE_CUSTOM]['/'][] = $this->applicationName - . '.http.middleware.route'; - $list[$this->applicationName][Priorities::BEFORE_CUSTOM]['/'][] = BodyParamsMiddleware::class; + $list[Priorities::BEFORE_CUSTOM]['/'][] = RouteMiddleware::class; + $list[Priorities::BEFORE_CUSTOM]['/'][] = BodyParamsMiddleware::class; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = $this->applicationName - . '.http.middleware.implicit_head'; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = ImplicitOptionsMiddleware::class; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = MethodNotAllowedMiddleware::class; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = RouteParamsExtraction::class; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = DispatchMiddleware::class; - $list[$this->applicationName][Priorities::AFTER_CUSTOM]['/'][] = MissingRouteDispatching::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = ImplicitHeadMiddleware::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = ImplicitOptionsMiddleware::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = MethodNotAllowedMiddleware::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = RouteParamsExtraction::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = DispatchMiddleware::class; + $list[Priorities::AFTER_CUSTOM]['/'][] = MissingRouteDispatching::class; return $list; } @@ -245,15 +237,23 @@ private function registerApplication( array $routes, array $middlewareList ): void { + if ($container->hasDefinition(ApplicationInterface::class)) { + throw new InvalidArgumentException('Registering multiple applications is deprecated.'); + } + $services = []; + $aliases = []; // for BC foreach ($routes as $route) { // @phpstan-ignore-next-line $services[] = $this->{self::BEHAVIORS[$route['behavior']]['callback']}( - $this->applicationName . '.http.route.' . $route['route_name'], + 'http.route.' . $route['route_name'], $route, $container ); + + $aliases[$this->applicationName . '.http.route.' . $route['route_name']] + = 'http.route.' . $route['route_name']; } $middleware = []; @@ -282,16 +282,12 @@ private function registerApplication( // -- middleware container $middlewareContainer = $this->createService(MiddlewareContainer::class, [$locator]); - $container->setDefinition($this->applicationName . '.http.middleware_container', $middlewareContainer); + $container->setDefinition(MiddlewareContainer::class, $middlewareContainer); + $aliases[$this->applicationName . '.http.middleware_container'] = MiddlewareContainer::class; // -- middleware factory - $middlewareFactory = $this->createService( - MiddlewareFactory::class, - [new Reference($this->applicationName . '.http.middleware_container')] - ); - - $container->setDefinition($this->applicationName . '.http.middleware_factory', $middlewareFactory); + $aliases[$this->applicationName . '.http.middleware_factory'] = MiddlewareFactory::class; // -- middleware pipeline @@ -301,28 +297,28 @@ private function registerApplication( $middlewarePipeline->addMethodCall('pipe', [new Reference($service)]); } - $container->setDefinition($this->applicationName . '.http.middleware_pipeline', $middlewarePipeline); + $container->setDefinition(MiddlewarePipe::class, $middlewarePipeline); + $aliases[$this->applicationName . '.http.middleware_pipeline'] = MiddlewarePipe::class; // -- routing - $appRouterConfig = $container->hasParameter($this->applicationName . '.router_config') - ? '%' . $this->applicationName . '.router_config%' - : []; - - $router = $this->createService(FastRouteRouter::class, [null, null, $appRouterConfig]); - - $container->setDefinition($this->applicationName . '.http.router', $router); - - $uriGenerator = $this->createService( - UriGenerator::class, - [new Reference($this->applicationName . '.http.router')] + $router = $this->createService( + FastRouteRouter::class, + [ + null, + null, + $this->readBCParameter($container, $this->applicationName . '.router_config', 'router_config', []), + ] ); - $container->setDefinition($this->applicationName . '.http.uri_generator', $uriGenerator); + $container->setDefinition(FastRouteRouter::class, $router); + $container->setAlias(RouterInterface::class, FastRouteRouter::class); + $aliases[$this->applicationName . '.http.router'] = FastRouteRouter::class; + $aliases[$this->applicationName . '.http.uri_generator'] = UriGeneratorInterface::class; $routeCollector = $this->createService( RouteCollector::class, - [new Reference($this->applicationName . '.http.router')] + [new Reference(FastRouteRouter::class)] ); foreach ($routes as $route) { @@ -330,31 +326,17 @@ private function registerApplication( 'route', [ $route['path'], - new Reference($this->applicationName . '.http.route.' . $route['route_name']), + new Reference('http.route.' . $route['route_name']), $route['methods'] ?? self::BEHAVIORS[$route['behavior']]['methods'], $route['route_name'], ] ); } - $container->setDefinition($this->applicationName . '.http.route_collector', $routeCollector); - - $routingMiddleware = $this->createService( - RouteMiddleware::class, - [new Reference($this->applicationName . '.http.router')] - ); - - $container->setDefinition($this->applicationName . '.http.middleware.route', $routingMiddleware); - - $implicitHeadMiddleware = $this->createService( - ImplicitHeadMiddleware::class, - [ - new Reference($this->applicationName . '.http.router'), - [new Reference(StreamFactoryInterface::class), 'createStream'], - ] - ); - - $container->setDefinition($this->applicationName . '.http.middleware.implicit_head', $implicitHeadMiddleware); + $container->setDefinition(RouteCollector::class, $routeCollector); + $aliases[$this->applicationName . '.http.route_collector'] = RouteCollector::class; + $aliases[$this->applicationName . '.http.middleware.route'] = RouteMiddleware::class; + $aliases[$this->applicationName . '.http.middleware.implicit_head'] = ImplicitHeadMiddleware::class; // -- content negotiation @@ -371,13 +353,15 @@ private function registerApplication( $formatters['application/problem+json'] = new Reference(Json::class); } - $applicationAllowedFormats = $this->applicationName . '.allowed_formats'; - $negotiator = $this->createService( ContentTypeMiddleware::class, [ - $container->hasParameter($applicationAllowedFormats) ? '%' . $applicationAllowedFormats . '%' - : '%chimera.default_allowed_formats%', + $this->readBCParameter( + $container, + $this->applicationName . '.allowed_formats', + 'allowed_formats', + '%chimera.default_allowed_formats%' + ), $formatters, new Reference(StreamFactoryInterface::class), ] @@ -385,39 +369,23 @@ private function registerApplication( $negotiator->setFactory([ContentTypeMiddleware::class, 'fromRecommendedSettings']); - $container->setDefinition($this->applicationName . '.http.middleware.content_negotiation', $negotiator); - - // --- request handler runner + $container->setDefinition(ContentTypeMiddleware::class, $negotiator); + $aliases[$this->applicationName . '.http.middleware.content_negotiation'] = ContentTypeMiddleware::class; + $aliases[$this->applicationName . '.http.request_handler_runner'] = RequestHandlerRunner::class; - $requestHandlerRunner = $this->createService( - RequestHandlerRunner::class, - [ - new Reference($this->applicationName . '.http.middleware_pipeline'), - new Reference(EmitterInterface::class), - [ServerRequestFactory::class, 'fromGlobals'], - new Reference(ServerRequestErrorResponseGenerator::class), - ] - ); + $app = new Definition(Application::class, [new Reference(Mezzio::class)]); + $app->setPublic(true); - $container->setDefinition($this->applicationName . '.http.request_handler_runner', $requestHandlerRunner); + $container->setDefinition(ApplicationInterface::class, $app); + $aliases[$this->applicationName . '.http'] = ApplicationInterface::class; - $container->setDefinition( - $this->applicationName . '.http_expressive', - new Definition( - Expressive::class, - [ - new Reference($this->applicationName . '.http.middleware_factory'), - new Reference($this->applicationName . '.http.middleware_pipeline'), - new Reference($this->applicationName . '.http.route_collector'), - new Reference($this->applicationName . '.http.request_handler_runner'), - ] - ) - ); - - $app = new Definition(Application::class, [new Reference($this->applicationName . '.http_expressive')]); - $app->setPublic(true); + foreach ($aliases as $alias => $service) { + $container->setAlias($alias, $service)->setDeprecated('chimera/di-symfony', '0.5.0', null); + } - $container->setDefinition($this->applicationName . '.http', $app); + $container->getAlias($this->applicationName . '.http') + ->setDeprecated('chimera/di-symfony', '0.5.0', null) + ->setPublic(true); } private function generateReadAction(string $name, string $query, ContainerBuilder $container): Reference @@ -457,7 +425,7 @@ private function wrapHandler(string $name, ContainerBuilder $container): string $middleware = $this->createService( LazyLoadingMiddleware::class, [ - new Reference($this->applicationName . '.http.middleware_container'), + new Reference(MiddlewareContainer::class), $name . '.handler', ] ); @@ -492,7 +460,7 @@ public function createOnly(string $routeServiceId, array $route, ContainerBuilde $this->generateWriteAction($routeServiceId . '.action', $route['command'], $container), new Reference(ResponseFactoryInterface::class), $route['redirect_to'], - new Reference($this->applicationName . '.http.uri_generator'), + new Reference(UriGeneratorInterface::class), new Reference(IdentifierGenerator::class), $route['async'] === true ? StatusCode::STATUS_ACCEPTED : StatusCode::STATUS_CREATED, ] @@ -513,7 +481,7 @@ public function createAndFetch(string $routeServiceId, array $route, ContainerBu $this->generateReadAction($routeServiceId . '.read_action', $route['query'], $container), new Reference(ResponseFactoryInterface::class), $route['redirect_to'], - new Reference($this->applicationName . '.http.uri_generator'), + new Reference(UriGeneratorInterface::class), new Reference(IdentifierGenerator::class), ] ); @@ -564,4 +532,22 @@ public function noBehavior(string $routeServiceId, array $route, ContainerBuilde return $this->wrapHandler($routeServiceId, $container); } + + /** + * @param string|mixed[] $default + * + * @return mixed[]|string + */ + private function readBCParameter(ContainerBuilder $container, string $legacyName, string $parameterName, $default) + { + if ($container->hasParameter($legacyName)) { + return '%' . $legacyName . '%'; + } + + if ($container->hasParameter($parameterName)) { + return '%' . $parameterName . '%'; + } + + return $default; + } } diff --git a/src/ValidateApplicationComponents.php b/src/ValidateApplicationComponents.php index 8f4a60ef..f473f656 100644 --- a/src/ValidateApplicationComponents.php +++ b/src/ValidateApplicationComponents.php @@ -3,6 +3,7 @@ namespace Chimera\DependencyInjection; +use Chimera\Routing\Application; use RuntimeException; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -22,9 +23,10 @@ public function __construct(string $appName) /** @throws InvalidArgumentException */ public function process(ContainerBuilder $container): void { - $httpInterface = $container->getDefinition($this->appName . '.http'); + $httpInterface = $container->getDefinition(Application::class); + $alias = $container->getAlias($this->appName . '.http'); - if (! $httpInterface->isPublic()) { + if (! $httpInterface->isPublic() || ! $alias->isPublic()) { throw new RuntimeException( sprintf('The HTTP interface for "%s" is not a public service', $this->appName) ); diff --git a/tests/Functional/ApplicationTestCase.php b/tests/Functional/ApplicationTestCase.php index 6b3d6cde..882d167f 100644 --- a/tests/Functional/ApplicationTestCase.php +++ b/tests/Functional/ApplicationTestCase.php @@ -44,11 +44,6 @@ final protected function createContainer(): ContainerInterface $builder->addPass( $this->makeServicesPublic( [ - 'sample-app.http.route_collector', - 'sample-app.http.middleware_pipeline', - 'sample-app.http.middleware.content_negotiation', - 'sample-app.http.middleware.route', - 'sample-app.http.middleware.implicit_head', 'sample-app.command_bus', 'sample-app.command_bus.decorated_bus', 'sample-app.command_bus.decorated_bus.handler', @@ -70,6 +65,11 @@ final protected function createContainer(): ContainerInterface ErrorConversionMiddleware::class, ], [ + 'sample-app.http.route_collector', + 'sample-app.http.middleware_pipeline', + 'sample-app.http.middleware.content_negotiation', + 'sample-app.http.middleware.route', + 'sample-app.http.middleware.implicit_head', 'sample-app.command_bus.decorated_bus.handler.locator', 'sample-app.query_bus.decorated_bus.handler.locator', ] diff --git a/tests/Unit/ValidateApplicationComponentsTest.php b/tests/Unit/ValidateApplicationComponentsTest.php new file mode 100644 index 00000000..d2c5715a --- /dev/null +++ b/tests/Unit/ValidateApplicationComponentsTest.php @@ -0,0 +1,71 @@ +setDefinition(Application::class, new Definition()); + $builder->setAlias('sample-app.http', Application::class)->setPublic(true); + + $pass = new ValidateApplicationComponents('sample-app'); + + $this->expectException(RuntimeException::class); + $this->expectErrorMessage('The HTTP interface for "sample-app" is not a public service'); + $pass->process($builder); + } + + /** + * @test + * + * @covers ::__construct + * @covers ::process + */ + public function exceptionShouldBeRaisedWhenLegacyAliasIsNotPublic(): void + { + $builder = new ContainerBuilder(); + $builder->setDefinition(Application::class, (new Definition())->setPublic(true)); + $builder->setAlias('sample-app.http', Application::class); + + $pass = new ValidateApplicationComponents('sample-app'); + + $this->expectException(RuntimeException::class); + $this->expectErrorMessage('The HTTP interface for "sample-app" is not a public service'); + $pass->process($builder); + } + + /** + * @test + * + * @covers ::__construct + * @covers ::process + */ + public function noExceptionShouldBeRaisedWhenExpectedServicesArePublic(): void + { + $builder = new ContainerBuilder(); + $builder->setDefinition(Application::class, (new Definition())->setPublic(true)); + $builder->setAlias('sample-app.http', Application::class)->setPublic(true); + + $pass = new ValidateApplicationComponents('sample-app'); + $pass->process($builder); + + $this->addToAssertionCount(1); + } +} From 13345759db9b5ebaf847de230ba865c63ef3e3e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Tue, 30 Nov 2021 22:32:27 +0100 Subject: [PATCH 4/4] Temporarily reduce MSI requirements The Expressive implementation is getting in the way of keeping a sane amount here. We'll review these requirements once the deprecated component is removed. --- infection.json.dist | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infection.json.dist b/infection.json.dist index cf685482..642f8012 100644 --- a/infection.json.dist +++ b/infection.json.dist @@ -9,5 +9,5 @@ "text": "infection.log" }, "minMsi": 40, - "minCoveredMsi": 80 + "minCoveredMsi": 75 }