From 02151549d5bf50b0280ef497a3ac5425c8dad6d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jib=C3=A9=20Barth?= Date: Mon, 4 Mar 2024 11:56:55 +0100 Subject: [PATCH 1/8] [HttpClient] Preserve float in JsonMockResponse --- Response/JsonMockResponse.php | 2 +- Tests/Response/JsonMockResponseTest.php | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Response/JsonMockResponse.php b/Response/JsonMockResponse.php index 66372aa..9372dbe 100644 --- a/Response/JsonMockResponse.php +++ b/Response/JsonMockResponse.php @@ -21,7 +21,7 @@ class JsonMockResponse extends MockResponse public function __construct(mixed $body = [], array $info = []) { try { - $json = json_encode($body, \JSON_THROW_ON_ERROR); + $json = json_encode($body, \JSON_THROW_ON_ERROR | \JSON_PRESERVE_ZERO_FRACTION); } catch (\JsonException $e) { throw new InvalidArgumentException('JSON encoding failed: '.$e->getMessage(), $e->getCode(), $e); } diff --git a/Tests/Response/JsonMockResponseTest.php b/Tests/Response/JsonMockResponseTest.php index b371c08..768353b 100644 --- a/Tests/Response/JsonMockResponseTest.php +++ b/Tests/Response/JsonMockResponseTest.php @@ -59,6 +59,22 @@ public function testJsonEncodeString() $this->assertSame('application/json', $response->getHeaders()['content-type'][0]); } + public function testJsonEncodeFloat() + { + $client = new MockHttpClient(new JsonMockResponse([ + 'foo' => 1.23, + 'ccc' => 1.0, + 'baz' => 10., + ])); + $response = $client->request('GET', 'https://symfony.com'); + + $this->assertSame([ + 'foo' => 1.23, + 'ccc' => 1., + 'baz' => 10., + ], $response->toArray()); + } + /** * @dataProvider responseHeadersProvider */ From ded8034a02fe737c0657bf2f190e9fabb839e0e8 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 6 Mar 2024 16:35:34 +0100 Subject: [PATCH 2/8] [HttpClient] Fix PHP 8.3 deprecation in tests --- Tests/HttpClientTraitTest.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Tests/HttpClientTraitTest.php b/Tests/HttpClientTraitTest.php index 613d80c..3e81f42 100644 --- a/Tests/HttpClientTraitTest.php +++ b/Tests/HttpClientTraitTest.php @@ -72,10 +72,8 @@ public function testPrepareRequestWithBodyIsArray() public function testNormalizeBodyMultipart() { $file = fopen('php://memory', 'r+'); - stream_context_set_option($file, ['http' => [ - 'filename' => 'test.txt', - 'content_type' => 'text/plain', - ]]); + stream_context_set_option($file, 'http', 'filename', 'test.txt'); + stream_context_set_option($file, 'http', 'content_type', 'text/plain'); fwrite($file, 'foobarbaz'); rewind($file); From ef00caa8ddf797ce630c08ddd3fb1bbb8a782c87 Mon Sep 17 00:00:00 2001 From: Arjen van der Meijden Date: Fri, 8 Mar 2024 15:43:25 +0100 Subject: [PATCH 3/8] [HttpClient] Lazily initialize CurlClientState --- CurlHttpClient.php | 61 +++++++++++++++++++++++++----------- Tests/CurlHttpClientTest.php | 4 +-- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/CurlHttpClient.php b/CurlHttpClient.php index 52e1c74..3a2fba0 100644 --- a/CurlHttpClient.php +++ b/CurlHttpClient.php @@ -50,6 +50,9 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface, */ private $logger; + private $maxHostConnections; + private $maxPendingPushes; + /** * An internal object to share state between the client and its responses. * @@ -70,18 +73,22 @@ public function __construct(array $defaultOptions = [], int $maxHostConnections throw new \LogicException('You cannot use the "Symfony\Component\HttpClient\CurlHttpClient" as the "curl" extension is not installed.'); } + $this->maxHostConnections = $maxHostConnections; + $this->maxPendingPushes = $maxPendingPushes; + $this->defaultOptions['buffer'] = $this->defaultOptions['buffer'] ?? \Closure::fromCallable([__CLASS__, 'shouldBuffer']); if ($defaultOptions) { [, $this->defaultOptions] = self::prepareRequest(null, null, $defaultOptions, $this->defaultOptions); } - - $this->multi = new CurlClientState($maxHostConnections, $maxPendingPushes); } public function setLogger(LoggerInterface $logger): void { - $this->logger = $this->multi->logger = $logger; + $this->logger = $logger; + if (isset($this->multi)) { + $this->multi->logger = $logger; + } } /** @@ -91,6 +98,8 @@ public function setLogger(LoggerInterface $logger): void */ public function request(string $method, string $url, array $options = []): ResponseInterface { + $multi = $this->ensureState(); + [$url, $options] = self::prepareRequest($method, $url, $options, $this->defaultOptions); $scheme = $url['scheme']; $authority = $url['authority']; @@ -161,25 +170,25 @@ public function request(string $method, string $url, array $options = []): Respo } // curl's resolve feature varies by host:port but ours varies by host only, let's handle this with our own DNS map - if (isset($this->multi->dnsCache->hostnames[$host])) { - $options['resolve'] += [$host => $this->multi->dnsCache->hostnames[$host]]; + if (isset($multi->dnsCache->hostnames[$host])) { + $options['resolve'] += [$host => $multi->dnsCache->hostnames[$host]]; } - if ($options['resolve'] || $this->multi->dnsCache->evictions) { + if ($options['resolve'] || $multi->dnsCache->evictions) { // First reset any old DNS cache entries then add the new ones - $resolve = $this->multi->dnsCache->evictions; - $this->multi->dnsCache->evictions = []; + $resolve = $multi->dnsCache->evictions; + $multi->dnsCache->evictions = []; $port = parse_url($authority, \PHP_URL_PORT) ?: ('http:' === $scheme ? 80 : 443); if ($resolve && 0x072A00 > CurlClientState::$curlVersion['version_number']) { // DNS cache removals require curl 7.42 or higher - $this->multi->reset(); + $multi->reset(); } foreach ($options['resolve'] as $host => $ip) { $resolve[] = null === $ip ? "-$host:$port" : "$host:$port:$ip"; - $this->multi->dnsCache->hostnames[$host] = $ip; - $this->multi->dnsCache->removals["-$host:$port"] = "-$host:$port"; + $multi->dnsCache->hostnames[$host] = $ip; + $multi->dnsCache->removals["-$host:$port"] = "-$host:$port"; } $curlopts[\CURLOPT_RESOLVE] = $resolve; @@ -281,8 +290,8 @@ public function request(string $method, string $url, array $options = []): Respo $curlopts += $options['extra']['curl']; } - if ($pushedResponse = $this->multi->pushedResponses[$url] ?? null) { - unset($this->multi->pushedResponses[$url]); + if ($pushedResponse = $multi->pushedResponses[$url] ?? null) { + unset($multi->pushedResponses[$url]); if (self::acceptPushForRequest($method, $options, $pushedResponse)) { $this->logger && $this->logger->debug(sprintf('Accepting pushed response: "%s %s"', $method, $url)); @@ -290,7 +299,7 @@ public function request(string $method, string $url, array $options = []): Respo // Reinitialize the pushed response with request's options $ch = $pushedResponse->handle; $pushedResponse = $pushedResponse->response; - $pushedResponse->__construct($this->multi, $url, $options, $this->logger); + $pushedResponse->__construct($multi, $url, $options, $this->logger); } else { $this->logger && $this->logger->debug(sprintf('Rejecting pushed response: "%s"', $url)); $pushedResponse = null; @@ -300,7 +309,7 @@ public function request(string $method, string $url, array $options = []): Respo if (!$pushedResponse) { $ch = curl_init(); $this->logger && $this->logger->info(sprintf('Request: "%s %s"', $method, $url)); - $curlopts += [\CURLOPT_SHARE => $this->multi->share]; + $curlopts += [\CURLOPT_SHARE => $multi->share]; } foreach ($curlopts as $opt => $value) { @@ -310,7 +319,7 @@ public function request(string $method, string $url, array $options = []): Respo } } - return $pushedResponse ?? new CurlResponse($this->multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host), CurlClientState::$curlVersion['version_number']); + return $pushedResponse ?? new CurlResponse($multi, $ch, $options, $this->logger, $method, self::createRedirectResolver($options, $host), CurlClientState::$curlVersion['version_number']); } /** @@ -324,9 +333,11 @@ public function stream($responses, ?float $timeout = null): ResponseStreamInterf throw new \TypeError(sprintf('"%s()" expects parameter 1 to be an iterable of CurlResponse objects, "%s" given.', __METHOD__, get_debug_type($responses))); } - if (\is_resource($this->multi->handle) || $this->multi->handle instanceof \CurlMultiHandle) { + $multi = $this->ensureState(); + + if (\is_resource($multi->handle) || $multi->handle instanceof \CurlMultiHandle) { $active = 0; - while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->multi->handle, $active)) { + while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($multi->handle, $active)) { } } @@ -335,7 +346,9 @@ public function stream($responses, ?float $timeout = null): ResponseStreamInterf public function reset() { - $this->multi->reset(); + if (isset($this->multi)) { + $this->multi->reset(); + } } /** @@ -439,6 +452,16 @@ private static function createRedirectResolver(array $options, string $host): \C }; } + private function ensureState(): CurlClientState + { + if (!isset($this->multi)) { + $this->multi = new CurlClientState($this->maxHostConnections, $this->maxPendingPushes); + $this->multi->logger = $this->logger; + } + + return $this->multi; + } + private function findConstantName(int $opt): ?string { $constants = array_filter(get_defined_constants(), static function ($v, $k) use ($opt) { diff --git a/Tests/CurlHttpClientTest.php b/Tests/CurlHttpClientTest.php index 284a243..ec43a83 100644 --- a/Tests/CurlHttpClientTest.php +++ b/Tests/CurlHttpClientTest.php @@ -63,9 +63,9 @@ public function testHandleIsReinitOnReset() { $httpClient = $this->getHttpClient(__FUNCTION__); - $r = new \ReflectionProperty($httpClient, 'multi'); + $r = new \ReflectionMethod($httpClient, 'ensureState'); $r->setAccessible(true); - $clientState = $r->getValue($httpClient); + $clientState = $r->invoke($httpClient); $initialShareId = $clientState->share; $httpClient->reset(); self::assertNotSame($initialShareId, $clientState->share); From 429aa3d16339606a506d1e90535ecb0007853b2c Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 20 Mar 2024 15:00:29 +0100 Subject: [PATCH 4/8] [HttpClient] Test with guzzle promises v2 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 7f546b3..735d69a 100644 --- a/composer.json +++ b/composer.json @@ -35,7 +35,7 @@ "amphp/http-client": "^4.2.1", "amphp/http-tunnel": "^1.0", "amphp/socket": "^1.1", - "guzzlehttp/promises": "^1.4", + "guzzlehttp/promises": "^1.4|^2.0", "nyholm/psr7": "^1.0", "php-http/httplug": "^1.0|^2.0", "php-http/message-factory": "^1.0", From ce87ad0961411b872119749afb9f74f9ec9f2356 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Tue, 26 Mar 2024 11:11:58 +0100 Subject: [PATCH 5/8] stop all server processes after tests have run --- Tests/DataCollector/HttpClientDataCollectorTest.php | 5 +++++ Tests/HttplugClientTest.php | 5 +++++ Tests/Psr18ClientTest.php | 5 +++++ Tests/RetryableHttpClientTest.php | 5 +++++ Tests/TraceableHttpClientTest.php | 5 +++++ composer.json | 2 +- 6 files changed, 26 insertions(+), 1 deletion(-) diff --git a/Tests/DataCollector/HttpClientDataCollectorTest.php b/Tests/DataCollector/HttpClientDataCollectorTest.php index 15a3136..54e160b 100644 --- a/Tests/DataCollector/HttpClientDataCollectorTest.php +++ b/Tests/DataCollector/HttpClientDataCollectorTest.php @@ -24,6 +24,11 @@ public static function setUpBeforeClass(): void TestHttpServer::start(); } + public static function tearDownAfterClass(): void + { + TestHttpServer::stop(); + } + public function testItCollectsRequestCount() { $httpClient1 = $this->httpClientThatHasTracedRequests([ diff --git a/Tests/HttplugClientTest.php b/Tests/HttplugClientTest.php index 48dabb6..0e62425 100644 --- a/Tests/HttplugClientTest.php +++ b/Tests/HttplugClientTest.php @@ -32,6 +32,11 @@ public static function setUpBeforeClass(): void TestHttpServer::start(); } + public static function tearDownAfterClass(): void + { + TestHttpServer::stop(); + } + public function testSendRequest() { $client = new HttplugClient(new NativeHttpClient()); diff --git a/Tests/Psr18ClientTest.php b/Tests/Psr18ClientTest.php index d4bae3a..d1f4deb 100644 --- a/Tests/Psr18ClientTest.php +++ b/Tests/Psr18ClientTest.php @@ -28,6 +28,11 @@ public static function setUpBeforeClass(): void TestHttpServer::start(); } + public static function tearDownAfterClass(): void + { + TestHttpServer::stop(); + } + public function testSendRequest() { $factory = new Psr17Factory(); diff --git a/Tests/RetryableHttpClientTest.php b/Tests/RetryableHttpClientTest.php index 9edf413..c15b0d2 100644 --- a/Tests/RetryableHttpClientTest.php +++ b/Tests/RetryableHttpClientTest.php @@ -27,6 +27,11 @@ class RetryableHttpClientTest extends TestCase { + public static function tearDownAfterClass(): void + { + TestHttpServer::stop(); + } + public function testRetryOnError() { $client = new RetryableHttpClient( diff --git a/Tests/TraceableHttpClientTest.php b/Tests/TraceableHttpClientTest.php index 5f20e19..052400b 100644 --- a/Tests/TraceableHttpClientTest.php +++ b/Tests/TraceableHttpClientTest.php @@ -29,6 +29,11 @@ public static function setUpBeforeClass(): void TestHttpServer::start(); } + public static function tearDownAfterClass(): void + { + TestHttpServer::stop(); + } + public function testItTracesRequest() { $httpClient = $this->createMock(HttpClientInterface::class); diff --git a/composer.json b/composer.json index 735d69a..2326e9f 100644 --- a/composer.json +++ b/composer.json @@ -25,7 +25,7 @@ "php": ">=7.2.5", "psr/log": "^1|^2|^3", "symfony/deprecation-contracts": "^2.1|^3", - "symfony/http-client-contracts": "^2.4", + "symfony/http-client-contracts": "^2.6", "symfony/polyfill-php73": "^1.11", "symfony/polyfill-php80": "^1.16", "symfony/service-contracts": "^1.0|^2|^3" From ac556bca5bacaa238c73ea053e70e8e46fbfdf84 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 1 Apr 2024 20:50:03 +0200 Subject: [PATCH 6/8] Revert bumping contract version --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 2326e9f..72cc232 100644 --- a/composer.json +++ b/composer.json @@ -25,7 +25,7 @@ "php": ">=7.2.5", "psr/log": "^1|^2|^3", "symfony/deprecation-contracts": "^2.1|^3", - "symfony/http-client-contracts": "^2.6", + "symfony/http-client-contracts": "^2.5", "symfony/polyfill-php73": "^1.11", "symfony/polyfill-php80": "^1.16", "symfony/service-contracts": "^1.0|^2|^3" From 2a292194f6d4cf22d2348248d1c637750f72309d Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 1 Apr 2024 20:54:44 +0200 Subject: [PATCH 7/8] Fix tests --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 72cc232..c340d20 100644 --- a/composer.json +++ b/composer.json @@ -25,7 +25,7 @@ "php": ">=7.2.5", "psr/log": "^1|^2|^3", "symfony/deprecation-contracts": "^2.1|^3", - "symfony/http-client-contracts": "^2.5", + "symfony/http-client-contracts": "^2.5.3", "symfony/polyfill-php73": "^1.11", "symfony/polyfill-php80": "^1.16", "symfony/service-contracts": "^1.0|^2|^3" From 6a46c0ea9b099f9a5132d560a51833ffcbd5b0d9 Mon Sep 17 00:00:00 2001 From: Christian Flothmann Date: Mon, 1 Apr 2024 22:35:36 +0200 Subject: [PATCH 8/8] fix contracts constraint --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 84aae6b..ef456a6 100644 --- a/composer.json +++ b/composer.json @@ -25,8 +25,8 @@ "php": ">=8.1", "psr/log": "^1|^2|^3", "symfony/deprecation-contracts": "^2.5|^3", - "symfony/http-client-contracts": "^3", - "symfony/service-contracts": "^2.5.3|^3.4.2" + "symfony/http-client-contracts": "^3.4.1", + "symfony/service-contracts": "^2.5|^3" }, "require-dev": { "amphp/amp": "^2.5",