From e52844d7d912ba0df403955f7e22d1e7158cec9e Mon Sep 17 00:00:00 2001 From: Quentin Pautrat Date: Mon, 23 Dec 2019 16:46:04 +0100 Subject: [PATCH] refactor: Rely on PSR HTTP factory interface instead of concrete implementation. (#24) JsonApiFactory had a concrete dependency. To improve usability and respect Dependency Inversion Principle we chose to use interface instead. It allows implementer to use the implementation he wants even if sensio/framework-extra-bundle combined with symfony/psr-http-message-bridge hardly suggest to use nyholm/psr7. That is why we think it is irrelevant to suggest an implementation because one of our requirements already dit that. --- composer.json | 7 ++++--- spec/Factory/JsonApiFactorySpec.php | 23 ++++++++++++++++++----- src/Factory/JsonApiFactory.php | 29 ++++++++++++++++++++--------- src/Request/Request.php | 19 ------------------- src/Resources/config/services.xml | 1 + 5 files changed, 43 insertions(+), 36 deletions(-) diff --git a/composer.json b/composer.json index d50af7d..db71711 100644 --- a/composer.json +++ b/composer.json @@ -12,13 +12,14 @@ ], "require": { "php": "^7.1.0", + "psr/http-factory": "^1.0", + "sensio/framework-extra-bundle": "^5.4", "symfony/psr-http-message-bridge": "^1.2", - "woohoolabs/yin": "^4.0", - "nyholm/psr7": "^1.1", - "sensio/framework-extra-bundle": "^5.4" + "woohoolabs/yin": "^4.0" }, "require-dev": { "friendsofphp/php-cs-fixer": "^2.0", + "nyholm/psr7": "^1.2", "phpspec/phpspec": "^4.3" }, "autoload": { diff --git a/spec/Factory/JsonApiFactorySpec.php b/spec/Factory/JsonApiFactorySpec.php index 09c4387..23dc80a 100644 --- a/spec/Factory/JsonApiFactorySpec.php +++ b/spec/Factory/JsonApiFactorySpec.php @@ -3,6 +3,7 @@ namespace spec\QP\WoohoolabsYinBundle\Factory; use PhpSpec\ObjectBehavior; +use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ServerRequestInterface; use QP\WoohoolabsYinBundle\Factory\JsonApiFactory; use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface; @@ -13,9 +14,15 @@ class JsonApiFactorySpec extends ObjectBehavior { - public function let(HttpMessageFactoryInterface $psr7Factory, RequestStack $requestStack, Request $request, ServerRequestInterface $psrRequest, ExceptionFactoryInterface $exceptionFactory) - { - $this->beConstructedWith($psr7Factory, $exceptionFactory); + public function let( + HttpMessageFactoryInterface $psr7Factory, + RequestStack $requestStack, + Request $request, + ServerRequestInterface $psrRequest, + ResponseFactoryInterface $responseFactory, + ExceptionFactoryInterface $exceptionFactory + ) { + $this->beConstructedWith($psr7Factory, $responseFactory, $exceptionFactory); $requestStack->getCurrentRequest()->willReturn($request); $psr7Factory->createRequest($request)->willReturn($psrRequest); } @@ -25,9 +32,15 @@ public function it_is_initializable() $this->shouldHaveType(JsonApiFactory::class); } - public function it_creates_jsonapi($requestStack, $request, $psr7Factory, $exceptionFactory) - { + public function it_creates_jsonapi( + RequestStack $requestStack, + Request $request, + HttpMessageFactoryInterface $psr7Factory, + ResponseFactoryInterface $responseFactory, + $exceptionFactory + ) { $psr7Factory->createRequest($request)->shouldBeCalled(); + $responseFactory->createResponse()->shouldBeCalled(); $this->create($requestStack)->shouldReturnAnInstanceOf(JsonApi::class); $this->create($requestStack)->getExceptionFactory()->shouldReturn($exceptionFactory); } diff --git a/src/Factory/JsonApiFactory.php b/src/Factory/JsonApiFactory.php index aec1fdc..226c918 100644 --- a/src/Factory/JsonApiFactory.php +++ b/src/Factory/JsonApiFactory.php @@ -2,7 +2,7 @@ namespace QP\WoohoolabsYinBundle\Factory; -use Nyholm\Psr7\Response; +use Psr\Http\Message\ResponseFactoryInterface; use QP\WoohoolabsYinBundle\Request\Request as JsonApiRequest; use Symfony\Bridge\PsrHttpMessage\HttpMessageFactoryInterface; use Symfony\Component\HttpFoundation\RequestStack; @@ -20,7 +20,10 @@ class JsonApiFactory * @var HttpMessageFactoryInterface */ private $psrFactory; - + /** + * @var ResponseFactoryInterface + */ + private $responseFactory; /** * @var ExceptionFactoryInterface */ @@ -28,26 +31,34 @@ class JsonApiFactory /** * Constructor. - * - * @param HttpMessageFactoryInterface $psrFactory */ - public function __construct(HttpMessageFactoryInterface $psrFactory, ExceptionFactoryInterface $exceptionFactory) - { + public function __construct( + HttpMessageFactoryInterface $psrFactory, + ResponseFactoryInterface $responseFactory, + ExceptionFactoryInterface $exceptionFactory + ) { $this->psrFactory = $psrFactory; + $this->responseFactory = $responseFactory; $this->exceptionFactory = $exceptionFactory; } /** * Create a new instance of JsonApi by transforming a HttpFoundation Request into PSR7 Request. * - * @param RequestStack $requestStack - * * @return JsonApi */ public function create(RequestStack $requestStack) { $request = $this->psrFactory->createRequest($requestStack->getCurrentRequest()); + $response = $this->responseFactory->createResponse(); - return new JsonApi(new JsonApiRequest($request, $this->exceptionFactory), new Response(), $this->exceptionFactory); + return new JsonApi( + new JsonApiRequest( + $request, + $this->exceptionFactory + ), + $response, + $this->exceptionFactory + ); } } diff --git a/src/Request/Request.php b/src/Request/Request.php index 8bc6fc0..8bdc779 100644 --- a/src/Request/Request.php +++ b/src/Request/Request.php @@ -10,33 +10,16 @@ class Request extends JsonApiRequest { - /** - * @param int|null $defaultPage - * - * @return FixedPageBasedPagination - */ public function getFixedPageBasedPagination(?int $defaultPage = null): FixedPageBasedPagination { return FixedPageBasedPagination::fromPaginationQueryParams($this->getPagination(), $defaultPage); } - /** - * @param int|null $defaultPage - * @param int|null $defaultSize - * - * @return PageBasedPagination - */ public function getPageBasedPagination(?int $defaultPage = null, ?int $defaultSize = null): PageBasedPagination { return PageBasedPagination::fromPaginationQueryParams($this->getPagination(), $defaultPage, $defaultSize); } - /** - * @param int|null $defaultOffset - * @param int|null $defaultLimit - * - * @return OffsetBasedPagination - */ public function getOffsetBasedPagination( ?int $defaultOffset = null, ?int $defaultLimit = null @@ -46,8 +29,6 @@ public function getOffsetBasedPagination( /** * @param mixed $defaultCursor - * - * @return CursorBasedPagination */ public function getCursorBasedPagination($defaultCursor = null): CursorBasedPagination { diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index d6ed0fe..09d32f9 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -13,6 +13,7 @@ +