From 016dad1b82ab3b5857cf09350b995eb8fb3bc6ce Mon Sep 17 00:00:00 2001 From: Hector Mendoza Jacobo Date: Mon, 11 Nov 2024 21:45:50 +0000 Subject: [PATCH] Fix style and improved readability --- src/ApplicationDefaultCredentials.php | 15 ++++--------- src/HttpHandler/Guzzle6HttpHandler.php | 2 +- src/HttpHandler/HttpHandlerFactory.php | 8 +++---- src/Logging/LoggingTrait.php | 17 +++++++++----- src/Logging/StdOutLogger.php | 6 ++--- tests/ApplicationDefaultCredentialsTest.php | 4 ---- tests/Logging/StdOutLoggerTest.php | 25 +++++---------------- 7 files changed, 27 insertions(+), 50 deletions(-) diff --git a/src/ApplicationDefaultCredentials.php b/src/ApplicationDefaultCredentials.php index 5474effc5..d08d524e4 100644 --- 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 index 644d94b08..80a79ef86 100644 --- 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 index d08e73192..49d72e263 100644 --- 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 index f0198c35a..7d4460eeb 100644 --- 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 $headers - * @return null|array + * @return null|array */ private function getJwtToken(array $headers): null|array { diff --git a/src/Logging/StdOutLogger.php b/src/Logging/StdOutLogger.php index 01aabaf2e..db4185a18 100644 --- a/src/Logging/StdOutLogger.php +++ b/src/Logging/StdOutLogger.php @@ -52,7 +52,7 @@ class StdOutLogger implements LoggerInterface */ public function __construct(string $level = LogLevel::DEBUG) { - $this->level = $this->getLevelMap($level); + $this->level = $this->getLevelFromName($level); } /** @@ -60,7 +60,7 @@ public function __construct(string $level = LogLevel::DEBUG) */ 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 index 38fae0640..a2ac57cb6 100644 --- 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 index f5f603313..a561adb28 100644 --- 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(); } }