Skip to content

Commit c633cc7

Browse files
stloydstloyd
committed
Polished API for CachedHttpClient, removed BC break in HttpClientInterface 
1 parent 910a807 commit c633cc7

File tree

8 files changed

+95
-45
lines changed

8 files changed

+95
-45
lines changed

‎lib/Github/HttpClient/Cache/CacheInterface.php

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,26 +7,32 @@
77
/**
88
* Caches github api responses
99
*
10-
* @author Florian Klein <florian.klein@free.fr>
10+
* @author Florian Klein <florian.klein@free.fr>
1111
*/
1212
interface CacheInterface
1313
{
1414
/**
15-
* @param string the id of the cached resource
16-
* @return int the modified since timestamp
17-
**/
15+
* @param string $id The id of the cached resource
16+
*
17+
* @return null|integer The modified since timestamp
18+
*/
1819
public function getModifiedSince($id);
1920

2021
/**
21-
* @param string the id of the cached resource
22-
* @return Response The cached response object
23-
**/
22+
* @param string $id The id of the cached resource
23+
*
24+
* @return Response The cached response object
25+
*
26+
* @throws \InvalidArgumentException If cache data don't exists
27+
*/
2428
public function get($id);
2529

2630
/**
27-
* @param string the id of the cached resource
28-
* @param Response the response to cache
29-
**/
31+
* @param string $id The id of the cached resource
32+
* @param Response $response The response to cache
33+
*
34+
* @throws \InvalidArgumentException If cache data cannot be saved
35+
*/
3036
public function set($id, Response $response);
3137
}
3238

‎lib/Github/HttpClient/Cache/FilesystemCache.php

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,22 @@
66

77
class FilesystemCache implements CacheInterface
88
{
9+
/**
10+
* @var string
11+
*/
912
protected $path;
1013

14+
/**
15+
* @param null|string $path
16+
*/
1117
public function __construct($path = null)
1218
{
1319
$this->path = $path ?: sys_get_temp_dir().DIRECTORY_SEPARATOR.'php-github-api-cache';
1420
}
1521

22+
/**
23+
* {@inheritdoc}
24+
*/
1625
public function get($id)
1726
{
1827
if (false !== $content = @file_get_contents($this->getPath($id))) {
@@ -22,24 +31,37 @@ public function get($id)
2231
throw new \InvalidArgumentException(sprintf('File "%s" not found', $this->getPath($id)));
2332
}
2433

34+
/**
35+
* {@inheritdoc}
36+
*/
2537
public function set($id, Response $response)
2638
{
2739
if (!is_dir($this->path)) {
28-
mkdir($this->path, 0777, true);
40+
@mkdir($this->path, 0777, true);
2941
}
3042

31-
if (false === $bytes = @file_put_contents($this->getPath($id), serialize($response))) {
43+
if (false === @file_put_contents($this->getPath($id), serialize($response))) {
3244
throw new \InvalidArgumentException(sprintf('Cannot put content in file "%s"', $this->getPath($id)));
3345
}
3446
}
3547

48+
/**
49+
* {@inheritdoc}
50+
*/
3651
public function getModifiedSince($id)
3752
{
3853
if (file_exists($this->getPath($id))) {
3954
return filemtime($this->getPath($id));
4055
}
56+
57+
return null;
4158
}
4259

60+
/**
61+
* @param $id string
62+
*
63+
* @return string
64+
*/
4365
protected function getPath($id)
4466
{
4567
return sprintf('%s%s%s', rtrim($this->path, DIRECTORY_SEPARATOR), DIRECTORY_SEPARATOR, md5($id));

‎lib/Github/HttpClient/CachedHttpClient.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ public function __construct(array $options = array(), ClientInterface $client =
3333
$this->cache = $cache ?: new FilesystemCache;
3434
}
3535

36-
public function request($path, array $parameters = array(), $httpMethod = 'GET', array $headers = array(), Response $response = null)
36+
public function request($path, array $parameters = array(), $httpMethod = 'GET', array $headers = array())
3737
{
38-
$response = parent::request($path, $parameters, $httpMethod, $headers, $response);
38+
$response = parent::request($path, $parameters, $httpMethod, $headers);
3939

4040
$key = trim($this->options['base_url'].$path, '/');
4141
if ($response->isNotModified()) {
@@ -49,6 +49,7 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET',
4949

5050
/**
5151
* Create requests with If-Modified-Since headers
52+
*
5253
* @param string $httpMethod
5354
* @param string $url
5455
*
@@ -57,8 +58,7 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET',
5758
protected function createRequest($httpMethod, $url)
5859
{
5960
$request = parent::createRequest($httpMethod, $url);
60-
$modifiedSince = date('r', $this->cache->getModifiedSince($url));
61-
$request->addHeader(sprintf('If-Modified-Since: %s', $modifiedSince));
61+
$request->addHeader(sprintf('If-Modified-Since: %s', date('r', $this->cache->getModifiedSince($url))));
6262

6363
return $request;
6464
}

‎lib/Github/HttpClient/HttpClient.php

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ public function put($path, array $parameters = array(), array $headers = array()
144144
/**
145145
* {@inheritDoc}
146146
*/
147-
public function request($path, array $parameters = array(), $httpMethod = 'GET', array $headers = array(), Response $response = null)
147+
public function request($path, array $parameters = array(), $httpMethod = 'GET', array $headers = array())
148148
{
149149
$path = trim($this->options['base_url'].$path, '/');
150150

@@ -159,9 +159,7 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET',
159159
}
160160
}
161161

162-
if (null === $response) {
163-
$response = new Response;
164-
}
162+
$response = $this->createResponse();
165163

166164
try {
167165
$this->client->send($request, $response);
@@ -213,4 +211,12 @@ protected function createRequest($httpMethod, $url)
213211

214212
return $request;
215213
}
214+
215+
/**
216+
* @return Response
217+
*/
218+
protected function createResponse()
219+
{
220+
return new Response();
221+
}
216222
}

‎lib/Github/HttpClient/HttpClientInterface.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
namespace Github\HttpClient;
44

55
use Github\Exception\InvalidArgumentException;
6-
use Github\HttpClient\Message\Response;
76

87
/**
98
* Performs requests on GitHub API. API documentation should be self-explanatory.
@@ -78,7 +77,7 @@ public function delete($path, array $parameters = array(), array $headers = arra
7877
*
7978
* @return array Data
8079
*/
81-
public function request($path, array $parameters = array(), $httpMethod = 'GET', array $headers = array(), Response $response = null);
80+
public function request($path, array $parameters = array(), $httpMethod = 'GET', array $headers = array());
8281

8382
/**
8483
* Change an option value.

‎test/Github/Tests/HttpClient/CachedHttpClientTest.php

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
use Github\HttpClient\CachedHttpClient;
66
use Github\HttpClient\Message\Response;
7-
use Github\HttpClient\Message\Request;
87

98
class CachedHttpClientTest extends HttpClientTest
109
{
@@ -14,7 +13,8 @@ class CachedHttpClientTest extends HttpClientTest
1413
public function shouldCacheResponseAtFirstTime()
1514
{
1615
$cache = $this->getCacheMock();
17-
$httpClient = new CachedHttpClient(
16+
17+
$httpClient = new TestCachedHttpClient(
1818
array('base_url' => ''),
1919
$this->getMock('Buzz\Client\ClientInterface', array('setTimeout', 'setVerifyPeer', 'send')),
2020
$cache
@@ -35,18 +35,19 @@ public function shouldGetCachedResponseWhileResourceNotModified()
3535

3636
$cache = $this->getCacheMock();
3737

38-
$httpClient = new CachedHttpClient(
38+
$response = new Response;
39+
$response->addHeader('HTTP/1.1 304 Not Modified');
40+
41+
$httpClient = new TestCachedHttpClient(
3942
array('base_url' => ''),
4043
$client,
4144
$cache
4245
);
46+
$httpClient->fakeResponse = $response;
4347

4448
$cache->expects($this->once())->method('get')->with('test');
4549

46-
$response = new Response;
47-
$response->addHeader('HTTP/1.1 304 Not Modified');
48-
49-
$httpClient->request('test', array(), 'GET', array(), $response);
50+
$httpClient->get('test');
5051
}
5152

5253
/**
@@ -59,23 +60,34 @@ public function shouldRenewCacheWhenResourceHasChanged()
5960

6061
$cache = $this->getCacheMock();
6162

62-
$httpClient = new CachedHttpClient(
63+
$response = new Response;
64+
$response->addHeader('HTTP/1.1 200 OK');
65+
66+
$httpClient = new TestCachedHttpClient(
6367
array('base_url' => ''),
6468
$client,
6569
$cache
6670
);
67-
68-
$response = new Response;
69-
$response->addHeader('HTTP/1.1 200 OK');
71+
$httpClient->fakeResponse = $response;
7072

7173
$cache->expects($this->once())->method('set')->with('test', $response);
7274
$cache->expects($this->once())->method('getModifiedSince')->with('test')->will($this->returnValue(1256953732));
7375

74-
$httpClient->request('test', array(), 'GET', array(), $response);
76+
$httpClient->get('test');
7577
}
7678

7779
public function getCacheMock()
7880
{
7981
return $this->getMock('Github\HttpClient\Cache\CacheInterface');
8082
}
8183
}
84+
85+
class TestCachedHttpClient extends CachedHttpClient
86+
{
87+
public $fakeResponse;
88+
89+
protected function createResponse()
90+
{
91+
return $this->fakeResponse ?: new Response();
92+
}
93+
}

‎test/Github/Tests/HttpClient/HttpClientTest.php

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ public function shouldAllowToReturnRawContent()
163163
$client = $this->getBrowserMock();
164164

165165
$httpClient = new TestHttpClient(array(), $client);
166-
$httpClient->setFakeResponse($message);
166+
$httpClient->fakeResponse = $message;
167167

168168
$response = $httpClient->get($path, $parameters, $headers);
169169

@@ -186,7 +186,7 @@ public function shouldThrowExceptionWhenApiIsExceeded()
186186
$response->addHeader('X-RateLimit-Remaining: 0');
187187

188188
$httpClient = new TestHttpClient(array(), $this->getBrowserMock());
189-
$httpClient->setFakeResponse($response);
189+
$httpClient->fakeResponse = $response;
190190

191191
$httpClient->get($path, $parameters, $headers);
192192
}
@@ -201,11 +201,6 @@ class TestHttpClient extends HttpClient
201201
{
202202
public $fakeResponse;
203203

204-
public function setFakeResponse($response)
205-
{
206-
$this->fakeResponse = $response;
207-
}
208-
209204
public function getOption($name, $default = null)
210205
{
211206
return isset($this->options[$name]) ? $this->options[$name] : $default;
@@ -215,10 +210,10 @@ public function clearHeaders()
215210
{
216211
}
217212

218-
public function request($path, array $parameters = array(), $httpMethod = 'GET', array $headers = array(), Response $response = null)
213+
public function request($path, array $parameters = array(), $httpMethod = 'GET', array $headers = array())
219214
{
220-
$request = new Request($httpMethod);
221-
$response = $this->fakeResponse ? $this->fakeResponse : new Response();
215+
$request = $this->createRequest($httpMethod, $path);
216+
$response = $this->createResponse();
222217
if (0 < count($this->listeners)) {
223218
foreach ($this->listeners as $listener) {
224219
$listener->postSend($request, $response);
@@ -227,4 +222,14 @@ public function request($path, array $parameters = array(), $httpMethod = 'GET',
227222

228223
return $response;
229224
}
225+
226+
protected function createRequest($httpMethod, $url)
227+
{
228+
return new Request($httpMethod);
229+
}
230+
231+
protected function createResponse()
232+
{
233+
return $this->fakeResponse ?: new Response();
234+
}
230235
}

‎test/Github/Tests/Mock/TestHttpClient.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public function patch($path, array $parameters = array(), array $headers = array
5353
$this->requests['patch'][] = $path;
5454
}
5555

56-
public function put($path, array $options = array())
56+
public function put($path, array $options = array(), array $headers = array())
5757
{
5858
$this->requests['put'][] = $path;
5959
}

0 commit comments

Comments
 (0)