From b9d680db86d5939453d53db7c0a9a9434e4b66ce Mon Sep 17 00:00:00 2001 From: Valentin Clavreul Date: Fri, 13 Oct 2023 09:30:25 +0200 Subject: [PATCH] feat: remove curl factory override --- .../M6WebGuzzleHttpExtension.php | 14 +--- src/Handler/CurlFactory.php | 28 ------- src/Resources/config/services.yml | 1 - tests/Units/Handler/CurlMultiHandler.php | 77 ++++--------------- 4 files changed, 16 insertions(+), 104 deletions(-) delete mode 100644 src/Handler/CurlFactory.php diff --git a/src/DependencyInjection/M6WebGuzzleHttpExtension.php b/src/DependencyInjection/M6WebGuzzleHttpExtension.php index ca1c13c..90300ca 100644 --- a/src/DependencyInjection/M6WebGuzzleHttpExtension.php +++ b/src/DependencyInjection/M6WebGuzzleHttpExtension.php @@ -151,24 +151,14 @@ protected function loadClient(ContainerBuilder $container, string $clientId, arr */ protected function setGuzzleProxyHandler(ContainerBuilder $container, ?string $clientId, array $config, bool $isDebugEnabled) { - // arguments (3 and 50) in handler factories below represents the maximum number of idle handles. - // the values are the default defined in guzzle CurlHandler and CurlMultiHandler - $handlerFactorySync = new Definition('%m6web_guzlehttp.handler.curlfactory.class%'); - $handlerFactorySync->setPublic(true); - $handlerFactorySync->setArguments([3]); - - $handlerFactoryNormal = new Definition('%m6web_guzlehttp.handler.curlfactory.class%'); - $handlerFactoryNormal->setPublic(true); - $handlerFactoryNormal->setArguments([50]); - $curlHandler = new Definition('%m6web_guzlehttp.handler.curlhandler.class%'); $curlHandler->setPublic(true); - $curlHandler->setArguments([new Reference('event_dispatcher'), ['handle_factory' => $handlerFactorySync]]); + $curlHandler->setArguments([new Reference('event_dispatcher')]); $curlHandler->addMethodCall('setDebug', [$isDebugEnabled]); $curlMultiHandler = new Definition('%m6web_guzlehttp.handler.curlmultihandler.class%'); $curlMultiHandler->setPublic(true); - $curlMultiHandler->setArguments([new Reference('event_dispatcher'), ['handle_factory' => $handlerFactoryNormal]]); + $curlMultiHandler->setArguments([new Reference('event_dispatcher')]); $curlMultiHandler->addMethodCall('setDebug', [$isDebugEnabled]); if (isset($config['guzzlehttp_cache'])) { diff --git a/src/Handler/CurlFactory.php b/src/Handler/CurlFactory.php deleted file mode 100644 index 316925a..0000000 --- a/src/Handler/CurlFactory.php +++ /dev/null @@ -1,28 +0,0 @@ - BC is not ensured) -use GuzzleHttp\Handler\EasyHandle; - -/** - * Extends the Guzzle curl factory to set curl info in response - * - * @deprecated the curlInfo dynamic property will be deleted - * in favor of [the native on_stat option](https://docs.guzzlephp.org/en/latest/request-options.html#on-stats) - */ -class CurlFactory extends GuzzleCurlFactory -{ - /** - * {@inheritdoc} - */ - public function release(EasyHandle $easy): void - { - if (!is_null($easy->response)) { - $easy->response->curlInfo = curl_getinfo($easy->handle); - } - - parent::release($easy); - } -} diff --git a/src/Resources/config/services.yml b/src/Resources/config/services.yml index 34b736a..11956c4 100644 --- a/src/Resources/config/services.yml +++ b/src/Resources/config/services.yml @@ -1,5 +1,4 @@ parameters: - m6web_guzlehttp.handler.curlfactory.class: 'M6Web\Bundle\GuzzleHttpBundle\Handler\CurlFactory' m6web_guzlehttp.handler.curlhandler.class: 'M6Web\Bundle\GuzzleHttpBundle\Handler\CurlHandler' m6web_guzlehttp.handler.curlmultihandler.class: 'M6Web\Bundle\GuzzleHttpBundle\Handler\CurlMultiHandler' diff --git a/tests/Units/Handler/CurlMultiHandler.php b/tests/Units/Handler/CurlMultiHandler.php index f6ead8b..45443e1 100644 --- a/tests/Units/Handler/CurlMultiHandler.php +++ b/tests/Units/Handler/CurlMultiHandler.php @@ -10,38 +10,29 @@ /** * Class CurlMultiHandler - * Used for testing trait and curlFactory + * Used for testing trait */ class CurlMultiHandler extends \atoum { public function testNoCache() { - $curlFactoryMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Handler\CurlFactory(3); - $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($request = new Request('GET', 'http://httpbin.org')) ->then ->object($response = $testedClass($request, [])->wait()) ->isInstanceOf('GuzzleHttp\Psr7\Response') ->integer($response->getStatusCode()) ->isEqualTo(200) - ->mock($curlFactoryMock) - ->call('release') - ->once() - ->array($response->curlInfo) - ->isNotEmpty() ; } public function testCacheSet() { - $curlFactoryMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Handler\CurlFactory(3); - $cacheMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Cache\CacheInterface(); $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($testedClass->setCache($cacheMock, 500, false)) ->and($request = new Request('GET', 'http://httpbin.org')) ->then @@ -49,11 +40,6 @@ public function testCacheSet() ->isInstanceOf('GuzzleHttp\Psr7\Response') ->integer($response->getStatusCode()) ->isEqualTo(200) - ->mock($curlFactoryMock) - ->call('release') - ->once() - ->array($response->curlInfo) - ->isNotEmpty() ->mock($cacheMock) ->call('set') ->withAtLeastArguments(['1' => $this->getSerializedResponse($response), '2' => 500]) @@ -63,8 +49,6 @@ public function testCacheSet() public function testCacheSetWithHeader() { - $curlFactoryMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Handler\CurlFactory(3); - $cacheData = []; $cacheMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Cache\CacheInterface(); $cacheMock->getMockController()->set = function ($key, $value) use (&$cacheData) { @@ -80,7 +64,7 @@ public function testCacheSetWithHeader() // Add Header as part of the cache key but "X-" headers must be ignored $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($testedClass->setCache($cacheMock, 500, false)) ->and($request1 = new Request('GET', 'http://httpbin.org', ['user-agent' => 'Netscape 1', 'X-Ddos-Me' => uniqid()])) ->and($request2 = new Request('GET', 'http://httpbin.org', ['user-agent' => 'Netscape 1', 'X-Ddos-Me' => uniqid()])) @@ -98,9 +82,6 @@ public function testCacheSetWithHeader() ->isEqualTo(200) ->integer($response3->getStatusCode()) ->isEqualTo(200) - ->mock($curlFactoryMock) - ->call('release') - ->twice() ->mock($cacheMock) ->call('get') ->thrice() @@ -113,7 +94,7 @@ public function testCacheSetWithHeader() // A header in the Vary should be in the cache even if it's an X- $cacheMock->getMockController()->resetCalls(); $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($testedClass->setCache($cacheMock, 500, false)) ->and($request1 = new Request('GET', 'http://httpbin.org', ['user-agent' => 'Netscape 1', 'X-Important' => 'raoul', 'Vary' => 'X-Important'])) ->and($request2 = new Request('GET', 'http://httpbin.org', ['user-agent' => 'Netscape 1', 'X-Important' => 'raoul2', 'Vary' => 'X-Important'])) @@ -132,8 +113,6 @@ public function testCacheSetWithHeader() public function testCacheGet() { - $curlFactoryMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Handler\CurlFactory(3); - $cacheMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Cache\CacheInterface(); $cacheMock->getMockController()->has = true; @@ -141,7 +120,7 @@ public function testCacheGet() $cacheMock->getMockController()->ttl = 256; $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($testedClass->setCache($cacheMock, 500, false)) ->and($testedClass->setDebug(true)) ->and($request = new Request('GET', 'http://httpbin.org')) @@ -165,9 +144,6 @@ public function testCacheGet() ->call('ttl') ->withAnyArguments() ->once() - ->mock($curlFactoryMock) - ->call('release') - ->never() ; // Test unserialize issue @@ -187,23 +163,18 @@ public function testCacheGet() ->call('ttl') ->withAnyArguments() ->once() - ->mock($curlFactoryMock) - ->call('release') - ->once() ; } public function testForceCache() { - $curlFactoryMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Handler\CurlFactory(3); - $cacheMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Cache\CacheInterface(); $cacheMock->getMockController()->has = false; $cacheMock->getMockController()->get = null; $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($testedClass->setCache($cacheMock, 500, false)) ->and($request = new Request('GET', 'http://httpbin.org')) ->then @@ -216,23 +187,18 @@ public function testForceCache() ->call('set') ->withAtLeastArguments(['1' => $this->getSerializedResponse($response), '2' => 500]) ->once() - ->mock($curlFactoryMock) - ->call('release') - ->once() ; } public function testCacheCustomTtl() { - $curlFactoryMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Handler\CurlFactory(3); - $cacheMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Cache\CacheInterface(); $cacheMock->getMockController()->has = false; $cacheMock->getMockController()->get = null; $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($testedClass->setCache($cacheMock, 500, false)) ->and($request = new Request('GET', 'http://httpbin.org')) ->then @@ -242,16 +208,11 @@ public function testCacheCustomTtl() ->call('set') ->withAtLeastArguments(['1' => $this->getSerializedResponse($response), '2' => 200]) ->once() - ->mock($curlFactoryMock) - ->call('release') - ->once() ; } public function testCacheUseHeader() { - $curlFactoryMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Handler\CurlFactory(3); - $cacheMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Cache\CacheInterface(); $cacheMock->getMockController()->has = false; @@ -259,7 +220,7 @@ public function testCacheUseHeader() // use header ttl $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($testedClass->setCache($cacheMock, 500, true)) ->and($request = new Request('GET', 'http://httpbin.org/cache/200')) ->then @@ -269,15 +230,12 @@ public function testCacheUseHeader() ->call('set') ->withAtLeastArguments(['1' => $this->getSerializedResponse($response), '2' => 200]) ->once() - ->mock($curlFactoryMock) - ->call('release') - ->once() ->and($this->resetMock($cacheMock)) ; // 200s of cache but force to 500 $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($testedClass->setCache($cacheMock, 500, false)) ->and($request = new Request('GET', 'http://httpbin.org/cache/200')) ->then @@ -292,7 +250,7 @@ public function testCacheUseHeader() // use header ttl and no cache $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($testedClass->setCache($cacheMock, 500, true)) ->and($request = new Request('GET', 'http://httpbin.org/cache/0')) ->then @@ -306,7 +264,7 @@ public function testCacheUseHeader() // no cache in header but forced to 500 $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($testedClass->setCache($cacheMock, 500, false)) ->and($request = new Request('GET', 'http://httpbin.org/cache/0')) ->then @@ -322,8 +280,6 @@ public function testCacheUseHeader() public function testCacheGetDebugOff() { - $curlFactoryMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Handler\CurlFactory(3); - $cacheMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Cache\CacheInterface(); $cacheMock->getMockController()->has = true; @@ -331,7 +287,7 @@ public function testCacheGetDebugOff() $cacheMock->getMockController()->ttl = 256; $this - ->if($testedClass = new TestedClass($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock])) + ->if($testedClass = new TestedClass($this->getMockDispatcher())) ->and($testedClass->setCache($cacheMock, 500, false)) ->and($testedClass->setDebug(false)) ->and($request = new Request('GET', 'http://httpbin.org')) @@ -352,9 +308,6 @@ public function testCacheGetDebugOff() ->once() ->call('ttl') ->never() - ->mock($curlFactoryMock) - ->call('release') - ->never() ; } @@ -372,9 +325,7 @@ protected function getSerializedResponse(Response $response) public function testGetKey() { - $curlFactoryMock = new \mock\M6Web\Bundle\GuzzleHttpBundle\Handler\CurlFactory(3); - - $testedClass = new FakeCurlMultiHandler($this->getMockDispatcher(), ['handle_factory' => $curlFactoryMock]); + $testedClass = new FakeCurlMultiHandler($this->getMockDispatcher()); $this->if( $request = new \mock\GuzzleHttp\Psr7\Request(