From 5d50b37bda2c8bcac424fe12469eb34ba9f78ed3 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 31 Mar 2017 17:12:59 +0200 Subject: [PATCH 01/10] Added CacheKeyGenerator Also added a default generator --- .../RequestLineAndBodyGeneratorSpec.php | 40 +++++++++++++++++++ src/CachePlugin.php | 14 +++++-- src/Generator/CacheKeyGenerator.php | 22 ++++++++++ src/Generator/RequestLineAndBodyGenerator.php | 23 +++++++++++ 4 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 spec/Generator/RequestLineAndBodyGeneratorSpec.php create mode 100644 src/Generator/CacheKeyGenerator.php create mode 100644 src/Generator/RequestLineAndBodyGenerator.php diff --git a/spec/Generator/RequestLineAndBodyGeneratorSpec.php b/spec/Generator/RequestLineAndBodyGeneratorSpec.php new file mode 100644 index 0000000..8f33524 --- /dev/null +++ b/spec/Generator/RequestLineAndBodyGeneratorSpec.php @@ -0,0 +1,40 @@ +shouldHaveType('Http\Client\Common\Plugin\Generator\RequestLineAndBodyGenerator'); + } + + function it_is_a_key_generator() + { + $this->shouldImplement('Http\Client\Common\Plugin\Generator\CacheKeyGenerator'); + } + + + function it_generates_cache_from_request(RequestInterface $request) + { + $request->getMethod()->shouldBeCalled()->willReturn('GET'); + $request->getUri()->shouldBeCalled()->willReturn('http://example.com/foo'); + $request->getBody()->shouldBeCalled()->willReturn('bar'); + + $this->generate($request)->shouldReturn('GET http://example.com/foo bar'); + } + + function it_generates_cache_from_request_with_no_body(RequestInterface $request) + { + $request->getMethod()->shouldBeCalled()->willReturn('GET'); + $request->getUri()->shouldBeCalled()->willReturn('http://example.com/foo'); + $request->getBody()->shouldBeCalled()->willReturn(''); + + // No extra space after uri + $this->generate($request)->shouldReturn('GET http://example.com/foo'); + } +} diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 10b1c25..0781066 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -4,6 +4,8 @@ use Http\Client\Common\Plugin; use Http\Client\Common\Plugin\Exception\RewindStreamException; +use Http\Client\Common\Plugin\Generator\CacheKeyGenerator; +use Http\Client\Common\Plugin\Generator\RequestLineAndBodyGenerator; use Http\Message\StreamFactory; use Http\Promise\FulfilledPromise; use Psr\Cache\CacheItemInterface; @@ -56,6 +58,7 @@ final class CachePlugin implements Plugin * 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 $respect_response_cache_directives list of cache directives this plugin will respect while caching responses. + * @var CacheKeyGenerator $cache_key_generator a class to generate the cache key. Defaults to RequestLineAndBodyGenerator * } */ public function __construct(CacheItemPoolInterface $pool, StreamFactory $streamFactory, array $config = []) @@ -282,12 +285,13 @@ private function getCacheControlDirective(ResponseInterface $response, $name) */ private function createCacheKey(RequestInterface $request) { - $body = (string) $request->getBody(); - if (!empty($body)) { - $body = ' '.$body; + if (null === $this->config['cache_key_generator']) { + $this->config['cache_key_generator'] = new RequestLineAndBodyGenerator(); } - return hash($this->config['hash_algo'], $request->getMethod().' '.$request->getUri().$body); + $key = $this->config['cache_key_generator']->generate($request); + + return hash($this->config['hash_algo'], $key); } /** @@ -338,12 +342,14 @@ private function configureOptions(OptionsResolver $resolver) 'hash_algo' => 'sha1', 'methods' => ['GET', 'HEAD'], 'respect_response_cache_directives' => ['no-cache', 'private', 'max-age', 'no-store'], + 'cache_key_generator' => null, ]); $resolver->setAllowedTypes('cache_lifetime', ['int', 'null']); $resolver->setAllowedTypes('default_ttl', ['int', 'null']); $resolver->setAllowedTypes('respect_cache_headers', 'bool'); $resolver->setAllowedTypes('methods', 'array'); + $resolver->setAllowedTypes('cache_key_generator', ['null', CacheKeyGenerator::class]); $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. */ diff --git a/src/Generator/CacheKeyGenerator.php b/src/Generator/CacheKeyGenerator.php new file mode 100644 index 0000000..c2626af --- /dev/null +++ b/src/Generator/CacheKeyGenerator.php @@ -0,0 +1,22 @@ + + */ +interface CacheKeyGenerator +{ + /** + * Generate a cache key from a Request. + * + * @param RequestInterface $request + * + * @return string + */ + public function generate(RequestInterface $request); +} diff --git a/src/Generator/RequestLineAndBodyGenerator.php b/src/Generator/RequestLineAndBodyGenerator.php new file mode 100644 index 0000000..7239192 --- /dev/null +++ b/src/Generator/RequestLineAndBodyGenerator.php @@ -0,0 +1,23 @@ + + */ +class RequestLineAndBodyGenerator implements CacheKeyGenerator +{ + public function generate(RequestInterface $request) + { + $body = (string) $request->getBody(); + if (!empty($body)) { + $body = ' '.$body; + } + + return $request->getMethod().' '.$request->getUri().$body; + } +} From b42d061a58819e33e98b7e7ee7820762f245d226 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 31 Mar 2017 17:15:25 +0200 Subject: [PATCH 02/10] cs fix --- src/CachePlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 0781066..e559634 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -57,7 +57,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 $respect_response_cache_directives list of cache directives this plugin will respect while caching responses. + * @var array $respect_response_cache_directives list of cache directives this plugin will respect while caching responses * @var CacheKeyGenerator $cache_key_generator a class to generate the cache key. Defaults to RequestLineAndBodyGenerator * } */ From b3a09d873c64c6920c364c4ae2fb1a96ba0beb49 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 31 Mar 2017 17:22:10 +0200 Subject: [PATCH 03/10] Support PSR-4 --- src/CachePlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index e559634..eb44657 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -349,7 +349,7 @@ private function configureOptions(OptionsResolver $resolver) $resolver->setAllowedTypes('default_ttl', ['int', 'null']); $resolver->setAllowedTypes('respect_cache_headers', 'bool'); $resolver->setAllowedTypes('methods', 'array'); - $resolver->setAllowedTypes('cache_key_generator', ['null', CacheKeyGenerator::class]); + $resolver->setAllowedTypes('cache_key_generator', ['null', 'Http\Client\Common\Plugin\Generator\CacheKeyGenerator']); $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 c116b88ec5178427dde5a948900a6712c0cc5743 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 31 Mar 2017 17:58:35 +0200 Subject: [PATCH 04/10] Added test to make sure cache_key_generator works --- spec/CachePluginSpec.php | 42 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/spec/CachePluginSpec.php b/spec/CachePluginSpec.php index b8f6560..b4bfba6 100644 --- a/spec/CachePluginSpec.php +++ b/spec/CachePluginSpec.php @@ -2,6 +2,7 @@ namespace spec\Http\Client\Common\Plugin; +use Http\Client\Common\Plugin\Generator\RequestLineAndBodyGenerator; use Prophecy\Argument; use Http\Message\StreamFactory; use Http\Promise\FulfilledPromise; @@ -400,6 +401,47 @@ function it_caches_private_responses_when_allowed( } + function it_can_be_initialized_with_custom_cache_key_generator( + CacheItemPoolInterface $pool, + CacheItemInterface $item, + StreamFactory $streamFactory, + RequestInterface $request, + ResponseInterface $response, + StreamInterface $stream, + RequestLineAndBodyGenerator $generator + ) { + $this->beConstructedThrough('clientCache', [$pool, $streamFactory, [ + 'cache_key_generator' => $generator, + ]]); + + $generator->generate($request)->shouldBeCalled()->willReturn('foo'); + + $stream->isSeekable()->willReturn(true); + $stream->rewind()->shouldBeCalled(); + $streamFactory->createStream(Argument::any())->willReturn($stream); + + $request->getMethod()->willReturn('GET'); + $request->getUri()->willReturn('/'); + $response->withBody(Argument::any())->willReturn($response); + + $pool->getItem(Argument::any())->shouldBeCalled()->willReturn($item); + $item->isHit()->willReturn(true); + $item->get()->willReturn([ + 'response' => $response->getWrappedObject(), + 'body' => 'body', + 'expiresAt' => null, + 'createdAt' => 0, + 'etag' => [] + ]); + + $next = function (RequestInterface $request) use ($response) { + return new FulfilledPromise($response->getWrappedObject()); + }; + + $this->handleRequest($request, $next, function () {}); + } + + /** * Private function to match cache item data. * From 2690ae100610aca58654dd9dcd4f376646889233 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 31 Mar 2017 18:33:52 +0200 Subject: [PATCH 05/10] cs --- spec/Generator/RequestLineAndBodyGeneratorSpec.php | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/spec/Generator/RequestLineAndBodyGeneratorSpec.php b/spec/Generator/RequestLineAndBodyGeneratorSpec.php index 8f33524..f9d1127 100644 --- a/spec/Generator/RequestLineAndBodyGeneratorSpec.php +++ b/spec/Generator/RequestLineAndBodyGeneratorSpec.php @@ -3,23 +3,21 @@ namespace spec\Http\Client\Common\Plugin\Generator; use PhpSpec\ObjectBehavior; -use Prophecy\Argument; use Psr\Http\Message\RequestInterface; class RequestLineAndBodyGeneratorSpec extends ObjectBehavior { - function it_is_initializable() + public function it_is_initializable() { $this->shouldHaveType('Http\Client\Common\Plugin\Generator\RequestLineAndBodyGenerator'); } - function it_is_a_key_generator() + public function it_is_a_key_generator() { $this->shouldImplement('Http\Client\Common\Plugin\Generator\CacheKeyGenerator'); } - - function it_generates_cache_from_request(RequestInterface $request) + public function it_generates_cache_from_request(RequestInterface $request) { $request->getMethod()->shouldBeCalled()->willReturn('GET'); $request->getUri()->shouldBeCalled()->willReturn('http://example.com/foo'); @@ -28,7 +26,7 @@ function it_generates_cache_from_request(RequestInterface $request) $this->generate($request)->shouldReturn('GET http://example.com/foo bar'); } - function it_generates_cache_from_request_with_no_body(RequestInterface $request) + public function it_generates_cache_from_request_with_no_body(RequestInterface $request) { $request->getMethod()->shouldBeCalled()->willReturn('GET'); $request->getUri()->shouldBeCalled()->willReturn('http://example.com/foo'); From 92e003969f67c8d25b8c96fca5c2e05cccebf92d Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 31 Mar 2017 23:41:42 +0200 Subject: [PATCH 06/10] Changes according to feedback. --- .../Generator/SimpleGeneratorSpec.php} | 8 ++++---- src/{ => Cache}/Generator/CacheKeyGenerator.php | 2 +- .../Generator/SimpleGenerator.php} | 4 ++-- src/CachePlugin.php | 12 ++++++------ 4 files changed, 13 insertions(+), 13 deletions(-) rename spec/{Generator/RequestLineAndBodyGeneratorSpec.php => Cache/Generator/SimpleGeneratorSpec.php} (77%) rename src/{ => Cache}/Generator/CacheKeyGenerator.php (87%) rename src/{Generator/RequestLineAndBodyGenerator.php => Cache/Generator/SimpleGenerator.php} (79%) diff --git a/spec/Generator/RequestLineAndBodyGeneratorSpec.php b/spec/Cache/Generator/SimpleGeneratorSpec.php similarity index 77% rename from spec/Generator/RequestLineAndBodyGeneratorSpec.php rename to spec/Cache/Generator/SimpleGeneratorSpec.php index f9d1127..9ee102c 100644 --- a/spec/Generator/RequestLineAndBodyGeneratorSpec.php +++ b/spec/Cache/Generator/SimpleGeneratorSpec.php @@ -1,20 +1,20 @@ shouldHaveType('Http\Client\Common\Plugin\Generator\RequestLineAndBodyGenerator'); + $this->shouldHaveType('Http\Client\Common\Plugin\Cache\Generator\SimpleGenerator'); } public function it_is_a_key_generator() { - $this->shouldImplement('Http\Client\Common\Plugin\Generator\CacheKeyGenerator'); + $this->shouldImplement('Http\Client\Common\Plugin\Cache\Generator\CacheKeyGenerator'); } public function it_generates_cache_from_request(RequestInterface $request) diff --git a/src/Generator/CacheKeyGenerator.php b/src/Cache/Generator/CacheKeyGenerator.php similarity index 87% rename from src/Generator/CacheKeyGenerator.php rename to src/Cache/Generator/CacheKeyGenerator.php index c2626af..d351e57 100644 --- a/src/Generator/CacheKeyGenerator.php +++ b/src/Cache/Generator/CacheKeyGenerator.php @@ -1,6 +1,6 @@ */ -class RequestLineAndBodyGenerator implements CacheKeyGenerator +class SimpleGenerator implements CacheKeyGenerator { public function generate(RequestInterface $request) { diff --git a/src/CachePlugin.php b/src/CachePlugin.php index eb44657..299e201 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -4,8 +4,8 @@ use Http\Client\Common\Plugin; use Http\Client\Common\Plugin\Exception\RewindStreamException; -use Http\Client\Common\Plugin\Generator\CacheKeyGenerator; -use Http\Client\Common\Plugin\Generator\RequestLineAndBodyGenerator; +use Http\Client\Common\Plugin\Cache\Generator\CacheKeyGenerator; +use Http\Client\Common\Plugin\Cache\Generator\SimpleGenerator; use Http\Message\StreamFactory; use Http\Promise\FulfilledPromise; use Psr\Cache\CacheItemInterface; @@ -76,6 +76,10 @@ public function __construct(CacheItemPoolInterface $pool, StreamFactory $streamF $optionsResolver = new OptionsResolver(); $this->configureOptions($optionsResolver); $this->config = $optionsResolver->resolve($config); + + if (null === $this->config['cache_key_generator']) { + $this->config['cache_key_generator'] = new SimpleGenerator(); + } } /** @@ -285,10 +289,6 @@ private function getCacheControlDirective(ResponseInterface $response, $name) */ private function createCacheKey(RequestInterface $request) { - if (null === $this->config['cache_key_generator']) { - $this->config['cache_key_generator'] = new RequestLineAndBodyGenerator(); - } - $key = $this->config['cache_key_generator']->generate($request); return hash($this->config['hash_algo'], $key); From a97aa41f562af0d8bdfe4fb7e8dc35284951634b Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 31 Mar 2017 23:41:59 +0200 Subject: [PATCH 07/10] Make plugin final --- src/Cache/Generator/SimpleGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Cache/Generator/SimpleGenerator.php b/src/Cache/Generator/SimpleGenerator.php index 4f0ee90..8e22777 100644 --- a/src/Cache/Generator/SimpleGenerator.php +++ b/src/Cache/Generator/SimpleGenerator.php @@ -9,7 +9,7 @@ * * @author Tobias Nyholm */ -class SimpleGenerator implements CacheKeyGenerator +final class SimpleGenerator implements CacheKeyGenerator { public function generate(RequestInterface $request) { From ae018f38ec98862bbb1357c9d8ccb6c4fd31b674 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 31 Mar 2017 23:43:32 +0200 Subject: [PATCH 08/10] Bugfixes --- spec/CachePluginSpec.php | 4 ++-- src/CachePlugin.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/CachePluginSpec.php b/spec/CachePluginSpec.php index b4bfba6..af8a483 100644 --- a/spec/CachePluginSpec.php +++ b/spec/CachePluginSpec.php @@ -2,7 +2,7 @@ namespace spec\Http\Client\Common\Plugin; -use Http\Client\Common\Plugin\Generator\RequestLineAndBodyGenerator; +use Http\Client\Common\Plugin\Cache\Generator\SimpleGenerator; use Prophecy\Argument; use Http\Message\StreamFactory; use Http\Promise\FulfilledPromise; @@ -408,7 +408,7 @@ function it_can_be_initialized_with_custom_cache_key_generator( RequestInterface $request, ResponseInterface $response, StreamInterface $stream, - RequestLineAndBodyGenerator $generator + SimpleGenerator $generator ) { $this->beConstructedThrough('clientCache', [$pool, $streamFactory, [ 'cache_key_generator' => $generator, diff --git a/src/CachePlugin.php b/src/CachePlugin.php index 299e201..e19b950 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -58,7 +58,7 @@ final class CachePlugin implements Plugin * 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 $respect_response_cache_directives list of cache directives this plugin will respect while caching responses - * @var CacheKeyGenerator $cache_key_generator a class to generate the cache key. Defaults to RequestLineAndBodyGenerator + * @var CacheKeyGenerator $cache_key_generator a class to generate the cache key. Defaults to SimpleGenerator * } */ public function __construct(CacheItemPoolInterface $pool, StreamFactory $streamFactory, array $config = []) From c6dd7d6680e6516bb2c2c0aa4b433877b9406b75 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 31 Mar 2017 23:43:45 +0200 Subject: [PATCH 09/10] Remove final --- src/Cache/Generator/SimpleGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Cache/Generator/SimpleGenerator.php b/src/Cache/Generator/SimpleGenerator.php index 8e22777..4f0ee90 100644 --- a/src/Cache/Generator/SimpleGenerator.php +++ b/src/Cache/Generator/SimpleGenerator.php @@ -9,7 +9,7 @@ * * @author Tobias Nyholm */ -final class SimpleGenerator implements CacheKeyGenerator +class SimpleGenerator implements CacheKeyGenerator { public function generate(RequestInterface $request) { From fa46a79c3c93c6ea421aba050e7f4d220475ac67 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Fri, 31 Mar 2017 23:49:24 +0200 Subject: [PATCH 10/10] typo --- src/CachePlugin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CachePlugin.php b/src/CachePlugin.php index e19b950..fd7d87f 100644 --- a/src/CachePlugin.php +++ b/src/CachePlugin.php @@ -349,7 +349,7 @@ private function configureOptions(OptionsResolver $resolver) $resolver->setAllowedTypes('default_ttl', ['int', 'null']); $resolver->setAllowedTypes('respect_cache_headers', 'bool'); $resolver->setAllowedTypes('methods', 'array'); - $resolver->setAllowedTypes('cache_key_generator', ['null', 'Http\Client\Common\Plugin\Generator\CacheKeyGenerator']); + $resolver->setAllowedTypes('cache_key_generator', ['null', 'Http\Client\Common\Plugin\Cache\Generator\CacheKeyGenerator']); $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. */