From 7a7a32354bb6588aa5855889a402e90730319734 Mon Sep 17 00:00:00 2001 From: Ian Barber Date: Wed, 29 Jan 2014 15:59:27 +0000 Subject: [PATCH 1/4] Handle black error details Fixed #57, by checking for presence of error details before passing them to the exception --- src/Google/Http/REST.php | 8 +++++++- tests/general/RestTest.php | 40 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/Google/Http/REST.php b/src/Google/Http/REST.php index 43d46b1a8..5ea4b7528 100644 --- a/src/Google/Http/REST.php +++ b/src/Google/Http/REST.php @@ -71,7 +71,13 @@ public static function decodeHttpResponse($response) $err .= ": ($code) $body"; } - throw new Google_Service_Exception($err, $code, null, $decoded['error']['errors']); + $errors = null; + // Specific check for APIs which don't return error details, such as Blogger. + if (isset($decoded['error']) && isset($decoded['error']['errors'])) { + $errors = $decoded['error']['errors']; + } + + throw new Google_Service_Exception($err, $code, null, $errors); } // Only attempt to decode the response, if the response code wasn't (204) 'no content' diff --git a/tests/general/RestTest.php b/tests/general/RestTest.php index 505d23a25..0cc719798 100644 --- a/tests/general/RestTest.php +++ b/tests/general/RestTest.php @@ -113,5 +113,45 @@ public function testCreateRequestUri() { $value = $this->rest->createRequestUri($basePath, '/plus', $params); $this->assertEquals("http://localhost/plus?u=%40me%2F", $value); } + + /** + * @expectedException Google_Service_Exception + */ + public function testBadErrorFormatting() + { + $request = new Google_Http_Request("/a/b"); + $request->setResponseHttpCode(500); + $request->setResponseBody('{ + "error": { + "code": 500, + "message": null + } + }'); + Google_Http_Rest::decodeHttpResponse($request); + } + + /** + * @expectedException Google_Service_Exception + */ + public function tesProperErrorFormatting() + { + $request = new Google_Http_Request("/a/b"); + $request->setResponseHttpCode(401); + $request->setResponseBody('{ + error: { + errors: [ + { + "domain": "global", + "reason": "authError", + "message": "Invalid Credentials", + "locationType": "header", + "location": "Authorization", + } + ], + "code": 401, + "message": "Invalid Credentials" + }'); + Google_Http_Rest::decodeHttpResponse($request); + } } From aebbd830fb69bda1ce47980688f274f089e29fc0 Mon Sep 17 00:00:00 2001 From: Ian Barber Date: Wed, 29 Jan 2014 16:31:59 +0000 Subject: [PATCH 2/4] Adding basic youtube test Trying to track down this null value issue. --- tests/AllTests.php | 2 ++ tests/OAuthHelper.php | 4 ++- tests/youtube/YouTubeTest.php | 52 +++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 tests/youtube/YouTubeTest.php diff --git a/tests/AllTests.php b/tests/AllTests.php index 75d9f22d4..df71b8623 100644 --- a/tests/AllTests.php +++ b/tests/AllTests.php @@ -29,11 +29,13 @@ require_once 'pagespeed/AllPageSpeedTests.php'; require_once 'urlshortener/AllUrlShortenerTests.php'; require_once 'plus/PlusTest.php'; +require_once 'youtube/YouTubeTest.php'; class AllTests { public static function suite() { $suite = new PHPUnit_Framework_TestSuite(); $suite->setName('All Google API PHP Client tests'); + $suite->addTestSuite(YouTubeTests::suite()); $suite->addTestSuite(AllTasksTests::suite()); $suite->addTestSuite(AllPageSpeedTests::suite()); $suite->addTestSuite(AllUrlShortenerTests::suite()); diff --git a/tests/OAuthHelper.php b/tests/OAuthHelper.php index 847a09cac..d7c8753dd 100644 --- a/tests/OAuthHelper.php +++ b/tests/OAuthHelper.php @@ -22,9 +22,11 @@ "https://www.googleapis.com/auth/plus.me", "https://www.googleapis.com/auth/urlshortener", "https://www.googleapis.com/auth/tasks", - "https://www.googleapis.com/auth/adsense" + "https://www.googleapis.com/auth/adsense", + "https://www.googleapis.com/auth/youtube" )); $client->setRedirectUri("urn:ietf:wg:oauth:2.0:oob"); +$client->setAccessType("offline"); // Visit https://code.google.com/apis/console to // generate your oauth2_client_id, oauth2_client_secret, and to // register your oauth2_redirect_uri. diff --git a/tests/youtube/YouTubeTest.php b/tests/youtube/YouTubeTest.php new file mode 100644 index 000000000..beb9f84e5 --- /dev/null +++ b/tests/youtube/YouTubeTest.php @@ -0,0 +1,52 @@ +setName('Google YouTube API tests'); + $suite->addTestSuite('YouTubeTest'); + return $suite; + } +} + +class YouTubeTest extends BaseTest { + /** @var Google_PlusService */ + public $plus; + public function __construct() { + parent::__construct(); + $this->youtube = new Google_Service_YouTube($this->getClient()); + } + + public function testMissingFieldsAreNull() { + $parts = "id,brandingSettings"; + $opts = array("mine" => true); + $channels = $this->youtube->channels->listChannels($parts, $opts); + + $newChannel = new Google_Service_YouTube_Channel(); + $newChannel->setId($channels[0]->getId()); + $newChannel->setBrandingSettings($channels[0]->getBrandingSettings()); + + $simpleOriginal = $channels[0]->toSimpleObject(); + $simpleNew = $newChannel->toSimpleObject(); + + $this->assertObjectHasAttribute('etag', $simpleOriginal); + $this->assertObjectNotHasAttribute('etag', $simpleNew); + } +} From 73c5024ffce8de91634b8f3af46333ae7629b27a Mon Sep 17 00:00:00 2001 From: Ian Barber Date: Wed, 29 Jan 2014 16:38:34 +0000 Subject: [PATCH 3/4] Adding more object test cases --- tests/youtube/YouTubeTest.php | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/youtube/YouTubeTest.php b/tests/youtube/YouTubeTest.php index beb9f84e5..5d4239e1c 100644 --- a/tests/youtube/YouTubeTest.php +++ b/tests/youtube/YouTubeTest.php @@ -48,5 +48,29 @@ public function testMissingFieldsAreNull() { $this->assertObjectHasAttribute('etag', $simpleOriginal); $this->assertObjectNotHasAttribute('etag', $simpleNew); + + $owner_details = new Google_Service_YouTube_ChannelContentOwnerDetails(); + $owner_details->setTimeLinked("123456789"); + $o_channel = new Google_Service_YouTube_Channel(); + $o_channel->setContentOwnerDetails($owner_details); + $simpleManual = $o_channel->toSimpleObject(); + $this->assertObjectHasAttribute('timeLinked', $simpleManual->contentOwnerDetails); + $this->assertObjectNotHasAttribute('contentOwner', $simpleManual->contentOwnerDetails); + + $owner_details = new Google_Service_YouTube_ChannelContentOwnerDetails(); + $owner_details->timeLinked = "123456789"; + $o_channel = new Google_Service_YouTube_Channel(); + $o_channel->setContentOwnerDetails($owner_details); + $simpleManual = $o_channel->toSimpleObject(); + $this->assertObjectHasAttribute('timeLinked', $simpleManual->contentOwnerDetails); + $this->assertObjectNotHasAttribute('contentOwner', $simpleManual->contentOwnerDetails); + + $owner_details = new Google_Service_YouTube_ChannelContentOwnerDetails(); + $owner_details['timeLinked'] = "123456789"; + $o_channel = new Google_Service_YouTube_Channel(); + $o_channel->setContentOwnerDetails($owner_details); + $simpleManual = $o_channel->toSimpleObject(); + $this->assertObjectHasAttribute('timeLinked', $simpleManual->contentOwnerDetails); + $this->assertObjectNotHasAttribute('contentOwner', $simpleManual->contentOwnerDetails); } } From 8698757246a1aa315677f47b6d89f2ca2c8b26f6 Mon Sep 17 00:00:00 2001 From: Ian Barber Date: Wed, 29 Jan 2014 17:16:39 +0000 Subject: [PATCH 4/4] Google_Model toSimpleObject didn't convert Arrays Array and Map types weren't be recursively converted, so we would be returning the raw object. This could lead to nulls or other unexpected data being sent. Should resolve the underlying issue for #52 --- src/Google/Model.php | 35 ++++++++++++++++++++++++++-------- tests/general/ApiModelTest.php | 7 +++++++ tests/youtube/YouTubeTest.php | 10 +++++++++- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/Google/Model.php b/src/Google/Model.php index d5d25e3c1..2b6e67ad5 100644 --- a/src/Google/Model.php +++ b/src/Google/Model.php @@ -113,23 +113,42 @@ public function toSimpleObject() $props = $reflect->getProperties(ReflectionProperty::IS_PUBLIC); foreach ($props as $member) { $name = $member->getName(); - if ($this->$name instanceof Google_Model) { - $object->$name = $this->$name->toSimpleObject(); - } else if ($this->$name !== null) { - $object->$name = $this->$name; + $result = $this->getSimpleValue($this->$name); + if ($result != null) { + $object->$name = $result; } } // Process all other data. foreach ($this->data as $key => $val) { - if ($val instanceof Google_Model) { - $object->$key = $val->toSimpleObject(); - } else if ($val !== null) { - $object->$key = $val; + $result = $this->getSimpleValue($val); + if ($result != null) { + $object->$key = $result; } } return $object; } + + /** + * Handle different types of values, primarily + * other objects and map and array data types. + */ + private function getSimpleValue($value) + { + if ($value instanceof Google_Model) { + return $value->toSimpleObject(); + } else if (is_array($value)) { + $return = array(); + foreach ($value as $key => $a_value) { + $a_value = $this->getSimpleValue($a_value); + if ($a_value != null) { + $return[$key] = $a_value; + } + } + return $return; + } + return $value; + } /** * Returns true only if the array is associative. diff --git a/tests/general/ApiModelTest.php b/tests/general/ApiModelTest.php index 740afaaaa..a7514f620 100644 --- a/tests/general/ApiModelTest.php +++ b/tests/general/ApiModelTest.php @@ -28,11 +28,18 @@ public function testJsonStructure() { $model2->publicC = 12345; $model2->publicD = null; $model->publicB = $model2; + $model3 = new Google_Model(); + $model3->publicE = 54321; + $model3->publicF = null; + $model->publicG = array($model3, "hello"); $string = json_encode($model->toSimpleObject()); $data = json_decode($string, true); $this->assertEquals(12345, $data['publicB']['publicC']); $this->assertEquals("This is a string", $data['publicA']); $this->assertArrayNotHasKey("publicD", $data['publicB']); + $this->assertArrayHasKey("publicE", $data['publicG'][0]); + $this->assertArrayNotHasKey("publicF", $data['publicG'][0]); + $this->assertEquals("hello", $data['publicG'][1]); $this->assertArrayNotHasKey("data", $data); } } diff --git a/tests/youtube/YouTubeTest.php b/tests/youtube/YouTubeTest.php index 5d4239e1c..747548771 100644 --- a/tests/youtube/YouTubeTest.php +++ b/tests/youtube/YouTubeTest.php @@ -72,5 +72,13 @@ public function testMissingFieldsAreNull() { $simpleManual = $o_channel->toSimpleObject(); $this->assertObjectHasAttribute('timeLinked', $simpleManual->contentOwnerDetails); $this->assertObjectNotHasAttribute('contentOwner', $simpleManual->contentOwnerDetails); + + $ping = new Google_Service_YouTube_ChannelConversionPing(); + $ping->setContext("hello"); + $pings = new Google_Service_YouTube_ChannelConversionPings(); + $pings->setPings(array($ping)); + $simplePings = $pings->toSimpleObject(); + $this->assertObjectHasAttribute('context', $simplePings->pings[0]); + $this->assertObjectNotHasAttribute('conversionUrl', $simplePings->pings[0]); } -} +} \ No newline at end of file