From 6fed80ff22821ee47c7798ede121d847f8ce7ab6 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Thu, 28 May 2015 23:45:07 +0100 Subject: [PATCH] Use laravel's new accepts method --- CHANGELOG.md | 2 + composer.json | 8 +-- config/exceptions.php | 1 + src/Displayers/HtmlDisplayer.php | 2 +- src/Displayers/JsonApiDisplayer.php | 30 ++++++++++++ src/Displayers/JsonDisplayer.php | 2 +- src/Filters/ContentTypeFilter.php | 33 +------------ tests/Filters/ContentTypeFilterTest.php | 65 ++++++++++++++++--------- tests/ServiceProviderTest.php | 2 +- 9 files changed, 83 insertions(+), 62 deletions(-) create mode 100644 src/Displayers/JsonApiDisplayer.php diff --git a/CHANGELOG.md b/CHANGELOG.md index f570fce..109688f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,11 @@ CHANGE LOG ## V3.0 (Upcoming) +* Drop support for laravel 5.0 * Improved the error info class * Return empty body and no content type if we can't match a displayer * Conformed to json api standards +* Use laravel's new accepts method ## V2.0.1 (26/05/2015) diff --git a/composer.json b/composer.json index bb77dca..c96e98e 100644 --- a/composer.json +++ b/composer.json @@ -11,10 +11,10 @@ ], "require": { "php": ">=5.5.9", - "illuminate/contracts": "5.0.*|5.1.*", - "illuminate/support": "5.0.*|5.1.*", - "symfony/debug": "2.6.*|2.7.*", - "symfony/http-foundation": "2.6.*|2.7.*", + "illuminate/contracts": "5.1.*", + "illuminate/support": "5.1.*", + "symfony/debug": "2.7.*", + "symfony/http-foundation": "2.7.*", "filp/whoops": "1.1.*", "psr/log": "~1.0" }, diff --git a/config/exceptions.php b/config/exceptions.php index 83b79b8..826aae9 100644 --- a/config/exceptions.php +++ b/config/exceptions.php @@ -29,6 +29,7 @@ 'GrahamCampbell\Exceptions\Displayers\DebugDisplayer', 'GrahamCampbell\Exceptions\Displayers\HtmlDisplayer', 'GrahamCampbell\Exceptions\Displayers\JsonDisplayer', + 'GrahamCampbell\Exceptions\Displayers\JsonApiDisplayer', ], /* diff --git a/src/Displayers/HtmlDisplayer.php b/src/Displayers/HtmlDisplayer.php index c5bb53e..01e5288 100644 --- a/src/Displayers/HtmlDisplayer.php +++ b/src/Displayers/HtmlDisplayer.php @@ -63,7 +63,7 @@ public function display(Exception $exception, $code, array $headers) { $info = $this->info->generate($exception, $code); - return new Response($this->render($info), $code, array_merge($headers, ['Content-Type' => 'text/html'])); + return new Response($this->render($info), $code, array_merge($headers, ['Content-Type' => $this->contentType()])); } /** diff --git a/src/Displayers/JsonApiDisplayer.php b/src/Displayers/JsonApiDisplayer.php new file mode 100644 index 0000000..18bf7e3 --- /dev/null +++ b/src/Displayers/JsonApiDisplayer.php @@ -0,0 +1,30 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace GrahamCampbell\Exceptions\Displayers; + +/** + * This is the json api displayer class. + * + * @author Graham Campbell + */ +class JsonApiDisplayer extends JsonDisplayer implements DisplayerInterface +{ + /** + * Get the supported content type. + * + * @return string + */ + public function contentType() + { + return 'application/vnd.api+json'; + } +} diff --git a/src/Displayers/JsonDisplayer.php b/src/Displayers/JsonDisplayer.php index 0f70136..6fecdd5 100644 --- a/src/Displayers/JsonDisplayer.php +++ b/src/Displayers/JsonDisplayer.php @@ -56,7 +56,7 @@ public function display(Exception $exception, $code, array $headers) $error = ['status' => $info['code'], 'title' => $info['name'], 'detail' => $info['detail']]; - return new JsonResponse(['errors' => [$error]], $code, array_merge($headers, ['Content-Type' => 'application/json'])); + return new JsonResponse(['errors' => [$error]], $code, array_merge($headers, ['Content-Type' => $this->contentType()])); } /** diff --git a/src/Filters/ContentTypeFilter.php b/src/Filters/ContentTypeFilter.php index 2573c50..eae10e8 100644 --- a/src/Filters/ContentTypeFilter.php +++ b/src/Filters/ContentTypeFilter.php @@ -12,7 +12,6 @@ namespace GrahamCampbell\Exceptions\Filters; use Exception; -use GrahamCampbell\Exceptions\Displayers\DisplayerInterface; use Illuminate\Http\Request; /** @@ -51,40 +50,12 @@ public function __construct(Request $request) */ public function filter(array $displayers, Exception $exception) { - $acceptable = $this->request->getAcceptableContentTypes(); - foreach ($displayers as $index => $displayer) { - foreach ($this->getContentTypes($displayer) as $type) { - if (in_array($type, $acceptable)) { - continue 2; - } - } - - $split = explode('/', $displayer->contentType()); - - foreach ($acceptable as $type) { - if (preg_match('/'.$split[0].'\/.+\+'.$split[1].'/', $type)) { - continue 2; - } + if (!$this->request->accepts($displayer->contentType())) { + unset($displayers[$index]); } - - unset($displayers[$index]); } return array_values($displayers); } - - /** - * Get the content types to match. - * - * @param \GrahamCampbell\Exceptions\Displayers\DisplayerInterface $displayer - * - * @return string[] - */ - protected function getContentTypes(DisplayerInterface $displayer) - { - $type = $displayer->contentType(); - - return ['*/*', $type, strtok($type, '/').'/*']; - } } diff --git a/tests/Filters/ContentTypeFilterTest.php b/tests/Filters/ContentTypeFilterTest.php index 4372037..b03b00c 100644 --- a/tests/Filters/ContentTypeFilterTest.php +++ b/tests/Filters/ContentTypeFilterTest.php @@ -14,6 +14,7 @@ use Exception; use GrahamCampbell\Exceptions\Displayers\DebugDisplayer; use GrahamCampbell\Exceptions\Displayers\HtmlDisplayer; +use GrahamCampbell\Exceptions\Displayers\JsonApiDisplayer; use GrahamCampbell\Exceptions\Displayers\JsonDisplayer; use GrahamCampbell\Exceptions\ExceptionInfo; use GrahamCampbell\Exceptions\Filters\ContentTypeFilter; @@ -33,8 +34,8 @@ public function testAcceptAll() $html = new HtmlDisplayer(new ExceptionInfo('foo'), 'foo'); $json = new JsonDisplayer(new ExceptionInfo('foo')); - $request = Mockery::mock('Illuminate\Http\Request'); - $request->shouldReceive('getAcceptableContentTypes')->once()->andReturn(['*/*']); + $request = Mockery::mock('Illuminate\Http\Request')->makePartial(); + $request->shouldReceive('getAcceptableContentTypes')->andReturn(['*/*']); $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json], new Exception()); @@ -47,8 +48,8 @@ public function testAcceptHtmlAndAll() $html = new HtmlDisplayer(new ExceptionInfo('foo'), 'foo'); $json = new JsonDisplayer(new ExceptionInfo('foo')); - $request = Mockery::mock('Illuminate\Http\Request'); - $request->shouldReceive('getAcceptableContentTypes')->once()->andReturn(['text/html', '*/*']); + $request = Mockery::mock('Illuminate\Http\Request')->makePartial(); + $request->shouldReceive('getAcceptableContentTypes')->andReturn(['text/html', '*/*']); $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json], new Exception()); @@ -61,8 +62,8 @@ public function testAcceptJustHtml() $html = new HtmlDisplayer(new ExceptionInfo('foo'), 'foo'); $json = new JsonDisplayer(new ExceptionInfo('foo')); - $request = Mockery::mock('Illuminate\Http\Request'); - $request->shouldReceive('getAcceptableContentTypes')->once()->andReturn(['text/html']); + $request = Mockery::mock('Illuminate\Http\Request')->makePartial(); + $request->shouldReceive('getAcceptableContentTypes')->andReturn(['text/html']); $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json], new Exception()); @@ -75,8 +76,8 @@ public function testAcceptText() $html = new HtmlDisplayer(new ExceptionInfo('foo'), 'foo'); $json = new JsonDisplayer(new ExceptionInfo('foo')); - $request = Mockery::mock('Illuminate\Http\Request'); - $request->shouldReceive('getAcceptableContentTypes')->once()->andReturn(['text/*']); + $request = Mockery::mock('Illuminate\Http\Request')->makePartial(); + $request->shouldReceive('getAcceptableContentTypes')->andReturn(['text/*']); $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json], new Exception()); @@ -89,8 +90,8 @@ public function testAcceptJsonAndAll() $html = new HtmlDisplayer(new ExceptionInfo('foo'), 'foo'); $json = new JsonDisplayer(new ExceptionInfo('foo')); - $request = Mockery::mock('Illuminate\Http\Request'); - $request->shouldReceive('getAcceptableContentTypes')->once()->andReturn(['application/json', '*/*']); + $request = Mockery::mock('Illuminate\Http\Request')->makePartial(); + $request->shouldReceive('getAcceptableContentTypes')->andReturn(['application/json', '*/*']); $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json], new Exception()); @@ -103,8 +104,8 @@ public function testAcceptJustJson() $html = new HtmlDisplayer(new ExceptionInfo('foo'), 'foo'); $json = new JsonDisplayer(new ExceptionInfo('foo')); - $request = Mockery::mock('Illuminate\Http\Request'); - $request->shouldReceive('getAcceptableContentTypes')->once()->andReturn(['application/json']); + $request = Mockery::mock('Illuminate\Http\Request')->makePartial(); + $request->shouldReceive('getAcceptableContentTypes')->andReturn(['application/json']); $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json], new Exception()); @@ -116,13 +117,14 @@ public function testAcceptApplication() $debug = new DebugDisplayer(); $html = new HtmlDisplayer(new ExceptionInfo('foo'), 'foo'); $json = new JsonDisplayer(new ExceptionInfo('foo')); + $api = new JsonApiDisplayer(new ExceptionInfo('bar')); - $request = Mockery::mock('Illuminate\Http\Request'); - $request->shouldReceive('getAcceptableContentTypes')->once()->andReturn(['application/*']); + $request = Mockery::mock('Illuminate\Http\Request')->makePartial(); + $request->shouldReceive('getAcceptableContentTypes')->andReturn(['application/*']); - $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json], new Exception()); + $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json, $api], new Exception()); - $this->assertSame([$json], $displayers); + $this->assertSame([$json, $api], $displayers); } public function testAcceptComplexJson() @@ -130,13 +132,28 @@ public function testAcceptComplexJson() $debug = new DebugDisplayer(); $html = new HtmlDisplayer(new ExceptionInfo('foo'), 'foo'); $json = new JsonDisplayer(new ExceptionInfo('foo')); + $api = new JsonApiDisplayer(new ExceptionInfo('bar')); - $request = Mockery::mock('Illuminate\Http\Request'); - $request->shouldReceive('getAcceptableContentTypes')->once()->andReturn(['application/foo+json']); + $request = Mockery::mock('Illuminate\Http\Request')->makePartial(); + $request->shouldReceive('getAcceptableContentTypes')->andReturn(['application/foo+json']); - $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json], new Exception()); + $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json, $api], new Exception()); - $this->assertSame([$json], $displayers); + $this->assertSame([], $displayers); + } + + public function testAcceptJsonApi() + { + $debug = new DebugDisplayer(); + $json = new JsonDisplayer(new ExceptionInfo('foo')); + $api = new JsonApiDisplayer(new ExceptionInfo('bar')); + + $request = Mockery::mock('Illuminate\Http\Request')->makePartial(); + $request->shouldReceive('getAcceptableContentTypes')->andReturn(['application/vnd.api+json']); + + $displayers = (new ContentTypeFilter($request))->filter([$debug, $json, $api], new Exception()); + + $this->assertSame([$api], $displayers); } public function testAcceptManyThings() @@ -145,8 +162,8 @@ public function testAcceptManyThings() $html = new HtmlDisplayer(new ExceptionInfo('foo'), 'foo'); $json = new JsonDisplayer(new ExceptionInfo('foo')); - $request = Mockery::mock('Illuminate\Http\Request'); - $request->shouldReceive('getAcceptableContentTypes')->once()->andReturn(['text/*', 'application/foo+xml']); + $request = Mockery::mock('Illuminate\Http\Request')->makePartial(); + $request->shouldReceive('getAcceptableContentTypes')->andReturn(['text/*', 'application/foo+xml']); $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json], new Exception()); @@ -159,8 +176,8 @@ public function testAcceptNothing() $html = new HtmlDisplayer(new ExceptionInfo('foo'), 'foo'); $json = new JsonDisplayer(new ExceptionInfo('foo')); - $request = Mockery::mock('Illuminate\Http\Request'); - $request->shouldReceive('getAcceptableContentTypes')->once()->andReturn(['application/xml']); + $request = Mockery::mock('Illuminate\Http\Request')->makePartial(); + $request->shouldReceive('getAcceptableContentTypes')->andReturn(['application/xml']); $displayers = (new ContentTypeFilter($request))->filter([$debug, $html, $json], new Exception()); diff --git a/tests/ServiceProviderTest.php b/tests/ServiceProviderTest.php index 2bd3037..b226acc 100644 --- a/tests/ServiceProviderTest.php +++ b/tests/ServiceProviderTest.php @@ -62,7 +62,7 @@ public function testDisplayerConfig() $displayers = $this->app->config->get('exceptions.displayers'); $this->assertInternalType('array', $displayers); - $this->assertCount(3, $displayers); + $this->assertCount(4, $displayers); foreach ($displayers as $displayer) { $this->assertTrue(starts_with($displayer, 'GrahamCampbell\\Exceptions\\Displayers\\'));