googleads#640 handlers stack of provided Guzzle HTTP client should no…

…t be changed by library
mrpavlikov · Feb 6, 2020 · 822dd2d · 822dd2d
1 parent 656e27b
commit 822dd2d
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 9 deletions.
diff --git a/src/Google/AdsApi/Common/AdsGuzzleHttpClientFactory.php b/src/Google/AdsApi/Common/AdsGuzzleHttpClientFactory.php
@@ -64,10 +64,10 @@ public function __construct(
  public function generateHttpClient()
  {
  $config = $this->config;
- if (!array_key_exists('handler', $config)
-  || $config['handler'] === null) {
- $config['handler'] = HandlerStack::create();
- }
+
+ $config['handler'] = isset($config['handler'])
+ ? clone $config['handler']
+  : HandlerStack::create();
 
  // Add a logging middleware required by this library.
  $config['handler']->before(

diff --git a/tests/Google/AdsApi/Common/AdsGuzzleHttpClientFactoryTest.php b/tests/Google/AdsApi/Common/AdsGuzzleHttpClientFactoryTest.php
@@ -70,6 +70,7 @@ public function testGenerateHttpClient_userProvidedStack()
  );
  $stack->before('http_errors', Middleware::tap());
  $client = new Client(['handler' => $stack]);
+ $originalStack = clone $stack;
 
  $httpClientFactory = new AdsGuzzleHttpClientFactory(
  $logger,
@@ -80,7 +81,12 @@ public function testGenerateHttpClient_userProvidedStack()
 
  $this->assertNotNull($httpClient);
  $this->assertInstanceOf(Client::class, $httpClient);
- $this->assertEquals($stack, $httpClient->getConfig()['handler']);
+ $this->assertEquals($originalStack, $stack, 'Stack of original HTTP client should stay unchanged');
+ $this->assertNotEquals(
+ $stack,
+ $httpClient->getConfig()['handler'],
+ 'Stack of factory created HTTP client should have logging middleware, thus differ from original'
+ );
  }
 
  /**
@@ -97,10 +103,11 @@ public function testGenerateHttpClient_userProvidedConfigs()
  GuzzleLogMessageHandler::log($logger, $guzzleLogMessageFormatter)
  );
  $client = new Client([
- 'handler' => $stack,
- 'verify' => true,
- 'cookies' => false
+  'handler' => $stack,
+  'verify' => true,
+  'cookies' => false
  ]);
+ $originalStack = clone $stack;
 
  $httpClientFactory = new AdsGuzzleHttpClientFactory(
  $logger,
@@ -111,7 +118,12 @@ public function testGenerateHttpClient_userProvidedConfigs()
 
  $this->assertNotNull($httpClient);
  $this->assertInstanceOf(Client::class, $httpClient);
- $this->assertEquals($stack, $httpClient->getConfig()['handler']);
+ $this->assertEquals($originalStack, $stack, 'Stack of original HTTP client should stay unchanged');
+ $this->assertNotEquals(
+ $stack,
+ $httpClient->getConfig()['handler'],
+ 'Stack of factory created HTTP Client should have logging middleware, thus differ from original'
+ );
  $this->assertTrue($httpClient->getConfig()['verify']);
  $this->assertFalse($httpClient->getConfig()['cookies']);
  }