Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace php-http/message-factory to psr/http-factory, HttpClientDiscovery to Psr18ClientDiscovery, MessageFactoryDiscovery to Psr17FactoryDiscovery and Http\Client\HttpClient to Psr\Http\Client\ClientInterface #269

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Replace php-http/message-factory to psr/http-factory, HttpClientDiscovery to Psr18ClientDiscovery, MessageFactoryDiscovery to Psr17FactoryDiscovery and Http\Client\HttpClient to Psr\Http\Client\ClientInterface #269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b1d990
chore: Replace php-http/message-factory to psr/http-factory
Apr 15, 2024
f6aa5e8
chore: Replace HttpClientDiscovery to Psr18ClientDiscovery, MessageFa…
Apr 15, 2024
6c4b2b5
fix: code lint
Apr 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
"php": "^7.2|^8",
"php-http/client-implementation": "^1",
"php-http/message": "^1.5",
"php-http/message-factory": "^1.1",
"psr/http-factory": "^1.0.2",
"php-http/discovery": "^1.14",
"symfony/http-foundation": "^2.1|^3|^4|^5|^6|^7",
"moneyphp/money": "^3.1|^4.0.3"
Expand Down
34 changes: 13 additions & 21 deletions src/Common/Http/Client.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,14 @@

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 Psr\Http\Client\ClientInterface as HttpClientInterface;
use Http\Discovery\Psr18ClientDiscovery;
use Http\Discovery\Psr17FactoryDiscovery;
use Omnipay\Common\Http\Exception\NetworkException;
use Omnipay\Common\Http\Exception\RequestException;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\RequestFactoryInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\StreamInterface;
use Psr\Http\Message\UriInterface;

class Client implements ClientInterface
Expand All @@ -20,38 +18,32 @@ 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 HttpClientInterface
*/
private $httpClient;

/**
* @var RequestFactory
* @var RequestFactoryInterface
*/
private $requestFactory;

public function __construct($httpClient = null, RequestFactory $requestFactory = null)
public function __construct($httpClient = null, RequestFactoryInterface $requestFactory = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So these are different interfaces, which would require significant BC break.

{
$this->httpClient = $httpClient ?: HttpClientDiscovery::find();
$this->requestFactory = $requestFactory ?: MessageFactoryDiscovery::find();
$this->httpClient = $httpClient ?: Psr18ClientDiscovery::find();
$this->requestFactory = $requestFactory ?: Psr17FactoryDiscovery::findRequestFactory();
}

/**
* @param $method
* @param $uri
* @param array $headers
* @param string|array|resource|StreamInterface|null $body
* @param string $protocolVersion
* @param string $method
* @param string|UriInterface $uri
* @return ResponseInterface
* @throws \Http\Client\Exception
*/
public function request(
$method,
$uri,
array $headers = [],
$body = null,
$protocolVersion = '1.1'
$uri
) {
$request = $this->requestFactory->createRequest($method, $uri, $headers, $body, $protocolVersion);
$request = $this->requestFactory->createRequest($method, $uri);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. Can we not set the headers/body/protocol on the request before sending?


return $this->sendRequest($request);
}
Expand Down
8 changes: 1 addition & 7 deletions src/Common/Http/ClientInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ interface ClientInterface
*
* @param string $method
* @param string|UriInterface $uri
* @param array $headers
* @param resource|string|StreamInterface|null $body
* @param string $protocolVersion
*
* @throws RequestException when the HTTP client is passed a request that is invalid and cannot be sent.
* @throws NetworkException if there is an error with the network or the remote server cannot be reached.
Expand All @@ -26,9 +23,6 @@ interface ClientInterface
*/
public function request(
$method,
$uri,
array $headers = [],
$body = null,
$protocolVersion = '1.1'
$uri
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also a breaking change

);
}
16 changes: 2 additions & 14 deletions tests/Common/Http/ClientTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
use Http\Client\Exception\NetworkException;
use Mockery as m;
use GuzzleHttp\Psr7\Request;
use Http\Client\HttpClient;
use Http\Message\RequestFactory;
use Psr\Http\Client\ClientInterface as HttpClient;
use Psr\Http\Message\RequestFactoryInterface as RequestFactory;
use Omnipay\Common\Http\Exception\RequestException;
use Omnipay\Tests\TestCase;

Expand All @@ -26,9 +26,6 @@ public function testSend()
$mockFactory->shouldReceive('createRequest')->withArgs([
'GET',
'/path',
[],
null,
'1.1',
])->andReturn($request);

$mockClient->shouldReceive('sendRequest')
Expand All @@ -52,9 +49,6 @@ public function testSendException()
$mockFactory->shouldReceive('createRequest')->withArgs([
'GET',
'/path',
[],
null,
'1.1',
])->andReturn($request);

$mockClient->shouldReceive('sendRequest')
Expand All @@ -79,9 +73,6 @@ public function testSendNetworkException()
$mockFactory->shouldReceive('createRequest')->withArgs([
'GET',
'/path',
[],
null,
'1.1',
])->andReturn($request);

$mockClient->shouldReceive('sendRequest')
Expand All @@ -106,9 +97,6 @@ public function testSendExceptionGetRequest()
$mockFactory->shouldReceive('createRequest')->withArgs([
'GET',
'/path',
[],
null,
'1.1',
])->andReturn($request);

$exception = new \Exception('Something went wrong');
Expand Down