From f074a25a49429d7a26c7a90afb5d5c092dd955cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pila=C5=99=20Jakub?= Date: Thu, 4 Jan 2024 13:15:52 +0100 Subject: [PATCH] Replace HTTP Factory class --- CHANGELOG.md | 4 ++ composer.json | 7 +-- phpstan.neon | 2 - src/Http/RequestManager.php | 60 +++++++++++++++++--------- src/Matej.php | 20 +++++++-- tests/unit/Http/RequestManagerTest.php | 48 +++++++++++---------- tests/unit/MatejTest.php | 35 +++++++-------- 7 files changed, 106 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a08a314..a046302 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ ## Unreleased ### Changed - Require PHP ^7.3 +- Drop old version php-http/client-common ^1.6 + +### Added +- Replace abandoned package php-http/message-factory. Using psr/http-factory instead. ## 4.1.0 - 2021-08-19 ### Added diff --git a/composer.json b/composer.json index e2dfd7b..0d16ab2 100644 --- a/composer.json +++ b/composer.json @@ -16,13 +16,14 @@ "beberlei/assert": "^2.8 || ^3.0", "fig/http-message-util": "^1.1", "myclabs/php-enum": "^1.6", - "php-http/client-common": "^1.6 || ^2.0", + "php-http/client-common": "^2.0", "php-http/client-implementation": "^1.0", - "php-http/discovery": "^1.0", + "php-http/discovery": "^1.17", "php-http/httplug": "^1.1 || ^2.0", "php-http/message": "^1.6", - "php-http/message-factory": "^1.0", "php-http/promise": "^1.0", + "psr/http-client": "^1.0", + "psr/http-factory": "^1.0", "psr/http-message": "^1.0", "ramsey/uuid": "^3.7 || ^4.0" }, diff --git a/phpstan.neon b/phpstan.neon index ea6f9ca..401162a 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,8 +4,6 @@ parameters: path: tests/ - message: '#Parameter .+ of class Lmc\\Matej\\Matej constructor expects string#' path: tests/integration/IntegrationTestCase.php - - message: '#Parameter \#4 \$body of method Http\\Message\\RequestFactory::createRequest\(\) expects#' - path: src/Http/RequestManager.php - '#Unsafe usage of new static\(\)#' - message: '#expects class-string#' path: tests/ diff --git a/src/Http/RequestManager.php b/src/Http/RequestManager.php index dfc1e7f..ab525cf 100644 --- a/src/Http/RequestManager.php +++ b/src/Http/RequestManager.php @@ -5,15 +5,16 @@ use Http\Client\Common\Plugin\AuthenticationPlugin; use Http\Client\Common\Plugin\HeaderSetPlugin; use Http\Client\Common\PluginClient; -use Http\Client\HttpClient; -use Http\Discovery\HttpClientDiscovery; -use Http\Discovery\MessageFactoryDiscovery; -use Http\Message\MessageFactory; +use Http\Discovery\Psr17FactoryDiscovery; +use Http\Discovery\Psr18ClientDiscovery; use Lmc\Matej\Http\Plugin\ExceptionPlugin; use Lmc\Matej\Matej; use Lmc\Matej\Model\Request; use Lmc\Matej\Model\Response; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\StreamFactoryInterface; /** * Encapsulates HTTP layer, ie. request/response handling. @@ -31,12 +32,14 @@ class RequestManager protected $accountId; /** @var string */ protected $apiKey; - /** @var HttpClient */ + /** @var ClientInterface */ protected $httpClient; - /** @var MessageFactory */ + /** @var RequestFactoryInterface */ protected $messageFactory; /** @var ResponseDecoderInterface */ protected $responseDecoder; + /** @var StreamFactoryInterface */ + protected $streamFactory; public function __construct(string $accountId, string $apiKey) { @@ -56,13 +59,13 @@ public function sendRequest(Request $request): Response } /** @codeCoverageIgnore */ - public function setHttpClient(HttpClient $httpClient): void + public function setHttpClient(ClientInterface $httpClient): void { $this->httpClient = $httpClient; } /** @codeCoverageIgnore */ - public function setMessageFactory(MessageFactory $messageFactory): void + public function setMessageFactory(RequestFactoryInterface $messageFactory): void { $this->messageFactory = $messageFactory; } @@ -73,27 +76,33 @@ public function setResponseDecoder(ResponseDecoderInterface $responseDecoder): v $this->responseDecoder = $responseDecoder; } + /** @codeCoverageIgnore */ + public function setStreamFactory(StreamFactoryInterface $streamFactory): void + { + $this->streamFactory = $streamFactory; + } + /** @codeCoverageIgnore */ public function setBaseUrl(string $baseUrl): void { $this->baseUrl = $baseUrl; } - protected function getHttpClient(): HttpClient + protected function getHttpClient(): ClientInterface { if ($this->httpClient === null) { // @codeCoverageIgnoreStart - $this->httpClient = HttpClientDiscovery::find(); + $this->httpClient = Psr18ClientDiscovery::find(); // @codeCoverageIgnoreEnd } return $this->httpClient; } - protected function getMessageFactory(): MessageFactory + protected function getMessageFactory(): RequestFactoryInterface { if ($this->messageFactory === null) { - $this->messageFactory = MessageFactoryDiscovery::find(); + $this->messageFactory = Psr17FactoryDiscovery::findRequestFactory(); } return $this->messageFactory; @@ -108,7 +117,16 @@ protected function getResponseDecoder(): ResponseDecoderInterface return $this->responseDecoder; } - protected function createConfiguredHttpClient(): HttpClient + protected function getStreamFactory(): StreamFactoryInterface + { + if ($this->streamFactory === null) { + $this->streamFactory = Psr17FactoryDiscovery::findStreamFactory(); + } + + return $this->streamFactory; + } + + protected function createConfiguredHttpClient(): ClientInterface { return new PluginClient( $this->getHttpClient(), @@ -122,19 +140,19 @@ protected function createConfiguredHttpClient(): HttpClient protected function createHttpRequestFromMatejRequest(Request $request): RequestInterface { - $requestBody = json_encode($request->getData()); // TODO: use \Safe\json_encode + $requestBody = $this->getStreamFactory()->createStream( + json_encode($request->getData(), JSON_THROW_ON_ERROR) + ); // TODO: use \Safe\json_encode $uri = $this->buildBaseUrl() . $request->getPath(); return $this->getMessageFactory() ->createRequest( $request->getMethod(), - $uri, - [ - 'Content-Type' => 'application/json', - static::REQUEST_ID_HEADER => $request->getRequestId(), - ], - $requestBody - ); + $uri + ) + ->withHeader('Content-Type', 'application/json') + ->withHeader(static::REQUEST_ID_HEADER, $request->getRequestId()) + ->withBody($requestBody); } protected function buildBaseUrl(): string diff --git a/src/Matej.php b/src/Matej.php index 78d8e79..be11325 100644 --- a/src/Matej.php +++ b/src/Matej.php @@ -2,11 +2,12 @@ namespace Lmc\Matej; -use Http\Client\HttpClient; -use Http\Message\MessageFactory; use Lmc\Matej\Http\RequestManager; use Lmc\Matej\Http\ResponseDecoderInterface; use Lmc\Matej\RequestBuilder\RequestBuilderFactory; +use Psr\Http\Client\ClientInterface; +use Psr\Http\Message\RequestFactoryInterface; +use Psr\Http\Message\StreamFactoryInterface; class Matej { @@ -27,7 +28,7 @@ public function request(): RequestBuilderFactory } /** @return $this */ - public function setHttpClient(HttpClient $client): self + public function setHttpClient(ClientInterface $client): self { $this->getRequestManager()->setHttpClient($client); @@ -49,7 +50,7 @@ public function setBaseUrl(string $baseUrl): self * @codeCoverageIgnore * @return $this */ - public function setHttpMessageFactory(MessageFactory $messageFactory): self + public function setHttpMessageFactory(RequestFactoryInterface $messageFactory): self { $this->getRequestManager()->setMessageFactory($messageFactory); @@ -67,6 +68,17 @@ public function setHttpResponseDecoder(ResponseDecoderInterface $responseDecoder return $this; } + /** + * @codeCoverageIgnore + * @return $this + */ + public function setStreamFactory(StreamFactoryInterface $streamFactory): self + { + $this->getRequestManager()->setStreamFactory($streamFactory); + + return $this; + } + protected function getRequestManager(): RequestManager { return $this->requestManager; diff --git a/tests/unit/Http/RequestManagerTest.php b/tests/unit/Http/RequestManagerTest.php index f1e7ee4..2e7d6d5 100644 --- a/tests/unit/Http/RequestManagerTest.php +++ b/tests/unit/Http/RequestManagerTest.php @@ -3,11 +3,11 @@ namespace Lmc\Matej\Http; use Fig\Http\Message\RequestMethodInterface; -use Http\Mock\Client; +use Http\Discovery\Psr18Client; use Lmc\Matej\Matej; use Lmc\Matej\Model\Request; -use Lmc\Matej\Model\Response; use Lmc\Matej\UnitTestCase; +use Psr\Http\Message\RequestInterface; /** * @covers \Lmc\Matej\Http\RequestManager @@ -26,8 +26,29 @@ public function shouldSendAndDecodeRequest(): void __DIR__ . '/Fixtures/response-one-successful-command.json' ); - $mockClient = new Client(); - $mockClient->addResponse($dummyHttpResponse); + $mockClient = $this->createMock(Psr18Client::class); + $mockClient->expects($this->once()) + ->method('sendRequest') + ->with($this->callback(function (RequestInterface $request) { + $this->assertMatchesRegularExpression( + '~https\://account\-id\.matej\.lmc\.cz/foo/endpoint\?hmac_timestamp\=[0-9]+&hmac_sign\=[[:alnum:]]~', + $request->getUri()->__toString() + ); + $this->assertSame(RequestMethodInterface::METHOD_PUT, $request->getMethod()); + $this->assertJsonStringEqualsJsonString( + '{"foo":"bar","list":{"lorem":"ipsum","dolor":333}}', + $request->getBody()->__toString() + ); + $this->assertSame(['application/json'], $request->getHeader('Content-Type')); + $this->assertSame(['custom-request-id'], $request->getHeader(RequestManager::REQUEST_ID_HEADER)); + $this->assertSame( + Matej::CLIENT_ID . '/' . Matej::VERSION, + $request->getHeader(RequestManager::CLIENT_VERSION_HEADER)[0] + ); + + return true; + })) + ->willReturn($dummyHttpResponse); $requestManager = new RequestManager('account-id', 'api-key'); $requestManager->setHttpClient($mockClient); @@ -41,24 +62,5 @@ public function shouldSendAndDecodeRequest(): void // Response decoding is comprehensively tested in ResponseDecoderTest $requestManager->sendRequest($request); - - // Assert properties of the send request - $recordedRequests = $mockClient->getRequests(); - $this->assertCount(1, $recordedRequests); - $this->assertMatchesRegularExpression( - '~https\://account\-id\.matej\.lmc\.cz/foo/endpoint\?hmac_timestamp\=[0-9]+&hmac_sign\=[[:alnum:]]~', - $recordedRequests[0]->getUri()->__toString() - ); - $this->assertSame(RequestMethodInterface::METHOD_PUT, $recordedRequests[0]->getMethod()); - $this->assertJsonStringEqualsJsonString( - '{"foo":"bar","list":{"lorem":"ipsum","dolor":333}}', - $recordedRequests[0]->getBody()->__toString() - ); - $this->assertSame(['application/json'], $recordedRequests[0]->getHeader('Content-Type')); - $this->assertSame(['custom-request-id'], $recordedRequests[0]->getHeader(RequestManager::REQUEST_ID_HEADER)); - $this->assertSame( - Matej::CLIENT_ID . '/' . Matej::VERSION, - $recordedRequests[0]->getHeader(RequestManager::CLIENT_VERSION_HEADER)[0] - ); } } diff --git a/tests/unit/MatejTest.php b/tests/unit/MatejTest.php index d5f073c..4740ce5 100644 --- a/tests/unit/MatejTest.php +++ b/tests/unit/MatejTest.php @@ -2,7 +2,8 @@ namespace Lmc\Matej; -use Http\Mock\Client; +use GuzzleHttp\Psr7\Request; +use Http\Discovery\Psr18Client; use Lmc\Matej\Model\Command\ItemPropertySetup; use Lmc\Matej\Model\CommandResponse; @@ -25,8 +26,10 @@ public function shouldExecuteRequestViaBuilder(): void __DIR__ . '/Http/Fixtures/response-one-successful-command.json' ); - $mockClient = new Client(); - $mockClient->addResponse($dummyHttpResponse); + $mockClient = $this->createMock(Psr18Client::class); + $mockClient->expects($this->once()) + ->method('sendRequest') + ->willReturn($dummyHttpResponse); $matej = new Matej('account-id', 'apiKey'); $matej->setHttpClient($mockClient); @@ -36,12 +39,6 @@ public function shouldExecuteRequestViaBuilder(): void ->addProperty(ItemPropertySetup::timestamp('valid_from')) ->send(); - $this->assertCount(1, $mockClient->getRequests()); - $this->assertStringStartsWith( - 'https://account-id.matej.lmc.cz/', - $mockClient->getRequests()[0]->getUri()->__toString() - ); - $this->assertSame(1, $response->getNumberOfCommands()); $this->assertSame(1, $response->getNumberOfSuccessfulCommands()); $this->assertSame(0, $response->getNumberOfSkippedCommands()); @@ -58,8 +55,18 @@ public function shouldOverwriteBaseUrl(): void __DIR__ . '/Http/Fixtures/response-one-successful-command.json' ); - $mockClient = new Client(); - $mockClient->addResponse($dummyHttpResponse); + $mockClient = $this->createMock(Psr18Client::class); + $mockClient->expects($this->once()) + ->method('sendRequest') + ->with($this->callback(function (Request $request) { + $this->assertStringStartsWith( + 'https://nobody.nowhere.com/account-id', + $request->getUri()->__toString() + ); + + return true; + })) + ->willReturn($dummyHttpResponse); $matej = new Matej('account-id', 'apiKey'); $matej->setHttpClient($mockClient); @@ -70,11 +77,5 @@ public function shouldOverwriteBaseUrl(): void ->setupItemProperties() ->addProperty(ItemPropertySetup::timestamp('valid_from')) ->send(); - - $this->assertCount(1, $mockClient->getRequests()); - $this->assertStringStartsWith( - 'https://nobody.nowhere.com/account-id', - $mockClient->getRequests()[0]->getUri()->__toString() - ); } }