From b865ae7f9ca059fc6cb54dbb0d1b4afd65ec3a0b Mon Sep 17 00:00:00 2001 From: alicejli Date: Mon, 29 Nov 2021 20:20:37 +0000 Subject: [PATCH 01/17] fix: update REST transport error details --- src/ApiException.php | 33 +++++++++++++++++++++++++++++++++ src/Transport/RestTransport.php | 2 +- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/ApiException.php b/src/ApiException.php index d0d93d73f..22d7f5db7 100644 --- a/src/ApiException.php +++ b/src/ApiException.php @@ -114,6 +114,39 @@ public static function createFromApiResponse( ); } + /** + * @param string $basicMessage + * @param int $rpcCode + * @param array|null $metadata + * @param \Exception $previous + * @return ApiException + */ + public static function createFromApiResponseREST( + $basicMessage, + $rpcCode, + array $metadata = null, + \Exception $previous = null + ) { + if (empty($metadata)) { + $decodedMetadata = []; + return self::create( + $basicMessage, + $rpcCode, + $metadata, + $decodedMetadata, + $previous + ); + } else { + return self::create( + $basicMessage, + $rpcCode, + $metadata, + $metadata, + $previous + ); + } + } + /** * Construct an ApiException with a useful message, including decoded metadata. * diff --git a/src/Transport/RestTransport.php b/src/Transport/RestTransport.php index 0919f9666..5d3f50c71 100644 --- a/src/Transport/RestTransport.php +++ b/src/Transport/RestTransport.php @@ -176,7 +176,7 @@ private function convertToApiException(RequestException $ex) ? ApiStatus::rpcCodeFromStatus($error['status']) : $ex->getCode(); $metadata = isset($error['details']) ? $error['details'] : null; - return ApiException::createFromApiResponse($basicMessage, $code, $metadata); + return ApiException::createFromApiResponseREST($basicMessage, $code, $metadata); } // Use the RPC code instead of the HTTP Status Code. $code = ApiStatus::rpcCodeFromHttpStatusCode($res->getStatusCode()); From 96046b28c44445c406ce8e39f57fe0f72e9ca132 Mon Sep 17 00:00:00 2001 From: Lucas Michot Date: Mon, 29 Nov 2021 22:42:11 +0100 Subject: [PATCH 02/17] chore: improve test assertions (#350) --- src/Testing/GeneratedTest.php | 2 +- tests/Tests/Unit/AgentHeaderTest.php | 2 +- tests/Tests/Unit/GapicClientTraitTest.php | 6 +- .../Unit/LongRunning/OperationsClientTest.php | 10 ++-- .../Unit/Middleware/RetryMiddlewareTest.php | 8 +-- tests/Tests/Unit/OperationResponseTest.php | 2 +- tests/Tests/Unit/RequestBuilderTest.php | 56 +++++++++---------- .../Transport/Rest/JsonStreamDecoderTest.php | 2 +- 8 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/Testing/GeneratedTest.php b/src/Testing/GeneratedTest.php index 25e82ef5b..6f709218a 100644 --- a/src/Testing/GeneratedTest.php +++ b/src/Testing/GeneratedTest.php @@ -50,7 +50,7 @@ public function assertProtobufEquals(&$expected, &$actual) $this->assertEquals($expected, $actual); } - $this->assertSame(count($expected), count($actual)); + $this->assertCount(count($expected), $actual); $expectedValues = $this->getValues($expected); $actualValues = $this->getValues($actual); diff --git a/tests/Tests/Unit/AgentHeaderTest.php b/tests/Tests/Unit/AgentHeaderTest.php index ba7de1ace..d5a1ee819 100644 --- a/tests/Tests/Unit/AgentHeaderTest.php +++ b/tests/Tests/Unit/AgentHeaderTest.php @@ -116,7 +116,7 @@ public function testGetGapicVersionWithVersionFile() public function testGetGapicVersionWithNoAvailableVersion() { - $this->assertEquals('', AgentHeader::readGapicVersionFromFile(__CLASS__)); + $this->assertSame('', AgentHeader::readGapicVersionFromFile(__CLASS__)); } public function testWithGrpcAndRest() diff --git a/tests/Tests/Unit/GapicClientTraitTest.php b/tests/Tests/Unit/GapicClientTraitTest.php index 4abbe3c39..73b2ab009 100644 --- a/tests/Tests/Unit/GapicClientTraitTest.php +++ b/tests/Tests/Unit/GapicClientTraitTest.php @@ -212,7 +212,7 @@ public function testGetGapicVersionWithVersionFile() public function testGetGapicVersionWithNoAvailableVersion() { $client = new GapicClientTraitStub(); - $this->assertEquals('', $client->call('getGapicVersion', [[]])); + $this->assertSame('', $client->call('getGapicVersion', [[]])); } public function testGetGapicVersionWithLibVersion() @@ -639,7 +639,7 @@ public function testModifyClientOptions() $updatedOptions = $client->call('buildClientOptions', [$options]); $this->assertArrayHasKey('addNewOption', $updatedOptions); - $this->assertSame(true, $updatedOptions['disableRetries']); + $this->assertTrue($updatedOptions['disableRetries']); } private function buildClientToTestModifyCallMethods() @@ -1229,7 +1229,7 @@ public function testMtlsClientOptionWithDefaultClientCertSource() $client = new GapicClientTraitStub(); $options = $client->call('buildClientOptions', [[]]); - $this->assertEquals('test.mtls.address.com:443', $options['apiEndpoint']); + $this->assertSame('test.mtls.address.com:443', $options['apiEndpoint']); $this->assertTrue(is_callable($options['clientCertSource'])); $this->assertEquals(['foo', 'foo'], $options['clientCertSource']()); } diff --git a/tests/Tests/Unit/LongRunning/OperationsClientTest.php b/tests/Tests/Unit/LongRunning/OperationsClientTest.php index e18115153..8f9b9556d 100644 --- a/tests/Tests/Unit/LongRunning/OperationsClientTest.php +++ b/tests/Tests/Unit/LongRunning/OperationsClientTest.php @@ -96,7 +96,7 @@ public function getOperationTest() $response = $client->getOperation($name); $this->assertEquals($expectedResponse, $response); $actualRequests = $transport->popReceivedCalls(); - $this->assertSame(1, count($actualRequests)); + $this->assertCount(1, $actualRequests); $actualFuncCall = $actualRequests[0]->getFuncCall(); $actualRequestObject = $actualRequests[0]->getRequestObject(); $this->assertSame('/google.longrunning.Operations/GetOperation', $actualFuncCall); @@ -173,11 +173,11 @@ public function listOperationsTest() $response = $client->listOperations($name, $filter); $this->assertEquals($expectedResponse, $response->getPage()->getResponseObject()); $resources = iterator_to_array($response->iterateAllElements()); - $this->assertSame(1, count($resources)); + $this->assertCount(1, $resources); $this->assertEquals($expectedResponse->getOperations()[0], $resources[0]); $actualRequests = $transport->popReceivedCalls(); - $this->assertSame(1, count($actualRequests)); + $this->assertCount(1, $actualRequests); $actualFuncCall = $actualRequests[0]->getFuncCall(); $actualRequestObject = $actualRequests[0]->getRequestObject(); $this->assertSame('/google.longrunning.Operations/ListOperations', $actualFuncCall); @@ -250,7 +250,7 @@ public function cancelOperationTest() $client->cancelOperation($name); $actualRequests = $transport->popReceivedCalls(); - $this->assertSame(1, count($actualRequests)); + $this->assertCount(1, $actualRequests); $actualFuncCall = $actualRequests[0]->getFuncCall(); $actualRequestObject = $actualRequests[0]->getRequestObject(); $this->assertSame('/google.longrunning.Operations/CancelOperation', $actualFuncCall); @@ -320,7 +320,7 @@ public function deleteOperationTest() $client->deleteOperation($name); $actualRequests = $transport->popReceivedCalls(); - $this->assertSame(1, count($actualRequests)); + $this->assertCount(1, $actualRequests); $actualFuncCall = $actualRequests[0]->getFuncCall(); $actualRequestObject = $actualRequests[0]->getRequestObject(); $this->assertSame('/google.longrunning.Operations/DeleteOperation', $actualFuncCall); diff --git a/tests/Tests/Unit/Middleware/RetryMiddlewareTest.php b/tests/Tests/Unit/Middleware/RetryMiddlewareTest.php index 1ab6fa7e6..d38042806 100644 --- a/tests/Tests/Unit/Middleware/RetryMiddlewareTest.php +++ b/tests/Tests/Unit/Middleware/RetryMiddlewareTest.php @@ -94,7 +94,7 @@ public function testRetryBackoff() [] )->wait(); - $this->assertEquals('Ok!', $response); + $this->assertSame('Ok!', $response); $this->assertEquals(3, $callCount); } @@ -203,9 +203,9 @@ public function testRetryLogicalTimeout() [] )->wait(); - $this->assertEquals('Ok!', $response); + $this->assertSame('Ok!', $response); $this->assertEquals(3, $callCount); - $this->assertEquals(3, count($observedTimeouts)); + $this->assertCount(3, $observedTimeouts); $this->assertEquals($observedTimeouts[0], $timeout); for ($i = 1; $i < count($observedTimeouts); $i++) { $this->assertTrue($observedTimeouts[$i-1] > $observedTimeouts[$i]); @@ -231,7 +231,7 @@ public function testNoRetryLogicalTimeout() [] )->wait(); - $this->assertEquals('Ok!', $response); + $this->assertSame('Ok!', $response); $this->assertEquals($observedTimeout, $timeout); } } diff --git a/tests/Tests/Unit/OperationResponseTest.php b/tests/Tests/Unit/OperationResponseTest.php index aa1dd614f..0f641564f 100644 --- a/tests/Tests/Unit/OperationResponseTest.php +++ b/tests/Tests/Unit/OperationResponseTest.php @@ -265,7 +265,7 @@ public function testCustomOperationError() $this->assertNotNull($error); $this->assertEquals(Code::INTERNAL, $error->getCode()); - $this->assertEquals('It failed, sorry :(', $error->getMessage()); + $this->assertSame('It failed, sorry :(', $error->getMessage()); } public function testEmptyCustomOperationErrorIsSuccessful() diff --git a/tests/Tests/Unit/RequestBuilderTest.php b/tests/Tests/Unit/RequestBuilderTest.php index 121168a87..5e557dbb9 100644 --- a/tests/Tests/Unit/RequestBuilderTest.php +++ b/tests/Tests/Unit/RequestBuilderTest.php @@ -57,7 +57,7 @@ public function testMethodWithUrlPlaceholder() $this->assertEmpty($uri->getQuery()); $this->assertEmpty((string) $request->getBody()); - $this->assertEquals('/v1/message/foo', $uri->getPath()); + $this->assertSame('/v1/message/foo', $uri->getPath()); } public function testMethodWithBody() @@ -72,7 +72,7 @@ public function testMethodWithBody() $uri = $request->getUri(); $this->assertEmpty($uri->getQuery()); - $this->assertEquals('/v1/message/foo', $uri->getPath()); + $this->assertSame('/v1/message/foo', $uri->getPath()); $this->assertEquals( ['name' => 'message/foo', 'nestedMessage' => ['name' => 'nested/foo']], json_decode($request->getBody(), true) @@ -91,7 +91,7 @@ public function testMethodWithNestedMessageAsBody() $uri = $request->getUri(); $this->assertEmpty($uri->getQuery()); - $this->assertEquals('/v1/message/foo', $uri->getPath()); + $this->assertSame('/v1/message/foo', $uri->getPath()); $this->assertEquals( ['name' => 'nested/foo'], json_decode($request->getBody(), true) @@ -155,7 +155,7 @@ public function testMethodWithNestedUrlPlaceholder() $uri = $request->getUri(); $this->assertEmpty($uri->getQuery()); - $this->assertEquals('/v1/nested/foo', $uri->getPath()); + $this->assertSame('/v1/nested/foo', $uri->getPath()); $this->assertEquals( ['name' => 'message/foo', 'nestedMessage' => ['name' => 'nested/foo']], json_decode($request->getBody(), true) @@ -172,8 +172,8 @@ public function testMethodWithUrlRepeatedField() $uri = $request->getUri(); $this->assertEmpty((string) $request->getBody()); - $this->assertEquals('/v1/message/foo', $uri->getPath()); - $this->assertEquals('repeatedField=bar1&repeatedField=bar2', $uri->getQuery()); + $this->assertSame('/v1/message/foo', $uri->getPath()); + $this->assertSame('repeatedField=bar1&repeatedField=bar2', $uri->getQuery()); } public function testMethodWithHeaders() @@ -186,9 +186,9 @@ public function testMethodWithHeaders() 'header2' => 'value2' ]); - $this->assertEquals('value1', $request->getHeaderLine('header1')); - $this->assertEquals('value2', $request->getHeaderLine('header2')); - $this->assertEquals('application/json', $request->getHeaderLine('Content-Type')); + $this->assertSame('value1', $request->getHeaderLine('header1')); + $this->assertSame('value2', $request->getHeaderLine('header2')); + $this->assertSame('application/json', $request->getHeaderLine('Content-Type')); } public function testMethodWithColon() @@ -200,7 +200,7 @@ public function testMethodWithColon() $uri = $request->getUri(); $this->assertEmpty($uri->getQuery()); - $this->assertEquals('/v1/message/foo:action', $uri->getPath()); + $this->assertSame('/v1/message/foo:action', $uri->getPath()); } public function testMethodWithMultipleWildcardsAndColonInUrl() @@ -216,7 +216,7 @@ public function testMethodWithMultipleWildcardsAndColonInUrl() $uri = $request->getUri(); $this->assertEmpty($uri->getQuery()); - $this->assertEquals('/v1/message/foo/number/10:action', $uri->getPath()); + $this->assertSame('/v1/message/foo/number/10:action', $uri->getPath()); } public function testMethodWithSimplePlaceholder() @@ -230,7 +230,7 @@ public function testMethodWithSimplePlaceholder() ); $uri = $request->getUri(); - $this->assertEquals('/v1/message-name', $uri->getPath()); + $this->assertSame('/v1/message-name', $uri->getPath()); } public function testMethodWithAdditionalBindings() @@ -239,19 +239,19 @@ public function testMethodWithAdditionalBindings() $message->setName('message/foo'); $request = $this->builder->build(self::SERVICE_NAME . '/MethodWithAdditionalBindings', $message); - $this->assertEquals('/v1/message/foo/additional/bindings', $request->getUri()->getPath()); + $this->assertSame('/v1/message/foo/additional/bindings', $request->getUri()->getPath()); $message->setName('different/format/foo'); $request = $this->builder->build(self::SERVICE_NAME . '/MethodWithAdditionalBindings', $message); - $this->assertEquals('/v1/different/format/foo/additional/bindings', $request->getUri()->getPath()); + $this->assertSame('/v1/different/format/foo/additional/bindings', $request->getUri()->getPath()); $nestedMessage = new MockRequestBody(); $nestedMessage->setName('nested/foo'); $message->setNestedMessage($nestedMessage); $request = $this->builder->build(self::SERVICE_NAME . '/MethodWithAdditionalBindings', $message); - $this->assertEquals('/v2/nested/foo/additional/bindings', $request->getUri()->getPath()); + $this->assertSame('/v2/nested/foo/additional/bindings', $request->getUri()->getPath()); } public function testMethodWithSpecialJsonMapping() @@ -301,22 +301,22 @@ public function testMethodWithSpecialJsonMapping() $query = Query::parse($uri->getQuery()); - $this->assertEquals('XDAwMA==', $query['bytesValue']); + $this->assertSame('XDAwMA==', $query['bytesValue']); // @todo (dwsupplee) Investigate differences in native protobuf implementation // between v3.7.0 and v3.9.0 - this passed previously with the value // "9001.000500000s". if (extension_loaded('protobuf')) { - $this->assertEquals('9001.000500000s', $query['durationValue']); + $this->assertSame('9001.000500000s', $query['durationValue']); } else { - $this->assertEquals('9001.000500s', $query['durationValue']); + $this->assertSame('9001.000500s', $query['durationValue']); } - $this->assertEquals('path1,path2', $query['fieldMask']); + $this->assertSame('path1,path2', $query['fieldMask']); $this->assertEquals(100, $query['int64Value']); $this->assertEquals(['val1', 'val2'], $query['listValue']); - $this->assertEquals('some-value', $query['stringValue']); - $this->assertEquals('val5', $query['structValue.test']); - $this->assertEquals('1970-01-01T02:30:01Z', $query['timestampValue']); - $this->assertEquals('some-value', $query['valueValue']); + $this->assertSame('some-value', $query['stringValue']); + $this->assertSame('val5', $query['structValue.test']); + $this->assertSame('1970-01-01T02:30:01Z', $query['timestampValue']); + $this->assertSame('some-value', $query['valueValue']); } public function testMethodWithoutPlaceholders() @@ -334,8 +334,8 @@ public function testMethodWithoutPlaceholders() $request = $this->builder->build(self::SERVICE_NAME . '/MethodWithoutPlaceholders', $message); $query = Query::parse($request->getUri()->getQuery()); - $this->assertEquals('path1,path2', $query['fieldMask']); - $this->assertEquals('some-value', $query['stringValue']); + $this->assertSame('path1,path2', $query['fieldMask']); + $this->assertSame('some-value', $query['stringValue']); } public function testMethodWithRequiredQueryParametersAndDefaultValues() @@ -347,7 +347,7 @@ public function testMethodWithRequiredQueryParametersAndDefaultValues() $request = $this->builder->build(self::SERVICE_NAME . '/MethodWithRequiredQueryParameters', $message); $query = Query::parse($request->getUri()->getQuery()); - $this->assertEquals('', $query['name']); + $this->assertSame('', $query['name']); $this->assertEquals(0, $query['number']); } @@ -363,7 +363,7 @@ public function testMethodWithComplexMessageInQueryString() $request = $this->builder->build(self::SERVICE_NAME . '/MethodWithoutPlaceholders', $message); $query = Query::parse($request->getUri()->getQuery()); - $this->assertEquals('some-name', $query['nestedMessage.name']); + $this->assertSame('some-name', $query['nestedMessage.name']); $this->assertEquals(10, $query['nestedMessage.number']); } @@ -375,7 +375,7 @@ public function testMethodWithOneOfInQueryString() $request = $this->builder->build(self::SERVICE_NAME . '/MethodWithoutPlaceholders', $message); $query = Query::parse($request->getUri()->getQuery()); - $this->assertEquals('some-value', $query['field1']); + $this->assertSame('some-value', $query['field1']); } /** diff --git a/tests/Tests/Unit/Transport/Rest/JsonStreamDecoderTest.php b/tests/Tests/Unit/Transport/Rest/JsonStreamDecoderTest.php index 9f5f8a6c7..b85933a8a 100644 --- a/tests/Tests/Unit/Transport/Rest/JsonStreamDecoderTest.php +++ b/tests/Tests/Unit/Transport/Rest/JsonStreamDecoderTest.php @@ -137,7 +137,7 @@ public function testJsonStreamDecoderBadClose($payload) { $stream->write($payload); $decoder = new JsonStreamDecoder($stream, Operation::class, ['readChunkSizeBytes' => 10]); foreach($decoder->decode() as $op) { - $this->assertEquals('foo', $op->getName()); + $this->assertSame('foo', $op->getName()); $stream->close(); } } From cd5c121f5a418185dd8e038d9eb067c85b016a17 Mon Sep 17 00:00:00 2001 From: alicejli Date: Mon, 29 Nov 2021 22:54:17 +0000 Subject: [PATCH 03/17] update name and cleanup function --- src/ApiException.php | 19 ++++++++++--------- src/Transport/RestTransport.php | 2 +- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/ApiException.php b/src/ApiException.php index 22d7f5db7..52ba51dba 100644 --- a/src/ApiException.php +++ b/src/ApiException.php @@ -115,13 +115,15 @@ public static function createFromApiResponse( } /** + * For REST-based responses, the metadata does not need to be decoded. + * * @param string $basicMessage * @param int $rpcCode * @param array|null $metadata * @param \Exception $previous * @return ApiException */ - public static function createFromApiResponseREST( + public static function createFromRestApiResponse( $basicMessage, $rpcCode, array $metadata = null, @@ -136,15 +138,14 @@ public static function createFromApiResponseREST( $decodedMetadata, $previous ); - } else { - return self::create( - $basicMessage, - $rpcCode, - $metadata, - $metadata, - $previous - ); } + return self::create( + $basicMessage, + $rpcCode, + $metadata, + $metadata, + $previous + ); } /** diff --git a/src/Transport/RestTransport.php b/src/Transport/RestTransport.php index 5d3f50c71..c9e6780e1 100644 --- a/src/Transport/RestTransport.php +++ b/src/Transport/RestTransport.php @@ -176,7 +176,7 @@ private function convertToApiException(RequestException $ex) ? ApiStatus::rpcCodeFromStatus($error['status']) : $ex->getCode(); $metadata = isset($error['details']) ? $error['details'] : null; - return ApiException::createFromApiResponseREST($basicMessage, $code, $metadata); + return ApiException::createFromRestApiResponse($basicMessage, $code, $metadata); } // Use the RPC code instead of the HTTP Status Code. $code = ApiStatus::rpcCodeFromHttpStatusCode($res->getStatusCode()); From 137cb8092e697338dc5ed791b81c857386cee847 Mon Sep 17 00:00:00 2001 From: alicejli Date: Tue, 30 Nov 2021 16:22:36 +0000 Subject: [PATCH 04/17] update tests to not use deprecated functions --- .../Unit/Transport/RestTransportTest.php | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/Tests/Unit/Transport/RestTransportTest.php b/tests/Tests/Unit/Transport/RestTransportTest.php index 93029e655..40d7b414c 100644 --- a/tests/Tests/Unit/Transport/RestTransportTest.php +++ b/tests/Tests/Unit/Transport/RestTransportTest.php @@ -39,10 +39,9 @@ use Google\ApiCore\Testing\MockRequest; use Google\ApiCore\Testing\MockResponse; use Google\ApiCore\Transport\RestTransport; -use Google\Auth\FetchAuthTokenInterface; use Google\Auth\HttpHandler\HttpHandlerFactory; use GuzzleHttp\Exception\RequestException; -use GuzzleHttp\Promise; +use GuzzleHttp\Promise\Create; use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\TestCase; @@ -95,7 +94,7 @@ public function testStartUnaryCall($apiEndpoint) $httpHandler = function (RequestInterface $request, array $options = []) use ($body, $expectedRequest) { $this->assertEquals($expectedRequest, $request); - return Promise\promise_for( + return Create::promiseFor( new Response( 200, [], @@ -127,7 +126,7 @@ public function startUnaryCallDataProvider() public function testStartUnaryCallThrowsException() { $httpHandler = function (RequestInterface $request, array $options = []) { - return Promise\rejection_for(new \Exception()); + return Create::rejectionFor(new \Exception()); }; $this->getTransport($httpHandler) @@ -141,7 +140,7 @@ public function testStartUnaryCallThrowsException() public function testStartUnaryCallThrowsRequestException() { $httpHandler = function (RequestInterface $request, array $options = []) { - return Promise\rejection_for( + return Create::rejectionFor( RequestException::create( new Request('POST', 'http://www.example.com'), new Response( @@ -264,7 +263,7 @@ public function buildInvalidData() public function testNonJsonResponseException() { $httpHandler = function (RequestInterface $request, array $options = []) { - return Promise\rejection_for( + return Create::rejectionFor( RequestException::create( new Request('POST', 'http://www.example.com'), new Response( @@ -286,7 +285,9 @@ public function testAudienceOption() $credentialsWrapper = $this->prophesize(CredentialsWrapper::class); $credentialsWrapper->getAuthorizationHeaderCallback('an-audience') ->shouldBeCalledOnce() - ->willReturn(function() { return []; }); + ->willReturn(function () { + return []; + }); $options = [ 'audience' => 'an-audience', @@ -294,7 +295,7 @@ public function testAudienceOption() ]; $httpHandler = function (RequestInterface $request, array $options = []) { - return Promise\promise_for(new Response(200, [], '{}')); + return Create::promiseFor(new Response(200, [], '{}')); }; $this->getTransport($httpHandler) @@ -325,7 +326,9 @@ public function testNonArrayAuthorizationHeaderThrowsException() $credentialsWrapper = $this->prophesize(CredentialsWrapper::class); $credentialsWrapper->getAuthorizationHeaderCallback(null) ->shouldBeCalledOnce() - ->willReturn(function() { return ''; }); + ->willReturn(function () { + return ''; + }); $options = [ 'credentialsWrapper' => $credentialsWrapper->reveal(), From c6233f7d4da0fefcf2373919510aebf39fa6759b Mon Sep 17 00:00:00 2001 From: Noah Dietz Date: Tue, 30 Nov 2021 14:38:38 -0800 Subject: [PATCH 05/17] feat: add REST server streaming support (#347) * feat: add REST server streaming support * refactor & fix premature closing/cancelation * refactor to Status * set all fields of MockStatus in MockServerStreamingCall * support streaming request exceptions * check if index 0 exists in exception body * skip newlines that break up items * add more tests for weird streams Co-authored-by: Lucas Michot Co-authored-by: Brent Shaffer --- src/ApiException.php | 34 ++++ src/ServerStream.php | 12 +- src/ServerStreamingCallInterface.php | 87 +++++++++ src/Testing/MockServerStreamingCall.php | 13 +- src/Testing/MockStatus.php | 2 +- .../Grpc/ServerStreamingCallWrapper.php | 132 +++++++++++++ src/Transport/GrpcTransport.php | 17 +- src/Transport/Rest/JsonStreamDecoder.php | 55 +++++- .../Rest/RestServerStreamingCall.php | 182 ++++++++++++++++++ src/Transport/RestTransport.php | 92 ++++++--- tests/Tests/Unit/ApiExceptionTest.php | 44 +++++ .../Transport/Rest/JsonStreamDecoderTest.php | 76 +++++++- .../Unit/Transport/RestTransportTest.php | 140 ++++++++++++++ 13 files changed, 832 insertions(+), 54 deletions(-) create mode 100644 src/ServerStreamingCallInterface.php create mode 100644 src/Transport/Grpc/ServerStreamingCallWrapper.php create mode 100644 src/Transport/Rest/RestServerStreamingCall.php diff --git a/src/ApiException.php b/src/ApiException.php index 52ba51dba..fc8c11f51 100644 --- a/src/ApiException.php +++ b/src/ApiException.php @@ -34,6 +34,7 @@ use Exception; use Google\Protobuf\Internal\RepeatedField; use Google\Rpc\Status; +use GuzzleHttp\Exception\RequestException; /** * Represents an exception thrown during an RPC. @@ -191,6 +192,39 @@ public static function createFromRpcStatus(Status $status) ); } + /** + * Creates an ApiException from a GuzzleHttp RequestException. + * + * @param RequestException $ex + * @param boolean $isStream + * @return ApiException + * @throws ValidationException + */ + public static function createFromRequestException(RequestException $ex, $isStream = false) + { + $res = $ex->getResponse(); + $body = (string) $res->getBody(); + $decoded = json_decode($body, true); + + // A streaming response body will return one error in an array. Parse + // that first (and only) error message, if provided. + if ($isStream && isset($decoded[0])) { + $decoded = $decoded[0]; + } + + if ($error = $decoded['error']) { + $basicMessage = $error['message']; + $code = isset($error['status']) + ? ApiStatus::rpcCodeFromStatus($error['status']) + : $ex->getCode(); + $metadata = isset($error['details']) ? $error['details'] : null; + return static::createFromApiResponse($basicMessage, $code, $metadata); + } + // Use the RPC code instead of the HTTP Status Code. + $code = ApiStatus::rpcCodeFromHttpStatusCode($res->getStatusCode()); + return static::createFromApiResponse($body, $code); + } + /** * @return null|string */ diff --git a/src/ServerStream.php b/src/ServerStream.php index 713273c28..66d019f54 100644 --- a/src/ServerStream.php +++ b/src/ServerStream.php @@ -34,7 +34,7 @@ use Google\Rpc\Code; /** - * ServerStream is the response object from a gRPC server streaming API call. + * ServerStream is the response object from a server streaming API call. */ class ServerStream { @@ -44,7 +44,7 @@ class ServerStream /** * ServerStream constructor. * - * @param \Grpc\ServerStreamingCall $serverStreamingCall The gRPC server streaming call object + * @param ServerStreamingCallInterface $serverStreamingCall The server streaming call object * @param array $streamingDescriptor */ public function __construct($serverStreamingCall, array $streamingDescriptor = []) @@ -77,15 +77,15 @@ public function readAll() } } $status = $this->call->getStatus(); - if (!($status->code == Code::OK)) { - throw ApiException::createFromStdClass($status); + if ($status->getCode() !== Code::OK) { + throw ApiException::createFromRpcStatus($status); } } /** - * Return the underlying gRPC call object + * Return the underlying call object. * - * @return \Grpc\ServerStreamingCall|mixed + * @return ServerStreamingCallInterface */ public function getServerStreamingCall() { diff --git a/src/ServerStreamingCallInterface.php b/src/ServerStreamingCallInterface.php new file mode 100644 index 000000000..bc4c77610 --- /dev/null +++ b/src/ServerStreamingCallInterface.php @@ -0,0 +1,87 @@ + a number (optional) + */ + public function start($data, array $metadata = [], array $options = []); + + /** + * @return mixed An iterator of response values. + */ + public function responses(); + + /** + * Return the status of the server stream. + * + * @return \Google\Rpc\Status The API status. + */ + public function getStatus(); + + /** + * @return mixed The metadata sent by the server. + */ + public function getMetadata(); + + /** + * @return mixed The trailing metadata sent by the server. + */ + public function getTrailingMetadata(); + + /** + * @return string The URI of the endpoint. + */ + public function getPeer(); + + /** + * Cancels the call. + */ + public function cancel(); + + /** + * Set the CallCredentials for the underlying Call. + * + * @param mixed $call_credentials The CallCredentials object + */ + public function setCallCredentials($call_credentials); +} diff --git a/src/Testing/MockServerStreamingCall.php b/src/Testing/MockServerStreamingCall.php index 2826b8871..7e8da83be 100644 --- a/src/Testing/MockServerStreamingCall.php +++ b/src/Testing/MockServerStreamingCall.php @@ -35,6 +35,7 @@ use Google\ApiCore\ApiException; use Google\ApiCore\ApiStatus; use Google\Rpc\Code; +use Google\Rpc\Status; /** * The MockServerStreamingCall class is used to mock out the \Grpc\ServerStreamingCall class @@ -58,7 +59,7 @@ public function __construct($responses, $deserialize = null, $status = null) $this->responses = $responses; $this->deserialize = $deserialize; if (is_null($status)) { - $status = new MockStatus(Code::OK); + $status = new MockStatus(Code::OK, 'OK', []); } $this->status = $status; } @@ -73,7 +74,7 @@ public function responses() } /** - * @return MockStatus|null|\stdClass + * @return \Google\Rpc\Status * @throws ApiException */ public function getStatus() @@ -85,6 +86,12 @@ public function getStatus() ApiStatus::INTERNAL ); } - return $this->status; + return new Status( + [ + 'code' => $this->status->code, + 'message' => $this->status->details, + 'details' => $this->status->metadata + ] + ); } } diff --git a/src/Testing/MockStatus.php b/src/Testing/MockStatus.php index 5be115c00..dfaf7ea70 100644 --- a/src/Testing/MockStatus.php +++ b/src/Testing/MockStatus.php @@ -40,7 +40,7 @@ class MockStatus public $code; public $details; public $metadata; - public function __construct($code, $details = null, $metadata = null) + public function __construct($code, $details = null, array $metadata = []) { $this->code = $code; $this->details = $details; diff --git a/src/Transport/Grpc/ServerStreamingCallWrapper.php b/src/Transport/Grpc/ServerStreamingCallWrapper.php new file mode 100644 index 000000000..9d4aae76b --- /dev/null +++ b/src/Transport/Grpc/ServerStreamingCallWrapper.php @@ -0,0 +1,132 @@ +stream = $stream; + } + + /** + * {@inheritdoc} + */ + public function start($data, array $metadata = [], array $callOptions = []) + { + $this->stream->start($data, $metadata, $callOptions); + } + + /** + * {@inheritdoc} + */ + public function responses() + { + foreach ($this->stream->responses() as $response) { + yield $response; + } + } + + /** + * {@inheritdoc} + */ + public function getStatus() + { + // Convert the \stdClass from the Grpc\ServerStreamingCall into a \Google\Rpc\Status. + $st = $this->stream->getStatus(); + $code = property_exists($st, 'code') ? $st->code : Code::OK; + return new Status( + [ + 'code' => $code, + // Field 'details' is actually a string in this \stdClass. + 'message' => property_exists($st, 'details') ? $st->details : '', + // Field 'metadata' is actually an array of objects in this \stdClass. + 'details' => property_exists($st, 'metadata') && $code !== Code::OK + ? Serializer::decodeMetadata($st->metadata) + : [] + ] + ); + } + + /** + * {@inheritdoc} + */ + public function getMetadata() + { + return $this->stream->getMetadata(); + } + + /** + * {@inheritdoc} + */ + public function getTrailingMetadata() + { + return $this->stream->getTrailingMetadata(); + } + + /** + * {@inheritdoc} + */ + public function getPeer() + { + return $this->stream->getPeer(); + } + + /** + * {@inheritdoc} + */ + public function cancel() + { + $this->stream->cancel(); + } + + /** + * {@inheritdoc} + */ + public function setCallCredentials($call_credentials) + { + $this->stream->setCallCredentials($call_credentials); + } +} diff --git a/src/Transport/GrpcTransport.php b/src/Transport/GrpcTransport.php index f227e0ceb..4bf5aa549 100644 --- a/src/Transport/GrpcTransport.php +++ b/src/Transport/GrpcTransport.php @@ -41,6 +41,7 @@ use Google\ApiCore\ServerStream; use Google\ApiCore\ServiceAddressTrait; use Google\ApiCore\Transport\Grpc\UnaryInterceptorInterface; +use Google\ApiCore\Transport\Grpc\ServerStreamingCallWrapper; use Google\ApiCore\ValidationException; use Google\ApiCore\ValidationTrait; use Google\Rpc\Code; @@ -195,14 +196,16 @@ public function startServerStreamingCall(Call $call, array $options) throw new \InvalidArgumentException('A message is required for ServerStreaming calls.'); } + // This simultaenously creates and starts a \Grpc\ServerStreamingCall. + $stream = $this->_serverStreamRequest( + '/' . $call->getMethod(), + $message, + [$call->getDecodeType(), 'decode'], + isset($options['headers']) ? $options['headers'] : [], + $this->getCallOptions($options) + ); return new ServerStream( - $this->_serverStreamRequest( - '/' . $call->getMethod(), - $message, - [$call->getDecodeType(), 'decode'], - isset($options['headers']) ? $options['headers'] : [], - $this->getCallOptions($options) - ), + new ServerStreamingCallWrapper($stream), $call->getDescriptor() ); } diff --git a/src/Transport/Rest/JsonStreamDecoder.php b/src/Transport/Rest/JsonStreamDecoder.php index 7197d1152..45995ae9e 100644 --- a/src/Transport/Rest/JsonStreamDecoder.php +++ b/src/Transport/Rest/JsonStreamDecoder.php @@ -33,11 +33,13 @@ namespace Google\ApiCore\Transport\Rest; use Psr\Http\Message\StreamInterface; +use RuntimeException; class JsonStreamDecoder { const ESCAPE_CHAR = '\\'; private $stream; + private $closeCalled = false; private $decodeType; private $ignoreUnknown = true; private $readChunkSize = 1024; @@ -83,10 +85,30 @@ public function __construct(StreamInterface $stream, $decodeType, $options = []) * completes. Throws an Exception if the stream is closed before the closing * byte is read or if it encounters an error while decoding a message. * - * @throws Exception + * @throws RuntimeException * @return \Generator */ public function decode() + { + try { + foreach ($this->_decode() as $response) { + yield $response; + } + } catch (RuntimeException $re) { + $msg = $re->getMessage(); + $streamClosedException = + strpos($msg, 'Stream is detached') !== false || + strpos($msg, 'Unexpected stream close') !== false; + + // Only throw the exception if close() was not called and it was not + // a closing-related exception. + if (!$this->closeCalled || !$streamClosedException) { + throw $re; + } + } + } + + private function _decode() { $decodeType = $this->decodeType; $str = false; @@ -115,6 +137,11 @@ public function decode() $str = !$str; } + // Skip over new lines that break up items. + if ($b === "\n" && $level === 1) { + $start++; + } + // Ignore commas separating messages in the stream array. if ($b === ',' && $level === 1) { $start++; @@ -129,14 +156,22 @@ public function decode() $start++; } } - // Track the closing of an array or object if not in a string - // value. - if (($b === '}' || $b === ']') && !$str) { + // Track the closing of an object if not in a string value. + if ($b === '}' && !$str) { $level--; if ($level === 1) { $end = $cursor+1; } } + // Track the closing of an array if not in a string value. + if ($b === ']' && !$str) { + $level--; + // If we are seeing a closing square bracket at the + // response message level, e.g. {"foo], there is a problem. + if ($level === 1) { + throw new \RuntimeException('Received closing byte mid-message'); + } + } // A message-closing byte was just buffered. Decode the // message with the decode type, clearing the messageBuffer, @@ -180,7 +215,17 @@ public function decode() } } if ($level > 0) { - throw new \Exception('Unexpected stream close before receiving the closing byte'); + throw new \RuntimeException('Unexpected stream close before receiving the closing byte'); } } + + /** + * Closes the underlying stream. If the stream is actively being decoded, an + * exception will not be thrown due to the interruption. + */ + public function close() + { + $this->closeCalled = true; + $this->stream->close(); + } } diff --git a/src/Transport/Rest/RestServerStreamingCall.php b/src/Transport/Rest/RestServerStreamingCall.php new file mode 100644 index 000000000..4e0685a96 --- /dev/null +++ b/src/Transport/Rest/RestServerStreamingCall.php @@ -0,0 +1,182 @@ +httpHandler = $httpHandler; + $this->decodeType = $decodeType; + $this->decoderOptions = $decoderOptions; + } + + /** + * {@inheritdoc} + */ + public function start($request, array $headers = [], array $callOptions = []) + { + $this->originalRequest = $this->appendHeaders($request, $headers); + + try { + $handler = $this->httpHandler; + $response = $handler( + $this->originalRequest, + $callOptions + )->wait(); + } catch (\Exception $ex) { + if ($ex instanceof RequestException && $ex->hasResponse()) { + $ex = ApiException::createFromRequestException($ex, /* isStream */ true); + } + throw $ex; + } + + // Create an OK Status for a successful request just so that it + // has a return value. + $this->status = new Status( + [ + 'code' => Code::OK, + 'message' => ApiStatus::OK, + 'details' => [] + ] + ); + $this->response = $response; + } + + private function appendHeaders($request, $headers) + { + foreach ($headers as $key => $value) { + $request = $request->hasHeader($key) ? + $request->withAddedHeader($key, $value) : + $request->withHeader($key, $value); + } + + return $request; + } + + /** + * {@inheritdoc} + */ + public function responses() + { + if (is_null($this->response)) { + throw new \Exception('Stream has not been started.'); + } + + // Decode the stream and yield responses as they are read. + $this->decoder = new JsonStreamDecoder( + $this->response->getBody(), + $this->decodeType, + $this->decoderOptions + ); + + foreach ($this->decoder->decode() as $message) { + yield $message; + } + } + + /** + * Return the status of the server stream. If the call has not been started + * this will be null. + * + * @return \Google\Rpc\Status The status, with integer $code, string + * $details, and array $metadata members + */ + public function getStatus() + { + return $this->status; + } + + /** + * {@inheritdoc} + */ + public function getMetadata() + { + return is_null($this->response) ? null : $this->response->getHeaders(); + } + + /** + * The Rest transport does not support trailing metadata. This is a + * passthrough to getMetadata(). + */ + public function getTrailingMetadata() + { + return $this->getMetadata(); + } + + /** + * {@inheritdoc} + */ + public function getPeer() + { + return $this->originalRequest->getUri(); + } + + /** + * {@inheritdoc} + */ + public function cancel() + { + if (!is_null($this->decoder)) { + $this->decoder->close(); + } + } + + /** + * For the REST transport this is a no-op. + */ + public function setCallCredentials($call_credentials) + { + // Do nothing. + } +} diff --git a/src/Transport/RestTransport.php b/src/Transport/RestTransport.php index c9e6780e1..4ec5fba56 100644 --- a/src/Transport/RestTransport.php +++ b/src/Transport/RestTransport.php @@ -32,10 +32,11 @@ namespace Google\ApiCore\Transport; use Google\ApiCore\ApiException; -use Google\ApiCore\ApiStatus; use Google\ApiCore\Call; use Google\ApiCore\RequestBuilder; +use Google\ApiCore\ServerStream; use Google\ApiCore\ServiceAddressTrait; +use Google\ApiCore\Transport\Rest\RestServerStreamingCall; use Google\ApiCore\ValidationException; use Google\ApiCore\ValidationTrait; use Google\Protobuf\Internal\Message; @@ -134,7 +135,7 @@ function (ResponseInterface $response) use ($call, $options) { }, function (\Exception $ex) { if ($ex instanceof RequestException && $ex->hasResponse()) { - throw $this->convertToApiException($ex); + throw ApiException::createFromRequestException($ex); } throw $ex; @@ -142,6 +143,71 @@ function (\Exception $ex) { ); } + /** + * {@inheritdoc} + */ + public function startServerStreamingCall(Call $call, array $options) + { + $message = $call->getMessage(); + if (!$message) { + throw new \InvalidArgumentException('A message is required for ServerStreaming calls.'); + } + + $headers = self::buildCommonHeaders($options); + $callOptions = $this->getCallOptions($options); + $request = $this->requestBuilder->build( + $call->getMethod(), + $call->getMessage() + // Exclude headers here because they will be added in _serverStreamRequest(). + ); + + $decoderOptions = []; + if (isset($options['decoderOptions'])) { + $decoderOptions = $options['decoderOptions']; + } + + return new ServerStream( + $this->_serverStreamRequest( + $this->httpHandler, + $request, + $headers, + $call->getDecodeType(), + $callOptions, + $decoderOptions + ), + $call->getDescriptor() + ); + } + + /** + * Creates and starts a RestServerStreamingCall. + * + * @param callable $httpHandler The HTTP Handler to invoke the request with. + * @param RequestInterface $request The request to invoke. + * @param array $headers The headers to include in the request. + * @param string $decodeType The response stream message type to decode. + * @param array $callOptions The call options to use when making the call. + * @param array $decoderOptions The options to use for the JsonStreamDecoder. + * + */ + private function _serverStreamRequest( + $httpHandler, + $request, + $headers, + $decodeType, + $callOptions, + $decoderOptions = [] + ) { + $call = new RestServerStreamingCall( + $httpHandler, + $decodeType, + $decoderOptions + ); + $call->start($request, $headers, $callOptions); + + return $call; + } + private function getCallOptions(array $options) { $callOptions = isset($options['transportOptions']['restOptions']) @@ -160,26 +226,4 @@ private function getCallOptions(array $options) return $callOptions; } - - /** - * @param RequestException $ex - * @return ApiException - * @throws ValidationException - */ - private function convertToApiException(RequestException $ex) - { - $res = $ex->getResponse(); - $body = (string) $res->getBody(); - if ($error = json_decode($body, true)['error']) { - $basicMessage = $error['message']; - $code = isset($error['status']) - ? ApiStatus::rpcCodeFromStatus($error['status']) - : $ex->getCode(); - $metadata = isset($error['details']) ? $error['details'] : null; - return ApiException::createFromRestApiResponse($basicMessage, $code, $metadata); - } - // Use the RPC code instead of the HTTP Status Code. - $code = ApiStatus::rpcCodeFromHttpStatusCode($res->getStatusCode()); - return ApiException::createFromApiResponse($body, $code); - } } diff --git a/tests/Tests/Unit/ApiExceptionTest.php b/tests/Tests/Unit/ApiExceptionTest.php index 806a1acfb..48ea7005f 100644 --- a/tests/Tests/Unit/ApiExceptionTest.php +++ b/tests/Tests/Unit/ApiExceptionTest.php @@ -44,6 +44,9 @@ use Google\Rpc\ResourceInfo; use Google\Rpc\RetryInfo; use Google\Rpc\Status; +use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Psr7\Request; +use GuzzleHttp\Psr7\Response; use PHPUnit\Framework\TestCase; class ApiExceptionTest extends TestCase @@ -243,4 +246,45 @@ public function getRpcStatusData() ] ]; } + + /** + * @dataProvider buildRequestExceptions + */ + public function testCreateFromRequestException($re, $stream, $expectedCode) + { + + $ae = ApiException::createFromRequestException($re, $stream); + $this->assertSame($expectedCode, $ae->getCode()); + } + + public function buildRequestExceptions() + { + $error = [ + 'error' => [ + 'status' => 'NOT_FOUND', + 'message' => 'Ruh-roh.', + ] + ]; + $stream = RequestException::create( + new Request('POST', 'http://www.example.com'), + new Response( + 404, + [], + json_encode([$error]) + ) + ); + $unary = RequestException::create( + new Request('POST', 'http://www.example.com'), + new Response( + 404, + [], + json_encode($error) + ) + ); + + return [ + [$stream, true, Code::NOT_FOUND], + [$unary, false, Code::NOT_FOUND] + ]; + } } diff --git a/tests/Tests/Unit/Transport/Rest/JsonStreamDecoderTest.php b/tests/Tests/Unit/Transport/Rest/JsonStreamDecoderTest.php index b85933a8a..3785b8d7f 100644 --- a/tests/Tests/Unit/Transport/Rest/JsonStreamDecoderTest.php +++ b/tests/Tests/Unit/Transport/Rest/JsonStreamDecoderTest.php @@ -32,6 +32,7 @@ namespace Google\ApiCore\Tests\Unit\Transport\Rest; +use Google\ApiCore\Testing\MockResponse; use Google\ApiCore\Tests\Unit\TestTrait; use Google\ApiCore\Transport\Rest\JsonStreamDecoder; use Google\LongRunning\Operation; @@ -49,7 +50,8 @@ class JsonStreamDecoderTest extends TestCase /** * @dataProvider buildResponseStreams */ - public function testJsonStreamDecoder(array $responses, $decodeType, $stream, $readChunkSizeBytes) { + public function testJsonStreamDecoder(array $responses, $decodeType, $stream, $readChunkSizeBytes) + { $decoder = new JsonStreamDecoder($stream, $decodeType, ['readChunkSizeBytes' => $readChunkSizeBytes]); $num = 0; foreach($decoder->decode() as $op) { @@ -59,7 +61,8 @@ public function testJsonStreamDecoder(array $responses, $decodeType, $stream, $r $this->assertEquals(count($responses), $num); } - public function buildResponseStreams() { + public function buildResponseStreams() + { $any = new Any(); $any->pack(new Operation([ 'name' => 'any_metadata', @@ -120,7 +123,8 @@ public function buildResponseStreams() { ]; } - private function messagesToStream(array $messages) { + private function messagesToStream(array $messages) + { $data = []; foreach($messages as $message) { $data[] = $message->serializeToJsonString(); @@ -128,13 +132,46 @@ private function messagesToStream(array $messages) { return Psr7\Utils::streamFor('['.implode(',', $data).']'); } + /** + * @dataProvider buildAlternateStreams + */ + public function testJsonStreamDecoderAlternate(array $responses, $decodeType, $stream) + { + $decoder = new JsonStreamDecoder($stream, $decodeType); + $num = 0; + foreach($decoder->decode() as $op) { + $this->assertEquals($responses[$num], $op); + $num++; + } + $this->assertEquals(count($responses), $num); + } + + public function buildAlternateStreams() + { + $res1 = new MockResponse(['name' => 'foo']); + $res1Str = $res1->serializeToJsonString(); + $res2 = new MockResponse(['name' => 'bar']); + $res2Str = $res2->serializeToJsonString(); + $responses = [$res1, $res2]; + + $newlines = "[\n\n\n".$res1Str."\n\n\n,\n\n\n".$res2Str."\n\n\n]"; + $commas = "[".$res1Str.",\n,\n,\n,".$res2Str."]"; + $blankspace = "[".$res1Str.", ".$res2Str."]"; + + return [ + [$responses, MockResponse::class, $this->initBufferStream($newlines)], + [$responses, MockResponse::class, $this->initBufferStream($commas)], + [$responses, MockResponse::class, $this->initBufferStream($blankspace)] + ]; + } + /** * @dataProvider buildBadPayloads - * @expectedException \Exception + * @expectedException \RuntimeException */ - public function testJsonStreamDecoderBadClose($payload) { - $stream = new BufferStream(); - $stream->write($payload); + public function testJsonStreamDecoderBadClose($payload) + { + $stream = $this->initBufferStream($payload); $decoder = new JsonStreamDecoder($stream, Operation::class, ['readChunkSizeBytes' => 10]); foreach($decoder->decode() as $op) { $this->assertSame('foo', $op->getName()); @@ -142,7 +179,8 @@ public function testJsonStreamDecoderBadClose($payload) { } } - public function buildBadPayloads() { + public function buildBadPayloads() + { return [ ['[{"name": "foo"},{'], @@ -151,4 +189,26 @@ public function buildBadPayloads() { ['[{"name": "foo"},{]'], ]; } + + public function testJsonStreamDecoderClosed() + { + $stream = new BufferStream(); + $stream->write('[{"name": "foo"},{'); + $decoder = new JsonStreamDecoder($stream, Operation::class, ['readChunkSizeBytes' => 10]); + $count = 0; + foreach($decoder->decode() as $op) { + $this->assertEquals('foo', $op->getName()); + $count++; + $decoder->close(); + } + + $this->assertEquals(1, $count); + } + + private function initBufferStream($data) + { + $stream = new BufferStream(); + $stream->write($data); + return $stream; + } } diff --git a/tests/Tests/Unit/Transport/RestTransportTest.php b/tests/Tests/Unit/Transport/RestTransportTest.php index 40d7b414c..36238e931 100644 --- a/tests/Tests/Unit/Transport/RestTransportTest.php +++ b/tests/Tests/Unit/Transport/RestTransportTest.php @@ -32,6 +32,7 @@ namespace Google\ApiCore\Tests\Unit\Transport; +use Google\ApiCore\ApiException; use Google\ApiCore\CredentialsWrapper; use Google\ApiCore\Call; use Google\ApiCore\RequestBuilder; @@ -40,6 +41,8 @@ use Google\ApiCore\Testing\MockResponse; use Google\ApiCore\Transport\RestTransport; use Google\Auth\HttpHandler\HttpHandlerFactory; +use Google\Protobuf\Any; +use Google\Rpc\ErrorInfo; use GuzzleHttp\Exception\RequestException; use GuzzleHttp\Promise\Create; use GuzzleHttp\Psr7\Request; @@ -161,6 +164,143 @@ public function testStartUnaryCallThrowsRequestException() ->startUnaryCall($this->call, []) ->wait(); } + /** + * @dataProvider buildServerStreamMessages + */ + public function testStartServerStreamingCall($messages) + { + $apiEndpoint = 'www.example.com'; + $expectedRequest = new Request( + 'POST', + $apiEndpoint, + [], + "" + ); + + $httpHandler = function (RequestInterface $request, array $options = []) use ($messages, $expectedRequest) { + $this->assertEquals($expectedRequest, $request); + return Create::promiseFor( + new Response( + 200, + [], + $this->encodeMessages($messages) + ) + ); + }; + + $stream = $this->getTransport($httpHandler, $apiEndpoint) + ->startServerStreamingCall($this->call, []); + + $num = 0; + foreach($stream->readAll() as $m) { + $this->assertEquals($messages[$num], $m); + $num++; + } + $this->assertEquals(count($messages), $num); + } + + /** + * @dataProvider buildServerStreamMessages + */ + public function testCancelServerStreamingCall($messages) + { + $apiEndpoint = 'www.example.com'; + $expectedRequest = new Request( + 'POST', + $apiEndpoint, + [], + "" + ); + + $httpHandler = function (RequestInterface $request, array $options = []) use ($messages, $expectedRequest) { + $this->assertEquals($expectedRequest, $request); + return Create::promiseFor( + new Response( + 200, + [], + $this->encodeMessages($messages) + ) + ); + }; + + $stream = $this->getTransport($httpHandler, $apiEndpoint) + ->startServerStreamingCall($this->call, []); + + $num = 0; + foreach($stream->readAll() as $m) { + $this->assertEquals($messages[$num], $m); + $num++; + + // Intentionally cancel the stream mid way through processing. + $stream->getServerStreamingCall()->cancel(); + } + + // Ensure only one message was ever yielded. + $this->assertEquals(1, $num); + } + + private function encodeMessages(array $messages) { + $data = []; + foreach($messages as $message) { + $data[] = $message->serializeToJsonString(); + } + return '['.implode(',', $data).']'; + } + + public function buildServerStreamMessages() { + return[ + [ + [ + new MockResponse([ + 'name' => 'foo', + 'number' => 1, + ]), + new MockResponse([ + 'name' => 'bar', + 'number' => 2, + ]), + new MockResponse([ + 'name' => 'baz', + 'number' => 3, + ]), + ] + ] + ]; + } + + /** + * @expectedException \Google\ApiCore\ApiException + * @expectedExceptionCode 5 + * @expectedExceptionMessage Ruh-roh. + */ + public function testStartServerStreamingCallThrowsRequestException() + { + $apiEndpoint = 'http://www.example.com'; + $errorInfo = new Any(); + $errorInfo->pack(new ErrorInfo(['domain' => 'googleapis.com'])); + $httpHandler = function (RequestInterface $request, array $options = []) use ($apiEndpoint, $errorInfo) { + return Create::rejectionFor( + RequestException::create( + new Request('POST', $apiEndpoint), + new Response( + 404, + [], + json_encode([[ + 'error' => [ + 'status' => 'NOT_FOUND', + 'message' => 'Ruh-roh.', + 'details' => [$errorInfo] + ] + ]]) + ) + ) + ); + }; + + $this->getTransport($httpHandler, $apiEndpoint) + ->startServerStreamingCall($this->call, []); + + } /** * @dataProvider buildDataRest From 64e0bd3fdabf41588052c4bcd038981438bda746 Mon Sep 17 00:00:00 2001 From: alicejli Date: Wed, 1 Dec 2021 14:11:40 +0000 Subject: [PATCH 06/17] update REST --- src/Transport/RestTransport.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/Transport/RestTransport.php b/src/Transport/RestTransport.php index 4ec5fba56..6c5d6389d 100644 --- a/src/Transport/RestTransport.php +++ b/src/Transport/RestTransport.php @@ -226,4 +226,26 @@ private function getCallOptions(array $options) return $callOptions; } + + /** + * @param RequestException $ex + * @return ApiException + * @throws ValidationException + */ + private function convertToApiException(RequestException $ex) + { + $res = $ex->getResponse(); + $body = (string) $res->getBody(); + if ($error = json_decode($body, true)['error']) { + $basicMessage = $error['message']; + $code = isset($error['status']) + ? ApiStatus::rpcCodeFromStatus($error['status']) + : $ex->getCode(); + $metadata = isset($error['details']) ? $error['details'] : null; + return ApiException::createFromApiResponseREST($basicMessage, $code, $metadata); + } + // Use the RPC code instead of the HTTP Status Code. + $code = ApiStatus::rpcCodeFromHttpStatusCode($res->getStatusCode()); + return ApiException::createFromApiResponse($body, $code); + } } From 6392ca6726cfbf62c7ae27ef724666270cd8156e Mon Sep 17 00:00:00 2001 From: alicejli Date: Wed, 1 Dec 2021 14:26:16 +0000 Subject: [PATCH 07/17] update ApiException --- src/ApiException.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ApiException.php b/src/ApiException.php index fc8c11f51..024e11ff1 100644 --- a/src/ApiException.php +++ b/src/ApiException.php @@ -218,7 +218,7 @@ public static function createFromRequestException(RequestException $ex, $isStrea ? ApiStatus::rpcCodeFromStatus($error['status']) : $ex->getCode(); $metadata = isset($error['details']) ? $error['details'] : null; - return static::createFromApiResponse($basicMessage, $code, $metadata); + return static::createFromRestApiResponse($basicMessage, $code, $metadata); } // Use the RPC code instead of the HTTP Status Code. $code = ApiStatus::rpcCodeFromHttpStatusCode($res->getStatusCode()); From 487a06f238a7575f1c497ccaa5bfec9c5ec86149 Mon Sep 17 00:00:00 2001 From: alicejli Date: Wed, 1 Dec 2021 15:41:50 +0000 Subject: [PATCH 08/17] updating Rest tests --- src/Transport/RestTransport.php | 22 ------ tests/Tests/Unit/ApiExceptionTest.php | 109 ++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 27 deletions(-) diff --git a/src/Transport/RestTransport.php b/src/Transport/RestTransport.php index 6c5d6389d..4ec5fba56 100644 --- a/src/Transport/RestTransport.php +++ b/src/Transport/RestTransport.php @@ -226,26 +226,4 @@ private function getCallOptions(array $options) return $callOptions; } - - /** - * @param RequestException $ex - * @return ApiException - * @throws ValidationException - */ - private function convertToApiException(RequestException $ex) - { - $res = $ex->getResponse(); - $body = (string) $res->getBody(); - if ($error = json_decode($body, true)['error']) { - $basicMessage = $error['message']; - $code = isset($error['status']) - ? ApiStatus::rpcCodeFromStatus($error['status']) - : $ex->getCode(); - $metadata = isset($error['details']) ? $error['details'] : null; - return ApiException::createFromApiResponseREST($basicMessage, $code, $metadata); - } - // Use the RPC code instead of the HTTP Status Code. - $code = ApiStatus::rpcCodeFromHttpStatusCode($res->getStatusCode()); - return ApiException::createFromApiResponse($body, $code); - } } diff --git a/tests/Tests/Unit/ApiExceptionTest.php b/tests/Tests/Unit/ApiExceptionTest.php index 48ea7005f..4a5a7f02b 100644 --- a/tests/Tests/Unit/ApiExceptionTest.php +++ b/tests/Tests/Unit/ApiExceptionTest.php @@ -184,7 +184,8 @@ public function getMetadata() /** * @dataProvider getMetadata */ - public function testCreateFromApiResponse($metadata, $metadataArray) { + public function testCreateFromApiResponse($metadata, $metadataArray) + { $basicMessage = 'testWithMetadata'; $code = Code::OK; $status = 'OK'; @@ -203,10 +204,105 @@ public function testCreateFromApiResponse($metadata, $metadataArray) { $this->assertSame($metadata, $apiException->getMetadata()); } + public function getRestMetadata() + { + $unknownBinData = [ + [ + '@type' => 'unknown-bin', + 'data' => '' + ] + ]; + $asciiData = [ + [ + '@type' => 'ascii', + 'data' => 'ascii-data' + ] + ]; + $retryInfoData = [ + [ + '@type' => 'google.rpc.retryinfo-bin', + 'retryDelay' => [ + 'seconds' => 1, + 'nanos' => 2, + ], + ] + ]; + $allKnownTypesData = [ + [ + '@type' => 'google.rpc.retryinfo-bin', + ], + [ + '@type' => 'google.rpc.debuginfo-bin', + "stackEntries" => [], + "detail" => "" + ], + [ + '@type' => 'google.rpc.quotafailure-bin', + 'violations' => [], + ], + [ + '@type' => 'google.rpc.badrequest-bin', + 'fieldViolations' => [] + ], + [ + '@type' => 'google.rpc.requestinfo-bin', + 'requestId' => '', + 'servingData' => '', + ], + [ + '@type' => 'google.rpc.resourceinfo-bin', + 'resourceType' => '', + 'resourceName' => '', + 'owner' => '', + 'description' => '', + ], + [ + '@type' => 'google.rpc.help-bin', + 'links' => [], + ], + [ + '@type' => 'google.rpc.localizedmessage-bin', + 'locale' => '', + 'message' => '', + ], + ]; + + return [ + [ + [[]], [[]], + [$unknownBinData, $unknownBinData], + [$asciiData, $asciiData], + [$allKnownTypesData, $allKnownTypesData] + ] + ]; + } + + /** + * @dataProvider getRestMetadata + */ + public function testCreateFromRestApiResponse($metadata) + { + $basicMessage = 'testWithRestMetadata'; + $code = Code::OK; + $status = 'OK'; + + $apiException = ApiException::createFromRestApiResponse($basicMessage, $code, $metadata); + + $expectedMessage = json_encode([ + 'message' => $basicMessage, + 'code' => $code, + 'status' => $status, + 'details' => $metadata + ], JSON_PRETTY_PRINT); + + $this->assertSame($expectedMessage, $apiException->getMessage()); + } + /** * @dataProvider getRpcStatusData */ - public function testCreateFromRpcStatus($status, $expectedApiException) { + public function testCreateFromRpcStatus($status, $expectedApiException) + { $actualApiException = ApiException::createFromRpcStatus($status); $this->assertEquals($expectedApiException, $actualApiException); } @@ -238,7 +334,11 @@ public function getRpcStatusData() return [ [ $status, - new ApiException($expectedMessage, Code::OK, 'OK', [ + new ApiException( + $expectedMessage, + Code::OK, + 'OK', + [ 'metadata' => $status->getDetails(), 'basicMessage' => $status->getMessage(), ] @@ -252,13 +352,12 @@ public function getRpcStatusData() */ public function testCreateFromRequestException($re, $stream, $expectedCode) { - $ae = ApiException::createFromRequestException($re, $stream); $this->assertSame($expectedCode, $ae->getCode()); } public function buildRequestExceptions() - { + { $error = [ 'error' => [ 'status' => 'NOT_FOUND', From 71ab9e118d8c440e8a988e496c6a87a97f41ba2d Mon Sep 17 00:00:00 2001 From: alicejli Date: Wed, 1 Dec 2021 16:02:13 +0000 Subject: [PATCH 09/17] rebasing From 9f6517c05209055ae176496703ae7f089f7d09f4 Mon Sep 17 00:00:00 2001 From: alicejli Date: Mon, 29 Nov 2021 20:20:37 +0000 Subject: [PATCH 10/17] fix: update REST transport error details --- src/ApiException.php | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/ApiException.php b/src/ApiException.php index 991e965bf..fcb065f92 100644 --- a/src/ApiException.php +++ b/src/ApiException.php @@ -115,6 +115,39 @@ public static function createFromApiResponse( ); } + /** + * @param string $basicMessage + * @param int $rpcCode + * @param array|null $metadata + * @param \Exception $previous + * @return ApiException + */ + public static function createFromApiResponseREST( + $basicMessage, + $rpcCode, + array $metadata = null, + \Exception $previous = null + ) { + if (empty($metadata)) { + $decodedMetadata = []; + return self::create( + $basicMessage, + $rpcCode, + $metadata, + $decodedMetadata, + $previous + ); + } else { + return self::create( + $basicMessage, + $rpcCode, + $metadata, + $metadata, + $previous + ); + } + } + /** * Construct an ApiException with a useful message, including decoded metadata. * From 95e2a094046ce483175a76880b23d057ef84e360 Mon Sep 17 00:00:00 2001 From: alicejli Date: Mon, 29 Nov 2021 22:54:17 +0000 Subject: [PATCH 11/17] update name and cleanup function --- src/ApiException.php | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/ApiException.php b/src/ApiException.php index fcb065f92..fc8c11f51 100644 --- a/src/ApiException.php +++ b/src/ApiException.php @@ -116,13 +116,15 @@ public static function createFromApiResponse( } /** + * For REST-based responses, the metadata does not need to be decoded. + * * @param string $basicMessage * @param int $rpcCode * @param array|null $metadata * @param \Exception $previous * @return ApiException */ - public static function createFromApiResponseREST( + public static function createFromRestApiResponse( $basicMessage, $rpcCode, array $metadata = null, @@ -137,15 +139,14 @@ public static function createFromApiResponseREST( $decodedMetadata, $previous ); - } else { - return self::create( - $basicMessage, - $rpcCode, - $metadata, - $metadata, - $previous - ); } + return self::create( + $basicMessage, + $rpcCode, + $metadata, + $metadata, + $previous + ); } /** From 23d3b18380a2371cd41d8569332f8245a5143616 Mon Sep 17 00:00:00 2001 From: alicejli Date: Tue, 30 Nov 2021 16:22:36 +0000 Subject: [PATCH 12/17] update tests to not use deprecated functions --- tests/Tests/Unit/Transport/RestTransportTest.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/Tests/Unit/Transport/RestTransportTest.php b/tests/Tests/Unit/Transport/RestTransportTest.php index 17351d64b..36238e931 100644 --- a/tests/Tests/Unit/Transport/RestTransportTest.php +++ b/tests/Tests/Unit/Transport/RestTransportTest.php @@ -40,7 +40,6 @@ use Google\ApiCore\Testing\MockRequest; use Google\ApiCore\Testing\MockResponse; use Google\ApiCore\Transport\RestTransport; -use Google\Auth\FetchAuthTokenInterface; use Google\Auth\HttpHandler\HttpHandlerFactory; use Google\Protobuf\Any; use Google\Rpc\ErrorInfo; @@ -426,7 +425,9 @@ public function testAudienceOption() $credentialsWrapper = $this->prophesize(CredentialsWrapper::class); $credentialsWrapper->getAuthorizationHeaderCallback('an-audience') ->shouldBeCalledOnce() - ->willReturn(function() { return []; }); + ->willReturn(function () { + return []; + }); $options = [ 'audience' => 'an-audience', @@ -465,7 +466,9 @@ public function testNonArrayAuthorizationHeaderThrowsException() $credentialsWrapper = $this->prophesize(CredentialsWrapper::class); $credentialsWrapper->getAuthorizationHeaderCallback(null) ->shouldBeCalledOnce() - ->willReturn(function() { return ''; }); + ->willReturn(function () { + return ''; + }); $options = [ 'credentialsWrapper' => $credentialsWrapper->reveal(), From a9d7088d6cbe2b9780675dc63978ea88df357248 Mon Sep 17 00:00:00 2001 From: alicejli Date: Wed, 1 Dec 2021 14:11:40 +0000 Subject: [PATCH 13/17] update REST --- src/Transport/RestTransport.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/Transport/RestTransport.php b/src/Transport/RestTransport.php index 4ec5fba56..6c5d6389d 100644 --- a/src/Transport/RestTransport.php +++ b/src/Transport/RestTransport.php @@ -226,4 +226,26 @@ private function getCallOptions(array $options) return $callOptions; } + + /** + * @param RequestException $ex + * @return ApiException + * @throws ValidationException + */ + private function convertToApiException(RequestException $ex) + { + $res = $ex->getResponse(); + $body = (string) $res->getBody(); + if ($error = json_decode($body, true)['error']) { + $basicMessage = $error['message']; + $code = isset($error['status']) + ? ApiStatus::rpcCodeFromStatus($error['status']) + : $ex->getCode(); + $metadata = isset($error['details']) ? $error['details'] : null; + return ApiException::createFromApiResponseREST($basicMessage, $code, $metadata); + } + // Use the RPC code instead of the HTTP Status Code. + $code = ApiStatus::rpcCodeFromHttpStatusCode($res->getStatusCode()); + return ApiException::createFromApiResponse($body, $code); + } } From b3c3c24a2b3fa37279104fd06103981c275b30ca Mon Sep 17 00:00:00 2001 From: alicejli Date: Wed, 1 Dec 2021 14:26:16 +0000 Subject: [PATCH 14/17] update ApiException --- src/ApiException.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ApiException.php b/src/ApiException.php index fc8c11f51..024e11ff1 100644 --- a/src/ApiException.php +++ b/src/ApiException.php @@ -218,7 +218,7 @@ public static function createFromRequestException(RequestException $ex, $isStrea ? ApiStatus::rpcCodeFromStatus($error['status']) : $ex->getCode(); $metadata = isset($error['details']) ? $error['details'] : null; - return static::createFromApiResponse($basicMessage, $code, $metadata); + return static::createFromRestApiResponse($basicMessage, $code, $metadata); } // Use the RPC code instead of the HTTP Status Code. $code = ApiStatus::rpcCodeFromHttpStatusCode($res->getStatusCode()); From dda3a7260c5f7ea9ae11620b1f99f006d722f968 Mon Sep 17 00:00:00 2001 From: alicejli Date: Wed, 1 Dec 2021 15:41:50 +0000 Subject: [PATCH 15/17] updating Rest tests --- src/Transport/RestTransport.php | 22 ------ tests/Tests/Unit/ApiExceptionTest.php | 109 ++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 27 deletions(-) diff --git a/src/Transport/RestTransport.php b/src/Transport/RestTransport.php index 6c5d6389d..4ec5fba56 100644 --- a/src/Transport/RestTransport.php +++ b/src/Transport/RestTransport.php @@ -226,26 +226,4 @@ private function getCallOptions(array $options) return $callOptions; } - - /** - * @param RequestException $ex - * @return ApiException - * @throws ValidationException - */ - private function convertToApiException(RequestException $ex) - { - $res = $ex->getResponse(); - $body = (string) $res->getBody(); - if ($error = json_decode($body, true)['error']) { - $basicMessage = $error['message']; - $code = isset($error['status']) - ? ApiStatus::rpcCodeFromStatus($error['status']) - : $ex->getCode(); - $metadata = isset($error['details']) ? $error['details'] : null; - return ApiException::createFromApiResponseREST($basicMessage, $code, $metadata); - } - // Use the RPC code instead of the HTTP Status Code. - $code = ApiStatus::rpcCodeFromHttpStatusCode($res->getStatusCode()); - return ApiException::createFromApiResponse($body, $code); - } } diff --git a/tests/Tests/Unit/ApiExceptionTest.php b/tests/Tests/Unit/ApiExceptionTest.php index 48ea7005f..4a5a7f02b 100644 --- a/tests/Tests/Unit/ApiExceptionTest.php +++ b/tests/Tests/Unit/ApiExceptionTest.php @@ -184,7 +184,8 @@ public function getMetadata() /** * @dataProvider getMetadata */ - public function testCreateFromApiResponse($metadata, $metadataArray) { + public function testCreateFromApiResponse($metadata, $metadataArray) + { $basicMessage = 'testWithMetadata'; $code = Code::OK; $status = 'OK'; @@ -203,10 +204,105 @@ public function testCreateFromApiResponse($metadata, $metadataArray) { $this->assertSame($metadata, $apiException->getMetadata()); } + public function getRestMetadata() + { + $unknownBinData = [ + [ + '@type' => 'unknown-bin', + 'data' => '' + ] + ]; + $asciiData = [ + [ + '@type' => 'ascii', + 'data' => 'ascii-data' + ] + ]; + $retryInfoData = [ + [ + '@type' => 'google.rpc.retryinfo-bin', + 'retryDelay' => [ + 'seconds' => 1, + 'nanos' => 2, + ], + ] + ]; + $allKnownTypesData = [ + [ + '@type' => 'google.rpc.retryinfo-bin', + ], + [ + '@type' => 'google.rpc.debuginfo-bin', + "stackEntries" => [], + "detail" => "" + ], + [ + '@type' => 'google.rpc.quotafailure-bin', + 'violations' => [], + ], + [ + '@type' => 'google.rpc.badrequest-bin', + 'fieldViolations' => [] + ], + [ + '@type' => 'google.rpc.requestinfo-bin', + 'requestId' => '', + 'servingData' => '', + ], + [ + '@type' => 'google.rpc.resourceinfo-bin', + 'resourceType' => '', + 'resourceName' => '', + 'owner' => '', + 'description' => '', + ], + [ + '@type' => 'google.rpc.help-bin', + 'links' => [], + ], + [ + '@type' => 'google.rpc.localizedmessage-bin', + 'locale' => '', + 'message' => '', + ], + ]; + + return [ + [ + [[]], [[]], + [$unknownBinData, $unknownBinData], + [$asciiData, $asciiData], + [$allKnownTypesData, $allKnownTypesData] + ] + ]; + } + + /** + * @dataProvider getRestMetadata + */ + public function testCreateFromRestApiResponse($metadata) + { + $basicMessage = 'testWithRestMetadata'; + $code = Code::OK; + $status = 'OK'; + + $apiException = ApiException::createFromRestApiResponse($basicMessage, $code, $metadata); + + $expectedMessage = json_encode([ + 'message' => $basicMessage, + 'code' => $code, + 'status' => $status, + 'details' => $metadata + ], JSON_PRETTY_PRINT); + + $this->assertSame($expectedMessage, $apiException->getMessage()); + } + /** * @dataProvider getRpcStatusData */ - public function testCreateFromRpcStatus($status, $expectedApiException) { + public function testCreateFromRpcStatus($status, $expectedApiException) + { $actualApiException = ApiException::createFromRpcStatus($status); $this->assertEquals($expectedApiException, $actualApiException); } @@ -238,7 +334,11 @@ public function getRpcStatusData() return [ [ $status, - new ApiException($expectedMessage, Code::OK, 'OK', [ + new ApiException( + $expectedMessage, + Code::OK, + 'OK', + [ 'metadata' => $status->getDetails(), 'basicMessage' => $status->getMessage(), ] @@ -252,13 +352,12 @@ public function getRpcStatusData() */ public function testCreateFromRequestException($re, $stream, $expectedCode) { - $ae = ApiException::createFromRequestException($re, $stream); $this->assertSame($expectedCode, $ae->getCode()); } public function buildRequestExceptions() - { + { $error = [ 'error' => [ 'status' => 'NOT_FOUND', From fcb4aa7ab6d5daf6e4530a69738e8dccad16d266 Mon Sep 17 00:00:00 2001 From: alicejli Date: Wed, 1 Dec 2021 16:02:13 +0000 Subject: [PATCH 16/17] rebasing From 2f425e57d00d6b0265d671f055b06e7cf23ec557 Mon Sep 17 00:00:00 2001 From: alicejli Date: Wed, 1 Dec 2021 17:50:52 +0000 Subject: [PATCH 17/17] refactor function and fix tests --- src/ApiException.php | 12 +----------- tests/Tests/Unit/ApiExceptionTest.php | 9 +++++---- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/ApiException.php b/src/ApiException.php index 024e11ff1..8826440a8 100644 --- a/src/ApiException.php +++ b/src/ApiException.php @@ -130,21 +130,11 @@ public static function createFromRestApiResponse( array $metadata = null, \Exception $previous = null ) { - if (empty($metadata)) { - $decodedMetadata = []; - return self::create( - $basicMessage, - $rpcCode, - $metadata, - $decodedMetadata, - $previous - ); - } return self::create( $basicMessage, $rpcCode, $metadata, - $metadata, + is_null($metadata) ? [] : $metadata, $previous ); } diff --git a/tests/Tests/Unit/ApiExceptionTest.php b/tests/Tests/Unit/ApiExceptionTest.php index 4a5a7f02b..6b3260214 100644 --- a/tests/Tests/Unit/ApiExceptionTest.php +++ b/tests/Tests/Unit/ApiExceptionTest.php @@ -269,10 +269,11 @@ public function getRestMetadata() return [ [ - [[]], [[]], - [$unknownBinData, $unknownBinData], - [$asciiData, $asciiData], - [$allKnownTypesData, $allKnownTypesData] + [[]], + [[null]], + [$unknownBinData], + [$asciiData], + [$allKnownTypesData] ] ]; }