From 0ae001c002b6ba0b8622a8b6ea43e0bf7189f946 Mon Sep 17 00:00:00 2001 From: shalvah Date: Sat, 15 Sep 2018 08:15:35 +0100 Subject: [PATCH 1/3] Refactor: remove redundant code --- .../ApiDoc/Commands/GenerateDocumentation.php | 51 ++++--------------- .../ApiDoc/Generators/AbstractGenerator.php | 24 ++++++--- .../ApiDoc/Generators/DingoGenerator.php | 24 --------- .../ApiDoc/Generators/LaravelGenerator.php | 14 +---- 4 files changed, 29 insertions(+), 84 deletions(-) diff --git a/src/Mpociot/ApiDoc/Commands/GenerateDocumentation.php b/src/Mpociot/ApiDoc/Commands/GenerateDocumentation.php index b74c4640..4b9a6e7e 100644 --- a/src/Mpociot/ApiDoc/Commands/GenerateDocumentation.php +++ b/src/Mpociot/ApiDoc/Commands/GenerateDocumentation.php @@ -2,11 +2,12 @@ namespace Mpociot\ApiDoc\Commands; +use Illuminate\Routing\Route; use ReflectionClass; use Illuminate\Console\Command; use Mpociot\Reflection\DocBlock; use Illuminate\Support\Collection; -use Illuminate\Support\Facades\Route; +use Illuminate\Support\Facades\Route as RouteFacade; use Mpociot\Documentarian\Documentarian; use Mpociot\ApiDoc\Postman\CollectionWriter; use Mpociot\ApiDoc\Generators\DingoGenerator; @@ -91,7 +92,7 @@ public function handle() if ($this->option('router') === 'laravel') { foreach ($routeDomains as $routeDomain) { foreach ($routePrefixes as $routePrefix) { - $parsedRoutes += $this->processLaravelRoutes($generator, $allowedRoutes, $routeDomain, $routePrefix, $middleware); + $parsedRoutes += $this->processRoutes($generator, $allowedRoutes, $routeDomain, $routePrefix, $middleware); } } } else { @@ -215,6 +216,7 @@ private function getBindings() if (empty($bindings)) { return []; } + $bindings = explode('|', $bindings); $resultBindings = []; foreach ($bindings as $binding) { @@ -250,9 +252,9 @@ private function setUserToBeImpersonated($actAs) private function getRoutes() { if ($this->option('router') === 'laravel') { - return Route::getRoutes(); + return RouteFacade::getRoutes(); } else { - return app('Dingo\Api\Routing\Router')->getRoutes()[$this->option('routePrefix')]; + return app('Dingo\Api\Routing\Router')->getRoutes(); } } @@ -264,13 +266,14 @@ private function getRoutes() * * @return array */ - private function processLaravelRoutes(AbstractGenerator $generator, $allowedRoutes, $routeDomain, $routePrefix, $middleware) + private function processRoutes(AbstractGenerator $generator, array $allowedRoutes, $routeDomain, $routePrefix, $middleware) { - $withResponse = $this->option('noResponseCalls') === false; + $withResponse = $this->option('noResponseCalls') == false; $routes = $this->getRoutes(); $bindings = $this->getBindings(); $parsedRoutes = []; foreach ($routes as $route) { + /** @var Route $route */ if (in_array($route->getName(), $allowedRoutes) || (str_is($routeDomain, $generator->getDomain($route)) && str_is($routePrefix, $generator->getUri($route))) @@ -288,46 +291,12 @@ private function processLaravelRoutes(AbstractGenerator $generator, $allowedRout return $parsedRoutes; } - /** - * @param AbstractGenerator $generator - * @param $allowedRoutes - * @param $routeDomain - * @param $routePrefix - * - * @return array - */ - private function processDingoRoutes(AbstractGenerator $generator, $allowedRoutes, $routeDomain, $routePrefix, $middleware) - { - $withResponse = $this->option('noResponseCalls') === false; - $routes = $this->getRoutes(); - $bindings = $this->getBindings(); - $parsedRoutes = []; - foreach ($routes as $route) { - if (empty($allowedRoutes) - // TODO extract this into a method - || in_array($route->getName(), $allowedRoutes) - || (str_is($routeDomain, $generator->getDomain($route)) - && str_is($routePrefix, $generator->getUri($route))) - || in_array($middleware, $route->middleware()) - ) { - if ($this->isValidRoute($route) && $this->isRouteVisibleForDocumentation($route->getAction()['uses'])) { - $parsedRoutes[] = $generator->processRoute($route, $bindings, $this->option('header'), $withResponse && in_array('GET', $generator->getMethods($route))); - $this->info('Processed route: ['.implode(',', $generator->getMethods($route)).'] '.$route->uri()); - } else { - $this->warn('Skipping route: ['.implode(',', $generator->getMethods($route)).'] '.$route->uri()); - } - } - } - - return $parsedRoutes; - } - /** * @param $route * * @return bool */ - private function isValidRoute($route) + private function isValidRoute(Route $route) { return ! is_callable($route->getAction()['uses']) && ! is_null($route->getAction()['uses']); } diff --git a/src/Mpociot/ApiDoc/Generators/AbstractGenerator.php b/src/Mpociot/ApiDoc/Generators/AbstractGenerator.php index 5d00e8f6..22ad0088 100644 --- a/src/Mpociot/ApiDoc/Generators/AbstractGenerator.php +++ b/src/Mpociot/ApiDoc/Generators/AbstractGenerator.php @@ -3,6 +3,7 @@ namespace Mpociot\ApiDoc\Generators; use Faker\Factory; +use Illuminate\Routing\Route; use League\Fractal\Manager; use League\Fractal\Resource\Collection; use League\Fractal\Resource\Item; @@ -19,25 +20,34 @@ abstract class AbstractGenerator { /** - * @param $route + * @param Route $route * * @return mixed */ - abstract public function getDomain($route); + public function getDomain(Route $route) + { + return $route->domain(); + } /** - * @param $route + * @param Route $route * * @return mixed */ - abstract public function getUri($route); + public function getUri(Route $route) + { + return $route->uri(); + } /** - * @param $route + * @param Route $route * * @return mixed */ - abstract public function getMethods($route); + public function getMethods(Route $route) + { + return array_diff($route->methods(), ['HEAD']); + } /** * @param \Illuminate\Routing\Route $route @@ -77,7 +87,7 @@ public function processRoute($route, $bindings = [], $headers = [], $withRespons try { $response = $this->getRouteResponse($route, $bindings, $headers); } catch (\Exception $e) { - dump("Couldn't get response for route: ".implode(',', $this->getMethods($route)).'] '.$route->uri().'', $e->getMessage()); + echo "Couldn't get response for route: ".implode(',', $this->getMethods($route)).$route->uri().']: '.$e->getMessage()."\n"; } } diff --git a/src/Mpociot/ApiDoc/Generators/DingoGenerator.php b/src/Mpociot/ApiDoc/Generators/DingoGenerator.php index 38f6dfff..314d3c70 100644 --- a/src/Mpociot/ApiDoc/Generators/DingoGenerator.php +++ b/src/Mpociot/ApiDoc/Generators/DingoGenerator.php @@ -30,28 +30,4 @@ public function callRoute($method, $uri, $parameters = [], $cookies = [], $files return call_user_func_array([$dispatcher, strtolower($method)], [$uri]); } - - /** - * {@inheritdoc} - */ - public function getDomain($route) - { - return $route->domain(); - } - - /** - * {@inheritdoc} - */ - public function getUri($route) - { - return $route->uri(); - } - - /** - * {@inheritdoc} - */ - public function getMethods($route) - { - return array_diff($route->getMethods(), ['HEAD']); - } } diff --git a/src/Mpociot/ApiDoc/Generators/LaravelGenerator.php b/src/Mpociot/ApiDoc/Generators/LaravelGenerator.php index 353db332..3efd4241 100644 --- a/src/Mpociot/ApiDoc/Generators/LaravelGenerator.php +++ b/src/Mpociot/ApiDoc/Generators/LaravelGenerator.php @@ -13,17 +13,7 @@ class LaravelGenerator extends AbstractGenerator * * @return mixed */ - public function getDomain($route) - { - return $route->domain(); - } - - /** - * @param Route $route - * - * @return mixed - */ - public function getUri($route) + public function getUri(Route $route) { if (version_compare(app()->version(), '5.4', '<')) { return $route->getUri(); @@ -37,7 +27,7 @@ public function getUri($route) * * @return mixed */ - public function getMethods($route) + public function getMethods(Route $route) { if (version_compare(app()->version(), '5.4', '<')) { $methods = $route->getMethods(); From 47e4816cae55c11a6003ecbe233679015db182c3 Mon Sep 17 00:00:00 2001 From: shalvah Date: Sat, 15 Sep 2018 08:52:22 +0100 Subject: [PATCH 2/3] Make tests work on Windows --- tests/GenerateDocumentationTest.php | 71 +++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 13 deletions(-) diff --git a/tests/GenerateDocumentationTest.php b/tests/GenerateDocumentationTest.php index 09af58a7..be3660ca 100644 --- a/tests/GenerateDocumentationTest.php +++ b/tests/GenerateDocumentationTest.php @@ -2,7 +2,6 @@ namespace Mpociot\ApiDoc\Tests; -use Illuminate\Routing\Route; use Orchestra\Testbench\TestCase; use Illuminate\Contracts\Console\Kernel; use Dingo\Api\Provider\LaravelServiceProvider; @@ -12,6 +11,8 @@ use Illuminate\Support\Facades\Route as RouteFacade; use Mpociot\ApiDoc\Tests\Fixtures\DingoTestController; use Mpociot\ApiDoc\Tests\Fixtures\TestResourceController; +use RecursiveDirectoryIterator; +use RecursiveIteratorIterator; class GenerateDocumentationTest extends TestCase { @@ -32,7 +33,20 @@ public function setUp() public function tearDown() { - exec('rm -rf '.__DIR__.'/../public/docs'); + // delete the generated docs - compatible cross-platform + $dir = __DIR__.'/../public/docs'; + if (is_dir($dir)) { + $files = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($dir, RecursiveDirectoryIterator::SKIP_DOTS), + RecursiveIteratorIterator::CHILD_FIRST + ); + + foreach ($files as $fileinfo) { + $todo = ($fileinfo->isDir() ? 'rmdir' : 'unlink'); + $todo($fileinfo->getRealPath()); + } + rmdir($dir); + } } /** @@ -48,7 +62,7 @@ protected function getPackageProviders($app) ]; } - public function testConsoleCommandNeedsAPrefixOrRoute() + public function testConsoleCommandNeedsPrefixesOrDomainsOrRoutes() { $output = $this->artisan('api:generate'); $this->assertEquals('You must provide either a route prefix, a route domain, a route or a middleware to generate the documentation.'.PHP_EOL, $output); @@ -108,9 +122,9 @@ public function testCanParseResourceRoutes() $output = $this->artisan('api:generate', [ '--routePrefix' => 'api/*', ]); - $generatedMarkdown = file_get_contents(__DIR__.'/../public/docs/source/index.md'); - $fixtureMarkdown = file_get_contents(__DIR__.'/Fixtures/resource_index.md'); - $this->assertSame($generatedMarkdown, $fixtureMarkdown); + $fixtureMarkdown = __DIR__.'/Fixtures/resource_index.md'; + $gneratedMarkdown = __DIR__.'/../public/docs/source/index.md'; + $this->assertFilesHaveSameContent($fixtureMarkdown, $gneratedMarkdown); } public function testGeneratedMarkdownFileIsCorrect() @@ -122,11 +136,11 @@ public function testGeneratedMarkdownFileIsCorrect() '--routePrefix' => 'api/*', ]); - $generatedMarkdown = file_get_contents(__DIR__.'/../public/docs/source/index.md'); - $compareMarkdown = file_get_contents(__DIR__.'/../public/docs/source/.compare.md'); - $fixtureMarkdown = file_get_contents(__DIR__.'/Fixtures/index.md'); - $this->assertSame($generatedMarkdown, $fixtureMarkdown); - $this->assertSame($compareMarkdown, $fixtureMarkdown); + $generatedMarkdown = __DIR__.'/../public/docs/source/index.md'; + $compareMarkdown = __DIR__.'/../public/docs/source/.compare.md'; + $fixtureMarkdown = __DIR__.'/Fixtures/index.md'; + $this->assertFilesHaveSameContent($fixtureMarkdown, $generatedMarkdown); + $this->assertFilesHaveSameContent($fixtureMarkdown, $compareMarkdown); } public function testAddsBindingsToGetRouteRules() @@ -171,8 +185,8 @@ public function testCanAppendCustomHttpHeaders() ], ]); - $generatedMarkdown = file_get_contents(__DIR__.'/../public/docs/source/index.md'); - $this->assertContains('"authorization": [ + $generatedMarkdown = $this->getFileContents(__DIR__.'/../public/docs/source/index.md'); + $this->assertContainsRaw('"authorization": [ "customAuthToken" ], "x-custom-header": [ @@ -204,4 +218,35 @@ public function artisan($command, $parameters = []) return $this->app[Kernel::class]->output(); } + + private function assertFilesHaveSameContent($pathToExpected, $pathToActual) + { + $actual = $this->getFileContents($pathToActual); + $expected = $this->getFileContents($pathToExpected); + $this->assertSame($expected, $actual); + } + + /** + * get the contents of a file in a cross-platform-compatible way + * + * @param $path + * @return mixed + */ + private function getFileContents($path) + { + return str_replace("\r\n", "\n", file_get_contents($path)); + } + + /** + * Assert that a string contains another string, ignoring all whitespace + * + * @param $needle + * @param $haystack + */ + private function assertContainsRaw($needle, $haystack) + { + $haystack = preg_replace('/\s/', '', $haystack); + $needle = preg_replace('/\s/', '', $needle); + $this->assertContains($needle, $haystack); + } } From 597618882902f64744c8a6249cc235b43fa003f9 Mon Sep 17 00:00:00 2001 From: shalvah Date: Sat, 15 Sep 2018 09:04:28 +0100 Subject: [PATCH 3/3] Code style fixes --- src/Mpociot/ApiDoc/Commands/GenerateDocumentation.php | 4 ++-- src/Mpociot/ApiDoc/Generators/AbstractGenerator.php | 11 +++++++---- tests/GenerateDocumentationTest.php | 11 ++++++----- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/Mpociot/ApiDoc/Commands/GenerateDocumentation.php b/src/Mpociot/ApiDoc/Commands/GenerateDocumentation.php index 4b9a6e7e..7428fd40 100644 --- a/src/Mpociot/ApiDoc/Commands/GenerateDocumentation.php +++ b/src/Mpociot/ApiDoc/Commands/GenerateDocumentation.php @@ -2,17 +2,17 @@ namespace Mpociot\ApiDoc\Commands; -use Illuminate\Routing\Route; use ReflectionClass; +use Illuminate\Routing\Route; use Illuminate\Console\Command; use Mpociot\Reflection\DocBlock; use Illuminate\Support\Collection; -use Illuminate\Support\Facades\Route as RouteFacade; use Mpociot\Documentarian\Documentarian; use Mpociot\ApiDoc\Postman\CollectionWriter; use Mpociot\ApiDoc\Generators\DingoGenerator; use Mpociot\ApiDoc\Generators\LaravelGenerator; use Mpociot\ApiDoc\Generators\AbstractGenerator; +use Illuminate\Support\Facades\Route as RouteFacade; class GenerateDocumentation extends Command { diff --git a/src/Mpociot/ApiDoc/Generators/AbstractGenerator.php b/src/Mpociot/ApiDoc/Generators/AbstractGenerator.php index 22ad0088..1870f7e5 100644 --- a/src/Mpociot/ApiDoc/Generators/AbstractGenerator.php +++ b/src/Mpociot/ApiDoc/Generators/AbstractGenerator.php @@ -3,15 +3,15 @@ namespace Mpociot\ApiDoc\Generators; use Faker\Factory; -use Illuminate\Routing\Route; -use League\Fractal\Manager; -use League\Fractal\Resource\Collection; -use League\Fractal\Resource\Item; use ReflectionClass; use Illuminate\Support\Arr; use Illuminate\Support\Str; +use League\Fractal\Manager; +use Illuminate\Routing\Route; use Mpociot\Reflection\DocBlock; +use League\Fractal\Resource\Item; use Mpociot\Reflection\DocBlock\Tag; +use League\Fractal\Resource\Collection; use Illuminate\Support\Facades\Validator; use Illuminate\Foundation\Http\FormRequest; use Mpociot\ApiDoc\Parsers\RuleDescriptionParser as Description; @@ -92,6 +92,7 @@ public function processRoute($route, $bindings = [], $headers = [], $withRespons } $content = $this->getResponseContent($response); + return $this->getParameters([ 'id' => md5($this->getUri($route).':'.implode($this->getMethods($route))), 'resource' => $routeGroup, @@ -674,6 +675,7 @@ protected function normalizeRule($rule) /** * @param $response + * * @return mixed */ private function getResponseContent($response) @@ -686,6 +688,7 @@ private function getResponseContent($response) } else { $content = $response->getContent(); } + return $content; } diff --git a/tests/GenerateDocumentationTest.php b/tests/GenerateDocumentationTest.php index be3660ca..d4ec3274 100644 --- a/tests/GenerateDocumentationTest.php +++ b/tests/GenerateDocumentationTest.php @@ -2,6 +2,8 @@ namespace Mpociot\ApiDoc\Tests; +use RecursiveIteratorIterator; +use RecursiveDirectoryIterator; use Orchestra\Testbench\TestCase; use Illuminate\Contracts\Console\Kernel; use Dingo\Api\Provider\LaravelServiceProvider; @@ -11,8 +13,6 @@ use Illuminate\Support\Facades\Route as RouteFacade; use Mpociot\ApiDoc\Tests\Fixtures\DingoTestController; use Mpociot\ApiDoc\Tests\Fixtures\TestResourceController; -use RecursiveDirectoryIterator; -use RecursiveIteratorIterator; class GenerateDocumentationTest extends TestCase { @@ -227,10 +227,11 @@ private function assertFilesHaveSameContent($pathToExpected, $pathToActual) } /** - * get the contents of a file in a cross-platform-compatible way + * Get the contents of a file in a cross-platform-compatible way. * * @param $path - * @return mixed + * + * @return string */ private function getFileContents($path) { @@ -238,7 +239,7 @@ private function getFileContents($path) } /** - * Assert that a string contains another string, ignoring all whitespace + * Assert that a string contains another string, ignoring all whitespace. * * @param $needle * @param $haystack