From 15bceb4e187185d9a733d09b0fedc5412d8fb5dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Fri, 8 Feb 2019 01:14:37 +0200 Subject: [PATCH] Fix defaults (#7) * adding defaults to paths that can't be reflected on * moving phpstan to dist file --- .travis.yml | 2 +- install-phalcon.sh | 2 +- phpstan.neon => phpstan.neon.dist | 11 +- phpunit.xml | 15 ++ src/Implementations/Controller.php | 3 +- src/Implementations/DefaultResponse.php | 31 +++ src/Implementations/PhalconPath2PathArray.php | 17 +- src/Implementations/Reflector.php | 15 +- test/DefaultResponseTest.php | 44 ++++ test/Path2PathTest.php | 192 +++++++++++++++--- test/ReflectorTest.php | 35 +++- 11 files changed, 315 insertions(+), 52 deletions(-) rename phpstan.neon => phpstan.neon.dist (54%) create mode 100644 phpunit.xml create mode 100644 src/Implementations/DefaultResponse.php create mode 100644 test/DefaultResponseTest.php diff --git a/.travis.yml b/.travis.yml index 64559d3..deab472 100644 --- a/.travis.yml +++ b/.travis.yml @@ -115,7 +115,7 @@ jobs: before_install: - phpenv config-rm xdebug.ini || true - mkdir ../tools && composer init --name=putg/tools --working-dir=../tools - - composer require phpstan/phpstan:^0.10 phpstan/phpstan-phpunit phpstan/phpstan-strict-rules --working-dir=../tools + - composer require phpstan/phpstan:^0.11 phpstan/phpstan-phpunit phpstan/phpstan-strict-rules:^0.11 --working-dir=../tools - sh install-phalcon.sh install: - composer update diff --git a/install-phalcon.sh b/install-phalcon.sh index f3d662e..0490fd5 100644 --- a/install-phalcon.sh +++ b/install-phalcon.sh @@ -2,7 +2,7 @@ mkdir tmp && \ mv composer.json tmp/ && \ composer require --dev techpivot/phalcon-ci-installer && \ -vendor/bin/install-phalcon.sh && \ +vendor/bin/install-phalcon.sh 3.2.x && \ rm composer.lock && \ rm composer.json && \ mv tmp/composer.json ./ && \ diff --git a/phpstan.neon b/phpstan.neon.dist similarity index 54% rename from phpstan.neon rename to phpstan.neon.dist index c86bb97..f6d8138 100644 --- a/phpstan.neon +++ b/phpstan.neon.dist @@ -1,10 +1,11 @@ +includes: +- ../tools/vendor/phpstan/phpstan-phpunit/extension.neon +- ../tools/vendor/phpstan/phpstan-phpunit/rules.neon +- ../tools/vendor/phpstan/phpstan-strict-rules/rules.neon parameters: ignoreErrors: - '#^Parameter \#3 \$subject of function preg_replace expects array\|string, array\|string\|null given\.$#' - '#^Parameter \#3 \$subject of function preg_replace expects array\|string, string\|null given\.$#' - '#^Access to an undefined property De\\Idrinth\\PhalconRoutes2OpenApi\\Implementations\\Controller::\$(router|request|di|response)\.$#' - - '#^Parameter \#4 ...\$sets of method De\\Idrinth\\PhalconRoutes2OpenApi\\Implementations\\NoValueConversionMerger::mergeAll\(\) expects array, string given\.$#' -includes: -- ../tools/vendor/phpstan/phpstan-phpunit/extension.neon -- ../tools/vendor/phpstan/phpstan-phpunit/rules.neon -- ../tools/vendor/phpstan/phpstan-strict-rules/rules.neon + - '#^Parameter \#4 \.\.\.\$sets of method De\\Idrinth\\PhalconRoutes2OpenApi\\Implementations\\NoValueConversionMerger::mergeAll\(\) expects array, string given\.$#' + - '#^Parameter \#1 \$sets \(array\) of method De\\Idrinth\\PhalconRoutes2OpenApi\\Implementations\\NoValueConversionMerger::mergeAll\(\) should be contravariant with parameter \$sets \(array\) of method De\\Idrinth\\PhalconRoutes2OpenApi\\Interfaces\\RecursiveMerger::mergeAll\(\)$#' diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 0000000..0cfef1a --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,15 @@ + + + + test + + + + + src + + + \ No newline at end of file diff --git a/src/Implementations/Controller.php b/src/Implementations/Controller.php index 85ab20b..768faf8 100644 --- a/src/Implementations/Controller.php +++ b/src/Implementations/Controller.php @@ -34,8 +34,9 @@ class Controller extends PhalconController implements ControllerInterface public function index(): ResponseInterface { $paths = []; + $converter = $this->di->get(Path2PathConverter::class); foreach ($this->router->getRoutes() as $route) { - $paths[] = $this->di->get(Path2PathConverter::class)->convert($route); + $paths[] = $converter->convert($route); } $merger = $this->di->get(RecursiveMerger::class); return $this diff --git a/src/Implementations/DefaultResponse.php b/src/Implementations/DefaultResponse.php new file mode 100644 index 0000000..2d26b94 --- /dev/null +++ b/src/Implementations/DefaultResponse.php @@ -0,0 +1,31 @@ + [ + "description" => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ]; + } + $route['summary'] = $route['summary'] ?? ''; + $route['description'] = $route['description'] ?? ''; + return $route; + } +} diff --git a/src/Implementations/PhalconPath2PathArray.php b/src/Implementations/PhalconPath2PathArray.php index 354f6db..151afd4 100644 --- a/src/Implementations/PhalconPath2PathArray.php +++ b/src/Implementations/PhalconPath2PathArray.php @@ -65,13 +65,14 @@ private function handleParams(string &$path, array &$openapi) $parts = explode(':', substr($match, 1, -1), 2); $param = [ 'in' => 'path', - 'name' => $parts[0] + 'name' => $parts[0], + 'required' => true, + 'schema' => [ + 'type' => 'string', + 'pattern' => count($parts) === 2 ? $parts[1] : '.+' + ] ]; if (count($parts) === 2) { - $param['schema'] = [ - 'type' => 'string', - 'pattern' => $parts[1] - ]; $path = str_replace($match, '{'.$parts[0].'}', $path); } $openapi['parameters'][] = $param; @@ -94,6 +95,7 @@ private function handleQuery(string &$path, array &$openapi, RouteInterface $rou $openapi['parameters'][] = [ 'name' => $name, 'in' => 'path', + 'required' => true, 'schema' => [ 'type' => 'string', 'pattern' => $match @@ -120,9 +122,14 @@ public function convert(RouteInterface $route):array (string) ($route->getPaths()['controller'] ?? ''), (string) ($route->getPaths()['action'] ?? '') ); + $methods = ['get']; foreach ((array)$route->getHttpMethods() as $method) { + $methods[] = strtolower($method); $openapi[strtolower($method)] = $this->merger->merge((array) ($openapi[strtolower($method)]??[]), $data); } + foreach (array_unique($methods) as $method) { + $openapi[$method] = DefaultResponse::add($openapi[$method]??[]); + } ksort($openapi); return [$path => $openapi]; } diff --git a/src/Implementations/Reflector.php b/src/Implementations/Reflector.php index 235aa82..8cb6a0c 100644 --- a/src/Implementations/Reflector.php +++ b/src/Implementations/Reflector.php @@ -49,16 +49,17 @@ public function __invoke(string $class, string $method):array $this->cache[$class]['____class'] = new ReflectionClass($class); } if (!isset($this->cache[$class][$method])) { - $this->cache[$class][$method] = $this->getReflect($this->cache[$class]['____class'], $method); + $this->cache[$class][$method] = DefaultResponse::add( + $this->getReflect($this->cache[$class]['____class'], $method) + ); } - return $this->cache[$class][$method]; } catch (Exception $e) { - return [ - "description" => '', - "summary" => '', - "responses" => [] - ]; + $this->cache[$class][$method] = DefaultResponse::add([ + "summary" => 'unretrievable definition', + "description" => "$class::$method could not be reflected on.", + ]); } + return $this->cache[$class][$method]; } /** diff --git a/test/DefaultResponseTest.php b/test/DefaultResponseTest.php new file mode 100644 index 0000000..6cde833 --- /dev/null +++ b/test/DefaultResponseTest.php @@ -0,0 +1,44 @@ + '', + 'description' => '', + 'responses' => [ + '200' => ['description' => 'unknown return','content' => ['*/*' => ['schema' => new stdClass()]]] + ] + ]; + return [ + 'empty' => [[], $default], + 'existing summary' => [['summary' => 'a'], array_merge($default, ['summary' => 'a'])], + 'existing description' => [['description' => 'b'], array_merge($default, ['description' => 'b'])], + 'existing response' => [ + ['responses' => ['200' => []]], + array_merge($default, ['responses' => ['200' => []]]) + ], + ]; + } + + /** + * @dataProvider provideAdd + * @param array $route + * @param array $expectation + * @return void + */ + public function testAdd(array $route, array $expectation) + { + self::assertEquals($expectation, DefaultResponse::add($route)); + } +} diff --git a/test/Path2PathTest.php b/test/Path2PathTest.php index d1db1cc..2fb65d3 100644 --- a/test/Path2PathTest.php +++ b/test/Path2PathTest.php @@ -7,6 +7,7 @@ use De\Idrinth\PhalconRoutes2OpenApi\Interfaces\PathTargetAnnotationResolver; use Phalcon\Mvc\Router\RouteInterface; use PHPUnit\Framework\TestCase; +use stdClass; class Path2PathTest extends TestCase { @@ -37,70 +38,129 @@ public function provideConvert(): array { return [ [ - $this->makeRoute('/', ['GET']), + '/', + ['GET'], + 0, + [], [ "/" => [ "description" => "", "get" => [ "description" => '', "summary" => '', - "responses" => [] + "responses" => [ + '200' => [ + "description" => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ] ] ] ], [ - $this->makeRoute('/{var}/', ['GET']), + '/{var}/', + ['GET'], + 0, + [], [ "/{var}/" => [ "description" => "", "get" => [ "description" => '', "summary" => '', - "responses" => [] + "responses" => [ + '200' => [ + "description" => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ], "parameters" => [ [ "name" => "var", - "in" => "path" + "in" => "path", + "required" => true, + 'schema' => [ + 'type' => 'string', + 'pattern' => '.+' + ], ] ] ] ] ], [ - $this->makeRoute('#^/request/{id}/$#u', ['GET']), + '#^/request/{id}/$#u', + ['GET'], + 0, + [], [ "/request/{id}/" => [ "description" => "", "get" => [ "description" => '', "summary" => '', - "responses" => [] + "responses" => [ + '200' => [ + "description" => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ], "parameters" => [ [ "name" => "id", - "in" => "path" + "in" => "path", + "required" => true, + 'schema' => [ + 'type' => 'string', + 'pattern' => '.+' + ], ] ] ] ] ], [ - $this->makeRoute('/admin/:controller/a/:action/:params/', ['GET']), + '/admin/:controller/a/:action/:params/', + ['GET'], + 0, + [], [ "/admin/{controller}/a/{action}/" => [ "description" => "", "get" => [ "description" => '', "summary" => '', - "responses" => [] + "responses" => [ + '200' => [ + "description" => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ], "parameters" => [ [ "name" => "controller", "in" => "path", + "required" => true, "schema" => [ "type" => "string", "pattern" => "([a-zA-Z0-9\_\-]+)" @@ -109,6 +169,7 @@ public function provideConvert(): array [ "name" => "action", "in" => "path", + "required" => true, "schema" => [ "type" => "string", "pattern" => "([a-zA-Z0-9\_\-]+)" @@ -119,19 +180,32 @@ public function provideConvert(): array ] ], [ - $this->makeRoute('/{var:[0-9]+}/', ['GET']), + '/{var:[0-9]+}/', + ['GET'], + 0, + [], [ "/{var}/" => [ "description" => "", "get" => [ "description" => '', "summary" => '', - "responses" => [] + "responses" => [ + '200' => [ + "description" => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ], "parameters" => [ [ "name" => "var", "in" => "path", + "required" => true, "schema" => [ "type" => "string", "pattern" => "[0-9]+" @@ -142,19 +216,32 @@ public function provideConvert(): array ] ], [ - $this->makeRoute('/([0-9a-z]+)/', ['GET'], 1, [1 => 'var']), + '/([0-9a-z]+)/', + ['GET'], + 1, + [1 => 'var'], [ "/{var}/" => [ "description" => "", "get" => [ "description" => '', "summary" => '', - "responses" => [] + "responses" => [ + '200' => [ + "description" => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ], "parameters" => [ [ "name" => "var", "in" => "path", + "required" => true, "schema" => [ "type" => "string", "pattern" => "[0-9a-z]+" @@ -165,19 +252,32 @@ public function provideConvert(): array ] ], [ - $this->makeRoute('/([0-9a-z]+)/hi/([0-9a-z]+)/', ['GET'], 1, [1 => 'var', 2 => 'abc']), + '/([0-9a-z]+)/hi/([0-9a-z]+)/', + ['GET'], + 1, + [1 => 'var', 2 => 'abc'], [ "/{var}/hi/{abc}/" => [ "description" => "", "get" => [ "description" => '', "summary" => '', - "responses" => [] + "responses" => [ + '200' => [ + "description" => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ], "parameters" => [ [ "name" => "var", "in" => "path", + "required" => true, "schema" => [ "type" => "string", "pattern" => "[0-9a-z]+" @@ -186,6 +286,7 @@ public function provideConvert(): array [ "name" => "abc", "in" => "path", + "required" => true, "schema" => [ "type" => "string", "pattern" => "[0-9a-z]+" @@ -196,37 +297,71 @@ public function provideConvert(): array ] ], [ - $this->makeRoute('/', ['GET', 'TRACE']), + '/', + ['GET', 'TRACE'], + 0, + [], [ "/" => [ "description" => "", "get" => [ "description" => '', "summary" => '', - "responses" => [] + "responses" => [ + '200' => [ + "description" => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ], "trace" => [ "description" => '', "summary" => '', - "responses" => [] + "responses" => [ + '200' => [ + "description" => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ] ] ] ], [ - $this->makeRoute('/any/{date:[0-9]{4}-[0-9]{2}-[0-9]{2}}/here/', ['GET']), + '/any/{date:[0-9]{4}-[0-9]{2}-[0-9]{2}}/here/', + ['GET'], + 0, + [], [ "/any/{date}/here/" => [ "description" => "", "get" => [ "description" => '', "summary" => '', - "responses" => [] + "responses" => [ + '200' => [ + "description" => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ], "parameters" => [ [ "name" => "date", "in" => "path", + "required" => true, "schema" => [ "type" => "string", "pattern" => "[0-9]{4}-[0-9]{2}-[0-9]{2}" @@ -242,12 +377,21 @@ public function provideConvert(): array /** * @test * @dataProvider provideConvert - * @param RouteInterface $route + * @param string $path + * @param string[] $methods + * @param int $calls + * @param array $config * @param array $result * @return void */ - public function testConvert(RouteInterface $route, array $result) - { + public function testConvert( + string $path, + array $methods, + int $calls, + array $config, + array $result + ) { + $route = $this->makeRoute($path, $methods, $calls, $config); $annotations = $this->getMockBuilder(PathTargetAnnotationResolver::class)->getMock(); $annotations->expects(static::any()) ->method('__invoke') diff --git a/test/ReflectorTest.php b/test/ReflectorTest.php index 6cb8cfc..99bd3c5 100644 --- a/test/ReflectorTest.php +++ b/test/ReflectorTest.php @@ -5,10 +5,11 @@ use De\Idrinth\PhalconRoutes2OpenApi\Implementations\NoValueConversionMerger; use De\Idrinth\PhalconRoutes2OpenApi\Implementations\Reflector; use phpDocumentor\Reflection\DocBlock; +use phpDocumentor\Reflection\DocBlock\Description; +use phpDocumentor\Reflection\DocBlock\Tags\Generic; use phpDocumentor\Reflection\DocBlockFactoryInterface; use PHPUnit\Framework\TestCase; -use phpDocumentor\Reflection\DocBlock\Tags\Generic; -use phpDocumentor\Reflection\DocBlock\Description; +use stdClass; class ReflectorTest extends TestCase { @@ -27,7 +28,16 @@ public function provideInvoke(): array [ "description" => "", "summary" => "", - "responses" => [] + "responses" => [ + '200' => [ + 'description' => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ], [] ], @@ -35,7 +45,16 @@ public function provideInvoke(): array [ "description" => "", "summary" => "", - "responses" => [] + "responses" => [ + '200' => [ + 'description' => 'unknown return', + 'content' => [ + '*/*' => [ + 'schema' => new stdClass() + ] + ] + ] + ] ], [$this->getTag('any-tag', "abc qq")] ], @@ -47,7 +66,7 @@ public function provideInvoke(): array 700 => [ 'description' => '', 'content' => ['abc/def' => [ - 'schema' => new \stdClass() + 'schema' => new stdClass() ]] ] ] @@ -62,7 +81,7 @@ public function provideInvoke(): array 700 => [ 'description' => '', 'content' => ['abc/def' => [ - 'schema' => new \stdClass() + 'schema' => new stdClass() ]] ] ] @@ -78,10 +97,10 @@ public function provideInvoke(): array 'description' => '', 'content' => [ 'abc/def' => [ - 'schema' => new \stdClass() + 'schema' => new stdClass() ], 'abc/defg' => [ - 'schema' => new \stdClass() + 'schema' => new stdClass() ] ] ]