Skip to content

Commit e232a4b

Browse files
bshafferbshaffer
committed
Merge pull request #876 from bshaffer/cleans-up-rest-decode-response
cleans up REST::decodeResponse
2 parents c171d86 + 951f387 commit e232a4b

File tree

2 files changed

+230
-28
lines changed

2 files changed

+230
-28
lines changed

‎src/Google/Http/REST.php‎

Lines changed: 60 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -107,44 +107,76 @@ public static function decodeHttpResponse(
107107
RequestInterface $request = null,
108108
$expectedClass = null
109109
) {
110-
$body = (string) $response->getBody();
111110
$code = $response->getStatusCode();
112-
$result = null;
113111

114-
// return raw response when "alt" is "media"
115-
$isJson = !($request && 'media' == $request->getUri()->getQuery('alt'));
112+
// retry strategy
113+
if ((intVal($code)) >= 400) {
114+
// if we errored out, it should be safe to grab the response body
115+
$body = (string) $response->getBody();
116116

117-
// set the result to the body if it's not set to anything else
118-
if ($isJson) {
119-
$result = json_decode($body, true);
120-
if (null === $result && 0 !== json_last_error()) {
121-
// in the event of a parse error, return the raw string
122-
$result = $body;
123-
}
124-
} else {
125-
$result = $body;
117+
// Check if we received errors, and add those to the Exception for convenience
118+
throw new Google_Service_Exception($body, $code, null, self::getResponseErrors($body));
126119
}
127120

128-
// retry strategy
129-
if ((intVal($code)) >= 400) {
130-
$errors = null;
131-
// Specific check for APIs which don't return error details, such as Blogger.
132-
if (isset($result['error']['errors'])) {
133-
$errors = $result['error']['errors'];
134-
}
135-
throw new Google_Service_Exception($body, $code, null, $errors);
121+
// Ensure we only pull the entire body into memory if the request is not
122+
// of media type
123+
$body = self::decodeBody($response, $request);
124+
125+
if ($expectedClass = self::determineExpectedClass($expectedClass, $request)) {
126+
$json = json_decode($body, true);
127+
128+
return new $expectedClass($json);
129+
}
130+
131+
return $response;
132+
}
133+
134+
private static function decodeBody(ResponseInterface $response, RequestInterface $request = null)
135+
{
136+
if (self::isAltMedia($request)) {
137+
// don't decode the body, it's probably a really long string
138+
return '';
139+
}
140+
141+
return (string) $response->getBody();
142+
}
143+
144+
private static function determineExpectedClass($expectedClass, RequestInterface $request = null)
145+
{
146+
// "false" is used to explicitly prevent an expected class from being returned
147+
if (false === $expectedClass) {
148+
return null;
136149
}
137150

138-
// use "is_null" because "false" is used to explicitly
139-
// prevent an expected class from being returned
140-
if (is_null($expectedClass) && $request) {
141-
$expectedClass = $request->getHeaderLine('X-Php-Expected-Class');
151+
// if we don't have a request, we just use what's passed in
152+
if (is_null($request)) {
153+
return $expectedClass;
142154
}
143155

144-
if (!empty($expectedClass)) {
145-
return new $expectedClass($result);
156+
// return what we have in the request header if one was not supplied
157+
return $expectedClass ?: $request->getHeaderLine('X-Php-Expected-Class');
158+
}
159+
160+
private static function getResponseErrors($body)
161+
{
162+
$json = json_decode($body, true);
163+
164+
if (isset($json['error']['errors'])) {
165+
return $json['error']['errors'];
146166
}
147167

148-
return $response;
168+
return null;
169+
}
170+
171+
private static function isAltMedia(RequestInterface $request = null)
172+
{
173+
if ($request && $qs = $request->getUri()->getQuery()) {
174+
parse_str($qs, $query);
175+
if (isset($query['alt']) && $query['alt'] == 'media') {
176+
return true;
177+
}
178+
}
179+
180+
return false;
149181
}
150182
}

‎tests/Google/Service/ResourceTest.php‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use GuzzleHttp\Psr7;
2222
use GuzzleHttp\Psr7\Request;
2323
use GuzzleHttp\Psr7\Response;
24+
use GuzzleHttp\Psr7\Stream;
2425

2526
class Test_Google_Service extends Google_Service
2627
{
@@ -34,6 +35,18 @@ public function __construct(Google_Client $client)
3435
}
3536
}
3637

38+
class Test_MediaType_Stream extends Stream
39+
{
40+
public $toStringCalled = false;
41+
42+
public function __toString()
43+
{
44+
$this->toStringCalled = true;
45+
46+
return parent::__toString();
47+
}
48+
}
49+
3750
class Google_Service_ResourceTest extends BaseTest
3851
{
3952
private $client;
@@ -263,7 +276,164 @@ public function testNoExpectedClassForAltMediaWithHttpFail()
263276
$decoded = $resource->call('testMethod', $arguments, $expectedClass);
264277
$this->fail('should have thrown exception');
265278
} catch (Google_Service_Exception $e) {
279+
// Alt Media on error should return a safe error
266280
$this->assertEquals('thisisnotvalidjson', $e->getMessage());
267281
}
268282
}
283+
284+
public function testErrorResponseWithVeryLongBody()
285+
{
286+
// set the "alt" parameter to "media"
287+
$arguments = [['alt' => 'media']];
288+
$request = new Request('GET', '/?alt=media');
289+
$body = Psr7\stream_for('this will be pulled into memory');
290+
$response = new Response(400, [], $body);
291+
292+
$http = $this->getMockBuilder("GuzzleHttp\Client")
293+
->disableOriginalConstructor()
294+
->getMock();
295+
$http->expects($this->once())
296+
->method('send')
297+
->will($this->returnValue($response));
298+
299+
if ($this->isGuzzle5()) {
300+
$http->expects($this->once())
301+
->method('createRequest')
302+
->will($this->returnValue(new GuzzleHttp\Message\Request('GET', '/?alt=media')));
303+
}
304+
305+
$client = new Google_Client();
306+
$client->setHttpClient($http);
307+
$service = new Test_Google_Service($client);
308+
309+
// set up mock objects
310+
$resource = new Google_Service_Resource(
311+
$service,
312+
"test",
313+
"testResource",
314+
array("methods" =>
315+
array(
316+
"testMethod" => array(
317+
"parameters" => array(),
318+
"path" => "method/path",
319+
"httpMethod" => "POST",
320+
)
321+
)
322+
)
323+
);
324+
325+
try {
326+
$expectedClass = 'ThisShouldBeIgnored';
327+
$decoded = $resource->call('testMethod', $arguments, $expectedClass);
328+
$this->fail('should have thrown exception');
329+
} catch (Google_Service_Exception $e) {
330+
// empty message - alt=media means no message
331+
$this->assertEquals('this will be pulled into memory', $e->getMessage());
332+
}
333+
}
334+
335+
public function testSuccessResponseWithVeryLongBody()
336+
{
337+
// set the "alt" parameter to "media"
338+
$arguments = [['alt' => 'media']];
339+
$request = new Request('GET', '/?alt=media');
340+
$resource = fopen('php://temp', 'r+');
341+
$stream = new Test_MediaType_Stream($resource);
342+
$response = new Response(200, [], $stream);
343+
344+
$http = $this->getMockBuilder("GuzzleHttp\Client")
345+
->disableOriginalConstructor()
346+
->getMock();
347+
$http->expects($this->once())
348+
->method('send')
349+
->will($this->returnValue($response));
350+
351+
if ($this->isGuzzle5()) {
352+
$http->expects($this->once())
353+
->method('createRequest')
354+
->will($this->returnValue(new GuzzleHttp\Message\Request('GET', '/?alt=media')));
355+
}
356+
357+
$client = new Google_Client();
358+
$client->setHttpClient($http);
359+
$service = new Test_Google_Service($client);
360+
361+
// set up mock objects
362+
$resource = new Google_Service_Resource(
363+
$service,
364+
"test",
365+
"testResource",
366+
array("methods" =>
367+
array(
368+
"testMethod" => array(
369+
"parameters" => array(),
370+
"path" => "method/path",
371+
"httpMethod" => "POST",
372+
)
373+
)
374+
)
375+
);
376+
377+
$expectedClass = 'ThisShouldBeIgnored';
378+
$response = $resource->call('testMethod', $arguments, $expectedClass);
379+
380+
$this->assertEquals(200, $response->getStatusCode());
381+
$this->assertFalse($stream->toStringCalled);
382+
}
383+
384+
public function testExceptionMessage()
385+
{
386+
// set the "alt" parameter to "media"
387+
$request = new Request('GET', '/');
388+
$errors = [ ["domain" => "foo"] ];
389+
390+
$body = Psr7\stream_for(json_encode([
391+
'error' => [
392+
'errors' => $errors
393+
]
394+
]));
395+
396+
$response = new Response(400, [], $body);
397+
398+
$http = $this->getMockBuilder("GuzzleHttp\Client")
399+
->disableOriginalConstructor()
400+
->getMock();
401+
$http->expects($this->once())
402+
->method('send')
403+
->will($this->returnValue($response));
404+
405+
if ($this->isGuzzle5()) {
406+
$http->expects($this->once())
407+
->method('createRequest')
408+
->will($this->returnValue(new GuzzleHttp\Message\Request('GET', '/?alt=media')));
409+
}
410+
411+
$client = new Google_Client();
412+
$client->setHttpClient($http);
413+
$service = new Test_Google_Service($client);
414+
415+
// set up mock objects
416+
$resource = new Google_Service_Resource(
417+
$service,
418+
"test",
419+
"testResource",
420+
array("methods" =>
421+
array(
422+
"testMethod" => array(
423+
"parameters" => array(),
424+
"path" => "method/path",
425+
"httpMethod" => "POST",
426+
)
427+
)
428+
)
429+
);
430+
431+
try {
432+
433+
$decoded = $resource->call('testMethod', array(array()));
434+
$this->fail('should have thrown exception');
435+
} catch (Google_Service_Exception $e) {
436+
$this->assertEquals($errors, $e->getErrors());
437+
}
438+
}
269439
}

0 commit comments

Comments
 (0)