From 4c0cbf82bac1f2ef96082c01c2ef82568f589109 Mon Sep 17 00:00:00 2001 From: "Barry vd. Heuvel" Date: Wed, 20 Mar 2019 15:55:37 +0100 Subject: [PATCH 1/8] Implement PSR-17 + PSR-18 --- .travis.yml | 6 ++-- composer.json | 14 +++++---- src/Common/Http/Client.php | 39 ++++++++++++++---------- tests/Omnipay/Common/Http/ClientTest.php | 36 ++++++++-------------- 4 files changed, 45 insertions(+), 50 deletions(-) diff --git a/.travis.yml b/.travis.yml index b674b347..80a63f9a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,6 @@ language: php php: - - 5.6 - - 7.0 - 7.1 - 7.2 - 7.3 @@ -24,9 +22,9 @@ env: matrix: include: - - php: 5.6 + - php: 7.1 env: setup=lowest symfony="^2.1" - - php: 5.6 + - php: 7.1 env: setup=lowest symfony="^3" - php: 7.1 env: setup=lowest symfony="^4" diff --git a/composer.json b/composer.json index 108d7756..ceda5877 100644 --- a/composer.json +++ b/composer.json @@ -38,12 +38,14 @@ "classmap": ["src/Omnipay.php"] }, "require": { - "php": "^5.6|^7", - "php-http/client-implementation": "^1", - "php-http/message": "^1.5", - "php-http/discovery": "^1.2.1", + "php": "^7.1", + "moneyphp/money": "^3.1", + "php-http/discovery": "^1.3", + "psr/http-client": "^1", + "psr/http-client-implementation": "^1", + "psr/http-factory": "^1", "symfony/http-foundation": "^2.1||^3||^4", - "moneyphp/money": "^3.1" + "zendframework/zend-diactoros": "^2.1" }, "require-dev": { "omnipay/tests": "^3", @@ -53,7 +55,7 @@ }, "extra": { "branch-alias": { - "dev-master": "3.0.x-dev" + "dev-master": "3.1.x-dev" } }, "suggest": { diff --git a/src/Common/Http/Client.php b/src/Common/Http/Client.php index abd4a53d..e6f12bfc 100644 --- a/src/Common/Http/Client.php +++ b/src/Common/Http/Client.php @@ -2,37 +2,34 @@ namespace Omnipay\Common\Http; -use function GuzzleHttp\Psr7\str; -use Http\Client\HttpClient; -use Http\Discovery\HttpClientDiscovery; -use Http\Discovery\MessageFactoryDiscovery; -use Http\Message\RequestFactory; +use Http\Discovery\Psr17FactoryDiscovery; +use Http\Discovery\Psr18ClientDiscovery; use Omnipay\Common\Http\Exception\NetworkException; use Omnipay\Common\Http\Exception\RequestException; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\StreamInterface; -use Psr\Http\Message\UriInterface; +use Psr\Http\Client\ClientInterface as Psr18ClientInterface; +use Psr\Http\Message\RequestFactoryInterface as Psr17RequestFactoryInterface; -class Client implements ClientInterface +final class Client implements ClientInterface { /** * The Http Client which implements `public function sendRequest(RequestInterface $request)` - * Note: Will be changed to PSR-18 when released * - * @var HttpClient + * @var Psr18ClientInterface */ private $httpClient; /** - * @var RequestFactory + * @var Psr17RequestFactoryInterface */ private $requestFactory; - public function __construct($httpClient = null, RequestFactory $requestFactory = null) + public function __construct($httpClient = null, $requestFactory = null) { - $this->httpClient = $httpClient ?: HttpClientDiscovery::find(); - $this->requestFactory = $requestFactory ?: MessageFactoryDiscovery::find(); + $this->httpClient = $httpClient ?: Psr18ClientDiscovery::find(); + $this->requestFactory = $requestFactory ?: Psr17FactoryDiscovery::findRequestFactory(); } /** @@ -51,7 +48,17 @@ public function request( $body = null, $protocolVersion = '1.1' ) { - $request = $this->requestFactory->createRequest($method, $uri, $headers, $body, $protocolVersion); + $request = $this->requestFactory + ->createRequest($method, $uri) + ->withProtocolVersion($protocolVersion); + + if ($body) { + $request = $request->withBody($body); + } + + foreach ($headers as $name => $value) { + $request = $request->withHeader($name, $value); + } return $this->sendRequest($request); } @@ -59,7 +66,7 @@ public function request( /** * @param RequestInterface $request * @return ResponseInterface - * @throws \Http\Client\Exception + * @throws NetworkException|RequestException */ private function sendRequest(RequestInterface $request) { @@ -67,7 +74,7 @@ private function sendRequest(RequestInterface $request) return $this->httpClient->sendRequest($request); } catch (\Http\Client\Exception\NetworkException $networkException) { throw new NetworkException($networkException->getMessage(), $request, $networkException); - } catch (\Exception $exception) { + } catch (\Throwable $exception) { throw new RequestException($exception->getMessage(), $request, $exception); } } diff --git a/tests/Omnipay/Common/Http/ClientTest.php b/tests/Omnipay/Common/Http/ClientTest.php index ab9fbf38..4e369f2f 100644 --- a/tests/Omnipay/Common/Http/ClientTest.php +++ b/tests/Omnipay/Common/Http/ClientTest.php @@ -6,10 +6,10 @@ use Http\Client\Exception\NetworkException; use Mockery as m; use GuzzleHttp\Psr7\Request; -use Http\Client\HttpClient; -use Http\Message\RequestFactory; use Omnipay\Common\Http\Exception\RequestException; use Omnipay\Tests\TestCase; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; class ClientTest extends TestCase { @@ -17,14 +17,14 @@ public function testEmptyConstruct() { $client = new Client(); - $this->assertAttributeInstanceOf(HttpClient::class, 'httpClient', $client); - $this->assertAttributeInstanceOf(RequestFactory::class, 'requestFactory', $client); + $this->assertAttributeInstanceOf(ClientInterface::class, 'httpClient', $client); + $this->assertAttributeInstanceOf(RequestFactoryInterface::class, 'requestFactory', $client); } public function testSend() { - $mockClient = m::mock(HttpClient::class); - $mockFactory = m::mock(RequestFactory::class); + $mockClient = m::mock(ClientInterface::class); + $mockFactory = m::mock(RequestFactoryInterface::class); $client = new Client($mockClient, $mockFactory); $request = new Request('GET', '/path'); @@ -33,9 +33,6 @@ public function testSend() $mockFactory->shouldReceive('createRequest')->withArgs([ 'GET', '/path', - [], - null, - '1.1', ])->andReturn($request); $mockClient->shouldReceive('sendRequest') @@ -49,8 +46,8 @@ public function testSend() public function testSendException() { - $mockClient = m::mock(HttpClient::class); - $mockFactory = m::mock(RequestFactory::class); + $mockClient = m::mock(ClientInterface::class); + $mockFactory = m::mock(RequestFactoryInterface::class); $client = new Client($mockClient, $mockFactory); $request = new Request('GET', '/path'); @@ -59,9 +56,6 @@ public function testSendException() $mockFactory->shouldReceive('createRequest')->withArgs([ 'GET', '/path', - [], - null, - '1.1', ])->andReturn($request); $mockClient->shouldReceive('sendRequest') @@ -76,8 +70,8 @@ public function testSendException() public function testSendNetworkException() { - $mockClient = m::mock(HttpClient::class); - $mockFactory = m::mock(RequestFactory::class); + $mockClient = m::mock(ClientInterface::class); + $mockFactory = m::mock(RequestFactoryInterface::class); $client = new Client($mockClient, $mockFactory); $request = new Request('GET', '/path'); @@ -86,9 +80,6 @@ public function testSendNetworkException() $mockFactory->shouldReceive('createRequest')->withArgs([ 'GET', '/path', - [], - null, - '1.1', ])->andReturn($request); $mockClient->shouldReceive('sendRequest') @@ -103,8 +94,8 @@ public function testSendNetworkException() public function testSendExceptionGetRequest() { - $mockClient = m::mock(HttpClient::class); - $mockFactory = m::mock(RequestFactory::class); + $mockClient = m::mock(ClientInterface::class); + $mockFactory = m::mock(RequestFactoryInterface::class); $client = new Client($mockClient, $mockFactory); $request = new Request('GET', '/path'); @@ -113,9 +104,6 @@ public function testSendExceptionGetRequest() $mockFactory->shouldReceive('createRequest')->withArgs([ 'GET', '/path', - [], - null, - '1.1', ])->andReturn($request); $exception = new \Exception('Something went wrong'); From 847601405a6898716429b11c0ae87df7885422b7 Mon Sep 17 00:00:00 2001 From: "Barry vd. Heuvel" Date: Wed, 20 Mar 2019 16:01:39 +0100 Subject: [PATCH 2/8] Bump discovery minimum version --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index ceda5877..2a13f059 100644 --- a/composer.json +++ b/composer.json @@ -40,7 +40,7 @@ "require": { "php": "^7.1", "moneyphp/money": "^3.1", - "php-http/discovery": "^1.3", + "php-http/discovery": "^1.6", "psr/http-client": "^1", "psr/http-client-implementation": "^1", "psr/http-factory": "^1", From a1e3a70c64ee842ce22727434cf026792847bbcd Mon Sep 17 00:00:00 2001 From: "Barry vd. Heuvel" Date: Fri, 22 Mar 2019 06:02:22 +0100 Subject: [PATCH 3/8] Update Client.php --- src/Common/Http/Client.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Common/Http/Client.php b/src/Common/Http/Client.php index e6f12bfc..0804c021 100644 --- a/src/Common/Http/Client.php +++ b/src/Common/Http/Client.php @@ -12,7 +12,7 @@ use Psr\Http\Client\ClientInterface as Psr18ClientInterface; use Psr\Http\Message\RequestFactoryInterface as Psr17RequestFactoryInterface; -final class Client implements ClientInterface +class Client implements ClientInterface { /** * The Http Client which implements `public function sendRequest(RequestInterface $request)` From 8b279c5f0c30f05a37e35fba52570e8d2a95d2f9 Mon Sep 17 00:00:00 2001 From: "Barry vd. Heuvel" Date: Fri, 22 Mar 2019 10:02:10 +0100 Subject: [PATCH 4/8] Add streamFactory, require factory implementation, lazyload factories --- composer.json | 1 + src/Common/Http/Client.php | 53 ++++++++++++++++++++---- tests/Omnipay/Common/Http/ClientTest.php | 52 +++++++++++++++++++---- 3 files changed, 88 insertions(+), 18 deletions(-) diff --git a/composer.json b/composer.json index 2a13f059..b07899b0 100644 --- a/composer.json +++ b/composer.json @@ -44,6 +44,7 @@ "psr/http-client": "^1", "psr/http-client-implementation": "^1", "psr/http-factory": "^1", + "psr/http-factory-implementation": "^1", "symfony/http-foundation": "^2.1||^3||^4", "zendframework/zend-diactoros": "^2.1" }, diff --git a/src/Common/Http/Client.php b/src/Common/Http/Client.php index e6f12bfc..837613e2 100644 --- a/src/Common/Http/Client.php +++ b/src/Common/Http/Client.php @@ -8,28 +8,62 @@ use Omnipay\Common\Http\Exception\RequestException; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Message\StreamInterface; -use Psr\Http\Client\ClientInterface as Psr18ClientInterface; -use Psr\Http\Message\RequestFactoryInterface as Psr17RequestFactoryInterface; +use Psr\Http\Client\ClientInterface as HttpClientInterface; +use Psr\Http\Message\RequestFactoryInterface as RequestFactoryInterface; +use Psr\Http\Message\UriFactoryInterface; final class Client implements ClientInterface { /** * The Http Client which implements `public function sendRequest(RequestInterface $request)` * - * @var Psr18ClientInterface + * @var HttpClientInterface */ private $httpClient; /** - * @var Psr17RequestFactoryInterface + * @var RequestFactoryInterface */ private $requestFactory; + /** + * @var StreamFactoryInterface + */ + private $streamFactory; + public function __construct($httpClient = null, $requestFactory = null) { - $this->httpClient = $httpClient ?: Psr18ClientDiscovery::find(); - $this->requestFactory = $requestFactory ?: Psr17FactoryDiscovery::findRequestFactory(); + $this->httpClient = $httpClient; + $this->requestFactory = $requestFactory; + } + + private function getHttpClient() : HttpClientInterface + { + if (!$this->httpClient) { + $this->httpClient = Psr18ClientDiscovery::find(); + } + + return $this->httpClient; + } + + private function getRequestFactory() : RequestFactoryInterface + { + if (!$this->requestFactory) { + $this->requestFactory = Psr17FactoryDiscovery::findRequestFactory(); + } + + return $this->requestFactory; + } + + private function getStreamFactory() : StreamFactoryInterface + { + if (!$this->streamFactory) { + $this->streamFactory = Psr17FactoryDiscovery::findStreamFactory(); + } + + return $this->streamFactory; } /** @@ -48,12 +82,13 @@ public function request( $body = null, $protocolVersion = '1.1' ) { - $request = $this->requestFactory + $request = $this->getRequestFactory() ->createRequest($method, $uri) ->withProtocolVersion($protocolVersion); if ($body) { - $request = $request->withBody($body); + $stream = $this->getStreamFactory()->createStream($body); + $request = $request->withBody($stream); } foreach ($headers as $name => $value) { @@ -71,7 +106,7 @@ public function request( private function sendRequest(RequestInterface $request) { try { - return $this->httpClient->sendRequest($request); + return $this->getHttpClient()->sendRequest($request); } catch (\Http\Client\Exception\NetworkException $networkException) { throw new NetworkException($networkException->getMessage(), $request, $networkException); } catch (\Throwable $exception) { diff --git a/tests/Omnipay/Common/Http/ClientTest.php b/tests/Omnipay/Common/Http/ClientTest.php index 4e369f2f..bb14adad 100644 --- a/tests/Omnipay/Common/Http/ClientTest.php +++ b/tests/Omnipay/Common/Http/ClientTest.php @@ -10,18 +10,11 @@ use Omnipay\Tests\TestCase; use Psr\Http\Client\ClientInterface; use Psr\Http\Message\RequestFactoryInterface; +use Psr\Http\Message\RequestInterface; class ClientTest extends TestCase { - public function testEmptyConstruct() - { - $client = new Client(); - - $this->assertAttributeInstanceOf(ClientInterface::class, 'httpClient', $client); - $this->assertAttributeInstanceOf(RequestFactoryInterface::class, 'requestFactory', $client); - } - - public function testSend() + public function testSendGet() { $mockClient = m::mock(ClientInterface::class); $mockFactory = m::mock(RequestFactoryInterface::class); @@ -41,7 +34,48 @@ public function testSend() ->once(); $this->assertSame($response, $client->request('GET', '/path')); + } + public function testSendPostJson() + { + $mockClient = m::mock(ClientInterface::class); + $mockFactory = m::mock(RequestFactoryInterface::class); + $client = new Client($mockClient, $mockFactory); + + $request = new Request('POST', '/path'); + $response = new Response(); + + $mockFactory->shouldReceive('createRequest')->withArgs([ + 'POST', + '/path', + ])->andReturn($request); + + $mockClient->shouldReceive('sendRequest') + ->with(m::on(function (RequestInterface $request) { + + if ($request->getMethod() !== 'POST') { + return false; + } + + if ($request->getHeader('Content-Type') !== ['application/json']) { + return false; + } + + if ($request->getBody()->getContents() !== '{foo:bar}') { + return false; + } + + return true; + })) + ->andReturn($response) + ->once(); + + $this->assertSame($response, $client->request( + 'POST', + '/path', + ['Content-Type' => 'application/json'], + '{foo:bar}' + )); } public function testSendException() From b29eb037ddb640da71d8fda0d817aacd5dac6ac3 Mon Sep 17 00:00:00 2001 From: "Barry vd. Heuvel" Date: Fri, 22 Mar 2019 10:04:30 +0100 Subject: [PATCH 5/8] Add streamFactory param --- src/Common/Http/Client.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Common/Http/Client.php b/src/Common/Http/Client.php index 9fcda33f..4a9f53da 100644 --- a/src/Common/Http/Client.php +++ b/src/Common/Http/Client.php @@ -11,8 +11,7 @@ use Psr\Http\Message\StreamFactoryInterface; use Psr\Http\Message\StreamInterface; use Psr\Http\Client\ClientInterface as HttpClientInterface; -use Psr\Http\Message\RequestFactoryInterface as RequestFactoryInterface; -use Psr\Http\Message\UriFactoryInterface; +use Psr\Http\Message\RequestFactoryInterface; class Client implements ClientInterface { @@ -33,10 +32,11 @@ class Client implements ClientInterface */ private $streamFactory; - public function __construct($httpClient = null, $requestFactory = null) + public function __construct($httpClient = null, $requestFactory = null, $streamFactory = null) { $this->httpClient = $httpClient; $this->requestFactory = $requestFactory; + $this->streamFactory = $streamFactory; } private function getHttpClient() : HttpClientInterface From f7067ae640334e68ed4193ff06375aeb298d1cfb Mon Sep 17 00:00:00 2001 From: "Barry vd. Heuvel" Date: Fri, 22 Mar 2019 10:15:46 +0100 Subject: [PATCH 6/8] Check body for string --- src/Common/Http/Client.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Common/Http/Client.php b/src/Common/Http/Client.php index 4a9f53da..ef4b83a1 100644 --- a/src/Common/Http/Client.php +++ b/src/Common/Http/Client.php @@ -70,10 +70,10 @@ private function getStreamFactory() : StreamFactoryInterface * @param $method * @param $uri * @param array $headers - * @param string|array|resource|StreamInterface|null $body + * @param string|null $body * @param string $protocolVersion * @return ResponseInterface - * @throws \Http\Client\Exception + * @throws NetworkException|RequestException */ public function request( $method, @@ -86,9 +86,9 @@ public function request( ->createRequest($method, $uri) ->withProtocolVersion($protocolVersion); - if ($body) { - $stream = $this->getStreamFactory()->createStream($body); - $request = $request->withBody($stream); + if ($body !== null && $body !== '' && is_string($body)) { + $body = $this->getStreamFactory()->createStream($body); + $request = $request->withBody($body); } foreach ($headers as $name => $value) { From 06cde89c5205fa26b3fa60e7bbe1f3b1fca795ff Mon Sep 17 00:00:00 2001 From: "Barry vd. Heuvel" Date: Fri, 22 Mar 2019 13:33:55 +0100 Subject: [PATCH 7/8] Use nyholm/psr7 --- composer.json | 2 +- tests/Omnipay/Common/Http/ClientTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index b07899b0..8852f6fa 100644 --- a/composer.json +++ b/composer.json @@ -46,7 +46,7 @@ "psr/http-factory": "^1", "psr/http-factory-implementation": "^1", "symfony/http-foundation": "^2.1||^3||^4", - "zendframework/zend-diactoros": "^2.1" + "nyholm/psr7": "^1" }, "require-dev": { "omnipay/tests": "^3", diff --git a/tests/Omnipay/Common/Http/ClientTest.php b/tests/Omnipay/Common/Http/ClientTest.php index bb14adad..c17a9cfc 100644 --- a/tests/Omnipay/Common/Http/ClientTest.php +++ b/tests/Omnipay/Common/Http/ClientTest.php @@ -61,7 +61,7 @@ public function testSendPostJson() return false; } - if ($request->getBody()->getContents() !== '{foo:bar}') { + if ((string) $request->getBody() !== '{foo:bar}') { return false; } From 8c6e4419bbadeb51aaf870756a4e16790d068fd1 Mon Sep 17 00:00:00 2001 From: "Barry vd. Heuvel" Date: Mon, 25 Mar 2019 10:13:39 +0100 Subject: [PATCH 8/8] Allow StreamInterface --- src/Common/Http/Client.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Common/Http/Client.php b/src/Common/Http/Client.php index ef4b83a1..931cf95d 100644 --- a/src/Common/Http/Client.php +++ b/src/Common/Http/Client.php @@ -70,7 +70,7 @@ private function getStreamFactory() : StreamFactoryInterface * @param $method * @param $uri * @param array $headers - * @param string|null $body + * @param string|null|StreamInterface $body * @param string $protocolVersion * @return ResponseInterface * @throws NetworkException|RequestException @@ -86,8 +86,10 @@ public function request( ->createRequest($method, $uri) ->withProtocolVersion($protocolVersion); - if ($body !== null && $body !== '' && is_string($body)) { - $body = $this->getStreamFactory()->createStream($body); + if ($body !== null && $body !== '') { + if (is_string($body)) { + $body = $this->getStreamFactory()->createStream($body); + } $request = $request->withBody($body); }