fix: ensure metadata return type is loaded into descriptor pool (#439)

googleapis · Jan 6, 2023 · a40cf8d · a40cf8d
1 parent d3f052d
commit a40cf8d
Show file tree

Hide file tree

Showing 4 changed files with 130 additions and 6 deletions.
diff --git a/src/GapicClientTrait.php b/src/GapicClientTrait.php
@@ -765,6 +765,7 @@ private function createCallStack(array $callConstructionOptions)
             'transportOptions',
             'metadataCallback',
             'audience',
+            'metadataReturnType'
         ]);
 
         return $callStack;
@@ -827,8 +828,8 @@ private function startOperationsCall(
         $callStack = $this->createCallStack(
             $this->configureCallConstructionOptions($methodName, $optionalArgs)
         );
-
         $descriptor = $this->descriptors[$methodName]['longRunning'];
+        $metadataReturnType = null;
 
         // Call the methods supplied in "additionalArgumentMethods" on the request Message object
         // to build the "additionalOperationArguments" option for the operation response.
@@ -841,6 +842,10 @@ private function startOperationsCall(
             unset($descriptor['additionalArgumentMethods']);
         }
 
+        if (isset($descriptor['metadataReturnType'])) {
+            $metadataReturnType = $descriptor['metadataReturnType'];
+        }
+
         $callStack = new OperationsMiddleware($callStack, $client, $descriptor);
 
         $call = new Call(
@@ -853,6 +858,7 @@ private function startOperationsCall(
 
         $this->modifyUnaryCallable($callStack);
         return $callStack($call, $optionalArgs + array_filter([
+            'metadataReturnType' => $metadataReturnType,
             'audience' => self::getDefaultAudience()
         ]));
     }

diff --git a/src/Transport/RestTransport.php b/src/Transport/RestTransport.php
@@ -128,10 +128,33 @@ function (ResponseInterface $response) use ($call, $options) {
                 $decodeType = $call->getDecodeType();
                 /** @var Message $return */
                 $return = new $decodeType;
-                $return->mergeFromJsonString(
-                    (string) $response->getBody(),
-                    true
-                );
+                $body = (string) $response->getBody();
+
+                // In some rare cases LRO response metadata may not be loaded
+                // in the descriptor pool, triggering an exception. The catch
+                // statement handles this case and attempts to add the LRO
+                // metadata type to the pool by directly instantiating the
+                // metadata class.
+                try {
+                    $return->mergeFromJsonString(
+                        $body,
+                        true
+                    );
+                } catch (\Exception $ex) {
+                    if (!isset($options['metadataReturnType'])) {
+                        throw $ex;
+                    }
+
+                    if (strpos($ex->getMessage(), 'Error occurred during parsing:') !== 0) {
+                        throw $ex;
+                    }
+
+                    new $options['metadataReturnType']();
+                    $return->mergeFromJsonString(
+                        $body,
+                        true
+                    );
+                }
 
                 if (isset($options['metadataCallback'])) {
                     $metadataCallback = $options['metadataCallback'];

diff --git a/tests/Tests/Unit/GapicClientTraitTest.php b/tests/Tests/Unit/GapicClientTraitTest.php
@@ -1208,7 +1208,8 @@ public function testModifyUnaryCallFromOperationsCall()
                     'headers' => AgentHeader::buildAgentHeader([]),
                     'credentialsWrapper' => CredentialsWrapper::build([
                         'keyFile' => __DIR__ . '/testdata/json-key-file.json'
-                    ])
+                    ]),
+                    'metadataReturnType' => 'metadataType'
                 ])
             )
             ->willReturn(new FulfilledPromise(new Operation()));
@@ -1471,6 +1472,7 @@ public function testDefaultAudienceWithOperations()
                     'audience' => 'https://service-address/',
                     'headers' => [],
                     'credentialsWrapper' => $credentialsWrapper,
+                    'metadataReturnType' => 'metadataType'
                 ]
             )
             ->shouldBeCalledOnce()

diff --git a/tests/Tests/Unit/Transport/RestTransportTest.php b/tests/Tests/Unit/Transport/RestTransportTest.php
@@ -44,8 +44,10 @@
 use Google\ApiCore\Transport\RestTransport;
 use Google\ApiCore\ValidationException;
 use Google\Auth\HttpHandler\HttpHandlerFactory;
+use Google\LongRunning\Operation;
 use Google\Protobuf\Any;
 use Google\Rpc\ErrorInfo;
+use Google\Type\DateTime;
 use GuzzleHttp\Exception\RequestException;
 use GuzzleHttp\Promise\Create;
 use GuzzleHttp\Psr7\Request;
@@ -144,6 +146,97 @@ public function testStartUnaryCallThrowsException()
             ->wait();
     }
 
+    /**
+     * @runInSeparateProcess
+     */
+    public function testStartUnaryCallWithValidProtoNotLoadedInDescPool()
+    {
+        $endpoint = 'www.example.com';
+        $expectedRequest = new Request(
+            'POST',
+            $endpoint,
+            [],
+            ''
+        );
+        $body = [
+            'name' => 'projects/my-project/locations/us-central1/operations/my-operation',
+            'metadata' => [
+                // This type is arbitrarily chosen and should not exist within the descriptor pool
+                // upon instantation of this test.
+                '@type' => 'type.googleapis.com/google.type.DateTime'
+            ]
+        ];
+        $httpHandler = function (RequestInterface $request) use ($body, $expectedRequest) {
+            $this->assertEquals($expectedRequest, $request);
+            return Create::promiseFor(
+                new Response(
+                    200,
+                    [],
+                    json_encode($body)
+                )
+            );
+        };
+        $call = new Call(
+            'Testing123',
+            Operation::class,
+            new MockRequest()
+        );
+
+        $response = $this->getTransport($httpHandler, $endpoint)
+            ->startUnaryCall($call, [
+                'metadataReturnType' => DateTime::class
+            ])
+            ->wait();
+
+        $this->assertInstanceOf(Operation::class, $response);
+        $this->assertEquals(
+            $body['metadata']['@type'],
+            $response->getMetadata()->getTypeUrl()
+        );
+    }
+
+    /**
+     * @runInSeparateProcess
+     */
+    public function testStartUnaryCallWithValidProtoNotLoadedInDescPoolThrowsExWithoutMetadataType()
+    {
+        $endpoint = 'www.example.com';
+        $expectedRequest = new Request(
+            'POST',
+            $endpoint,
+            [],
+            ''
+        );
+        $body = [
+            'name' => 'projects/my-project/locations/us-central1/operations/my-operation',
+            'metadata' => [
+                // This type is arbitrarily chosen and should not exist within the descriptor pool
+                // upon instantation of this test.
+                '@type' => 'type.googleapis.com/google.type.DateTime'
+            ]
+        ];
+        $httpHandler = function (RequestInterface $request) use ($body, $expectedRequest) {
+            $this->assertEquals($expectedRequest, $request);
+            return Create::promiseFor(
+                new Response(
+                    200,
+                    [],
+                    json_encode($body)
+                )
+            );
+        };
+        $call = new Call(
+            'Testing123',
+            Operation::class,
+            new MockRequest()
+        );
+        $this->expectException(\Exception::class);
+        $this->expectExceptionMessageMatches('/^Error occurred during parsing:/');
+        $this->getTransport($httpHandler, $endpoint)
+            ->startUnaryCall($call, [])
+            ->wait();
+    }
+
     public function testServerStreamingCallThrowsBadMethodCallException()
     {
         $request = new Request('POST', 'http://www.example.com');