-
Notifications
You must be signed in to change notification settings - Fork 6
Added a PSR6 cache plugin #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
<?php | ||
|
||
namespace spec\Http\Client\Plugin; | ||
|
||
use Http\Client\Tools\Promise\FulfilledPromise; | ||
use PhpSpec\ObjectBehavior; | ||
use Psr\Cache\CacheItemInterface; | ||
use Psr\Cache\CacheItemPoolInterface; | ||
use Psr\Http\Message\RequestInterface; | ||
use Psr\Http\Message\ResponseInterface; | ||
|
||
class CachePluginSpec extends ObjectBehavior | ||
{ | ||
function let(CacheItemPoolInterface $pool) | ||
{ | ||
$this->beConstructedWith($pool, ['default_ttl'=>60]); | ||
} | ||
|
||
function it_is_initializable(CacheItemPoolInterface $pool) | ||
{ | ||
$this->shouldHaveType('Http\Client\Plugin\CachePlugin'); | ||
} | ||
|
||
function it_is_a_plugin() | ||
{ | ||
$this->shouldImplement('Http\Client\Plugin\Plugin'); | ||
} | ||
|
||
function it_caches_responses(CacheItemPoolInterface $pool, CacheItemInterface $item, RequestInterface $request, ResponseInterface $response) | ||
{ | ||
$request->getMethod()->willReturn('GET'); | ||
$request->getUri()->willReturn('/'); | ||
$response->getStatusCode()->willReturn(200); | ||
$response->getHeader('Cache-Control')->willReturn(array()); | ||
$response->getHeader('Expires')->willReturn(array()); | ||
|
||
$pool->getItem('e3b717d5883a45ef9493d009741f7c64')->shouldBeCalled()->willReturn($item); | ||
$item->isHit()->willReturn(false); | ||
$item->set($response)->willReturn($item)->shouldBeCalled(); | ||
$item->expiresAfter(60)->willReturn($item)->shouldBeCalled(); | ||
$pool->save($item)->shouldBeCalled(); | ||
|
||
$next = function (RequestInterface $request) use ($response) { | ||
return new FulfilledPromise($response->getWrappedObject()); | ||
}; | ||
|
||
$this->handleRequest($request, $next, function () {}); | ||
} | ||
|
||
function it_doesnt_store_failed_responses(CacheItemPoolInterface $pool, CacheItemInterface $item, RequestInterface $request, ResponseInterface $response) | ||
{ | ||
$request->getMethod()->willReturn('GET'); | ||
$request->getUri()->willReturn('/'); | ||
$response->getStatusCode()->willReturn(400); | ||
$response->getHeader('Cache-Control')->willReturn(array()); | ||
$response->getHeader('Expires')->willReturn(array()); | ||
|
||
$pool->getItem('e3b717d5883a45ef9493d009741f7c64')->shouldBeCalled()->willReturn($item); | ||
$item->isHit()->willReturn(false); | ||
|
||
$next = function (RequestInterface $request) use ($response) { | ||
return new FulfilledPromise($response->getWrappedObject()); | ||
}; | ||
|
||
$this->handleRequest($request, $next, function () {}); | ||
} | ||
|
||
function it_doesnt_store_post_requests(CacheItemPoolInterface $pool, CacheItemInterface $item, RequestInterface $request, ResponseInterface $response) | ||
{ | ||
$request->getMethod()->willReturn('POST'); | ||
$request->getUri()->willReturn('/'); | ||
|
||
$next = function (RequestInterface $request) use ($response) { | ||
return new FulfilledPromise($response->getWrappedObject()); | ||
}; | ||
|
||
$this->handleRequest($request, $next, function () {}); | ||
} | ||
|
||
|
||
function it_calculate_age_from_response(CacheItemPoolInterface $pool, CacheItemInterface $item, RequestInterface $request, ResponseInterface $response) | ||
{ | ||
$request->getMethod()->willReturn('GET'); | ||
$request->getUri()->willReturn('/'); | ||
$response->getStatusCode()->willReturn(200); | ||
$response->getHeader('Cache-Control')->willReturn(array('max-age=40')); | ||
$response->getHeader('Age')->willReturn(array('15')); | ||
$response->getHeader('Expires')->willReturn(array()); | ||
|
||
$pool->getItem('e3b717d5883a45ef9493d009741f7c64')->shouldBeCalled()->willReturn($item); | ||
$item->isHit()->willReturn(false); | ||
|
||
// 40-15 should be 25 | ||
$item->set($response)->willReturn($item)->shouldBeCalled(); | ||
$item->expiresAfter(25)->willReturn($item)->shouldBeCalled(); | ||
$pool->save($item)->shouldBeCalled(); | ||
|
||
$next = function (RequestInterface $request) use ($response) { | ||
return new FulfilledPromise($response->getWrappedObject()); | ||
}; | ||
|
||
$this->handleRequest($request, $next, function () {}); | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,168 @@ | ||
<?php | ||
|
||
namespace Http\Client\Plugin; | ||
|
||
use Http\Client\Tools\Promise\FulfilledPromise; | ||
use Psr\Cache\CacheItemPoolInterface; | ||
use Psr\Http\Message\RequestInterface; | ||
use Psr\Http\Message\ResponseInterface; | ||
|
||
/** | ||
* Allow for caching a response. | ||
* | ||
* @author Tobias Nyholm <[email protected]> | ||
*/ | ||
class CachePlugin implements Plugin | ||
{ | ||
/** | ||
* @var CacheItemPoolInterface | ||
*/ | ||
private $pool; | ||
|
||
/** | ||
* Default time to store object in cache. This value is used if CachePlugin::respectCacheHeaders is false or | ||
* if cache headers are missing. | ||
* | ||
* @var int | ||
*/ | ||
private $defaultTtl; | ||
|
||
/** | ||
* Look at the cache headers to know how long this response is going to be cached. | ||
* | ||
* @var bool | ||
*/ | ||
private $respectCacheHeaders; | ||
|
||
/** | ||
* @param CacheItemPoolInterface $pool | ||
* @param array $options | ||
*/ | ||
public function __construct(CacheItemPoolInterface $pool, array $options = []) | ||
{ | ||
$this->pool = $pool; | ||
$this->defaultTtl = isset($options['default_ttl']) ? $options['default_ttl'] : null; | ||
$this->respectCacheHeaders = isset($options['respect_cache_headers']) ? $options['respect_cache_headers'] : true; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function handleRequest(RequestInterface $request, callable $next, callable $first) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inheritdoc |
||
{ | ||
$method = strtoupper($request->getMethod()); | ||
|
||
// if the request not is cachable, move to $next | ||
if ($method !== 'GET' && $method !== 'HEAD') { | ||
return $next($request); | ||
} | ||
|
||
// If we can cache the request | ||
$key = $this->createCacheKey($request); | ||
$cacheItem = $this->pool->getItem($key); | ||
|
||
if ($cacheItem->isHit()) { | ||
// return cached response | ||
return new FulfilledPromise($cacheItem->get()); | ||
} | ||
|
||
return $next($request)->then(function (ResponseInterface $response) use ($cacheItem) { | ||
if ($this->isCacheable($response)) { | ||
$cacheItem->set($response) | ||
->expiresAfter($this->getMaxAge($response)); | ||
$this->pool->save($cacheItem); | ||
} | ||
|
||
return $response; | ||
}); | ||
} | ||
|
||
/** | ||
* Verify that we can cache this response. | ||
* | ||
* @param ResponseInterface $response | ||
* | ||
* @return bool | ||
*/ | ||
protected function isCacheable(ResponseInterface $response) | ||
{ | ||
if (!in_array($response->getStatusCode(), [200, 203, 300, 301, 302, 404, 410])) { | ||
return false; | ||
} | ||
if ($this->getCacheControlDirective($response, 'no-store') || $this->getCacheControlDirective($response, 'private')) { | ||
return false; | ||
} | ||
|
||
return true; | ||
} | ||
|
||
/** | ||
* Returns the value of a parameter in the cache control header. If not found we return false. If found with no | ||
* value return true. | ||
* | ||
* @param ResponseInterface $response | ||
* @param string $name | ||
* | ||
* @return bool|string | ||
*/ | ||
private function getCacheControlDirective(ResponseInterface $response, $name) | ||
{ | ||
$headers = $response->getHeader('Cache-Control'); | ||
foreach ($headers as $header) { | ||
if (preg_match(sprintf('|%s=?([0-9]+)?|i', $name), $header, $matches)) { | ||
|
||
// return the value for $name if it exists | ||
if (isset($matches[1])) { | ||
return $matches[1]; | ||
} | ||
|
||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
/** | ||
* @param RequestInterface $request | ||
* | ||
* @return string | ||
*/ | ||
private function createCacheKey(RequestInterface $request) | ||
{ | ||
return md5($request->getMethod().' '.$request->getUri()); | ||
} | ||
|
||
/** | ||
* Get a ttl in seconds. It could return null if we do not respect cache headers and got no defaultTtl. | ||
* | ||
* @param ResponseInterface $response | ||
* | ||
* @return int|null | ||
*/ | ||
private function getMaxAge(ResponseInterface $response) | ||
{ | ||
if (!$this->respectCacheHeaders) { | ||
return $this->defaultTtl; | ||
} | ||
|
||
// check for max age in the Cache-Control header | ||
$maxAge = $this->getCacheControlDirective($response, 'max-age'); | ||
if (!is_bool($maxAge)) { | ||
$ageHeaders = $response->getHeader('Age'); | ||
foreach ($ageHeaders as $age) { | ||
return $maxAge - ((int) $age); | ||
} | ||
|
||
return $maxAge; | ||
} | ||
|
||
// check for ttl in the Expires header | ||
$headers = $response->getHeader('Expires'); | ||
foreach ($headers as $header) { | ||
return (new \DateTime($header))->getTimestamp() - (new \DateTime())->getTimestamp(); | ||
} | ||
|
||
return $this->defaultTtl; | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a fan of having an options array for only 2 paramters, can we not use direct parameter in the constructor ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We mentioned it here and I changed to an array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the discussion in the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but expect for having less characters in the constructor is there another reason @dbu ?
Having options array is useful for dynamic keys / parameters, however in this case we loose the potential of type hinting and make things more confuse for the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not just less characters. Less number of parameters. Not just now, but in the future as well.
Adding additional arguments is not forbidden. Just config-like options are groupped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but if a config like option is an instance of an object ?
Don't really mind to have those kinds of options array, but in my experience if they are not well checked (with something like symfony/options resolver for example) they often lead to many bugs and user misunderstanding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we depend on the symfony OptionsResolver already. i would propose to emply the resolver here to do validation.
the plus side of the array is that adding more optional configuration with default value will be simply with an array, and much trickier if they all need an explicit order. (for BC, parameter order can not change, so required parameters could end up after optional parameters and whatnot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOL, I've never thought about parameter order being a BC change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.