From 7d9573ddf61cc256b290f63ee9e03441ece584d6 Mon Sep 17 00:00:00 2001 From: Guilhem Niot Date: Mon, 6 Jul 2020 19:50:34 +0200 Subject: [PATCH] Move the OpenApi processing to ApiDocGenerator (#1671) * Move the OpenApi processing to ApiDocGenerator * Temporary fix for https://github.com/zircote/swagger-php/pull/791 * Stop using the ModelRegistry in OpenApiPhpDescriber --- ApiDocGenerator.php | 22 +++++++++++ DependencyInjection/NelmioApiDocExtension.php | 2 +- Describer/DefaultDescriber.php | 8 ++-- Describer/OpenApiPhpDescriber.php | 36 +---------------- .../SymfonyConstraintAnnotationReader.php | 17 ++++---- OpenApiPhp/AddDefaults.php | 39 ------------------- OpenApiPhp/Util.php | 2 + PropertyDescriber/ObjectPropertyDescriber.php | 2 +- Tests/ApiDocGeneratorTest.php | 4 +- Tests/Functional/Controller/ApiController.php | 4 +- Tests/Functional/FunctionalTest.php | 3 +- 11 files changed, 49 insertions(+), 90 deletions(-) delete mode 100644 OpenApiPhp/AddDefaults.php diff --git a/ApiDocGenerator.php b/ApiDocGenerator.php index 480cdf9..777073e 100644 --- a/ApiDocGenerator.php +++ b/ApiDocGenerator.php @@ -15,6 +15,8 @@ use Nelmio\ApiDocBundle\Describer\DescriberInterface; use Nelmio\ApiDocBundle\Describer\ModelRegistryAwareInterface; use Nelmio\ApiDocBundle\Model\ModelRegistry; use Nelmio\ApiDocBundle\ModelDescriber\ModelDescriberInterface; +use Nelmio\ApiDocBundle\OpenApiPhp\ModelRegister; +use OpenApi\Analysis; use OpenApi\Annotations\OpenApi; use Psr\Cache\CacheItemPoolInterface; @@ -38,6 +40,9 @@ final class ApiDocGenerator /** @var string[] */ private $alternativeNames = []; + /** @var string[] */ + private $mediaTypes = ['json']; + /** * @param DescriberInterface[]|iterable $describers * @param ModelDescriberInterface[]|iterable $modelDescribers @@ -55,6 +60,11 @@ final class ApiDocGenerator $this->alternativeNames = $alternativeNames; } + public function setMediaTypes(array $mediaTypes) + { + $this->mediaTypes = $mediaTypes; + } + public function generate(): OpenApi { if (null !== $this->openApi) { @@ -77,8 +87,20 @@ final class ApiDocGenerator $describer->describe($this->openApi); } + + $analysis = new Analysis(); + $analysis->addAnnotation($this->openApi, null); + + // Register model annotations + $modelRegister = new ModelRegister($modelRegistry, $this->mediaTypes); + $modelRegister($analysis); + + // Calculate the associated schemas $modelRegistry->registerSchemas(); + $analysis->process(); + $analysis->validate(); + if (isset($item)) { $this->cacheItemPool->save($item->set($this->openApi)); } diff --git a/DependencyInjection/NelmioApiDocExtension.php b/DependencyInjection/NelmioApiDocExtension.php index 7a1193f..5333409 100644 --- a/DependencyInjection/NelmioApiDocExtension.php +++ b/DependencyInjection/NelmioApiDocExtension.php @@ -68,6 +68,7 @@ final class NelmioApiDocExtension extends Extension implements PrependExtensionI $container->register(sprintf('nelmio_api_doc.generator.%s', $area), ApiDocGenerator::class) ->setPublic(true) ->addMethodCall('setAlternativeNames', [$nameAliases]) + ->addMethodCall('setMediaTypes', [$config['media_types']]) ->setArguments([ new TaggedIteratorArgument(sprintf('nelmio_api_doc.describer.%s', $area)), new TaggedIteratorArgument('nelmio_api_doc.model_describer'), @@ -89,7 +90,6 @@ final class NelmioApiDocExtension extends Extension implements PrependExtensionI new Reference('nelmio_api_doc.controller_reflector'), new Reference('annotations.reader'), // We cannot use the cached version of the annotation reader since the construction of the annotations is context dependant... new Reference('logger'), - $config['media_types'], ]) ->addTag(sprintf('nelmio_api_doc.describer.%s', $area), ['priority' => -200]); diff --git a/Describer/DefaultDescriber.php b/Describer/DefaultDescriber.php index 50d6a54..5217d3c 100644 --- a/Describer/DefaultDescriber.php +++ b/Describer/DefaultDescriber.php @@ -34,12 +34,14 @@ final class DefaultDescriber implements DescriberInterface } // Paths - $paths = OA\UNDEFINED === $api->paths ? [] : $api->paths; - foreach ($paths as $path) { + if (OA\UNDEFINED === $api->paths) { + $api->paths = []; + } + foreach ($api->paths as $path) { foreach (Util::OPERATIONS as $method) { /** @var OA\Operation $operation */ $operation = $path->{$method}; - if (OA\UNDEFINED !== $operation && null !== $operation && empty($operation->responses ?? [])) { + if (OA\UNDEFINED !== $operation && null !== $operation && (OA\UNDEFINED === $operation->responses || empty($operation->responses))) { /** @var OA\Response $response */ $response = Util::getIndexedCollectionItem($operation, OA\Response::class, 'default'); $response->description = ''; diff --git a/Describer/OpenApiPhpDescriber.php b/Describer/OpenApiPhpDescriber.php index cf02b7e..128a529 100644 --- a/Describer/OpenApiPhpDescriber.php +++ b/Describer/OpenApiPhpDescriber.php @@ -14,12 +14,9 @@ namespace Nelmio\ApiDocBundle\Describer; use Doctrine\Common\Annotations\Reader; use Nelmio\ApiDocBundle\Annotation\Operation; use Nelmio\ApiDocBundle\Annotation\Security; -use Nelmio\ApiDocBundle\OpenApiPhp\AddDefaults; -use Nelmio\ApiDocBundle\OpenApiPhp\ModelRegister; use Nelmio\ApiDocBundle\OpenApiPhp\Util; use Nelmio\ApiDocBundle\Util\ControllerReflector; use OpenApi\Analyser; -use OpenApi\Analysis; use OpenApi\Annotations as OA; use Psr\Log\LoggerInterface; use Symfony\Component\Routing\Route; @@ -28,49 +25,25 @@ use Symfony\Component\Routing\RouteCollection; // Help opcache.preload discover Swagger\Annotations\Swagger class_exists(OA\OpenApi::class); -final class OpenApiPhpDescriber implements ModelRegistryAwareInterface +final class OpenApiPhpDescriber { - use ModelRegistryAwareTrait; - private $routeCollection; private $controllerReflector; private $annotationReader; private $logger; - private $mediaTypes; private $overwrite; - public function __construct(RouteCollection $routeCollection, ControllerReflector $controllerReflector, Reader $annotationReader, LoggerInterface $logger, array $mediaTypes, bool $overwrite = false) + public function __construct(RouteCollection $routeCollection, ControllerReflector $controllerReflector, Reader $annotationReader, LoggerInterface $logger, bool $overwrite = false) { $this->routeCollection = $routeCollection; $this->controllerReflector = $controllerReflector; $this->annotationReader = $annotationReader; $this->logger = $logger; - $this->mediaTypes = $mediaTypes; $this->overwrite = $overwrite; } public function describe(OA\OpenApi $api) { - $analysis = $this->getAnnotations($api); - $analysis->process($this->getProcessors()); - $analysis->validate(); - } - - private function getProcessors(): array - { - $processors = [ - new AddDefaults(), - new ModelRegister($this->modelRegistry, $this->mediaTypes), - ]; - - return array_merge($processors, Analysis::processors()); - } - - private function getAnnotations(OA\OpenApi $api): Analysis - { - $analysis = new Analysis(); - $analysis->openapi = $api; - $classAnnotations = []; /** @var \ReflectionMethod $method */ @@ -150,9 +123,6 @@ final class OpenApiPhpDescriber implements ModelRegistryAwareInterface continue; } - // Registers new annotations - $analysis->addAnnotations($implicitAnnotations, null); - foreach ($httpMethods as $httpMethod) { $operation = Util::getOperation($path, $httpMethod); $operation->merge($implicitAnnotations); @@ -162,8 +132,6 @@ final class OpenApiPhpDescriber implements ModelRegistryAwareInterface // Reset the Analyser after the parsing Analyser::$context = null; - - return $analysis; } private function getMethodsToParse(): \Generator diff --git a/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php b/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php index 58f28ab..e8eb58d 100644 --- a/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php +++ b/ModelDescriber/Annotations/SymfonyConstraintAnnotationReader.php @@ -59,25 +59,26 @@ class SymfonyConstraintAnnotationReader $this->schema->required = array_values(array_unique($existingRequiredFields)); } elseif ($annotation instanceof Assert\Length) { - $property->minLength = $annotation->min; - $property->maxLength = $annotation->max; + $property->minLength = (int) $annotation->min; + $property->maxLength = (int) $annotation->max; } elseif ($annotation instanceof Assert\Regex) { $this->appendPattern($property, $annotation->getHtmlPattern()); } elseif ($annotation instanceof Assert\Count) { - $property->minItems = $annotation->min; - $property->maxItems = $annotation->max; + $property->minItems = (int) $annotation->min; + $property->maxItems = (int) $annotation->max; } elseif ($annotation instanceof Assert\Choice) { $values = $annotation->callback ? call_user_func(is_array($annotation->callback) ? $annotation->callback : [$reflectionProperty->class, $annotation->callback]) : $annotation->choices; $property->enum = array_values($values); } elseif ($annotation instanceof Assert\Expression) { $this->appendPattern($property, $annotation->message); } elseif ($annotation instanceof Assert\Range) { - $property->minimum = $annotation->min; - $property->maximum = $annotation->max; + $property->minimum = (int) $annotation->min; + $property->maximum = (int) $annotation->max; } elseif ($annotation instanceof Assert\LessThan) { - $property->exclusiveMaximum= $annotation->value; + $property->exclusiveMaximum = true; + $property->maximum = (int) $annotation->value; } elseif ($annotation instanceof Assert\LessThanOrEqual) { - $property->maximum = $annotation->value; + $property->maximum = (int) $annotation->value; } } } diff --git a/OpenApiPhp/AddDefaults.php b/OpenApiPhp/AddDefaults.php deleted file mode 100644 index 2dc53ba..0000000 --- a/OpenApiPhp/AddDefaults.php +++ /dev/null @@ -1,39 +0,0 @@ -getAnnotationsOfType(OA\Info::class)) { - return; - } - if (($annotations = $analysis->getAnnotationsOfType(OA\OpenApi::class)) && OA\UNDEFINED !== $annotations[0]->info) { - return; - } - if (OA\UNDEFINED !== $analysis->openapi->info) { - return; - } - - $analysis->addAnnotation(new OA\Info(['title' => '', 'version' => '0.0.0', '_context' => new Context(['generated' => true])]), null); - } -} diff --git a/OpenApiPhp/Util.php b/OpenApiPhp/Util.php index 0065173..9ca9690 100644 --- a/OpenApiPhp/Util.php +++ b/OpenApiPhp/Util.php @@ -360,6 +360,8 @@ final class Util */ public static function createContext(array $properties = [], Context $parent = null): Context { + $properties['comment'] = ''; // TODO: remove this when https://github.com/zircote/swagger-php/commit/708a25208797ca05ebeae572bbccad8b13de14d8 is released + return new Context($properties, $parent); } diff --git a/PropertyDescriber/ObjectPropertyDescriber.php b/PropertyDescriber/ObjectPropertyDescriber.php index 45c1da3..eb5d4bc 100644 --- a/PropertyDescriber/ObjectPropertyDescriber.php +++ b/PropertyDescriber/ObjectPropertyDescriber.php @@ -27,7 +27,7 @@ class ObjectPropertyDescriber implements PropertyDescriberInterface, ModelRegist if ($types[0]->isNullable()) { $property->nullable = true; - $property->allOf = [['$ref' => $this->modelRegistry->register(new Model($type, $groups))]]; + $property->allOf = [new OA\Schema(['ref' => $this->modelRegistry->register(new Model($type, $groups))])]; return; } diff --git a/Tests/ApiDocGeneratorTest.php b/Tests/ApiDocGeneratorTest.php index 77772a4..ec51d4c 100644 --- a/Tests/ApiDocGeneratorTest.php +++ b/Tests/ApiDocGeneratorTest.php @@ -23,7 +23,7 @@ class ApiDocGeneratorTest extends TestCase $adapter = new ArrayAdapter(); $generator = new ApiDocGenerator([new DefaultDescriber()], [], $adapter); - $this->assertEquals($generator->generate(), $adapter->getItem('openapi_doc')->get()); + $this->assertEquals(json_encode($generator->generate()), json_encode($adapter->getItem('openapi_doc')->get())); } public function testCacheWithCustomId() @@ -31,6 +31,6 @@ class ApiDocGeneratorTest extends TestCase $adapter = new ArrayAdapter(); $generator = new ApiDocGenerator([new DefaultDescriber()], [], $adapter, 'custom_id'); - $this->assertEquals($generator->generate(), $adapter->getItem('custom_id')->get()); + $this->assertEquals(json_encode($generator->generate()), json_encode($adapter->getItem('custom_id')->get())); } } diff --git a/Tests/Functional/Controller/ApiController.php b/Tests/Functional/Controller/ApiController.php index f916455..072d57d 100644 --- a/Tests/Functional/Controller/ApiController.php +++ b/Tests/Functional/Controller/ApiController.php @@ -30,10 +30,12 @@ use Symfony\Component\Routing\Annotation\Route; class ApiController { /** - * @OA\Response( + * @OA\Get( + * @OA\Response( * response="200", * description="Success", * @Model(type=Article::class, groups={"light"})) + * ) * ) * @OA\Parameter(ref="#/components/parameters/test") * @Route("/article/{id}", methods={"GET"}) diff --git a/Tests/Functional/FunctionalTest.php b/Tests/Functional/FunctionalTest.php index b6f317c..8b8c7dd 100644 --- a/Tests/Functional/FunctionalTest.php +++ b/Tests/Functional/FunctionalTest.php @@ -383,7 +383,8 @@ class FunctionalTest extends WebTestCase ], 'propertyLessThan' => [ 'type' => 'integer', - 'exclusiveMaximum' => 42, + 'exclusiveMaximum' => true, + 'maximum' => 42, ], 'propertyLessThanOrEqual' => [ 'type' => 'integer',