diff --git a/composer.json b/composer.json index 52d3859a2..32cfb9372 100644 --- a/composer.json +++ b/composer.json @@ -7,7 +7,7 @@ "license": "BSD-3-Clause", "require": { "php": "^8.1", - "google/auth": "dev-http-logging as 1.3", + "google/auth": "^1.45", "google/grpc-gcp": "^0.4", "grpc/grpc": "^1.13", "google/protobuf": "^v3.25.3||^4.26.1", diff --git a/src/BidiStream.php b/src/BidiStream.php index 49867025a..e4d6718b2 100644 --- a/src/BidiStream.php +++ b/src/BidiStream.php @@ -172,12 +172,12 @@ public function read() if ($this->logger) { $responseEvent = new RpcLogEvent(); - $responseEvent->headers = (is_null($result)) ? $this->call->getMetadata() : null; - $responseEvent->status = (is_null($result)) ? $status->code : null; + $responseEvent->headers = $this->call->getMetadata(); + $responseEvent->status = $status->code ?? null; $responseEvent->processId = (int) getmypid(); $responseEvent->requestId = crc32((string) spl_object_id($this) . getmypid()); - if ($result && $result instanceof Message) { + if ($result instanceof Message) { $responseEvent->payload = $result->serializeToJsonString(); } diff --git a/src/ClientOptionsTrait.php b/src/ClientOptionsTrait.php index dfba2bada..62ea0ec36 100644 --- a/src/ClientOptionsTrait.php +++ b/src/ClientOptionsTrait.php @@ -129,11 +129,15 @@ private function buildClientOptions(array $options) $options += $defaultOptions; // If logger is explicitly set to false, logging is disabled - if ($options['logger'] !== false) { - $options['logger'] = $options['logger'] ?? ApplicationDefaultCredentials::getDefaultLogger(); + if (is_null($options['logger'])) { + $options['logger'] = ApplicationDefaultCredentials::getDefaultLogger(); } - if ($options['logger'] !== null && $options['logger'] !== false && !$options['logger'] instanceof LoggerInterface) { + if ( + $options['logger'] !== null + && $options['logger'] !== false + && !$options['logger'] instanceof LoggerInterface + ) { throw new ValidationException( 'The "logger" option in the options array should be PSR-3 LoggerInterface compatible' ); @@ -143,7 +147,9 @@ private function buildClientOptions(array $options) $this->logConfiguration($options['logger'], $clientSuppliedOptions); if (isset($options['logger'])) { - $options['credentialsConfig']['authHttpHandler'] = HttpHandlerFactory::build(logger: $options['logger']); + $options['credentialsConfig']['authHttpHandler'] = HttpHandlerFactory::build( + logger: $options['logger'] + ); } $options['credentialsConfig'] += $defaultOptions['credentialsConfig']; diff --git a/src/ClientStream.php b/src/ClientStream.php index 5fa5236a5..6c249551c 100644 --- a/src/ClientStream.php +++ b/src/ClientStream.php @@ -71,6 +71,7 @@ public function __construct(// @phpstan-ignore-line */ public function write($request) { + // In some cases, $request can be a string if ($this->logger && $request instanceof Message) { $requestEvent = new RpcLogEvent(); @@ -102,7 +103,7 @@ public function readResponse() $responseEvent->processId = (int) getmypid(); $responseEvent->requestId = crc32((string) spl_object_id($this) . getmypid()); - if ($response && $response instanceof Message) { + if ($response instanceof Message) { $response->serializeToJsonString(); } diff --git a/src/Transport/GrpcFallbackTransport.php b/src/Transport/GrpcFallbackTransport.php index 7b2e417be..b248b052e 100644 --- a/src/Transport/GrpcFallbackTransport.php +++ b/src/Transport/GrpcFallbackTransport.php @@ -87,9 +87,10 @@ public static function build(string $apiEndpoint, array $config = []) $config += [ 'httpHandler' => null, 'clientCertSource' => null, + 'logger' => null, ]; list($baseUri, $port) = self::normalizeServiceAddress($apiEndpoint); - $httpHandler = $config['httpHandler'] ?: self::buildHttpHandlerAsync(logger: $config['logger'] ?? null); + $httpHandler = $config['httpHandler'] ?: self::buildHttpHandlerAsync(logger: $config['logger']); $transport = new GrpcFallbackTransport("$baseUri:$port", $httpHandler); if ($config['clientCertSource']) { $transport->configureMtlsChannel($config['clientCertSource']); diff --git a/src/Transport/GrpcTransport.php b/src/Transport/GrpcTransport.php index c5ab936be..ebcbc07b2 100644 --- a/src/Transport/GrpcTransport.php +++ b/src/Transport/GrpcTransport.php @@ -135,6 +135,7 @@ public static function build(string $apiEndpoint, array $config = []) 'channel' => null, 'interceptors' => [], 'clientCertSource' => null, + 'logger' => null, ]; list($addr, $port) = self::normalizeServiceAddress($apiEndpoint); $host = "$addr:$port"; @@ -157,10 +158,10 @@ public static function build(string $apiEndpoint, array $config = []) ); } try { - if (isset($config['logger']) && $config['logger'] === false) { + if ($config['logger'] === false) { $config['logger'] = null; } - return new GrpcTransport($host, $stubOpts, $channel, $config['interceptors'], $config['logger'] ?? null); + return new GrpcTransport($host, $stubOpts, $channel, $config['interceptors'], $config['logger']); } catch (Exception $ex) { throw new ValidationException( 'Failed to build GrpcTransport: ' . $ex->getMessage(), diff --git a/src/Transport/HttpUnaryTransportTrait.php b/src/Transport/HttpUnaryTransportTrait.php index d2cccd11e..38c4ca1f5 100644 --- a/src/Transport/HttpUnaryTransportTrait.php +++ b/src/Transport/HttpUnaryTransportTrait.php @@ -130,7 +130,7 @@ private static function buildCommonHeaders(array $options) private static function buildHttpHandlerAsync(null|false|LoggerInterface $logger = null) { try { - return [HttpHandlerFactory::build(null, $logger), 'async']; + return [HttpHandlerFactory::build(logger: $logger), 'async']; } catch (Exception $ex) { throw new ValidationException('Failed to build HttpHandler', $ex->getCode(), $ex); } diff --git a/src/Transport/RestTransport.php b/src/Transport/RestTransport.php index 871fbdb41..296937fa4 100644 --- a/src/Transport/RestTransport.php +++ b/src/Transport/RestTransport.php @@ -95,12 +95,13 @@ public static function build(string $apiEndpoint, string $restConfigPath, array 'httpHandler' => null, 'clientCertSource' => null, 'hasEmulator' => false, + 'logger' => null, ]; list($baseUri, $port) = self::normalizeServiceAddress($apiEndpoint); $requestBuilder = $config['hasEmulator'] ? new InsecureRequestBuilder("$baseUri:$port", $restConfigPath) : new RequestBuilder("$baseUri:$port", $restConfigPath); - $httpHandler = $config['httpHandler'] ?: self::buildHttpHandlerAsync($config['logger'] ?? null); + $httpHandler = $config['httpHandler'] ?: self::buildHttpHandlerAsync($config['logger']); $transport = new RestTransport($requestBuilder, $httpHandler); if ($config['clientCertSource']) { $transport->configureMtlsChannel($config['clientCertSource']); diff --git a/tests/Unit/Middleware/RetryMiddlewareTest.php b/tests/Unit/Middleware/RetryMiddlewareTest.php index a59b74eef..256f91fa9 100644 --- a/tests/Unit/Middleware/RetryMiddlewareTest.php +++ b/tests/Unit/Middleware/RetryMiddlewareTest.php @@ -387,9 +387,7 @@ public function testMaxRetries() public function testRetriesAreIncludedInTheOptionsArray() { - $call = $this->getMockBuilder(Call::class) - ->disableOriginalConstructor() - ->getMock(); + $call = $this->prophesize(Call::class)->reveal(); $maxRetries = 1; $reportedRetries = 0; @@ -528,31 +526,4 @@ public function testUnlimitedMaxRetries() $middleware($call->reveal(), [])->wait(); } -} - -class MockHandler implements MiddlewareInterface -{ - public int $calls; - public bool $containsRetries = false; - - public function __construct() - { - $this->calls = 0; - } - - public function __invoke(Call $call, array $options) - { - $promise = new Promise(); - - if ($this->calls === 0) { - $this->calls += 1; - return $promise->reject(); - } - - if (array_key_exists('retryAttempt') && $options['retryAttempt'] === 1) { - $this->containsRetries = true; - } - - return $promise->resolve(); - } } \ No newline at end of file diff --git a/tests/Unit/Transport/GrpcTransportTest.php b/tests/Unit/Transport/GrpcTransportTest.php index 829334d85..580ba9a76 100644 --- a/tests/Unit/Transport/GrpcTransportTest.php +++ b/tests/Unit/Transport/GrpcTransportTest.php @@ -439,17 +439,6 @@ public function testClientCertSourceOptionInvalid() ); } - // public function testLoggerGetsCalledIfLoggerSupplied() - // { - // // $logger = $this->prophesize(StdOutLogger::class); - // // $logger->debug(Argument::cetera()) - // // ->shouldBeCalledTimes(2); - // // $logger->info(Argument::cetera()) - // // ->shouldBeCalledTimes(1); - // // $logger->log(Argument::cetera()) - // // ->shouldBeCalled(); - // } - /** * @dataProvider buildDataGrpc */