From 63c3f5fc41937cc7378f2c2e6b2c6521978ff7a6 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 18 Dec 2015 22:24:49 +0100 Subject: [PATCH 1/4] Make sure we cache the message body --- composer.json | 3 ++- spec/CachePluginSpec.php | 22 ++++++++++++++++------ src/CachePlugin.php | 18 +++++++++++++++--- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/composer.json b/composer.json index d965b15..817417e 100644 --- a/composer.json +++ b/composer.json @@ -13,7 +13,8 @@ "require": { "php": ">=5.4", "php-http/httplug": "1.0.0-beta", - "php-http/client-tools": "^0.1@dev" + "php-http/client-tools": "^0.1@dev", + "php-http/message-factory": "^1.0" }, "require-dev": { "phpspec/phpspec": "^2.4-alpha", diff --git a/spec/CachePluginSpec.php b/spec/CachePluginSpec.php index 2b84c33..6d1f104 100644 --- a/spec/CachePluginSpec.php +++ b/spec/CachePluginSpec.php @@ -3,17 +3,19 @@ namespace spec\Http\Client\Plugin; use Http\Client\Tools\Promise\FulfilledPromise; +use Http\Message\StreamFactory; use PhpSpec\ObjectBehavior; use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\StreamInterface; class CachePluginSpec extends ObjectBehavior { - function let(CacheItemPoolInterface $pool) + function let(CacheItemPoolInterface $pool, StreamFactory $streamFactory) { - $this->beConstructedWith($pool, ['default_ttl'=>60]); + $this->beConstructedWith($pool, $streamFactory, ['default_ttl'=>60]); } function it_is_initializable(CacheItemPoolInterface $pool) @@ -26,17 +28,21 @@ function it_is_a_plugin() $this->shouldImplement('Http\Client\Plugin\Plugin'); } - function it_caches_responses(CacheItemPoolInterface $pool, CacheItemInterface $item, RequestInterface $request, ResponseInterface $response) + function it_caches_responses(CacheItemPoolInterface $pool, CacheItemInterface $item, RequestInterface $request, ResponseInterface $response, StreamInterface $stream) { + $httpBody = 'body'; + $stream->__toString()->willReturn($httpBody); + $request->getMethod()->willReturn('GET'); $request->getUri()->willReturn('/'); $response->getStatusCode()->willReturn(200); + $response->getBody()->willReturn($stream); $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->set(['response' => $response, 'body' => $httpBody])->willReturn($item)->shouldBeCalled(); $item->expiresAfter(60)->willReturn($item)->shouldBeCalled(); $pool->save($item)->shouldBeCalled(); @@ -78,11 +84,15 @@ function it_doesnt_store_post_requests(CacheItemPoolInterface $pool, CacheItemIn } - function it_calculate_age_from_response(CacheItemPoolInterface $pool, CacheItemInterface $item, RequestInterface $request, ResponseInterface $response) + function it_calculate_age_from_response(CacheItemPoolInterface $pool, CacheItemInterface $item, RequestInterface $request, ResponseInterface $response, StreamInterface $stream) { + $httpBody = 'body'; + $stream->__toString()->willReturn($httpBody); + $request->getMethod()->willReturn('GET'); $request->getUri()->willReturn('/'); $response->getStatusCode()->willReturn(200); + $response->getBody()->willReturn($stream); $response->getHeader('Cache-Control')->willReturn(array('max-age=40')); $response->getHeader('Age')->willReturn(array('15')); $response->getHeader('Expires')->willReturn(array()); @@ -91,7 +101,7 @@ function it_calculate_age_from_response(CacheItemPoolInterface $pool, CacheItemI $item->isHit()->willReturn(false); // 40-15 should be 25 - $item->set($response)->willReturn($item)->shouldBeCalled(); + $item->set(['response' => $response, 'body' => $httpBody])->willReturn($item)->shouldBeCalled(); $item->expiresAfter(25)->willReturn($item)->shouldBeCalled(); $pool->save($item)->shouldBeCalled(); diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 4e07342..12d791c 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -3,6 +3,7 @@ namespace Http\Client\Plugin; use Http\Client\Tools\Promise\FulfilledPromise; +use Http\Message\StreamFactory; use Psr\Cache\CacheItemPoolInterface; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; @@ -19,6 +20,11 @@ class CachePlugin implements Plugin */ private $pool; + /** + * @var StreamFactory + */ + private $streamFactory; + /** * Default time to store object in cache. This value is used if CachePlugin::respectCacheHeaders is false or * if cache headers are missing. @@ -40,11 +46,13 @@ class CachePlugin implements Plugin * - respect_cache_headers: Whether to look at the cache directives or ignore them. * * @param CacheItemPoolInterface $pool + * @param StreamFactory $streamFactory * @param array $options */ - public function __construct(CacheItemPoolInterface $pool, array $options = []) + public function __construct(CacheItemPoolInterface $pool, StreamFactory $streamFactory, array $options = []) { $this->pool = $pool; + $this->streamFactory = $streamFactory; $this->defaultTtl = isset($options['default_ttl']) ? $options['default_ttl'] : null; $this->respectCacheHeaders = isset($options['respect_cache_headers']) ? $options['respect_cache_headers'] : true; } @@ -67,12 +75,16 @@ public function handleRequest(RequestInterface $request, callable $next, callabl if ($cacheItem->isHit()) { // return cached response - return new FulfilledPromise($cacheItem->get()); + $data = $cacheItem->get(); + $response = $data['response']; + $response = $response->withBody($this->streamFactory->createStream($data['body'])); + + return new FulfilledPromise($response); } return $next($request)->then(function (ResponseInterface $response) use ($cacheItem) { if ($this->isCacheable($response)) { - $cacheItem->set($response) + $cacheItem->set(['response' => $response, 'body' => $response->getBody()->__toString()]) ->expiresAfter($this->getMaxAge($response)); $this->pool->save($cacheItem); } From 7b9ae72e1378ee7e84a762248e65930ea4d42448 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Sat, 19 Dec 2015 01:49:27 +0100 Subject: [PATCH 2/4] Added options resolver --- composer.json | 3 ++- src/CachePlugin.php | 55 +++++++++++++++++++++++++++------------------ 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/composer.json b/composer.json index 817417e..012381d 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,8 @@ "php": ">=5.4", "php-http/httplug": "1.0.0-beta", "php-http/client-tools": "^0.1@dev", - "php-http/message-factory": "^1.0" + "php-http/message-factory": "^1.0", + "symfony/options-resolver": "^2.6|^3.0" }, "require-dev": { "phpspec/phpspec": "^2.4-alpha", diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 12d791c..4b2d8b8 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -7,6 +7,7 @@ use Psr\Cache\CacheItemPoolInterface; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; +use Symfony\Component\OptionsResolver\OptionsResolver; /** * Allow for caching a response. @@ -26,35 +27,27 @@ class CachePlugin implements Plugin private $streamFactory; /** - * 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 whether this response may be cached and to - * decide how it can be cached. - * - * @var bool Defaults to true + * @var array */ - private $respectCacheHeaders; + private $config; /** * Available options are * - respect_cache_headers: Whether to look at the cache directives or ignore them. - * + * - default_ttl: If we do not respect cache headers or can't calculate a good ttl, use this value. + * * @param CacheItemPoolInterface $pool * @param StreamFactory $streamFactory - * @param array $options + * @param array $config */ - public function __construct(CacheItemPoolInterface $pool, StreamFactory $streamFactory, array $options = []) + public function __construct(CacheItemPoolInterface $pool, StreamFactory $streamFactory, array $config = []) { $this->pool = $pool; $this->streamFactory = $streamFactory; - $this->defaultTtl = isset($options['default_ttl']) ? $options['default_ttl'] : null; - $this->respectCacheHeaders = isset($options['respect_cache_headers']) ? $options['respect_cache_headers'] : true; + + $optionsResolver = new OptionsResolver(); + $this->configureOptions($optionsResolver); + $this->config = $optionsResolver->resolve($config); } /** @@ -105,7 +98,7 @@ protected function isCacheable(ResponseInterface $response) if (!in_array($response->getStatusCode(), [200, 203, 300, 301, 302, 404, 410])) { return false; } - if (!$this->respectCacheHeaders) { + if (!$this->config['respect_cache_headers']) { return true; } if ($this->getCacheControlDirective($response, 'no-store') || $this->getCacheControlDirective($response, 'private')) { @@ -160,8 +153,8 @@ private function createCacheKey(RequestInterface $request) */ private function getMaxAge(ResponseInterface $response) { - if (!$this->respectCacheHeaders) { - return $this->defaultTtl; + if (!$this->config['respect_cache_headers']) { + return $this->config['default_ttl']; } // check for max age in the Cache-Control header @@ -181,6 +174,24 @@ private function getMaxAge(ResponseInterface $response) return (new \DateTime($header))->getTimestamp() - (new \DateTime())->getTimestamp(); } - return $this->defaultTtl; + return $this->config['default_ttl']; + } + + /** + * Configure an options resolver. + * + * @param OptionsResolver $resolver + * + * @return array + */ + private function configureOptions(OptionsResolver $resolver) + { + $resolver->setDefaults([ + 'default_ttl' => null, + 'respect_cache_headers' => true, + ]); + + $resolver->setAllowedTypes('default_ttl', ['int', 'null']); + $resolver->setAllowedTypes('respect_cache_headers', 'bool'); } } From ddd69ba0aa4c186a364cb0dcde36b70805cb5cb2 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 21 Dec 2015 22:13:59 +0100 Subject: [PATCH 3/4] Make sure to rewind the stream or create a new one if needed --- spec/CachePluginSpec.php | 4 ++++ src/CachePlugin.php | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/spec/CachePluginSpec.php b/spec/CachePluginSpec.php index 6d1f104..36f19e7 100644 --- a/spec/CachePluginSpec.php +++ b/spec/CachePluginSpec.php @@ -32,6 +32,8 @@ function it_caches_responses(CacheItemPoolInterface $pool, CacheItemInterface $i { $httpBody = 'body'; $stream->__toString()->willReturn($httpBody); + $stream->isSeekable()->willReturn(true); + $stream->rewind()->shouldBeCalled(); $request->getMethod()->willReturn('GET'); $request->getUri()->willReturn('/'); @@ -88,6 +90,8 @@ function it_calculate_age_from_response(CacheItemPoolInterface $pool, CacheItemI { $httpBody = 'body'; $stream->__toString()->willReturn($httpBody); + $stream->isSeekable()->willReturn(true); + $stream->rewind()->shouldBeCalled(); $request->getMethod()->willReturn('GET'); $request->getUri()->willReturn('/'); diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 4b2d8b8..50ea6fb 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -77,7 +77,15 @@ public function handleRequest(RequestInterface $request, callable $next, callabl return $next($request)->then(function (ResponseInterface $response) use ($cacheItem) { if ($this->isCacheable($response)) { - $cacheItem->set(['response' => $response, 'body' => $response->getBody()->__toString()]) + $bodyStream = $response->getBody(); + $body = $bodyStream->__toString(); + if ($bodyStream->isSeekable()) { + $bodyStream->rewind(); + } else { + $response = $response->withBody($this->streamFactory->createStream($body)); + } + + $cacheItem->set(['response' => $response, 'body' => $body]) ->expiresAfter($this->getMaxAge($response)); $this->pool->save($cacheItem); } From 5a6cbb14e89bf717f98135d30c9ea37834a1b303 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Tue, 22 Dec 2015 17:41:24 +0100 Subject: [PATCH 4/4] minor --- src/CachePlugin.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 50ea6fb..6d1bb9c 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -189,8 +189,6 @@ private function getMaxAge(ResponseInterface $response) * Configure an options resolver. * * @param OptionsResolver $resolver - * - * @return array */ private function configureOptions(OptionsResolver $resolver) {