Fix meaning of exception only (#12)

* Fixed the constructor argument of * Updated changelog * Updated readme file
gmponos · Oct 1, 2018 · 074be30 · 074be30
1 parent d9031ef
commit 074be30
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,14 @@
 
 All Notable changes to `gmponos/guzzle_logger` will be documented in this file
 
+## 0.5.0 - 2018-10-01
+
+### Changed
+- **BREAKING CHANGE** Renamed the variable `$logRequestOnExceptionOnly` to `$onExceptionOnly`. The purpose of this constructor argument was 
+to log request and responses only if an exceptgition occurs. If you were manually setting this argument as true now you must set it
+as false as the variables meaning is inverted.
+- Deprecated the option `requests`. It will be removed on my next version.
+
 ## 0.4.0 - 2018-09-12
 
 ### Changed

diff --git a/README.md b/README.md
@@ -55,7 +55,7 @@ The signature of the LoggerMiddleware class is the following:
 ``LoggerMiddleware(LoggerInterface $logger, $logRequests = true, $logStatistics = false, array $thresholds = [])``
 
 - **logger** - The PSR-3 logger to use for logging.
-- **logRequests** - By default the middleware is set to log every request and response. If you wish that to log only the requests and responses that you retrieve a status code above 4xx set this as false.
+- **$onExceptionOnly** - By default the middleware is set to log every request and response. If you wish that to log only the requests and responses that you retrieve a status code above 4xx set this as true.
 - **logStatistics** - If you set logStatistics as true and this as true then guzzle will also log statistics about the requests.
 - **thresholds** - An array that you may use to change the thresholds of logging the responses. 
 
@@ -66,7 +66,7 @@ You can set on each request options about your log.
 ```php
 $client->get("/", [
     'log' => [
-        'requests' => true,
+        'on_exception_only' => true,
         'statistics' => true,
         'error_threshold' => null,
         'warning_threshold' => null,
@@ -81,8 +81,8 @@ $client->get("/", [
 ```
 
 - ``sensitive`` if you set this to true then the body of request/response will not be logged as it will be considered that it contains sensitive information.
-- ``requests`` Do not log anything unless if the request is above the threshold or inside the levels.
-- ``statistics`` if the requests variable is true and this is also true the logger will log statistics about the request
+- ``on_exception_only`` Do not log anything unless if the response status code is above the threshold.
+- ``statistics`` if the `on_exception_only` option/variable is true and this is also true the middleware will log statistics about the HTTP call.
 - ``levels`` set custom log levels for each response status code
 
 ## Change log

diff --git a/src/Middleware/LoggerMiddleware.php b/src/Middleware/LoggerMiddleware.php
@@ -19,7 +19,7 @@ class LoggerMiddleware
     /**
      * @var bool Whether or not to log requests as they are made.
      */
-    private $logRequestOnExceptionOnly;
+    private $onExceptionOnly;
 
     /**
      * @var bool
@@ -50,18 +50,18 @@ class LoggerMiddleware
      * Creates a callable middleware for logging requests and responses.
      *
      * @param LoggerInterface $logger
-     * @param bool $logRequestOnExceptionOnly
-     * @param bool $logStatistics
+     * @param bool $onExceptionOnly The request and the response will be logged only in cases there is an exception or if they status code exceeds the thresholds.
+     * @param bool $logStatistics If this is true an extra row will be added that will contain some HTTP statistics.
      * @param array $thresholds
      */
     public function __construct(
         LoggerInterface $logger,
-        $logRequestOnExceptionOnly = true,
+        $onExceptionOnly = false,
         $logStatistics = false,
         array $thresholds = []
     ) {
         $this->logger = $logger;
-        $this->logRequestOnExceptionOnly = $logRequestOnExceptionOnly;
+        $this->onExceptionOnly = $onExceptionOnly;
         $this->logStatistics = $logStatistics;
         $this->thresholds = array_merge([
             'error' => 499,
@@ -80,7 +80,7 @@ public function __invoke(callable $handler)
         return function (RequestInterface $request, array $options) use ($handler) {
             $this->setOptions($options);
 
-            if ($this->logRequestOnExceptionOnly === true) {
+            if ($this->onExceptionOnly === false) {
                 $this->logRequest($request);
                 if ($this->logStatistics && !isset($options['on_stats'])) {
                     $options['on_stats'] = $this->logStatistics();
@@ -183,7 +183,7 @@ private function logResponse(ResponseInterface $response)
     private function handleSuccess(RequestInterface $request)
     {
         return function (ResponseInterface $response) use ($request) {
-            if ($this->logRequestOnExceptionOnly === true) {
+            if ($this->onExceptionOnly === false) {
                 $this->logResponse($response);
                 return $response;
             }
@@ -205,7 +205,7 @@ private function handleSuccess(RequestInterface $request)
     private function handleFailure(RequestInterface $request)
     {
         return function (\Exception $reason) use ($request) {
-            if ($this->logRequestOnExceptionOnly === false) {
+            if ($this->onExceptionOnly === true) {
                 $this->logRequest($request);
             }
 
@@ -314,20 +314,28 @@ private function setOptions(array $options)
             return;
         }
 
+        $options = $options['log'];
+        if (isset($options['requests'])) {
+            @trigger_error('Using option "requests" is deprecated and it will be removed on the next version. Use "on_exception_only"', E_USER_DEPRECATED);
+            if (!isset($options['on_exception_only'])) {
+                $options['on_exception_only'] = !$options['requests'];
+            }
+        }
+
         $defaults = [
-            'requests' => $this->logRequestOnExceptionOnly,
+            'on_exception_only' => $this->onExceptionOnly,
             'statistics' => $this->logStatistics,
             'warning_threshold' => 399,
             'error_threshold' => 499,
             'levels' => [],
             'sensitive' => false,
         ];
 
-        $options = array_merge($defaults, $options['log']);
+        $options = array_merge($defaults, $options);
         $this->logCodeLevel = $options['levels'];
         $this->thresholds['warning'] = $options['warning_threshold'];
         $this->thresholds['error'] = $options['error_threshold'];
-        $this->logRequestOnExceptionOnly = $options['requests'];
+        $this->onExceptionOnly = $options['on_exception_only'];
         $this->logStatistics = $options['statistics'];
         $this->sensitive = $options['sensitive'];
     }

diff --git a/tests/TestApp/HistoryLogger.php b/tests/TestApp/HistoryLogger.php
@@ -6,7 +6,6 @@
 
 class HistoryLogger implements LoggerInterface
 {
-
     /**
      * @var array
      */
@@ -16,7 +15,7 @@ class HistoryLogger implements LoggerInterface
      * System is unusable.
      *
      * @param string $message
-     * @param array  $context
+     * @param array $context
      * @return void
      */
     public function emergency($message, array $context = [])
@@ -35,7 +34,7 @@ public function emergency($message, array $context = [])
      * trigger the SMS alerts and wake you up.
      *
      * @param string $message
-     * @param array  $context
+     * @param array $context
      * @return void
      */
     public function alert($message, array $context = [])
@@ -53,7 +52,7 @@ public function alert($message, array $context = [])
      * Example: Application component unavailable, unexpected exception.
      *
      * @param string $message
-     * @param array  $context
+     * @param array $context
      * @return void
      */
     public function critical($message, array $context = [])
@@ -70,7 +69,7 @@ public function critical($message, array $context = [])
      * be logged and monitored.
      *
      * @param string $message
-     * @param array  $context
+     * @param array $context
      * @return void
      */
     public function error($message, array $context = [])
@@ -89,7 +88,7 @@ public function error($message, array $context = [])
      * that are not necessarily wrong.
      *
      * @param string $message
-     * @param array  $context
+     * @param array $context
      * @return void
      */
     public function warning($message, array $context = [])
@@ -105,7 +104,7 @@ public function warning($message, array $context = [])
      * Normal but significant events.
      *
      * @param string $message
-     * @param array  $context
+     * @param array $context
      * @return void
      */
     public function notice($message, array $context = [])
@@ -123,7 +122,7 @@ public function notice($message, array $context = [])
      * Example: User logs in, SQL logs.
      *
      * @param string $message
-     * @param array  $context
+     * @param array $context
      * @return void
      */
     public function info($message, array $context = [])
@@ -139,7 +138,7 @@ public function info($message, array $context = [])
      * Detailed debug information.
      *
      * @param string $message
-     * @param array  $context
+     * @param array $context
      * @return void
      */
     public function debug($message, array $context = [])
@@ -154,9 +153,9 @@ public function debug($message, array $context = [])
     /**
      * Logs with an arbitrary level.
      *
-     * @param mixed  $level
+     * @param mixed $level
      * @param string $message
-     * @param array  $context
+     * @param array $context
      * @return void
      */
     public function log($level, $message, array $context = [])
@@ -167,4 +166,12 @@ public function log($level, $message, array $context = [])
             'context' => $context,
         ];
     }
+
+    /**
+     * Cleans the history of the logger.
+     */
+    public function clean()
+    {
+        $this->history = [];
+    }
 }
diff --git a/tests/Unit/LoggerMiddlewareTest.php b/tests/Unit/LoggerMiddlewareTest.php
@@ -85,7 +85,7 @@ public function logSuccessfulTransaction()
     /**
      * @test
      */
-    public function doNotLogSuccessfulTransaction()
+    public function logOnlyResponseWhenLogRequestsIsSetToFalse()
     {
         $this->appendResponse(200)
             ->createClient([
@@ -96,6 +96,17 @@ public function doNotLogSuccessfulTransaction()
             ->get('/');
 
         $this->assertCount(0, $this->logger->history);
+
+        $this->logger->clean();
+
+        $this->appendResponse(200)->createClient()
+            ->get('/', [
+                'log' => [
+                    'requests' => false,
+                ],
+            ]);
+
+        $this->assertCount(0, $this->logger->history);
     }
 
     /**