From 0c17a3854d8f5c2c5782ff03482e110abc8d364d Mon Sep 17 00:00:00 2001 From: Stephan Vock Date: Wed, 15 Nov 2023 10:42:36 +0000 Subject: [PATCH] Replace PHP-HTTP usage with PSR-17 --- CHANGELOG.md | 6 +- composer.json | 13 +++- lib/Bitbucket/API/Http/Client.php | 26 +++++--- .../API/Http/HttpPluginClientBuilder.php | 62 ++++++++++++++++--- .../Http/Plugin/ApiOneCollectionPlugin.php | 25 ++++---- .../API/Http/Plugin/ApiVersionPlugin.php | 3 +- lib/Bitbucket/API/Http/Response/Pager.php | 28 ++++----- test/Bitbucket/Tests/API/Http/ClientTest.php | 12 ++-- .../Plugin/ApiOneCollectionPluginTest.php | 4 +- .../Http/Plugin/NormalizeArrayPluginTest.php | 4 +- .../Tests/API/Http/Response/PagerTest.php | 5 +- test/Bitbucket/Tests/API/TestCase.php | 19 ++++-- 12 files changed, 137 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d25a07..130eed3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,9 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/). -## 2.2.0 / Unreleased -### Added: +## 2.2.0 / 2021-11-15 ### Changed: + - Uses PSR-17 internally instead of the deprecated PHP-HTTP `MessageFactory` ### Deprecated: - Usage of `Api:api` with not fully qualified class name - Usage of `Api::HTTP_RESPONSE_*` constants @@ -12,6 +12,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). - Passing a string as `$params` to the methods `create` and `update` of the `PullRequests`, `BranchRestrictions`, `Pipelines`, and `Repository` API - `ClientInterface`, use `Client` instead - Passing a string as `$params` to the `request` method of `Client` + - Usage of `Client::getMessageFactory` use `Client::getRequestFactory` instead + - Passing `Http\Message\MessageFactory` to `HttpPluginClientBuilder` ## 2.1.0 / 2021-07-23 diff --git a/composer.json b/composer.json index 3047ecf..821f204 100644 --- a/composer.json +++ b/composer.json @@ -23,14 +23,16 @@ "php-http/discovery": "^1.0", "php-http/client-implementation": "^1.0", "php-http/client-common": "^2.0", - "symfony/deprecation-contracts": "^2.2 || ^3.0" + "symfony/deprecation-contracts": "^2.2 || ^3.0", + "psr/http-factory-implementation": "^1.0" }, "require-dev": { "phpunit/phpunit":"^7.5|^8|^9", "php-http/mock-client": " ^1.2", "squizlabs/php_codesniffer": "^3.5", - "php-http/guzzle6-adapter": "^2.0", - "phpstan/phpstan": "^0.12.90|^1" + "phpstan/phpstan": "^0.12.90|^1", + "nyholm/psr7": "^1.6.1", + "php-http/message-factory": "^1.0" }, "replace": { "gentle/bitbucket-api": "*" @@ -53,5 +55,10 @@ "scripts": { "style": "php vendor/bin/phpcs --standard=psr2 lib/ test --ignore=*/HistoryVersionBridge.php", "test": "php vendor/bin/phpunit" + }, + "config": { + "allow-plugins": { + "php-http/discovery": true + } } } diff --git a/lib/Bitbucket/API/Http/Client.php b/lib/Bitbucket/API/Http/Client.php index 2dabe3e..2979e44 100644 --- a/lib/Bitbucket/API/Http/Client.php +++ b/lib/Bitbucket/API/Http/Client.php @@ -14,10 +14,11 @@ use Bitbucket\API\Http\Plugin\HistoryPlugin; use Http\Client\Common\HttpMethodsClient; use Http\Client\Common\Plugin; -use Http\Discovery\UriFactoryDiscovery; -use Http\Message\MessageFactory; +use Http\Discovery\Psr17FactoryDiscovery; +use Psr\Http\Message\RequestFactoryInterface; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\StreamFactoryInterface; /** * @author Alexandru G. @@ -40,8 +41,10 @@ class Client implements ClientInterface /** @var HttpPluginClientBuilder */ private $httpClientBuilder; - /** @var MessageFactory */ - private $messageFactory; + /** @var RequestFactoryInterface */ + private $requestFactory; + /** @var StreamFactoryInterface */ + private $streamFactory; /** @var HistoryPlugin */ private $responseHistory; @@ -55,7 +58,7 @@ public function __construct(array $options = array(), HttpPluginClientBuilder $h $this->httpClientBuilder = $httpClientBuilder ?: new HttpPluginClientBuilder(); $this->httpClientBuilder->addPlugin( - new Plugin\AddHostPlugin(UriFactoryDiscovery::find()->createUri($this->options['base_url'])) + new Plugin\AddHostPlugin(Psr17FactoryDiscovery::findUriFactory()->createUri($this->options['base_url'])) ); $this->httpClientBuilder->addPlugin(new Plugin\RedirectPlugin()); $this->httpClientBuilder->addPlugin(new Plugin\HeaderDefaultsPlugin([ @@ -65,7 +68,8 @@ public function __construct(array $options = array(), HttpPluginClientBuilder $h $this->setApiVersion($this->options['api_version']); - $this->messageFactory = $this->httpClientBuilder->getMessageFactory(); + $this->requestFactory = $this->httpClientBuilder->getRequestFactory(); + $this->streamFactory = Psr17FactoryDiscovery::findStreamFactory(); } /** @@ -124,7 +128,7 @@ public function request($endpoint, $params = array(), $method = 'GET', array $he } } - $body = null; + $body = ''; if (is_string($paramsString) && $paramsString !== null) { $body = $paramsString; } @@ -140,7 +144,13 @@ public function request($endpoint, $params = array(), $method = 'GET', array $he $endpoint .= (strpos($endpoint, '?') === false ? '?' : '&').'format='.$this->getResponseFormat(); } - $request = $this->messageFactory->createRequest($method, $endpoint, $headers, $body); + $request = $this->requestFactory + ->createRequest($method, $endpoint) + ->withBody($this->streamFactory->createStream($body)); + + foreach ($headers as $name => $value) { + $request = $request->withHeader($name, $value); + } return $this->getClient()->sendRequest($request); } diff --git a/lib/Bitbucket/API/Http/HttpPluginClientBuilder.php b/lib/Bitbucket/API/Http/HttpPluginClientBuilder.php index 6e349a5..e2d9ad3 100644 --- a/lib/Bitbucket/API/Http/HttpPluginClientBuilder.php +++ b/lib/Bitbucket/API/Http/HttpPluginClientBuilder.php @@ -6,26 +6,56 @@ use Http\Client\Common\HttpMethodsClient; use Http\Client\Common\Plugin; use Http\Client\Common\PluginClient; -use Http\Client\HttpClient; -use Http\Discovery\HttpClientDiscovery; use Http\Discovery\MessageFactoryDiscovery; +use Http\Discovery\Psr17FactoryDiscovery; +use Http\Discovery\Psr18ClientDiscovery; use Http\Message\MessageFactory; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; +use Psr\Http\Message\StreamFactoryInterface; class HttpPluginClientBuilder { - /** @var HttpClient */ + /** @var ClientInterface */ private $httpClient; /** @var HttpMethodsClient|null */ private $pluginClient; - /** @var MessageFactory */ - private $messageFactory; + /** @var MessageFactory|RequestFactoryInterface */ + private $requestFactory; + /** @var StreamFactoryInterface */ + private $streamFactory; /** @var Plugin[] */ private $plugins = []; - public function __construct(HttpClient $httpClient = null, MessageFactory $messageFactory = null) + /** + * @param MessageFactory|RequestFactoryInterface|null $requestFactory + */ + public function __construct(ClientInterface $httpClient = null, $requestFactory = null, StreamFactoryInterface $streamFactory = null) { - $this->httpClient = $httpClient ?: HttpClientDiscovery::find(); - $this->messageFactory = $messageFactory ?: MessageFactoryDiscovery::find(); + $requestFactory = $requestFactory ?? Psr17FactoryDiscovery::findRequestFactory(); + if ($requestFactory instanceof MessageFactory) { + // Use same format as symfony/deprecation-contracts. + @trigger_error(sprintf( + 'Since %s %s: %s is deprecated, use %s instead.', + 'private-packagist/bitbucket-api', + '2.2.0', + '\Http\Message\MessageFactory', + RequestFactoryInterface::class + ), \E_USER_DEPRECATED); + } elseif (!$requestFactory instanceof RequestFactoryInterface) { + /** @var mixed $requestFactory value unknown; set to mixed, prevent PHPStan complaining about guard clauses */ + throw new \TypeError(sprintf( + '%s::__construct(): Argument #2 ($requestFactory) must be of type %s|%s, %s given', + self::class, + '\Http\Message\MessageFactory', + RequestFactoryInterface::class, + is_object($requestFactory) ? get_class($requestFactory) : gettype($requestFactory) + )); + } + + $this->httpClient = $httpClient ?: Psr18ClientDiscovery::find(); + $this->requestFactory = $requestFactory; + $this->streamFactory = $streamFactory ?? Psr17FactoryDiscovery::findStreamFactory(); } /** @@ -79,7 +109,8 @@ public function getHttpClient() if (!$this->pluginClient) { $this->pluginClient = new HttpMethodsClient( new PluginClient($this->httpClient, $this->plugins), - $this->messageFactory + $this->requestFactory, + $this->streamFactory ); } @@ -88,9 +119,20 @@ public function getHttpClient() /** * @return MessageFactory + * @deprecated Use getRequestFactory instead. message will be removed with 3.0 */ public function getMessageFactory() { - return $this->messageFactory; + return $this->requestFactory instanceof MessageFactory + ? $this->requestFactory + : MessageFactoryDiscovery::find(); + } + + /** + * @return RequestFactoryInterface + */ + public function getRequestFactory() + { + return $this->requestFactory; } } diff --git a/lib/Bitbucket/API/Http/Plugin/ApiOneCollectionPlugin.php b/lib/Bitbucket/API/Http/Plugin/ApiOneCollectionPlugin.php index 3257211..a2821ff 100644 --- a/lib/Bitbucket/API/Http/Plugin/ApiOneCollectionPlugin.php +++ b/lib/Bitbucket/API/Http/Plugin/ApiOneCollectionPlugin.php @@ -10,10 +10,10 @@ namespace Bitbucket\API\Http\Plugin; use Http\Client\Common\Plugin; -use Http\Discovery\MessageFactoryDiscovery; -use Http\Message\ResponseFactory; +use Http\Discovery\Psr17FactoryDiscovery; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\StreamFactoryInterface; /** * Helper for `Pager` @@ -28,8 +28,8 @@ class ApiOneCollectionPlugin implements Plugin { use Plugin\VersionBridgePlugin; - /** @var ResponseFactory */ - private $responseFactory; + /** @var StreamFactoryInterface */ + private $streamFactory; /** @var array */ private $urlQueryComponents; @@ -40,9 +40,13 @@ class ApiOneCollectionPlugin implements Plugin /** @var array */ private $content; - public function __construct(ResponseFactory $responseFactory = null) + /** + * @param object|null $responseFactory This argument is deprecated and will be removed in 3.0 + */ + public function __construct($responseFactory = null) { - $this->responseFactory = $responseFactory ?: MessageFactoryDiscovery::find(); + $this->streamFactory = Psr17FactoryDiscovery::findStreamFactory(); + unset($responseFactory); } /** @@ -61,13 +65,8 @@ protected function doHandleRequest(RequestInterface $request, callable $next, ca $request ); - return $this->responseFactory->createResponse( - $response->getStatusCode(), - $response->getReasonPhrase(), - $response->getHeaders(), - json_encode($content), - $response->getProtocolVersion() - ); + return $response + ->withBody($this->streamFactory->createStream(json_encode($content))); } } diff --git a/lib/Bitbucket/API/Http/Plugin/ApiVersionPlugin.php b/lib/Bitbucket/API/Http/Plugin/ApiVersionPlugin.php index f4df70f..a8f7aa5 100644 --- a/lib/Bitbucket/API/Http/Plugin/ApiVersionPlugin.php +++ b/lib/Bitbucket/API/Http/Plugin/ApiVersionPlugin.php @@ -5,6 +5,7 @@ use Http\Client\Common\Plugin; use Http\Promise\Promise; use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\ResponseInterface; class ApiVersionPlugin implements Plugin { @@ -26,7 +27,7 @@ public function __construct($version) * @param callable $next Next middleware in the chain, the request is passed as the first argument * @param callable $first First middleware in the chain, used to to restart a request * - * @return Promise Resolves a PSR-7 Response or fails with an Http\Client\Exception (The same as HttpAsyncClient). + * @return Promise Resolves a PSR-7 Response or fails with an Http\Client\Exception (The same as HttpAsyncClient). */ protected function doHandleRequest(RequestInterface $request, callable $next, callable $first) { diff --git a/lib/Bitbucket/API/Http/Response/Pager.php b/lib/Bitbucket/API/Http/Response/Pager.php index 0492c09..ed50b8c 100644 --- a/lib/Bitbucket/API/Http/Response/Pager.php +++ b/lib/Bitbucket/API/Http/Response/Pager.php @@ -10,9 +10,9 @@ namespace Bitbucket\API\Http\Response; use Bitbucket\API\Http\HttpPluginClientBuilder; -use Http\Discovery\MessageFactoryDiscovery; -use Http\Message\MessageFactory; +use Http\Discovery\Psr17FactoryDiscovery; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\StreamFactoryInterface; /** * @author Alexandru Guzinschi @@ -21,22 +21,24 @@ class Pager implements PagerInterface { /** @var HttpPluginClientBuilder */ private $httpPluginClientBuilder; - /** @var MessageFactory */ - private $messageFactory; + /** @var StreamFactoryInterface */ + private $streamFactory; /** @var ResponseInterface */ private $response; /** * @param HttpPluginClientBuilder $httpPluginClientBuilder * @param ResponseInterface $response - * @param MessageFactory $messageFactory + * @param object|null $messageFactory This argument is deprecated and will be removed in 3.0.0 + * @param StreamFactoryInterface|null $streamFactory * * @throws \UnexpectedValueException */ public function __construct( HttpPluginClientBuilder $httpPluginClientBuilder, ResponseInterface $response, - MessageFactory $messageFactory = null + $messageFactory = null, + StreamFactoryInterface $streamFactory = null ) { /** @var ResponseInterface $response */ if ($response->getStatusCode() >= 400) { @@ -45,7 +47,9 @@ public function __construct( $this->httpPluginClientBuilder = $httpPluginClientBuilder; $this->response = $response; - $this->messageFactory = $messageFactory ? : MessageFactoryDiscovery::find(); + $this->streamFactory = $streamFactory ?: Psr17FactoryDiscovery::findStreamFactory(); + + unset($messageFactory); } /** @@ -117,13 +121,9 @@ public function fetchAll() } $content['values'] = $values; - $this->response = $this->messageFactory->createResponse( - $this->response->getStatusCode(), - $this->response->getReasonPhrase(), - $this->response->getHeaders(), - json_encode($content), - $this->response->getProtocolVersion() - ); + + $this->response = $this->response + ->withBody($this->streamFactory->createStream(json_encode($content))); return $this->response; } diff --git a/test/Bitbucket/Tests/API/Http/ClientTest.php b/test/Bitbucket/Tests/API/Http/ClientTest.php index a3a2af5..0265603 100644 --- a/test/Bitbucket/Tests/API/Http/ClientTest.php +++ b/test/Bitbucket/Tests/API/Http/ClientTest.php @@ -79,7 +79,7 @@ public function testShouldDoGetRequestAndReturnResponseInstance(): void { $endpoint = '/repositories/gentle/eof/issues/3'; $params = ['format' => 'json']; - $headers = ['2' => '4']; + $headers = ['Test-Header' => '4']; $baseClient = $this->getHttpPluginClientBuilder(); $client = new Client( [ @@ -98,7 +98,7 @@ public function testShouldDoPostRequestWithContentAndReturnResponseInstance(): v { $endpoint = '/repositories/gentle/eof/issues/3'; $params = ['1' => '2']; - $headers = ['3' => '4']; + $headers = ['Test-Header' => '4']; $baseClient = $this->getHttpPluginClientBuilder(); $client = new Client(['user_agent' => 'tests'], $baseClient); $response = $client->post($endpoint, $params, $headers); @@ -108,7 +108,7 @@ public function testShouldDoPostRequestWithContentAndReturnResponseInstance(): v $this->assertEquals([ 'Content-Type' => ['application/x-www-form-urlencoded'], 'User-Agent' => ['tests'], - '3' => ['4'], + 'Test-Header' => ['4'], 'Host' => ['api.bitbucket.org'], ], $client->getLastRequest()->getHeaders()); } @@ -138,7 +138,7 @@ public function testShouldDoPutRequestAndReturnResponseInstance(): void { $endpoint = '/repositories/gentle/eof/issues/3'; $params = ['1' => '2']; - $headers = ['3' => '4']; + $headers = ['Test-Header' => '4']; $baseClient = $this->getHttpPluginClientBuilder(); $client = new Client([], $baseClient); $response = $client->put($endpoint, $params, $headers); @@ -150,7 +150,7 @@ public function testShouldDoDeleteRequestAndReturnResponseInstance(): void { $endpoint = '/repositories/gentle/eof/issues/3'; $params = ['1' => '2']; - $headers = ['3' => '4']; + $headers = ['Test-Header' => '4']; $baseClient = $this->getHttpPluginClientBuilder(); $client = new Client([], $baseClient); $response = $client->delete($endpoint, $params, $headers); @@ -162,7 +162,7 @@ public function testShouldDoPatchRequestAndReturnResponseInstance(): void { $endpoint = '/repositories/gentle/eof/issues/3'; $params = ['1' => '2']; - $headers = ['3' => '4']; + $headers = ['Test-Header' => '4']; $baseClient = $this->getHttpPluginClientBuilder(); $client = new Client([], $baseClient); $response = $client->request($endpoint, $params, 'PATCH', $headers); diff --git a/test/Bitbucket/Tests/API/Http/Plugin/ApiOneCollectionPluginTest.php b/test/Bitbucket/Tests/API/Http/Plugin/ApiOneCollectionPluginTest.php index 0366ee7..f81c81d 100644 --- a/test/Bitbucket/Tests/API/Http/Plugin/ApiOneCollectionPluginTest.php +++ b/test/Bitbucket/Tests/API/Http/Plugin/ApiOneCollectionPluginTest.php @@ -4,9 +4,9 @@ use Bitbucket\Tests\API as Tests; use Bitbucket\API\Http\Plugin\ApiOneCollectionPlugin; -use GuzzleHttp\Psr7\Request; -use GuzzleHttp\Psr7\Response; use Http\Client\Promise\HttpFulfilledPromise; +use Nyholm\Psr7\Request; +use Nyholm\Psr7\Response; use Psr\Http\Message\ResponseInterface; /** diff --git a/test/Bitbucket/Tests/API/Http/Plugin/NormalizeArrayPluginTest.php b/test/Bitbucket/Tests/API/Http/Plugin/NormalizeArrayPluginTest.php index 1adf0b8..ba3fb2b 100644 --- a/test/Bitbucket/Tests/API/Http/Plugin/NormalizeArrayPluginTest.php +++ b/test/Bitbucket/Tests/API/Http/Plugin/NormalizeArrayPluginTest.php @@ -4,9 +4,9 @@ use Bitbucket\API\Http\Plugin\NormalizeArrayPlugin; use Bitbucket\Tests\API as Tests; -use GuzzleHttp\Psr7\Request; -use GuzzleHttp\Psr7\Response; use Http\Client\Promise\HttpFulfilledPromise; +use Nyholm\Psr7\Request; +use Nyholm\Psr7\Response; /** * @author Alexandru G. diff --git a/test/Bitbucket/Tests/API/Http/Response/PagerTest.php b/test/Bitbucket/Tests/API/Http/Response/PagerTest.php index 22bdc18..c6f3f17 100644 --- a/test/Bitbucket/Tests/API/Http/Response/PagerTest.php +++ b/test/Bitbucket/Tests/API/Http/Response/PagerTest.php @@ -4,7 +4,6 @@ use Bitbucket\API\Http\Response\Pager; use Bitbucket\Tests\API\TestCase; -use GuzzleHttp\Psr7\Response; use Psr\Http\Message\ResponseInterface; /** @@ -105,9 +104,7 @@ public function testFetchAllWithEmptyResponseShouldReturnEmptyValuesArray(): voi public function testFetchAllWithInvalidJsonShouldReturnEmptyValuesArray(): void { $expected = ['values' => []]; - $response = new Response(200, [], '{"something": "yes"'); - $this->getMockHttpClient()->addResponse($response); - + $response = $this->fakeResponse('{"something": "yes"'); $page = new Pager($this->getHttpPluginClientBuilder(), $response); $response = $page->fetchAll(); diff --git a/test/Bitbucket/Tests/API/TestCase.php b/test/Bitbucket/Tests/API/TestCase.php index ae4ea5c..31ef328 100644 --- a/test/Bitbucket/Tests/API/TestCase.php +++ b/test/Bitbucket/Tests/API/TestCase.php @@ -6,6 +6,7 @@ use Bitbucket\API\Http\HttpPluginClientBuilder; use Http\Discovery\MessageFactoryDiscovery; use Http\Mock\Client; +use Nyholm\Psr7\Factory\Psr17Factory; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; @@ -13,6 +14,15 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase { /** @var ?Client */ protected $mockClient; + /** @var Psr17Factory */ + private $psr17Factory; + + protected function setUp(): void + { + parent::setUp(); + + $this->psr17Factory = new Psr17Factory(); + } /** * @template T of \Bitbucket\API\Api @@ -41,14 +51,13 @@ protected function getHttpPluginClientBuilder(): HttpPluginClientBuilder } /** - * @param array $data + * @param array|string $data */ - protected function fakeResponse(array $data, int $statusCode = 200): ResponseInterface + protected function fakeResponse($data, int $statusCode = 200): ResponseInterface { - $messageFactory = MessageFactoryDiscovery::find(); + $response = $this->psr17Factory->createResponse($statusCode) + ->withBody($this->psr17Factory->createStream(is_array($data) ? json_encode($data) : (string) $data)); - $responseBody = json_encode($data); - $response = $messageFactory->createResponse($statusCode, null, [], $responseBody); $this->getMockHttpClient()->addResponse($response); return $response;