Merge pull request #326 from shalvah/code-enhancements

Code enhancements

src/Mpociot/ApiDoc/Commands/GenerateDocumentation.php

@@ -3,15 +3,16 @@
 namespace Mpociot\ApiDoc\Commands;
 
 use ReflectionClass;
+use Illuminate\Routing\Route;
 use Illuminate\Console\Command;
 use Mpociot\Reflection\DocBlock;
 use Illuminate\Support\Collection;
-use Illuminate\Support\Facades\Route;
 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
 {
@@ -91,7 +92,7 @@ class GenerateDocumentation extends Command
         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 @@ class GenerateDocumentation extends Command
         if (empty($bindings)) {
             return [];
         }
+
         $bindings = explode('|', $bindings);
         $resultBindings = [];
         foreach ($bindings as $binding) {
@@ -250,9 +252,9 @@ class GenerateDocumentation extends Command
     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 @@ class GenerateDocumentation extends Command
      *
      * @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 @@ class GenerateDocumentation extends Command
         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']);
     }

src/Mpociot/ApiDoc/Generators/AbstractGenerator.php

@@ -7,6 +7,7 @@ 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;
@@ -19,25 +20,34 @@ use Illuminate\Contracts\Validation\Factory as ValidationFactory;
 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 @@ abstract class AbstractGenerator
             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";
             }
         }
 

src/Mpociot/ApiDoc/Generators/DingoGenerator.php

@@ -30,28 +30,4 @@ class DingoGenerator extends AbstractGenerator
 
         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']);
-    }
 }

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 @@ class LaravelGenerator extends AbstractGenerator
      *
      * @return mixed
      */
-    public function getMethods($route)
+    public function getMethods(Route $route)
     {
         if (version_compare(app()->version(), '5.4', '<')) {
             $methods = $route->getMethods();

tests/GenerateDocumentationTest.php

@@ -2,7 +2,8 @@
 
 namespace Mpociot\ApiDoc\Tests;
 
-use Illuminate\Routing\Route;
+use RecursiveIteratorIterator;
+use RecursiveDirectoryIterator;
 use Orchestra\Testbench\TestCase;
 use Illuminate\Contracts\Console\Kernel;
 use Dingo\Api\Provider\LaravelServiceProvider;
@@ -32,7 +33,20 @@ class GenerateDocumentationTest extends TestCase
 
     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 @@ class GenerateDocumentationTest extends TestCase
         ];
     }
 
-    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 @@ class GenerateDocumentationTest extends TestCase
         $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 @@ class GenerateDocumentationTest extends TestCase
             '--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 @@ class GenerateDocumentationTest extends TestCase
             ],
         ]);
 
-        $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,36 @@ class GenerateDocumentationTest extends TestCase
 
         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 string
+     */
+    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);
+    }
 }