Fix style and improved readability

googleapis · Nov 11, 2024 · 016dad1 · 016dad1
1 parent a57a184
commit 016dad1
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 50 deletions.
diff --git a/src/ApplicationDefaultCredentials.php b/src/ApplicationDefaultCredentials.php
@@ -29,7 +29,6 @@
 use Google\Auth\Subscriber\AuthTokenSubscriber;
 use GuzzleHttp\Client;
 use InvalidArgumentException;
-use PHPUnit\TextUI\XmlConfiguration\Logging\Logging;
 use Psr\Cache\CacheItemPoolInterface;
 use Psr\Log\LoggerInterface;
 
@@ -338,22 +337,16 @@ public static function getDefaultLogger(): null|LoggerInterface
         $loggingFlag = getenv(self::SDK_DEBUG_FLAG);
 
         // Env var is not set
-        if (!is_string($loggingFlag)) {
-            if (is_array($loggingFlag)) {
-                trigger_error('The ' . self::SDK_DEBUG_FLAG . ' is set, but it is set to another value than false, true, 0 or 1. Logging is disabled');
-                return null;
-            }
-
+        if (!$loggingFlag) {
             return null;
         }
 
         $loggingFlag = strtolower($loggingFlag);
 
         // Env Var is not true
-        if ($loggingFlag !== 'true' && $loggingFlag !== '1') {
-            // Env var is set to a non valid value
-            if ($loggingFlag !== 'false' && $loggingFlag !== '0') {
-                trigger_error('The ' . self::SDK_DEBUG_FLAG . ' is set, but it is set to another value than false, true, 0 or 1. Logging is disabled');
+        if ($loggingFlag !== 'true') {
+            if ($loggingFlag !== 'false') {
+                trigger_error('The ' . self::SDK_DEBUG_FLAG . ' is set, but it is set to another value than false or true. Logging is disabled');
             }
 
             return null;

diff --git a/src/HttpHandler/Guzzle6HttpHandler.php b/src/HttpHandler/Guzzle6HttpHandler.php
@@ -106,7 +106,7 @@ public function async(RequestInterface $request, array $options = [])
             $requestEvent = new LogEvent();
 
             $requestEvent->method = $request->getMethod();
-            $requestEvent->url = $request->getUri()->__toString();
+            $requestEvent->url = (string) $request->getUri();
             $requestEvent->headers = $request->getHeaders();
             $requestEvent->payload = $request->getBody()->getContents();
             $requestEvent->retryAttempt = $options['retryAttempt'] ?? null;

diff --git a/src/HttpHandler/HttpHandlerFactory.php b/src/HttpHandler/HttpHandlerFactory.php
@@ -51,11 +51,9 @@ public static function build(
             $client = new Client(['handler' => $stack]);
         }
 
-        if ($logger === false) {
-            $logger = null;
-        } else {
-            $logger = $logger ?? ApplicationDefaultCredentials::getDefaultLogger();
-        }
+        $logger = ($logger === false)
+            ? null
+            : $logger ?? ApplicationDefaultCredentials::getDefaultLogger();
 
         $version = null;
         if (defined('GuzzleHttp\ClientInterface::MAJOR_VERSION')) {

diff --git a/src/Logging/LoggingTrait.php b/src/Logging/LoggingTrait.php
@@ -84,12 +84,10 @@ private function logResponse(LogEvent $event): void
         $stringifiedEvent = json_encode($debugEvent);
 
         // There was an error stringifying the event, return to not break execution
-        if ($stringifiedEvent === false) {
-            return;
+        if ($stringifiedEvent !== false) {
+            $this->logger->debug($stringifiedEvent);
         }
 
-        $this->logger->debug($stringifiedEvent);
-
         if ($event->status) {
             $infoEvent = [
                 'timestamp' => $event->timestamp,
@@ -136,12 +134,19 @@ private function logStatus(LogEvent $event): void
             fn ($value) => !is_null($value)
         );
 
-        $this->logger->info((string) json_encode($infoEvent));
+        $stringifiedEvent = json_encode($infoEvent);
+
+        // There was an error stringifying the event, return to not break execution
+        if ($stringifiedEvent === false) {
+            return;
+        }
+
+        $this->logger->info($stringifiedEvent);
     }
 
     /**
      * @param array<mixed> $headers
-     * @return null|array<mixed, mixed>
+     * @return null|array<string, string|false>
      */
     private function getJwtToken(array $headers): null|array
     {

diff --git a/src/Logging/StdOutLogger.php b/src/Logging/StdOutLogger.php
@@ -52,15 +52,15 @@ class StdOutLogger implements LoggerInterface
      */
     public function __construct(string $level = LogLevel::DEBUG)
     {
-        $this->level = $this->getLevelMap($level);
+        $this->level = $this->getLevelFromName($level);
     }
 
     /**
      * {@inheritdoc}
      */
     public function log($level, string|Stringable $message, array $context = []): void
     {
-        if ($this->getLevelMap($level) < $this->level) {
+        if ($this->getLevelFromName($level) < $this->level) {
             return;
         }
 
@@ -72,7 +72,7 @@ public function log($level, string|Stringable $message, array $context = []): vo
      * @return int
      * @throws InvalidArgumentException
      */
-    private function getLevelMap(string $levelName): int
+    private function getLevelFromName(string $levelName): int
     {
         if (!array_key_exists($levelName, $this->levelMapping)) {
             throw new InvalidArgumentException('The level supplied to the Logger is not valid');

diff --git a/tests/ApplicationDefaultCredentialsTest.php b/tests/ApplicationDefaultCredentialsTest.php
@@ -783,10 +783,6 @@ public function testGetDefaultLoggerReturnStdOutLoggerIfEnvVarIsPresent()
         putenv($this::SDK_DEBUG_FLAG . '=true');
         $logger = ApplicationDefaultCredentials::getDefaultLogger();
         $this->assertTrue($logger instanceof StdOutLogger);
-
-        putenv($this::SDK_DEBUG_FLAG . '=1');
-        $logger = ApplicationDefaultCredentials::getDefaultLogger();
-        $this->assertTrue($logger instanceof StdOutLogger);
     }
 
     public function testGetDefaultLoggerReturnsNullIfNotEnvVar()

diff --git a/tests/Logging/StdOutLoggerTest.php b/tests/Logging/StdOutLoggerTest.php
@@ -32,43 +32,28 @@ public function testConstructsWithAnIncorrectLevelThrowsException()
 
     public function testLoggingOnSameLevelWritesToStdOut()
     {
-        ob_start();
+        $expectedString = 'test';
+        $this->expectOutputString($expectedString . "\n");
 
         $logger = new StdOutLogger(LogLevel::DEBUG);
-        $expectedString = 'test';
         $logger->debug($expectedString);
-        $buffer = ob_get_contents();
-
-        $this->assertEquals($expectedString . "\n", $buffer);
-
-        ob_end_clean();
     }
 
     public function testLoggingOnHigherLeverWritesToStdOut()
     {
-        ob_start();
+        $expectedString = 'test';
+        $this->expectOutputString($expectedString. "\n");
 
         $logger = new StdOutLogger(LogLevel::WARNING);
-        $expectedString = 'test';
         $logger->error($expectedString);
-        $buffer = ob_get_contents();
-
-        $this->assertEquals($expectedString . "\n", $buffer);
-
-        ob_end_clean();
     }
 
     public function testLoggingOnLowerLeverDoesNotWriteToStdOut()
     {
-        ob_start();
+        $this->expectOutputString('');
 
         $logger = new StdOutLogger(LogLevel::WARNING);
         $expectedString = 'test';
         $logger->debug($expectedString);
-        $buffer = ob_get_contents();
-
-        $this->assertEmpty($buffer);
-
-        ob_end_clean();
     }
 }