Skip to content

Commit 9d049da

Browse files
author
Roberto Espinoza
committed
Fix wrong class use for Request object (closes #26)
- Added test for methods under official endpoint proxy - Moved decoder logic into a class (easier to test) - Add test for response decoder
1 parent 90ba3bf commit 9d049da

File tree

4 files changed

+345
-73
lines changed

4 files changed

+345
-73
lines changed

‎src/Service/Decoder/ResponseDecoder.php‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace App\Service\Decoder;
5+
6+
use App\Service\Cache;
7+
use Http\Client\Exception\NetworkException;
8+
use Http\Client\HttpClient;
9+
use Nyholm\Psr7\Request;
10+
use Symfony\Component\Cache\Simple\FilesystemCache;
11+
12+
class ResponseDecoder
13+
{
14+
/**
15+
* @var bool
16+
*/
17+
private $cacheEndpoint;
18+
19+
/**
20+
* @var HttpClient
21+
*/
22+
private $client;
23+
24+
/**
25+
* @var FilesystemCache
26+
*/
27+
private $cache;
28+
29+
public function __construct(bool $cacheEndpoint, HttpClient $client, Cache $cache)
30+
{
31+
$this->cacheEndpoint = $cacheEndpoint;
32+
$this->client = $client;
33+
$this->cache = $cache();
34+
}
35+
36+
/**
37+
* @param Request $request
38+
* @return array|mixed|\Psr\Http\Message\StreamInterface
39+
* @throws \Http\Client\Exception
40+
* @throws \Psr\SimpleCache\InvalidArgumentException
41+
*/
42+
public function getDecodedResponse(Request $request)
43+
{
44+
try {
45+
$response = $this->client->sendRequest($request);
46+
$decodedResponse = json_decode($response->getBody(), true);
47+
48+
if(!in_array($response->getStatusCode(), range(200, 299))) {
49+
if ($this->cacheEndpoint && $this->cache->has($this->getCacheId($request))) {
50+
return $this->cache->get($this->getCacheId($request));
51+
}
52+
return [];
53+
}
54+
55+
if (json_last_error() !== JSON_ERROR_NONE) {
56+
$decodedResponse = $response->getBody();
57+
}
58+
59+
if ($this->cacheEndpoint) {
60+
$this->cache->set($this->getCacheId($request), $decodedResponse);
61+
}
62+
63+
return $decodedResponse;
64+
} catch (NetworkException $e) {
65+
if ($this->cacheEndpoint && $this->cache->has($this->getCacheId($request))) {
66+
return $this->cache->get($this->getCacheId($request));
67+
}
68+
throw $e;
69+
}
70+
}
71+
72+
/**
73+
* @param Request $request
74+
* @return string
75+
*/
76+
private function getCacheId(Request $request)
77+
{
78+
$id = $request->getMethod() . $request->getUri();
79+
return sha1(preg_replace('/[^A-Za-z0-9\.\- ]/', '', $id));
80+
}
81+
}

‎src/Service/OfficialEndpointProxy.php‎

Lines changed: 15 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,8 @@
1111

1212
namespace App\Service;
1313

14-
use App\Exception\OfficialEndpointNotAvailableExtension;
15-
use GuzzleHttp\Psr7\Request;
16-
use Http\Client\Exception\NetworkException;
17-
use Http\Client\HttpClient;
18-
use Symfony\Component\Cache\Simple\FilesystemCache;
19-
use Symfony\Component\HttpFoundation\Response;
14+
use App\Service\Decoder\ResponseDecoder;
15+
use Nyholm\Psr7\Request;
2016

2117
/**
2218
* Class OfficialEndpointProxy
@@ -28,32 +24,22 @@ class OfficialEndpointProxy
2824
/** @var string */
2925
private $endpoint;
3026

31-
/** @var bool */
32-
private $cacheEndpoint;
33-
34-
/** @var HttpClient */
35-
private $client;
36-
37-
/** @var FilesystemCache */
38-
private $cache;
27+
/**
28+
* @var ResponseDecoder
29+
*/
30+
private $decoder;
3931

4032
/**
4133
* OfficialEndpointProxy constructor.
4234
* @param string $officialEndpoint
43-
* @param bool $cacheEndpoint
44-
* @param HttpClient $client
45-
* @param Cache $cache
35+
* @param ResponseDecoder $decoder
4636
*/
4737
public function __construct(
4838
string $officialEndpoint,
49-
bool $cacheEndpoint,
50-
HttpClient $client,
51-
Cache $cache
39+
ResponseDecoder $decoder
5240
) {
53-
$this->cacheEndpoint = $cacheEndpoint;
54-
$this->client = $client;
5541
$this->endpoint = $officialEndpoint;
56-
$this->cache = $cache();
42+
$this->decoder = $decoder;
5743
}
5844

5945
/**
@@ -62,11 +48,12 @@ public function __construct(
6248
* @return array
6349
* @throws \Exception
6450
* @throws \Http\Client\Exception
51+
* @throws \Psr\SimpleCache\InvalidArgumentException
6552
*/
6653
public function getAliases()
6754
{
6855
$request = new Request('GET', $this->endpoint . 'aliases.json');
69-
return $this->getDecodedResponse($request);
56+
return $this->decoder->getDecodedResponse($request);
7057
}
7158

7259
/**
@@ -75,11 +62,12 @@ public function getAliases()
7562
* @return array
7663
* @throws \Exception
7764
* @throws \Http\Client\Exception
65+
* @throws \Psr\SimpleCache\InvalidArgumentException
7866
*/
7967
public function getVersions()
8068
{
8169
$request = new Request('GET', $this->endpoint . 'versions.json');
82-
return $this->getDecodedResponse($request);
70+
return $this->decoder->getDecodedResponse($request);
8371
}
8472

8573
/**
@@ -89,57 +77,11 @@ public function getVersions()
8977
* @return array|string
9078
* @throws \Exception
9179
* @throws \Http\Client\Exception
80+
* @throws \Psr\SimpleCache\InvalidArgumentException
9281
*/
9382
public function getPackages(string $packagesRequestString)
9483
{
9584
$request = new Request('GET', $this->endpoint . 'p/' . $packagesRequestString);
96-
return $this->getDecodedResponse($request);
97-
}
98-
99-
/**
100-
* @param Request $request
101-
* @return array|string
102-
* @throws \Exception
103-
* @throws \Http\Client\Exception
104-
*/
105-
private function getDecodedResponse(Request $request)
106-
{
107-
try {
108-
$response = $this->client->sendRequest($request);
109-
$decodedResponse = json_decode($response->getBody(), true);
110-
111-
if(!in_array($response->getStatusCode(), range(200, 299))) {
112-
if ($this->cacheEndpoint && $this->cache->has($this->getCacheId($request))) {
113-
return $this->cache->get($this->getCacheId($request));
114-
}
115-
return [];
116-
}
117-
118-
if (json_last_error() !== JSON_ERROR_NONE) {
119-
$decodedResponse = $response->getBody();
120-
}
121-
122-
if ($this->cacheEndpoint) {
123-
$this->cache->set($this->getCacheId($request), $decodedResponse);
124-
}
125-
126-
return $decodedResponse;
127-
} catch (NetworkException $e) {
128-
if ($this->cacheEndpoint && $this->cache->has($this->getCacheId($request))) {
129-
return $this->cache->get($this->getCacheId($request));
130-
}
131-
throw $e;
132-
}
85+
return $this->decoder->getDecodedResponse($request);
13386
}
134-
135-
/**
136-
* @param Request $request
137-
* @return string
138-
*/
139-
private function getCacheId(Request $request)
140-
{
141-
$id = $request->getMethod() . $request->getUri();
142-
return sha1(preg_replace('/[^A-Za-z0-9\.\- ]/', '', $id));
143-
}
144-
14587
}

‎tests/Service/Decoder/ResponseDecoderTest.php‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace App\Tests\Service\Decoder;
5+
6+
use App\Service\Cache;
7+
use App\Service\Decoder\ResponseDecoder;
8+
use Http\Client\Exception\NetworkException;
9+
use Http\Client\HttpClient;
10+
use Nyholm\Psr7\Request;
11+
use PHPUnit\Framework\TestCase;
12+
use Psr\Http\Message\ResponseInterface;
13+
use Symfony\Component\Cache\Simple\FilesystemCache;
14+
15+
class ResponseDecoderTest extends TestCase
16+
{
17+
/**
18+
* @var HttpClient|\Prophecy\Prophecy\ObjectProphecy
19+
*/
20+
private $client;
21+
22+
/**
23+
* @var Cache|\Prophecy\Prophecy\ObjectProphecy
24+
*/
25+
private $cache;
26+
27+
/**
28+
* @var \Prophecy\Prophecy\ObjectProphecy|FilesystemCache
29+
*/
30+
private $simpleCache;
31+
32+
protected function setUp()
33+
{
34+
$this->client = $this->prophesize(HttpClient::class);
35+
$this->cache = $this->prophesize(Cache::class);
36+
$this->simpleCache = $this->prophesize(FilesystemCache::class);
37+
$this->cache->__invoke()->willReturn($this->simpleCache->reveal());
38+
}
39+
40+
public function testGetDecodedResponse()
41+
{
42+
$decoder = new ResponseDecoder(false, $this->client->reveal(), $this->cache->reveal());
43+
44+
$request = new Request('GET', 'endpoint/data.json');
45+
$response = $this->prophesize(ResponseInterface::class);
46+
$response->getStatusCode()->willReturn(200);
47+
$response->getBody()->willReturn(json_encode(['json' => 'data']));
48+
49+
$this->client->sendRequest($request)->willReturn($response->reveal());
50+
51+
$this->assertSame(
52+
['json' => 'data'],
53+
$decoder->getDecodedResponse($request)
54+
);
55+
}
56+
57+
public function testGetDecodedResponseEmptyOnRequestError()
58+
{
59+
$decoder = new ResponseDecoder(false, $this->client->reveal(), $this->cache->reveal());
60+
61+
$request = new Request('GET', 'endpoint/data.json');
62+
$response = $this->prophesize(ResponseInterface::class);
63+
$response->getStatusCode()->willReturn(500);
64+
$response->getBody()->willReturn('');
65+
66+
$this->client->sendRequest($request)->willReturn($response->reveal());
67+
68+
$this->assertSame(
69+
[],
70+
$decoder->getDecodedResponse($request)
71+
);
72+
}
73+
74+
public function testGetDecodedResponseFromCacheOnRequestError()
75+
{
76+
$decoder = new ResponseDecoder(true, $this->client->reveal(), $this->cache->reveal());
77+
78+
$request = new Request('GET', 'endpoint/data.json');
79+
$response = $this->prophesize(ResponseInterface::class);
80+
$response->getStatusCode()->willReturn(500);
81+
$response->getBody()->willReturn('');
82+
83+
$this->simpleCache->has('4429b090fd82239e188859ae626162e5e790b4db')->willReturn(true);
84+
$this->simpleCache->get('4429b090fd82239e188859ae626162e5e790b4db')->willReturn(['json' => 'cache']);
85+
86+
$this->client->sendRequest($request)->willReturn($response->reveal());
87+
88+
$this->assertSame(
89+
['json' => 'cache'],
90+
$decoder->getDecodedResponse($request)
91+
);
92+
}
93+
94+
public function testGetDecodedResponseCachesDataIfEnabled()
95+
{
96+
$decoder = new ResponseDecoder(true, $this->client->reveal(), $this->cache->reveal());
97+
98+
$request = new Request('GET', 'endpoint/data.json');
99+
$response = $this->prophesize(ResponseInterface::class);
100+
$response->getStatusCode()->willReturn(200);
101+
$response->getBody()->willReturn(json_encode(['json' => 'cache']));
102+
103+
$this->simpleCache->has('4429b090fd82239e188859ae626162e5e790b4db')->willReturn(false);
104+
$this->simpleCache->set('4429b090fd82239e188859ae626162e5e790b4db', ['json' => 'cache'])->shouldBeCalledOnce();
105+
106+
$this->client->sendRequest($request)->willReturn($response->reveal());
107+
108+
$this->assertSame(
109+
['json' => 'cache'],
110+
$decoder->getDecodedResponse($request)
111+
);
112+
}
113+
114+
public function testGetDecodedResponseFromCacheWhenNetworkFails()
115+
{
116+
$decoder = new ResponseDecoder(true, $this->client->reveal(), $this->cache->reveal());
117+
118+
$request = new Request('GET', 'endpoint/data.json');
119+
120+
$this->simpleCache->has('4429b090fd82239e188859ae626162e5e790b4db')->willReturn(true);
121+
$this->simpleCache->get('4429b090fd82239e188859ae626162e5e790b4db')->willReturn(['json' => 'cache']);
122+
123+
$this->client->sendRequest($request)->willThrow(NetworkException::class);
124+
125+
$this->assertSame(
126+
['json' => 'cache'],
127+
$decoder->getDecodedResponse($request)
128+
);
129+
}
130+
131+
/**
132+
* @throws \Http\Client\Exception
133+
* @throws \Psr\SimpleCache\InvalidArgumentException
134+
* @expectedException \Http\Client\Exception\NetworkException
135+
*/
136+
public function testGetDecodedResponseThrowsNetworkExceptionWhenClientFailsAndNoCachedVersion()
137+
{
138+
$decoder = new ResponseDecoder(true, $this->client->reveal(), $this->cache->reveal());
139+
$request = new Request('GET', 'endpoint/data.json');
140+
141+
$this->simpleCache->has('4429b090fd82239e188859ae626162e5e790b4db')->willReturn(false);
142+
$this->client->sendRequest($request)->willThrow(NetworkException::class);
143+
144+
$decoder->getDecodedResponse($request);
145+
}
146+
147+
/**
148+
* @todo this seems like a bug, we should not store in the cache invalid json? should we throw an error?
149+
* @throws \Http\Client\Exception
150+
* @throws \Psr\SimpleCache\InvalidArgumentException
151+
*/
152+
public function testGetDecodedResponseReturnsBodyWhenJsonDecodingFails()
153+
{
154+
$decoder = new ResponseDecoder(true, $this->client->reveal(), $this->cache->reveal());
155+
156+
$request = new Request('GET', 'endpoint/data.json');
157+
$response = $this->prophesize(ResponseInterface::class);
158+
$response->getStatusCode()->willReturn(200);
159+
$response->getBody()->willReturn('{invalid_json');
160+
161+
$this->client->sendRequest($request)->willReturn($response->reveal());
162+
$this->simpleCache->set('4429b090fd82239e188859ae626162e5e790b4db', '{invalid_json')->shouldBeCalledOnce();
163+
164+
$this->assertSame(
165+
'{invalid_json',
166+
$decoder->getDecodedResponse($request)
167+
);
168+
169+
$this->markAsRisky();
170+
}
171+
}

0 commit comments

Comments
 (0)