From 847089e46440ae705f4db63dc76d0ffabe97a47a Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Sat, 19 Jan 2013 20:11:37 +0100 Subject: [PATCH 01/15] Move Buzz stuff in Adapter\Buzz --- lib/Github/Client.php | 16 +--- .../{ => Adapter/Buzz}/CachedHttpClient.php | 2 +- .../{ => Adapter/Buzz}/HttpClient.php | 42 +++++++-- .../Buzz}/Listener/AuthListener.php | 22 ++++- .../Buzz}/Listener/ErrorListener.php | 4 +- .../{ => Adapter/Buzz}/Message/Request.php | 2 +- .../{ => Adapter/Buzz}/Message/Response.php | 10 +- .../HttpClient/Cache/CacheInterface.php | 10 +- .../HttpClient/Cache/FilesystemCache.php | 4 +- lib/Github/HttpClient/HttpClientInterface.php | 14 +++ lib/Github/HttpClient/ResponseInterface.php | 29 ++++++ test/Github/Tests/Api/AbstractApiTest.php | 2 +- test/Github/Tests/Api/TestCase.php | 2 +- test/Github/Tests/ClientTest.php | 57 +----------- .../Adapter/Buzz/CachedHttpClientTest.php | 93 +++++++++++++++++++ .../{ => Adapter/Buzz}/HttpClientTest.php | 70 +++++++++++++- .../Buzz}/Listener/AuthListenerTest.php | 7 +- .../Buzz}/Listener/ErrorListenerTest.php | 16 ++-- .../HttpClient/Cache/FilesystemCacheTest.php | 5 +- .../Tests/HttpClient/CachedHttpClientTest.php | 93 ------------------- 20 files changed, 292 insertions(+), 208 deletions(-) rename lib/Github/HttpClient/{ => Adapter/Buzz}/CachedHttpClient.php (97%) rename lib/Github/HttpClient/{ => Adapter/Buzz}/HttpClient.php (85%) rename lib/Github/HttpClient/{ => Adapter/Buzz}/Listener/AuthListener.php (89%) rename lib/Github/HttpClient/{ => Adapter/Buzz}/Listener/ErrorListener.php (96%) rename lib/Github/HttpClient/{ => Adapter/Buzz}/Message/Request.php (64%) rename lib/Github/HttpClient/{ => Adapter/Buzz}/Message/Response.php (89%) create mode 100644 lib/Github/HttpClient/ResponseInterface.php create mode 100644 test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php rename test/Github/Tests/HttpClient/{ => Adapter/Buzz}/HttpClientTest.php (72%) rename test/Github/Tests/HttpClient/{ => Adapter/Buzz}/Listener/AuthListenerTest.php (97%) rename test/Github/Tests/HttpClient/{ => Adapter/Buzz}/Listener/ErrorListenerTest.php (88%) delete mode 100644 test/Github/Tests/HttpClient/CachedHttpClientTest.php diff --git a/lib/Github/Client.php b/lib/Github/Client.php index 3338a1b3b7c..e85e68b0672 100644 --- a/lib/Github/Client.php +++ b/lib/Github/Client.php @@ -2,14 +2,10 @@ namespace Github; -use Buzz\Client\Curl; -use Buzz\Client\ClientInterface; - use Github\Api\ApiInterface; use Github\Exception\InvalidArgumentException; -use Github\HttpClient\HttpClient; +use Github\HttpClient\Adapter\Buzz\HttpClient; use Github\HttpClient\HttpClientInterface; -use Github\HttpClient\Listener\AuthListener; /** * Simple yet very cool PHP GitHub client @@ -160,15 +156,7 @@ public function authenticate($tokenOrLogin, $password = null, $authMethod = null $password = null; } - $this->httpClient->addListener( - new AuthListener( - $authMethod, - array( - 'tokenOrLogin' => $tokenOrLogin, - 'password' => $password - ) - ) - ); + $this->httpClient->authenticate($authMethod, $tokenOrLogin, $password); } /** diff --git a/lib/Github/HttpClient/CachedHttpClient.php b/lib/Github/HttpClient/Adapter/Buzz/CachedHttpClient.php similarity index 97% rename from lib/Github/HttpClient/CachedHttpClient.php rename to lib/Github/HttpClient/Adapter/Buzz/CachedHttpClient.php index 5cb2c826dfc..c698ed782cc 100644 --- a/lib/Github/HttpClient/CachedHttpClient.php +++ b/lib/Github/HttpClient/Adapter/Buzz/CachedHttpClient.php @@ -1,6 +1,6 @@ listeners[get_class($listener)] = $listener; } + /** + * Returns listeners + * + * @return array + */ + public function getListeners() + { + return $this->listeners; + } + /** * {@inheritDoc} */ @@ -199,6 +209,22 @@ public function getLastResponse() return $this->lastResponse; } + /** + * {@inheritdoc} + */ + public function authenticate($method, $tokenOrLogin, $password = null) + { + $this->addListener( + new AuthListener( + $method, + array( + 'tokenOrLogin' => $tokenOrLogin, + 'password' => $password + ) + ) + );; + } + /** * @param string $httpMethod * @param string $url diff --git a/lib/Github/HttpClient/Listener/AuthListener.php b/lib/Github/HttpClient/Adapter/Buzz/Listener/AuthListener.php similarity index 89% rename from lib/Github/HttpClient/Listener/AuthListener.php rename to lib/Github/HttpClient/Adapter/Buzz/Listener/AuthListener.php index eba95669ad2..46746be47dd 100644 --- a/lib/Github/HttpClient/Listener/AuthListener.php +++ b/lib/Github/HttpClient/Adapter/Buzz/Listener/AuthListener.php @@ -1,6 +1,6 @@ options = $options; } + /** + * Returns authentication method + * + * @return string + */ + public function getMethod() + { + return $this->method; + } + + /** + * Returns Options + * + * @return array + */ + public function getOptions() + { + return $this->options; + } + /** * {@inheritDoc} * diff --git a/lib/Github/HttpClient/Listener/ErrorListener.php b/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php similarity index 96% rename from lib/Github/HttpClient/Listener/ErrorListener.php rename to lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php index 1d537240900..94e7cff6fc8 100644 --- a/lib/Github/HttpClient/Listener/ErrorListener.php +++ b/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php @@ -1,6 +1,6 @@ isClientError() || $response->isServerError()) { $remaining = $response->getHeader('X-RateLimit-Remaining'); if (null !== $remaining && 1 > $remaining) { diff --git a/lib/Github/HttpClient/Message/Request.php b/lib/Github/HttpClient/Adapter/Buzz/Message/Request.php similarity index 64% rename from lib/Github/HttpClient/Message/Request.php rename to lib/Github/HttpClient/Adapter/Buzz/Message/Request.php index cbc206809de..32127d018f9 100644 --- a/lib/Github/HttpClient/Message/Request.php +++ b/lib/Github/HttpClient/Adapter/Buzz/Message/Request.php @@ -1,6 +1,6 @@ path)) { @mkdir($this->path, 0777, true); diff --git a/lib/Github/HttpClient/HttpClientInterface.php b/lib/Github/HttpClient/HttpClientInterface.php index 7c327d625e3..fd8dce96d24 100644 --- a/lib/Github/HttpClient/HttpClientInterface.php +++ b/lib/Github/HttpClient/HttpClientInterface.php @@ -95,4 +95,18 @@ public function setOption($name, $value); * @param array $headers */ public function setHeaders(array $headers); + + /** + * Removes all headers previously set + */ + public function clearHeaders(); + + /** + * Authenticate HttpClient requests with parameters + * + * @param string $method + * @param string $tokenOrLogin + * @param string|null $password + */ + public function authenticate($method, $tokenOrLogin, $password = null); } diff --git a/lib/Github/HttpClient/ResponseInterface.php b/lib/Github/HttpClient/ResponseInterface.php new file mode 100644 index 00000000000..c5439a63005 --- /dev/null +++ b/lib/Github/HttpClient/ResponseInterface.php @@ -0,0 +1,29 @@ +getMock('Github\HttpClient\HttpClient', array(), array(array(), $this->getHttpClientMock())); + return $this->getMock('Github\HttpClient\HttpClientInterface', array(), array(array(), $this->getHttpClientMock())); } protected function getHttpClientMock() diff --git a/test/Github/Tests/Api/TestCase.php b/test/Github/Tests/Api/TestCase.php index 5bae9b02972..cd59a88aecb 100644 --- a/test/Github/Tests/Api/TestCase.php +++ b/test/Github/Tests/Api/TestCase.php @@ -21,7 +21,7 @@ protected function getApiMock() ->expects($this->any()) ->method('send'); - $mock = $this->getMock('Github\HttpClient\HttpClient', array(), array(array(), $httpClient)); + $mock = $this->getMock('Github\HttpClient\HttpClientInterface', array(), array(array(), $httpClient)); $client = new \Github\Client($mock); $client->setHttpClient($mock); diff --git a/test/Github/Tests/ClientTest.php b/test/Github/Tests/ClientTest.php index 4e96dfcccbb..005121d12e8 100644 --- a/test/Github/Tests/ClientTest.php +++ b/test/Github/Tests/ClientTest.php @@ -3,8 +3,7 @@ namespace Github\Tests; use Github\Client; -use Github\HttpClient\Listener\AuthListener; -use Github\Exception\InvalidArgumentException; +use Github\HttpClient\Adapter\Buzz\Listener\AuthListener; class ClientTest extends \PHPUnit_Framework_TestCase { @@ -15,7 +14,7 @@ public function shouldNotHaveToPassHttpClientToConstructor() { $client = new Client(); - $this->assertInstanceOf('Github\HttpClient\HttpClient', $client->getHttpClient()); + $this->assertInstanceOf('Github\HttpClient\HttpClientInterface', $client->getHttpClient()); } /** @@ -28,54 +27,6 @@ public function shouldPassHttpClientInterfaceToConstructor() $this->assertInstanceOf('Github\HttpClient\HttpClientInterface', $client->getHttpClient()); } - /** - * @test - * @dataProvider getAuthenticationFullData - */ - public function shouldAuthenticateUsingAllGivenParameters($login, $password, $method) - { - $httpClient = $this->getHttpClientMock(array('addListener')); - $httpClient->expects($this->once()) - ->method('addListener') - ->with(new AuthListener($method, array('tokenOrLogin' => $login, 'password' => $password))); - - $client = new Client($httpClient); - $client->authenticate($login, $password, $method); - } - - public function getAuthenticationFullData() - { - return array( - array('login', 'password', Client::AUTH_HTTP_PASSWORD), - array('token', null, Client::AUTH_HTTP_TOKEN), - array('token', null, Client::AUTH_URL_TOKEN), - array('client_id', 'client_secret', Client::AUTH_URL_CLIENT_ID), - ); - } - - /** - * @test - * @dataProvider getAuthenticationPartialData - */ - public function shouldAuthenticateUsingGivenParameters($token, $method) - { - $httpClient = $this->getHttpClientMock(array('addListener')); - $httpClient->expects($this->once()) - ->method('addListener') - ->with(new AuthListener($method, array('tokenOrLogin' => $token, 'password' => null))); - - $client = new Client($httpClient); - $client->authenticate($token, $method); - } - - public function getAuthenticationPartialData() - { - return array( - array('token', Client::AUTH_HTTP_TOKEN), - array('token', Client::AUTH_URL_TOKEN), - ); - } - /** * @test * @expectedException InvalidArgumentException @@ -93,7 +44,7 @@ public function shouldThrowExceptionWhenAuthenticatingWithoutMethodSet() */ public function shouldClearHeadersLazy() { - $httpClient = $this->getHttpClientMock(array('clearHeaders')); + $httpClient = $this->getHttpClientMock(); $httpClient->expects($this->once())->method('clearHeaders'); $client = new Client($httpClient); @@ -172,7 +123,7 @@ public function getApiClassesProvider() public function getHttpClientMock(array $methods = array()) { $methods = array_merge( - array('get', 'post', 'patch', 'put', 'delete', 'request', 'setOption', 'setHeaders'), + array('get', 'post', 'patch', 'put', 'delete', 'request', 'setOption', 'setHeaders', 'clearHeaders', 'authenticate'), $methods ); diff --git a/test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php b/test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php new file mode 100644 index 00000000000..d1fb6915a9e --- /dev/null +++ b/test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php @@ -0,0 +1,93 @@ +getCacheMock(); +// +// $httpClient = new TestCachedHttpClient( +// array('base_url' => ''), +// $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')), +// $cache +// ); +// +// $cache->expects($this->once())->method('set')->with('test', new Response); +// +// $httpClient->get('test'); +// } + +// /** +// * @test +// */ +// public function shouldGetCachedResponseWhileResourceNotModified() +// { +// $client = $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')); +// $client->expects($this->once())->method('send'); +// +// $cache = $this->getCacheMock(); +// +// $response = new Response; +// $response->addHeader('HTTP/1.1 304 Not Modified'); +// +// $httpClient = new TestCachedHttpClient( +// array('base_url' => ''), +// $client, +// $cache +// ); +// $httpClient->fakeResponse = $response; +// +// $cache->expects($this->once())->method('get')->with('test'); +// +// $httpClient->get('test'); +// } +// +// /** +// * @test +// */ +// public function shouldRenewCacheWhenResourceHasChanged() +// { +// $client = $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')); +// $client->expects($this->once())->method('send'); +// +// $cache = $this->getCacheMock(); +// +// $response = new Response; +// $response->addHeader('HTTP/1.1 200 OK'); +// +// $httpClient = new TestCachedHttpClient( +// array('base_url' => ''), +// $client, +// $cache +// ); +// $httpClient->fakeResponse = $response; +// +// $cache->expects($this->once())->method('set')->with('test', $response); +// $cache->expects($this->once())->method('getModifiedSince')->with('test')->will($this->returnValue(1256953732)); +// +// $httpClient->get('test'); +// } + + public function getCacheMock() + { + return $this->getMock('Github\HttpClient\Cache\CacheInterface'); + } +} + +class TestCachedHttpClient extends CachedHttpClient +{ + public $fakeResponse; + + protected function createResponse() + { + return $this->fakeResponse ?: new Response(); + } +} diff --git a/test/Github/Tests/HttpClient/HttpClientTest.php b/test/Github/Tests/HttpClient/Adapter/Buzz/HttpClientTest.php similarity index 72% rename from test/Github/Tests/HttpClient/HttpClientTest.php rename to test/Github/Tests/HttpClient/Adapter/Buzz/HttpClientTest.php index ca3f5766c91..a9e175787b1 100644 --- a/test/Github/Tests/HttpClient/HttpClientTest.php +++ b/test/Github/Tests/HttpClient/Adapter/Buzz/HttpClientTest.php @@ -1,10 +1,12 @@ 'b'); $headers = array('c' => 'd'); - $message = $this->getMock('Github\HttpClient\Message\Response'); + $message = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); $message->expects($this->once()) ->method('getContent') ->will($this->returnValue('Just raw context')); @@ -186,6 +188,64 @@ public function shouldThrowExceptionWhenApiIsExceeded() $httpClient->get($path, $parameters, $headers); } + /** + * @test + * @dataProvider getAuthenticationFullData + */ + public function shouldAuthenticateUsingAllGivenParameters($login, $password, $method) + { + $httpClient = new HttpClient(); + + $n = count($httpClient->getListeners()); + + $httpClient->authenticate($method, $login, $password); + + $listeners = $httpClient->getListeners(); + $this->assertCount($n + 1, $listeners); + + $listener = array_pop($listeners); + $this->assertEquals($method, $listener->getMethod()); + $this->assertEquals(array('tokenOrLogin' => $login, 'password' => $password), $listener->getOptions()); + } + + public function getAuthenticationFullData() + { + return array( + array('login', 'password', Client::AUTH_HTTP_PASSWORD), + array('token', null, Client::AUTH_HTTP_TOKEN), + array('token', null, Client::AUTH_URL_TOKEN), + array('client_id', 'client_secret', Client::AUTH_URL_CLIENT_ID), + ); + } + + /** + * @test + * @dataProvider getAuthenticationPartialData + */ + public function shouldAuthenticateUsingGivenParameters($token, $method) + { + $httpClient = new HttpClient(); + + $n = count($httpClient->getListeners()); + + $httpClient->authenticate($method, $token); + + $listeners = $httpClient->getListeners(); + $this->assertCount($n + 1, $listeners); + + $listener = array_pop($listeners); + $this->assertEquals($method, $listener->getMethod()); + $this->assertEquals(array('tokenOrLogin' => $token, 'password' => null), $listener->getOptions()); + } + + public function getAuthenticationPartialData() + { + return array( + array('token', Client::AUTH_HTTP_TOKEN), + array('token', Client::AUTH_URL_TOKEN), + ); + } + protected function getBrowserMock() { return $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')); diff --git a/test/Github/Tests/HttpClient/Listener/AuthListenerTest.php b/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/AuthListenerTest.php similarity index 97% rename from test/Github/Tests/HttpClient/Listener/AuthListenerTest.php rename to test/Github/Tests/HttpClient/Adapter/Buzz/Listener/AuthListenerTest.php index cfd741c7a0c..838bf139046 100644 --- a/test/Github/Tests/HttpClient/Listener/AuthListenerTest.php +++ b/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/AuthListenerTest.php @@ -1,11 +1,10 @@ getMock('Github\HttpClient\Message\Response'); + $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(false)); @@ -26,7 +26,7 @@ public function shouldPassIfResponseNotHaveErrorStatus() */ public function shouldFailWhenApiLimitWasExceed() { - $response = $this->getMock('Github\HttpClient\Message\Response'); + $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(true)); @@ -45,7 +45,7 @@ public function shouldFailWhenApiLimitWasExceed() */ public function shouldNotPassWhenContentWasNotValidJson() { - $response = $this->getMock('Github\HttpClient\Message\Response'); + $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(true)); @@ -67,7 +67,7 @@ public function shouldNotPassWhenContentWasNotValidJson() */ public function shouldNotPassWhenContentWasValidJsonButStatusIsNotCovered() { - $response = $this->getMock('Github\HttpClient\Message\Response'); + $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(true)); @@ -92,7 +92,7 @@ public function shouldNotPassWhenContentWasValidJsonButStatusIsNotCovered() */ public function shouldNotPassWhen400IsSent() { - $response = $this->getMock('Github\HttpClient\Message\Response'); + $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(true)); @@ -129,7 +129,7 @@ public function shouldNotPassWhen422IsSentWithErrorCode($errorCode) ) ); - $response = $this->getMock('Github\HttpClient\Message\Response'); + $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(true)); diff --git a/test/Github/Tests/HttpClient/Cache/FilesystemCacheTest.php b/test/Github/Tests/HttpClient/Cache/FilesystemCacheTest.php index f3655705d15..f11e9592fe0 100644 --- a/test/Github/Tests/HttpClient/Cache/FilesystemCacheTest.php +++ b/test/Github/Tests/HttpClient/Cache/FilesystemCacheTest.php @@ -2,7 +2,6 @@ namespace Github\Tests\HttpClient\Cache; -use Github\HttpClient\Message\Response; use Github\HttpClient\Cache\FilesystemCache; class FilesystemCacheTest extends \PHPUnit_Framework_TestCase @@ -14,7 +13,7 @@ public function shouldStoreAResponseForAGivenKey() { $cache = new FilesystemCache('/tmp/github-api-test'); - $cache->set('test', new Response); + $cache->set('test', $this->getMock('Github\HttpClient\ResponseInterface')); $this->assertNotNull($cache->get('test')); } @@ -26,7 +25,7 @@ public function shouldGetATimestampForExistingFile() { $cache = new FilesystemCache('/tmp/github-api-test'); - $cache->set('test', new Response); + $cache->set('test', $this->getMock('Github\HttpClient\ResponseInterface')); $this->assertInternalType('int', $cache->getModifiedSince('test')); } diff --git a/test/Github/Tests/HttpClient/CachedHttpClientTest.php b/test/Github/Tests/HttpClient/CachedHttpClientTest.php deleted file mode 100644 index 3ab2f0b303e..00000000000 --- a/test/Github/Tests/HttpClient/CachedHttpClientTest.php +++ /dev/null @@ -1,93 +0,0 @@ -getCacheMock(); - - $httpClient = new TestCachedHttpClient( - array('base_url' => ''), - $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')), - $cache - ); - - $cache->expects($this->once())->method('set')->with('test', new Response); - - $httpClient->get('test'); - } - - /** - * @test - */ - public function shouldGetCachedResponseWhileResourceNotModified() - { - $client = $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')); - $client->expects($this->once())->method('send'); - - $cache = $this->getCacheMock(); - - $response = new Response; - $response->addHeader('HTTP/1.1 304 Not Modified'); - - $httpClient = new TestCachedHttpClient( - array('base_url' => ''), - $client, - $cache - ); - $httpClient->fakeResponse = $response; - - $cache->expects($this->once())->method('get')->with('test'); - - $httpClient->get('test'); - } - - /** - * @test - */ - public function shouldRenewCacheWhenResourceHasChanged() - { - $client = $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')); - $client->expects($this->once())->method('send'); - - $cache = $this->getCacheMock(); - - $response = new Response; - $response->addHeader('HTTP/1.1 200 OK'); - - $httpClient = new TestCachedHttpClient( - array('base_url' => ''), - $client, - $cache - ); - $httpClient->fakeResponse = $response; - - $cache->expects($this->once())->method('set')->with('test', $response); - $cache->expects($this->once())->method('getModifiedSince')->with('test')->will($this->returnValue(1256953732)); - - $httpClient->get('test'); - } - - public function getCacheMock() - { - return $this->getMock('Github\HttpClient\Cache\CacheInterface'); - } -} - -class TestCachedHttpClient extends CachedHttpClient -{ - public $fakeResponse; - - protected function createResponse() - { - return $this->fakeResponse ?: new Response(); - } -} From 602e4f9a77f6dfdce4f13beddb01a97b661575be Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Sat, 19 Jan 2013 20:13:22 +0100 Subject: [PATCH 02/15] Reactivate tests --- .../Adapter/Buzz/CachedHttpClientTest.php | 136 +++++++++--------- 1 file changed, 68 insertions(+), 68 deletions(-) diff --git a/test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php b/test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php index d1fb6915a9e..a63bcdc7d88 100644 --- a/test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php +++ b/test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php @@ -7,74 +7,74 @@ class CachedHttpClientTest extends HttpClientTest { -// /** -// * @test -// */ -// public function shouldCacheResponseAtFirstTime() -// { -// $cache = $this->getCacheMock(); -// -// $httpClient = new TestCachedHttpClient( -// array('base_url' => ''), -// $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')), -// $cache -// ); -// -// $cache->expects($this->once())->method('set')->with('test', new Response); -// -// $httpClient->get('test'); -// } - -// /** -// * @test -// */ -// public function shouldGetCachedResponseWhileResourceNotModified() -// { -// $client = $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')); -// $client->expects($this->once())->method('send'); -// -// $cache = $this->getCacheMock(); -// -// $response = new Response; -// $response->addHeader('HTTP/1.1 304 Not Modified'); -// -// $httpClient = new TestCachedHttpClient( -// array('base_url' => ''), -// $client, -// $cache -// ); -// $httpClient->fakeResponse = $response; -// -// $cache->expects($this->once())->method('get')->with('test'); -// -// $httpClient->get('test'); -// } -// -// /** -// * @test -// */ -// public function shouldRenewCacheWhenResourceHasChanged() -// { -// $client = $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')); -// $client->expects($this->once())->method('send'); -// -// $cache = $this->getCacheMock(); -// -// $response = new Response; -// $response->addHeader('HTTP/1.1 200 OK'); -// -// $httpClient = new TestCachedHttpClient( -// array('base_url' => ''), -// $client, -// $cache -// ); -// $httpClient->fakeResponse = $response; -// -// $cache->expects($this->once())->method('set')->with('test', $response); -// $cache->expects($this->once())->method('getModifiedSince')->with('test')->will($this->returnValue(1256953732)); -// -// $httpClient->get('test'); -// } + /** + * @test + */ + public function shouldCacheResponseAtFirstTime() + { + $cache = $this->getCacheMock(); + + $httpClient = new TestCachedHttpClient( + array('base_url' => ''), + $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')), + $cache + ); + + $cache->expects($this->once())->method('set')->with('test', new Response); + + $httpClient->get('test'); + } + + /** + * @test + */ + public function shouldGetCachedResponseWhileResourceNotModified() + { + $client = $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')); + $client->expects($this->once())->method('send'); + + $cache = $this->getCacheMock(); + + $response = new Response; + $response->addHeader('HTTP/1.1 304 Not Modified'); + + $httpClient = new TestCachedHttpClient( + array('base_url' => ''), + $client, + $cache + ); + $httpClient->fakeResponse = $response; + + $cache->expects($this->once())->method('get')->with('test'); + + $httpClient->get('test'); + } + + /** + * @test + */ + public function shouldRenewCacheWhenResourceHasChanged() + { + $client = $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')); + $client->expects($this->once())->method('send'); + + $cache = $this->getCacheMock(); + + $response = new Response; + $response->addHeader('HTTP/1.1 200 OK'); + + $httpClient = new TestCachedHttpClient( + array('base_url' => ''), + $client, + $cache + ); + $httpClient->fakeResponse = $response; + + $cache->expects($this->once())->method('set')->with('test', $response); + $cache->expects($this->once())->method('getModifiedSince')->with('test')->will($this->returnValue(1256953732)); + + $httpClient->get('test'); + } public function getCacheMock() { From f2cd2b18598e53e679343d5cb33329915ffa63ae Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Sun, 20 Jan 2013 18:41:40 +0100 Subject: [PATCH 03/15] First Guzzle implementation --- .../HttpClient/Adapter/Guzzle/HttpClient.php | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php diff --git a/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php b/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php new file mode 100644 index 00000000000..a634d284741 --- /dev/null +++ b/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php @@ -0,0 +1,184 @@ + 'https://api.github.com/', + + 'user_agent' => 'php-github-api (http://github.com/KnpLabs/php-github-api)', + 'timeout' => 10, + + 'api_limit' => 5000, + 'api_version' => 'beta', + + 'cache_dir' => null + ); + /** + * @var array + */ + protected $headers = array(); + + private $lastResponse; + private $lastRequest; + + /** + * @param array $options + * @param ClientInterface $client + */ + public function __construct(array $options = array(), ClientInterface $client = null) + { + $client = $client ?: new Client(); + + $client->setBaseUrl($this->options['base_url']); + $client->setSslVerification(false); + + $opts = $client->getConfig(Client::CURL_OPTIONS); + $opts[CURLOPT_TIMEOUT] = $this->options['timeout']; + + $client->getConfig()->set(Client::CURL_OPTIONS, $opts); + + $this->options = array_merge($this->options, $options); + $this->client = $client; + +// $this->addListener(new ErrorListener($this->options)); + + $this->clearHeaders(); + } + + /** + * {@inheritDoc} + */ + public function setOption($name, $value) + { + $this->options[$name] = $value; + } + + /** + * {@inheritDoc} + */ + public function setHeaders(array $headers) + { + $this->headers = array_merge($this->headers, $headers); + } + + /** + * {@inheritDoc} + */ + public function clearHeaders() + { + $this->headers = array( + sprintf('Accept: application/vnd.github.%s+json', $this->options['api_version']) + ); + } + + /** + * {@inheritDoc} + */ + public function get($path, array $parameters = array(), array $headers = array()) + { + if (0 < count($parameters)) { + $path .= (false === strpos($path, '?') ? '?' : '&').http_build_query($parameters, '', '&'); + } + + return $this->request($path, array(), 'GET', $headers); + } + + /** + * {@inheritDoc} + */ + public function post($path, array $parameters = array(), array $headers = array()) + { + return $this->request($path, $parameters, 'POST', $headers); + } + + /** + * {@inheritDoc} + */ + public function patch($path, array $parameters = array(), array $headers = array()) + { + return $this->request($path, $parameters, 'PATCH', $headers); + } + + /** + * {@inheritDoc} + */ + public function delete($path, array $parameters = array(), array $headers = array()) + { + return $this->request($path, $parameters, 'DELETE', $headers); + } + + /** + * {@inheritDoc} + */ + public function put($path, array $parameters = array(), array $headers = array()) + { + return $this->request($path, $parameters, 'PUT', $headers); + } + + /** + * {@inheritDoc} + */ + public function request($path, array $parameters = array(), $httpMethod = 'GET', array $headers = array()) + { + $path = trim($path, '/'); + + $request = $this->client->createRequest($httpMethod, $path, $headers, json_encode($parameters)); + + try { + $response = $request->send(); + } catch (\Guzzle\Common\Exception\GuzzleException $e) { + throw new RuntimeException($e->getMessage()); + } + + $this->lastRequest = $request; + $this->lastResponse = $response; + + return $response; + } + + /** + * @return Request + */ + public function getLastRequest() + { + return $this->lastRequest; + } + + /** + * @return Response + */ + public function getLastResponse() + { + return $this->lastResponse; + } + + /** + * {@inheritdoc} + */ + public function authenticate($method, $tokenOrLogin, $password = null) + { + if (GithubClient::AUTH_HTTP_TOKEN === $method) { + $this->headers['Authorization'] = sprintf('token %s', $tokenOrLogin); + } + + throw new \RuntimeException(sprintf('%s not yet implemented', $method)); + } +} From 1e83e6f4acd073cec2fbee161710dd3d77896829 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Sun, 20 Jan 2013 18:57:16 +0100 Subject: [PATCH 04/15] Add Guzzle Response --- .../HttpClient/Adapter/Guzzle/HttpClient.php | 4 +- .../Adapter/Guzzle/Message/Response.php | 71 +++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 lib/Github/HttpClient/Adapter/Guzzle/Message/Response.php diff --git a/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php b/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php index a634d284741..11d1609d8b6 100644 --- a/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php +++ b/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php @@ -9,6 +9,7 @@ use Github\Exception\RuntimeException; use Guzzle\Http\Message\Request; use Guzzle\Http\Message\Response; +use Github\HttpClient\Adapter\Guzzle\Message\Response; use Github\HttpClient\HttpClientInterface; class HttpClient implements HttpClientInterface @@ -143,7 +144,7 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET', $request = $this->client->createRequest($httpMethod, $path, $headers, json_encode($parameters)); try { - $response = $request->send(); + $response = new Response($request->send()); } catch (\Guzzle\Common\Exception\GuzzleException $e) { throw new RuntimeException($e->getMessage()); } @@ -177,6 +178,7 @@ public function authenticate($method, $tokenOrLogin, $password = null) { if (GithubClient::AUTH_HTTP_TOKEN === $method) { $this->headers['Authorization'] = sprintf('token %s', $tokenOrLogin); + return; } throw new \RuntimeException(sprintf('%s not yet implemented', $method)); diff --git a/lib/Github/HttpClient/Adapter/Guzzle/Message/Response.php b/lib/Github/HttpClient/Adapter/Guzzle/Message/Response.php new file mode 100644 index 00000000000..37b75068473 --- /dev/null +++ b/lib/Github/HttpClient/Adapter/Guzzle/Message/Response.php @@ -0,0 +1,71 @@ +response = $response; + } + + + public function getContent() + { + $content = json_decode($this->response->getBody(true), true); + + if (JSON_ERROR_NONE !== json_last_error()) { + return $this->response; + } + + return $content; + } + + /** + * @return array|null + */ + public function getPagination() + { + $header = $this->response->getHeader('Link', true); + if (empty($header)) { + return null; + } + + $pagination = array(); + foreach (explode(',', $header) as $link) { + preg_match('/<(.*)>; rel="(.*)"/i', trim($link, ','), $match); + + if (3 === count($match)) { + $pagination[$match[2]] = $match[1]; + } + } + + return $pagination; + } + + public function getApiLimit() + { + $header = $this->response->getHeader('X-RateLimit-Remaining', true); + if (!empty($header)) { + $this->remainingCalls = $header; + } + + if (null !== $this->remainingCalls && 1 > $this->remainingCalls) { + throw new ApiLimitExceedException($this->options['api_limit']); + } + } + + public function isNotModified() + { + return 304 === $this->response->getStatusCode(); + } +} From 992588a43b1fa672eae00979e958e0055bdcc14a Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Sat, 2 Feb 2013 15:37:45 +0100 Subject: [PATCH 05/15] Final API change --- lib/Github/Api/AbstractApi.php | 10 +- lib/Github/Client.php | 29 +++- lib/Github/Exception/InvalidJsonResponse.php | 19 +++ lib/Github/HttpClient/AbstractAdapter.php | 100 ++++++++++++++ .../HttpClient/Adapter/Buzz/HttpClient.php | 75 ++--------- .../Adapter/Buzz/Listener/ErrorListener.php | 18 +-- .../Adapter/Buzz/Message/Request.php | 15 ++- .../Adapter/Buzz/Message/Response.php | 74 ++++------ .../HttpClient/Adapter/Guzzle/HttpClient.php | 126 +++++++----------- .../Adapter/Guzzle/Message/Response.php | 64 +++------ lib/Github/HttpClient/HttpClientInterface.php | 28 ++++ .../HttpClient/Message/AbstractResponse.php | 53 ++++++++ lib/Github/HttpClient/RequestInterface.php | 11 ++ lib/Github/HttpClient/ResponseInterface.php | 30 ++++- test/Github/Tests/Api/AbstractApiTest.php | 10 +- test/Github/Tests/ClientTest.php | 11 +- .../Buzz/Listener/AuthListenerTest.php | 5 +- .../Buzz/Listener/ErrorListenerTest.php | 10 +- 18 files changed, 418 insertions(+), 270 deletions(-) create mode 100644 lib/Github/Exception/InvalidJsonResponse.php create mode 100644 lib/Github/HttpClient/AbstractAdapter.php create mode 100644 lib/Github/HttpClient/Message/AbstractResponse.php create mode 100644 lib/Github/HttpClient/RequestInterface.php diff --git a/lib/Github/Api/AbstractApi.php b/lib/Github/Api/AbstractApi.php index be6063d3cfb..4eadcbba0f2 100644 --- a/lib/Github/Api/AbstractApi.php +++ b/lib/Github/Api/AbstractApi.php @@ -35,7 +35,7 @@ public function configure() */ protected function get($path, array $parameters = array(), $requestHeaders = array()) { - $response = $this->client->getHttpClient()->get($path, $parameters, $requestHeaders); + $response = $this->client->executeCommand('GET', $path, $parameters, $requestHeaders); return $response->getContent(); } @@ -45,7 +45,7 @@ protected function get($path, array $parameters = array(), $requestHeaders = arr */ protected function post($path, array $parameters = array(), $requestHeaders = array()) { - $response = $this->client->getHttpClient()->post($path, $parameters, $requestHeaders); + $response = $this->client->executeCommand('POST', $path, $parameters, $requestHeaders); return $response->getContent(); } @@ -55,7 +55,7 @@ protected function post($path, array $parameters = array(), $requestHeaders = ar */ protected function patch($path, array $parameters = array(), $requestHeaders = array()) { - $response = $this->client->getHttpClient()->patch($path, $parameters, $requestHeaders); + $response = $this->client->executeCommand('PATCH', $path, $parameters, $requestHeaders); return $response->getContent(); } @@ -65,7 +65,7 @@ protected function patch($path, array $parameters = array(), $requestHeaders = a */ protected function put($path, array $parameters = array(), $requestHeaders = array()) { - $response = $this->client->getHttpClient()->put($path, $parameters, $requestHeaders); + $response = $this->client->executeCommand('PUT', $path, $parameters, $requestHeaders); return $response->getContent(); } @@ -75,7 +75,7 @@ protected function put($path, array $parameters = array(), $requestHeaders = arr */ protected function delete($path, array $parameters = array(), $requestHeaders = array()) { - $response = $this->client->getHttpClient()->delete($path, $parameters, $requestHeaders); + $response = $this->client->executeCommand('DELETE', $path, $parameters, $requestHeaders); return $response->getContent(); } diff --git a/lib/Github/Client.php b/lib/Github/Client.php index e85e68b0672..41adef8c9f2 100644 --- a/lib/Github/Client.php +++ b/lib/Github/Client.php @@ -49,7 +49,6 @@ class Client 'user_agent' => 'php-github-api (http://github.com/KnpLabs/php-github-api)', 'timeout' => 10, - 'api_limit' => 5000, 'api_version' => 'beta', 'cache_dir' => null @@ -156,9 +155,37 @@ public function authenticate($tokenOrLogin, $password = null, $authMethod = null $password = null; } + switch ($authMethod) { + case Client::AUTH_HTTP_PASSWORD: + if (!$tokenOrLogin || !$password) { + throw new InvalidArgumentException('You need to set username with password!'); + } + break; + case Client::AUTH_HTTP_TOKEN: + if (!$tokenOrLogin) { + throw new InvalidArgumentException('You need to set OAuth token!'); + } + break; + case Client::AUTH_URL_CLIENT_ID: + if (!$tokenOrLogin || !$password) { + throw new InvalidArgumentException('You need to set client_id and client_secret!'); + } + break; + case Client::AUTH_URL_TOKEN: + if (!$tokenOrLogin) { + throw new InvalidArgumentException('You need to set OAuth token!'); + } + break; + } + $this->httpClient->authenticate($authMethod, $tokenOrLogin, $password); } + public function executeCommand($method, $command, $parameters, $headers) + { + return $this->httpClient->request($command, $parameters, $method, $headers)->getContent(); + } + /** * @return HttpClient */ diff --git a/lib/Github/Exception/InvalidJsonResponse.php b/lib/Github/Exception/InvalidJsonResponse.php new file mode 100644 index 00000000000..b0344086893 --- /dev/null +++ b/lib/Github/Exception/InvalidJsonResponse.php @@ -0,0 +1,19 @@ +body = $body; + parent::__construct('Invalid Json Response', $code, $previous); + } + + public function getBody() + { + return $this->body; + } +} diff --git a/lib/Github/HttpClient/AbstractAdapter.php b/lib/Github/HttpClient/AbstractAdapter.php new file mode 100644 index 00000000000..392aeda0d2f --- /dev/null +++ b/lib/Github/HttpClient/AbstractAdapter.php @@ -0,0 +1,100 @@ + 'https://api.github.com/', + 'user_agent' => 'php-github-api (http://github.com/KnpLabs/php-github-api)', + 'timeout' => 10, + 'api_version' => 'beta', + 'cache_dir' => null + ); + /** + * @var array + */ + protected $headers; + + /** + * {@inheritDoc} + */ + public function setOption($name, $value) + { + $this->options[$name] = $value; + } + + /** + * {@inheritDoc} + */ + public function setHeaders(array $headers) + { + $this->headers = array_merge($this->headers, $headers); + } + + /** + * {@inheritDoc} + */ + public function clearHeaders() + { + $this->headers = array( + sprintf('Accept: application/vnd.github.%s+json', $this->options['api_version']) + ); + } + + /** + * {@inheritDoc} + */ + public function getLastRequest() + { + return $this->lastRequest; + } + + /** + * {@inheritDoc} + */ + public function getLastResponse() + { + return $this->lastResponse; + } + + /** + * {@inheritDoc} + */ + public function getAPILimit() + { + if (!$this->getLastRequest()) { + $this->get('/rate_limit'); + } + + return (int) $this->getLastRequest()->getHeaderAsString('X-RateLimit-Limit'); + } + + /** + * {@inheritDoc} + */ + public function getAPIRemaining() + { + if (!$this->getLastRequest()) { + $this->get('/rate_limit'); + } + + return (int) $this->getLastRequest()->getHeaderAsString('X-RateLimit-Remaining'); + } +} diff --git a/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php b/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php index 8d888869226..155363c7dec 100644 --- a/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php +++ b/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php @@ -5,21 +5,22 @@ use Buzz\Client\Curl; use Buzz\Client\ClientInterface; use Buzz\Listener\ListenerInterface; - +use Buzz\Message\Request as BuzzRequest; +use Buzz\Message\Response as BuzzResponse; use Github\Exception\ErrorException; use Github\Exception\RuntimeException; +use Github\HttpClient\AbstractAdapter; use Github\HttpClient\Adapter\Buzz\Listener\AuthListener; use Github\HttpClient\Adapter\Buzz\Listener\ErrorListener; use Github\HttpClient\Adapter\Buzz\Message\Request; use Github\HttpClient\Adapter\Buzz\Message\Response; -use Github\HttpClient\HttpClientInterface; /** * Performs requests on GitHub API. API documentation should be self-explanatory. * * @author Joseph Bielawski */ -class HttpClient implements HttpClientInterface +class HttpClient extends AbstractAdapter { /** * @var array @@ -29,8 +30,6 @@ class HttpClient implements HttpClientInterface 'user_agent' => 'php-github-api (http://github.com/KnpLabs/php-github-api)', 'timeout' => 10, - - 'api_limit' => 5000, 'api_version' => 'beta', 'cache_dir' => null @@ -39,13 +38,6 @@ class HttpClient implements HttpClientInterface * @var array */ protected $listeners = array(); - /** - * @var array - */ - protected $headers = array(); - - private $lastResponse; - private $lastRequest; /** * @param array $options @@ -60,37 +52,11 @@ public function __construct(array $options = array(), ClientInterface $client = $this->options = array_merge($this->options, $options); $this->client = $client; - $this->addListener(new ErrorListener($this->options)); + $this->addListener(new ErrorListener()); $this->clearHeaders(); } - /** - * {@inheritDoc} - */ - public function setOption($name, $value) - { - $this->options[$name] = $value; - } - - /** - * {@inheritDoc} - */ - public function setHeaders(array $headers) - { - $this->headers = array_merge($this->headers, $headers); - } - - /** - * {@inheritDoc} - */ - public function clearHeaders() - { - $this->headers = array( - sprintf('Accept: application/vnd.github.%s+json', $this->options['api_version']) - ); - } - /** * @param ListenerInterface $listener */ @@ -160,6 +126,7 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET', { $path = trim($this->options['base_url'].$path, '/'); + var_dump($httpMethod, $path); $request = $this->createRequest($httpMethod, $path); $request->addHeaders($headers); $request->setContent(json_encode($parameters)); @@ -181,32 +148,16 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET', throw new RuntimeException($e->getMessage()); } - $this->lastRequest = $request; - $this->lastResponse = $response; - if ($hasListeners) { foreach ($this->listeners as $listener) { $listener->postSend($request, $response); } } - return $response; - } + $this->lastRequest = new Request($request); + $this->lastResponse = new Response($response); - /** - * @return Request - */ - public function getLastRequest() - { - return $this->lastRequest; - } - - /** - * @return Response - */ - public function getLastResponse() - { - return $this->lastResponse; + return $response; } /** @@ -231,9 +182,9 @@ public function authenticate($method, $tokenOrLogin, $password = null) * * @return Request */ - protected function createRequest($httpMethod, $url) + private function createRequest($httpMethod, $url) { - $request = new Request($httpMethod); + $request = new BuzzRequest($httpMethod); $request->setHeaders($this->headers); $request->fromUrl($url); @@ -243,8 +194,8 @@ protected function createRequest($httpMethod, $url) /** * @return Response */ - protected function createResponse() + private function createResponse() { - return new Response(); + return new BuzzResponse(); } } diff --git a/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php b/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php index 94e7cff6fc8..cb031e90801 100644 --- a/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php +++ b/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php @@ -15,19 +15,6 @@ */ class ErrorListener implements ListenerInterface { - /** - * @var array - */ - private $options; - - /** - * @param array $options - */ - public function __construct(array $options) - { - $this->options = $options; - } - /** * {@inheritDoc} */ @@ -42,9 +29,8 @@ public function postSend(RequestInterface $request, MessageInterface $response) { /** @var $response \Github\HttpClient\ResponseInterface */ if ($response->isClientError() || $response->isServerError()) { - $remaining = $response->getHeader('X-RateLimit-Remaining'); - if (null !== $remaining && 1 > $remaining) { - throw new ApiLimitExceedException($this->options['api_limit']); + if (0 === (int) $response->getHeader('X-RateLimit-Remaining')) { + throw new ApiLimitExceedException($response->getHeader('X-RateLimit-Limit')); } $content = $response->getContent(); diff --git a/lib/Github/HttpClient/Adapter/Buzz/Message/Request.php b/lib/Github/HttpClient/Adapter/Buzz/Message/Request.php index 32127d018f9..60809beec70 100644 --- a/lib/Github/HttpClient/Adapter/Buzz/Message/Request.php +++ b/lib/Github/HttpClient/Adapter/Buzz/Message/Request.php @@ -2,9 +2,20 @@ namespace Github\HttpClient\Adapter\Buzz\Message; -use Buzz\Message\Request as BaseRequest; +use Buzz\Message\Request as BuzzRequest; +use Github\HttpClient\RequestInterface; -class Request extends BaseRequest +class Request implements RequestInterface { + private $request; + public function __construct(BuzzRequest $request) + { + $this->request = $request; + } + + public function getAdapterRequest() + { + return $this->request; + } } diff --git a/lib/Github/HttpClient/Adapter/Buzz/Message/Response.php b/lib/Github/HttpClient/Adapter/Buzz/Message/Response.php index 6c8815fd40c..7d10b004427 100644 --- a/lib/Github/HttpClient/Adapter/Buzz/Message/Response.php +++ b/lib/Github/HttpClient/Adapter/Buzz/Message/Response.php @@ -2,74 +2,58 @@ namespace Github\HttpClient\Adapter\Buzz\Message; -use Buzz\Message\Response as BaseResponse; -use Github\Exception\ApiLimitExceedException; -use Github\HttpClient\ResponseInterface; +use Buzz\Message\Response as BuzzResponse; +use Github\HttpClient\Message\AbstractResponse; -class Response extends BaseResponse implements ResponseInterface +class Response extends AbstractResponse { + /** @var BuzzResponse */ + private $response; + + public function __construct(BuzzResponse $response) + { + $this->response = $response; + } + /** - * @var integer + * {@inheritdoc} */ - public $remainingCalls; + public function getStatusCode() + { + return $this->response->getStatusCode(); + } /** - * {@inheritDoc} + * {@inheritdoc} */ - public function getContent() + public function getHeaderAsString($name) { - $response = parent::getContent(); - $content = json_decode($response, true); - - if (JSON_ERROR_NONE !== json_last_error()) { - return $response; - } - - return $content; + return $this->response->getHeader($name); } /** - * @return array|null + * {@inheritdoc} */ - public function getPagination() + public function isNotModified() { - $header = $this->getHeader('Link'); - if (empty($header)) { - return null; - } - - $pagination = array(); - foreach (explode(',', $header) as $link) { - preg_match('/<(.*)>; rel="(.*)"/i', trim($link, ','), $match); - - if (3 === count($match)) { - $pagination[$match[2]] = $match[1]; - } - } - - return $pagination; + return 304 === $this->getStatusCode(); } /** - * {@inheritDoc} + * {@inheritdoc} */ - public function getApiLimit() + public function getAdapterResponse() { - $header = $this->getHeaderAttributes('X-RateLimit-Remaining'); - if (!empty($header)) { - $this->remainingCalls = $header; - } - - if (null !== $this->remainingCalls && 1 > $this->remainingCalls) { - throw new ApiLimitExceedException($this->options['api_limit']); - } + return $this->response; } /** * {@inheritdoc} + * + * @return BuzzResponse */ - public function isNotModified() + public function getRawBody() { - return 304 === $this->getStatusCode(); + return $this->response->getContent(); } } diff --git a/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php b/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php index 11d1609d8b6..0b92e664048 100644 --- a/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php +++ b/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php @@ -2,43 +2,20 @@ namespace Github\HttpClient\Adapter\Guzzle; -use Guzzle\Http\ClientInterface; -use Guzzle\Http\Client; - use Github\Client as GithubClient; use Github\Exception\RuntimeException; -use Guzzle\Http\Message\Request; -use Guzzle\Http\Message\Response; use Github\HttpClient\Adapter\Guzzle\Message\Response; -use Github\HttpClient\HttpClientInterface; +use Github\HttpClient\AbstractAdapter; +use Guzzle\Common\Event; +use Guzzle\Http\ClientInterface; +use Guzzle\Http\Client; -class HttpClient implements HttpClientInterface +class HttpClient extends AbstractAdapter { /** - * @var \Guzzle\Http\ClientInterface + * @var ClientInterface */ private $client; - /** - * @var array - */ - protected $options = array( - 'base_url' => 'https://api.github.com/', - - 'user_agent' => 'php-github-api (http://github.com/KnpLabs/php-github-api)', - 'timeout' => 10, - - 'api_limit' => 5000, - 'api_version' => 'beta', - - 'cache_dir' => null - ); - /** - * @var array - */ - protected $headers = array(); - - private $lastResponse; - private $lastRequest; /** * @param array $options @@ -46,7 +23,7 @@ class HttpClient implements HttpClientInterface */ public function __construct(array $options = array(), ClientInterface $client = null) { - $client = $client ?: new Client(); + $client = $client ? : new Client(); $client->setBaseUrl($this->options['base_url']); $client->setSslVerification(false); @@ -57,46 +34,20 @@ public function __construct(array $options = array(), ClientInterface $client = $client->getConfig()->set(Client::CURL_OPTIONS, $opts); $this->options = array_merge($this->options, $options); - $this->client = $client; + $this->client = $client; // $this->addListener(new ErrorListener($this->options)); $this->clearHeaders(); } - /** - * {@inheritDoc} - */ - public function setOption($name, $value) - { - $this->options[$name] = $value; - } - - /** - * {@inheritDoc} - */ - public function setHeaders(array $headers) - { - $this->headers = array_merge($this->headers, $headers); - } - - /** - * {@inheritDoc} - */ - public function clearHeaders() - { - $this->headers = array( - sprintf('Accept: application/vnd.github.%s+json', $this->options['api_version']) - ); - } - /** * {@inheritDoc} */ public function get($path, array $parameters = array(), array $headers = array()) { if (0 < count($parameters)) { - $path .= (false === strpos($path, '?') ? '?' : '&').http_build_query($parameters, '', '&'); + $path .= (false === strpos($path, '?') ? '?' : '&') . http_build_query($parameters, '', '&'); } return $this->request($path, array(), 'GET', $headers); @@ -149,38 +100,53 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET', throw new RuntimeException($e->getMessage()); } - $this->lastRequest = $request; + $this->lastRequest = $request; $this->lastResponse = $response; return $response; } - /** - * @return Request - */ - public function getLastRequest() - { - return $this->lastRequest; - } - - /** - * @return Response - */ - public function getLastResponse() - { - return $this->lastResponse; - } - /** * {@inheritdoc} */ public function authenticate($method, $tokenOrLogin, $password = null) { - if (GithubClient::AUTH_HTTP_TOKEN === $method) { - $this->headers['Authorization'] = sprintf('token %s', $tokenOrLogin); - return; + switch ($method) { + case GithubClient::AUTH_HTTP_PASSWORD: + $this->client->getEventDispatcher()->addListener('request.create', function (Event $event) { + $event['request']->setHeader('Authorization', sprintf('Basic ', base64_encode($tokenOrLogin . ':' . $password))); + }); + break; + case GithubClient::AUTH_HTTP_TOKEN: + $this->client->getEventDispatcher()->addListener('request.create', function (Event $event) { + $event['request']->setHeader('Authorization', sprintf('token ', $tokenOrLogin)); + }); + break; + case GithubClient::AUTH_URL_CLIENT_ID: + $this->client->getEventDispatcher()->addListener('request.create', function (Event $event) { + $url = $event['request']->getUrl(); + + $parameters = array( + 'client_id' => $tokenOrLogin, + 'client_secret' => $password, + ); + + $url .= (false === strpos($url, '?') ? '?' : '&') . utf8_encode(http_build_query($parameters, '', '&')); + + $event['request']->setUrl($url); + }); + break; + case GithubClient::AUTH_URL_TOKEN: + $this->client->getEventDispatcher()->addListener('request.create', function (Event $event) { + $url = $event['request']->getUrl(); + $url .= (false === strpos($url, '?') ? '?' : '&') . utf8_encode(http_build_query(array('access_token' => $tokenOrLogin), '', '&')); + + $event['request']->setUrl($url); + }); + break; + default: + throw new RuntimeException(sprintf('%s not yet implemented', $method)); + break; } - - throw new \RuntimeException(sprintf('%s not yet implemented', $method)); } } diff --git a/lib/Github/HttpClient/Adapter/Guzzle/Message/Response.php b/lib/Github/HttpClient/Adapter/Guzzle/Message/Response.php index 37b75068473..6f300baac84 100644 --- a/lib/Github/HttpClient/Adapter/Guzzle/Message/Response.php +++ b/lib/Github/HttpClient/Adapter/Guzzle/Message/Response.php @@ -3,69 +3,49 @@ namespace Github\HttpClient\Adapter\Guzzle\Message; use Guzzle\Http\Message\Response as GuzzleResponse; -use Github\HttpClient\ResponseInterface; +use Github\HttpClient\Message\AbstractResponse; -class Response implements ResponseInterface +class Response extends AbstractResponse { /** @var GuzzleResponse */ private $response; - private $remainingCalls; - // fix this - private $options = array(); public function __construct(GuzzleResponse $response) { $this->response = $response; } - - public function getContent() + /** + * {@inheritdoc} + */ + public function getRawBody() { - $content = json_decode($this->response->getBody(true), true); - - if (JSON_ERROR_NONE !== json_last_error()) { - return $this->response; - } - - return $content; + return $this->response->getBody(true); } /** - * @return array|null + * {@inheritdoc} */ - public function getPagination() + public function getStatusCode() { - $header = $this->response->getHeader('Link', true); - if (empty($header)) { - return null; - } - - $pagination = array(); - foreach (explode(',', $header) as $link) { - preg_match('/<(.*)>; rel="(.*)"/i', trim($link, ','), $match); - - if (3 === count($match)) { - $pagination[$match[2]] = $match[1]; - } - } - - return $pagination; + return $this->response->getStatusCode(); } - public function getApiLimit() + /** + * {@inheritdoc} + */ + public function getHeaderAsString($name) { - $header = $this->response->getHeader('X-RateLimit-Remaining', true); - if (!empty($header)) { - $this->remainingCalls = $header; - } - - if (null !== $this->remainingCalls && 1 > $this->remainingCalls) { - throw new ApiLimitExceedException($this->options['api_limit']); - } + return $this->response->getHeader($name, true); } - public function isNotModified() + /** + * {@inheritdoc} + * + * @return GuzzleResponse + */ + public function getAdapterResponse() { - return 304 === $this->response->getStatusCode(); + return $this->response; } } diff --git a/lib/Github/HttpClient/HttpClientInterface.php b/lib/Github/HttpClient/HttpClientInterface.php index fd8dce96d24..830e1de2776 100644 --- a/lib/Github/HttpClient/HttpClientInterface.php +++ b/lib/Github/HttpClient/HttpClientInterface.php @@ -109,4 +109,32 @@ public function clearHeaders(); * @param string|null $password */ public function authenticate($method, $tokenOrLogin, $password = null); + + /** + * Return the last request that has been executed + * + * @return RequestInterface + */ + public function getLastRequest(); + + /** + * Return the last response that has been received + * + * @return ResponseInterface + */ + public function getLastResponse(); + + /** + * Return the API Limit with the current authentication state + * + * @return Integer + */ + public function getAPILimit(); + + /** + * Return the API remaining request quantity with the current authentication state + * + * @return Integer + */ + public function getAPIRemaining(); } diff --git a/lib/Github/HttpClient/Message/AbstractResponse.php b/lib/Github/HttpClient/Message/AbstractResponse.php new file mode 100644 index 00000000000..64d6f4ce2a6 --- /dev/null +++ b/lib/Github/HttpClient/Message/AbstractResponse.php @@ -0,0 +1,53 @@ +getRawBody(); + $content = json_decode($rawBody, true); + + if (JSON_ERROR_NONE !== json_last_error()) { + throw new InvalidJsonResponse($rawBody); + } + + return $content; + } + + /** + * {@inheritdoc} + */ + public function getPagination() + { + $header = $this->getHeaderAsString('Link'); + if (empty($header)) { + return null; + } + + $pagination = array(); + foreach (explode(',', $header) as $link) { + preg_match('/<(.*)>; rel="(.*)"/i', trim($link, ','), $match); + + if (3 === count($match)) { + $pagination[$match[2]] = $match[1]; + } + } + + return $pagination; + } + + /** + * {@inheritdoc} + */ + public function isNotModified() + { + return 304 === $this->getStatusCode(); + } +} diff --git a/lib/Github/HttpClient/RequestInterface.php b/lib/Github/HttpClient/RequestInterface.php new file mode 100644 index 00000000000..5a2131af0f8 --- /dev/null +++ b/lib/Github/HttpClient/RequestInterface.php @@ -0,0 +1,11 @@ +client->getHttpClient()->get($path, $parameters, $requestHeaders); + return $this->client->executeCommand('GET', $path, $parameters, $requestHeaders); } /** @@ -166,7 +166,7 @@ public function get($path, array $parameters = array(), $requestHeaders = array( */ public function post($path, array $parameters = array(), $requestHeaders = array()) { - return $this->client->getHttpClient()->post($path, $parameters, $requestHeaders); + return $this->client->executeCommand('POST', $path, $parameters, $requestHeaders); } /** @@ -174,7 +174,7 @@ public function post($path, array $parameters = array(), $requestHeaders = array */ public function patch($path, array $parameters = array(), $requestHeaders = array()) { - return $this->client->getHttpClient()->patch($path, $parameters, $requestHeaders); + return $this->client->executeCommand('PATCH', $path, $parameters, $requestHeaders); } /** @@ -182,7 +182,7 @@ public function patch($path, array $parameters = array(), $requestHeaders = arra */ public function put($path, array $parameters = array(), $requestHeaders = array()) { - return $this->client->getHttpClient()->put($path, $parameters, $requestHeaders); + return $this->client->executeCommand('PUT', $path, $parameters, $requestHeaders); } /** @@ -190,6 +190,6 @@ public function put($path, array $parameters = array(), $requestHeaders = array( */ public function delete($path, array $parameters = array(), $requestHeaders = array()) { - return $this->client->getHttpClient()->delete($path, $parameters, $requestHeaders); + return $this->client->executeCommand('DELETE', $path, $parameters, $requestHeaders); } } diff --git a/test/Github/Tests/ClientTest.php b/test/Github/Tests/ClientTest.php index 005121d12e8..f41f1c42432 100644 --- a/test/Github/Tests/ClientTest.php +++ b/test/Github/Tests/ClientTest.php @@ -22,9 +22,10 @@ public function shouldNotHaveToPassHttpClientToConstructor() */ public function shouldPassHttpClientInterfaceToConstructor() { - $client = new Client($this->getHttpClientMock()); + $httpClient = $this->getHttpClientMock(); + $client = new Client($httpClient); - $this->assertInstanceOf('Github\HttpClient\HttpClientInterface', $client->getHttpClient()); + $this->assertEquals($httpClient, $client->getHttpClient()); } /** @@ -123,7 +124,11 @@ public function getApiClassesProvider() public function getHttpClientMock(array $methods = array()) { $methods = array_merge( - array('get', 'post', 'patch', 'put', 'delete', 'request', 'setOption', 'setHeaders', 'clearHeaders', 'authenticate'), + array( + 'get', 'post', 'patch', 'put', 'delete', 'request', + 'setOption', 'setHeaders', 'clearHeaders', 'authenticate', + 'getLastRequest', 'getLastResponse', 'getAPILimit', 'getAPIRemaining' + ), $methods ); diff --git a/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/AuthListenerTest.php b/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/AuthListenerTest.php index 838bf139046..b8bb006b385 100644 --- a/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/AuthListenerTest.php +++ b/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/AuthListenerTest.php @@ -5,6 +5,7 @@ use Github\Client; use Github\HttpClient\Adapter\Buzz\Listener\AuthListener; use Github\HttpClient\Adapter\Buzz\Message\Request; +use Buzz\Message\Request as BuzzRequest; class AuthListenerTest extends \PHPUnit_Framework_TestCase { @@ -155,7 +156,7 @@ public function shouldSetAuthTokenHeaderForAuthPassMethod() */ public function shouldSetTokenInUrlForAuthUrlMethod() { - $request = new Request(Request::METHOD_GET, '/res'); + $request = new BuzzRequest(BuzzRequest::METHOD_GET, '/res'); $listener = new AuthListener(Client::AUTH_URL_TOKEN, array('tokenOrLogin' => 'test')); $listener->preSend($request); @@ -168,7 +169,7 @@ public function shouldSetTokenInUrlForAuthUrlMethod() */ public function shouldSetClientDetailsInUrlForAuthUrlMethod() { - $request = new Request(Request::METHOD_GET, '/res'); + $request = new BuzzRequest(BuzzRequest::METHOD_GET, '/res'); $listener = new AuthListener(Client::AUTH_URL_CLIENT_ID, array('tokenOrLogin' => 'clientId', 'password' => 'clientSsecret')); $listener->preSend($request); diff --git a/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/ErrorListenerTest.php b/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/ErrorListenerTest.php index 3a1a0c3da7e..e3f11dee106 100644 --- a/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/ErrorListenerTest.php +++ b/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/ErrorListenerTest.php @@ -16,7 +16,7 @@ public function shouldPassIfResponseNotHaveErrorStatus() ->method('isClientError') ->will($this->returnValue(false)); - $listener = new ErrorListener(array('api_limit' => 5000)); + $listener = new ErrorListener(); $listener->postSend($this->getMock('Buzz\Message\RequestInterface'), $response); } @@ -35,7 +35,7 @@ public function shouldFailWhenApiLimitWasExceed() ->with('X-RateLimit-Remaining') ->will($this->returnValue(0)); - $listener = new ErrorListener(array('api_limit' => 5000)); + $listener = new ErrorListener(); $listener->postSend($this->getMock('Buzz\Message\RequestInterface'), $response); } @@ -57,7 +57,7 @@ public function shouldNotPassWhenContentWasNotValidJson() ->method('getContent') ->will($this->returnValue('fail')); - $listener = new ErrorListener(array('api_limit' => 5000)); + $listener = new ErrorListener(); $listener->postSend($this->getMock('Buzz\Message\RequestInterface'), $response); } @@ -82,7 +82,7 @@ public function shouldNotPassWhenContentWasValidJsonButStatusIsNotCovered() ->method('getStatusCode') ->will($this->returnValue(404)); - $listener = new ErrorListener(array('api_limit' => 5000)); + $listener = new ErrorListener(); $listener->postSend($this->getMock('Buzz\Message\RequestInterface'), $response); } @@ -107,7 +107,7 @@ public function shouldNotPassWhen400IsSent() ->method('getStatusCode') ->will($this->returnValue(400)); - $listener = new ErrorListener(array('api_limit' => 5000)); + $listener = new ErrorListener(); $listener->postSend($this->getMock('Buzz\Message\RequestInterface'), $response); } From fecbccaa40bed75a0c1b1be199a831e65f25f767 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Sat, 2 Feb 2013 15:58:34 +0100 Subject: [PATCH 06/15] Remove var_dump --- lib/Github/HttpClient/Adapter/Buzz/HttpClient.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php b/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php index 155363c7dec..fd201afd234 100644 --- a/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php +++ b/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php @@ -126,7 +126,6 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET', { $path = trim($this->options['base_url'].$path, '/'); - var_dump($httpMethod, $path); $request = $this->createRequest($httpMethod, $path); $request->addHeaders($headers); $request->setContent(json_encode($parameters)); From 96e00ea06850273e0f770fb01308292bf0ab365a Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Sat, 2 Feb 2013 15:59:20 +0100 Subject: [PATCH 07/15] Rename `Client::executeCommand` to `Client::executeRequest` --- lib/Github/Api/AbstractApi.php | 10 +++++----- lib/Github/Client.php | 4 ++-- test/Github/Tests/Api/AbstractApiTest.php | 10 +++++----- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/Github/Api/AbstractApi.php b/lib/Github/Api/AbstractApi.php index 4eadcbba0f2..ecf74d5cd88 100644 --- a/lib/Github/Api/AbstractApi.php +++ b/lib/Github/Api/AbstractApi.php @@ -35,7 +35,7 @@ public function configure() */ protected function get($path, array $parameters = array(), $requestHeaders = array()) { - $response = $this->client->executeCommand('GET', $path, $parameters, $requestHeaders); + $response = $this->client->executeRequest('GET', $path, $parameters, $requestHeaders); return $response->getContent(); } @@ -45,7 +45,7 @@ protected function get($path, array $parameters = array(), $requestHeaders = arr */ protected function post($path, array $parameters = array(), $requestHeaders = array()) { - $response = $this->client->executeCommand('POST', $path, $parameters, $requestHeaders); + $response = $this->client->executeRequest('POST', $path, $parameters, $requestHeaders); return $response->getContent(); } @@ -55,7 +55,7 @@ protected function post($path, array $parameters = array(), $requestHeaders = ar */ protected function patch($path, array $parameters = array(), $requestHeaders = array()) { - $response = $this->client->executeCommand('PATCH', $path, $parameters, $requestHeaders); + $response = $this->client->executeRequest('PATCH', $path, $parameters, $requestHeaders); return $response->getContent(); } @@ -65,7 +65,7 @@ protected function patch($path, array $parameters = array(), $requestHeaders = a */ protected function put($path, array $parameters = array(), $requestHeaders = array()) { - $response = $this->client->executeCommand('PUT', $path, $parameters, $requestHeaders); + $response = $this->client->executeRequest('PUT', $path, $parameters, $requestHeaders); return $response->getContent(); } @@ -75,7 +75,7 @@ protected function put($path, array $parameters = array(), $requestHeaders = arr */ protected function delete($path, array $parameters = array(), $requestHeaders = array()) { - $response = $this->client->executeCommand('DELETE', $path, $parameters, $requestHeaders); + $response = $this->client->executeRequest('DELETE', $path, $parameters, $requestHeaders); return $response->getContent(); } diff --git a/lib/Github/Client.php b/lib/Github/Client.php index 41adef8c9f2..34b62dc1194 100644 --- a/lib/Github/Client.php +++ b/lib/Github/Client.php @@ -181,9 +181,9 @@ public function authenticate($tokenOrLogin, $password = null, $authMethod = null $this->httpClient->authenticate($authMethod, $tokenOrLogin, $password); } - public function executeCommand($method, $command, $parameters, $headers) + public function executeRequest($method, $path, $parameters, $headers) { - return $this->httpClient->request($command, $parameters, $method, $headers)->getContent(); + return $this->httpClient->request($path, $parameters, $method, $headers)->getContent(); } /** diff --git a/test/Github/Tests/Api/AbstractApiTest.php b/test/Github/Tests/Api/AbstractApiTest.php index 2116e5e1a2a..17d73050ffd 100644 --- a/test/Github/Tests/Api/AbstractApiTest.php +++ b/test/Github/Tests/Api/AbstractApiTest.php @@ -158,7 +158,7 @@ class AbstractApiTestInstance extends AbstractApi */ public function get($path, array $parameters = array(), $requestHeaders = array()) { - return $this->client->executeCommand('GET', $path, $parameters, $requestHeaders); + return $this->client->executeRequest('GET', $path, $parameters, $requestHeaders); } /** @@ -166,7 +166,7 @@ public function get($path, array $parameters = array(), $requestHeaders = array( */ public function post($path, array $parameters = array(), $requestHeaders = array()) { - return $this->client->executeCommand('POST', $path, $parameters, $requestHeaders); + return $this->client->executeRequest('POST', $path, $parameters, $requestHeaders); } /** @@ -174,7 +174,7 @@ public function post($path, array $parameters = array(), $requestHeaders = array */ public function patch($path, array $parameters = array(), $requestHeaders = array()) { - return $this->client->executeCommand('PATCH', $path, $parameters, $requestHeaders); + return $this->client->executeRequest('PATCH', $path, $parameters, $requestHeaders); } /** @@ -182,7 +182,7 @@ public function patch($path, array $parameters = array(), $requestHeaders = arra */ public function put($path, array $parameters = array(), $requestHeaders = array()) { - return $this->client->executeCommand('PUT', $path, $parameters, $requestHeaders); + return $this->client->executeRequest('PUT', $path, $parameters, $requestHeaders); } /** @@ -190,6 +190,6 @@ public function put($path, array $parameters = array(), $requestHeaders = array( */ public function delete($path, array $parameters = array(), $requestHeaders = array()) { - return $this->client->executeCommand('DELETE', $path, $parameters, $requestHeaders); + return $this->client->executeRequest('DELETE', $path, $parameters, $requestHeaders); } } From 379ac96b7cb7cd6e3e2de6bedc0d203d0a9b60f3 Mon Sep 17 00:00:00 2001 From: Evgeniy Guseletov Date: Sun, 20 Oct 2013 21:17:27 +0300 Subject: [PATCH 08/15] Fix AbstractApiTest --- .../HttpClient/Adapter/Buzz/HttpClient.php | 4 +- test/Github/Tests/Api/AbstractApiTest.php | 59 +++++++++++++------ .../Adapter/Buzz/HttpClientTest.php | 15 ----- 3 files changed, 44 insertions(+), 34 deletions(-) diff --git a/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php b/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php index a855c7214c9..c33ea89da41 100644 --- a/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php +++ b/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php @@ -45,8 +45,8 @@ class HttpClient extends AbstractAdapter */ protected $headers = array(); - private $lastResponse; - private $lastRequest; + protected $lastResponse; + protected $lastRequest; /** * @param array $options diff --git a/test/Github/Tests/Api/AbstractApiTest.php b/test/Github/Tests/Api/AbstractApiTest.php index 17d73050ffd..2fd03fe5502 100644 --- a/test/Github/Tests/Api/AbstractApiTest.php +++ b/test/Github/Tests/Api/AbstractApiTest.php @@ -12,13 +12,15 @@ class AbstractApiTest extends \PHPUnit_Framework_TestCase public function shouldPassGETRequestToClient() { $expectedArray = array('value'); + $response = $this->getResponseMock($expectedArray); $httpClient = $this->getHttpMock(); $httpClient - ->expects($this->any()) - ->method('get') - ->with('/path', array('param1' => 'param1value'), array('header1' => 'header1value')) - ->will($this->returnValue($expectedArray)); + ->expects($this->once()) + ->method('request') + ->with('/path', array('param1' => 'param1value'), 'GET', array('header1' => 'header1value')) + ->will($this->returnValue($response)); + $client = $this->getClientMock(); $client->setHttpClient($httpClient); @@ -37,9 +39,10 @@ public function shouldPassPOSTRequestToClient() $httpClient = $this->getHttpMock(); $httpClient ->expects($this->once()) - ->method('post') - ->with('/path', array('param1' => 'param1value'), array('option1' => 'option1value')) - ->will($this->returnValue($expectedArray)); + ->method('request') + ->with('/path', array('param1' => 'param1value'), 'POST', array('option1' => 'option1value')) + ->will($this->returnValue($this->getResponseMock($expectedArray))); + $client = $this->getClientMock(); $client->setHttpClient($httpClient); @@ -58,9 +61,9 @@ public function shouldPassPATCHRequestToClient() $httpClient = $this->getHttpMock(); $httpClient ->expects($this->once()) - ->method('patch') - ->with('/path', array('param1' => 'param1value'), array('option1' => 'option1value')) - ->will($this->returnValue($expectedArray)); + ->method('request') + ->with('/path', array('param1' => 'param1value'), 'PATCH', array('option1' => 'option1value')) + ->will($this->returnValue($this->getResponseMock($expectedArray))); $client = $this->getClientMock(); $client->setHttpClient($httpClient); @@ -79,9 +82,9 @@ public function shouldPassPUTRequestToClient() $httpClient = $this->getHttpMock(); $httpClient ->expects($this->once()) - ->method('put') - ->with('/path', array('param1' => 'param1value'), array('option1' => 'option1value')) - ->will($this->returnValue($expectedArray)); + ->method('request') + ->with('/path', array('param1' => 'param1value'), 'PUT', array('option1' => 'option1value')) + ->will($this->returnValue($this->getResponseMock($expectedArray))); $client = $this->getClientMock(); $client->setHttpClient($httpClient); @@ -100,9 +103,9 @@ public function shouldPassDELETERequestToClient() $httpClient = $this->getHttpMock(); $httpClient ->expects($this->once()) - ->method('delete') - ->with('/path', array('param1' => 'param1value'), array('option1' => 'option1value')) - ->will($this->returnValue($expectedArray)); + ->method('request') + ->with('/path', array('param1' => 'param1value'), 'DELETE', array('option1' => 'option1value')) + ->will($this->returnValue($this->getResponseMock($expectedArray))); $client = $this->getClientMock(); $client->setHttpClient($httpClient); @@ -134,7 +137,7 @@ protected function getHttpMock() protected function getHttpClientMock() { - $mock = $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')); + $mock = $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send', 'request')); $mock ->expects($this->any()) ->method('setTimeout') @@ -149,6 +152,28 @@ protected function getHttpClientMock() return $mock; } + + protected function getResponseMock($expectedValue, $times = 1) + { + $mock = $this + ->getMockBuilder('Github\\HttpClient\\ResponseInterface') + ->setMethods(array( + 'getContent', 'getPagination', + 'isNotModified', 'getHeaderAsString', + 'getStatusCode', 'getRawBody', + 'getAdapterResponse' + )) + ->getMock() + ; + + $mock + ->expects($this->exactly($times)) + ->method('getContent') + ->will($this->returnValue($expectedValue)) + ; + + return $mock; + } } class AbstractApiTestInstance extends AbstractApi diff --git a/test/Github/Tests/HttpClient/Adapter/Buzz/HttpClientTest.php b/test/Github/Tests/HttpClient/Adapter/Buzz/HttpClientTest.php index 6ee5e57d524..1960df136d2 100644 --- a/test/Github/Tests/HttpClient/Adapter/Buzz/HttpClientTest.php +++ b/test/Github/Tests/HttpClient/Adapter/Buzz/HttpClientTest.php @@ -3,7 +3,6 @@ namespace Github\Tests\HttpClient\Adapter\Buzz; use Github\Client; -use Github\HttpClient\Adapter\Buzz\Listener\AuthListener; use Github\HttpClient\Adapter\Buzz\HttpClient; use Github\HttpClient\Adapter\Buzz\Message\Request; use Github\HttpClient\Adapter\Buzz\Message\Response; @@ -34,16 +33,6 @@ public function shouldBeAbleToSetOption() $this->assertEquals(666, $httpClient->getOption('timeout')); } - public function getAuthenticationFullData() - { - return array( - array('login', 'password', Client::AUTH_HTTP_PASSWORD), - array('token', null, Client::AUTH_HTTP_TOKEN), - array('token', null, Client::AUTH_URL_TOKEN), - array('client_id', 'client_secret', Client::AUTH_URL_CLIENT_ID), - ); - } - /** * @test */ @@ -72,8 +61,6 @@ public function shouldDoPOSTRequest() $httpClient = new HttpClient(array(), $client); $httpClient->post($path, $parameters, $headers); - - $this->assertEquals('{"a":"b"}', $httpClient->getLastRequest()->getContent()); } /** @@ -87,8 +74,6 @@ public function shouldDoPOSTRequestWithoutContent() $httpClient = new HttpClient(array(), $client); $httpClient->post($path); - - $this->assertEmpty($httpClient->getLastRequest()->getContent()); } /** From 19d41faf87effa03b6de4a4a7d564d0d288fba4a Mon Sep 17 00:00:00 2001 From: Evgeniy Guseletov Date: Mon, 21 Oct 2013 02:21:46 +0300 Subject: [PATCH 09/15] Green tests, use guzzle as main client --- composer.json | 5 ++- doc/result_pager.md | 2 +- lib/Github/Client.php | 2 +- .../Adapter/Buzz/CachedHttpClient.php | 9 ++-- .../HttpClient/Adapter/Buzz/HttpClient.php | 4 +- .../Adapter/Buzz/Listener/ErrorListener.php | 2 +- .../HttpClient/Message/AbstractResponse.php | 1 + lib/Github/ResultPager.php | 2 +- .../Adapter/Buzz/CachedHttpClientTest.php | 11 ++--- .../Adapter/Buzz/HttpClientTest.php | 42 ++++++++++++++----- .../Buzz/Listener/ErrorListenerTest.php | 24 +++++------ test/Github/Tests/ResultPagerTest.php | 6 +-- 12 files changed, 69 insertions(+), 41 deletions(-) diff --git a/composer.json b/composer.json index d3471164d68..37efa72628e 100644 --- a/composer.json +++ b/composer.json @@ -19,11 +19,14 @@ "require": { "php": ">=5.3.2", "ext-curl": "*", - "kriswallsmith/buzz": ">=0.7" + "guzzle/guzzle": "3.7.*" }, "require-dev": { "phpunit/phpunit": ">=3.6.0" }, + "suggests": { + "kriswallsmith/buzz": "You may use old Buzz adapter for your project" + }, "autoload": { "psr-0": { "Github\\": "lib/" } }, diff --git a/doc/result_pager.md b/doc/result_pager.md index a802be1caa9..a7910f3c182 100644 --- a/doc/result_pager.md +++ b/doc/result_pager.md @@ -21,7 +21,7 @@ $client = new Github\Client(); $organizationApi = $client->api('organization'); -$paginator = new Github\ResultPager( $client ); +$paginator = new Github\ResultPager($client); $parameters = array('github'); $result = $paginator->fetch($organizationApi, 'repositories', $parameters); ``` diff --git a/lib/Github/Client.php b/lib/Github/Client.php index 8f1ca1844e3..800158b713f 100644 --- a/lib/Github/Client.php +++ b/lib/Github/Client.php @@ -4,7 +4,7 @@ use Github\Api\ApiInterface; use Github\Exception\InvalidArgumentException; -use Github\HttpClient\Adapter\Buzz\HttpClient; +use Github\HttpClient\Adapter\Guzzle\HttpClient; use Github\HttpClient\HttpClientInterface; /** diff --git a/lib/Github/HttpClient/Adapter/Buzz/CachedHttpClient.php b/lib/Github/HttpClient/Adapter/Buzz/CachedHttpClient.php index cff301a741c..395ebb9d767 100644 --- a/lib/Github/HttpClient/Adapter/Buzz/CachedHttpClient.php +++ b/lib/Github/HttpClient/Adapter/Buzz/CachedHttpClient.php @@ -4,6 +4,7 @@ use Github\HttpClient\Cache\CacheInterface; use Github\HttpClient\Cache\FilesystemCache; +use Github\HttpClient\Adapter\Buzz\Message\Response; /** * Performs requests on GitHub API using If-Modified-Since headers. @@ -44,16 +45,16 @@ public function setCache(CacheInterface $cache) */ public function request($path, array $parameters = array(), $httpMethod = 'GET', array $headers = array()) { - $response = parent::request($path, $parameters, $httpMethod, $headers); + $buzzResponse = parent::request($path, $parameters, $httpMethod, $headers); $key = trim($this->options['base_url'].$path, '/'); - if (304 == $response->getStatusCode()) { + if (304 == $buzzResponse->getStatusCode()) { return $this->getCache()->get($key); } - $this->getCache()->set($key, $response); + $this->getCache()->set($key, new Response($buzzResponse)); - return $response; + return $buzzResponse; } /** diff --git a/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php b/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php index c33ea89da41..a7524048bfb 100644 --- a/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php +++ b/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php @@ -230,7 +230,7 @@ public function getLastResponse() * * @return Request */ - private function createRequest($httpMethod, $url) + protected function createRequest($httpMethod, $url) { $request = new BuzzRequest($httpMethod); $request->setHeaders($this->headers); @@ -242,7 +242,7 @@ private function createRequest($httpMethod, $url) /** * @return Response */ - private function createResponse() + protected function createResponse() { return new BuzzResponse(); } diff --git a/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php b/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php index 34138b0854a..b976f3027fd 100644 --- a/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php +++ b/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php @@ -40,7 +40,7 @@ public function preSend(RequestInterface $request) */ public function postSend(RequestInterface $request, MessageInterface $response) { - /** @var $response \Github\HttpClient\ResponseInterface */ + /** @var $response \Buzz\Message\Response */ if ($response->isClientError() || $response->isServerError()) { $remaining = $response->getHeader('X-RateLimit-Remaining'); diff --git a/lib/Github/HttpClient/Message/AbstractResponse.php b/lib/Github/HttpClient/Message/AbstractResponse.php index 64d6f4ce2a6..069f3201886 100644 --- a/lib/Github/HttpClient/Message/AbstractResponse.php +++ b/lib/Github/HttpClient/Message/AbstractResponse.php @@ -2,6 +2,7 @@ namespace Github\HttpClient\Message; +use Github\Exception\InvalidJsonResponse; use Github\HttpClient\ResponseInterface; abstract class AbstractResponse implements ResponseInterface diff --git a/lib/Github/ResultPager.php b/lib/Github/ResultPager.php index 83f2692a29a..5ea61990045 100644 --- a/lib/Github/ResultPager.php +++ b/lib/Github/ResultPager.php @@ -14,7 +14,7 @@ class ResultPager implements ResultPagerInterface { /** - * @var Github\Client client + * @var \Github\Client client */ protected $client; diff --git a/test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php b/test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php index 93968beb5fe..61e6ff1e043 100644 --- a/test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php +++ b/test/Github/Tests/HttpClient/Adapter/Buzz/CachedHttpClientTest.php @@ -2,6 +2,7 @@ namespace Github\Tests\HttpClient\Adapter\Buzz; +use Buzz\Message\Response as BuzzResponse; use Github\HttpClient\Adapter\Buzz\CachedHttpClient; use Github\HttpClient\Adapter\Buzz\Message\Response; @@ -20,7 +21,7 @@ public function shouldCacheResponseAtFirstTime() ); $httpClient->setCache($cache); - $cache->expects($this->once())->method('set')->with('test', new Response); + $cache->expects($this->once())->method('set')->with('test', new Response(new BuzzResponse())); $httpClient->get('test'); } @@ -35,7 +36,7 @@ public function shouldGetCachedResponseWhileResourceNotModified() $cache = $this->getCacheMock(); - $response = new Response; + $response = new BuzzResponse(); $response->addHeader('HTTP/1.1 304 Not Modified'); $httpClient = new TestCachedHttpClient( @@ -60,7 +61,7 @@ public function shouldRenewCacheWhenResourceHasChanged() $cache = $this->getCacheMock(); - $response = new Response; + $response = new BuzzResponse(); $response->addHeader('HTTP/1.1 200 OK'); $httpClient = new TestCachedHttpClient( @@ -70,7 +71,7 @@ public function shouldRenewCacheWhenResourceHasChanged() $httpClient->setCache($cache); $httpClient->fakeResponse = $response; - $cache->expects($this->once())->method('set')->with('test', $response); + $cache->expects($this->once())->method('set')->with('test', new Response($response)); $cache->expects($this->once())->method('getModifiedSince')->with('test')->will($this->returnValue(1256953732)); $httpClient->get('test'); @@ -88,6 +89,6 @@ class TestCachedHttpClient extends CachedHttpClient protected function createResponse() { - return $this->fakeResponse ?: new Response(); + return $this->fakeResponse ?: new BuzzResponse(); } } diff --git a/test/Github/Tests/HttpClient/Adapter/Buzz/HttpClientTest.php b/test/Github/Tests/HttpClient/Adapter/Buzz/HttpClientTest.php index 1960df136d2..f9bf35169f9 100644 --- a/test/Github/Tests/HttpClient/Adapter/Buzz/HttpClientTest.php +++ b/test/Github/Tests/HttpClient/Adapter/Buzz/HttpClientTest.php @@ -2,6 +2,8 @@ namespace Github\Tests\HttpClient\Adapter\Buzz; +use Buzz\Message\Response as BuzzResponse; +use Buzz\Message\Request as BuzzRequest; use Github\Client; use Github\HttpClient\Adapter\Buzz\HttpClient; use Github\HttpClient\Adapter\Buzz\Message\Request; @@ -144,8 +146,8 @@ public function shouldHandlePagination() $parameters = array('a' => 'b'); $headers = array('c' => 'd'); - $response = new Response(); - $response->addHeader("Link:; rel=\"page2\", \n; rel=\"page4\""); + $response = new Response(new BuzzResponse()); + $response->getAdapterResponse()->addHeader("Link:; rel=\"page2\", \n; rel=\"page4\""); $client = $this->getBrowserMock(); @@ -164,11 +166,31 @@ public function shouldAllowToReturnRawContent() $parameters = array('a' => 'b'); $headers = array('c' => 'd'); - $message = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); + $buzzMessage = $this + ->getMockBuilder('Buzz\Message\Response') + ->getMock(); + + $buzzMessage->expects($this->once()) + ->method('isClientError') + ->will($this->returnValue(false)); + + $buzzMessage->expects($this->once()) + ->method('isServerError') + ->will($this->returnValue(false)); + + $message = $this + ->getMockBuilder('Github\HttpClient\Adapter\Buzz\Message\Response') + ->disableOriginalConstructor() + ->getMock(); + $message->expects($this->once()) ->method('getContent') ->will($this->returnValue('Just raw context')); + $message->expects($this->exactly(2)) + ->method('getAdapterResponse') + ->will($this->returnValue($buzzMessage)); + $client = $this->getBrowserMock(); $httpClient = new TestHttpClient(array(), $client); @@ -177,7 +199,7 @@ public function shouldAllowToReturnRawContent() $response = $httpClient->get($path, $parameters, $headers); $this->assertEquals("Just raw context", $response->getContent()); - $this->assertInstanceOf('Buzz\Message\MessageInterface', $response); + $this->assertInstanceOf('Buzz\Message\MessageInterface', $response->getAdapterResponse()); } /** @@ -190,9 +212,9 @@ public function shouldThrowExceptionWhenApiIsExceeded() $parameters = array('a' => 'b'); $headers = array('c' => 'd'); - $response = new Response(); - $response->addHeader('HTTP/1.1 403 Forbidden'); - $response->addHeader('X-RateLimit-Remaining: 0'); + $response = new Response(new BuzzResponse()); + $response->getAdapterResponse()->addHeader('HTTP/1.1 403 Forbidden'); + $response->getAdapterResponse()->addHeader('X-RateLimit-Remaining: 0'); $httpClient = new TestHttpClient(array(), $this->getBrowserMock()); $httpClient->fakeResponse = $response; @@ -290,7 +312,7 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET', $response = $this->createResponse(); if (0 < count($this->listeners)) { foreach ($this->listeners as $listener) { - $listener->postSend($request, $response); + $listener->postSend($request->getAdapterRequest(), $response->getAdapterResponse()); } } @@ -299,11 +321,11 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET', protected function createRequest($httpMethod, $url) { - return new Request($httpMethod); + return new Request(new BuzzRequest($httpMethod)); } protected function createResponse() { - return $this->fakeResponse ?: new Response(); + return $this->fakeResponse ?: new Response(new BuzzResponse()); } } diff --git a/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/ErrorListenerTest.php b/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/ErrorListenerTest.php index 226f9bc4b4c..1eb9bb13c54 100644 --- a/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/ErrorListenerTest.php +++ b/test/Github/Tests/HttpClient/Adapter/Buzz/Listener/ErrorListenerTest.php @@ -9,14 +9,14 @@ class ErrorListenerTest extends \PHPUnit_Framework_TestCase /** * @test */ - public function shouldPassIfResponseNotHaveErrorStatus() + public function shouldPassIfResponseDoesNotHaveErrorStatus() { - $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); + $response = $this->getMock('Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(false)); - $listener = new ErrorListener(); + $listener = new ErrorListener(array('api_limit' => 5000)); $listener->postSend($this->getMock('Buzz\Message\RequestInterface'), $response); } @@ -26,7 +26,7 @@ public function shouldPassIfResponseNotHaveErrorStatus() */ public function shouldFailWhenApiLimitWasExceed() { - $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); + $response = $this->getMock('Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(true)); @@ -35,7 +35,7 @@ public function shouldFailWhenApiLimitWasExceed() ->with('X-RateLimit-Remaining') ->will($this->returnValue(0)); - $listener = new ErrorListener(); + $listener = new ErrorListener(array('api_limit' => 5000)); $listener->postSend($this->getMock('Buzz\Message\RequestInterface'), $response); } @@ -45,7 +45,7 @@ public function shouldFailWhenApiLimitWasExceed() */ public function shouldNotPassWhenContentWasNotValidJson() { - $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); + $response = $this->getMock('Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(true)); @@ -57,7 +57,7 @@ public function shouldNotPassWhenContentWasNotValidJson() ->method('getContent') ->will($this->returnValue('fail')); - $listener = new ErrorListener(); + $listener = new ErrorListener(array('api_limit' => 5000)); $listener->postSend($this->getMock('Buzz\Message\RequestInterface'), $response); } @@ -67,7 +67,7 @@ public function shouldNotPassWhenContentWasNotValidJson() */ public function shouldNotPassWhenContentWasValidJsonButStatusIsNotCovered() { - $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); + $response = $this->getMock('Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(true)); @@ -82,7 +82,7 @@ public function shouldNotPassWhenContentWasValidJsonButStatusIsNotCovered() ->method('getStatusCode') ->will($this->returnValue(404)); - $listener = new ErrorListener(); + $listener = new ErrorListener(array('api_limit' => 5000)); $listener->postSend($this->getMock('Buzz\Message\RequestInterface'), $response); } @@ -92,7 +92,7 @@ public function shouldNotPassWhenContentWasValidJsonButStatusIsNotCovered() */ public function shouldNotPassWhen400IsSent() { - $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); + $response = $this->getMock('Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(true)); @@ -107,7 +107,7 @@ public function shouldNotPassWhen400IsSent() ->method('getStatusCode') ->will($this->returnValue(400)); - $listener = new ErrorListener(); + $listener = new ErrorListener(array('api_limit' => 5000)); $listener->postSend($this->getMock('Buzz\Message\RequestInterface'), $response); } @@ -130,7 +130,7 @@ public function shouldNotPassWhen422IsSentWithErrorCode($errorCode) ) ); - $response = $this->getMock('Github\HttpClient\Adapter\Buzz\Message\Response'); + $response = $this->getMock('Buzz\Message\Response'); $response->expects($this->once()) ->method('isClientError') ->will($this->returnValue(true)); diff --git a/test/Github/Tests/ResultPagerTest.php b/test/Github/Tests/ResultPagerTest.php index 7d37bb4a957..6ef5e452569 100644 --- a/test/Github/Tests/ResultPagerTest.php +++ b/test/Github/Tests/ResultPagerTest.php @@ -97,7 +97,7 @@ public function postFetch() ); // response mock - $responseMock = $this->getMock('Github\HttpClient\Message\Response'); + $responseMock = $this->getMock('Github\HttpClient\Message\AbstractResponse'); $responseMock ->expects($this->any()) ->method('getPagination') @@ -187,7 +187,7 @@ public function shouldHavePrevious() protected function getResponseMock(array $pagination) { // response mock - $responseMock = $this->getMock('Github\HttpClient\Message\Response'); + $responseMock = $this->getMock('Github\HttpClient\Message\AbstractResponse'); $responseMock ->expects($this->any()) ->method('getPagination') @@ -226,7 +226,7 @@ protected function getHttpClientMock($responseMock = null) ->method('send'); // create the httpClient mock - $httpClientMock = $this->getMock('Github\HttpClient\HttpClient', array(), array(array(), $clientInterfaceMock)); + $httpClientMock = $this->getMock('Github\HttpClient\Adapter\Buzz\HttpClient', array(), array(array(), $clientInterfaceMock)); if ($responseMock) { $httpClientMock From e0267ee0abdae9db1737be8d97f17196d6b8fd6c Mon Sep 17 00:00:00 2001 From: Evgeniy Guseletov Date: Mon, 21 Oct 2013 02:31:42 +0300 Subject: [PATCH 10/15] Update travis.yml --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 776d6a9b15b..d9babc0aff0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,7 @@ php: before_script: - composer install --dev --prefer-source + - composer require "kriswallsmith/buzz: >=0.7" script: - phpunit --coverage-text From b683c16b0692e517c2425894d4abc59adec56ffb Mon Sep 17 00:00:00 2001 From: Evgeniy Guseletov Date: Mon, 21 Oct 2013 02:32:19 +0300 Subject: [PATCH 11/15] Update travis.yml --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d9babc0aff0..e0797524fa6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,7 @@ php: before_script: - composer install --dev --prefer-source - - composer require "kriswallsmith/buzz: >=0.7" + - composer require "kriswallsmith/buzz:>=0.7" script: - phpunit --coverage-text From c9158c164dd42462bfc817200b651fbc1915087c Mon Sep 17 00:00:00 2001 From: Evgeniy Guseletov Date: Mon, 21 Oct 2013 02:34:37 +0300 Subject: [PATCH 12/15] Add Buzz version note --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 37efa72628e..1feef413cb1 100644 --- a/composer.json +++ b/composer.json @@ -25,7 +25,7 @@ "phpunit/phpunit": ">=3.6.0" }, "suggests": { - "kriswallsmith/buzz": "You may use old Buzz adapter for your project" + "kriswallsmith/buzz": "You may use old Buzz(>=0.7) adapter for your project" }, "autoload": { "psr-0": { "Github\\": "lib/" } From 0232512c26b648fad5e38bd8a6bed100bb800505 Mon Sep 17 00:00:00 2001 From: Evgeniy Guseletov Date: Mon, 21 Oct 2013 02:48:04 +0300 Subject: [PATCH 13/15] Generic adapter pager test --- test/Github/Tests/ResultPagerTest.php | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/test/Github/Tests/ResultPagerTest.php b/test/Github/Tests/ResultPagerTest.php index 6ef5e452569..819023cb2e5 100644 --- a/test/Github/Tests/ResultPagerTest.php +++ b/test/Github/Tests/ResultPagerTest.php @@ -211,22 +211,7 @@ protected function getClientMock(HttpClientInterface $httpClient = null) protected function getHttpClientMock($responseMock = null) { - // mock the client interface - $clientInterfaceMock = $this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')); - $clientInterfaceMock - ->expects($this->any()) - ->method('setTimeout') - ->with(10); - $clientInterfaceMock - ->expects($this->any()) - ->method('setVerifyPeer') - ->with(false); - $clientInterfaceMock - ->expects($this->any()) - ->method('send'); - - // create the httpClient mock - $httpClientMock = $this->getMock('Github\HttpClient\Adapter\Buzz\HttpClient', array(), array(array(), $clientInterfaceMock)); + $httpClientMock = $this->getMock('Github\HttpClient\AbstractAdapter', array()); if ($responseMock) { $httpClientMock From 8397a79a17254a1028aca7acc6b0714d44f9d7d7 Mon Sep 17 00:00:00 2001 From: Evgeniy Guseletov Date: Tue, 22 Oct 2013 18:23:35 +0300 Subject: [PATCH 14/15] Fix docblocks, change imports --- .../HttpClient/Adapter/Buzz/HttpClient.php | 17 +++--- .../HttpClient/Adapter/Guzzle/HttpClient.php | 54 +++++++++++-------- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php b/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php index a7524048bfb..6b6f9009704 100644 --- a/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php +++ b/lib/Github/HttpClient/Adapter/Buzz/HttpClient.php @@ -171,11 +171,8 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET', $request->setContent(json_encode($parameters, empty($parameters) ? JSON_FORCE_OBJECT : 0)); } - $hasListeners = 0 < count($this->listeners); - if ($hasListeners) { - foreach ($this->listeners as $listener) { - $listener->preSend($request); - } + foreach ($this->listeners as $listener) { + $listener->preSend($request); } $response = $this->createResponse(); @@ -188,10 +185,8 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET', throw new RuntimeException($e->getMessage()); } - if ($hasListeners) { - foreach ($this->listeners as $listener) { - $listener->postSend($request, $response); - } + foreach ($this->listeners as $listener) { + $listener->postSend($request, $response); } $this->lastRequest = new Request($request); @@ -228,7 +223,7 @@ public function getLastResponse() * @param string $httpMethod * @param string $url * - * @return Request + * @return BuzzRequest */ protected function createRequest($httpMethod, $url) { @@ -240,7 +235,7 @@ protected function createRequest($httpMethod, $url) } /** - * @return Response + * @return BuzzResponse */ protected function createResponse() { diff --git a/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php b/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php index 0b92e664048..550b9e2bae4 100644 --- a/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php +++ b/lib/Github/HttpClient/Adapter/Guzzle/HttpClient.php @@ -2,13 +2,14 @@ namespace Github\HttpClient\Adapter\Guzzle; -use Github\Client as GithubClient; +use Github\Client; use Github\Exception\RuntimeException; use Github\HttpClient\Adapter\Guzzle\Message\Response; use Github\HttpClient\AbstractAdapter; use Guzzle\Common\Event; use Guzzle\Http\ClientInterface; -use Guzzle\Http\Client; +use Guzzle\Http\Client as GuzzleClient; +use Guzzle\Common\Exception\GuzzleException; class HttpClient extends AbstractAdapter { @@ -23,15 +24,15 @@ class HttpClient extends AbstractAdapter */ public function __construct(array $options = array(), ClientInterface $client = null) { - $client = $client ? : new Client(); + $client = $client ? : new GuzzleClient(); $client->setBaseUrl($this->options['base_url']); $client->setSslVerification(false); - $opts = $client->getConfig(Client::CURL_OPTIONS); + $opts = $client->getConfig(GuzzleClient::CURL_OPTIONS); $opts[CURLOPT_TIMEOUT] = $this->options['timeout']; - $client->getConfig()->set(Client::CURL_OPTIONS, $opts); + $client->getConfig()->set(GuzzleClient::CURL_OPTIONS, $opts); $this->options = array_merge($this->options, $options); $this->client = $client; @@ -96,7 +97,7 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET', try { $response = new Response($request->send()); - } catch (\Guzzle\Common\Exception\GuzzleException $e) { + } catch (GuzzleException $e) { throw new RuntimeException($e->getMessage()); } @@ -112,18 +113,23 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET', public function authenticate($method, $tokenOrLogin, $password = null) { switch ($method) { - case GithubClient::AUTH_HTTP_PASSWORD: - $this->client->getEventDispatcher()->addListener('request.create', function (Event $event) { - $event['request']->setHeader('Authorization', sprintf('Basic ', base64_encode($tokenOrLogin . ':' . $password))); - }); + case Client::AUTH_HTTP_PASSWORD: + $handler = function (Event $event) use ($tokenOrLogin, $password) { + $event['request']->setHeader( + 'Authorization', + sprintf('Basic ', base64_encode($tokenOrLogin . ':' . $password)) + ); + }; break; - case GithubClient::AUTH_HTTP_TOKEN: - $this->client->getEventDispatcher()->addListener('request.create', function (Event $event) { + + case Client::AUTH_HTTP_TOKEN: + $handler = function (Event $event) use ($tokenOrLogin, $password) { $event['request']->setHeader('Authorization', sprintf('token ', $tokenOrLogin)); - }); + }; break; - case GithubClient::AUTH_URL_CLIENT_ID: - $this->client->getEventDispatcher()->addListener('request.create', function (Event $event) { + + case Client::AUTH_URL_CLIENT_ID: + $handler = function (Event $event) use ($tokenOrLogin, $password) { $url = $event['request']->getUrl(); $parameters = array( @@ -131,22 +137,28 @@ public function authenticate($method, $tokenOrLogin, $password = null) 'client_secret' => $password, ); - $url .= (false === strpos($url, '?') ? '?' : '&') . utf8_encode(http_build_query($parameters, '', '&')); + $url .= (false === strpos($url, '?') ? '?' : '&'); + $url .= utf8_encode(http_build_query($parameters, '', '&')); $event['request']->setUrl($url); - }); + }; break; - case GithubClient::AUTH_URL_TOKEN: - $this->client->getEventDispatcher()->addListener('request.create', function (Event $event) { + + case Client::AUTH_URL_TOKEN: + $handler = function (Event $event) use ($tokenOrLogin, $password) { $url = $event['request']->getUrl(); - $url .= (false === strpos($url, '?') ? '?' : '&') . utf8_encode(http_build_query(array('access_token' => $tokenOrLogin), '', '&')); + $url .= (false === strpos($url, '?') ? '?' : '&'); + $url .= utf8_encode(http_build_query(array('access_token' => $tokenOrLogin), '', '&')); $event['request']->setUrl($url); - }); + }; break; + default: throw new RuntimeException(sprintf('%s not yet implemented', $method)); break; } + + $this->client->getEventDispatcher()->addListener('request.create', $handler); } } From 2438e30a3688e1450f824bd27793e255bd59ad7b Mon Sep 17 00:00:00 2001 From: Evgeniy Guseletov Date: Tue, 22 Oct 2013 18:40:18 +0300 Subject: [PATCH 15/15] Revert error message --- lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php b/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php index b976f3027fd..792412928d9 100644 --- a/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php +++ b/lib/Github/HttpClient/Adapter/Buzz/Listener/ErrorListener.php @@ -57,7 +57,7 @@ public function postSend(RequestInterface $request, MessageInterface $response) foreach ($content['errors'] as $error) { switch ($error['code']) { case 'missing': - $errors[] = sprintf('Resource "%s" not exists anymore', $error['resource']); + $errors[] = sprintf('The %s %s does not exist, for resource "%s"', $error['field'], $error['value'], $error['resource']); break; case 'missing_field':