From c8d8faef4e9cd9535dd6f0e2b2dbb1fc8c57bed2 Mon Sep 17 00:00:00 2001 From: hectorhammett Date: Tue, 2 Jun 2026 20:58:43 +0000 Subject: [PATCH 1/3] Modify the order for the appendMiddleware logic --- src/GapicClientTrait.php | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/GapicClientTrait.php b/src/GapicClientTrait.php index 7a527ee32..829108058 100644 --- a/src/GapicClientTrait.php +++ b/src/GapicClientTrait.php @@ -700,26 +700,28 @@ private function createCallStack(array $callConstructionOptions) $callStack = new TransportCallMiddleware($this->transport, $this->transportCallMethods); + $callStack = new CredentialsWrapperMiddleware($callStack, $this->credentialsWrapper); + + $callStack = new OptionsFilterMiddleware($callStack, [ + 'headers', + 'timeoutMillis', + 'transportOptions', + 'metadataCallback', + 'audience', + 'metadataReturnType' + ]); + foreach ($this->prependMiddlewareCallables as $fn) { /** @var MiddlewareInterface $callStack */ $callStack = $fn($callStack); } - $callStack = new CredentialsWrapperMiddleware($callStack, $this->credentialsWrapper); $callStack = new FixedHeaderMiddleware($callStack, $fixedHeaders, true); $callStack = new RetryMiddleware($callStack, $callConstructionOptions['retrySettings']); $callStack = new RequestAutoPopulationMiddleware( $callStack, $callConstructionOptions['autoPopulationSettings'], ); - $callStack = new OptionsFilterMiddleware($callStack, [ - 'headers', - 'timeoutMillis', - 'transportOptions', - 'metadataCallback', - 'audience', - 'metadataReturnType' - ]); foreach (\array_reverse($this->middlewareCallables) as $fn) { /** @var MiddlewareInterface $callStack */ From bdc8f278894ef72594493b47dba8cfdcfc8a07fc Mon Sep 17 00:00:00 2001 From: hectorhammett Date: Tue, 2 Jun 2026 23:17:02 +0000 Subject: [PATCH 2/3] Modify logic order for the OptionsFilterMiddleware --- src/GapicClientTrait.php | 6 +- tests/Unit/GapicClientTraitTest.php | 106 ++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 3 deletions(-) diff --git a/src/GapicClientTrait.php b/src/GapicClientTrait.php index 829108058..29b6e3f4e 100644 --- a/src/GapicClientTrait.php +++ b/src/GapicClientTrait.php @@ -700,15 +700,14 @@ private function createCallStack(array $callConstructionOptions) $callStack = new TransportCallMiddleware($this->transport, $this->transportCallMethods); - $callStack = new CredentialsWrapperMiddleware($callStack, $this->credentialsWrapper); - $callStack = new OptionsFilterMiddleware($callStack, [ 'headers', 'timeoutMillis', 'transportOptions', 'metadataCallback', 'audience', - 'metadataReturnType' + 'metadataReturnType', + 'credentialsWrapper' ]); foreach ($this->prependMiddlewareCallables as $fn) { @@ -716,6 +715,7 @@ private function createCallStack(array $callConstructionOptions) $callStack = $fn($callStack); } + $callStack = new CredentialsWrapperMiddleware($callStack, $this->credentialsWrapper); $callStack = new FixedHeaderMiddleware($callStack, $fixedHeaders, true); $callStack = new RetryMiddleware($callStack, $callConstructionOptions['retrySettings']); $callStack = new RequestAutoPopulationMiddleware( diff --git a/tests/Unit/GapicClientTraitTest.php b/tests/Unit/GapicClientTraitTest.php index 8f702c57c..267688674 100644 --- a/tests/Unit/GapicClientTraitTest.php +++ b/tests/Unit/GapicClientTraitTest.php @@ -135,6 +135,112 @@ public function testHeadersOverwriteBehavior() ); } + public function testPrependedMiddlewareCanSeeCallOptionAddedByAppendedMiddleware() + { + $unaryDescriptors = [ + 'callType' => Call::UNARY_CALL, + 'responseType' => 'decodeType' + ]; + $request = new MockRequestBody([]); + $transport = $this->prophesize(TransportInterface::class); + + $transport->startUnaryCall( + Argument::type(Call::class), + Argument::any() + ) + ->shouldBeCalledOnce() + ->willReturn($this->prophesize(PromiseInterface::class)->reveal()); + + $credentialsWrapper = CredentialsWrapper::build([ + 'keyFile' => __DIR__ . '/testdata/json-key-file.json' + ]); + + $client = new StubGapicClient(); + $client->set('agentHeader', []); + $client->set( + 'retrySettings', + ['method' => $this->prophesize(RetrySettings::class)->reveal()] + ); + $client->set('transport', $transport->reveal()); + $client->set('credentialsWrapper', $credentialsWrapper); + $client->set('descriptors', ['method' => $unaryDescriptors]); + + // 1. Append a middleware (outermost) that adds a custom option to callOptions + $client->addMiddleware(function (callable $handler) { + return function (Call $call, array $options) use ($handler) { + $options['myCustomOption'] = 'myCustomValue'; + return $handler($call, $options); + }; + }); + + // 2. Prepend a middleware (innermost) that confirms it can see the custom option + $prependedMiddlewareCalled = false; + $client->prependMiddleware(function (callable $handler) use (&$prependedMiddlewareCalled) { + return function (Call $call, array $options) use ($handler, &$prependedMiddlewareCalled) { + $prependedMiddlewareCalled = true; + $this->assertTrue(isset($options['myCustomOption'])); + $this->assertEquals('myCustomValue', $options['myCustomOption']); + return $handler($call, $options); + }; + }); + + $client->startApiCall( + 'method', + $request + ); + + $this->assertTrue($prependedMiddlewareCalled); + } + + public function testOptionsFilterMiddlewareAllowsCredentialsWrapperButStripsOtherCustomOptions() + { + $unaryDescriptors = [ + 'callType' => Call::UNARY_CALL, + 'responseType' => 'decodeType' + ]; + $request = new MockRequestBody([]); + $transport = $this->prophesize(TransportInterface::class); + + $customCredentialsWrapper = CredentialsWrapper::build([ + 'keyFile' => __DIR__ . '/testdata/json-key-file.json' + ]); + + $transport->startUnaryCall( + Argument::type(Call::class), + Argument::that(function ($options) use ($customCredentialsWrapper) { + $hasCredentialsWrapper = isset($options['credentialsWrapper']) + && $options['credentialsWrapper'] === $customCredentialsWrapper; + $doesNotHaveCustomOption = !isset($options['myCustomOption']); + return $hasCredentialsWrapper && $doesNotHaveCustomOption; + }) + ) + ->shouldBeCalledOnce() + ->willReturn($this->prophesize(PromiseInterface::class)->reveal()); + + $client = new StubGapicClient(); + $client->set('agentHeader', []); + $client->set( + 'retrySettings', + ['method' => $this->prophesize(RetrySettings::class)->reveal()] + ); + $client->set('transport', $transport->reveal()); + $client->set('credentialsWrapper', $customCredentialsWrapper); + $client->set('descriptors', ['method' => $unaryDescriptors]); + + $client->addMiddleware(function (callable $handler) use ($customCredentialsWrapper) { + return function (Call $call, array $options) use ($handler, $customCredentialsWrapper) { + $options['credentialsWrapper'] = $customCredentialsWrapper; + $options['myCustomOption'] = 'myCustomValue'; + return $handler($call, $options); + }; + }); + + $client->startApiCall( + 'method', + $request + ); + } + public function testVersionedHeadersOverwriteBehavior() { $unaryDescriptors = [ From d3c251c10871aae44ba4e366482b09841808a492 Mon Sep 17 00:00:00 2001 From: hectorhammett Date: Wed, 3 Jun 2026 19:15:28 +0000 Subject: [PATCH 3/3] Added a new option to the filterOptions middleware --- src/GapicClientTrait.php | 19 +++---- tests/Unit/GapicClientTraitTest.php | 85 +++++++---------------------- 2 files changed, 30 insertions(+), 74 deletions(-) diff --git a/src/GapicClientTrait.php b/src/GapicClientTrait.php index 29b6e3f4e..3bd91ece3 100644 --- a/src/GapicClientTrait.php +++ b/src/GapicClientTrait.php @@ -700,16 +700,6 @@ private function createCallStack(array $callConstructionOptions) $callStack = new TransportCallMiddleware($this->transport, $this->transportCallMethods); - $callStack = new OptionsFilterMiddleware($callStack, [ - 'headers', - 'timeoutMillis', - 'transportOptions', - 'metadataCallback', - 'audience', - 'metadataReturnType', - 'credentialsWrapper' - ]); - foreach ($this->prependMiddlewareCallables as $fn) { /** @var MiddlewareInterface $callStack */ $callStack = $fn($callStack); @@ -722,6 +712,15 @@ private function createCallStack(array $callConstructionOptions) $callStack, $callConstructionOptions['autoPopulationSettings'], ); + $callStack = new OptionsFilterMiddleware($callStack, [ + 'headers', + 'timeoutMillis', + 'transportOptions', + 'metadataCallback', + 'audience', + 'metadataReturnType', + 'middlewareOptions' + ]); foreach (\array_reverse($this->middlewareCallables) as $fn) { /** @var MiddlewareInterface $callStack */ diff --git a/tests/Unit/GapicClientTraitTest.php b/tests/Unit/GapicClientTraitTest.php index 267688674..1155f5741 100644 --- a/tests/Unit/GapicClientTraitTest.php +++ b/tests/Unit/GapicClientTraitTest.php @@ -135,7 +135,7 @@ public function testHeadersOverwriteBehavior() ); } - public function testPrependedMiddlewareCanSeeCallOptionAddedByAppendedMiddleware() + public function testMiddlewareOptionsIsPreservedByFilterAndVisibleToMiddlewares() { $unaryDescriptors = [ 'callType' => Call::UNARY_CALL, @@ -144,9 +144,14 @@ public function testPrependedMiddlewareCanSeeCallOptionAddedByAppendedMiddleware $request = new MockRequestBody([]); $transport = $this->prophesize(TransportInterface::class); + $middlewareOptions = ['myCustomKey' => 'myCustomValue']; + $transport->startUnaryCall( Argument::type(Call::class), - Argument::any() + Argument::that(function ($options) use ($middlewareOptions) { + return isset($options['middlewareOptions']) + && $options['middlewareOptions'] === $middlewareOptions; + }) ) ->shouldBeCalledOnce() ->willReturn($this->prophesize(PromiseInterface::class)->reveal()); @@ -165,80 +170,32 @@ public function testPrependedMiddlewareCanSeeCallOptionAddedByAppendedMiddleware $client->set('credentialsWrapper', $credentialsWrapper); $client->set('descriptors', ['method' => $unaryDescriptors]); - // 1. Append a middleware (outermost) that adds a custom option to callOptions - $client->addMiddleware(function (callable $handler) { - return function (Call $call, array $options) use ($handler) { - $options['myCustomOption'] = 'myCustomValue'; + $appendedCalled = false; + $client->addMiddleware(function (callable $handler) use (&$appendedCalled, $middlewareOptions) { + return function (Call $call, array $options) use ($handler, &$appendedCalled, $middlewareOptions) { + $appendedCalled = true; + $this->assertEquals($middlewareOptions, $options['middlewareOptions'] ?? null); return $handler($call, $options); }; }); - // 2. Prepend a middleware (innermost) that confirms it can see the custom option - $prependedMiddlewareCalled = false; - $client->prependMiddleware(function (callable $handler) use (&$prependedMiddlewareCalled) { - return function (Call $call, array $options) use ($handler, &$prependedMiddlewareCalled) { - $prependedMiddlewareCalled = true; - $this->assertTrue(isset($options['myCustomOption'])); - $this->assertEquals('myCustomValue', $options['myCustomOption']); + $prependedCalled = false; + $client->prependMiddleware(function (callable $handler) use (&$prependedCalled, $middlewareOptions) { + return function (Call $call, array $options) use ($handler, &$prependedCalled, $middlewareOptions) { + $prependedCalled = true; + $this->assertEquals($middlewareOptions, $options['middlewareOptions'] ?? null); return $handler($call, $options); }; }); $client->startApiCall( 'method', - $request - ); - - $this->assertTrue($prependedMiddlewareCalled); - } - - public function testOptionsFilterMiddlewareAllowsCredentialsWrapperButStripsOtherCustomOptions() - { - $unaryDescriptors = [ - 'callType' => Call::UNARY_CALL, - 'responseType' => 'decodeType' - ]; - $request = new MockRequestBody([]); - $transport = $this->prophesize(TransportInterface::class); - - $customCredentialsWrapper = CredentialsWrapper::build([ - 'keyFile' => __DIR__ . '/testdata/json-key-file.json' - ]); - - $transport->startUnaryCall( - Argument::type(Call::class), - Argument::that(function ($options) use ($customCredentialsWrapper) { - $hasCredentialsWrapper = isset($options['credentialsWrapper']) - && $options['credentialsWrapper'] === $customCredentialsWrapper; - $doesNotHaveCustomOption = !isset($options['myCustomOption']); - return $hasCredentialsWrapper && $doesNotHaveCustomOption; - }) - ) - ->shouldBeCalledOnce() - ->willReturn($this->prophesize(PromiseInterface::class)->reveal()); - - $client = new StubGapicClient(); - $client->set('agentHeader', []); - $client->set( - 'retrySettings', - ['method' => $this->prophesize(RetrySettings::class)->reveal()] + $request, + ['middlewareOptions' => $middlewareOptions] ); - $client->set('transport', $transport->reveal()); - $client->set('credentialsWrapper', $customCredentialsWrapper); - $client->set('descriptors', ['method' => $unaryDescriptors]); - - $client->addMiddleware(function (callable $handler) use ($customCredentialsWrapper) { - return function (Call $call, array $options) use ($handler, $customCredentialsWrapper) { - $options['credentialsWrapper'] = $customCredentialsWrapper; - $options['myCustomOption'] = 'myCustomValue'; - return $handler($call, $options); - }; - }); - $client->startApiCall( - 'method', - $request - ); + $this->assertTrue($appendedCalled, 'Appended middleware should have been called'); + $this->assertTrue($prependedCalled, 'Prepended middleware should have been called'); } public function testVersionedHeadersOverwriteBehavior()