Skip to content

Commit 51ba284

Browse files
NyholmNyholm
committed
Bugfixes
1 parent 86d9d00 commit 51ba284

File tree

6 files changed

+112
-79
lines changed

6 files changed

+112
-79
lines changed

‎lib/Github/Client.php

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -111,21 +111,21 @@ class Client
111111
public function __construct(Builder $httpClientBuilder = null, $apiVersion = null, $enterpriseUrl = null)
112112
{
113113
$this->responseHistory = new History();
114-
$this->httpClientBuilder = new Builder();
115-
$this->httpClientBuilder->addDefaultHeaders([
114+
$this->httpClientBuilder = $httpClientBuilder ?: new Builder();
115+
$this->getHttpClientBuilder()->addDefaultHeaders([
116116
'Accept' => sprintf('application/vnd.github.%s+json', $this->getApiVersion()),
117117
]);
118118

119-
$this->httpClientBuilder->addPlugin(new GithubExceptionThrower());
120-
$this->httpClientBuilder->addPlugin(new Plugin\HistoryPlugin($this->responseHistory));
121-
$this->httpClientBuilder->addPlugin(new Plugin\RedirectPlugin());
122-
$this->httpClientBuilder->addPlugin(new Plugin\AddHostPlugin(UriFactoryDiscovery::find()->createUri('https://api.github.com')));
123-
$this->httpClientBuilder->addPlugin(new Plugin\HeaderDefaultsPlugin(array(
119+
$this->getHttpClientBuilder()->addPlugin(new GithubExceptionThrower());
120+
$this->getHttpClientBuilder()->addPlugin(new Plugin\HistoryPlugin($this->responseHistory));
121+
$this->getHttpClientBuilder()->addPlugin(new Plugin\RedirectPlugin());
122+
$this->getHttpClientBuilder()->addPlugin(new Plugin\AddHostPlugin(UriFactoryDiscovery::find()->createUri('https://api.github.com')));
123+
$this->getHttpClientBuilder()->addPlugin(new Plugin\HeaderDefaultsPlugin(array(
124124
'User-Agent' => 'php-github-api (http://github.com/KnpLabs/php-github-api)',
125125
)));
126126

127127
$this->apiVersion = $apiVersion ?: 'v3';
128-
$this->httpClientBuilder->addHeaders(['Accept' => sprintf('application/vnd.github.%s+json', $this->apiVersion)]);
128+
$this->getHttpClientBuilder()->addHeaders(['Accept' => sprintf('application/vnd.github.%s+json', $this->apiVersion)]);
129129

130130
if ($enterpriseUrl) {
131131
$this->setEnterpriseUrl($enterpriseUrl);
@@ -279,8 +279,8 @@ public function authenticate($tokenOrLogin, $password = null, $authMethod = null
279279
$authMethod = self::AUTH_HTTP_PASSWORD;
280280
}
281281

282-
$this->httpClientBuilder->removePlugin(Authentication::class);
283-
$this->httpClientBuilder->addPlugin(new Authentication($tokenOrLogin, $password, $authMethod));
282+
$this->getHttpClientBuilder()->removePlugin(Authentication::class);
283+
$this->getHttpClientBuilder()->addPlugin(new Authentication($tokenOrLogin, $password, $authMethod));
284284
}
285285

286286
/**
@@ -290,11 +290,11 @@ public function authenticate($tokenOrLogin, $password = null, $authMethod = null
290290
*/
291291
private function setEnterpriseUrl($enterpriseUrl)
292292
{
293-
$this->httpClientBuilder->removePlugin(Plugin\AddHostPlugin::class);
294-
$this->httpClientBuilder->removePlugin(PathPrepend::class);
293+
$this->getHttpClientBuilder()->removePlugin(Plugin\AddHostPlugin::class);
294+
$this->getHttpClientBuilder()->removePlugin(PathPrepend::class);
295295

296-
$this->httpClientBuilder->addPlugin(new Plugin\AddHostPlugin(UriFactoryDiscovery::find()->createUri($enterpriseUrl)));
297-
$this->httpClientBuilder->addPlugin(new PathPrepend(sprintf('/api/%s/', $this->getApiVersion())));
296+
$this->getHttpClientBuilder()->addPlugin(new Plugin\AddHostPlugin(UriFactoryDiscovery::find()->createUri($enterpriseUrl)));
297+
$this->getHttpClientBuilder()->addPlugin(new PathPrepend(sprintf('/api/%s/', $this->getApiVersion())));
298298
}
299299

300300
/**
@@ -313,15 +313,15 @@ public function getApiVersion()
313313
*/
314314
public function addCache(CacheItemPoolInterface $cachePool, array $config = [])
315315
{
316-
$this->httpClientBuilder->addCache($cachePool, $config);
316+
$this->getHttpClientBuilder()->addCache($cachePool, $config);
317317
}
318318

319319
/**
320320
* Remove the cache plugin.
321321
*/
322322
public function removeCache()
323323
{
324-
$this->httpClientBuilder->removeCache();
324+
$this->getHttpClientBuilder()->removeCache();
325325
}
326326

327327
/**
@@ -353,6 +353,14 @@ public function getLastResponse()
353353
*/
354354
public function getHttpClient()
355355
{
356-
return $this->httpClientBuilder->getHttpClient();
356+
return $this->getHttpClientBuilder()->getHttpClient();
357+
}
358+
359+
/**
360+
* @return Builder
361+
*/
362+
protected function getHttpClientBuilder()
363+
{
364+
return $this->httpClientBuilder;
357365
}
358366
}

‎lib/Github/HttpClient/Builder.php

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -142,15 +142,6 @@ public function removePlugin($fqcn)
142142
}
143143
}
144144

145-
/**
146-
* @param HttpClient $httpClient
147-
*/
148-
public function setHttpClient(HttpClient $httpClient)
149-
{
150-
$this->httpClientModified = true;
151-
$this->httpClient = $httpClient;
152-
}
153-
154145
/**
155146
* Clears used headers.
156147
*/

‎test/Github/Tests/Api/TestCase.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Github\Tests\Api;
44

5+
use Github\HttpClient\Builder;
56
use ReflectionMethod;
67

78
abstract class TestCase extends \PHPUnit_Framework_TestCase
@@ -23,7 +24,7 @@ protected function getApiMock()
2324
->expects($this->any())
2425
->method('sendRequest');
2526

26-
$client = new \Github\Client($httpClient);
27+
$client = new \Github\Client(new Builder($httpClient));
2728

2829
return $this->getMockBuilder($this->getApiClass())
2930
->setMethods(array('get', 'post', 'postRaw', 'patch', 'delete', 'put', 'head'))

‎test/Github/Tests/ClientTest.php

Lines changed: 27 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Github\Api;
66
use Github\Client;
77
use Github\Exception\BadMethodCallException;
8+
use Github\HttpClient\Builder;
89
use Github\HttpClient\Plugin\Authentication;
910
use Http\Client\Common\Plugin;
1011

@@ -23,11 +24,12 @@ public function shouldNotHaveToPassHttpClientToConstructor()
2324
/**
2425
* @test
2526
*/
26-
public function shouldPassHttpClientInterfaceToConstructor()
27+
public function shouldPassHttpClientBulderInterfaceToConstructor()
2728
{
2829
$httpClientMock = $this->getMockBuilder(\Http\Client\HttpClient::class)
2930
->getMock();
30-
$client = new Client($httpClientMock);
31+
32+
$client = new Client(new Builder($httpClientMock));
3133

3234
$this->assertInstanceOf(\Http\Client\HttpClient::class, $client->getHttpClient());
3335
}
@@ -38,17 +40,25 @@ public function shouldPassHttpClientInterfaceToConstructor()
3840
*/
3941
public function shouldAuthenticateUsingAllGivenParameters($login, $password, $method)
4042
{
41-
$client = $this->getMockBuilder(\Github\Client::class)
43+
$builder = $this->getMockBuilder(\Github\HttpClient\Builder::class)
4244
->setMethods(array('addPlugin', 'removePlugin'))
45+
->disableOriginalConstructor()
4346
->getMock();
44-
$client->expects($this->once())
47+
$builder->expects($this->once())
4548
->method('addPlugin')
4649
->with($this->equalTo(new Authentication($login, $password, $method)));
47-
48-
$client->expects($this->once())
50+
$builder->expects($this->once())
4951
->method('removePlugin')
5052
->with(Authentication::class);
5153

54+
$client = $this->getMockBuilder(\Github\Client::class)
55+
->disableOriginalConstructor()
56+
->setMethods(['getHttpClientBuilder'])
57+
->getMock();
58+
$client->expects($this->any())
59+
->method('getHttpClientBuilder')
60+
->willReturn($builder);
61+
5262
$client->authenticate($login, $password, $method);
5363
}
5464

@@ -68,17 +78,25 @@ public function getAuthenticationFullData()
6878
*/
6979
public function shouldAuthenticateUsingGivenParameters($token, $method)
7080
{
71-
$client = $this->getMockBuilder(\Github\Client::class)
81+
$builder = $this->getMockBuilder(\Github\HttpClient\Builder::class)
7282
->setMethods(array('addPlugin', 'removePlugin'))
7383
->getMock();
74-
$client->expects($this->once())
84+
$builder->expects($this->once())
7585
->method('addPlugin')
7686
->with($this->equalTo(new Authentication($token, null, $method)));
7787

78-
$client->expects($this->once())
88+
$builder->expects($this->once())
7989
->method('removePlugin')
8090
->with(Authentication::class);
8191

92+
$client = $this->getMockBuilder(\Github\Client::class)
93+
->disableOriginalConstructor()
94+
->setMethods(['getHttpClientBuilder'])
95+
->getMock();
96+
$client->expects($this->any())
97+
->method('getHttpClientBuilder')
98+
->willReturn($builder);
99+
82100
$client->authenticate($token, $method);
83101
}
84102

@@ -101,47 +119,6 @@ public function shouldThrowExceptionWhenAuthenticatingWithoutMethodSet()
101119
$client->authenticate('login', null, null);
102120
}
103121

104-
/**
105-
* @test
106-
*/
107-
public function shouldClearHeaders()
108-
{
109-
$client = $this->getMockBuilder(\Github\Client::class)
110-
->setMethods(array('addPlugin', 'removePlugin'))
111-
->getMock();
112-
$client->expects($this->once())
113-
->method('addPlugin')
114-
->with($this->isInstanceOf(Plugin\HeaderAppendPlugin::class));
115-
116-
$client->expects($this->once())
117-
->method('removePlugin')
118-
->with(Plugin\HeaderAppendPlugin::class);
119-
120-
$client->clearHeaders();
121-
}
122-
123-
/**
124-
* @test
125-
*/
126-
public function shouldAddHeaders()
127-
{
128-
$headers = array('header1', 'header2');
129-
130-
$client = $this->getMockBuilder(\Github\Client::class)
131-
->setMethods(array('addPlugin', 'removePlugin'))
132-
->getMock();
133-
$client->expects($this->once())
134-
->method('addPlugin')
135-
// TODO verify that headers exists
136-
->with($this->isInstanceOf(Plugin\HeaderAppendPlugin::class));
137-
138-
$client->expects($this->once())
139-
->method('removePlugin')
140-
->with(Plugin\HeaderAppendPlugin::class);
141-
142-
$client->addHeaders($headers);
143-
}
144-
145122
/**
146123
* @test
147124
* @dataProvider getApiClassesProvider

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php
2+
3+
namespace Github\Tests\HttpClient;
4+
5+
use Github\Api;
6+
use Github\Client;
7+
use Github\Exception\BadMethodCallException;
8+
use Github\HttpClient\Plugin\Authentication;
9+
use Http\Client\Common\Plugin;
10+
11+
class BuilderTest extends \PHPUnit_Framework_TestCase
12+
{
13+
/**
14+
* @test
15+
*/
16+
public function shouldClearHeaders()
17+
{
18+
$builder = $this->getMockBuilder(\Github\HttpClient\Builder::class)
19+
->setMethods(array('addPlugin', 'removePlugin'))
20+
->getMock();
21+
$builder->expects($this->once())
22+
->method('addPlugin')
23+
->with($this->isInstanceOf(Plugin\HeaderAppendPlugin::class));
24+
25+
$builder->expects($this->once())
26+
->method('removePlugin')
27+
->with(Plugin\HeaderAppendPlugin::class);
28+
29+
$builder->clearHeaders();
30+
}
31+
32+
33+
34+
/**
35+
* @test
36+
*/
37+
public function shouldAddHeaders()
38+
{
39+
$headers = array('header1', 'header2');
40+
41+
$client = $this->getMockBuilder(\Github\HttpClient\Builder::class)
42+
->setMethods(array('addPlugin', 'removePlugin'))
43+
->getMock();
44+
$client->expects($this->once())
45+
->method('addPlugin')
46+
// TODO verify that headers exists
47+
->with($this->isInstanceOf(Plugin\HeaderAppendPlugin::class));
48+
49+
$client->expects($this->once())
50+
->method('removePlugin')
51+
->with(Plugin\HeaderAppendPlugin::class);
52+
53+
$client->addHeaders($headers);
54+
}
55+
}

‎test/Github/Tests/ResultPagerTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Github\Api\Organization\Members;
77
use Github\Api\Search;
88
use Github\Client;
9+
use Github\HttpClient\Builder;
910
use Github\ResultPager;
1011
use Github\Tests\Mock\PaginatedResponse;
1112
use Http\Client\HttpClient;
@@ -39,7 +40,7 @@ public function shouldGetAllResults()
3940
->method('sendRequest')
4041
->will($this->returnValue($response));
4142

42-
$client = new Client($httpClientMock);
43+
$client = new Client(new Builder($httpClientMock));
4344

4445
// memberApi Mock
4546
$memberApi = new Members($client);
@@ -86,7 +87,7 @@ public function shouldGetAllSearchResults()
8687
->method('sendRequest')
8788
->will($this->returnValue($response));
8889

89-
$client = new Client($httpClientMock);
90+
$client = new Client(new Builder($httpClientMock));
9091

9192
$searchApi = new Search($client);
9293
$method = 'users';

0 commit comments

Comments
 (0)