From 47ac710a987ed49bb15968eca037fcb9b2366d94 Mon Sep 17 00:00:00 2001 From: Till Hildebrandt Date: Mon, 25 Nov 2019 14:09:02 +0100 Subject: [PATCH 1/8] Added blacklisted_paths option - adds a list of strings (regular expressions) to describe patterns of paths not to be cached. --- CHANGELOG.md | 6 ++++++ src/CachePlugin.php | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4491df1..8ec1737 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Change Log +## 1.6.1 - 2019-11-25 + +### Added + +* Added `blacklisted_paths` option, which takes an array of `strings` (regular expressions) and allows to define paths, that shall not be cached in any case. + ## 1.6.0 - 2019-01-23 ### Added diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 5ca9b6d..0f0cafb 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -60,6 +60,7 @@ final class CachePlugin implements Plugin * we have to store the cache for a longer time than the server originally says it is valid for. * We store a cache item for $cache_lifetime + max age of the response. * @var array $methods list of request methods which can be cached + * @var array $blacklisted_paths list of regex patterns of paths explicitly not to be cached. * @var array $respect_response_cache_directives list of cache directives this plugin will respect while caching responses * @var CacheKeyGenerator $cache_key_generator an object to generate the cache key. Defaults to a new instance of SimpleGenerator * @var CacheListener[] $cache_listeners an array of objects to act on the response based on the results of the cache check. @@ -183,7 +184,7 @@ protected function doHandleRequest(RequestInterface $request, callable $next, ca return $this->handleCacheListeners($request, $this->createResponseFromCacheItem($cacheItem), true, $cacheItem); } - if ($this->isCacheable($response)) { + if ($this->isCacheable($request, $response)) { $bodyStream = $response->getBody(); $body = $bodyStream->__toString(); if ($bodyStream->isSeekable()) { @@ -246,16 +247,23 @@ private function calculateResponseExpiresAt($maxAge) /** * Verify that we can cache this response. * + * @param RequestInterface $request * @param ResponseInterface $response * * @return bool */ - protected function isCacheable(ResponseInterface $response) + protected function isCacheable(RequestInterface $request, ResponseInterface $response) { if (!in_array($response->getStatusCode(), [200, 203, 300, 301, 302, 404, 410])) { return false; } + foreach ($this->config['blacklisted_paths'] as $not_to_cache_path) { + if (1 === preg_match('/'.$not_to_cache_path.'/', $request->getRequestTarget())) { + return false; + } + } + $nocacheDirectives = array_intersect($this->config['respect_response_cache_directives'], $this->noCacheFlags); foreach ($nocacheDirectives as $nocacheDirective) { if ($this->getCacheControlDirective($response, $nocacheDirective)) { @@ -353,6 +361,7 @@ private function configureOptions(OptionsResolver $resolver) 'respect_response_cache_directives' => ['no-cache', 'private', 'max-age', 'no-store'], 'cache_key_generator' => null, 'cache_listeners' => [], + 'blacklisted_paths' => [], // restricted for ]); $resolver->setAllowedTypes('cache_lifetime', ['int', 'null']); @@ -360,6 +369,7 @@ private function configureOptions(OptionsResolver $resolver) $resolver->setAllowedTypes('respect_cache_headers', ['bool', 'null']); $resolver->setAllowedTypes('methods', 'array'); $resolver->setAllowedTypes('cache_key_generator', ['null', 'Http\Client\Common\Plugin\Cache\Generator\CacheKeyGenerator']); + $resolver->setAllowedTypes('blacklisted_paths', 'array'); $resolver->setAllowedValues('hash_algo', hash_algos()); $resolver->setAllowedValues('methods', function ($value) { /* RFC7230 sections 3.1.1 and 3.2.6 except limited to uppercase characters. */ From 5fcfc394f149a83e257733b754e8fadc791e6306 Mon Sep 17 00:00:00 2001 From: Till Hildebrandt Date: Mon, 25 Nov 2019 14:40:56 +0100 Subject: [PATCH 2/8] Fix CI --- src/CachePlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 0f0cafb..06b2ea3 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -247,7 +247,7 @@ private function calculateResponseExpiresAt($maxAge) /** * Verify that we can cache this response. * - * @param RequestInterface $request + * @param RequestInterface $request * @param ResponseInterface $response * * @return bool From 0ed5c85b6ac8dd86efd3f79b60252f6c9f58c03f Mon Sep 17 00:00:00 2001 From: Till Hildebrandt Date: Mon, 25 Nov 2019 14:43:26 +0100 Subject: [PATCH 3/8] Fix CI # 2 - honestly, the lengths of a line should be fixed. --- src/CachePlugin.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 06b2ea3..6848e05 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -60,7 +60,7 @@ final class CachePlugin implements Plugin * we have to store the cache for a longer time than the server originally says it is valid for. * We store a cache item for $cache_lifetime + max age of the response. * @var array $methods list of request methods which can be cached - * @var array $blacklisted_paths list of regex patterns of paths explicitly not to be cached. + * @var array $blacklisted_paths list of regex patterns of paths explicitly not to be cached * @var array $respect_response_cache_directives list of cache directives this plugin will respect while caching responses * @var CacheKeyGenerator $cache_key_generator an object to generate the cache key. Defaults to a new instance of SimpleGenerator * @var CacheListener[] $cache_listeners an array of objects to act on the response based on the results of the cache check. @@ -73,12 +73,10 @@ public function __construct(CacheItemPoolInterface $pool, StreamFactory $streamF $this->streamFactory = $streamFactory; if (isset($config['respect_cache_headers']) && isset($config['respect_response_cache_directives'])) { - throw new \InvalidArgumentException( - 'You can\'t provide config option "respect_cache_headers" and "respect_response_cache_directives". '. - 'Use "respect_response_cache_directives" instead.' - ); + throw new \InvalidArgumentException('You can\'t provide config option "respect_cache_headers" and "respect_response_cache_directives". '.'Use "respect_response_cache_directives" instead.'); } + $optionsResolver = new OptionsResolver(); $this->configureOptions($optionsResolver); $this->config = $optionsResolver->resolve($config); From a9e1d11af4334052365514088385ac700f0048b6 Mon Sep 17 00:00:00 2001 From: Till Hildebrandt Date: Mon, 25 Nov 2019 14:44:42 +0100 Subject: [PATCH 4/8] Fix CI # 3 - that has been a fine restriction --- src/CachePlugin.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 6848e05..d4e5b74 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -76,7 +76,6 @@ public function __construct(CacheItemPoolInterface $pool, StreamFactory $streamF throw new \InvalidArgumentException('You can\'t provide config option "respect_cache_headers" and "respect_response_cache_directives". '.'Use "respect_response_cache_directives" instead.'); } - $optionsResolver = new OptionsResolver(); $this->configureOptions($optionsResolver); $this->config = $optionsResolver->resolve($config); From 005d609442dd14e8e9d11fcd8f7fb5f990957c81 Mon Sep 17 00:00:00 2001 From: Till Hildebrandt Date: Mon, 25 Nov 2019 18:11:37 +0100 Subject: [PATCH 5/8] Fixing Pull Request Requests - outsourced checking of blacklisted_paths to new method "isCacheableRequest" - removed related functionality from "isCacheable" - removed obsolete comment --- CHANGELOG.md | 2 +- src/CachePlugin.php | 29 ++++++++++++++++++++--------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ec1737..ffe9ddd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Change Log -## 1.6.1 - 2019-11-25 +## 1.7.0 - unreleased ### Added diff --git a/src/CachePlugin.php b/src/CachePlugin.php index d4e5b74..f2749da 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -181,7 +181,7 @@ protected function doHandleRequest(RequestInterface $request, callable $next, ca return $this->handleCacheListeners($request, $this->createResponseFromCacheItem($cacheItem), true, $cacheItem); } - if ($this->isCacheable($request, $response)) { + if ($this->isCacheable($response) && $this->isCacheableRequest($request)) { $bodyStream = $response->getBody(); $body = $bodyStream->__toString(); if ($bodyStream->isSeekable()) { @@ -244,26 +244,37 @@ private function calculateResponseExpiresAt($maxAge) /** * Verify that we can cache this response. * - * @param RequestInterface $request * @param ResponseInterface $response * * @return bool */ - protected function isCacheable(RequestInterface $request, ResponseInterface $response) + protected function isCacheable(ResponseInterface $response) { if (!in_array($response->getStatusCode(), [200, 203, 300, 301, 302, 404, 410])) { return false; } - foreach ($this->config['blacklisted_paths'] as $not_to_cache_path) { - if (1 === preg_match('/'.$not_to_cache_path.'/', $request->getRequestTarget())) { + $nocacheDirectives = array_intersect($this->config['respect_response_cache_directives'], $this->noCacheFlags); + foreach ($nocacheDirectives as $nocacheDirective) { + if ($this->getCacheControlDirective($response, $nocacheDirective)) { return false; } } - $nocacheDirectives = array_intersect($this->config['respect_response_cache_directives'], $this->noCacheFlags); - foreach ($nocacheDirectives as $nocacheDirective) { - if ($this->getCacheControlDirective($response, $nocacheDirective)) { + return true; + } + + /** + * Verify that we can cache this request. + * + * @param RequestInterface $request + * + * @return bool + */ + protected function isCacheableRequest(RequestInterface $request) + { + foreach ($this->config['blacklisted_paths'] as $not_to_cache_path) { + if (1 === preg_match('/'.$not_to_cache_path.'/', $request->getRequestTarget())) { return false; } } @@ -358,7 +369,7 @@ private function configureOptions(OptionsResolver $resolver) 'respect_response_cache_directives' => ['no-cache', 'private', 'max-age', 'no-store'], 'cache_key_generator' => null, 'cache_listeners' => [], - 'blacklisted_paths' => [], // restricted for + 'blacklisted_paths' => [], ]); $resolver->setAllowedTypes('cache_lifetime', ['int', 'null']); From 817a4d8d48d23f5e873a20992d8f2e5eeefef72c Mon Sep 17 00:00:00 2001 From: Till Hildebrandt Date: Tue, 26 Nov 2019 10:00:50 +0100 Subject: [PATCH 6/8] Fixing Pull Request Requests - added spec tests for blacklisted paths - it_does_not_store_responses_of_requests_to_blacklisted_paths - it_stores_responses_of_requests_not_in_blacklisted_paths --- spec/CachePluginSpec.php | 92 ++++++++++++++++++++++++++++++++++++++++ src/CachePlugin.php | 2 +- 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/spec/CachePluginSpec.php b/spec/CachePluginSpec.php index 7139b97..7e94bc5 100644 --- a/spec/CachePluginSpec.php +++ b/spec/CachePluginSpec.php @@ -400,6 +400,98 @@ function it_caches_private_responses_when_allowed( $this->handleRequest($request, $next, function () {}); } + function it_does_not_store_responses_of_requests_to_blacklisted_paths( + CacheItemPoolInterface $pool, + CacheItemInterface $item, + RequestInterface $request, + ResponseInterface $response, + StreamFactory $streamFactory, + StreamInterface $stream + ) { + $this->beConstructedThrough('clientCache', [$pool, $streamFactory, [ + 'default_ttl' => 60, + 'cache_lifetime' => 1000, + 'blacklisted_paths' => ['\/foo'] + ]]); + + $httpBody = 'body'; + $stream->__toString()->willReturn($httpBody); + $stream->isSeekable()->willReturn(true); + + $request->getMethod()->willReturn('GET'); + $request->getUri()->willReturn('/foo'); + $request->getBody()->shouldBeCalled(); + + $response->getStatusCode()->willReturn(200); + $response->getBody()->willReturn($stream); + $response->getHeader('Cache-Control')->willReturn([])->shouldBeCalled(); + + $pool->getItem('231392a16d98e1cf631845c79b7d45f40bab08f3')->shouldBeCalled()->willReturn($item); + $item->isHit()->willReturn(false); + + $item->set($this->getCacheItemMatcher([ + 'response' => $response->getWrappedObject(), + 'body' => $httpBody, + 'expiresAt' => 0, + 'createdAt' => 0 + ]))->willReturn($item)->shouldNotBeCalled(); + $pool->save(Argument::any())->shouldNotBeCalled(); + + $next = function (RequestInterface $request) use ($response) { + return new FulfilledPromise($response->getWrappedObject()); + }; + + $this->handleRequest($request, $next, function () {}); + } + + function it_stores_responses_of_requests_not_in_blacklisted_paths( + CacheItemPoolInterface $pool, + CacheItemInterface $item, + RequestInterface $request, + ResponseInterface $response, + StreamFactory $streamFactory, + StreamInterface $stream + ) { + $this->beConstructedThrough('clientCache', [$pool, $streamFactory, [ + 'default_ttl' => 60, + 'cache_lifetime' => 1000, + 'blacklisted_paths' => ['\/foo'] + ]]); + + $httpBody = 'body'; + $stream->__toString()->willReturn($httpBody); + $stream->isSeekable()->willReturn(true); + $stream->rewind()->shouldBeCalled(); + + $request->getMethod()->willReturn('GET'); + $request->getUri()->willReturn('/'); + $request->getBody()->shouldBeCalled(); + + $response->getStatusCode()->willReturn(200); + $response->getBody()->willReturn($stream); + $response->getHeader('Cache-Control')->willReturn([])->shouldBeCalled(); + $response->getHeader('Expires')->willReturn([])->shouldBeCalled(); + $response->getHeader('ETag')->willReturn([])->shouldBeCalled(); + + $pool->getItem('d20f64acc6e70b6079845f2fe357732929550ae1')->shouldBeCalled()->willReturn($item); + $item->isHit()->willReturn(false); + $item->expiresAfter(1060)->willReturn($item)->shouldBeCalled(); + + $item->set($this->getCacheItemMatcher([ + 'response' => $response->getWrappedObject(), + 'body' => $httpBody, + 'expiresAt' => 0, + 'createdAt' => 0, + 'etag' => [] + ]))->willReturn($item)->shouldBeCalled(); + $pool->save(Argument::any())->shouldBeCalled(); + + $next = function (RequestInterface $request) use ($response) { + return new FulfilledPromise($response->getWrappedObject()); + }; + + $this->handleRequest($request, $next, function () {}); + } function it_can_be_initialized_with_custom_cache_key_generator( CacheItemPoolInterface $pool, diff --git a/src/CachePlugin.php b/src/CachePlugin.php index f2749da..21f45c5 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -274,7 +274,7 @@ protected function isCacheable(ResponseInterface $response) protected function isCacheableRequest(RequestInterface $request) { foreach ($this->config['blacklisted_paths'] as $not_to_cache_path) { - if (1 === preg_match('/'.$not_to_cache_path.'/', $request->getRequestTarget())) { + if (1 === preg_match('/'.$not_to_cache_path.'/', $request->getUri())) { return false; } } From ba4b1aab7c6c236c052595ad4d47adca5326885f Mon Sep 17 00:00:00 2001 From: Till Hildebrandt Date: Tue, 26 Nov 2019 10:04:13 +0100 Subject: [PATCH 7/8] Fixing CI --- src/CachePlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 21f45c5..e093d7a 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -267,7 +267,7 @@ protected function isCacheable(ResponseInterface $response) /** * Verify that we can cache this request. * - * @param RequestInterface $request + * @param RequestInterface $request * * @return bool */ From 61aab950a803e9a26df4cf46502df04a936c449b Mon Sep 17 00:00:00 2001 From: Till Hildebrandt Date: Tue, 26 Nov 2019 10:24:54 +0100 Subject: [PATCH 8/8] Implemented Feedback - removed unnecessary string concatenation - set isCacheableRequest to private --- src/CachePlugin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index e093d7a..805d361 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -73,7 +73,7 @@ public function __construct(CacheItemPoolInterface $pool, StreamFactory $streamF $this->streamFactory = $streamFactory; if (isset($config['respect_cache_headers']) && isset($config['respect_response_cache_directives'])) { - throw new \InvalidArgumentException('You can\'t provide config option "respect_cache_headers" and "respect_response_cache_directives". '.'Use "respect_response_cache_directives" instead.'); + throw new \InvalidArgumentException('You can\'t provide config option "respect_cache_headers" and "respect_response_cache_directives". Use "respect_response_cache_directives" instead.'); } $optionsResolver = new OptionsResolver(); @@ -271,7 +271,7 @@ protected function isCacheable(ResponseInterface $response) * * @return bool */ - protected function isCacheableRequest(RequestInterface $request) + private function isCacheableRequest(RequestInterface $request) { foreach ($this->config['blacklisted_paths'] as $not_to_cache_path) { if (1 === preg_match('/'.$not_to_cache_path.'/', $request->getUri())) {