From 4c6415e720a59c49fe79f29f58364b37bb44a047 Mon Sep 17 00:00:00 2001 From: Tim MacDonald Date: Sun, 14 Jul 2019 20:12:03 +1000 Subject: [PATCH 01/11] update docs --- readme.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 41b5abb..0d89714 100644 --- a/readme.md +++ b/readme.md @@ -1,6 +1,6 @@ # Multi-format Response Object for Laravel -[![Latest Stable Version](https://poser.pugx.org/timacdonald/multiformat-response-objects/v/stable)](https://packagist.org/packages/timacdonald/multiformat-response-objects) [![Total Downloads](https://poser.pugx.org/timacdonald/multiformat-response-objects/downloads)](https://packagist.org/packages/timacdonald/multiformat-response-objects) [![License](https://poser.pugx.org/timacdonald/multiformat-response-objects/license)](https://packagist.org/packages/timacdonald/multiformat-response-objects) +[![Total Downloads](https://poser.pugx.org/timacdonald/multiformat-response-objects/downloads)](https://packagist.org/packages/timacdonald/multiformat-response-objects) [![License](https://poser.pugx.org/timacdonald/multiformat-response-objects/license)](https://packagist.org/packages/timacdonald/multiformat-response-objects) In some situations you may want to support multiple return formats (HTML, JSON, CSV, XLSX) for the one endpoint and controller. This package gives you a base class that helps you return different formats of the same data. It supports specifying the return format as a file extension or as an `Accept` header. It also allows you to have shared and format specific logic, all while sharing the same route and controller. @@ -14,7 +14,7 @@ $ composer require timacdonald/multiformat-response-objects ## Getting started -This package is designed to help if you have ever created a controller that looks like this... +This package is designed to help if you have ever created two different controllers just to provide different formats (HTML / JSON) but the controllers have a lot of shared logic, or if you have ever created a controller that looks like this... ```php class UserController From 775ef186e71e1aeeb3335636eecf72616c69fdbf Mon Sep 17 00:00:00 2001 From: Tim MacDonald Date: Tue, 16 Jul 2019 08:14:38 +1000 Subject: [PATCH 02/11] Use sentence casing --- readme.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/readme.md b/readme.md index 0d89714..83910b6 100644 --- a/readme.md +++ b/readme.md @@ -1,4 +1,4 @@ -# Multi-format Response Object for Laravel +# Multiformat response object for Laravel [![Total Downloads](https://poser.pugx.org/timacdonald/multiformat-response-objects/downloads)](https://packagist.org/packages/timacdonald/multiformat-response-objects) [![License](https://poser.pugx.org/timacdonald/multiformat-response-objects/license)](https://packagist.org/packages/timacdonald/multiformat-response-objects) @@ -156,7 +156,7 @@ In order to support a format, you create a `to{Format}Response` method, where `{ - HTML: `toHtmlResponse()` - XLSX: `toXlsxResponse()` -### Dependency Injection +### Dependency injection As mentioned previously, the format method will be called by the container, allowing you to resolve **format specific dependencies** from the container. As seen in the basic usage example, the html format has no dependencies, however the csv format has a `CsvWriter` dependency. @@ -279,7 +279,7 @@ class UserResponse extends Response return new UserResponse($query); ``` -## The Journey +## The journey You've read the readme, you've seen the code, now read the journey. If you wanna see how I came to this solution, you can read my blog post: https://timacdonald.me/versatile-response-objects-laravel/. Warning: it's a bit of a rant. From c356d72d163af3f6ff0096470382da238183a1a8 Mon Sep 17 00:00:00 2001 From: Tim MacDonald Date: Tue, 16 Jul 2019 11:34:18 +1000 Subject: [PATCH 03/11] Update readme --- readme.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/readme.md b/readme.md index 83910b6..ff80b45 100644 --- a/readme.md +++ b/readme.md @@ -279,6 +279,10 @@ class UserResponse extends Response return new UserResponse($query); ``` +## Coming soon + +- Handling of API versioning + ## The journey You've read the readme, you've seen the code, now read the journey. If you wanna see how I came to this solution, you can read my blog post: https://timacdonald.me/versatile-response-objects-laravel/. Warning: it's a bit of a rant. From 5a117b350834fbae1dbd10b65c8c9b0cde3e0520 Mon Sep 17 00:00:00 2001 From: Tim MacDonald Date: Fri, 15 May 2020 09:48:20 +1000 Subject: [PATCH 04/11] Fix highlighting --- readme.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/readme.md b/readme.md index ff80b45..94b7ac1 100644 --- a/readme.md +++ b/readme.md @@ -17,6 +17,8 @@ $ composer require timacdonald/multiformat-response-objects This package is designed to help if you have ever created two different controllers just to provide different formats (HTML / JSON) but the controllers have a lot of shared logic, or if you have ever created a controller that looks like this... ```php + ['mpga', 'mp2', 'mp2a', 'mp3', 'm2a', 'm3a'], ``` @@ -205,6 +217,8 @@ This package will resolve the first match, i.e. `mpga` as the format type. If yo ### In the controller ```php + 'users.index', 'uses' => 'UserController@index', @@ -261,6 +279,8 @@ This route will be able to respond to the following urls and formats in the resp That's cool. Not everyone loves it. You don't have to use the `make` method. Just add your own contructor and set your class attributes as you like! ```php + Date: Mon, 7 Sep 2020 17:15:18 +1000 Subject: [PATCH 05/11] asdfasdf --- readme.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readme.md b/readme.md index 94b7ac1..7188f58 100644 --- a/readme.md +++ b/readme.md @@ -4,7 +4,7 @@ In some situations you may want to support multiple return formats (HTML, JSON, CSV, XLSX) for the one endpoint and controller. This package gives you a base class that helps you return different formats of the same data. It supports specifying the return format as a file extension or as an `Accept` header. It also allows you to have shared and format specific logic, all while sharing the same route and controller. -## Installation +## Installationasdf You can install using [composer](https://getcomposer.org/) from [Packagist](https://packagist.org/packages/timacdonald/multiformat-response-objects) From 6817cc9e917e61e41432b5ce25e5cd16495a16bf Mon Sep 17 00:00:00 2001 From: Tim MacDonald Date: Fri, 21 Aug 2020 21:11:45 +1000 Subject: [PATCH 06/11] add deps and normalise --- composer.json | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index c68652b..67d4a81 100644 --- a/composer.json +++ b/composer.json @@ -1,13 +1,13 @@ { "name": "timacdonald/multiformat-response-objects", "description": "A response object that handles multiple response formats within the one controller", - "license": "MIT", "keywords": [ "multiformat", "responsable", "response objects", "laravel" ], + "license": "MIT", "authors": [ { "name": "Tim MacDonald", @@ -17,13 +17,21 @@ ], "require": { "php": "^7.2", - "illuminate/support": "5.8.*", "illuminate/http": "5.8.*", + "illuminate/support": "5.8.*", "symfony/mime": "^4.3" }, "require-dev": { + "ergebnis/composer-normalize": "^2.7", + "infection/infection": "^0.17.2", + "orchestra/testbench": "^3.5", + "phpstan/phpstan": "^0.12.38", "phpunit/phpunit": "^8.0", - "orchestra/testbench": "^3.5" + "vimeo/psalm": "^3.14" + }, + "config": { + "preferred-install": "dist", + "sort-packages": true }, "autoload": { "psr-4": { @@ -34,5 +42,31 @@ "psr-4": { "Tests\\": "tests/" } + }, + "minimum-stability": "stable", + "prefer-stable": true, + "scripts": { + "fix": [ + "clear", + "@composer normalize", + "./vendor/bin/php-cs-fixer fix" + ], + "lint": [ + "clear", + "@composer normalize --dry-run", + "./vendor/bin/php-cs-fixer fix --dry-run", + "./vendor/bin/psalm --threads=8", + "./vendor/bin/phpstan analyse" + ], + "test": [ + "clear", + "./vendor/bin/phpunit", + "./vendor/bin/infection --threads=8" + ] + }, + "support": { + "issues": "https://github.com/timacdonald/multiformat-response-objects/issues", + "source": "https://github.com/timacdonald/multiformat-response-objects/releases/latest", + "docs": "https://github.com/timacdonald/multiformat-response-objects/blob/master/readme.md" } } From 8010dde136273d8fb891b7237563eb428526f4fa Mon Sep 17 00:00:00 2001 From: Tim MacDonald Date: Sat, 12 Sep 2020 18:50:06 +1000 Subject: [PATCH 07/11] wip --- .gitignore | 9 +- composer.json | 9 +- infection.json.dist | 13 ++ phpstan.neon | 7 + phpunit.xml | 15 -- phpunit.xml.dist | 19 ++ psalm.xml | 30 ++++ readme.md | 2 +- src/BaseMultiformatResponse.php | 10 ++ src/Contracts/Extension.php | 11 ++ src/Contracts/ExtensionGuesser.php | 10 ++ src/ExplicitExtension.php | 24 +++ src/Extension.php | 46 +++-- src/FallbackExtension.php | 21 +++ src/Method.php | 35 ++++ src/MimeExtension.php | 50 +++--- src/MimeTypes.php | 27 +++ src/Multiformat.php | 86 +++++++++ src/MultiformatResponseServiceProvider.php | 46 +++++ src/Response.php | 86 --------- src/UrlExtension.php | 32 ++-- ...seTest.php => MultiformatResponseTest.php} | 168 +++++++++++------- tests/TestResponse.php | 35 ++++ 23 files changed, 568 insertions(+), 223 deletions(-) create mode 100644 infection.json.dist create mode 100644 phpstan.neon delete mode 100644 phpunit.xml create mode 100644 phpunit.xml.dist create mode 100644 psalm.xml create mode 100644 src/BaseMultiformatResponse.php create mode 100644 src/Contracts/Extension.php create mode 100644 src/Contracts/ExtensionGuesser.php create mode 100644 src/ExplicitExtension.php create mode 100644 src/FallbackExtension.php create mode 100644 src/Method.php create mode 100644 src/MimeTypes.php create mode 100644 src/Multiformat.php create mode 100644 src/MultiformatResponseServiceProvider.php delete mode 100644 src/Response.php rename tests/{MultFormatResponseTest.php => MultiformatResponseTest.php} (58%) create mode 100644 tests/TestResponse.php diff --git a/.gitignore b/.gitignore index 47d1cb8..417d292 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,6 @@ -vendor/ -.phpunit.result.cache -composer.lock +/vendor +/composer.lock +/.phpunit.result.cache +/.php_cs.cache +/coverage +/infection.log diff --git a/composer.json b/composer.json index 67d4a81..ac16692 100644 --- a/composer.json +++ b/composer.json @@ -33,9 +33,16 @@ "preferred-install": "dist", "sort-packages": true }, + "extra": { + "laravel": { + "providers": [ + "TiMacDonald\\Multiformat\\MultiformatResponseServiceProvider" + ] + } + }, "autoload": { "psr-4": { - "TiMacDonald\\MultiFormat\\": "src" + "TiMacDonald\\Multiformat\\": "src" } }, "autoload-dev": { diff --git a/infection.json.dist b/infection.json.dist new file mode 100644 index 0000000..eda367b --- /dev/null +++ b/infection.json.dist @@ -0,0 +1,13 @@ +{ + "source": { + "directories": [ + "src" + ] + }, + "logs": { + "text": "infection.log" + }, + "mutators": { + "@default": true + } +} \ No newline at end of file diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..cddc7d2 --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,7 @@ +parameters: + checkMissingIterableValueType: false + level: max + paths: + - src + - tests + ignoreErrors: diff --git a/phpunit.xml b/phpunit.xml deleted file mode 100644 index 5421b2a..0000000 --- a/phpunit.xml +++ /dev/null @@ -1,15 +0,0 @@ - - - - - ./tests/ - - - diff --git a/phpunit.xml.dist b/phpunit.xml.dist new file mode 100644 index 0000000..7dbde61 --- /dev/null +++ b/phpunit.xml.dist @@ -0,0 +1,19 @@ + + + + + tests + + + + + + src + + + diff --git a/psalm.xml b/psalm.xml new file mode 100644 index 0000000..fbfa6a8 --- /dev/null +++ b/psalm.xml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + diff --git a/readme.md b/readme.md index 7188f58..327fd12 100644 --- a/readme.md +++ b/readme.md @@ -97,7 +97,7 @@ You can type hint these methods and the dependencies will be resolved from the c ```php extension = $extension; + } + + public function guess(Request $request): ?string + { + return $this->extension; + } +} diff --git a/src/Extension.php b/src/Extension.php index 259f5bf..f08a6ea 100644 --- a/src/Extension.php +++ b/src/Extension.php @@ -1,33 +1,47 @@ formatOverrides = $formatOverrides; - } + /** + * @var \TiMacDonald\Multiformat\FallbackExtension + */ + private $fallbackExtension; - public function parse(Request $request): ?string + /** + * @param \TiMacDonald\Multiformat\Contracts\ExtensionGuesser[] $guessers + */ + public function __construct(array $guessers, FallbackExtension $fallbackExtension) { - return $this->urlExtension($request) ?? $this->acceptHeaderExtension($request); - } + $this->guessers = $guessers; - private function acceptHeaderExtension(Request $request) : ?string - { - return (new MimeExtension($this->formatOverrides))->parse($request); + $this->fallbackExtension = $fallbackExtension; } - private function urlExtension(Request $request): ?string + public function parse(Request $request, ?FallbackExtension $fallbackExtension): string { - return (new UrlExtension)->parse($request); + $extension = Collection::make($this->guessers) + ->merge([ + new ExplicitExtension($fallbackExtension ? $fallbackExtension->value() : null), + new ExplicitExtension($this->fallbackExtension->value()), + ]) + ->reduce(function (?string $carry, ExtensionGuesser $guesser) use ($request): ?string { + return $carry ?? $guesser->guess($request); + }, null); + + assert(is_string($extension)); + + return $extension; } } diff --git a/src/FallbackExtension.php b/src/FallbackExtension.php new file mode 100644 index 0000000..fdbed86 --- /dev/null +++ b/src/FallbackExtension.php @@ -0,0 +1,21 @@ +value = $value; + } + + public function value(): string + { + return $this->value; + } +} diff --git a/src/Method.php b/src/Method.php new file mode 100644 index 0000000..09f5685 --- /dev/null +++ b/src/Method.php @@ -0,0 +1,35 @@ +method($response, $this->name($response, $extension)); + } + + private function method(object $response, string $name): callable + { + $method = [$response, $name]; + + assert(is_callable($method)); + + return $method; + } + + private function name(object $response, string $extension): string + { + $name = 'to'.Str::studly($extension).'Response'; + + if (! method_exists($response, $name)) { + throw new Exception('Method '.get_class($response).'::'.$name.'() does not exist'); + } + + return $name; + } +} diff --git a/src/MimeExtension.php b/src/MimeExtension.php index 88b31d9..0be186d 100644 --- a/src/MimeExtension.php +++ b/src/MimeExtension.php @@ -1,49 +1,53 @@ overrides = $overrides; + $this->guesser = $guesser; - $this->mimeTypes = new MimeTypes; + $this->mimeTypes = $mimeTypes; } - public function parse(Request $request): ?string + public function guess(Request $request): ?string { - foreach ($request->getAcceptableContentTypes() as $contentType) { - $extension = $this->getOverride($contentType) ?? $this->getExtension($contentType); - - if ($extension !== null) { - return $extension; - } + $extension = Collection::make($request->getAcceptableContentTypes()) + ->map(function (string $contentType): ?string { + return $this->findContentTypeExtension($contentType); + })->first(function (?string $extension): bool { + return $extension !== null; + }); + + if ($extension === null) { + return null; } - return $request->format(null); - } + assert(is_string($extension)); - private function getExtension(string $contentType): ?string - { - return $this->mimeTypes->getExtensions($contentType)[0] ?? null; + return $extension; } - private function getOverride(string $contentType): ?string + private function findContentTypeExtension(string $contentType): ?string { - return $this->overrides[$contentType] ?? null; + return $this->mimeTypes->value()[$contentType] ?? + $this->guesser->getExtensions($contentType)[0] ?? + null; } } diff --git a/src/MimeTypes.php b/src/MimeTypes.php new file mode 100644 index 0000000..4b95105 --- /dev/null +++ b/src/MimeTypes.php @@ -0,0 +1,27 @@ +value = $value; + } + + /** + * @return string[] + */ + public function value(): array + { + return $this->value; + } +} diff --git a/src/Multiformat.php b/src/Multiformat.php new file mode 100644 index 0000000..c556d1d --- /dev/null +++ b/src/Multiformat.php @@ -0,0 +1,86 @@ +with($data); + } + + /** + * @return static + */ + public function with(array $data) + { + $this->data = array_merge($this->data, $data); + + return $this; + } + + /** + * @return static + */ + public function withFallbackExtension(string $extension) + { + $this->fallbackExtension = $extension; + + return $this; + } + + /** + * @psalm-suppress MixedInferredReturnType + * + * @param \Illuminate\Http\Request $request + * @return \Symfony\Component\HttpFoundation\Response + */ + public function toResponse($request) + { + $extension = app(Extension::class)->parse( + $request, + $this->fallbackExtension ? new FallbackExtension($this->fallbackExtension) : null + ); + + $method = app(Method::class)->parse($this, $extension); + + return app()->call($method, ['request' => $request]); + } + + /** + * @return mixed + */ + public function __get(string $key) + { + if (array_key_exists($key, $this->data)) { + return $this->data[$key]; + } + + throw new Exception('Accessing undefined attribute '.static::class.'::'.$key); + } +} diff --git a/src/MultiformatResponseServiceProvider.php b/src/MultiformatResponseServiceProvider.php new file mode 100644 index 0000000..7774a96 --- /dev/null +++ b/src/MultiformatResponseServiceProvider.php @@ -0,0 +1,46 @@ +app->singleton(FallbackExtension::class, function (): FallbackExtension { + return new FallbackExtension('html'); + }); + + $this->app->singleton(MimeTypes::class, function (): MimeTypes { + return new MimeTypes([]); + }); + + $this->app->bind(ExtensionContract::class, function (Application $app): ExtensionContract { + $urlExtension = $app->make(UrlExtension::class); + $mimeExtension = $app->make(MimeExtension::class); + $fallbackExtension = $app->make(FallbackExtension::class); + + assert($urlExtension instanceof UrlExtension); + assert($mimeExtension instanceof MimeExtension); + assert($fallbackExtension instanceof FallbackExtension); + + return new Extension([ + $urlExtension, + $mimeExtension + ], $fallbackExtension); + }); + } + + public function provides() + { + return [ + MimeTypes::class, + FallbackExtension::class, + ExtensionContract::class, + ]; + } +} diff --git a/src/Response.php b/src/Response.php deleted file mode 100644 index 119c72d..0000000 --- a/src/Response.php +++ /dev/null @@ -1,86 +0,0 @@ -with($data); - } - - public function with($data): self - { - $this->data = array_merge($this->data, $data); - - return $this; - } - - public function withDefaultFormat(string $format): self - { - $this->defaultFormat = $format; - - return $this; - } - - public function withFormatOverrides(array $formatOverrides): self - { - $this->formatOverrides = array_merge($this->formatOverrides, $formatOverrides); - - return $this; - } - - /** - * @param \Illuminate\Http\Request $request - * @return \Symfony\Component\HttpFoundation\Response - */ - public function toResponse($request) - { - return Container::getInstance()->call([$this, $this->responseMethod($request)], [ - 'request' => $request, - ]); - } - - private function responseMethod(Request $request): string - { - return 'to'.Str::studly($this->extension($request)).'Response'; - } - - private function extension(Request $request): string - { - return (new Extension($this->formatOverrides))->parse($request) ?? $this->defaultFormat; - } - - /** - * @return mixed - */ - public function __get(string $key) - { - if (array_key_exists($key, $this->data)) { - return $this->data[$key]; - } - - throw new Exception('Accessing undefined attribute '.static::class.'::'.$key); - } -} diff --git a/src/UrlExtension.php b/src/UrlExtension.php index 85dcce9..e869b9e 100644 --- a/src/UrlExtension.php +++ b/src/UrlExtension.php @@ -1,29 +1,41 @@ extension($this->filename($request)); + $this->formatOverrides = $formatOverrides; } - private function extension(string $filename): ?string + public function guess(Request $request): ?string { + $filename = Arr::last(explode('/', $request->path())); + + assert(is_string($filename)); + if (! Str::contains($filename, '.')) { return null; } - return Arr::last(explode('.', $filename)); - } + $extension = Arr::last(explode('.', $filename)); - private function filename(Request $request): string - { - return Arr::last(explode('/', $request->path())); + assert(is_string($extension)); + + return $extension; } } diff --git a/tests/MultFormatResponseTest.php b/tests/MultiformatResponseTest.php similarity index 58% rename from tests/MultFormatResponseTest.php rename to tests/MultiformatResponseTest.php index e14c2ae..15a5ee2 100644 --- a/tests/MultFormatResponseTest.php +++ b/tests/MultiformatResponseTest.php @@ -3,12 +3,21 @@ namespace Tests; use Exception; +use Illuminate\Config\Repository; +use Illuminate\Contracts\Foundation\Application; +use Illuminate\Contracts\Support\Responsable; +use Illuminate\Http\Request; use Illuminate\Support\Facades\Route; use Orchestra\Testbench\TestCase; -use TiMacDonald\MultiFormat\Response; +use TiMacDonald\Multiformat\BaseMultiformatResponse; +use TiMacDonald\Multiformat\MimeExtension; +use TiMacDonald\Multiformat\MimeTypes; +use TiMacDonald\Multiformat\Multiformat; +use TiMacDonald\Multiformat\MultiformatResponseServiceProvider; +use TiMacDonald\Multiformat\Response; use stdClass; -class MultFormatResponseTest extends TestCase +class MultiformatResponseTest extends TestCase { protected function setUp(): void { @@ -17,29 +26,40 @@ protected function setUp(): void $this->withoutExceptionHandling(); } - public function test_can_instantiate_instance_with_make_and_data_is_available() + protected function getPackageProviders($app) + { + return [ + MultiformatResponseServiceProvider::class, + ]; + } + + public function test_can_instantiate_instance_with_make_and_data_is_available(): void { $instance = TestResponse::make(['property' => 'expected']); $this->assertSame('expected', $instance->property); } - public function test_can_add_data_using_with_and_retrieve_with_magic_get() + public function test_can_add_data_using_with_and_retrieve_with_magic_get(): void { $instance = (new TestResponse)->with(['property' => 'expected value']); $this->assertSame('expected value', $instance->property); } - public function test_access_to_non_existent_attribute_throws_exception() + public function test_access_to_non_existent_attribute_throws_exception(): void { $this->expectException(Exception::class); $this->expectExceptionMessage('Accessing undefined attribute Tests\TestResponse::not_set'); + /** + * @psalm-suppress UndefinedMagicPropertyFetch + * @phpstan-ignore-next-line + */ (new TestResponse)->not_set; } - public function test_with_merges_data() + public function test_with_merges_data(): void { $instance = new TestResponse; $instance->with(['property_1' => 'expected value 1']); @@ -49,18 +69,18 @@ public function test_with_merges_data() $this->assertSame('expected value 2', $instance->property_2); } - public function test_with_overrides_when_passing_duplicate_key() + public function test_with_overrides_when_passing_duplicate_key(): void { $instance = new TestResponse; - $instance->with(['property' => 1]); - $instance->with(['property' => 2]); + $instance->with(['property' => 'first']); + $instance->with(['property' => 'second']); - $this->assertSame(2, $instance->property); + $this->assertSame('second', $instance->property); } - public function test_is_defaults_to_html_format() + public function test_is_defaults_to_html_format(): void { - Route::get('location', function () { + Route::get('location', function (): Responsable { return new TestResponse; }); @@ -70,9 +90,9 @@ public function test_is_defaults_to_html_format() $this->assertSame('expected html response', $response->content()); } - public function test_responds_to_extension_in_the_route() + public function test_responds_to_extension_in_the_route(): void { - Route::get('location.csv', function () { + Route::get('location.csv', function (): Responsable { return new TestResponse; }); @@ -82,9 +102,9 @@ public function test_responds_to_extension_in_the_route() $this->assertSame('expected csv response', $response->content()); } - public function test_responds_to_accept_header() + public function test_responds_to_accept_header(): void { - Route::get('location', function () { + Route::get('location', function (): Responsable { return new TestResponse; }); @@ -96,9 +116,9 @@ public function test_responds_to_accept_header() $this->assertSame('expected json response', $response->content()); } - public function test_responds_to_first_matching_accepts_header() + public function test_responds_to_first_matching_accepts_header(): void { - Route::get('location', function () { + Route::get('location', function (): Responsable { return new TestResponse; }); @@ -110,9 +130,9 @@ public function test_responds_to_first_matching_accepts_header() $this->assertSame('expected csv response', $response->content()); } - public function test_responds_to_a_more_obscure_accept_header() + public function test_responds_to_a_more_obscure_accept_header(): void { - Route::get('location', function () { + Route::get('location', function (): Responsable { return new TestResponse; }); @@ -124,9 +144,9 @@ public function test_responds_to_a_more_obscure_accept_header() $this->assertSame('expected xlsx response', $response->content()); } - public function test_last_dot_segement_is_used_as_the_extension_type() + public function test_last_dot_segement_is_used_as_the_extension_type(): void { - Route::get('websites/{domain}{format}', function () { + Route::get('websites/{domain}{format}', function (): Responsable { return new TestResponse; })->where('format', '.json'); @@ -136,9 +156,9 @@ public function test_last_dot_segement_is_used_as_the_extension_type() $this->assertSame('expected json response', $response->content()); } - public function test_file_extension_takes_precendence_over_accept_header() + public function test_file_extension_takes_precendence_over_accept_header(): void { - Route::get('location{format}', function () { + Route::get('location{format}', function (): Responsable { return new TestResponse; }); @@ -150,10 +170,10 @@ public function test_file_extension_takes_precendence_over_accept_header() $this->assertSame('expected csv response', $response->content()); } - public function test_root_domain_returns_html_by_default() + public function test_root_domain_returns_html_by_default(): void { - $this->app->config->set('app.url', 'http://timacdonald.me'); - Route::get('', function () { + $this->config()->set('app.url', 'http://timacdonald.me'); + Route::get('', function (): Responsable { return new TestResponse; }); @@ -163,10 +183,10 @@ public function test_root_domain_returns_html_by_default() $this->assertSame('expected html response', $response->content()); } - public function test_root_domain_response_to_other_formats() + public function test_root_domain_response_to_other_formats(): void { - $this->app->config->set('app.url', 'http://timacdonald.me'); - Route::get('.csv', function () { + $this->config()->set('app.url', 'http://timacdonald.me'); + Route::get('.csv', function (): Responsable { return new TestResponse; }); @@ -176,9 +196,9 @@ public function test_root_domain_response_to_other_formats() $this->assertSame('expected csv response', $response->content()); } - public function test_query_string_has_no_impact() + public function test_query_string_has_no_impact(): void { - Route::get('location', function () { + Route::get('location', function (): Responsable { return new TestResponse; }); @@ -188,12 +208,16 @@ public function test_query_string_has_no_impact() $this->assertSame('expected html response', $response->content()); } - public function test_container_passes_request_into_format_methods() + public function test_container_passes_request_into_format_methods(): void { - Route::get('location.csv', function () { - return new class extends Response { - public function toCsvResponse($request) { - return $request->query('parameter'); + Route::get('location.csv', function (): Responsable { + return new class() extends BaseMultiformatResponse { + public function toCsvResponse(Request $request): string { + $query = $request->query('parameter'); + + assert(is_string($query)); + + return $query; } }; }); @@ -204,16 +228,18 @@ public function toCsvResponse($request) { $this->assertSame('expected value', $response->content()); } - public function test_container_resolves_dependencies_in_format_methods() + public function test_container_resolves_dependencies_in_format_methods(): void { $this->app->bind(stdClass::class, function () { $instance = new stdClass; $instance->property = 'expected value'; return $instance; }); - Route::get('location.csv', function () { - return new class extends Response { - public function toCsvResponse(stdClass $stdClass) { + Route::get('location.csv', function (): Responsable { + return new class() extends BaseMultiformatResponse { + public function toCsvResponse(stdClass $stdClass): string { + assert(is_string($stdClass->property)); + return $stdClass->property; } }; @@ -225,10 +251,10 @@ public function toCsvResponse(stdClass $stdClass) { $this->assertSame('expected value', $response->content()); } - public function test_can_set_default_response_format() + public function test_can_set_default_response_format(): void { - Route::get('location', function () { - return TestResponse::make()->withDefaultFormat('csv'); + Route::get('location', function (): Responsable { + return TestResponse::make()->withFallbackExtension('csv'); }); $response = $this->get('location', ['Accept' => null]); @@ -237,21 +263,21 @@ public function test_can_set_default_response_format() $this->assertSame('expected csv response', $response->content()); } - public function test_exception_is_throw_if_no_response_method_exists() + public function test_exception_is_throw_if_no_response_method_exists(): void { $this->expectExceptionMessage('Method Tests\TestResponse::toMp3Response() does not exist'); - Route::get('location{format}', function () { + Route::get('location{format}', function (): Responsable { return new TestResponse; }); - $response = $this->get('location.mp3'); + $this->get('location.mp3'); } - public function test_url_html_format_is_used_when_the_default_has_another_value() + public function test_url_html_format_is_used_when_the_default_has_another_value(): void { - Route::get('location{format}', function () { - return (new TestResponse)->withDefaultFormat('csv'); + Route::get('location{format}', function (): Responsable { + return (new TestResponse)->withFallbackExtension('csv'); }); $response = $this->get('location.html'); @@ -260,10 +286,13 @@ public function test_url_html_format_is_used_when_the_default_has_another_value( $this->assertSame('expected html response', $response->content()); } - public function test_can_override_formats() + public function test_can_override_formats(): void { - Route::get('location', function () { - return (new TestResponse)->withFormatOverrides(['text/csv' => 'json']); + $this->app->singleton(MimeTypes::class, function (Application $app): MimeTypes { + return new MimeTypes(['text/csv' => 'json']); + }); + Route::get('location', function (): Responsable { + return (new TestResponse); }); $response = $this->get('location', ['Accept' => 'text/csv']); @@ -271,28 +300,31 @@ public function test_can_override_formats() $response->assertOk(); $this->assertSame('expected json response', $response->content()); } -} -class TestResponse extends Response -{ - public function toHtmlResponse() + public function test_untyped_request_variable_is_passed_through(): void { - return 'expected html response'; - } + Route::get('location.csv', function (): Responsable { + return new class() extends BaseMultiformatResponse { + use Multiformat; - public function toJsonResponse() - { - return 'expected json response'; - } + public function toCsvResponse($response): string { + return $response->input('query'); + } + }; + }); - public function toCsvResponse() - { - return 'expected csv response'; + $response = $this->get('location.csv?query=expected query'); + + $this->assertSame('expected query', $response->content()); } - public function toXlsxResponse() + private function config(): Repository { - return 'expected xlsx response'; + $config = $this->app->make('config'); + + assert($config instanceof Repository); + + return $config; } } diff --git a/tests/TestResponse.php b/tests/TestResponse.php new file mode 100644 index 0000000..8d348d3 --- /dev/null +++ b/tests/TestResponse.php @@ -0,0 +1,35 @@ + Date: Tue, 15 Sep 2020 19:25:25 +1000 Subject: [PATCH 08/11] wip --- .php_cs.dist | 8 ++ composer.json | 3 +- src/BaseMultiformatResponse.php | 2 + src/Contracts/Extension.php | 2 + src/Contracts/ExtensionGuesser.php | 2 + src/{MimeTypes.php => CustomMimeTypes.php} | 6 +- src/ExplicitExtension.php | 2 + src/Extension.php | 23 ++- src/FallbackExtension.php | 2 + src/Method.php | 7 +- src/MimeExtension.php | 10 +- src/Multiformat.php | 21 +-- src/MultiformatResponseServiceProvider.php | 14 +- src/UrlExtension.php | 7 +- tests/MultiformatResponseTest.php | 160 ++++++++++++--------- tests/TestResponse.php | 4 +- 16 files changed, 175 insertions(+), 98 deletions(-) create mode 100644 .php_cs.dist rename src/{MimeTypes.php => CustomMimeTypes.php} (75%) diff --git a/.php_cs.dist b/.php_cs.dist new file mode 100644 index 0000000..14db556 --- /dev/null +++ b/.php_cs.dist @@ -0,0 +1,8 @@ +in(__DIR__.'/src') + ->in(__DIR__.'/tests'); + +return TiMacDonald\styles($finder); + diff --git a/composer.json b/composer.json index ac16692..3f0066c 100644 --- a/composer.json +++ b/composer.json @@ -27,7 +27,8 @@ "orchestra/testbench": "^3.5", "phpstan/phpstan": "^0.12.38", "phpunit/phpunit": "^8.0", - "vimeo/psalm": "^3.14" + "timacdonald/php-style": "dev-master", + "vimeo/psalm": "^3.15" }, "config": { "preferred-install": "dist", diff --git a/src/BaseMultiformatResponse.php b/src/BaseMultiformatResponse.php index 398c899..4d3ea26 100644 --- a/src/BaseMultiformatResponse.php +++ b/src/BaseMultiformatResponse.php @@ -1,5 +1,7 @@ value = $value; } diff --git a/src/ExplicitExtension.php b/src/ExplicitExtension.php index 9b5949b..e52aa82 100644 --- a/src/ExplicitExtension.php +++ b/src/ExplicitExtension.php @@ -1,5 +1,7 @@ guessers = $guessers; @@ -36,12 +40,27 @@ public function parse(Request $request, ?FallbackExtension $fallbackExtension): new ExplicitExtension($fallbackExtension ? $fallbackExtension->value() : null), new ExplicitExtension($this->fallbackExtension->value()), ]) - ->reduce(function (?string $carry, ExtensionGuesser $guesser) use ($request): ?string { + ->reduce(static function (?string $carry, ExtensionGuesser $guesser) use ($request): ?string { return $carry ?? $guesser->guess($request); }, null); assert(is_string($extension)); + // do we even need this? Maybe we should just make it that + // you have to be explicit. Need to brainstorm this a little more. + if ($extension === 'html') { + return $this->fallbackExtension($fallbackExtension); + } + return $extension; } + + private function fallbackExtension(?FallbackExtension $fallbackExtension): string + { + if ($fallbackExtension === null) { + return $this->fallbackExtension->value(); + } + + return $fallbackExtension->value(); + } } diff --git a/src/FallbackExtension.php b/src/FallbackExtension.php index fdbed86..d92eb00 100644 --- a/src/FallbackExtension.php +++ b/src/FallbackExtension.php @@ -1,5 +1,7 @@ guesser = $guesser; @@ -31,7 +35,7 @@ public function guess(Request $request): ?string $extension = Collection::make($request->getAcceptableContentTypes()) ->map(function (string $contentType): ?string { return $this->findContentTypeExtension($contentType); - })->first(function (?string $extension): bool { + })->first(static function (?string $extension): bool { return $extension !== null; }); diff --git a/src/Multiformat.php b/src/Multiformat.php index c556d1d..c3bd287 100644 --- a/src/Multiformat.php +++ b/src/Multiformat.php @@ -1,13 +1,13 @@ with($data); + return (new static())->with($data); } /** * @return static */ - public function with(array $data) + public function with(array $data): self { $this->data = array_merge($this->data, $data); @@ -47,7 +47,7 @@ public function with(array $data) /** * @return static */ - public function withFallbackExtension(string $extension) + public function withFallbackExtension(string $extension): self { $this->fallbackExtension = $extension; @@ -58,7 +58,8 @@ public function withFallbackExtension(string $extension) * @psalm-suppress MixedInferredReturnType * * @param \Illuminate\Http\Request $request - * @return \Symfony\Component\HttpFoundation\Response + * + * @return mixed */ public function toResponse($request) { diff --git a/src/MultiformatResponseServiceProvider.php b/src/MultiformatResponseServiceProvider.php index 7774a96..bad0a19 100644 --- a/src/MultiformatResponseServiceProvider.php +++ b/src/MultiformatResponseServiceProvider.php @@ -1,7 +1,10 @@ app->singleton(FallbackExtension::class, function (): FallbackExtension { + $this->app->singleton(FallbackExtension::class, static function (): FallbackExtension { return new FallbackExtension('html'); }); - $this->app->singleton(MimeTypes::class, function (): MimeTypes { - return new MimeTypes([]); - }); - - $this->app->bind(ExtensionContract::class, function (Application $app): ExtensionContract { + $this->app->bind(ExtensionContract::class, static function (Application $app): ExtensionContract { $urlExtension = $app->make(UrlExtension::class); $mimeExtension = $app->make(MimeExtension::class); $fallbackExtension = $app->make(FallbackExtension::class); @@ -30,7 +29,7 @@ public function register(): void return new Extension([ $urlExtension, - $mimeExtension + $mimeExtension, ], $fallbackExtension); }); } @@ -38,7 +37,6 @@ public function register(): void public function provides() { return [ - MimeTypes::class, FallbackExtension::class, ExtensionContract::class, ]; diff --git a/src/UrlExtension.php b/src/UrlExtension.php index e869b9e..dc21da1 100644 --- a/src/UrlExtension.php +++ b/src/UrlExtension.php @@ -1,10 +1,15 @@ path())); + $filename = Arr::last(explode('/', $request->path())); assert(is_string($filename)); diff --git a/tests/MultiformatResponseTest.php b/tests/MultiformatResponseTest.php index 15a5ee2..ee5278f 100644 --- a/tests/MultiformatResponseTest.php +++ b/tests/MultiformatResponseTest.php @@ -1,22 +1,28 @@ 'expected']); $this->assertSame('expected', $instance->property); } - public function test_can_add_data_using_with_and_retrieve_with_magic_get(): void + public function testCanAddDataUsingWithAndRetrieveWithMagicGet(): void { - $instance = (new TestResponse)->with(['property' => 'expected value']); + $instance = (new TestResponse())->with(['property' => 'expected value']); $this->assertSame('expected value', $instance->property); } - public function test_access_to_non_existent_attribute_throws_exception(): void + public function testAccessToNonExistentAttributeThrowsException(): void { $this->expectException(Exception::class); - $this->expectExceptionMessage('Accessing undefined attribute Tests\TestResponse::not_set'); + $this->expectExceptionMessage('Accessing undefined attribute Tests\\TestResponse::not_set'); /** * @psalm-suppress UndefinedMagicPropertyFetch * @phpstan-ignore-next-line */ - (new TestResponse)->not_set; + (new TestResponse())->not_set; } - public function test_with_merges_data(): void + public function testWithMergesData(): void { - $instance = new TestResponse; + $instance = new TestResponse(); $instance->with(['property_1' => 'expected value 1']); $instance->with(['property_2' => 'expected value 2']); @@ -69,19 +75,19 @@ public function test_with_merges_data(): void $this->assertSame('expected value 2', $instance->property_2); } - public function test_with_overrides_when_passing_duplicate_key(): void + public function testWithOverridesWhenPassingDuplicateKey(): void { - $instance = new TestResponse; + $instance = new TestResponse(); $instance->with(['property' => 'first']); $instance->with(['property' => 'second']); $this->assertSame('second', $instance->property); } - public function test_is_defaults_to_html_format(): void + public function testIsDefaultsToHtmlFormat(): void { - Route::get('location', function (): Responsable { - return new TestResponse; + Route::get('location', static function (): Responsable { + return new TestResponse(); }); $response = $this->get('location'); @@ -90,10 +96,10 @@ public function test_is_defaults_to_html_format(): void $this->assertSame('expected html response', $response->content()); } - public function test_responds_to_extension_in_the_route(): void + public function testRespondsToExtensionInTheRoute(): void { - Route::get('location.csv', function (): Responsable { - return new TestResponse; + Route::get('location.csv', static function (): Responsable { + return new TestResponse(); }); $response = $this->get('location.csv'); @@ -102,10 +108,10 @@ public function test_responds_to_extension_in_the_route(): void $this->assertSame('expected csv response', $response->content()); } - public function test_responds_to_accept_header(): void + public function testRespondsToAcceptHeader(): void { - Route::get('location', function (): Responsable { - return new TestResponse; + Route::get('location', static function (): Responsable { + return new TestResponse(); }); $response = $this->get('location', [ @@ -116,10 +122,10 @@ public function test_responds_to_accept_header(): void $this->assertSame('expected json response', $response->content()); } - public function test_responds_to_first_matching_accepts_header(): void + public function testRespondsToFirstMatchingAcceptsHeader(): void { - Route::get('location', function (): Responsable { - return new TestResponse; + Route::get('location', static function (): Responsable { + return new TestResponse(); }); $response = $this->get('location', [ @@ -130,10 +136,10 @@ public function test_responds_to_first_matching_accepts_header(): void $this->assertSame('expected csv response', $response->content()); } - public function test_responds_to_a_more_obscure_accept_header(): void + public function testRespondsToAMoreObscureAcceptHeader(): void { - Route::get('location', function (): Responsable { - return new TestResponse; + Route::get('location', static function (): Responsable { + return new TestResponse(); }); $response = $this->get('location', [ @@ -144,10 +150,10 @@ public function test_responds_to_a_more_obscure_accept_header(): void $this->assertSame('expected xlsx response', $response->content()); } - public function test_last_dot_segement_is_used_as_the_extension_type(): void + public function testLastDotSegementIsUsedAsTheExtensionType(): void { - Route::get('websites/{domain}{format}', function (): Responsable { - return new TestResponse; + Route::get('websites/{domain}{format}', static function (): Responsable { + return new TestResponse(); })->where('format', '.json'); $response = $this->get('websites/timacdonald.me.json'); @@ -156,10 +162,10 @@ public function test_last_dot_segement_is_used_as_the_extension_type(): void $this->assertSame('expected json response', $response->content()); } - public function test_file_extension_takes_precendence_over_accept_header(): void + public function testFileExtensionTakesPrecendenceOverAcceptHeader(): void { - Route::get('location{format}', function (): Responsable { - return new TestResponse; + Route::get('location{format}', static function (): Responsable { + return new TestResponse(); }); $response = $this->get('location.csv', [ @@ -170,11 +176,11 @@ public function test_file_extension_takes_precendence_over_accept_header(): void $this->assertSame('expected csv response', $response->content()); } - public function test_root_domain_returns_html_by_default(): void + public function testRootDomainReturnsHtmlByDefault(): void { $this->config()->set('app.url', 'http://timacdonald.me'); - Route::get('', function (): Responsable { - return new TestResponse; + Route::get('', static function (): Responsable { + return new TestResponse(); }); $response = $this->get(''); @@ -183,11 +189,11 @@ public function test_root_domain_returns_html_by_default(): void $this->assertSame('expected html response', $response->content()); } - public function test_root_domain_response_to_other_formats(): void + public function testRootDomainResponseToOtherFormats(): void { $this->config()->set('app.url', 'http://timacdonald.me'); - Route::get('.csv', function (): Responsable { - return new TestResponse; + Route::get('.csv', static function (): Responsable { + return new TestResponse(); }); $response = $this->get('.csv'); @@ -196,10 +202,10 @@ public function test_root_domain_response_to_other_formats(): void $this->assertSame('expected csv response', $response->content()); } - public function test_query_string_has_no_impact(): void + public function testQueryStringHasNoImpact(): void { - Route::get('location', function (): Responsable { - return new TestResponse; + Route::get('location', static function (): Responsable { + return new TestResponse(); }); $response = $this->get('location?format=.csv'); @@ -208,11 +214,12 @@ public function test_query_string_has_no_impact(): void $this->assertSame('expected html response', $response->content()); } - public function test_container_passes_request_into_format_methods(): void + public function testContainerPassesRequestIntoFormatMethods(): void { - Route::get('location.csv', function (): Responsable { + Route::get('location.csv', static function (): Responsable { return new class() extends BaseMultiformatResponse { - public function toCsvResponse(Request $request): string { + public function toCsvResponse(Request $request): string + { $query = $request->query('parameter'); assert(is_string($query)); @@ -228,16 +235,18 @@ public function toCsvResponse(Request $request): string { $this->assertSame('expected value', $response->content()); } - public function test_container_resolves_dependencies_in_format_methods(): void + public function testContainerResolvesDependenciesInFormatMethods(): void { - $this->app->bind(stdClass::class, function () { - $instance = new stdClass; + $this->app->bind(stdClass::class, static function () { + $instance = new stdClass(); $instance->property = 'expected value'; + return $instance; }); - Route::get('location.csv', function (): Responsable { + Route::get('location.csv', static function (): Responsable { return new class() extends BaseMultiformatResponse { - public function toCsvResponse(stdClass $stdClass): string { + public function toCsvResponse(stdClass $stdClass): string + { assert(is_string($stdClass->property)); return $stdClass->property; @@ -251,9 +260,9 @@ public function toCsvResponse(stdClass $stdClass): string { $this->assertSame('expected value', $response->content()); } - public function test_can_set_default_response_format(): void + public function testCanSetDefaultResponseFormat(): void { - Route::get('location', function (): Responsable { + Route::get('location', static function (): Responsable { return TestResponse::make()->withFallbackExtension('csv'); }); @@ -263,21 +272,21 @@ public function test_can_set_default_response_format(): void $this->assertSame('expected csv response', $response->content()); } - public function test_exception_is_throw_if_no_response_method_exists(): void + public function testExceptionIsThrowIfNoResponseMethodExists(): void { - $this->expectExceptionMessage('Method Tests\TestResponse::toMp3Response() does not exist'); + $this->expectExceptionMessage('Method Tests\\TestResponse::toMp3Response() does not exist'); - Route::get('location{format}', function (): Responsable { - return new TestResponse; + Route::get('location{format}', static function (): Responsable { + return new TestResponse(); }); $this->get('location.mp3'); } - public function test_url_html_format_is_used_when_the_default_has_another_value(): void + public function testUrlHtmlFormatIsUsedWhenTheDefaultHasAnotherValue(): void { - Route::get('location{format}', function (): Responsable { - return (new TestResponse)->withFallbackExtension('csv'); + Route::get('location{format}', static function (): Responsable { + return (new TestResponse())->withFallbackExtension('csv'); }); $response = $this->get('location.html'); @@ -286,13 +295,13 @@ public function test_url_html_format_is_used_when_the_default_has_another_value( $this->assertSame('expected html response', $response->content()); } - public function test_can_override_formats(): void + public function testCanOverrideFormats(): void { - $this->app->singleton(MimeTypes::class, function (Application $app): MimeTypes { + $this->app->singleton(MimeTypes::class, static function (Application $app): MimeTypes { return new MimeTypes(['text/csv' => 'json']); }); - Route::get('location', function (): Responsable { - return (new TestResponse); + Route::get('location', static function (): Responsable { + return new TestResponse(); }); $response = $this->get('location', ['Accept' => 'text/csv']); @@ -301,13 +310,14 @@ public function test_can_override_formats(): void $this->assertSame('expected json response', $response->content()); } - public function test_untyped_request_variable_is_passed_through(): void + public function testUntypedRequestVariableIsPassedThrough(): void { - Route::get('location.csv', function (): Responsable { + Route::get('location.csv', static function (): Responsable { return new class() extends BaseMultiformatResponse { use Multiformat; - public function toCsvResponse($response): string { + public function toCsvResponse($response): string + { return $response->input('query'); } }; @@ -318,6 +328,21 @@ public function toCsvResponse($response): string { $this->assertSame('expected query', $response->content()); } + public function testOverridingFallbackExtensionGlobally(): void + { + $this->app->bind(FallbackExtension::class, static function (): FallbackExtension { + return new FallbackExtension('json'); + }); + Route::get('location', static function (): Responsable { + return new TestResponse(); + }); + + $response = $this->get('location'); + + $response->assertOk(); + $this->assertSame('expected json response', $response->content()); + } + private function config(): Repository { $config = $this->app->make('config'); @@ -327,4 +352,3 @@ private function config(): Repository return $config; } } - diff --git a/tests/TestResponse.php b/tests/TestResponse.php index 8d348d3..51468db 100644 --- a/tests/TestResponse.php +++ b/tests/TestResponse.php @@ -1,10 +1,10 @@ Date: Tue, 15 Sep 2020 20:43:38 +1000 Subject: [PATCH 09/11] wip --- ...Extension.php => ApiFallbackExtension.php} | 2 +- src/Contracts/Extension.php | 3 +- src/CustomMimeTypes.php | 2 +- src/ExplicitExtension.php | 6 ++-- src/Extension.php | 32 +++---------------- src/Method.php | 23 ++++++++++--- src/Multiformat.php | 20 ++++++------ src/MultiformatResponseServiceProvider.php | 19 ++++++++--- src/UrlExtension.php | 2 +- tests/MultiformatResponseTest.php | 20 ++++++------ 10 files changed, 64 insertions(+), 65 deletions(-) rename src/{FallbackExtension.php => ApiFallbackExtension.php} (91%) diff --git a/src/FallbackExtension.php b/src/ApiFallbackExtension.php similarity index 91% rename from src/FallbackExtension.php rename to src/ApiFallbackExtension.php index d92eb00..bd39d4b 100644 --- a/src/FallbackExtension.php +++ b/src/ApiFallbackExtension.php @@ -4,7 +4,7 @@ namespace TiMacDonald\Multiformat; -class FallbackExtension +class ApiFallbackExtension { /** * @var string diff --git a/src/Contracts/Extension.php b/src/Contracts/Extension.php index 9853567..3aa6b67 100644 --- a/src/Contracts/Extension.php +++ b/src/Contracts/Extension.php @@ -5,9 +5,8 @@ namespace TiMacDonald\Multiformat\Contracts; use Illuminate\Http\Request; -use TiMacDonald\Multiformat\FallbackExtension; interface Extension { - public function parse(Request $request, ?FallbackExtension $fallbackExtension): string; + public function parse(Request $request): ?string; } diff --git a/src/CustomMimeTypes.php b/src/CustomMimeTypes.php index bb47e51..3871656 100644 --- a/src/CustomMimeTypes.php +++ b/src/CustomMimeTypes.php @@ -14,7 +14,7 @@ class CustomMimeTypes /** * @param string[] $value */ - public function __construct(array $value = []) + public function __construct(array $value) { $this->value = $value; } diff --git a/src/ExplicitExtension.php b/src/ExplicitExtension.php index e52aa82..f4ec2ab 100644 --- a/src/ExplicitExtension.php +++ b/src/ExplicitExtension.php @@ -10,16 +10,16 @@ class ExplicitExtension implements ExtensionGuesser { /** - * @var ?string + * @var string */ private $extension; - public function __construct(?string $extension) + public function __construct(string $extension) { $this->extension = $extension; } - public function guess(Request $request): ?string + public function guess(Request $request): string { return $this->extension; } diff --git a/src/Extension.php b/src/Extension.php index 8f0c09b..15c9c2a 100644 --- a/src/Extension.php +++ b/src/Extension.php @@ -18,49 +18,27 @@ class Extension implements ExtensionContract */ private $guessers; - /** - * @var \TiMacDonald\Multiformat\FallbackExtension - */ - private $fallbackExtension; - /** * @param \TiMacDonald\Multiformat\Contracts\ExtensionGuesser[] $guessers */ public function __construct(array $guessers) { $this->guessers = $guessers; - - $this->fallbackExtension = $fallbackExtension; } - public function parse(Request $request, ?FallbackExtension $fallbackExtension): string + public function parse(Request $request): ?string { $extension = Collection::make($this->guessers) - ->merge([ - new ExplicitExtension($fallbackExtension ? $fallbackExtension->value() : null), - new ExplicitExtension($this->fallbackExtension->value()), - ]) ->reduce(static function (?string $carry, ExtensionGuesser $guesser) use ($request): ?string { return $carry ?? $guesser->guess($request); }, null); - assert(is_string($extension)); - - // do we even need this? Maybe we should just make it that - // you have to be explicit. Need to brainstorm this a little more. - if ($extension === 'html') { - return $this->fallbackExtension($fallbackExtension); + if ($extension === null) { + return null; } - return $extension; - } - - private function fallbackExtension(?FallbackExtension $fallbackExtension): string - { - if ($fallbackExtension === null) { - return $this->fallbackExtension->value(); - } + assert(is_string($extension)); - return $fallbackExtension->value(); + return $extension; } } diff --git a/src/Method.php b/src/Method.php index af2a54e..6e034e2 100644 --- a/src/Method.php +++ b/src/Method.php @@ -7,18 +7,33 @@ use function assert; use Exception; use function get_class; +use Illuminate\Http\Request; use Illuminate\Support\Str; use function is_callable; use function method_exists; +use TiMacDonald\Multiformat\Contracts\Extension; +use TiMacDonald\Multiformat\ApiFallbackExtension; class Method { - public function parse(object $response, string $extension): callable + /** + * @var Extension + */ + private $extension; + + public function __construct(Extension $extension) { - return $this->method($response, $this->name($response, $extension)); + $this->extension = $extension; + } + + public function parse(Request $request, object $response, ApiFallbackExtension $fallbackExtension): callable + { + $extension = $this->extension->parse($request) ?? $fallbackExtension->value(); + + return self::method($response, self::name($response, $extension)); } - private function method(object $response, string $name): callable + private static function method(object $response, string $name): callable { $method = [$response, $name]; @@ -27,7 +42,7 @@ private function method(object $response, string $name): callable return $method; } - private function name(object $response, string $extension): string + private static function name(object $response, string $extension): string { $name = 'to'.Str::studly($extension).'Response'; diff --git a/src/Multiformat.php b/src/Multiformat.php index c3bd287..7fd04c6 100644 --- a/src/Multiformat.php +++ b/src/Multiformat.php @@ -8,14 +8,13 @@ use function array_key_exists; use function array_merge; use Exception; -use TiMacDonald\Multiformat\Contracts\Extension; trait Multiformat { /** - * @var ?string + * @var ?\TiMacDonald\Multiformat\ApiFallbackExtension */ - protected $fallbackExtension; + private $apiFallbackExtension; /** * @var mixed[] @@ -25,7 +24,7 @@ trait Multiformat /** * @return static */ - public static function make(array $data): self + public static function make(array $data) { /** * @psalm-suppress UnsafeInstantiation @@ -37,7 +36,7 @@ public static function make(array $data): self /** * @return static */ - public function with(array $data): self + public function with(array $data) { $this->data = array_merge($this->data, $data); @@ -47,9 +46,9 @@ public function with(array $data): self /** * @return static */ - public function withFallbackExtension(string $extension): self + public function withApiFallbackExtension(string $extension) { - $this->fallbackExtension = $extension; + $this->apiFallbackExtension = new ApiFallbackExtension($extension); return $this; } @@ -63,13 +62,12 @@ public function withFallbackExtension(string $extension): self */ public function toResponse($request) { - $extension = app(Extension::class)->parse( + $method = app(Method::class)->parse( $request, - $this->fallbackExtension ? new FallbackExtension($this->fallbackExtension) : null + $this, + $this->apiFallbackExtension ?? app(ApiFallbackExtension::class) ); - $method = app(Method::class)->parse($this, $extension); - return app()->call($method, ['request' => $request]); } diff --git a/src/MultiformatResponseServiceProvider.php b/src/MultiformatResponseServiceProvider.php index bad0a19..cd22b04 100644 --- a/src/MultiformatResponseServiceProvider.php +++ b/src/MultiformatResponseServiceProvider.php @@ -9,23 +9,32 @@ use Illuminate\Contracts\Support\DeferrableProvider; use Illuminate\Support\ServiceProvider; use TiMacDonald\Multiformat\Contracts\Extension as ExtensionContract; +use TiMacDonald\Multiformat\ApiFallbackExtension; class MultiformatResponseServiceProvider extends ServiceProvider implements DeferrableProvider { public function register(): void { - $this->app->singleton(FallbackExtension::class, static function (): FallbackExtension { - return new FallbackExtension('html'); + $this->app->bind(CustomMimeTypes::class, static function (): CustomMimeTypes { + return new CustomMimeTypes([]); + }); + + $this->app->bind(ApiFallbackExtension::class, static function (): ApiFallbackExtension { + return new ApiFallbackExtension('html'); + }); + + $this->app->bind(UrlExtension::class, static function (): UrlExtension { + return new UrlExtension([]); }); $this->app->bind(ExtensionContract::class, static function (Application $app): ExtensionContract { $urlExtension = $app->make(UrlExtension::class); $mimeExtension = $app->make(MimeExtension::class); - $fallbackExtension = $app->make(FallbackExtension::class); + $fallbackExtension = $app->make(ApiFallbackExtension::class); assert($urlExtension instanceof UrlExtension); assert($mimeExtension instanceof MimeExtension); - assert($fallbackExtension instanceof FallbackExtension); + assert($fallbackExtension instanceof ApiFallbackExtension); return new Extension([ $urlExtension, @@ -37,7 +46,7 @@ public function register(): void public function provides() { return [ - FallbackExtension::class, + ApiFallbackExtension::class, ExtensionContract::class, ]; } diff --git a/src/UrlExtension.php b/src/UrlExtension.php index dc21da1..c2957a5 100644 --- a/src/UrlExtension.php +++ b/src/UrlExtension.php @@ -22,7 +22,7 @@ class UrlExtension implements ExtensionGuesser /** * @param string[] $formatOverrides */ - public function __construct(array $formatOverrides = []) + public function __construct(array $formatOverrides) { $this->formatOverrides = $formatOverrides; } diff --git a/tests/MultiformatResponseTest.php b/tests/MultiformatResponseTest.php index ee5278f..e10e16e 100644 --- a/tests/MultiformatResponseTest.php +++ b/tests/MultiformatResponseTest.php @@ -14,9 +14,9 @@ use function is_string; use Orchestra\Testbench\TestCase; use stdClass; +use TiMacDonald\Multiformat\ApiFallbackExtension; use TiMacDonald\Multiformat\BaseMultiformatResponse; -use TiMacDonald\Multiformat\FallbackExtension; -use TiMacDonald\Multiformat\MimeTypes; +use TiMacDonald\Multiformat\CustomMimeTypes; use TiMacDonald\Multiformat\Multiformat; use TiMacDonald\Multiformat\MultiformatResponseServiceProvider; @@ -263,7 +263,7 @@ public function toCsvResponse(stdClass $stdClass): string public function testCanSetDefaultResponseFormat(): void { Route::get('location', static function (): Responsable { - return TestResponse::make()->withFallbackExtension('csv'); + return TestResponse::make([])->withApiFallbackExtension('csv'); }); $response = $this->get('location', ['Accept' => null]); @@ -286,7 +286,7 @@ public function testExceptionIsThrowIfNoResponseMethodExists(): void public function testUrlHtmlFormatIsUsedWhenTheDefaultHasAnotherValue(): void { Route::get('location{format}', static function (): Responsable { - return (new TestResponse())->withFallbackExtension('csv'); + return (new TestResponse())->withApiFallbackExtension('csv'); }); $response = $this->get('location.html'); @@ -297,8 +297,8 @@ public function testUrlHtmlFormatIsUsedWhenTheDefaultHasAnotherValue(): void public function testCanOverrideFormats(): void { - $this->app->singleton(MimeTypes::class, static function (Application $app): MimeTypes { - return new MimeTypes(['text/csv' => 'json']); + $this->app->singleton(CustomMimeTypes::class, static function (Application $app): CustomMimeTypes { + return new CustomMimeTypes(['text/csv' => 'json']); }); Route::get('location', static function (): Responsable { return new TestResponse(); @@ -328,16 +328,16 @@ public function toCsvResponse($response): string $this->assertSame('expected query', $response->content()); } - public function testOverridingFallbackExtensionGlobally(): void + public function testOverridingFallbackExtensionGloballyForApis(): void { - $this->app->bind(FallbackExtension::class, static function (): FallbackExtension { - return new FallbackExtension('json'); + $this->app->bind(ApiFallbackExtension::class, static function (): ApiFallbackExtension { + return new ApiFallbackExtension('json'); }); Route::get('location', static function (): Responsable { return new TestResponse(); }); - $response = $this->get('location'); + $response = $this->get('location', ['Accept' => null]); $response->assertOk(); $this->assertSame('expected json response', $response->content()); From fbd23679b1ba4f98f0c0aeacdfa4094632d351ae Mon Sep 17 00:00:00 2001 From: Tim MacDonald Date: Tue, 15 Sep 2020 21:12:09 +1000 Subject: [PATCH 10/11] wip --- src/Method.php | 1 - src/Multiformat.php | 33 +++++++++++---- src/MultiformatResponseServiceProvider.php | 14 +------ tests/MultiformatResponseTest.php | 48 ++++++++++++++++++---- 4 files changed, 68 insertions(+), 28 deletions(-) diff --git a/src/Method.php b/src/Method.php index 6e034e2..17e2ab1 100644 --- a/src/Method.php +++ b/src/Method.php @@ -12,7 +12,6 @@ use function is_callable; use function method_exists; use TiMacDonald\Multiformat\Contracts\Extension; -use TiMacDonald\Multiformat\ApiFallbackExtension; class Method { diff --git a/src/Multiformat.php b/src/Multiformat.php index 7fd04c6..e463c36 100644 --- a/src/Multiformat.php +++ b/src/Multiformat.php @@ -4,10 +4,12 @@ namespace TiMacDonald\Multiformat; -use function app; use function array_key_exists; use function array_merge; +use function assert; use Exception; +use Illuminate\Contracts\Support\Responsable; +use Illuminate\Foundation\Application; trait Multiformat { @@ -58,17 +60,32 @@ public function withApiFallbackExtension(string $extension) * * @param \Illuminate\Http\Request $request * - * @return mixed + * @return \Symfony\Component\HttpFoundation\Response */ public function toResponse($request) { - $method = app(Method::class)->parse( - $request, - $this, - $this->apiFallbackExtension ?? app(ApiFallbackExtension::class) - ); + $app = Application::getInstance(); + + $method = $app->make(Method::class); + assert($method instanceof Method); + + $fallback = $this->apiFallbackExtension; + + if ($fallback === null) { + $fallback = $app->make(ApiFallbackExtension::class); + + assert($fallback instanceof ApiFallbackExtension); + } + + $callable = $method->parse($request, $this, $fallback); + + $response = $app->call($callable, ['request' => $request]); + + while ($response instanceof Responsable) { + $response = $response->toResponse($request); + } - return app()->call($method, ['request' => $request]); + return $response; } /** diff --git a/src/MultiformatResponseServiceProvider.php b/src/MultiformatResponseServiceProvider.php index cd22b04..5e5b754 100644 --- a/src/MultiformatResponseServiceProvider.php +++ b/src/MultiformatResponseServiceProvider.php @@ -6,12 +6,10 @@ use function assert; use Illuminate\Contracts\Foundation\Application; -use Illuminate\Contracts\Support\DeferrableProvider; use Illuminate\Support\ServiceProvider; use TiMacDonald\Multiformat\Contracts\Extension as ExtensionContract; -use TiMacDonald\Multiformat\ApiFallbackExtension; -class MultiformatResponseServiceProvider extends ServiceProvider implements DeferrableProvider +class MultiformatResponseServiceProvider extends ServiceProvider { public function register(): void { @@ -39,15 +37,7 @@ public function register(): void return new Extension([ $urlExtension, $mimeExtension, - ], $fallbackExtension); + ]); }); } - - public function provides() - { - return [ - ApiFallbackExtension::class, - ExtensionContract::class, - ]; - } } diff --git a/tests/MultiformatResponseTest.php b/tests/MultiformatResponseTest.php index e10e16e..c7174d5 100644 --- a/tests/MultiformatResponseTest.php +++ b/tests/MultiformatResponseTest.php @@ -7,12 +7,13 @@ use function assert; use Exception; use Illuminate\Config\Repository; -use Illuminate\Contracts\Foundation\Application; use Illuminate\Contracts\Support\Responsable; use Illuminate\Http\Request; +use Illuminate\Http\Response; use Illuminate\Support\Facades\Route; use function is_string; use Orchestra\Testbench\TestCase; +use function response; use stdClass; use TiMacDonald\Multiformat\ApiFallbackExtension; use TiMacDonald\Multiformat\BaseMultiformatResponse; @@ -260,7 +261,7 @@ public function toCsvResponse(stdClass $stdClass): string $this->assertSame('expected value', $response->content()); } - public function testCanSetDefaultResponseFormat(): void + public function testCanSetDefaultResponseFormatForApis(): void { Route::get('location', static function (): Responsable { return TestResponse::make([])->withApiFallbackExtension('csv'); @@ -283,13 +284,13 @@ public function testExceptionIsThrowIfNoResponseMethodExists(): void $this->get('location.mp3'); } - public function testUrlHtmlFormatIsUsedWhenTheDefaultHasAnotherValue(): void + public function testUrlHtmlFormatIsUsedWhenTheDefaultHasAnotherValueForApis(): void { Route::get('location{format}', static function (): Responsable { return (new TestResponse())->withApiFallbackExtension('csv'); }); - $response = $this->get('location.html'); + $response = $this->get('location.html', ['Accept' => null]); $response->assertOk(); $this->assertSame('expected html response', $response->content()); @@ -297,7 +298,7 @@ public function testUrlHtmlFormatIsUsedWhenTheDefaultHasAnotherValue(): void public function testCanOverrideFormats(): void { - $this->app->singleton(CustomMimeTypes::class, static function (Application $app): CustomMimeTypes { + $this->app->bind(CustomMimeTypes::class, static function (): CustomMimeTypes { return new CustomMimeTypes(['text/csv' => 'json']); }); Route::get('location', static function (): Responsable { @@ -316,9 +317,13 @@ public function testUntypedRequestVariableIsPassedThrough(): void return new class() extends BaseMultiformatResponse { use Multiformat; - public function toCsvResponse($response): string + public function toCsvResponse(Request $request): string { - return $response->input('query'); + $query = $request->input('query'); + + assert(is_string($query)); + + return $query; } }; }); @@ -343,6 +348,35 @@ public function testOverridingFallbackExtensionGloballyForApis(): void $this->assertSame('expected json response', $response->content()); } + public function testItCanReturnNestedResponsables(): void + { + Route::get('location', static function (): Responsable { + return new class() implements Responsable { + use Multiformat; + + public function toHtmlResponse(): Responsable + { + return new class() implements Responsable { + /** + * @param \Illuminate\Http\Request $request + * + * @return \Symfony\Component\HttpFoundation\Response + */ + public function toResponse($request) + { + return new Response('expected from nexted'); + } + }; + } + }; + }); + + $response = $this->get('location'); + + $response->assertOk(); + $this->assertSame('expected from nexted', $response->content()); + } + private function config(): Repository { $config = $this->app->make('config'); From 549af6b5d9f3c6db912bb04c002a12cf7f3ae88c Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 16 Sep 2020 15:47:14 +0000 Subject: [PATCH 11/11] Update symfony/mime requirement from ^4.3 to ^5.1 Updates the requirements on [symfony/mime](https://github.com/symfony/mime) to permit the latest version. - [Release notes](https://github.com/symfony/mime/releases) - [Changelog](https://github.com/symfony/mime/blob/master/CHANGELOG.md) - [Commits](https://github.com/symfony/mime/compare/v4.3.0...v5.1.5) Signed-off-by: dependabot-preview[bot] --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 3f0066c..eb1b85b 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "php": "^7.2", "illuminate/http": "5.8.*", "illuminate/support": "5.8.*", - "symfony/mime": "^4.3" + "symfony/mime": "^5.1" }, "require-dev": { "ergebnis/composer-normalize": "^2.7",