From 1a17a5f5da82bcd83b8a61c1f58d6be39f4f5f1d Mon Sep 17 00:00:00 2001 From: Guilhem N Date: Fri, 30 Dec 2016 13:37:02 +0100 Subject: [PATCH] Allow implicit operations with SwaggerPhp annotations --- .php_cs.dist | 6 +- .../Compiler/AddRouteDescribersPass.php | 2 +- .../Compiler/PriorityTaggedServiceTrait.php | 6 +- Describer/RouteDescriber.php | 68 +----- Describer/SwaggerPhpDescriber.php | 23 ++- Resources/config/services.xml | 8 +- Resources/config/swagger_php.xml | 12 ++ SwaggerPhp/OperationResolver.php | 195 ++++++++++++++++++ Tests/Describer/RouteDescriberTest.php | 7 +- .../Fixtures/SwaggerPhp/{Info.php => Api.php} | 6 +- Tests/Functional/Controller/ApiController.php | 25 +++ Tests/Functional/FunctionalTest.php | 44 +++- Util/ControllerReflector.php | 106 ++++++++++ composer.json | 2 +- 14 files changed, 435 insertions(+), 75 deletions(-) create mode 100644 SwaggerPhp/OperationResolver.php rename Tests/Fixtures/SwaggerPhp/{Info.php => Api.php} (81%) create mode 100644 Util/ControllerReflector.php diff --git a/.php_cs.dist b/.php_cs.dist index ebd0219..2bb967f 100644 --- a/.php_cs.dist +++ b/.php_cs.dist @@ -2,7 +2,7 @@ $finder = PhpCsFixer\Finder::create() ->in(__DIR__) - ->exclude('tests/Fixtures/app/cache') + ->exclude('Tests/Functional/cache') ; return PhpCsFixer\Config::create() @@ -11,14 +11,14 @@ return PhpCsFixer\Config::create() 'ordered_imports' => true, 'phpdoc_order' => true, 'header_comment' => [ - 'header' => << <<
setFinder($finder) diff --git a/DependencyInjection/Compiler/AddRouteDescribersPass.php b/DependencyInjection/Compiler/AddRouteDescribersPass.php index 0e0ec19..64ddde3 100644 --- a/DependencyInjection/Compiler/AddRouteDescribersPass.php +++ b/DependencyInjection/Compiler/AddRouteDescribersPass.php @@ -22,6 +22,6 @@ class AddRouteDescribersPass implements CompilerPassInterface { $routeDescribers = $this->findAndSortTaggedServices('nelmio_api_doc.route_describer', $container); - $container->getDefinition('nelmio_api_doc.describers.route')->replaceArgument(3, $routeDescribers); + $container->getDefinition('nelmio_api_doc.describers.route')->replaceArgument(2, $routeDescribers); } } diff --git a/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php b/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php index 1d7f7e2..cbd22dd 100644 --- a/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php +++ b/DependencyInjection/Compiler/PriorityTaggedServiceTrait.php @@ -1,8 +1,9 @@ + * (c) Nelmio * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. @@ -50,6 +51,7 @@ trait PriorityTaggedServiceTrait krsort($services); $services = call_user_func_array('array_merge', $services); } + return $services; } } diff --git a/Describer/RouteDescriber.php b/Describer/RouteDescriber.php index 6ebffdb..eb7dca6 100644 --- a/Describer/RouteDescriber.php +++ b/Describer/RouteDescriber.php @@ -11,32 +11,27 @@ namespace Nelmio\ApiDocBundle\Describer; -use Doctrine\Common\Util\ClassUtils; use EXSyst\Component\Swagger\Swagger; use Nelmio\ApiDocBundle\RouteDescriber\RouteDescriberInterface; -use Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser; -use Symfony\Component\DependencyInjection\ContainerInterface; +use Nelmio\ApiDocBundle\Util\ControllerReflector; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; final class RouteDescriber implements DescriberInterface { - private $container; private $routeCollection; - private $controllerNameParser; + private $controllerReflector; private $routeDescribers; /** - * @param ContainerInterface $container * @param RouteCollection $routeCollection - * @param ControllerNameParser $controllerNameParser + * @param ControllerReflector $controllerReflector * @param RouteDescriberInterface[] $routeDescribers */ - public function __construct(ContainerInterface $container, RouteCollection $routeCollection, ControllerNameParser $controllerNameParser, array $routeDescribers) + public function __construct(RouteCollection $routeCollection, ControllerReflector $controllerReflector, array $routeDescribers) { - $this->container = $container; $this->routeCollection = $routeCollection; - $this->controllerNameParser = $controllerNameParser; + $this->controllerReflector = $controllerReflector; $this->routeDescribers = $routeDescribers; } @@ -47,8 +42,13 @@ final class RouteDescriber implements DescriberInterface } foreach ($this->routeCollection->all() as $route) { + if (!$route->hasDefault('_controller')) { + continue; + } + // if able to resolve the controller - if ($method = $this->getReflectionMethod($route->getDefault('_controller') ?? '')) { + $controller = $route->getDefault('_controller'); + if ($method = $this->controllerReflector->getReflectionMethod($controller)) { // Extract as many informations as possible about this route foreach ($this->routeDescribers as $describer) { $describer->describe($api, $route, $method); @@ -56,50 +56,4 @@ final class RouteDescriber implements DescriberInterface } } } - - /** - * Returns the ReflectionMethod for the given controller string. - * - * @param string $controller - * - * @return \ReflectionMethod|null - */ - private function getReflectionMethod(string $controller) - { - if (false === strpos($controller, '::') && 2 === substr_count($controller, ':')) { - $controller = $this->controllerNameParser->parse($controller); - } - - if (preg_match('#(.+)::([\w]+)#', $controller, $matches)) { - $class = $matches[1]; - $method = $matches[2]; - } elseif (class_exists($controller)) { - $class = $controller; - $method = '__invoke'; - } else { - if (preg_match('#(.+):([\w]+)#', $controller, $matches)) { - $controller = $matches[1]; - $method = $matches[2]; - } - - if ($this->container->has($controller)) { - if (class_exists(ClassUtils::class)) { - $class = ClassUtils::getRealClass(get_class($this->container->get($controller))); - } - - if (!isset($method) && method_exists($class, '__invoke')) { - $method = '__invoke'; - } - } - } - - if (isset($class) && isset($method)) { - try { - return new \ReflectionMethod($class, $method); - } catch (\ReflectionException $e) { - // In case we can't reflect the controller, we just - // ignore the route - } - } - } } diff --git a/Describer/SwaggerPhpDescriber.php b/Describer/SwaggerPhpDescriber.php index d68735c..2716a08 100644 --- a/Describer/SwaggerPhpDescriber.php +++ b/Describer/SwaggerPhpDescriber.php @@ -11,12 +11,17 @@ namespace Nelmio\ApiDocBundle\Describer; +use Nelmio\ApiDocBundle\SwaggerPhp\OperationResolver; +use Swagger\Analysis; + final class SwaggerPhpDescriber extends ExternalDocDescriber { + private $operationResolver; + public function __construct(string $projectPath, bool $overwrite = false) { parent::__construct(function () use ($projectPath) { - // Catch notices as the documentation can be completed by other describers + // Ignore notices as the documentation can be completed by other describers $prevHandler = set_error_handler(function ($type, $message, $file, $line, $context) use (&$prevHandler) { if (E_USER_NOTICE === $type || E_USER_WARNING === $type) { return; @@ -26,7 +31,12 @@ final class SwaggerPhpDescriber extends ExternalDocDescriber }); try { - $annotation = \Swagger\scan($projectPath); + $options = []; + if (null !== $this->operationResolver) { + $options['processors'] = array_merge([$this->operationResolver], Analysis::processors()); + } + + $annotation = \Swagger\scan($projectPath, $options); return json_decode(json_encode($annotation)); } finally { @@ -34,4 +44,13 @@ final class SwaggerPhpDescriber extends ExternalDocDescriber } }, $overwrite); } + + /** + * If set, the describer will try to complete paths and create + * implicit operations. + */ + public function setOperationResolver(OperationResolver $operationResolver) + { + $this->operationResolver = $operationResolver; + } } diff --git a/Resources/config/services.xml b/Resources/config/services.xml index 70ee49b..1a7296e 100644 --- a/Resources/config/services.xml +++ b/Resources/config/services.xml @@ -8,6 +8,11 @@ + + + + + @@ -15,7 +20,6 @@ - @@ -26,7 +30,7 @@ - + diff --git a/Resources/config/swagger_php.xml b/Resources/config/swagger_php.xml index 1faa1d1..0ef8082 100644 --- a/Resources/config/swagger_php.xml +++ b/Resources/config/swagger_php.xml @@ -6,9 +6,21 @@ %kernel.root_dir% + + + + + + + + + + + + diff --git a/SwaggerPhp/OperationResolver.php b/SwaggerPhp/OperationResolver.php new file mode 100644 index 0000000..f7220c5 --- /dev/null +++ b/SwaggerPhp/OperationResolver.php @@ -0,0 +1,195 @@ +routeCollection = $routeCollection; + $this->controllerReflector = $controllerReflector; + } + + public function __invoke(Analysis $analysis) + { + $this->resolveOperationsPath($analysis); + $this->createImplicitOperations($analysis); + } + + private function resolveOperationsPath(Analysis $analysis) + { + $operations = $analysis->getAnnotationsOfType(SWG\Operation::class); + foreach ($operations as $operation) { + if (null !== $operation->path || $operation->_context->not('method')) { + continue; + } + + $paths = $this->getPaths($operation->_context, $operation->method); + if (0 === count($paths)) { + continue; + } + + // Define the path of the first annotation + $operation->path = array_pop($paths); + + // If there are other paths, clone the annotation + foreach ($paths as $path) { + $alias = clone $operation; + $alias->path = $path; + + $analysis->addAnnotation($alias, $alias->_context); + } + } + } + + private function createImplicitOperations(Analysis $analysis) + { + $annotations = array_merge($analysis->getAnnotationsOfType(SWG\Response::class), $analysis->getAnnotationsOfType(SWG\Parameter::class), $analysis->getAnnotationsOfType(SWG\ExternalDocumentation::class)); + $map = []; + foreach ($annotations as $annotation) { + $context = $annotation->_context; + if ($context->not('method')) { + continue; + } + + $class = $this->getClass($context); + $method = $context->method; + + $id = $class.'|'.$method; + if (!isset($map[$id])) { + $map[$id] = []; + } + + $map[$id][] = $annotation; + } + + $operationAnnotations = [ + 'get' => SWG\Get::class, + 'post' => SWG\Post::class, + 'put' => SWG\Put::class, + 'patch' => SWG\Patch::class, + 'delete' => SWG\Delete::class, + 'options' => SWG\Options::class, + 'head' => SWG\Head::class, + ]; + foreach ($map as $id => $annotations) { + $context = $annotations[0]->_context; + $httpMethods = $this->getHttpMethods($context); + foreach ($httpMethods as $httpMethod => $paths) { + $annotationClass = $operationAnnotations[$httpMethod]; + foreach ($paths as $path => $v) { + $operation = new $annotationClass(['path' => $path, 'value' => $annotations], $context); + $analysis->addAnnotation($operation, $context); + } + } + + foreach ($annotations as $annotation) { + $analysis->annotations->detach($annotation); + } + } + } + + private function getPaths(Context $context, string $httpMethod): array + { + $httpMethods = $this->getHttpMethods($context); + if (!isset($httpMethods[$httpMethod])) { + return []; + } + + return array_keys($httpMethods[$httpMethod]); + } + + private function getHttpMethods(Context $context) + { + if (null === $this->controllerMap) { + $this->buildMap(); + } + + $class = $this->getClass($context); + $method = $context->method; + + // Checks if a route corresponds to this method + if (!isset($this->controllerMap[$class][$method])) { + return []; + } + + return $this->controllerMap[$class][$method]; + } + + private function getClass(Context $context) + { + return ltrim($context->namespace.'\\'.$context->class, '\\'); + } + + private function buildMap() + { + $this->controllerMap = []; + foreach ($this->routeCollection->all() as $route) { + if (!$route->hasDefault('_controller')) { + continue; + } + + $controller = $route->getDefault('_controller'); + if ($callable = $this->controllerReflector->getReflectionClassAndMethod($controller)) { + list($class, $method) = $callable; + $class = $class->name; + $method = $method->name; + + if (!isset($this->controllerMap[$class])) { + $this->controllerMap[$class] = []; + } + if (!isset($this->controllerMap[$class][$method])) { + $this->controllerMap[$class][$method] = []; + } + + $httpMethods = $route->getMethods() ?: Swagger::$METHODS; + foreach ($httpMethods as $httpMethod) { + $httpMethod = strtolower($httpMethod); + if (!isset($this->controllerMap[$class][$method][$httpMethod])) { + $this->controllerMap[$class][$method][$httpMethod] = []; + } + + $path = $this->normalizePath($route->getPath()); + $this->controllerMap[$class][$method][$httpMethod][$path] = true; + } + } + } + } + + private function normalizePath(string $path) + { + if (substr($path, -10) === '.{_format}') { + $path = substr($path, 0, -10); + } + + return $path; + } +} diff --git a/Tests/Describer/RouteDescriberTest.php b/Tests/Describer/RouteDescriberTest.php index 1943959..27ba812 100644 --- a/Tests/Describer/RouteDescriberTest.php +++ b/Tests/Describer/RouteDescriberTest.php @@ -14,6 +14,7 @@ namespace Nelmio\ApiDocBundle\Tests\Describer; use EXSyst\Component\Swagger\Swagger; use Nelmio\ApiDocBundle\Describer\RouteDescriber; use Nelmio\ApiDocBundle\RouteDescriber\RouteDescriberInterface; +use Nelmio\ApiDocBundle\Util\ControllerReflector; use Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser; use Symfony\Component\DependencyInjection\Container; use Symfony\Component\Routing\Route; @@ -38,9 +39,11 @@ class RouteDescriberTest extends AbstractDescriberTest $this->routeDescriber = $this->createMock(RouteDescriberInterface::class); $this->routes = new RouteCollection(); $this->describer = new RouteDescriber( - new Container(), $this->routes, - $this->createMock(ControllerNameParser::class), + new ControllerReflector( + new Container(), + $this->createMock(ControllerNameParser::class) + ), [$this->routeDescriber] ); } diff --git a/Tests/Fixtures/SwaggerPhp/Info.php b/Tests/Fixtures/SwaggerPhp/Api.php similarity index 81% rename from Tests/Fixtures/SwaggerPhp/Info.php rename to Tests/Fixtures/SwaggerPhp/Api.php index 0d3e607..b1b8efb 100644 --- a/Tests/Fixtures/SwaggerPhp/Info.php +++ b/Tests/Fixtures/SwaggerPhp/Api.php @@ -11,14 +11,14 @@ namespace Nelmio\ApiDocBundle\Tests\Functional\Fixtures\SwaggerPhp; -use Swagger\Annotations\Info as InfoAnnotation; +use Swagger\Annotations\Info; /** - * @InfoAnnotation( + * @Info( * title="My Awesome App", * version="1.3" * ) */ -class Info +class Api { } diff --git a/Tests/Functional/Controller/ApiController.php b/Tests/Functional/Controller/ApiController.php index 80a65f0..424f660 100644 --- a/Tests/Functional/Controller/ApiController.php +++ b/Tests/Functional/Controller/ApiController.php @@ -15,12 +15,37 @@ use FOS\RestBundle\Controller\Annotations\QueryParam; use FOS\RestBundle\Controller\Annotations\RequestParam; use Nelmio\ApiDocBundle\Annotation\ApiDoc; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route; +use Swagger\Annotations as SWG; /** * @Route("/api") */ class ApiController { + /** + * @Route("/swagger", methods={"GET"}) + * @Route("/swagger2", methods={"GET"}) + * @SWG\Get( + * @SWG\Response(response="201", description="An example resource") + * ) + */ + public function swaggerAction() + { + } + + /** + * @Route("/swagger/implicit", methods={"GET", "POST"}) + * @SWG\Response(response="201", description="Operation automatically detected") + * @SWG\Parameter( + * name="foo", + * in="query", + * description="This is a parameter" + * ) + */ + public function implicitSwaggerAction() + { + } + /** * @Route("/test/{user}", methods={"GET"}, schemes={"https"}, requirements={"user"="/foo/"}) */ diff --git a/Tests/Functional/FunctionalTest.php b/Tests/Functional/FunctionalTest.php index 9c0780d..ccbf223 100644 --- a/Tests/Functional/FunctionalTest.php +++ b/Tests/Functional/FunctionalTest.php @@ -22,6 +22,46 @@ class FunctionalTest extends WebTestCase $this->assertFalse($paths->has('/api/admin')); } + /** + * Tests that the paths are automatically resolved in Swagger annotations. + * + * @dataProvider swaggerActionPathsProvider + */ + public function testSwaggerAction($path) + { + $operation = $this->getOperation($path, 'get'); + + $responses = $operation->getResponses(); + $this->assertTrue($responses->has('201')); + $this->assertEquals('An example resource', $responses->get('201')->getDescription()); + } + + public function swaggerActionPathsProvider() + { + return [['/api/swagger'], ['/api/swagger2']]; + } + + /** + * @dataProvider implicitSwaggerActionMethodsProvider + */ + public function testImplicitSwaggerAction($method) + { + $operation = $this->getOperation('/api/swagger/implicit', $method); + + $responses = $operation->getResponses(); + $this->assertTrue($responses->has('201')); + $this->assertEquals('Operation automatically detected', $responses->get('201')->getDescription()); + + $parameters = $operation->getParameters(); + $this->assertTrue($parameters->has('foo', 'query')); + $this->assertEquals('This is a parameter', $parameters->get('foo', 'query')->getDescription()); + } + + public function implicitSwaggerActionMethodsProvider() + { + return [['get'], ['post']]; + } + public function testUserAction() { $operation = $this->getOperation('/api/test/{user}', 'get'); @@ -93,10 +133,10 @@ class FunctionalTest extends WebTestCase $api = $this->getSwaggerDefinition(); $paths = $api->getPaths(); - $this->assertTrue($paths->has($path)); + $this->assertTrue($paths->has($path), sprintf('Path "%s" does not exist', $path)); $action = $paths->get($path); - $this->assertTrue($action->hasOperation($method)); + $this->assertTrue($action->hasOperation($method), sprintf('Operation "%s" for path "%s" does not exist', $path, $method)); return $action->getOperation($method); } diff --git a/Util/ControllerReflector.php b/Util/ControllerReflector.php new file mode 100644 index 0000000..a4b7d0f --- /dev/null +++ b/Util/ControllerReflector.php @@ -0,0 +1,106 @@ +container = $container; + $this->controllerNameParser = $controllerNameParser; + } + + /** + * Returns the ReflectionMethod for the given controller string. + * + * @param string $controller + * + * @return \ReflectionMethod|null + */ + public function getReflectionMethod(string $controller) + { + $callable = $this->getClassAndMethod($controller); + if (null === $callable) { + return; + } + + list($class, $method) = $callable; + try { + return new \ReflectionMethod($class, $method); + } catch (\ReflectionException $e) { + // In case we can't reflect the controller, we just + // ignore the route + } + } + + public function getReflectionClassAndMethod(string $controller) + { + $callable = $this->getClassAndMethod($controller); + if (null === $callable) { + return; + } + + list($class, $method) = $callable; + try { + return [new \ReflectionClass($class), new \ReflectionMethod($class, $method)]; + } catch (\ReflectionException $e) { + // In case we can't reflect the controller, we just + // ignore the route + } + } + + private function getClassAndMethod(string $controller) + { + if (false === strpos($controller, '::') && 2 === substr_count($controller, ':')) { + $controller = $this->controllerNameParser->parse($controller); + } + + if (preg_match('#(.+)::([\w]+)#', $controller, $matches)) { + $class = $matches[1]; + $method = $matches[2]; + } elseif (class_exists($controller)) { + $class = $controller; + $method = '__invoke'; + } else { + if (preg_match('#(.+):([\w]+)#', $controller, $matches)) { + $controller = $matches[1]; + $method = $matches[2]; + } + + if ($this->container->has($controller)) { + $class = get_class($this->container->get($controller)); + if (class_exists(ClassUtils::class)) { + $class = ClassUtils::getRealClass($class); + } + + if (!isset($method) && method_exists($class, '__invoke')) { + $method = '__invoke'; + } + } + } + + if (!isset($class) || !isset($method)) { + return; + } + + return [$class, $method]; + } +} diff --git a/composer.json b/composer.json index e508fd0..0622946 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,7 @@ "require": { "php": "~7.0|~7.1", "symfony/framework-bundle": "^2.8|^3.0", - "exsyst/swagger": "~0.2.2" + "exsyst/swagger": "~0.2.3" }, "require-dev": { "symfony/twig-bundle": "^2.8|^3.0",