Skip to content

Commit 7a53af9

Browse files
shalvahshalvah
authored
Merge pull request #326 from shalvah/code-enhancements
Code enhancements
2 parents 6258b74 + 4dc8498 commit 7a53af9

File tree

5 files changed

+88
-97
lines changed

5 files changed

+88
-97
lines changed

‎src/Mpociot/ApiDoc/Commands/GenerateDocumentation.php‎

Lines changed: 10 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,16 @@
33
namespace Mpociot\ApiDoc\Commands;
44

55
use ReflectionClass;
6+
use Illuminate\Routing\Route;
67
use Illuminate\Console\Command;
78
use Mpociot\Reflection\DocBlock;
89
use Illuminate\Support\Collection;
9-
use Illuminate\Support\Facades\Route;
1010
use Mpociot\Documentarian\Documentarian;
1111
use Mpociot\ApiDoc\Postman\CollectionWriter;
1212
use Mpociot\ApiDoc\Generators\DingoGenerator;
1313
use Mpociot\ApiDoc\Generators\LaravelGenerator;
1414
use Mpociot\ApiDoc\Generators\AbstractGenerator;
15+
use Illuminate\Support\Facades\Route as RouteFacade;
1516

1617
class GenerateDocumentation extends Command
1718
{
@@ -91,7 +92,7 @@ public function handle()
9192
if ($this->option('router') === 'laravel') {
9293
foreach ($routeDomains as $routeDomain) {
9394
foreach ($routePrefixes as $routePrefix) {
94-
$parsedRoutes += $this->processLaravelRoutes($generator, $allowedRoutes, $routeDomain, $routePrefix, $middleware);
95+
$parsedRoutes += $this->processRoutes($generator, $allowedRoutes, $routeDomain, $routePrefix, $middleware);
9596
}
9697
}
9798
} else {
@@ -215,6 +216,7 @@ private function getBindings()
215216
if (empty($bindings)) {
216217
return [];
217218
}
219+
218220
$bindings = explode('|', $bindings);
219221
$resultBindings = [];
220222
foreach ($bindings as $binding) {
@@ -250,9 +252,9 @@ private function setUserToBeImpersonated($actAs)
250252
private function getRoutes()
251253
{
252254
if ($this->option('router') === 'laravel') {
253-
return Route::getRoutes();
255+
return RouteFacade::getRoutes();
254256
} else {
255-
return app('Dingo\Api\Routing\Router')->getRoutes()[$this->option('routePrefix')];
257+
return app('Dingo\Api\Routing\Router')->getRoutes();
256258
}
257259
}
258260

@@ -264,13 +266,14 @@ private function getRoutes()
264266
*
265267
* @return array
266268
*/
267-
private function processLaravelRoutes(AbstractGenerator $generator, $allowedRoutes, $routeDomain, $routePrefix, $middleware)
269+
private function processRoutes(AbstractGenerator $generator, array $allowedRoutes, $routeDomain, $routePrefix, $middleware)
268270
{
269-
$withResponse = $this->option('noResponseCalls') === false;
271+
$withResponse = $this->option('noResponseCalls') == false;
270272
$routes = $this->getRoutes();
271273
$bindings = $this->getBindings();
272274
$parsedRoutes = [];
273275
foreach ($routes as $route) {
276+
/** @var Route $route */
274277
if (in_array($route->getName(), $allowedRoutes)
275278
|| (str_is($routeDomain, $generator->getDomain($route))
276279
&& str_is($routePrefix, $generator->getUri($route)))
@@ -288,46 +291,12 @@ private function processLaravelRoutes(AbstractGenerator $generator, $allowedRout
288291
return $parsedRoutes;
289292
}
290293

291-
/**
292-
* @param AbstractGenerator $generator
293-
* @param $allowedRoutes
294-
* @param $routeDomain
295-
* @param $routePrefix
296-
*
297-
* @return array
298-
*/
299-
private function processDingoRoutes(AbstractGenerator $generator, $allowedRoutes, $routeDomain, $routePrefix, $middleware)
300-
{
301-
$withResponse = $this->option('noResponseCalls') === false;
302-
$routes = $this->getRoutes();
303-
$bindings = $this->getBindings();
304-
$parsedRoutes = [];
305-
foreach ($routes as $route) {
306-
if (empty($allowedRoutes)
307-
// TODO extract this into a method
308-
|| in_array($route->getName(), $allowedRoutes)
309-
|| (str_is($routeDomain, $generator->getDomain($route))
310-
&& str_is($routePrefix, $generator->getUri($route)))
311-
|| in_array($middleware, $route->middleware())
312-
) {
313-
if ($this->isValidRoute($route) && $this->isRouteVisibleForDocumentation($route->getAction()['uses'])) {
314-
$parsedRoutes[] = $generator->processRoute($route, $bindings, $this->option('header'), $withResponse && in_array('GET', $generator->getMethods($route)));
315-
$this->info('Processed route: ['.implode(',', $generator->getMethods($route)).'] '.$route->uri());
316-
} else {
317-
$this->warn('Skipping route: ['.implode(',', $generator->getMethods($route)).'] '.$route->uri());
318-
}
319-
}
320-
}
321-
322-
return $parsedRoutes;
323-
}
324-
325294
/**
326295
* @param $route
327296
*
328297
* @return bool
329298
*/
330-
private function isValidRoute($route)
299+
private function isValidRoute(Route $route)
331300
{
332301
return ! is_callable($route->getAction()['uses']) && ! is_null($route->getAction()['uses']);
333302
}

‎src/Mpociot/ApiDoc/Generators/AbstractGenerator.php‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Illuminate\Support\Arr;
88
use Illuminate\Support\Str;
99
use League\Fractal\Manager;
10+
use Illuminate\Routing\Route;
1011
use Mpociot\Reflection\DocBlock;
1112
use League\Fractal\Resource\Item;
1213
use Mpociot\Reflection\DocBlock\Tag;
@@ -19,25 +20,34 @@
1920
abstract class AbstractGenerator
2021
{
2122
/**
22-
* @param $route
23+
* @param Route $route
2324
*
2425
* @return mixed
2526
*/
26-
abstract public function getDomain($route);
27+
public function getDomain(Route $route)
28+
{
29+
return $route->domain();
30+
}
2731

2832
/**
29-
* @param $route
33+
* @param Route $route
3034
*
3135
* @return mixed
3236
*/
33-
abstract public function getUri($route);
37+
public function getUri(Route $route)
38+
{
39+
return $route->uri();
40+
}
3441

3542
/**
36-
* @param $route
43+
* @param Route $route
3744
*
3845
* @return mixed
3946
*/
40-
abstract public function getMethods($route);
47+
public function getMethods(Route $route)
48+
{
49+
return array_diff($route->methods(), ['HEAD']);
50+
}
4151

4252
/**
4353
* @param \Illuminate\Routing\Route $route
@@ -77,7 +87,7 @@ public function processRoute($route, $bindings = [], $headers = [], $withRespons
7787
try {
7888
$response = $this->getRouteResponse($route, $bindings, $headers);
7989
} catch (\Exception $e) {
80-
dump("Couldn't get response for route: ".implode(',', $this->getMethods($route)).'] '.$route->uri().'', $e->getMessage());
90+
echo "Couldn't get response for route: ".implode(',', $this->getMethods($route)).$route->uri().']: '.$e->getMessage()."\n";
8191
}
8292
}
8393

‎src/Mpociot/ApiDoc/Generators/DingoGenerator.php‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,28 +30,4 @@ public function callRoute($method, $uri, $parameters = [], $cookies = [], $files
3030

3131
return call_user_func_array([$dispatcher, strtolower($method)], [$uri]);
3232
}
33-
34-
/**
35-
* {@inheritdoc}
36-
*/
37-
public function getDomain($route)
38-
{
39-
return $route->domain();
40-
}
41-
42-
/**
43-
* {@inheritdoc}
44-
*/
45-
public function getUri($route)
46-
{
47-
return $route->uri();
48-
}
49-
50-
/**
51-
* {@inheritdoc}
52-
*/
53-
public function getMethods($route)
54-
{
55-
return array_diff($route->getMethods(), ['HEAD']);
56-
}
5733
}

‎src/Mpociot/ApiDoc/Generators/LaravelGenerator.php‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,7 @@ class LaravelGenerator extends AbstractGenerator
1313
*
1414
* @return mixed
1515
*/
16-
public function getDomain($route)
17-
{
18-
return $route->domain();
19-
}
20-
21-
/**
22-
* @param Route $route
23-
*
24-
* @return mixed
25-
*/
26-
public function getUri($route)
16+
public function getUri(Route $route)
2717
{
2818
if (version_compare(app()->version(), '5.4', '<')) {
2919
return $route->getUri();
@@ -37,7 +27,7 @@ public function getUri($route)
3727
*
3828
* @return mixed
3929
*/
40-
public function getMethods($route)
30+
public function getMethods(Route $route)
4131
{
4232
if (version_compare(app()->version(), '5.4', '<')) {
4333
$methods = $route->getMethods();

‎tests/GenerateDocumentationTest.php‎

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
namespace Mpociot\ApiDoc\Tests;
44

5-
use Illuminate\Routing\Route;
5+
use RecursiveIteratorIterator;
6+
use RecursiveDirectoryIterator;
67
use Orchestra\Testbench\TestCase;
78
use Illuminate\Contracts\Console\Kernel;
89
use Dingo\Api\Provider\LaravelServiceProvider;
@@ -32,7 +33,20 @@ public function setUp()
3233

3334
public function tearDown()
3435
{
35-
exec('rm -rf '.__DIR__.'/../public/docs');
36+
// delete the generated docs - compatible cross-platform
37+
$dir = __DIR__.'/../public/docs';
38+
if (is_dir($dir)) {
39+
$files = new RecursiveIteratorIterator(
40+
new RecursiveDirectoryIterator($dir, RecursiveDirectoryIterator::SKIP_DOTS),
41+
RecursiveIteratorIterator::CHILD_FIRST
42+
);
43+
44+
foreach ($files as $fileinfo) {
45+
$todo = ($fileinfo->isDir() ? 'rmdir' : 'unlink');
46+
$todo($fileinfo->getRealPath());
47+
}
48+
rmdir($dir);
49+
}
3650
}
3751

3852
/**
@@ -48,7 +62,7 @@ protected function getPackageProviders($app)
4862
];
4963
}
5064

51-
public function testConsoleCommandNeedsAPrefixOrRoute()
65+
public function testConsoleCommandNeedsPrefixesOrDomainsOrRoutes()
5266
{
5367
$output = $this->artisan('api:generate');
5468
$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()
108122
$output = $this->artisan('api:generate', [
109123
'--routePrefix' => 'api/*',
110124
]);
111-
$generatedMarkdown = file_get_contents(__DIR__.'/../public/docs/source/index.md');
112-
$fixtureMarkdown = file_get_contents(__DIR__.'/Fixtures/resource_index.md');
113-
$this->assertSame($generatedMarkdown, $fixtureMarkdown);
125+
$fixtureMarkdown = __DIR__.'/Fixtures/resource_index.md';
126+
$gneratedMarkdown = __DIR__.'/../public/docs/source/index.md';
127+
$this->assertFilesHaveSameContent($fixtureMarkdown, $gneratedMarkdown);
114128
}
115129

116130
public function testGeneratedMarkdownFileIsCorrect()
@@ -122,11 +136,11 @@ public function testGeneratedMarkdownFileIsCorrect()
122136
'--routePrefix' => 'api/*',
123137
]);
124138

125-
$generatedMarkdown = file_get_contents(__DIR__.'/../public/docs/source/index.md');
126-
$compareMarkdown = file_get_contents(__DIR__.'/../public/docs/source/.compare.md');
127-
$fixtureMarkdown = file_get_contents(__DIR__.'/Fixtures/index.md');
128-
$this->assertSame($generatedMarkdown, $fixtureMarkdown);
129-
$this->assertSame($compareMarkdown, $fixtureMarkdown);
139+
$generatedMarkdown = __DIR__.'/../public/docs/source/index.md';
140+
$compareMarkdown = __DIR__.'/../public/docs/source/.compare.md';
141+
$fixtureMarkdown = __DIR__.'/Fixtures/index.md';
142+
$this->assertFilesHaveSameContent($fixtureMarkdown, $generatedMarkdown);
143+
$this->assertFilesHaveSameContent($fixtureMarkdown, $compareMarkdown);
130144
}
131145

132146
public function testAddsBindingsToGetRouteRules()
@@ -171,8 +185,8 @@ public function testCanAppendCustomHttpHeaders()
171185
],
172186
]);
173187

174-
$generatedMarkdown = file_get_contents(__DIR__.'/../public/docs/source/index.md');
175-
$this->assertContains('"authorization": [
188+
$generatedMarkdown = $this->getFileContents(__DIR__.'/../public/docs/source/index.md');
189+
$this->assertContainsRaw('"authorization": [
176190
"customAuthToken"
177191
],
178192
"x-custom-header": [
@@ -204,4 +218,36 @@ public function artisan($command, $parameters = [])
204218

205219
return $this->app[Kernel::class]->output();
206220
}
221+
222+
private function assertFilesHaveSameContent($pathToExpected, $pathToActual)
223+
{
224+
$actual = $this->getFileContents($pathToActual);
225+
$expected = $this->getFileContents($pathToExpected);
226+
$this->assertSame($expected, $actual);
227+
}
228+
229+
/**
230+
* Get the contents of a file in a cross-platform-compatible way.
231+
*
232+
* @param $path
233+
*
234+
* @return string
235+
*/
236+
private function getFileContents($path)
237+
{
238+
return str_replace("\r\n", "\n", file_get_contents($path));
239+
}
240+
241+
/**
242+
* Assert that a string contains another string, ignoring all whitespace.
243+
*
244+
* @param $needle
245+
* @param $haystack
246+
*/
247+
private function assertContainsRaw($needle, $haystack)
248+
{
249+
$haystack = preg_replace('/\s/', '', $haystack);
250+
$needle = preg_replace('/\s/', '', $needle);
251+
$this->assertContains($needle, $haystack);
252+
}
207253
}

0 commit comments

Comments
 (0)