diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4b05237..5975bcb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,26 +26,46 @@ jobs: - '8.1' - '8.2' - '8.3' + - '8.4' symfony: - '3.*' - '4.*' - '5.*' - '6.*' + - '7.*' dependency: + - 'lowest' - 'highest' - include: - - { php: '7.1', symfony: '3.*', dependency: 'lowest' } exclude: - { php: '8.0', symfony: '3.*' } - { php: '8.1', symfony: '3.*' } - { php: '8.2', symfony: '3.*' } - { php: '8.3', symfony: '3.*' } + - { php: '8.4', symfony: '3.*' } - { php: '7.1', symfony: '5.*' } - { php: '7.1', symfony: '6.*' } - { php: '7.2', symfony: '6.*' } - { php: '7.3', symfony: '6.*' } - { php: '7.4', symfony: '6.*' } - - { php: '8.0', symfony: '6.*' } + - { php: '7.1', symfony: '7.*' } + - { php: '7.2', symfony: '7.*' } + - { php: '7.3', symfony: '7.*' } + - { php: '7.4', symfony: '7.*' } + - { php: '8.0', symfony: '7.*' } + - { php: '8.1', symfony: '7.*' } + - { php: '8.0', symfony: '4.*', dependency: 'lowest' } + - { php: '8.1', symfony: '4.*', dependency: 'lowest' } + - { php: '8.1', symfony: '5.*', dependency: 'lowest' } + - { php: '8.1', symfony: '6.*', dependency: 'lowest' } + - { php: '8.2', symfony: '4.*', dependency: 'lowest' } + - { php: '8.2', symfony: '5.*', dependency: 'lowest' } + - { php: '8.2', symfony: '6.*', dependency: 'lowest' } + - { php: '8.3', symfony: '4.*', dependency: 'lowest' } + - { php: '8.3', symfony: '5.*', dependency: 'lowest' } + - { php: '8.3', symfony: '6.*', dependency: 'lowest' } + - { php: '8.4', symfony: '4.*', dependency: 'lowest' } + - { php: '8.4', symfony: '5.*', dependency: 'lowest' } + - { php: '8.4', symfony: '6.*', dependency: 'lowest' } steps: - name: Checkout diff --git a/CHANGELOG.md b/CHANGELOG.md index 67337b9..ebdf875 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,84 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.0.0] +### Added +- Support for Symfony 7.4 and `psr/log` 3; CI runs PHP 8.4, Symfony 7 and lowest dependencies +- Explicit requirements for `ext-tokenizer` and the Symfony components the bundle uses directly: `symfony/config`, + `dependency-injection`, `http-foundation`, `http-kernel`, `property-access`, `routing` and `security-core` + +### Changed +- `Configuration::getConfigTreeBuilder()` declares its `TreeBuilder` return type, as Symfony 7 requires, and the class + is `@final`. Breaking for a subclass that overrides the method without the return type +- `PayseraApiExtension` extends `Symfony\Component\DependencyInjection\Extension\Extension` instead of HttpKernel's + `Extension`, which is internal since Symfony 7.1. Breaking for a subclass that calls `addAnnotatedClassesToCompile()` +- Services are registered by `PayseraApiExtension` instead of the XML files in `Resources/config`, which are removed, + because Symfony 7.4 deprecates XML configuration. Service ids, classes, arguments, calls and tags are unchanged, + except that both internal route loaders take the new `paysera_api.rest_route_configurator` service (with the new + `paysera_api.docblock_annotation_guard`) instead of the request helper and the options builders +- An error while Symfony's annotation reader reads a route's docblock annotations for the bundle, or while the bundle + builds its attributes, from an annotation or attribute constructor too, is wrapped in a `ConfigurationException` + naming the action. Symfony's own loader reads most docblocks with that reader first, so their errors come from Symfony + unwrapped, and a class with a missing parent (before PHP 7.3) or trait (any version) still stops PHP. An attribute + named under `Paysera\Bundle\ApiBundle\` that is none of the bundle's attributes (a misspelt name or namespace) fails + too, and so does one of the bundle's attributes written without its `use` statement, except where Security says it is + not detected +- Optional parameters are declared nullable explicitly (`?Type $parameter = null`), as PHP 8.4 expects +- `doctrine/annotations` is required at 1.14.4 or 2.0.2 and later, whose reader refuses to run with + `opcache.save_comments=0` instead of reading the stripped docblocks as empty +- `LocaleListener` marks "no configured locale matched" with a `,` placeholder instead of `default`, which no unquoted + `Accept-Language` item can contain. A header listing the language `default` or `i-default` before a configured locale + now selects that locale, where 1.x kept the default locale + +### Fixed +- On Symfony 7.1 and later, `LocaleListener` kept the default locale for `de-DE` (and for `de` from Symfony 7.3) with + `de` configured, because Symfony matched the language against the placeholder `default` first + +### Security +- Where Symfony passes no annotation reader to the route loader (Symfony 7, and Symfony 5.4 on PHP 8 or Symfony 6 with + `framework.annotations` disabled), 1.x ignored the bundle's docblock annotations, `@RequiredPermissions` among them, + without an error on Symfony 6.4, and failed to load any route before it. Loading the routes now fails there with a + `ConfigurationException` naming the action, the file and what was found, when the file of the routed class, of the + class declaring the action, of the action itself or of any trait these classes use imports anything from + `Paysera\Bundle\ApiBundle\Annotation` (plain, aliased, grouped or as a namespace alias), or has a docblock tag named + like one of the bundle's annotations (`@RequiredPermissions`, `@Rest\RequiredPermissions`, + `@\Paysera\Bundle\ApiBundle\Annotation\RequiredPermissions`, split across lines too, in any letter case, with or + without arguments). The check reads the source files, so OPcache stripping the doc comments + (`opcache.save_comments=0`) does not hide them, and a file that cannot be read fails the load too. It refuses more + than Doctrine would read: an unused import of the namespace (remove it), a tag of another library named like one of + the bundle's annotations (import the other library's class under another alias), a trait with annotations next to the + action, a bundle tag in a docblock Doctrine reads for no route, such as a property's (remove it), a bundle tag at the + very start of a line or right after a double quote or a non-ASCII character, which Doctrine reads in some of these + places and skips in others (move it after a space), a tag path with non-ASCII characters beyond the long s and the + Kelvin sign that Doctrine accepts inside a name (remove or reword it; for another library's tag, import its class + under another alias), and a bundle tag in a docblock with invalid UTF-8 that Doctrine would read nothing from; the + message names the import, the tag or the trait. Write the options as attributes, or on Symfony 6.4 keep + `framework.annotations` enabled. Not detected: an annotation class of your own implementing `RestAnnotationInterface`, + which nothing in the controller's file reveals (write such an option as an attribute implementing + `RestAttributeInterface`, which is applied on every path), and an alias of a bundle annotation class made with + `class_alias()` +- On PHP 8 before Symfony 6.4, 1.x ignored the bundle's attributes, `#[RequiredPermissions]` among them, without an + error. They are now applied on every supported Symfony version +- On PHP 8, a bundle attribute written without its `use` statement, `#[RequiredPermissions]` among them, names a class + in the namespace it is written in that cannot be loaded, which PHP and 1.x skip without an error. Loading the routes + now fails with a `ConfigurationException` naming the action and the bundle's attribute, for an attribute whose class + cannot be loaded, whose name without its namespace is one of the bundle's attributes, and whose name lies in the + namespace of the class it is written in (for an action, of its declaring class or a trait in its file) or below it; in + the global namespace, it must have no namespace part. An attribute imported from a package that is not installed, such + as swagger-php's `#[OA\Query]`, and a class of that name that can be loaded, such as the application's own `Body` + attribute, are left alone. Not detected: any other attribute whose class cannot be loaded, outside + `Paysera\Bundle\ApiBundle\`, such as a misspelt `#[RequiredPermission]`, the application's own attribute of another + name without its `use` statement, one imported from a misspelt namespace such as `Paysera\ApiBundle\Attribute`, and, + in a class of the global namespace, `#[Rest\RequiredPermissions]` without its alias import, which PHP reads as a + complete name +- On PHP 8, a route configured with both docblock annotations and attributes of the bundle, on the action or on its + class, fails to load with a `ConfigurationException`. Before, the attributes silently replaced the docblock options + from Symfony 6.4, so `#[Body]` on an action dropped the `@RequiredPermissions` of its class, and were ignored before +- These fixes are not backported to 1.x. 1.x still drops the bundle's permission checks without an error on Symfony 6.4 + with `framework.annotations` disabled, on PHP 8 before Symfony 6.4 for the bundle's attributes, on routes configured + with both, and for an attribute written without its `use` statement. Upgrade to 2.0.0, which applies the checks there + or fails to load the routes where it cannot, except in the cases listed above as not detected + ## [1.8.2] ### Changed - CI allows packages with security advisories, so Symfony 3.4 and 4.4 jobs can install dependencies with Composer 2.10 diff --git a/README.md b/README.md index eec01c8..eedbc09 100644 --- a/README.md +++ b/README.md @@ -147,12 +147,15 @@ class ApiController } } ``` -Controller example using attributes: +Controller example using attributes. The examples import `Symfony\Component\Routing\Attribute\Route`, which exists +from Symfony 6.4. On Symfony 5.2 to 6.3, import `Symfony\Component\Routing\Annotation\Route` instead; Symfony skips +an attribute class it does not know, so the route would not exist. Symfony before 5.2 reads no `#[Route]`: use a +docblock `@Route` there. ```php ``` +From Symfony 6.1, import them with `type: attribute`, which up to 6.4 also loads docblock `@Route` controllers (with +annotations enabled); Symfony 7 reads only `#[Route]`. Symfony 6.4 deprecates `type: annotation` and Symfony 7 rejects +it (Symfony 7.4 also deprecates XML route files): + ```yaml acme_something: resource: "@AcmeSomethingBundle/Controller/" - type: annotation + type: attribute prefix: /rest/v1/ ``` +```xml + + +``` + This also requires that your controller's service ID would be the same as its FQCN. #### HTTP example @@ -257,7 +277,7 @@ Controller example using attributes: setSupportedContentTypes($options['supportedContentTypes'] ?? $supportedContentTypes); diff --git a/src/Attribute/PathAttribute.php b/src/Attribute/PathAttribute.php index 471f9a7..fca6827 100644 --- a/src/Attribute/PathAttribute.php +++ b/src/Attribute/PathAttribute.php @@ -35,8 +35,8 @@ class PathAttribute implements RestAttributeInterface public function __construct( array $options = [], - string $parameterName = null, - string $pathPartName = null, + ?string $parameterName = null, + ?string $pathPartName = null, ?string $resolverType = null, ?bool $resolutionMandatory = null ) { diff --git a/src/Attribute/Query.php b/src/Attribute/Query.php index ea47fb8..0a23815 100644 --- a/src/Attribute/Query.php +++ b/src/Attribute/Query.php @@ -35,7 +35,7 @@ class Query implements RestAttributeInterface public function __construct( array $options = [], - string $parameterName = null, + ?string $parameterName = null, ?string $denormalizationType = null, ?string $denormalizationGroup = null, ?Validation $validation = null diff --git a/src/Attribute/RequiredPermissions.php b/src/Attribute/RequiredPermissions.php index 648d0c1..4960d7a 100644 --- a/src/Attribute/RequiredPermissions.php +++ b/src/Attribute/RequiredPermissions.php @@ -18,7 +18,7 @@ class RequiredPermissions implements RestAttributeInterface public function __construct( array $options = [], - array $permissions = null + ?array $permissions = null ) { $this->setPermissions($options['permissions'] ?? $permissions); } diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 99b4c98..088fca6 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -8,9 +8,12 @@ use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; +/** + * @final + */ class Configuration implements ConfigurationInterface { - public function getConfigTreeBuilder() + public function getConfigTreeBuilder(): TreeBuilder { $treeBuilder = new TreeBuilder('paysera_api'); $rootNode = method_exists($treeBuilder, 'getRootNode') diff --git a/src/DependencyInjection/PayseraApiExtension.php b/src/DependencyInjection/PayseraApiExtension.php index 2a1db01..66434e7 100644 --- a/src/DependencyInjection/PayseraApiExtension.php +++ b/src/DependencyInjection/PayseraApiExtension.php @@ -5,43 +5,72 @@ namespace Paysera\Bundle\ApiBundle\DependencyInjection; use Doctrine\Persistence\ObjectRepository; +use Paysera\Bundle\ApiBundle\Listener\LocaleListener; +use Paysera\Bundle\ApiBundle\Listener\RestExceptionListener; +use Paysera\Bundle\ApiBundle\Listener\RestRequestListener; +use Paysera\Bundle\ApiBundle\Listener\RestResponseListener; +use Paysera\Bundle\ApiBundle\Normalizer\ErrorNormalizer; +use Paysera\Bundle\ApiBundle\Normalizer\Pagination\PagedQueryNormalizer; +use Paysera\Bundle\ApiBundle\Normalizer\Pagination\PagerDenormalizer; +use Paysera\Bundle\ApiBundle\Normalizer\Pagination\ResultNormalizer; +use Paysera\Bundle\ApiBundle\Normalizer\ViolationNormalizer; +use Paysera\Bundle\ApiBundle\Service\ContentTypeMatcher; +use Paysera\Bundle\ApiBundle\Service\ErrorBuilder; use Paysera\Bundle\ApiBundle\Service\PathAttributeResolver\DoctrinePathAttributeResolver; +use Paysera\Bundle\ApiBundle\Service\PathAttributeResolver\PathAttributeResolutionManager; +use Paysera\Bundle\ApiBundle\Service\PathAttributeResolver\PathAttributeResolverRegistry; +use Paysera\Bundle\ApiBundle\Service\ResponseBuilder; +use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; +use Paysera\Bundle\ApiBundle\Service\RestRequestOptionsRegistry; +use Paysera\Bundle\ApiBundle\Service\RestRequestOptionsValidator; +use Paysera\Bundle\ApiBundle\Service\RoutingLoader\DocblockAnnotationGuard; +use Paysera\Bundle\ApiBundle\Service\RoutingLoader\RestRequestAnnotationOptionsBuilder; +use Paysera\Bundle\ApiBundle\Service\RoutingLoader\RestRequestAttributeOptionsBuilder; +use Paysera\Bundle\ApiBundle\Service\RoutingLoader\RestRouteConfigurator; +use Paysera\Bundle\ApiBundle\Service\RoutingLoader\RoutingAnnotationLoader; +use Paysera\Bundle\ApiBundle\Service\RoutingLoader\RoutingAttributeLoader; +use Paysera\Bundle\ApiBundle\Service\Validation\CamelCaseToSnakeCaseConverter; +use Paysera\Bundle\ApiBundle\Service\Validation\EntityValidator; +use Paysera\Pagination\Service\CursorBuilder; +use Paysera\Pagination\Service\Doctrine\QueryAnalyser; +use Paysera\Pagination\Service\Doctrine\ResultProvider; use RuntimeException; use Symfony\Bundle\FrameworkBundle\Routing\AttributeRouteControllerLoader; -use Symfony\Component\Config\FileLocator; +use Symfony\Component\DependencyInjection\Alias; +use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; -use Symfony\Component\DependencyInjection\Loader; +use Symfony\Component\DependencyInjection\Extension\Extension; use Symfony\Component\DependencyInjection\Reference; -use Symfony\Component\HttpKernel\DependencyInjection\Extension; +use Symfony\Component\PropertyAccess\PropertyAccess; +use Symfony\Component\PropertyAccess\PropertyAccessor; class PayseraApiExtension extends Extension { + /** + * @return void + */ public function load(array $configs, ContainerBuilder $container) { $configuration = new Configuration(); $config = $this->processConfiguration($configuration, $configs); - $loader = new Loader\XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); - $loader->load('services.xml'); - - $loader = new Loader\XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config/services')); - class_exists(AttributeRouteControllerLoader::class) - ? $loader->load('attributes.xml') - : $loader->load('annotations.xml') - ; - - $container->setParameter('paysera_api.locales', $config['locales']); - if (count($config['locales']) === 0) { - $container->removeDefinition('paysera_api.listener.locale'); + if (count($config['locales']) > 0) { + $container->setDefinition('paysera_api.listener.locale', $this->createLocaleListenerDefinition()); } + $container->addDefinitions($this->createListenerDefinitions()); + $container->addDefinitions($this->createNormalizerDefinitions()); + $container->addDefinitions($this->createPaginationDefinitions()); + $container->addDefinitions($this->createServiceDefinitions()); + $container->setAlias( + 'paysera_api.validation.property_path_converter', + $config['validation']['property_path_converter'] + ?? new Alias('paysera_api.camel_case_to_snake_case_converter', false) + ); + $container->addDefinitions($this->createRoutingLoaderDefinitions()); - if ($config['validation']['property_path_converter'] !== null) { - $container->setAlias( - 'paysera_api.validation.property_path_converter', - $config['validation']['property_path_converter'] - ); - } + $container->setParameter('paysera_api.locales', $config['locales']); if (count($config['path_attribute_resolvers']) > 0 && !class_exists('Doctrine\ORM\EntityManager')) { throw new RuntimeException( @@ -59,6 +88,183 @@ class_exists(AttributeRouteControllerLoader::class) $this->configurePagination($container, $config['pagination']); } + private function createLocaleListenerDefinition(): Definition + { + return (new Definition(LocaleListener::class, [ + new Reference('paysera_api.rest_request_helper'), + '%paysera_api.locales%', + ]))->addTag('kernel.event_listener', ['event' => 'kernel.request', 'priority' => 20]); + } + + /** + * @return array + */ + private function createListenerDefinitions(): array + { + $restRequestHelper = new Reference('paysera_api.rest_request_helper'); + $coreNormalizer = new Reference('paysera_normalization.core_normalizer'); + $responseBuilder = new Reference('paysera_api.response_builder'); + + return [ + 'paysera_api.listener.rest_exception' => (new Definition(RestExceptionListener::class, [ + $restRequestHelper, + new Reference('paysera_api.error_builder'), + $coreNormalizer, + $responseBuilder, + new Reference('logger', ContainerInterface::NULL_ON_INVALID_REFERENCE), + ]))->addTag('kernel.event_listener', ['event' => 'kernel.exception', 'priority' => 1]), + 'paysera_api.listener.rest_request' => (new Definition(RestRequestListener::class, [ + new Reference('paysera_normalization.core_denormalizer'), + new Reference('security.authorization_checker'), + new Reference('security.token_storage'), + $restRequestHelper, + new Reference('paysera_api.entity_validator'), + new Reference('paysera_api.content_type_matcher'), + new Reference('paysera_api.path_attribute_resolution_manager'), + ])) + ->addTag('kernel.event_listener', ['event' => 'kernel.controller']) + ->addTag('kernel.event_listener', ['event' => 'kernel.request', 'priority' => 31]), + 'paysera_api.listener.rest_response' => (new Definition(RestResponseListener::class, [ + $coreNormalizer, + $restRequestHelper, + $responseBuilder, + ]))->addTag('kernel.event_listener', ['event' => 'kernel.view']), + ]; + } + + /** + * @return array + */ + private function createNormalizerDefinitions(): array + { + $tag = 'paysera_normalization.autoconfigured_normalizer'; + + return [ + 'paysera_api.normalizer.violation' => (new Definition(ViolationNormalizer::class))->addTag($tag), + 'paysera_api.normalizer.error' => (new Definition(ErrorNormalizer::class))->addTag($tag), + 'paysera_api.normalizer.result' => (new Definition(ResultNormalizer::class))->addTag($tag), + 'paysera_api.normalizer.paged_query' => (new Definition(PagedQueryNormalizer::class, [ + new Reference('paysera_api.pagination.result_provider'), + '%paysera_api.pagination.default_total_count_strategy%', + '%paysera_api.pagination.maximum_offset%', + ]))->addTag($tag), + 'paysera_api.normalizer.pager_denormalizer' => (new Definition(PagerDenormalizer::class, [ + '%paysera_api.pagination.default_limit%', + '%paysera_api.pagination.maximum_limit%', + ]))->addTag($tag), + ]; + } + + /** + * @return array + */ + private function createPaginationDefinitions(): array + { + $propertyAccessor = (new Definition(PropertyAccessor::class)) + ->setFactory([PropertyAccess::class, 'createPropertyAccessor']) + ; + + return [ + 'paysera_api.pagination.result_provider' => new Definition(ResultProvider::class, [ + new Definition(QueryAnalyser::class), + new Definition(CursorBuilder::class, [$propertyAccessor]), + ]), + ]; + } + + /** + * @return array + */ + private function createServiceDefinitions(): array + { + $optionsValidator = new Reference('paysera_api.rest_request_options_validator'); + $resolverRegistry = new Reference('paysera_api.path_attribute_resolver_registry'); + + return [ + 'paysera_api.rest_request_options_registry' => new Definition(RestRequestOptionsRegistry::class), + 'paysera_api.rest_request_helper' => new Definition(RestRequestHelper::class, [ + new Reference('paysera_api.rest_request_options_registry'), + ]), + 'paysera_api.annotations.options_builder' => new Definition( + RestRequestAnnotationOptionsBuilder::class, + [$optionsValidator] + ), + 'paysera_api.attributes.options_builder' => new Definition( + RestRequestAttributeOptionsBuilder::class, + [$optionsValidator] + ), + 'paysera_api.docblock_annotation_guard' => new Definition(DocblockAnnotationGuard::class), + 'paysera_api.rest_route_configurator' => new Definition(RestRouteConfigurator::class, [ + new Reference('paysera_api.rest_request_helper'), + new Reference('paysera_api.annotations.options_builder'), + new Reference('paysera_api.attributes.options_builder'), + new Reference('paysera_api.docblock_annotation_guard'), + ]), + 'paysera_api.response_builder' => new Definition(ResponseBuilder::class), + 'paysera_api.error_builder' => $this->createErrorBuilderDefinition(), + 'paysera_api.content_type_matcher' => new Definition(ContentTypeMatcher::class), + 'paysera_api.camel_case_to_snake_case_converter' => new Definition(CamelCaseToSnakeCaseConverter::class), + 'paysera_api.entity_validator' => new Definition(EntityValidator::class, [ + new Reference('validator', ContainerInterface::NULL_ON_INVALID_REFERENCE), + new Reference('paysera_api.validation.property_path_converter'), + ]), + 'paysera_api.rest_request_options_validator' => new Definition(RestRequestOptionsValidator::class, [ + new Reference('paysera_normalization.normalizer_registry'), + $resolverRegistry, + ]), + 'paysera_api.path_attribute_resolver_registry' => new Definition(PathAttributeResolverRegistry::class), + 'paysera_api.path_attribute_resolution_manager' => new Definition( + PathAttributeResolutionManager::class, + [$resolverRegistry] + ), + ]; + } + + private function createErrorBuilderDefinition(): Definition + { + $definition = new Definition(ErrorBuilder::class); + $errors = [ + ['invalid_request', 400, 'Request content is invalid'], + ['invalid_parameters', 400, 'Some required parameter is missing or it\'s format is invalid'], + ['invalid_state', 409, 'Requested action cannot be made to the current state of resource'], + ['unauthorized', 401, 'You have not provided any credentials or they are invalid'], + ['forbidden', 403, 'You have no rights to access requested resource or make requested action'], + ['not_found', 404, 'Resource was not found'], + ['internal_server_error', 500, 'Unexpected internal system error'], + ['not_acceptable', 406, 'Unknown request or response format'], + ]; + foreach ($errors as $error) { + $definition->addMethodCall('configureError', $error); + } + + return $definition; + } + + /** + * @return array + */ + private function createRoutingLoaderDefinitions(): array + { + $restRouteConfigurator = new Reference('paysera_api.rest_route_configurator'); + + if (!class_exists(AttributeRouteControllerLoader::class)) { + return [ + 'paysera_api.annotations.loader' => (new ChildDefinition('routing.loader.annotation')) + ->setClass(RoutingAnnotationLoader::class) + ->setDecoratedService('routing.loader.annotation') + ->addMethodCall('setRestRouteConfigurator', [$restRouteConfigurator]), + ]; + } + + $attributeLoader = (new ChildDefinition('routing.loader.attribute')) + ->setClass(RoutingAttributeLoader::class) + ->setDecoratedService('routing.loader.attribute') + ->addMethodCall('setRestRouteConfigurator', [$restRouteConfigurator]) + ; + + return ['paysera_api.loader.attribute' => $attributeLoader]; + } + private function buildPathAttributeResolverDefinition(string $className, string $field): Definition { $repositoryDefinition = (new Definition(ObjectRepository::class, [$className])) diff --git a/src/Entity/QueryResolverOptions.php b/src/Entity/QueryResolverOptions.php index b898a53..e3701c9 100644 --- a/src/Entity/QueryResolverOptions.php +++ b/src/Entity/QueryResolverOptions.php @@ -58,7 +58,7 @@ public function setDenormalizationGroup($denormalizationGroup): self * @param ValidationOptions|null $validationOptions * @return $this */ - public function setValidationOptions(ValidationOptions $validationOptions = null): self + public function setValidationOptions(?ValidationOptions $validationOptions = null): self { $this->validationOptions = $validationOptions; return $this; diff --git a/src/Exception/ApiException.php b/src/Exception/ApiException.php index ae8d096..010c1ad 100644 --- a/src/Exception/ApiException.php +++ b/src/Exception/ApiException.php @@ -60,7 +60,7 @@ public function __construct( $errorCode, $message = null, $statusCode = null, - Exception $previous = null, + ?Exception $previous = null, $properties = null, $data = null, array $violations = [] diff --git a/src/Listener/LocaleListener.php b/src/Listener/LocaleListener.php index 241101f..7197f6d 100644 --- a/src/Listener/LocaleListener.php +++ b/src/Listener/LocaleListener.php @@ -13,6 +13,8 @@ */ class LocaleListener { + private const NO_LOCALE_MATCHED = ','; + private $requestHelper; private $locales; @@ -21,7 +23,7 @@ public function __construct( array $locales ) { $this->requestHelper = $requestHelper; - $this->locales = $locales; + $this->locales = array_values(array_diff(array_map('strval', $locales), [''])); } /** @@ -42,15 +44,10 @@ public function onKernelRequest($event) } } - private function resolveFromHeaders(Request $request) + private function resolveFromHeaders(Request $request): ?string { - $defaultLocale = 'default'; - $preferredLanguage = $request->getPreferredLanguage(array_merge([$defaultLocale], $this->locales)); - - if ($preferredLanguage !== null && $preferredLanguage !== $defaultLocale) { - return $preferredLanguage; - } + $locale = (string)$request->getPreferredLanguage(array_merge([self::NO_LOCALE_MATCHED], $this->locales)); - return null; + return $locale === self::NO_LOCALE_MATCHED ? null : $locale; } } diff --git a/src/Listener/RestExceptionListener.php b/src/Listener/RestExceptionListener.php index 3d71a9b..6b3e9e8 100644 --- a/src/Listener/RestExceptionListener.php +++ b/src/Listener/RestExceptionListener.php @@ -32,7 +32,7 @@ public function __construct( ErrorBuilderInterface $errorBuilder, CoreNormalizer $coreNormalizer, ResponseBuilder $responseBuilder, - LoggerInterface $logger = null + ?LoggerInterface $logger = null ) { $this->requestHelper = $requestHelper; $this->errorBuilder = $errorBuilder; diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml deleted file mode 100644 index cccb84d..0000000 --- a/src/Resources/config/services.xml +++ /dev/null @@ -1,105 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - invalid_request - 400 - Request content is invalid - - - invalid_parameters - 400 - Some required parameter is missing or it's format is invalid - - - invalid_state - 409 - Requested action cannot be made to the current state of resource - - - unauthorized - 401 - You have not provided any credentials or they are invalid - - - forbidden - 403 - You have no rights to access requested resource or make requested action - - - not_found - 404 - Resource was not found - - - internal_server_error - 500 - Unexpected internal system error - - - not_acceptable - 406 - Unknown request or response format - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/src/Resources/config/services/annotations.xml b/src/Resources/config/services/annotations.xml deleted file mode 100644 index 35b69eb..0000000 --- a/src/Resources/config/services/annotations.xml +++ /dev/null @@ -1,20 +0,0 @@ - - - - - - - - - - - - - - - diff --git a/src/Resources/config/services/attributes.xml b/src/Resources/config/services/attributes.xml deleted file mode 100644 index 8ecc323..0000000 --- a/src/Resources/config/services/attributes.xml +++ /dev/null @@ -1,23 +0,0 @@ - - - - - - - - - - - - - - - - - - diff --git a/src/Resources/config/services/listeners.xml b/src/Resources/config/services/listeners.xml deleted file mode 100644 index 2324df9..0000000 --- a/src/Resources/config/services/listeners.xml +++ /dev/null @@ -1,49 +0,0 @@ - - - - - - - - - - %paysera_api.locales% - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/src/Resources/config/services/normalizers.xml b/src/Resources/config/services/normalizers.xml deleted file mode 100644 index f2e00dd..0000000 --- a/src/Resources/config/services/normalizers.xml +++ /dev/null @@ -1,37 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - %paysera_api.pagination.default_total_count_strategy% - %paysera_api.pagination.maximum_offset% - - - - - %paysera_api.pagination.default_limit% - %paysera_api.pagination.maximum_limit% - - - diff --git a/src/Resources/config/services/pagination.xml b/src/Resources/config/services/pagination.xml deleted file mode 100644 index f4707d3..0000000 --- a/src/Resources/config/services/pagination.xml +++ /dev/null @@ -1,24 +0,0 @@ - - - - - - - - - - - - - - - - - - - - diff --git a/src/Service/RoutingLoader/DocblockAnnotationGuard.php b/src/Service/RoutingLoader/DocblockAnnotationGuard.php new file mode 100644 index 0000000..dfd37b2 --- /dev/null +++ b/src/Service/RoutingLoader/DocblockAnnotationGuard.php @@ -0,0 +1,282 @@ + + */ + private $findings; + + /** + * @var string + */ + private $tagPattern; + + public function __construct() + { + $this->findings = []; + $names = array_map(function (string $annotation): string { + return substr($annotation, strrpos($annotation, '\\') + 1); + }, self::ANNOTATIONS); + $this->tagPattern = sprintf( + '/(?getFiles($class, $method) as $file) { + $finding = $this->findAnnotationUse($class, $method, $file); + if ($finding !== null) { + throw new ConfigurationException(sprintf( + '%s::%s(): %s %s, but %s%s', + $class->getName(), + $method->getName(), + $file, + $finding, + self::NOT_READ, + $this->isTagOutsideAnnotationNamespace($finding) ? self::OTHER_LIBRARY : '' + )); + } + } + } + + /** + * @return ReflectionClass[] + */ + public function getActionScopes(ReflectionMethod $method): array + { + $declaringClass = $method->getDeclaringClass(); + $scopes = [$declaringClass]; + foreach ($this->getTraits($declaringClass) as $trait) { + if ($trait->getFileName() === $method->getFileName()) { + $scopes[] = $trait; + } + } + + return $scopes; + } + + /** + * @return array + */ + private function getFiles(ReflectionClass $class, ReflectionMethod $method): array + { + $declaringClass = $method->getDeclaringClass(); + $sources = array_merge( + [$method, $class, $declaringClass], + $this->getTraits($class), + $this->getTraits($declaringClass) + ); + $files = []; + foreach ($sources as $source) { + $files[] = $source->getFileName() ?: null; + } + + return array_unique($files); + } + + /** + * @return ReflectionClass[] + */ + private function getTraits(ReflectionClass $class): array + { + $traits = []; + foreach ($class->getTraits() as $trait) { + $traits[] = $trait; + $traits = array_merge($traits, $this->getTraits($trait)); + } + + return $traits; + } + + private function findAnnotationUse(ReflectionClass $class, ReflectionMethod $method, ?string $file): ?string + { + if ($file !== null && array_key_exists($file, $this->findings)) { + return $this->findings[$file]; + } + + $source = $file === null ? false : @file_get_contents($file); + if ($source === false) { + throw new ConfigurationException(sprintf( + '%s::%s(): %s cannot be read, so its docblocks cannot be checked for annotations of the bundle; %s', + $class->getName(), + $method->getName(), + $file === null ? 'the source of ' . $class->getName() : $file, + self::NOT_READ + )); + } + + $this->findings[$file] = $this->findAnnotationUseInSource($source); + + return $this->findings[$file]; + } + + private function findAnnotationUseInSource(string $source): ?string + { + $tokens = token_get_all($source); + foreach ($this->parseImports($tokens) as $import) { + $normalizedImport = strtolower(ltrim($import, '\\')); + if ( + $normalizedImport === self::ANNOTATION_NAMESPACE + || strpos($normalizedImport, self::ANNOTATION_NAMESPACE . '\\') === 0 + ) { + return 'imports ' . $import; + } + } + + foreach ($tokens as $token) { + if (is_array($token) && $token[0] === T_DOC_COMMENT) { + $tag = $this->findTag($token[1]); + if ($tag !== null) { + return $tag; + } + } + } + + return null; + } + + private function findTag(string $docComment): ?string + { + $joined = $this->joinNamesSplitAcrossLines($docComment); + $matched = $joined === null ? false : preg_match($this->tagPattern, $joined, $match); + if ($matched === false) { + return 'has a docblock that cannot be checked (preg error ' . preg_last_error() . ')'; + } + + return $matched === 1 ? self::TAG_FOUND . $match[1] : null; + } + + private function isTagOutsideAnnotationNamespace(string $finding): bool + { + if (strpos($finding, self::TAG_FOUND) !== 0) { + return false; + } + $name = ltrim(substr($finding, strlen(self::TAG_FOUND)), '\\'); + + return stripos($name, self::ANNOTATION_NAMESPACE . '\\') !== 0; + } + + private function joinNamesSplitAcrossLines(string $docComment): ?string + { + $joined = preg_replace(self::SPLIT_NAME_PATTERN . 'u', '\\\\', $docComment); + if ($joined === null && preg_last_error() === PREG_BAD_UTF8_ERROR) { + $joined = preg_replace(self::SPLIT_NAME_BYTE_PATTERN, '\\\\', $docComment); + } + + return $joined; + } + + /** + * @param array $tokens + * @return string[] + */ + private function parseImports(array $tokens): array + { + $imports = []; + $count = count($tokens); + for ($index = 0; $index < $count; $index++) { + if (!is_array($tokens[$index]) || $tokens[$index][0] !== T_USE) { + continue; + } + + $statement = ''; + while (++$index < $count) { + $token = $tokens[$index]; + if (is_array($token)) { + $isSpace = in_array($token[0], [T_WHITESPACE, T_COMMENT, T_DOC_COMMENT], true); + $statement .= $isSpace ? ' ' : $token[1]; + continue; + } + if ($token === ';') { + break; + } + if ($this->isClosureOrTraitBlockOpening($token, $statement)) { + $statement = ''; + break; + } + $statement .= $token; + } + + $imports = array_merge($imports, $this->expandImports($statement)); + } + + return $imports; + } + + private function isClosureOrTraitBlockOpening(string $token, string $statement): bool + { + return $token === '(' || ($token === '{' && substr(rtrim($statement), -1) !== '\\'); + } + + /** + * @return string[] + */ + private function expandImports(string $statement): array + { + $normalized = trim($statement); + if ($normalized === '') { + return []; + } + + $prefix = ''; + $names = $normalized; + $openingBrace = strpos($normalized, '{'); + if ($openingBrace !== false) { + $closingBrace = strrpos($normalized, '}'); + $prefix = trim(substr($normalized, 0, $openingBrace)); + $names = substr( + $normalized, + $openingBrace + 1, + ($closingBrace === false ? strlen($normalized) : $closingBrace) - $openingBrace - 1 + ); + } + + $imports = []; + foreach (explode(',', $names) as $name) { + $withoutAlias = preg_replace('/\s+as\s+\S+$/i', '', trim($name)); + $imports[] = str_replace(' ', '', $prefix . ($withoutAlias === null ? $name : $withoutAlias)); + } + + return $imports; + } +} diff --git a/src/Service/RoutingLoader/RestRouteConfigurator.php b/src/Service/RoutingLoader/RestRouteConfigurator.php new file mode 100644 index 0000000..8680f7d --- /dev/null +++ b/src/Service/RoutingLoader/RestRouteConfigurator.php @@ -0,0 +1,272 @@ + + */ + private $classAnnotations; + + public function __construct( + RestRequestHelper $restRequestHelper, + RestRequestAnnotationOptionsBuilder $annotationOptionsBuilder, + RestRequestAttributeOptionsBuilder $attributeOptionsBuilder, + DocblockAnnotationGuard $docblockAnnotationGuard + ) { + $this->restRequestHelper = $restRequestHelper; + $this->annotationOptionsBuilder = $annotationOptionsBuilder; + $this->attributeOptionsBuilder = $attributeOptionsBuilder; + $this->docblockAnnotationGuard = $docblockAnnotationGuard; + $this->classAnnotations = []; + } + + public function configureRoute( + Route $route, + ReflectionClass $class, + ReflectionMethod $method, + ?Reader $reader + ): void { + $attributes = $this->getRestAttributes($class, $method); + if ($reader === null) { + $this->docblockAnnotationGuard->failOnDocblockAnnotations($class, $method); + $annotations = []; + } else { + $annotations = $this->getRestAnnotations($class, $method, $reader); + } + + if ($annotations !== [] && $attributes !== []) { + throw new ConfigurationException(sprintf( + '%s::%s() is configured with both docblock annotations and attributes of the bundle, use one of them', + $class->getName(), + $method->getName() + )); + } + + if ($annotations !== []) { + $this->restRequestHelper->setOptionsForRoute( + $route, + $this->annotationOptionsBuilder->buildOptions($annotations, $method) + ); + } + + if ($attributes !== []) { + $this->restRequestHelper->setOptionsForRoute( + $route, + $this->attributeOptionsBuilder->buildOptions($attributes, $method) + ); + } + } + + /** + * @return RestAnnotationInterface[] + */ + private function getRestAnnotations(ReflectionClass $class, ReflectionMethod $method, Reader $reader): array + { + try { + $annotations = array_merge( + $this->getClassAnnotations($reader, $class), + $reader->getMethodAnnotations($method) + ); + } catch (Throwable $exception) { + throw $this->createReadException('docblock annotations', $class, $method, $exception); + } + + return array_values(array_filter($annotations, static function ($annotation): bool { + return $annotation instanceof RestAnnotationInterface; + })); + } + + /** + * @return object[] + */ + private function getClassAnnotations(Reader $reader, ReflectionClass $class): array + { + $className = $class->getName(); + if (!isset($this->classAnnotations[$className])) { + $this->classAnnotations[$className] = $reader->getClassAnnotations($class); + } + + return $this->classAnnotations[$className]; + } + + /** + * @return RestAttributeInterface[] + */ + private function getRestAttributes(ReflectionClass $class, ReflectionMethod $method): array + { + if (PHP_VERSION_ID < 80000) { + return []; + } + + $attributes = array_merge( + $class->getAttributes(RestAttributeInterface::class, ReflectionAttribute::IS_INSTANCEOF), + $method->getAttributes(RestAttributeInterface::class, ReflectionAttribute::IS_INSTANCEOF) + ); + $this->failOnUnknownBundleAttributes($class, $method); + + $restAttributes = []; + try { + foreach ($attributes as $attribute) { + $restAttributes[] = $attribute->newInstance(); + } + } catch (Throwable $exception) { + throw $this->createReadException('attributes', $class, $method, $exception); + } + + return $restAttributes; + } + + private function failOnUnknownBundleAttributes(ReflectionClass $class, ReflectionMethod $method): void + { + $this->failOnUnknownAttributes($class->getAttributes(), [$class], $class, $method); + $this->failOnUnknownAttributes( + $method->getAttributes(), + $this->docblockAnnotationGuard->getActionScopes($method), + $class, + $method + ); + } + + /** + * @param ReflectionAttribute[] $attributes + * @param ReflectionClass[] $scopes + */ + private function failOnUnknownAttributes( + array $attributes, + array $scopes, + ReflectionClass $class, + ReflectionMethod $method + ): void { + foreach ($attributes as $attribute) { + $name = $attribute->getName(); + if (stripos($name, self::BUNDLE_NAMESPACE) === 0 && !is_a($name, RestAttributeInterface::class, true)) { + throw new ConfigurationException(sprintf( + '%s::%s() uses %s, which is no attribute of the bundle', + $class->getName(), + $method->getName(), + $name + )); + } + + $bundleAttribute = $this->findBundleAttributeNamed($this->getShortName($name)); + if ($bundleAttribute !== null && $this->isInNamespaceOf($name, $scopes) && !class_exists($name)) { + throw new ConfigurationException(sprintf( + '%s::%s() uses %s, which is not a class that can be loaded;' + . ' if the bundle\'s attribute is meant, the name must resolve to %s', + $class->getName(), + $method->getName(), + $name, + $bundleAttribute + )); + } + } + } + + /** + * @param ReflectionClass[] $scopes + */ + private function isInNamespaceOf(string $name, array $scopes): bool + { + foreach ($scopes as $scope) { + $namespace = $scope->getNamespaceName(); + if ($namespace === '' ? strpos($name, '\\') === false : stripos($name, $namespace . '\\') === 0) { + return true; + } + } + + return false; + } + + private function findBundleAttributeNamed(string $shortName): ?string + { + foreach (self::BUNDLE_ATTRIBUTES as $bundleAttribute) { + if (strcasecmp($this->getShortName($bundleAttribute), $shortName) === 0) { + return $bundleAttribute; + } + } + + return null; + } + + private function getShortName(string $className): string + { + $position = strrpos($className, '\\'); + + return $position === false ? $className : substr($className, $position + 1); + } + + private function createReadException( + string $source, + ReflectionClass $class, + ReflectionMethod $method, + Throwable $exception + ): ConfigurationException { + return new ConfigurationException( + sprintf( + 'Cannot read the %s of %s::%s(): %s', + $source, + $class->getName(), + $method->getName(), + $exception->getMessage() + ), + 0, + $exception + ); + } +} diff --git a/src/Service/RoutingLoader/RoutingAnnotationLoader.php b/src/Service/RoutingLoader/RoutingAnnotationLoader.php index da85461..47971ad 100644 --- a/src/Service/RoutingLoader/RoutingAnnotationLoader.php +++ b/src/Service/RoutingLoader/RoutingAnnotationLoader.php @@ -3,8 +3,6 @@ namespace Paysera\Bundle\ApiBundle\Service\RoutingLoader; -use Paysera\Bundle\ApiBundle\Annotation\RestAnnotationInterface; -use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; use Symfony\Bundle\FrameworkBundle\Routing\AnnotatedRouteControllerLoader; use Symfony\Component\Routing\Route; use ReflectionClass; @@ -16,49 +14,19 @@ class RoutingAnnotationLoader extends AnnotatedRouteControllerLoader { /** - * @var RestRequestHelper + * @var RestRouteConfigurator */ - private $restRequestHelper; + private $restRouteConfigurator; - /** - * @var RestRequestAnnotationOptionsBuilder - */ - private $restRequestOptionsBuilder; - - public function setRequestHelper(RestRequestHelper $restRequestHelper) + public function setRestRouteConfigurator(RestRouteConfigurator $restRouteConfigurator) { - $this->restRequestHelper = $restRequestHelper; - } - - public function setRestRequestOptionsBuilder(RestRequestAnnotationOptionsBuilder $restRequestOptionsBuilder) - { - $this->restRequestOptionsBuilder = $restRequestOptionsBuilder; + $this->restRouteConfigurator = $restRouteConfigurator; } protected function configureRoute(Route $route, ReflectionClass $class, ReflectionMethod $method, $routeAnnotation) { parent::configureRoute($route, $class, $method, $routeAnnotation); - $annotations = []; - foreach ($this->reader->getClassAnnotations($class) as $annotation) { - if ($annotation instanceof RestAnnotationInterface) { - $annotations[] = $annotation; - } - } - - foreach ($this->reader->getMethodAnnotations($method) as $annotation) { - if ($annotation instanceof RestAnnotationInterface) { - $annotations[] = $annotation; - } - } - - if (count($annotations) === 0) { - return; - } - - $this->restRequestHelper->setOptionsForRoute( - $route, - $this->restRequestOptionsBuilder->buildOptions($annotations, $method) - ); + $this->restRouteConfigurator->configureRoute($route, $class, $method, $this->reader); } } diff --git a/src/Service/RoutingLoader/RoutingAttributeLoader.php b/src/Service/RoutingLoader/RoutingAttributeLoader.php index 955f27d..78afdcd 100644 --- a/src/Service/RoutingLoader/RoutingAttributeLoader.php +++ b/src/Service/RoutingLoader/RoutingAttributeLoader.php @@ -4,9 +4,6 @@ namespace Paysera\Bundle\ApiBundle\Service\RoutingLoader; -use Paysera\Bundle\ApiBundle\Annotation\RestAnnotationInterface; -use Paysera\Bundle\ApiBundle\Attribute\RestAttributeInterface; -use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; use ReflectionClass; use ReflectionMethod; use Symfony\Bundle\FrameworkBundle\Routing\AttributeRouteControllerLoader; @@ -18,33 +15,13 @@ class RoutingAttributeLoader extends AttributeRouteControllerLoader { /** - * @var RestRequestHelper + * @var RestRouteConfigurator */ - private $restRequestHelper; + private $restRouteConfigurator; - /** - * @var RestRequestAnnotationOptionsBuilder - */ - private $annotationOptionsBuilder; - - /** - * @var RestRequestAttributeOptionsBuilder - */ - private $attributeOptionsBuilder; - - public function setRequestHelper(RestRequestHelper $restRequestHelper) + public function setRestRouteConfigurator(RestRouteConfigurator $restRouteConfigurator) { - $this->restRequestHelper = $restRequestHelper; - } - - public function setRestRequestAnnotationOptionsBuilder(RestRequestAnnotationOptionsBuilder $annotationOptionsBuilder - ) { - $this->annotationOptionsBuilder = $annotationOptionsBuilder; - } - - public function setRestRequestAttributeOptionsBuilder(RestRequestAttributeOptionsBuilder $attributeOptionsBuilder) - { - $this->attributeOptionsBuilder = $attributeOptionsBuilder; + $this->restRouteConfigurator = $restRouteConfigurator; } protected function configureRoute( @@ -55,59 +32,6 @@ protected function configureRoute( ): void { parent::configureRoute($route, $class, $method, $annot); - $this->loadAnnotations($route, $class, $method); - $this->loadAttributes($route, $class, $method); - } - - private function loadAnnotations(Route $route, ReflectionClass $class, ReflectionMethod $method): void - { - if (!isset($this->reader)) { - return; - } - - $annotations = []; - foreach ($this->reader->getClassAnnotations($class) as $annotation) { - if ($annotation instanceof RestAnnotationInterface) { - $annotations[] = $annotation; - } - } - - foreach ($this->reader->getMethodAnnotations($method) as $annotation) { - if ($annotation instanceof RestAnnotationInterface) { - $annotations[] = $annotation; - } - } - - if ($annotations === []) { - return; - } - - $this->restRequestHelper->setOptionsForRoute( - $route, - $this->annotationOptionsBuilder->buildOptions($annotations, $method) - ); - } - - private function loadAttributes(Route $route, ReflectionClass $class, ReflectionMethod $method): void - { - $attributes = array_merge($class->getAttributes(), $method->getAttributes()); - - $restAttributes = []; - foreach ($attributes as $attribute) { - if (is_subclass_of($attribute->getName(), RestAttributeInterface::class)) { - /** @var RestAttributeInterface $instance */ - $instance = $attribute->newInstance(); - $restAttributes[] = $instance; - } - } - - if ($restAttributes === []) { - return; - } - - $this->restRequestHelper->setOptionsForRoute( - $route, - $this->attributeOptionsBuilder->buildOptions($restAttributes, $method) - ); + $this->restRouteConfigurator->configureRoute($route, $class, $method, $this->reader ?? null); } } diff --git a/src/Service/Validation/EntityValidator.php b/src/Service/Validation/EntityValidator.php index 830d215..4335bae 100644 --- a/src/Service/Validation/EntityValidator.php +++ b/src/Service/Validation/EntityValidator.php @@ -26,8 +26,8 @@ class EntityValidator protected $propertyPathConverter; public function __construct( - ValidatorInterface $validator = null, - PropertyPathConverterInterface $propertyPathConverter = null + ?ValidatorInterface $validator = null, + ?PropertyPathConverterInterface $propertyPathConverter = null ) { $this->validator = $validator; $this->propertyPathConverter = $propertyPathConverter; diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedClassRequiredPermissionsController.php b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedClassRequiredPermissionsController.php index f93ebd4..9d9cd4a 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedClassRequiredPermissionsController.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedClassRequiredPermissionsController.php @@ -6,7 +6,7 @@ use Paysera\Bundle\ApiBundle\Attribute\RequiredPermissions; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Routing\Annotation\Route; +use Symfony\Component\Routing\Attribute\Route; #[RequiredPermissions(permissions: ['ROLE_USER'])] class AttributedClassRequiredPermissionsController diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedClassValidationController.php b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedClassValidationController.php index 63d74dd..b60d7d0 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedClassValidationController.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedClassValidationController.php @@ -8,7 +8,7 @@ use Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Entity\MyObject; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Routing\Annotation\Route; +use Symfony\Component\Routing\Attribute\Route; #[Validation(groups: ['internal_field1_email'], violationPathMap: ['internalField1' => 'internal.field1'])] class AttributedClassValidationController diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedController.php b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedController.php index 6544ab7..8b0f6c7 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedController.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedController.php @@ -14,7 +14,7 @@ use Paysera\Pagination\Entity\Pager; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Routing\Annotation\Route; +use Symfony\Component\Routing\Attribute\Route; class AttributedController { diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPagedQueryController.php b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPagedQueryController.php new file mode 100644 index 0000000..f0e8df2 --- /dev/null +++ b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPagedQueryController.php @@ -0,0 +1,34 @@ +entityManager = $entityManager; + } + + #[Route(path: '/attributed/paged-query/simple', methods: ['GET'])] + #[Query(parameterName: 'pager')] + #[Query(parameterName: 'filter')] + public function findSimplePersistedEntities(Pager $pager, PersistedEntityFilter $filter): PagedQuery + { + /** @var PersistedEntityRepository $repository */ + $repository = $this->entityManager->getRepository(PersistedEntity::class); + $configuredQuery = $repository->buildConfiguredQuery($filter); + return new PagedQuery($configuredQuery, $pager); + } +} diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPersistedEntityController.php b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPersistedEntityController.php new file mode 100644 index 0000000..6fa98cb --- /dev/null +++ b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Attribute/AttributedPersistedEntityController.php @@ -0,0 +1,27 @@ +getId()); + } + + #[Route(path: '/attributed/simple-persisted-entities/{identifier}', methods: ['GET'])] + #[PathAttribute(parameterName: 'entity', pathPartName: 'identifier')] + public function findSimplePersistedEntity(SimplePersistedEntity $entity): Response + { + return new Response((string)$entity->getId()); + } +} diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Controller/Docblock/AttributedDocblockOptionsController.php b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Docblock/AttributedDocblockOptionsController.php new file mode 100644 index 0000000..cf8ff92 --- /dev/null +++ b/tests/Functional/Fixtures/FixtureTestBundle/Controller/Docblock/AttributedDocblockOptionsController.php @@ -0,0 +1,22 @@ + + diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/PersistedEntity.orm.xml b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/PersistedEntity.orm.xml new file mode 100644 index 0000000..9587ae8 --- /dev/null +++ b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/PersistedEntity.orm.xml @@ -0,0 +1,12 @@ + + + + + + + + + diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/SimplePersistedEntity.orm.xml b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/SimplePersistedEntity.orm.xml new file mode 100644 index 0000000..e3b92e0 --- /dev/null +++ b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/doctrine/SimplePersistedEntity.orm.xml @@ -0,0 +1,10 @@ + + + + + + + + diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/explicit_routing.yml b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/explicit_routing.yml new file mode 100644 index 0000000..58be1d0 --- /dev/null +++ b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/explicit_routing.yml @@ -0,0 +1,43 @@ +paysera_fixture_test_1: + path: /config/by-full-controller + controller: Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action1Action + +paysera_fixture_test_1b: + path: /config/by-full-controller-and-class-name + controller: Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action1bAction + +paysera_fixture_test_2: + path: /config/by-class-name + controller: Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action2 + +paysera_fixture_test_3: + path: /config/by-service-name + controller: paysera_fixture_test.controller.default_controller::action3 + +paysera_fixture_test_4: + path: /config/routing-by-service-name-config-by-class-name + controller: paysera_fixture_test.controller.default_controller::action4 + +paysera_fixture_test_5: + path: /config/routing-by-class-name-config-by-service-name + controller: Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action5 + +paysera_fixture_test: + path: / + controller: paysera_fixture_test.controller.default_controller::action + +paysera_fixture_test_with_return: + path: /with-return + controller: paysera_fixture_test.controller.default_controller::actionWithReturn + +paysera_fixture_test_with_url_parameter: + path: /with-url-parameter/{urlParameter} + controller: Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action + +paysera_fixture_test_with_several_url_parameters: + path: /with-url-parameters/{urlParameter1}/{urlParameter2} + controller: paysera_fixture_test.controller.default_controller::actionWithMultipleParameters + +paysera_fixture_test_with_several_parameters: + path: /with-parameters + controller: paysera_fixture_test.controller.default_controller::actionWithMultipleParameters diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/routing.xml b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/routing.xml index 3a06c43..7b0b7dc 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/routing.xml +++ b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/routing.xml @@ -11,46 +11,5 @@ - - Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action1Action - - - - Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action1bAction - - - - - Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action2 - - - - paysera_fixture_test.controller.default_controller::action3 - - - paysera_fixture_test.controller.default_controller::action4 - - - - Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action5 - - - - - paysera_fixture_test.controller.default_controller::action - - - paysera_fixture_test.controller.default_controller::actionWithReturn - - - - Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\DefaultController::action - - - - paysera_fixture_test.controller.default_controller::actionWithMultipleParameters - - - paysera_fixture_test.controller.default_controller::actionWithMultipleParameters - + diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/services.xml b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/services.xml index 46836bd..24833de 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/services.xml +++ b/tests/Functional/Fixtures/FixtureTestBundle/Resources/config/services.xml @@ -92,6 +92,10 @@ public="true"> + + + diff --git a/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php b/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php index a3133ff..e092728 100644 --- a/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php +++ b/tests/Functional/Fixtures/FixtureTestBundle/Service/TestHelper.php @@ -5,6 +5,7 @@ namespace Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Service; use Symfony\Bundle\FrameworkBundle\Routing\AttributeRouteControllerLoader; +use Symfony\Component\Routing\Loader\AttributeClassLoader; class TestHelper { @@ -12,4 +13,9 @@ public static function phpAttributeSupportExists(): bool { return class_exists(AttributeRouteControllerLoader::class); } + + public static function docblockRoutingSupportExists(): bool + { + return !class_exists(AttributeClassLoader::class) || property_exists(AttributeClassLoader::class, 'reader'); + } } diff --git a/tests/Functional/Fixtures/TestKernel.php b/tests/Functional/Fixtures/TestKernel.php index 9fc2a51..e0810d4 100644 --- a/tests/Functional/Fixtures/TestKernel.php +++ b/tests/Functional/Fixtures/TestKernel.php @@ -43,7 +43,7 @@ public function registerContainerConfiguration(LoaderInterface $loader) $loader->load(__DIR__ . '/config/' . $this->configFile); $loader->load(__DIR__ . '/config/' . $this->commonFile); - if (TestHelper::phpAttributeSupportExists()) { + if (TestHelper::phpAttributeSupportExists() && TestHelper::docblockRoutingSupportExists()) { $loader->load(__DIR__ . '/config/attributed_common.yml'); } } diff --git a/tests/Functional/Fixtures/config/sf7_common.yml b/tests/Functional/Fixtures/config/sf7_common.yml new file mode 100644 index 0000000..aa76955 --- /dev/null +++ b/tests/Functional/Fixtures/config/sf7_common.yml @@ -0,0 +1,19 @@ +framework: + router: + resource: '%kernel.project_dir%/tests/Functional/Fixtures/config/sf7_routing.yml' + validation: + email_validation_mode: html5 + property_info: + with_constructor_extractor: false + +security: + firewalls: + config: + pattern: ^/(config)/ + security: false + main: + http_basic: ~ + stateless: true + password_hashers: + Symfony\Component\Security\Core\User\InMemoryUser: + algorithm: plaintext diff --git a/tests/Functional/Fixtures/config/sf7_docblock_common.yml b/tests/Functional/Fixtures/config/sf7_docblock_common.yml new file mode 100644 index 0000000..62b028d --- /dev/null +++ b/tests/Functional/Fixtures/config/sf7_docblock_common.yml @@ -0,0 +1,6 @@ +imports: + - { resource: sf7_common.yml } + +framework: + router: + resource: '%kernel.project_dir%/tests/Functional/Fixtures/config/sf7_docblock_routing.yml' diff --git a/tests/Functional/Fixtures/config/sf7_docblock_routing.yml b/tests/Functional/Fixtures/config/sf7_docblock_routing.yml new file mode 100644 index 0000000..6eddc93 --- /dev/null +++ b/tests/Functional/Fixtures/config/sf7_docblock_routing.yml @@ -0,0 +1,13 @@ +paysera_fixture_test: + resource: "@PayseraFixtureTestBundle/Resources/config/explicit_routing.yml" + prefix: / + +paysera_fixture_attributed_test: + resource: '@PayseraFixtureTestBundle/Controller/Attribute/' + type: attribute + prefix: / + +paysera_fixture_docblock_options_test: + resource: '@PayseraFixtureTestBundle/Controller/Docblock/' + type: attribute + prefix: / diff --git a/tests/Functional/Fixtures/config/sf7_routing.yml b/tests/Functional/Fixtures/config/sf7_routing.yml new file mode 100644 index 0000000..6146637 --- /dev/null +++ b/tests/Functional/Fixtures/config/sf7_routing.yml @@ -0,0 +1,8 @@ +paysera_fixture_test: + resource: "@PayseraFixtureTestBundle/Resources/config/explicit_routing.yml" + prefix: / + +paysera_fixture_attributed_test: + resource: '@PayseraFixtureTestBundle/Controller/Attribute/' + type: attribute + prefix: / diff --git a/tests/Functional/FunctionalAnnotationsTest.php b/tests/Functional/FunctionalAnnotationsTest.php index 41b6f7f..803969f 100644 --- a/tests/Functional/FunctionalAnnotationsTest.php +++ b/tests/Functional/FunctionalAnnotationsTest.php @@ -3,9 +3,13 @@ namespace Paysera\Bundle\ApiBundle\Tests\Functional; +use Paysera\Bundle\ApiBundle\Exception\ConfigurationException; +use Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Controller\Docblock; use Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Service\TestHelper; +use ReflectionClass; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Throwable; class FunctionalAnnotationsTest extends FunctionalTestCase { @@ -24,7 +28,7 @@ protected function setUp(): void public function testAnnotatedRestRequestConfiguration( Response $expectedResponse, Request $request, - Response $extraResponseVersion = null + ?Response $extraResponseVersion = null ) { $this->makeTest('annotated', $expectedResponse, $request, $extraResponseVersion); } @@ -38,20 +42,86 @@ public function testAnnotatedRestRequestConfiguration( public function testAttributedRestRequestConfiguration( Response $expectedResponse, Request $request, - Response $extraResponseVersion = null + ?Response $extraResponseVersion = null ) { $this->makeTest('attributed', $expectedResponse, $request, $extraResponseVersion); } + /** + * @dataProvider docblockOptionsOnAttributeRouteProvider + */ + public function testDocblockOptionsOnAttributeRoute( + int $expectedStatusCode, + string $expectedContent, + ?string $username + ): void { + if (!TestHelper::docblockRoutingSupportExists()) { + $this->markTestSkipped('Symfony 7 passes no annotation reader: see the refusal test'); + } + + $this->makeTest( + 'attributed', + new Response($expectedContent, $expectedStatusCode), + $this->createRequest('GET', '/docblock/testRequiredPermissions', null, [], $username) + ); + } + + public static function docblockOptionsOnAttributeRouteProvider(): array + { + return [ + 'without auth' => [ + 401, + '{"error":"unauthorized","error_description":"This API endpoint requires authentication, none found"}', + null, + ], + 'without enough permissions' => [ + 403, + '{"error":"forbidden",' + . '"error_description":"Access to this API endpoint is forbidden for current client"}', + 'user', + ], + 'with all needed permissions' => [200, 'OK', 'admin'], + ]; + } + + public function testDocblockOptionsOnAttributeRouteAreRefusedWithoutReader(): void + { + if (!TestHelper::phpAttributeSupportExists() || TestHelper::docblockRoutingSupportExists()) { + $this->markTestSkipped('Symfony before 7 passes an annotation reader to the route loader'); + } + $this->tearDown(); + $container = $this->setUpContainer('basic', 'docblock_common.yml'); + + $cause = null; + try { + $container->get('router')->getRouteCollection(); + } catch (Throwable $exception) { + $cause = $exception; + while ($cause !== null && !$cause instanceof ConfigurationException) { + $cause = $cause->getPrevious(); + } + $this->assertNotNull($cause, get_class($exception) . ': ' . $exception->getMessage()); + } + + $this->assertNotNull($cause, 'Expected the route loading to fail'); + $this->assertStringContainsString( + sprintf( + '%s::testRequiredPermissions(): %s imports Paysera\Bundle\ApiBundle\Annotation\RequiredPermissions,' + . ' but docblock annotations of the bundle are not read here', + Docblock\AttributedDocblockOptionsController::class, + (new ReflectionClass(Docblock\AttributedDocblockOptionsController::class))->getFileName() + ), + $cause->getMessage() + ); + } + private function makeTest( string $pathPrefix, Response $expectedResponse, Request $request, - Response $extraResponseVersion = null + ?Response $extraResponseVersion = null ): void { - if ($pathPrefix === 'attributed' && !TestHelper::phpAttributeSupportExists()) { - $this->markTestSkipped('Unsupported environment'); - } + $this->skipUnlessRoutesAreLoaded($pathPrefix); $request->server->set( 'REQUEST_URI', @@ -177,6 +247,29 @@ public function restRequestsConfigurationProvider(): array ['Content-Type' => 'text/plain'] ), ], + 'testBodyNormalizationWithCustomContentType and no content type' => [ + new Response( + '{"error":"invalid_request","error_description":"Content-Type must be provided"}', + 400 + ), + $this->createRequest( + 'POST', + '/testBodyNormalizationWithCustomContentType', + 'my_text' + ), + ], + 'testBodyNormalizationWithExtractedKeyValue and a body that is not JSON' => [ + new Response( + '{"error":"invalid_request","error_description":"Cannot decode request body to JSON"}', + 400 + ), + $this->createRequest( + 'POST', + '/testBodyNormalizationWithExtractedKeyValue', + '{"key": ', + ['Content-Type' => 'application/json'] + ), + ], 'testBodyNormalizationWithCustomContentTypeAndJsonDecode and JSON content-type' => [ new Response( '{"error":"invalid_request","error_description":"This Content-Type (application/json) is not supported"}', diff --git a/tests/Functional/FunctionalFindDenormalizersTest.php b/tests/Functional/FunctionalFindDenormalizersTest.php index 56ed6ec..ae4d755 100644 --- a/tests/Functional/FunctionalFindDenormalizersTest.php +++ b/tests/Functional/FunctionalFindDenormalizersTest.php @@ -15,9 +15,14 @@ protected function setUp(): void $this->setUpDatabase(); } - public function testRestRequestWithCustomPathConverter() + /** + * @dataProvider routesProvider + */ + public function testRestRequestWithCustomPathConverter(string $routes, string $pathPrefix) { - $response = $this->makeGetRequest('/persisted-entities/someField1'); + $this->skipUnlessRoutesAreLoaded($routes); + + $response = $this->makeGetRequest($pathPrefix . '/persisted-entities/someField1'); $this->assertEquals(404, $response->getStatusCode()); $manager = $this->getEntityManager(); @@ -25,15 +30,23 @@ public function testRestRequestWithCustomPathConverter() $manager->persist((new SimplePersistedEntity())->setId(420)); $manager->flush(); - $response = $this->makeGetRequest('/persisted-entities/someField1'); + $response = $this->makeGetRequest($pathPrefix . '/persisted-entities/someField1'); $this->assertEquals(200, $response->getStatusCode()); $this->assertEquals('42', $response->getContent()); - $response = $this->makeGetRequest('/simple-persisted-entities/1'); + $response = $this->makeGetRequest($pathPrefix . '/simple-persisted-entities/1'); $this->assertEquals(404, $response->getStatusCode()); - $response = $this->makeGetRequest('/simple-persisted-entities/420'); + $response = $this->makeGetRequest($pathPrefix . '/simple-persisted-entities/420'); $this->assertEquals(200, $response->getStatusCode()); $this->assertEquals('420', $response->getContent()); } + + public static function routesProvider(): array + { + return [ + 'annotated' => ['annotated', ''], + 'attributed' => ['attributed', '/attributed'], + ]; + } } diff --git a/tests/Functional/FunctionalPagedQueryTest.php b/tests/Functional/FunctionalPagedQueryTest.php index 86983a2..0eef291 100644 --- a/tests/Functional/FunctionalPagedQueryTest.php +++ b/tests/Functional/FunctionalPagedQueryTest.php @@ -19,6 +19,24 @@ class FunctionalPagedQueryTest extends FunctionalTestCase */ public function testRequestWithPagedQuery(Response $expectedResponse, Request $request, string $testCase = 'basic') { + $this->makeTest('annotated', $expectedResponse, $request, $testCase); + } + + /** + * @dataProvider requestWithPagedQueryProvider + */ + public function testAttributedRequestWithPagedQuery( + Response $expectedResponse, + Request $request, + string $testCase = 'basic' + ) { + $request->server->set('REQUEST_URI', '/attributed' . $request->server->get('REQUEST_URI')); + $this->makeTest('attributed', $expectedResponse, $request, $testCase); + } + + private function makeTest(string $routes, Response $expectedResponse, Request $request, string $testCase): void + { + $this->skipUnlessRoutesAreLoaded($routes); $this->setUpFor($testCase); $response = $this->handleRequest($request); $this->assertEquals( diff --git a/tests/Functional/FunctionalTestCase.php b/tests/Functional/FunctionalTestCase.php index 0a1f80e..675daf2 100644 --- a/tests/Functional/FunctionalTestCase.php +++ b/tests/Functional/FunctionalTestCase.php @@ -5,9 +5,9 @@ use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Tools\SchemaTool; +use Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\FixtureTestBundle\Service\TestHelper; use Paysera\Bundle\ApiBundle\Tests\Functional\Fixtures\TestKernel; use PHPUnit\Framework\TestCase; -use Symfony\Bundle\FrameworkBundle\Routing\AttributeRouteControllerLoader; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\ResettableContainerInterface; use Symfony\Component\HttpKernel\Kernel; @@ -20,7 +20,7 @@ abstract class FunctionalTestCase extends TestCase { /** - * @var TestKernel + * @var TestKernel|null */ protected $kernel; @@ -31,14 +31,33 @@ abstract class FunctionalTestCase extends TestCase */ protected function setUpContainer($testCase, $commonFile = 'common.yml') { - $prefix = Kernel::MAJOR_VERSION <= 4 ? 'legacy_' : ''; + $prefix = ''; + if (Kernel::MAJOR_VERSION <= 4) { + $prefix = 'legacy_'; + } elseif (!TestHelper::docblockRoutingSupportExists()) { + $prefix = 'sf7_'; + } $this->kernel = new TestKernel($testCase, $prefix . $commonFile); $this->kernel->boot(); return $this->kernel->getContainer(); } + protected function skipUnlessRoutesAreLoaded(string $routes): void + { + if ($routes === 'attributed' && !TestHelper::phpAttributeSupportExists()) { + $this->markTestSkipped('Unsupported environment'); + } + if ($routes === 'annotated' && !TestHelper::docblockRoutingSupportExists()) { + $this->markTestSkipped('Symfony 7 reads no @Route docblocks'); + } + } + protected function tearDown(): void { + if ($this->kernel === null) { + return; + } + $container = $this->kernel->getContainer(); $this->kernel->shutdown(); if ($container instanceof ResettableContainerInterface || $container instanceof ResetInterface) { @@ -57,9 +76,9 @@ protected function makeGetRequest(string $uri): Response protected function createRequest( string $method, string $uri, - string $content = null, + ?string $content = null, array $headers = [], - string $username = null + ?string $username = null ): Request { $parts = parse_url($uri); parse_str($parts['query'] ?? '', $query); diff --git a/tests/Unit/DependencyInjection/Fixtures/services.json b/tests/Unit/DependencyInjection/Fixtures/services.json new file mode 100644 index 0000000..86fc973 --- /dev/null +++ b/tests/Unit/DependencyInjection/Fixtures/services.json @@ -0,0 +1,409 @@ +{ + "paysera_api.annotations.loader": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\RoutingLoader\\RoutingAnnotationLoader", + "parent": "routing.loader.annotation", + "calls": [ + [ + "setRestRouteConfigurator", + [ + { + "reference": "paysera_api.rest_route_configurator", + "invalid_behavior": 1 + } + ] + ] + ], + "decorated": [ + "routing.loader.annotation", + null, + 0 + ] + }, + "paysera_api.annotations.options_builder": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\RoutingLoader\\RestRequestAnnotationOptionsBuilder", + "arguments": [ + { + "reference": "paysera_api.rest_request_options_validator", + "invalid_behavior": 1 + } + ] + }, + "paysera_api.attributes.options_builder": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\RoutingLoader\\RestRequestAttributeOptionsBuilder", + "arguments": [ + { + "reference": "paysera_api.rest_request_options_validator", + "invalid_behavior": 1 + } + ] + }, + "paysera_api.camel_case_to_snake_case_converter": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\Validation\\CamelCaseToSnakeCaseConverter" + }, + "paysera_api.content_type_matcher": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\ContentTypeMatcher" + }, + "paysera_api.docblock_annotation_guard": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\RoutingLoader\\DocblockAnnotationGuard" + }, + "paysera_api.entity_validator": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\Validation\\EntityValidator", + "arguments": [ + { + "reference": "validator", + "invalid_behavior": 2 + }, + { + "reference": "paysera_api.validation.property_path_converter", + "invalid_behavior": 1 + } + ] + }, + "paysera_api.error_builder": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\ErrorBuilder", + "calls": [ + [ + "configureError", + [ + "invalid_request", + 400, + "Request content is invalid" + ] + ], + [ + "configureError", + [ + "invalid_parameters", + 400, + "Some required parameter is missing or it's format is invalid" + ] + ], + [ + "configureError", + [ + "invalid_state", + 409, + "Requested action cannot be made to the current state of resource" + ] + ], + [ + "configureError", + [ + "unauthorized", + 401, + "You have not provided any credentials or they are invalid" + ] + ], + [ + "configureError", + [ + "forbidden", + 403, + "You have no rights to access requested resource or make requested action" + ] + ], + [ + "configureError", + [ + "not_found", + 404, + "Resource was not found" + ] + ], + [ + "configureError", + [ + "internal_server_error", + 500, + "Unexpected internal system error" + ] + ], + [ + "configureError", + [ + "not_acceptable", + 406, + "Unknown request or response format" + ] + ] + ] + }, + "paysera_api.listener.locale": { + "class": "Paysera\\Bundle\\ApiBundle\\Listener\\LocaleListener", + "arguments": [ + { + "reference": "paysera_api.rest_request_helper", + "invalid_behavior": 1 + }, + "%paysera_api.locales%" + ], + "tags": { + "kernel.event_listener": [ + { + "event": "kernel.request", + "priority": 20 + } + ] + } + }, + "paysera_api.listener.rest_exception": { + "class": "Paysera\\Bundle\\ApiBundle\\Listener\\RestExceptionListener", + "arguments": [ + { + "reference": "paysera_api.rest_request_helper", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.error_builder", + "invalid_behavior": 1 + }, + { + "reference": "paysera_normalization.core_normalizer", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.response_builder", + "invalid_behavior": 1 + }, + { + "reference": "logger", + "invalid_behavior": 2 + } + ], + "tags": { + "kernel.event_listener": [ + { + "event": "kernel.exception", + "priority": 1 + } + ] + } + }, + "paysera_api.listener.rest_request": { + "class": "Paysera\\Bundle\\ApiBundle\\Listener\\RestRequestListener", + "arguments": [ + { + "reference": "paysera_normalization.core_denormalizer", + "invalid_behavior": 1 + }, + { + "reference": "security.authorization_checker", + "invalid_behavior": 1 + }, + { + "reference": "security.token_storage", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.rest_request_helper", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.entity_validator", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.content_type_matcher", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.path_attribute_resolution_manager", + "invalid_behavior": 1 + } + ], + "tags": { + "kernel.event_listener": [ + { + "event": "kernel.controller" + }, + { + "event": "kernel.request", + "priority": 31 + } + ] + } + }, + "paysera_api.listener.rest_response": { + "class": "Paysera\\Bundle\\ApiBundle\\Listener\\RestResponseListener", + "arguments": [ + { + "reference": "paysera_normalization.core_normalizer", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.rest_request_helper", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.response_builder", + "invalid_behavior": 1 + } + ], + "tags": { + "kernel.event_listener": [ + { + "event": "kernel.view" + } + ] + } + }, + "paysera_api.loader.attribute": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\RoutingLoader\\RoutingAttributeLoader", + "parent": "routing.loader.attribute", + "calls": [ + [ + "setRestRouteConfigurator", + [ + { + "reference": "paysera_api.rest_route_configurator", + "invalid_behavior": 1 + } + ] + ] + ], + "decorated": [ + "routing.loader.attribute", + null, + 0 + ] + }, + "paysera_api.normalizer.error": { + "class": "Paysera\\Bundle\\ApiBundle\\Normalizer\\ErrorNormalizer", + "tags": { + "paysera_normalization.autoconfigured_normalizer": [ + [] + ] + } + }, + "paysera_api.normalizer.paged_query": { + "class": "Paysera\\Bundle\\ApiBundle\\Normalizer\\Pagination\\PagedQueryNormalizer", + "arguments": [ + { + "reference": "paysera_api.pagination.result_provider", + "invalid_behavior": 1 + }, + "%paysera_api.pagination.default_total_count_strategy%", + "%paysera_api.pagination.maximum_offset%" + ], + "tags": { + "paysera_normalization.autoconfigured_normalizer": [ + [] + ] + } + }, + "paysera_api.normalizer.pager_denormalizer": { + "class": "Paysera\\Bundle\\ApiBundle\\Normalizer\\Pagination\\PagerDenormalizer", + "arguments": [ + "%paysera_api.pagination.default_limit%", + "%paysera_api.pagination.maximum_limit%" + ], + "tags": { + "paysera_normalization.autoconfigured_normalizer": [ + [] + ] + } + }, + "paysera_api.normalizer.result": { + "class": "Paysera\\Bundle\\ApiBundle\\Normalizer\\Pagination\\ResultNormalizer", + "tags": { + "paysera_normalization.autoconfigured_normalizer": [ + [] + ] + } + }, + "paysera_api.normalizer.violation": { + "class": "Paysera\\Bundle\\ApiBundle\\Normalizer\\ViolationNormalizer", + "tags": { + "paysera_normalization.autoconfigured_normalizer": [ + [] + ] + } + }, + "paysera_api.pagination.result_provider": { + "class": "Paysera\\Pagination\\Service\\Doctrine\\ResultProvider", + "arguments": [ + { + "inline": { + "class": "Paysera\\Pagination\\Service\\Doctrine\\QueryAnalyser" + } + }, + { + "inline": { + "class": "Paysera\\Pagination\\Service\\CursorBuilder", + "arguments": [ + { + "inline": { + "class": "Symfony\\Component\\PropertyAccess\\PropertyAccessor", + "factory": [ + "Symfony\\Component\\PropertyAccess\\PropertyAccess", + "createPropertyAccessor" + ] + } + } + ] + } + } + ] + }, + "paysera_api.path_attribute_resolution_manager": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\PathAttributeResolver\\PathAttributeResolutionManager", + "arguments": [ + { + "reference": "paysera_api.path_attribute_resolver_registry", + "invalid_behavior": 1 + } + ] + }, + "paysera_api.path_attribute_resolver_registry": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\PathAttributeResolver\\PathAttributeResolverRegistry" + }, + "paysera_api.response_builder": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\ResponseBuilder" + }, + "paysera_api.rest_request_helper": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\RestRequestHelper", + "arguments": [ + { + "reference": "paysera_api.rest_request_options_registry", + "invalid_behavior": 1 + } + ] + }, + "paysera_api.rest_request_options_registry": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\RestRequestOptionsRegistry" + }, + "paysera_api.rest_request_options_validator": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\RestRequestOptionsValidator", + "arguments": [ + { + "reference": "paysera_normalization.normalizer_registry", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.path_attribute_resolver_registry", + "invalid_behavior": 1 + } + ] + }, + "paysera_api.rest_route_configurator": { + "class": "Paysera\\Bundle\\ApiBundle\\Service\\RoutingLoader\\RestRouteConfigurator", + "arguments": [ + { + "reference": "paysera_api.rest_request_helper", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.annotations.options_builder", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.attributes.options_builder", + "invalid_behavior": 1 + }, + { + "reference": "paysera_api.docblock_annotation_guard", + "invalid_behavior": 1 + } + ] + } +} diff --git a/tests/Unit/DependencyInjection/PayseraApiExtensionTest.php b/tests/Unit/DependencyInjection/PayseraApiExtensionTest.php new file mode 100644 index 0000000..bcb7a11 --- /dev/null +++ b/tests/Unit/DependencyInjection/PayseraApiExtensionTest.php @@ -0,0 +1,166 @@ +> $configs + * @param array $expected + */ + public function testLoad(array $configs, array $expected): void + { + $container = new ContainerBuilder(); + $empty = new ContainerBuilder(); + + (new PayseraApiExtension())->load($configs, $container); + + $definitions = array_diff_key($container->getDefinitions(), $empty->getDefinitions()); + ksort($definitions); + $aliases = array_diff_key($container->getAliases(), $empty->getAliases()); + ksort($aliases); + $this->assertSame($expected, [ + 'definitions' => array_map( + function (Definition $definition): array { + return $this->dumpDefinition($definition, false); + }, + $definitions + ), + 'aliases' => array_map( + static function (Alias $alias): array { + return ['id' => (string)$alias, 'public' => $alias->isPublic()]; + }, + $aliases + ), + 'parameters' => $container->getParameterBag()->all(), + ]); + } + + public static function loadDataProvider(): array + { + $definitions = json_decode((string)file_get_contents(__DIR__ . '/Fixtures/services.json'), true); + unset($definitions[ + class_exists(AttributeRouteControllerLoader::class) + ? 'paysera_api.annotations.loader' + : 'paysera_api.loader.attribute' + ]); + $withoutLocaleListener = $definitions; + unset($withoutLocaleListener['paysera_api.listener.locale']); + $pagination = [ + 'paysera_api.pagination.default_total_count_strategy' => 'optional', + 'paysera_api.pagination.maximum_offset' => 1000, + 'paysera_api.pagination.default_limit' => 100, + 'paysera_api.pagination.maximum_limit' => 1000, + ]; + + return [ + 'no configuration' => [ + [], + [ + 'definitions' => $withoutLocaleListener, + 'aliases' => [ + 'paysera_api.validation.property_path_converter' => [ + 'id' => 'paysera_api.camel_case_to_snake_case_converter', + 'public' => false, + ], + ], + 'parameters' => ['paysera_api.locales' => []] + $pagination, + ], + ], + 'locales and a property path converter' => [ + [['locales' => ['en', 'lt'], 'validation' => ['property_path_converter' => 'app.converter']]], + [ + 'definitions' => $definitions, + 'aliases' => [ + 'paysera_api.validation.property_path_converter' => [ + 'id' => 'app.converter', + 'public' => (new Alias('app.converter'))->isPublic(), + ], + ], + 'parameters' => ['paysera_api.locales' => ['en', 'lt']] + $pagination, + ], + ], + ]; + } + + public function testLoadingTriggersNoDeprecation(): void + { + $deprecations = []; + set_error_handler(static function (int $level, string $message) use (&$deprecations): bool { + $deprecations[] = $message; + + return true; + }, E_USER_DEPRECATED); + try { + (new PayseraApiExtension())->load([], new ContainerBuilder()); + } finally { + restore_error_handler(); + } + + $this->assertSame([], $deprecations); + } + + /** + * @return array + */ + private function dumpDefinition(Definition $definition, bool $inline): array + { + return array_filter( + [ + 'class' => $definition->getClass(), + 'parent' => $definition instanceof ChildDefinition ? $definition->getParent() : null, + 'public' => $inline || $definition->isPublic() === (new Definition())->isPublic() + ? null + : $definition->isPublic(), + 'shared' => $definition->isShared() ? null : false, + 'lazy' => $definition->isLazy() ?: null, + 'abstract' => $definition->isAbstract() ?: null, + 'synthetic' => $definition->isSynthetic() ?: null, + 'autowired' => $definition->isAutowired() ?: null, + 'autoconfigured' => $definition->isAutoconfigured() ?: null, + 'deprecated' => $definition->isDeprecated() ?: null, + 'factory' => $this->export($definition->getFactory()), + 'arguments' => $this->export($definition->getArguments()), + 'calls' => $this->export($definition->getMethodCalls()), + 'properties' => $this->export($definition->getProperties()), + 'configurator' => $this->export($definition->getConfigurator()), + 'tags' => $definition->getTags(), + 'decorated' => $definition->getDecoratedService(), + ], + static function ($value): bool { + return $value !== null && $value !== []; + } + ); + } + + /** + * @param mixed $value + * @return mixed + */ + private function export($value) + { + if ($value instanceof Reference) { + return ['reference' => (string)$value, 'invalid_behavior' => $value->getInvalidBehavior()]; + } + if ($value instanceof Definition) { + return ['inline' => $this->dumpDefinition($value, true)]; + } + if (is_array($value)) { + return array_map([$this, 'export'], $value); + } + + return $value; + } +} diff --git a/tests/Unit/Helper/HttpKernelHelper.php b/tests/Unit/Helper/HttpKernelHelper.php index 67a50b0..4e3d23a 100644 --- a/tests/Unit/Helper/HttpKernelHelper.php +++ b/tests/Unit/Helper/HttpKernelHelper.php @@ -10,7 +10,7 @@ class HttpKernelHelper { public static function getMainRequestConstValue(): int { - if (defined(HttpKernelInterface::class . 'MAIN_REQUEST')) { + if (defined(HttpKernelInterface::class . '::MAIN_REQUEST')) { return HttpKernelInterface::MAIN_REQUEST; } diff --git a/tests/Unit/Listener/LocaleListenerTest.php b/tests/Unit/Listener/LocaleListenerTest.php index c404e8e..695b69a 100644 --- a/tests/Unit/Listener/LocaleListenerTest.php +++ b/tests/Unit/Listener/LocaleListenerTest.php @@ -8,10 +8,13 @@ use Paysera\Bundle\ApiBundle\Listener\LocaleListener; use Paysera\Bundle\ApiBundle\Service\RestRequestHelper; use Paysera\Bundle\ApiBundle\Tests\Unit\Helper\HttpKernelHelper; +use ReflectionClass; +use Symfony\Component\HttpFoundation\AcceptHeader; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; +use Throwable; class LocaleListenerTest extends MockeryTestCase { @@ -24,6 +27,25 @@ class LocaleListenerTest extends MockeryTestCase * @param bool $rest */ public function testOnKernelRequest(string $expectedLocale, array $locales, string $acceptLanguage, bool $rest) + { + $this->assertSame($expectedLocale, $this->resolveLocale($locales, $acceptLanguage, $rest)); + } + + public function testAnItemOfOnlyASemicolonIsNotALanguage() + { + try { + AcceptHeader::fromString(';'); + } catch (Throwable $error) { + $this->markTestSkipped('This http-foundation fails on an empty Accept-Language item itself'); + } + + $this->assertSame('unchanged', $this->resolveLocale(['de'], ';', true)); + } + + /** + * @param array $locales + */ + private function resolveLocale(array $locales, string $acceptLanguage, bool $rest): string { $helper = Mockery::mock(RestRequestHelper::class); $kernel = Mockery::mock(HttpKernelInterface::class); @@ -42,11 +64,13 @@ public function testOnKernelRequest(string $expectedLocale, array $locales, stri $listener->onKernelRequest($event); - $this->assertSame($expectedLocale, $request->getLocale()); + return $request->getLocale(); } - public function provider() + public static function provider(): array { + $tagsFormattedBySymfony = (new ReflectionClass(Request::class))->hasMethod('formatLocale'); + return [ [ 'unchanged', @@ -90,12 +114,114 @@ public function provider() 'en, de-CH', true, ], - 'higher priority wins' => [ - 'de', + 'higher priority wins, from Symfony 7.1 through the language of the first tag' => [ + $tagsFormattedBySymfony ? 'en' : 'de', ['de', 'en'], 'en-US,en;q=0.8, de-CH;q=0.9', true, ], + 'German among other locales' => [ + 'de', + ['en', 'lt', 'de'], + 'de', + true, + ], + 'German region among other locales' => [ + 'de', + ['en', 'lt', 'de'], + 'de-DE', + true, + ], + 'a tag that a configured locale starts with, from Symfony 7.1 matched to that locale' => [ + $tagsFormattedBySymfony ? 'de' : 'unchanged', + ['de'], + 'd', + true, + ], + 'a bare language, from Symfony 7.1 matched to a configured locale with a region' => [ + $tagsFormattedBySymfony ? 'en_US' : 'unchanged', + ['en_US', 'lt_LT'], + 'en', + true, + ], + 'another region of the language, from Symfony 7.1 matched to a configured locale with a region' => [ + $tagsFormattedBySymfony ? 'en_US' : 'unchanged', + ['en_US', 'lt_LT'], + 'en-GB', + true, + ], + 'a bare language of the second configured locale' => [ + $tagsFormattedBySymfony ? 'lt_LT' : 'unchanged', + ['en_US', 'lt_LT'], + 'lt', + true, + ], + 'no match keeps the locale' => [ + 'unchanged', + ['en', 'lt', 'de'], + 'fr-FR, fr;q=0.9', + true, + ], + 'no header keeps the locale' => [ + 'unchanged', + ['en', 'lt', 'de'], + '', + true, + ], + 'a locale configured as a number' => [ + '1', + [1], + 'de, 1', + true, + ], + 'a null configured next to a locale' => [ + 'de', + [null, 'de'], + 'de', + true, + ], + 'an empty i- item with a null configured next to a locale' => [ + 'de', + [null, 'de'], + 'i-, de', + true, + ], + 'an empty i- item with only a null configured' => [ + 'unchanged', + [null], + 'i-', + true, + ], + 'a malformed item is not a language' => [ + 'unchanged', + ['de'], + "'", + true, + ], + 'an empty i- item, which Symfony 3.4 to 7.0 reads as an empty language' => [ + 'de', + ['de'], + 'i-, de', + true, + ], + 'the language default before a configured locale selects that locale' => [ + 'de', + ['de'], + 'default, de', + true, + ], + 'the language i-default before a configured locale selects that locale' => [ + 'de', + ['de'], + 'i-default, de', + true, + ], + 'a malformed item next to a language' => [ + 'de', + ['de'], + "de, '", + true, + ], ]; } } diff --git a/tests/Unit/Listener/RestExceptionListenerTest.php b/tests/Unit/Listener/RestExceptionListenerTest.php index 3bdc879..f042855 100644 --- a/tests/Unit/Listener/RestExceptionListenerTest.php +++ b/tests/Unit/Listener/RestExceptionListenerTest.php @@ -29,7 +29,7 @@ class RestExceptionListenerTest extends MockeryTestCase * @param int $statusCode * @param string|null $logLevel */ - public function testOnKernelException(bool $restRequest, int $statusCode = 400, string $logLevel = null) + public function testOnKernelException(bool $restRequest, int $statusCode = 400, ?string $logLevel = null) { $helper = Mockery::mock(RestRequestHelper::class); $errorBuilder = Mockery::mock(ErrorBuilderInterface::class); diff --git a/tests/Unit/Normalizer/PagedQueryNormalizerTest.php b/tests/Unit/Normalizer/PagedQueryNormalizerTest.php index 9cf702a..a912137 100644 --- a/tests/Unit/Normalizer/PagedQueryNormalizerTest.php +++ b/tests/Unit/Normalizer/PagedQueryNormalizerTest.php @@ -85,7 +85,7 @@ public function testNormalize( array $explicitlyIncluded, string $defaultStrategy, string $queryStrategy, - int $maximumOffset = null + ?int $maximumOffset = null ) { $normalizationContext = Mockery::mock(NormalizationContext::class); diff --git a/tests/Unit/Service/RoutingLoader/DocblockAnnotationGuardTest.php b/tests/Unit/Service/RoutingLoader/DocblockAnnotationGuardTest.php new file mode 100644 index 0000000..733d12a --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/DocblockAnnotationGuardTest.php @@ -0,0 +1,616 @@ + + */ + public static function refusedProvider(): array + { + $imports = 'imports ' . self::ANNOTATION_NAMESPACE . '\\'; + $fullyQualifiedTag = 'has the docblock tag @\\' . self::ANNOTATION_NAMESPACE . '\RequiredPermissions'; + + return [ + 'import of an annotation' => [ + DocblockOptionsOnAttributeRouteController::class, + 'show', + DocblockOptionsOnAttributeRouteController::class, + $imports . 'RequiredPermissions', + ], + 'imports of two annotations, on the class and the action' => [ + ClassAndActionDocblockController::class, + 'show', + ClassAndActionDocblockController::class, + $imports . 'RequiredPermissions', + ], + 'import with a group use statement' => [ + GroupUseDocblockController::class, + 'show', + GroupUseDocblockController::class, + $imports . 'RequiredPermissions', + ], + 'import of the annotation namespace with an alias' => [ + MisspeltDocblockAnnotationController::class, + 'show', + MisspeltDocblockAnnotationController::class, + 'imports ' . self::ANNOTATION_NAMESPACE, + ], + 'import of an annotation that no docblock uses' => [ + UnusedAnnotationImportController::class, + 'show', + UnusedAnnotationImportController::class, + $imports . 'Body', + ], + 'import of an annotation written in lowercase' => [ + LowercaseAnnotationImportController::class, + 'show', + LowercaseAnnotationImportController::class, + 'imports paysera\bundle\apibundle\annotation\RequiredPermissions', + ], + 'import of an annotation next to a tag that is not an annotation' => [ + UnimportedNoteTagController::class, + 'show', + UnimportedNoteTagController::class, + $imports . 'RequiredPermissions', + ], + 'import of an annotation on the class, next to an attribute on the action' => [ + ClassDocblockAndMethodAttributeController::class, + 'update', + ClassDocblockAndMethodAttributeController::class, + $imports . 'RequiredPermissions', + ], + 'tag through an alias of a parent namespace' => [ + AliasedNamespaceDocblockController::class, + 'show', + AliasedNamespaceDocblockController::class, + 'has the docblock tag @Api\Annotation\RequiredPermissions', + self::OTHER_LIBRARY, + ], + 'fully qualified tag' => [ + FullyQualifiedDocblockController::class, + 'show', + FullyQualifiedDocblockController::class, + $fullyQualifiedTag, + ], + 'fully qualified tag on the class' => [ + ClassDocblockFullyQualifiedController::class, + 'show', + ClassDocblockFullyQualifiedController::class, + $fullyQualifiedTag, + ], + 'fully qualified tag split across docblock lines' => [ + SplitFullyQualifiedDocblockController::class, + 'show', + SplitFullyQualifiedDocblockController::class, + $fullyQualifiedTag, + ], + 'fully qualified tag with two leading backslashes' => [ + DoubleBackslashDocblockController::class, + 'show', + DoubleBackslashDocblockController::class, + 'has the docblock tag @\\\\' . self::ANNOTATION_NAMESPACE . '\RequiredPermissions', + ], + 'fully qualified tag after a line of prose ending with a backslash' => [ + BackslashBeforeTagDocblockController::class, + 'show', + BackslashBeforeTagDocblockController::class, + $fullyQualifiedTag, + ], + 'fully qualified tag written in lowercase, without arguments' => [ + LowercaseFullyQualifiedDocblockController::class, + 'show', + LowercaseFullyQualifiedDocblockController::class, + 'has the docblock tag @\paysera\bundle\apibundle\annotation\validation', + ], + 'tag through an alias that is not imported, in the parent declaring the action' => [ + ChildImportingForParentController::class, + 'show', + ParentWithoutImportController::class, + 'has the docblock tag @Rest\RequiredPermissions', + self::OTHER_LIBRARY, + ], + 'import in the trait declaring the action' => [ + TraitDocblockOptionsController::class, + 'show', + DocblockOptionsTrait::class, + $imports . 'RequiredPermissions', + ], + 'import in a trait used by another trait' => [ + NestedTraitActionController::class, + 'show', + DocblockOptionsTrait::class, + $imports . 'RequiredPermissions', + ], + 'import in a nested trait, for an own action next to it' => [ + OwnActionNextToNestedTraitController::class, + 'index', + DocblockOptionsTrait::class, + $imports . 'RequiredPermissions', + ], + 'import in a trait, for an own action next to it' => [ + OwnActionNextToAnnotatedTraitController::class, + 'index', + DocblockOptionsTrait::class, + $imports . 'RequiredPermissions', + ], + 'import in the trait of an action renamed with as' => [ + AliasedTraitMethodController::class, + 'index', + DocblockOptionsTrait::class, + $imports . 'RequiredPermissions', + ], + 'fully qualified tag in the trait of an action renamed with as' => [ + AliasedFullyQualifiedTraitMethodController::class, + 'index', + FullyQualifiedOptionsTrait::class, + $fullyQualifiedTag, + ], + 'import in the parent declaring the action' => [ + ChildOfImportingParentController::class, + 'index', + ImportingParentController::class, + $imports . 'RequiredPermissions', + ], + ]; + } + + /** + * @return array + */ + public static function passingProvider(): array + { + return [ + 'attribute' => [AttributeOptionsController::class], + 'attribute next to a tag that is not an annotation' => [AttributeOptionsWithNoteTagController::class], + 'attribute through an alias of the bundle namespace' => [AliasedNamespaceAttributeController::class], + 'docblock references to the attribute classes, a tag of another library, a string and a closure' => [ + AttributeDocblockReferencesController::class, + ], + 'tags named like imports that are not annotations' => [TagNamedLikeImportsController::class], + 'tag-shaped text in a string and a comment' => [NamespaceMentionedInStringController::class], + 'tag of another class outside the route\'s docblocks' => [ + MissingParentInUnrelatedDocblockController::class, + ], + 'tag of another class that cannot be loaded' => [MissingParentTagController::class], + 'own action of a child whose parent imports an annotation' => [ + ChildOfImportingParentController::class, + ], + 'annotation class of the application, imported: not detected' => [AppAnnotationController::class], + 'annotation class of the application, fully qualified: not detected' => [ + AppAnnotationFullyQualifiedController::class, + ], + 'annotation class of the application, in the namespace of the controller: not detected' => [ + OwnNamespaceAppAnnotationController::class, + ], + 'annotation class of the application, through a lowercase alias: not detected' => [ + LowercaseAliasAppAnnotationController::class, + ], + 'annotation class of the application, through an alias of a parent namespace: not detected' => [ + NamespaceAliasAppAnnotationController::class, + ], + 'annotation class of the application, split across lines through an alias: not detected' => [ + SplitAliasAppAnnotationController::class, + ], + ]; + } + + /** + * @return array + */ + public static function generatedRefusedProvider(): array + { + $tag = '@\\' . self::ANNOTATION_NAMESPACE . '\RequiredPermissions(permissions={"ROLE_ADMIN"})'; + $fullyQualifiedTag = 'has the docblock tag @\\' . self::ANNOTATION_NAMESPACE . '\RequiredPermissions'; + + return [ + 'Latin-1 prose before a fully qualified tag' => [ + 'GeneratedLatin1ProseController', + '', + "R\xE9sum\xE9 of the exports\n * " . $tag, + $fullyQualifiedTag, + ], + 'Latin-1 prose after a fully qualified tag' => [ + 'GeneratedLatin1ProseAfterTagController', + '', + $tag . "\n * R\xE9sum\xE9 of the exports", + $fullyQualifiedTag, + ], + 'Latin-1 prose ending with a backslash before a fully qualified tag' => [ + 'GeneratedLatin1BackslashProseController', + '', + "R\xE9sum\xE9 under C:\\exports\\\n * " . $tag, + $fullyQualifiedTag, + ], + 'Latin-1 prose before a tag split across lines at a no-break space' => [ + 'GeneratedLatin1ProseSplitTagController', + '', + "R\xE9sum\xE9 of the exports\n * @\\Paysera\\Bundle\\\n" + . " *\xC2\xA0ApiBundle\\Annotation\\RequiredPermissions(permissions={\"ROLE_ADMIN\"})", + $fullyQualifiedTag, + ], + 'fully qualified tag split across lines at a no-break space' => [ + 'GeneratedNoBreakSpaceSplitController', + '', + "@\\Paysera\\Bundle\\\n" + . " *\u{00A0}ApiBundle\\Annotation\\RequiredPermissions(permissions={\"ROLE_ADMIN\"})", + $fullyQualifiedTag, + ], + 'fully qualified tag right after a quoted string of another tag' => [ + 'GeneratedTagAfterStringController', + '', + '@deprecated "x"' . $tag, + $fullyQualifiedTag, + ], + 'fully qualified tag quoted in prose' => [ + 'GeneratedQuotedTagController', + '', + '"@\\' . self::ANNOTATION_NAMESPACE . '\RequiredPermissions" is gone', + $fullyQualifiedTag, + ], + 'fully qualified tag at the start of a docblock line' => [ + 'GeneratedLineStartTagController', + '', + "\n" . $tag, + $fullyQualifiedTag, + ], + 'import of an annotation in a comma-separated use statement' => [ + 'GeneratedCommaImportController', + 'use Psr\Log\LoggerInterface, ' . self::ANNOTATION_NAMESPACE . '\RequiredPermissions as Perm;', + '@Perm(permissions={"ROLE_ADMIN"})', + 'imports ' . self::ANNOTATION_NAMESPACE . '\RequiredPermissions', + ], + 'tag through an alias of a parent namespace spelt with a long s' => [ + 'GeneratedLongSAliasController', + "use Paysera\\Bundle\\ApiBundle as Api\u{017F};", + "@Api\u{017F}\\Annotation\\RequiredPermissions(permissions={\"ROLE_ADMIN\"})", + "has the docblock tag @Api\u{017F}\\Annotation\\RequiredPermissions", + self::OTHER_LIBRARY, + ], + 'tag through an alias of a parent namespace spelt with a Kelvin sign' => [ + 'GeneratedKelvinAliasController', + "use Paysera\\Bundle\\ApiBundle as Api\u{212A};", + "@Api\u{212A}\\Annotation\\Validation", + "has the docblock tag @Api\u{212A}\\Annotation\\Validation", + self::OTHER_LIBRARY, + ], + 'fully qualified tag name followed by a dash and a digit' => [ + 'GeneratedDashDigitTagController', + '', + '@\\' . self::ANNOTATION_NAMESPACE . '\Validation-1', + 'has the docblock tag @\\' . self::ANNOTATION_NAMESPACE . '\Validation', + ], + 'tag whose alias has a non-ASCII letter Doctrine does not accept' => [ + 'GeneratedUmlautAliasController', + '', + "@\u{00DC}nicode\\Query(type=\"String\")", + "has the docblock tag @\u{00DC}nicode\\Query", + self::OTHER_LIBRARY, + ], + ]; + } + + /** + * @return array + */ + public static function generatedPassingProvider(): array + { + $name = '@\\' . self::ANNOTATION_NAMESPACE . '\RequiredPermissions'; + + return [ + 'fully qualified tag name followed by a dash' => [ + 'GeneratedDashedTagController', + '', + $name . '-only(permissions={"ROLE_ADMIN"})', + ], + 'fully qualified tag in backticks in prose' => [ + 'GeneratedBacktickProseController', + '', + 'Replaces the `' . $name . '(permissions={"ROLE_ADMIN"})` of 1.x', + ], + 'fully qualified tag in parentheses in prose' => [ + 'GeneratedParenthesisProseController', + '', + '(' . $name . ' is gone)', + ], + 'fully qualified tag in braces in prose' => [ + 'GeneratedBraceProseController', + '', + '{' . $name . '}', + ], + 'fully qualified tag right after a single-quoted string' => [ + 'GeneratedTagAfterSingleQuotedStringController', + '', + "@deprecated 'x'" . $name . '(permissions={"ROLE_ADMIN"})', + ], + ]; + } + + /** + * @dataProvider refusedProvider + */ + public function testFailOnDocblockAnnotationsRefuses( + string $controller, + string $method, + string $fileOwner, + string $finding, + string $remedy = '' + ): void { + $this->assertRefusedWith( + $this->buildRefusalMessage($controller, $method, $fileOwner, $finding) . $remedy, + $controller, + $method + ); + } + + /** + * @dataProvider generatedRefusedProvider + */ + public function testFailOnDocblockAnnotationsRefusesAGeneratedController( + string $shortName, + string $imports, + string $docblock, + string $finding, + string $remedy = '' + ): void { + $controller = $this->declareController($shortName, $docblock, $imports); + + $this->assertRefusedWith( + $this->buildRefusalMessage($controller, 'show', $controller, $finding) . $remedy, + $controller + ); + } + + /** + * @dataProvider generatedPassingProvider + */ + public function testFailOnDocblockAnnotationsPassesAGeneratedController( + string $shortName, + string $imports, + string $docblock + ): void { + $this->check($this->declareController($shortName, $docblock, $imports)); + + $this->addToAssertionCount(1); + } + + public function testFailOnDocblockAnnotationsNamesTheRemedyForAnotherLibrary(): void + { + $controller = $this->declareController('GeneratedOtherLibraryTagController', '@GQL\Query(type="String")'); + + $this->assertRefusedWith( + $this->buildRefusalMessage($controller, 'show', $controller, 'has the docblock tag @GQL\Query') + . self::OTHER_LIBRARY, + $controller + ); + } + + /** + * @dataProvider passingProvider + */ + public function testFailOnDocblockAnnotationsPasses(string $controller, string $method = 'show'): void + { + $this->check($controller, $method); + + $this->addToAssertionCount(1); + } + + public function testFailOnDocblockAnnotationsRefusesEveryAnnotationOfTheBundle(): void + { + $annotationPattern = __DIR__ . '/../../../../src/Annotation/*.php'; + $annotationFiles = glob($annotationPattern); + if ($annotationFiles === false || $annotationFiles === []) { + throw new RuntimeException('No annotation class found by the pattern ' . $annotationPattern); + } + + $checked = 0; + foreach ($annotationFiles as $file) { + $annotation = basename($file, '.php'); + if (interface_exists(self::ANNOTATION_NAMESPACE . '\\' . $annotation)) { + continue; + } + + $tag = '@\\' . self::ANNOTATION_NAMESPACE . '\\' . $annotation; + $controller = $this->declareController('Generated' . $annotation . 'Controller', $tag); + try { + $this->check($controller); + $this->fail('Expected a ConfigurationException for ' . $tag); + } catch (ConfigurationException $exception) { + $this->assertStringContainsString('has the docblock tag ' . $tag, $exception->getMessage()); + } + $checked++; + } + + $this->assertSame(count($annotationFiles) - 1, $checked); + } + + public function testFailOnDocblockAnnotationsFailsWhenTheSourceCannotBeRead(): void + { + $controller = $this->declareController('GeneratedUnreadableController', '@note kept for older clients'); + $file = (new ReflectionClass($controller))->getFileName(); + unlink($file); + + $this->assertRefusedWith($this->buildUnreadableMessage($controller, 'show', $file), $controller); + } + + public function testFailOnDocblockAnnotationsFailsWhenTheClassIsEvaluatedCode(): void + { + eval(sprintf( + 'namespace %s; class EvaluatedController { public function show(): void {} }', + self::GENERATED_NAMESPACE + )); + $controller = self::GENERATED_NAMESPACE . '\EvaluatedController'; + + $this->assertRefusedWith( + $this->buildUnreadableMessage($controller, 'show', (new ReflectionClass($controller))->getFileName()), + $controller + ); + } + + public function testFailOnDocblockAnnotationsFailsWhenTheClassHasNoSourceFile(): void + { + $this->assertRefusedWith( + $this->buildUnreadableMessage(ArrayObject::class, 'count', 'the source of ' . ArrayObject::class), + ArrayObject::class, + 'count' + ); + } + + public function testFailOnDocblockAnnotationsFailsWhenTheDocblockCannotBeMatched(): void + { + $jit = ini_get('pcre.jit'); + $backtrackLimit = ini_get('pcre.backtrack_limit'); + ini_set('pcre.jit', '0'); + ini_set('pcre.backtrack_limit', '1'); + try { + $this->check(AttributeOptionsWithNoteTagController::class); + $this->fail('Expected a ConfigurationException'); + } catch (ConfigurationException $exception) { + $this->assertStringContainsString( + 'has a docblock that cannot be checked (preg error ' . PREG_BACKTRACK_LIMIT_ERROR . ')', + $exception->getMessage() + ); + } finally { + ini_set('pcre.jit', (string)$jit); + ini_set('pcre.backtrack_limit', (string)$backtrackLimit); + } + } + + private function buildRefusalMessage(string $controller, string $method, string $fileOwner, string $finding): string + { + return sprintf( + '%s::%s(): %s %s%s', + $controller, + (new ReflectionMethod($controller, $method))->getName(), + (new ReflectionClass($fileOwner))->getFileName(), + $finding, + self::NOT_READ + ); + } + + private function buildUnreadableMessage(string $controller, string $method, string $source): string + { + return sprintf( + '%s::%s(): %s cannot be read, so its docblocks cannot be checked for annotations of the bundle; %s', + $controller, + $method, + $source, + self::NOT_READ_REASON + ); + } + + private function declareController(string $shortName, string $docblock, string $imports = ''): string + { + $placeholder = tempnam(sys_get_temp_dir(), 'guard'); + if ($placeholder === false) { + throw new RuntimeException('Cannot create a temporary file'); + } + $file = $placeholder . '.php'; + unlink($placeholder); + self::$generatedFiles[] = $file; + file_put_contents($file, sprintf( + "check($controller, $method); + $this->fail('Expected a ConfigurationException'); + } catch (ConfigurationException $exception) { + $this->assertSame($message, $exception->getMessage()); + } + } + + private function check(string $controller, string $method = 'show'): void + { + (new DocblockAnnotationGuard())->failOnDocblockAnnotations( + new ReflectionClass($controller), + new ReflectionMethod($controller, $method) + ); + } +} diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/AliasedFullyQualifiedTraitMethodController.php b/tests/Unit/Service/RoutingLoader/Fixtures/AliasedFullyQualifiedTraitMethodController.php new file mode 100644 index 0000000..29674cf --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/AliasedFullyQualifiedTraitMethodController.php @@ -0,0 +1,13 @@ +setRequiredPermissions(['ROLE_AUDITOR']); + } +} diff --git a/tests/Unit/Service/RoutingLoader/Fixtures/AttributeDocblockReferencesController.php b/tests/Unit/Service/RoutingLoader/Fixtures/AttributeDocblockReferencesController.php new file mode 100644 index 0000000..c8b7fa3 --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/Fixtures/AttributeDocblockReferencesController.php @@ -0,0 +1,38 @@ +setRestRouteConfigurator(new RestRouteConfigurator( + new RestRequestHelper(new RestRequestOptionsRegistry()), + new RestRequestAnnotationOptionsBuilder(Mockery::spy(RestRequestOptionsValidator::class)), + new RestRequestAttributeOptionsBuilder(Mockery::spy(RestRequestOptionsValidator::class)), + new DocblockAnnotationGuard() +)); + +$results = ['doc_comments_stripped' => (new ReflectionClass(AppPermission::class))->getDocComment() === false]; +foreach (array_slice($argv, 1) as $controller) { + try { + $permissions = []; + foreach ($loader->load($controller)->all() as $route) { + $options = $route->getDefault(RestRequestHelper::SERIALIZED_REST_OPTIONS_KEY); + $permissions[] = $options === null ? null : unserialize($options)->getRequiredPermissions(); + } + $results[$controller] = $permissions; + } catch (ConfigurationException $exception) { + $results[$controller] = $exception->getMessage(); + } +} + +echo json_encode($results); diff --git a/tests/Unit/Service/RoutingLoader/ReflectionMethodWrapperTest.php b/tests/Unit/Service/RoutingLoader/ReflectionMethodWrapperTest.php index 030e782..dce8753 100644 --- a/tests/Unit/Service/RoutingLoader/ReflectionMethodWrapperTest.php +++ b/tests/Unit/Service/RoutingLoader/ReflectionMethodWrapperTest.php @@ -58,7 +58,7 @@ public function testGetFriendlyName(): void ); } - public function fixtureMethod(string $param1, DateTime $param2 = null, $param3 = null): string + public function fixtureMethod(string $param1, ?DateTime $param2 = null, $param3 = null): string { return $param1; } diff --git a/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php b/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php index a36d451..b5350bd 100644 --- a/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php +++ b/tests/Unit/Service/RoutingLoader/RestRequestAnnotationOptionsBuilderTest.php @@ -68,10 +68,11 @@ public function testBuildOptionsWithSeveralUnsupportedAnnotations(): void $builder = new RestRequestAnnotationOptionsBuilder($optionsValidator); $this->expectException(ConfigurationException::class); + $this->expectExceptionMessage('Only one annotation of type ' . Body::class . ' is supported'); $builder->buildOptions([ - new Body(['parameterName' => 'a']), - new Body(['parameterName' => 'b']), + new Body(['parameterName' => 'a', 'denormalizationType' => 'type_a', 'optional' => false]), + new Body(['parameterName' => 'b', 'denormalizationType' => 'type_b', 'optional' => false]), ], new ReflectionMethod(self::class, 'fixtureMethod')); } diff --git a/tests/Unit/Service/RoutingLoader/RestRouteConfiguratorTest.php b/tests/Unit/Service/RoutingLoader/RestRouteConfiguratorTest.php new file mode 100644 index 0000000..f392d50 --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/RestRouteConfiguratorTest.php @@ -0,0 +1,262 @@ +configureRoute(MisspeltDocblockAnnotationController::class, new AnnotationReader()); + $this->fail('Expected a ConfigurationException'); + } catch (ConfigurationException $exception) { + $this->assertStringStartsWith( + sprintf( + 'Cannot read the docblock annotations of %s::show(): [Semantical Error]', + MisspeltDocblockAnnotationController::class + ), + $exception->getMessage() + ); + $this->assertInstanceOf(AnnotationException::class, $exception->getPrevious()); + } + } + + public function testConfigureRouteWithoutReaderRefusesDocblockAnnotations(): void + { + $this->expectException(ConfigurationException::class); + $this->expectExceptionMessage(sprintf( + '%s::show(): %s imports Paysera\Bundle\ApiBundle\Annotation, but docblock annotations of the bundle are' + . ' not read here, because Symfony passes no annotation reader to the route loader;' + . ' use the attributes of the bundle instead', + MisspeltDocblockAnnotationController::class, + (new ReflectionClass(MisspeltDocblockAnnotationController::class))->getFileName() + )); + + $this->configureRoute(MisspeltDocblockAnnotationController::class, null); + } + + /** + * @dataProvider invalidAttributeProvider + */ + public function testConfigureRouteFailsOnInvalidAttributes( + string $controller, + string $message, + string $method = 'show' + ): void { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('PHP 7 has no attributes'); + } + $this->requireApplicationFixtures(); + + $this->expectException(ConfigurationException::class); + $this->expectExceptionMessage(sprintf($message, $controller)); + + $this->configureRoute($controller, null, $method); + } + + public static function invalidAttributeProvider(): array + { + $unknown = '%s::show() uses Paysera\Bundle\ApiBundle\%s, which is no attribute of the bundle'; + $missing = '%s::%s() uses %s, which is not a class that can be loaded;' + . ' if the bundle\'s attribute is meant, the name must resolve to Paysera\Bundle\ApiBundle\Attribute\%s'; + + $dataSets = [ + 'constructor error' => [InvalidAttributeController::class, 'Cannot read the attributes of %s::show(): '], + 'misspelt name' => [ + MisspeltAttributeController::class, + sprintf($unknown, '%s', 'Attribute\RequiredPermission'), + ], + 'misspelt namespace' => [ + MisspeltAttributeNamespaceController::class, + sprintf($unknown, '%s', 'Attributes\RequiredPermissions'), + ], + 'misspelt namespace written in lowercase' => [ + LowercaseMisspeltAttributeController::class, + '%s::show() uses paysera\bundle\apibundle\attributes\RequiredPermissions,' + . ' which is no attribute of the bundle', + ], + 'misspelt name in the annotation namespace' => [ + MisspeltAnnotationAliasAttributeController::class, + sprintf($unknown, '%s', 'Annotation\RequiredPermission'), + ], + 'class of the bundle that is no attribute' => [ + NonAttributeBundleClassController::class, + sprintf($unknown, '%s', 'Entity\RestRequestOptions'), + ], + 'annotation class used as an attribute' => [ + AnnotationClassAsAttributeController::class, + 'Cannot read the attributes of %s::show(): Attempting to use non-attribute class', + ], + 'name of an attribute without its use statement, in lowercase' => [ + MissingUseController::class, + sprintf( + $missing, + '%s', + 'showLowercaseName', + 'App\Controller\requiredpermissions', + 'RequiredPermissions' + ), + 'showLowercaseName', + ], + 'qualified name of an attribute without the import of its alias' => [ + MissingUseController::class, + sprintf( + $missing, + '%s', + 'showQualifiedName', + 'App\Controller\Rest\RequiredPermissions', + 'RequiredPermissions' + ), + 'showQualifiedName', + ], + 'name of an attribute without its use statement, on the class' => [ + MissingUseClassController::class, + sprintf($missing, '%s', 'show', 'App\Controller\RequiredPermissions', 'RequiredPermissions'), + ], + 'name of an attribute without its use statement, in a trait of another namespace' => [ + MissingUseTraitController::class, + sprintf($missing, '%s', 'showFromTrait', 'App\Shared\RequiredPermissions', 'RequiredPermissions'), + 'showFromTrait', + ], + 'name of an attribute without its use statement, in the global namespace' => [ + GlobalMissingUseController::class, + sprintf($missing, '%s', 'show', 'RequiredPermissions', 'RequiredPermissions'), + ], + ]; + + $attributePattern = __DIR__ . '/../../../../src/Attribute/*.php'; + $attributeFiles = glob($attributePattern); + if ($attributeFiles === false || $attributeFiles === []) { + throw new RuntimeException('No attribute class found by the pattern ' . $attributePattern); + } + + foreach ($attributeFiles as $file) { + $attribute = basename($file, '.php'); + if (interface_exists('Paysera\Bundle\ApiBundle\Attribute\\' . $attribute)) { + continue; + } + + $dataSets['name of an attribute without its use statement: ' . $attribute] = [ + MissingUseController::class, + sprintf($missing, '%s', 'show' . $attribute, 'App\Controller\\' . $attribute, $attribute), + 'show' . $attribute, + ]; + } + + return $dataSets; + } + + /** + * @dataProvider loadingAttributeProvider + */ + public function testConfigureRouteLeavesOtherAttributesAlone(string $controller, string $method): void + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('PHP 7 has no attributes'); + } + $this->requireApplicationFixtures(); + + $this->assertEquals(new Route('/'), $this->configureRoute($controller, null, $method)); + } + + public static function loadingAttributeProvider(): array + { + return [ + 'class of another library named like an attribute' => [ + OtherLibraryAttributeController::class, + 'showClassNamedLikeAttribute', + ], + 'class of another library named like an attribute, on the class' => [ + OtherLibraryClassAttributeController::class, + 'show', + ], + 'class of the application named like an attribute, in the namespace of the controller' => [ + OwnAttributeController::class, + 'show', + ], + 'missing class in the namespace of the controller with a name that is none of the attributes' => [ + MissingUseController::class, + 'showMissingClassWithOtherName', + ], + 'attribute of a package that is not installed, imported' => [ + MissingUseController::class, + 'showUninstalledPackageAttribute', + ], + 'attribute of a package that is not installed, imported in the global namespace' => [ + GlobalMissingUseController::class, + 'showUninstalledPackageAttribute', + ], + ]; + } + + private function requireApplicationFixtures(): void + { + foreach (self::APPLICATION_FIXTURES as $fixture) { + require_once __DIR__ . '/Fixtures/App/' . $fixture; + } + } + + private function configureRoute(string $controller, ?Reader $reader, string $method = 'show'): Route + { + $configurator = new RestRouteConfigurator( + new RestRequestHelper(new RestRequestOptionsRegistry()), + new RestRequestAnnotationOptionsBuilder(Mockery::spy(RestRequestOptionsValidator::class)), + new RestRequestAttributeOptionsBuilder(Mockery::spy(RestRequestOptionsValidator::class)), + new DocblockAnnotationGuard() + ); + $route = new Route('/'); + $configurator->configureRoute( + $route, + new ReflectionClass($controller), + new ReflectionMethod($controller, $method), + $reader + ); + + return $route; + } +} diff --git a/tests/Unit/Service/RoutingLoader/RoutingAnnotationLoaderTest.php b/tests/Unit/Service/RoutingLoader/RoutingAnnotationLoaderTest.php new file mode 100644 index 0000000..560b4a0 --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/RoutingAnnotationLoaderTest.php @@ -0,0 +1,134 @@ +markTestSkipped('PHP 7 has no attributes'); + } + + if (class_exists(AttributeRouteControllerLoader::class)) { + $this->markTestSkipped('Symfony 6.4 and later load routes through RoutingAttributeLoader'); + } + + if ((new ReflectionClass(Route::class))->getAttributes() === []) { + $this->markTestSkipped('Symfony 5.1 and older read no #[Route] attributes'); + } + } + + /** + * @dataProvider optionsProvider + */ + public function testLoadAppliesOptions(string $controller, ?Reader $reader): void + { + $routes = $this->createLoader($reader)->load($controller)->all(); + + $this->assertCount(1, $routes); + $this->assertEquals( + (new RestRequestOptions())->setRequiredPermissions(['ROLE_ADMIN']), + unserialize(reset($routes)->getDefault(RestRequestHelper::SERIALIZED_REST_OPTIONS_KEY)) + ); + } + + public static function optionsProvider(): array + { + return [ + 'attributes with the reader of Symfony' => [ + NativeRouteAttributeOptionsController::class, + new AnnotationReader(), + ], + 'attributes with framework.annotations disabled' => [NativeRouteAttributeOptionsController::class, null], + 'docblock annotations with the reader of Symfony' => [ + NativeRouteDocblockOptionsController::class, + new AnnotationReader(), + ], + ]; + } + + /** + * @dataProvider failureProvider + */ + public function testLoadFails(string $controller, ?Reader $reader, string $expectedMessage): void + { + $this->expectException(ConfigurationException::class); + $this->expectExceptionMessage($expectedMessage); + + $this->createLoader($reader)->load($controller); + } + + public static function failureProvider(): array + { + return [ + 'docblock annotations with framework.annotations disabled' => [ + NativeRouteDocblockOptionsController::class, + null, + sprintf( + '%s::show(): %s imports Paysera\Bundle\ApiBundle\Annotation%s', + NativeRouteDocblockOptionsController::class, + (new ReflectionClass(NativeRouteDocblockOptionsController::class))->getFileName(), + self::NOT_READ + ), + ], + 'docblock annotations and attributes with the reader of Symfony' => [ + NativeRouteMixedOptionsController::class, + new AnnotationReader(), + sprintf( + '%s::update() is configured with both docblock annotations and attributes of the bundle', + NativeRouteMixedOptionsController::class + ), + ], + 'docblock annotations and attributes with framework.annotations disabled' => [ + NativeRouteMixedOptionsController::class, + null, + sprintf( + '%s::update(): %s imports Paysera\Bundle\ApiBundle\Annotation\RequiredPermissions%s', + NativeRouteMixedOptionsController::class, + (new ReflectionClass(NativeRouteMixedOptionsController::class))->getFileName(), + self::NOT_READ + ), + ], + ]; + } + + private function createLoader(?Reader $reader): RoutingAnnotationLoader + { + $loader = new RoutingAnnotationLoader($reader); + $loader->setRestRouteConfigurator(new RestRouteConfigurator( + new RestRequestHelper(new RestRequestOptionsRegistry()), + new RestRequestAnnotationOptionsBuilder(Mockery::spy(RestRequestOptionsValidator::class)), + new RestRequestAttributeOptionsBuilder(Mockery::spy(RestRequestOptionsValidator::class)), + new DocblockAnnotationGuard() + )); + + return $loader; + } +} diff --git a/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php b/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php new file mode 100644 index 0000000..2c34da0 --- /dev/null +++ b/tests/Unit/Service/RoutingLoader/RoutingAttributeLoaderTest.php @@ -0,0 +1,404 @@ +markTestSkipped('Symfony 6.3 and older load routes through RoutingAnnotationLoader'); + } + } + + public function testLoadWithAnnotationReaderUsesIt(): void + { + if (!property_exists(AttributeClassLoader::class, 'reader')) { + $this->markTestSkipped('Symfony 7 passes no annotation reader to the route loader'); + } + + $reader = Mockery::mock(Reader::class); + $reader->shouldReceive('getClassAnnotation')->andReturn(null); + $reader->shouldReceive('getClassAnnotations')->andReturn([]); + $reader + ->shouldReceive('getMethodAnnotations') + ->andReturn([new stdClass(), new RequiredPermissions(['permissions' => ['ROLE_FROM_READER']])]) + ; + + $this->assertEquals( + [ + DocblockOptionsOnAttributeRouteController::class . '::show' => (new RestRequestOptions()) + ->setRequiredPermissions(['ROLE_FROM_READER']), + ], + $this->loadOptions($this->createLoader($reader), DocblockOptionsOnAttributeRouteController::class) + ); + } + + /** + * @dataProvider loadWithoutAnnotationReaderProvider + * @param array $expectedOptions + */ + public function testLoadWithoutAnnotationReader(string $controller, array $expectedOptions): void + { + $this->assertEquals($expectedOptions, $this->loadOptions($this->createLoader(), $controller)); + } + + public static function loadWithoutAnnotationReaderProvider(): array + { + $adminOnly = (new RestRequestOptions())->setRequiredPermissions(['ROLE_ADMIN']); + + return [ + 'attribute' => [ + AttributeOptionsController::class, + [AttributeOptionsController::class . '::show' => $adminOnly], + ], + 'attribute next to a tag that is not an annotation' => [ + AttributeOptionsWithNoteTagController::class, + [AttributeOptionsWithNoteTagController::class . '::show' => $adminOnly], + ], + 'attribute through an alias of the bundle namespace' => [ + AliasedNamespaceAttributeController::class, + [AliasedNamespaceAttributeController::class . '::show' => $adminOnly], + ], + 'attribute next to docblock references to the attribute classes and a tag of another library' => [ + AttributeDocblockReferencesController::class, + [AttributeDocblockReferencesController::class . '::show' => $adminOnly], + ], + 'tags named like imports that are not annotations of the application' => [ + TagNamedLikeImportsController::class, + [TagNamedLikeImportsController::class . '::show' => $adminOnly], + ], + 'tag-shaped text in a string and a comment' => [ + NamespaceMentionedInStringController::class, + [NamespaceMentionedInStringController::class . '::show' => $adminOnly], + ], + 'tag of another class that cannot be loaded, outside the route\'s docblocks' => [ + MissingParentInUnrelatedDocblockController::class, + [MissingParentInUnrelatedDocblockController::class . '::show' => $adminOnly], + ], + 'tag of another class that cannot be loaded, on the action: the class is not loaded' => [ + MissingParentTagController::class, + [MissingParentTagController::class . '::show' => null], + ], + 'annotation class of the application, imported: not detected without a reader' => [ + AppAnnotationController::class, + [AppAnnotationController::class . '::show' => null], + ], + 'annotation class of the application, fully qualified: not detected without a reader' => [ + AppAnnotationFullyQualifiedController::class, + [AppAnnotationFullyQualifiedController::class . '::show' => null], + ], + ]; + } + + /** + * @dataProvider loadWithoutAnnotationReaderFailsProvider + */ + public function testLoadWithoutAnnotationReaderFails( + string $controller, + ?string $owner, + ?string $method, + string $fileOwner, + string $finding + ): void { + $this->expectException(ConfigurationException::class); + $this->expectExceptionMessage($this->buildRefusalMessage($owner, $method, $fileOwner, $finding)); + + $this->createLoader()->load($controller); + } + + public static function loadWithoutAnnotationReaderFailsProvider(): array + { + $imports = 'imports ' . self::ANNOTATION_NAMESPACE . '\\'; + $fullyQualifiedTag = 'has the docblock tag @\\' . self::ANNOTATION_NAMESPACE . '\RequiredPermissions'; + + return [ + 'annotation on the action' => [ + DocblockOptionsOnAttributeRouteController::class, + DocblockOptionsOnAttributeRouteController::class, + 'show', + DocblockOptionsOnAttributeRouteController::class, + $imports . 'RequiredPermissions', + ], + 'annotations on the class and the action' => [ + ClassAndActionDocblockController::class, + ClassAndActionDocblockController::class, + 'show', + ClassAndActionDocblockController::class, + $imports . 'RequiredPermissions', + ], + 'annotation on the class and attribute on the action' => [ + ClassDocblockAndMethodAttributeController::class, + ClassDocblockAndMethodAttributeController::class, + 'update', + ClassDocblockAndMethodAttributeController::class, + $imports . 'RequiredPermissions', + ], + 'annotation imported with a group use statement' => [ + GroupUseDocblockController::class, + GroupUseDocblockController::class, + 'show', + GroupUseDocblockController::class, + $imports . 'RequiredPermissions', + ], + 'annotation next to a tag that is not an annotation' => [ + UnimportedNoteTagController::class, + UnimportedNoteTagController::class, + 'show', + UnimportedNoteTagController::class, + $imports . 'RequiredPermissions', + ], + 'import of an annotation that no docblock uses' => [ + UnusedAnnotationImportController::class, + UnusedAnnotationImportController::class, + 'show', + UnusedAnnotationImportController::class, + $imports . 'Body', + ], + 'import of an annotation written in lowercase' => [ + LowercaseAnnotationImportController::class, + LowercaseAnnotationImportController::class, + 'show', + LowercaseAnnotationImportController::class, + 'imports paysera\bundle\apibundle\annotation\RequiredPermissions', + ], + 'annotation written through an alias of a parent namespace' => [ + AliasedNamespaceDocblockController::class, + AliasedNamespaceDocblockController::class, + 'show', + AliasedNamespaceDocblockController::class, + 'has the docblock tag @Api\Annotation\RequiredPermissions', + ], + 'fully qualified annotation without a use statement' => [ + FullyQualifiedDocblockController::class, + FullyQualifiedDocblockController::class, + 'show', + FullyQualifiedDocblockController::class, + $fullyQualifiedTag, + ], + 'fully qualified annotation on the class without a use statement' => [ + ClassDocblockFullyQualifiedController::class, + ClassDocblockFullyQualifiedController::class, + 'show', + ClassDocblockFullyQualifiedController::class, + $fullyQualifiedTag, + ], + 'fully qualified annotation split across docblock lines' => [ + SplitFullyQualifiedDocblockController::class, + SplitFullyQualifiedDocblockController::class, + 'show', + SplitFullyQualifiedDocblockController::class, + $fullyQualifiedTag, + ], + 'fully qualified annotation written in lowercase, without arguments' => [ + LowercaseFullyQualifiedDocblockController::class, + LowercaseFullyQualifiedDocblockController::class, + 'show', + LowercaseFullyQualifiedDocblockController::class, + 'has the docblock tag @\paysera\bundle\apibundle\annotation\validation', + ], + 'annotation through an alias that only the routed class imports, for an action of its parent' => [ + ChildImportingForParentController::class, + ChildImportingForParentController::class, + 'show', + ParentWithoutImportController::class, + 'has the docblock tag @Rest\RequiredPermissions', + ], + 'action of a trait that imports the annotation' => [ + TraitDocblockOptionsController::class, + TraitDocblockOptionsController::class, + 'show', + DocblockOptionsTrait::class, + $imports . 'RequiredPermissions', + ], + 'action of a trait used by another trait' => [ + NestedTraitActionController::class, + NestedTraitActionController::class, + 'show', + DocblockOptionsTrait::class, + $imports . 'RequiredPermissions', + ], + 'own action next to a nested trait that imports the annotation' => [ + OwnActionNextToNestedTraitController::class, + OwnActionNextToNestedTraitController::class, + 'index', + DocblockOptionsTrait::class, + $imports . 'RequiredPermissions', + ], + 'own action next to a trait that imports the annotation' => [ + OwnActionNextToAnnotatedTraitController::class, + null, + null, + DocblockOptionsTrait::class, + $imports . 'RequiredPermissions', + ], + 'trait action renamed with as' => [ + AliasedTraitMethodController::class, + AliasedTraitMethodController::class, + 'index', + DocblockOptionsTrait::class, + $imports . 'RequiredPermissions', + ], + 'fully qualified annotation on a trait action renamed with as' => [ + AliasedFullyQualifiedTraitMethodController::class, + AliasedFullyQualifiedTraitMethodController::class, + 'index', + FullyQualifiedOptionsTrait::class, + $fullyQualifiedTag, + ], + 'attribute-only child of a parent that imports the annotation for an inherited action' => [ + ChildOfImportingParentController::class, + null, + null, + ImportingParentController::class, + $imports . 'RequiredPermissions', + ], + ]; + } + + public function testLoadWithoutAnnotationReaderWhenOpcacheStripsDocComments(): void + { + if (!extension_loaded('Zend OPcache')) { + $this->markTestSkipped('Needs the OPcache extension'); + } + + $attributeOnlyControllers = [ + AttributeOptionsController::class, + AttributeOptionsWithNoteTagController::class, + TagNamedLikeImportsController::class, + MissingParentInUnrelatedDocblockController::class, + ]; + $annotatedControllers = [ + DocblockOptionsOnAttributeRouteController::class, + AliasedNamespaceDocblockController::class, + FullyQualifiedDocblockController::class, + TraitDocblockOptionsController::class, + ]; + exec( + sprintf( + '%s -d opcache.enable_cli=1 -d opcache.save_comments=0 -d opcache.file_update_protection=0 %s %s', + escapeshellarg(PHP_BINARY), + escapeshellarg(__DIR__ . '/Fixtures/load-routes.php'), + implode(' ', array_map('escapeshellarg', array_merge($attributeOnlyControllers, $annotatedControllers))) + ), + $output + ); + $results = json_decode(implode("\n", $output), true); + + $this->assertIsArray($results, implode("\n", $output)); + $this->assertTrue($results['doc_comments_stripped']); + foreach ($attributeOnlyControllers as $controller) { + $this->assertSame([['ROLE_ADMIN']], $results[$controller], $controller); + } + foreach ($annotatedControllers as $controller) { + $this->assertStringContainsString( + self::NOT_READ, + (string)json_encode($results[$controller]), + $controller + ); + } + } + + private function buildRefusalMessage( + ?string $controller, + ?string $method, + string $fileOwner, + string $finding + ): string { + return sprintf( + '%s%s %s%s', + $controller === null ? '' : sprintf('%s::%s(): ', $controller, $method), + (new ReflectionClass($fileOwner))->getFileName(), + $finding, + self::NOT_READ + ); + } + + private function createLoader(?Reader $reader = null): RoutingAttributeLoader + { + $loader = $reader === null ? new RoutingAttributeLoader() : new RoutingAttributeLoader($reader); + $loader->setRestRouteConfigurator(new RestRouteConfigurator( + new RestRequestHelper(new RestRequestOptionsRegistry()), + new RestRequestAnnotationOptionsBuilder(Mockery::spy(RestRequestOptionsValidator::class)), + new RestRequestAttributeOptionsBuilder(Mockery::spy(RestRequestOptionsValidator::class)), + new DocblockAnnotationGuard() + )); + + return $loader; + } + + /** + * @return array + */ + private function loadOptions(RoutingAttributeLoader $loader, string $controller): array + { + $options = []; + foreach ($loader->load($controller)->all() as $route) { + $serializedOptions = $route->getDefault(RestRequestHelper::SERIALIZED_REST_OPTIONS_KEY); + $options[$route->getDefault('_controller')] = $serializedOptions === null + ? null + : unserialize($serializedOptions); + } + + return $options; + } +}