Merge pull request #641 from mrpavlikov/i640-logger-duplicates-fix

Prevent provided HTTP Client handler stack modification

Author: fiboknacky

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(

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']);
     }