Merge pull request #1 from dldl/logger-improvements

Logger improvements

AbstractAdapter.php

@@ -17,75 +17,49 @@ abstract class AbstractAdapter implements AdapterInterface
     /**
      * @var null|CacheHelperInterface
      */
-    protected $cache;
+    private $cache;
 
     /**
      * @var ParameterBagInterface
      */
-    protected $parameters;
+    private $parameters;
 
     /**
      * @var LoggerHelper
      */
-    protected $log;
+    private $log;
 
-    public function __construct(LoggerInterface $logger)
+    public function __construct()
     {
-        $this->cache = null;
         $this->parameters = new ParameterBag();
-        $this->log = new LoggerHelper($logger);
     }
 
     abstract protected function handleRequest(Request $request);
 
-    public function getParameters()
-    {
-        return $this->parameters;
-    }
-
-    public function setCache(CacheHelperInterface $cache = null)
-    {
-        $this->cache = $cache;
-    }
-
-    public function hasCache()
-    {
-        return null !== $this->cache;
-    }
-
-    public function getCache()
-    {
-        return $this->cache;
-    }
-
     public function sendRequest(Request $request)
     {
         if (!$this->isConnected() || $this->getHost() === null) {
-            throw new ConnectionException(
-                (new \ReflectionClass($this))->getShortName().' must be connected before sending data.'
-            );
+            throw new ConnectionException($this->currentAdapter().' must be connected before sending data.');
         }
 
         if (!$this->supportsMethod($request->getMethod())) {
-            throw new RequestException(
-                'Method '.$request->getMethod().' is not supported by '.(new \ReflectionClass($this))->getShortName().'.'
-            );
+            throw new RequestException('Method '.$request->getMethod().' is not supported by '.$this->currentAdapter().'.');
         }
 
-        if ($this->hasCache() && $this->cache->getPool()->hasItem($request)) {
+        if ($this->hasCache() && $this->getCache()->getPool()->hasItem($request)) {
             return $this->getFromCache($request);
         }
 
         try {
-            $this->log->request($this->getHost(), $request, (new \ReflectionClass($this))->getShortName());
+            $this->log->request($this, $request);
             $response = $this->handleRequest($request);
-            $this->log->response($this->getHost(), $response, $request);
+            $this->log->response($this, $response, $request);
 
             if ($this->hasCache()) {
                 $this->saveToCache($request, $response);
             }
         } catch (WebServiceException $e) {
-            $this->log->requestFailure($this->getHost(), $request, (new \ReflectionClass($this))->getShortName(), $e->getMessage());
+            $this->log->requestFailure($this, $request, $e->getMessage());
 
             throw new WebServiceException('Unable to contact WebService : '.$e->getMessage());
         }
@@ -95,27 +69,57 @@ public function sendRequest(Request $request)
 
     private function getFromCache(Request $request)
     {
-        $this->log->cacheGet($this->getHost(), $request, (new \ReflectionClass($this->cache))->getShortName());
+        $this->log->cacheGet($this->getHost(), $request, (new \ReflectionClass($this->getCache()))->getShortName());
 
-        $response = $this->cache->getPool()->getItem($request);
+        $response = $this->getCache()->getPool()->getItem($request);
 
-        $this->log->response($this->getHost(), $response, $request);
+        $this->log->response($this, $response, $request);
 
         return $response;
     }
 
     private function saveToCache(Request $request, $response)
     {
-        $duration = $this->cache->getConfig()->get($request, CacheHelperInterface::DEFAULT_DURATION);
+        $duration = $this->getCache()->getConfig()->get($request, CacheHelperInterface::DEFAULT_DURATION);
 
-        $item = $this->cache->getPool()
+        $item = $this->getCache()->getPool()
             ->getItem($request)
             ->set($response)
             ->expiresAfter($duration)
         ;
 
-        $this->cache->getPool()->save($item);
+        $this->getCache()->getPool()->save($item);
+
+        $this->log->cacheAdd($this, $request, (new \ReflectionClass($this->getCache()))->getShortName(), $duration);
+    }
+
+    private function currentAdapter()
+    {
+        return (new \ReflectionClass($this))->getShortName();
+    }
+
+    public function getParameters()
+    {
+        return $this->parameters;
+    }
+
+    public function setCache(CacheHelperInterface $cache = null)
+    {
+        $this->cache = $cache;
+    }
 
-        $this->log->cacheAdd($this->getHost(), $request, (new \ReflectionClass($this->cache))->getShortName(), $duration);
+    public function hasCache()
+    {
+        return null !== $this->cache;
+    }
+
+    public function getCache()
+    {
+        return $this->cache;
+    }
+
+    public function setLogger(LoggerInterface $logger)
+    {
+        $this->log = new LoggerHelper($logger);
     }
 }

Adapter/LoggerHelper.php

@@ -2,6 +2,7 @@
 
 namespace dLdL\WebService\Adapter;
 
+use dLdL\WebService\AdapterInterface;
 use dLdL\WebService\Http\Request;
 use Psr\Log\LoggerInterface;
 
@@ -14,56 +15,65 @@ public function __construct(LoggerInterface $logger)
         $this->logger = $logger;
     }
 
-    public function request($host, Request $request, $class)
+    public function request(AdapterInterface $adapter, Request $request)
     {
         $this->logger->info(
-            sprintf('Sending request to %s using adapter %s.', $host.'/'.$request->getUrl(), $class),
+            sprintf('Sending request to %s%s using %s.',
+                $adapter->getHost(), $request->getUrl(), $this->className($adapter)
+            ),
             $request->getParameters()
         );
     }
 
-    public function response($host, $response, Request $request)
+    public function response(AdapterInterface $adapter, $response, Request $request)
     {
         $this->logger->debug(
-            sprintf('Response trace for request to %s.', $host.'/'.$request->getUrl()),
+            sprintf('Response trace for %s%s.', $adapter->getHost(), $request->getUrl()),
             [$response]
         );
     }
 
-    public function cacheGet($host, Request $request, $class)
+    public function cacheGet($host, Request $request, $cacheClass)
     {
         $this->logger->info(
-            sprintf('Retrieving data for url %s from cache %s.', $host.'/'.$request->getUrl(), $class),
+            sprintf('Retrieving data for %s%s from cache %s.', $host, $request->getUrl(), $cacheClass),
             $request->getParameters()
         );
     }
 
-    public function cacheAdd($host, Request $request, $class, $time)
+    public function cacheAdd($host, Request $request, $cacheClass, $time)
     {
         $this->logger->info(
-            sprintf('Adding data to cache %s for url %s (will expire in %s seconds).', $class, $host.'/'.$request->getUrl(), $time),
+            sprintf('Adding response for %s%s to cache %s (will expire in %s seconds).',
+                $host, $request->getUrl(), $cacheClass, $time
+            ),
             $request->getParameters()
         );
     }
 
-    public function connectionFailure($host, Request $request, $class)
+    public function connectionFailure(AdapterInterface $adapter, Request $request)
     {
         $this->logger->error(
-            sprintf('Failed to connect to %s using the adapter %s.', $host.'/'.$request->getUrl(), $class),
+            sprintf('Failed to connect to %s%s using %s.',
+                $adapter->getHost(), $request->getUrl(), $this->className($adapter)
+            ),
             $request->getParameters()
         );
     }
 
-    public function requestFailure($host, Request $request, $class, $exceptionMessage)
+    public function requestFailure(AdapterInterface $adapter, Request $request, $exceptionMessage)
     {
         $this->logger->error(
             sprintf(
-                'Failed to send request to %s using the adapter %s. Exception message : %s',
-                $host.'/'.$request->getUrl(),
-                $class,
-                $exceptionMessage
+                'Failed to send request to %s%s using %s. Exception message : %s',
+                $adapter->getHost(), $request->getUrl(), $this->className($adapter), $exceptionMessage
             ),
             $request->getParameters()
         );
     }
+
+    private function className(AdapterInterface $adapter)
+    {
+        return (new \ReflectionClass($adapter))->getShortName();
+    }
 }

Adapter/ParameterBagInterface.php

@@ -31,8 +31,8 @@ public function get($name, $defaultValue = null);
     /**
      * Add a value to the bag.
      *
-     * @param string $name Key of the added value
-     * @param mixed $value Value added to the bag
+     * @param string $name  Key of the added value
+     * @param mixed  $value Value added to the bag
      */
     public function add($name, $value);
 

AdapterInterface.php

@@ -28,6 +28,8 @@ public function connect($host);
      * @throws ConnectionException if adapter is not connected
      *
      * @see AdapterInterface::isConnected() to check if adapter is connected
+     *
+     * @return string The current hostname
      */
     public function getHost();
 

ParserInterface.php

@@ -11,7 +11,8 @@ interface ParserInterface
      * This method can be used to parse a raw response from an adapter and convert
      * it to a business object.
      *
-     * @param mixed $response Response to parse.
+     * @param mixed $response Response to parse
+     *
      * @return mixed
      */
     public function parse($response);

Tests/AbstractAdapterTest.php

@@ -11,7 +11,9 @@ class AbstractAdapterTest extends AbstractTestCase
 {
     protected function getTestedClass()
     {
-        return $this->getMockForAbstractClass(AbstractAdapter::class, [$this->createMock(LoggerInterface::class)]);
+        $abstractAdapter = $this->getMockForAbstractClass(AbstractAdapter::class);
+
+        return $abstractAdapter;
     }
 
     public function testInitialization()
@@ -63,6 +65,7 @@ public function testUnsupportedRequestMethod()
     public function testRequest()
     {
         $abstractAdapter = $this->getTestedClass();
+        $abstractAdapter->setLogger($this->createMock(LoggerInterface::class));
 
         $abstractAdapter->expects($this->exactly(3))
             ->method('getHost')

Tests/AbstractTestCase.php

@@ -15,6 +15,4 @@ protected function getFakeGetRequest()
     {
         return new Request('/fake/get/url', 'GET', ['test' => true]);
     }
-
-    protected abstract function getTestedClass();
 }