From ba70e21ed80ec0938b3c2fd5f2496f4f7ce4edec Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 31 Dec 2016 14:52:05 +0100 Subject: [PATCH 01/16] First stab at reworking to caching strategies --- src/CacheMiddleware.php | 101 ++++++++++++++++++++++++++++++-------- src/Options.php | 8 +++ src/Strategy/Always.php | 25 ++++++++++ src/StrategyInterface.php | 13 +++++ 4 files changed, 126 insertions(+), 21 deletions(-) create mode 100644 src/Options.php create mode 100644 src/Strategy/Always.php create mode 100644 src/StrategyInterface.php diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index df8cd07..d3c1249 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -10,7 +10,11 @@ use Psr\Http\Message\UriInterface; use React\Cache\CacheInterface; use React\Promise\CancellablePromiseInterface; +use React\Promise\PromiseInterface; use function React\Promise\resolve; +use React\Stream\BufferedSink; +use React\Stream\ReadableStreamInterface; +use RingCentral\Psr7\BufferStream; class CacheMiddleware implements MiddlewareInterface { @@ -26,6 +30,21 @@ class CacheMiddleware implements MiddlewareInterface */ private $key; + /** + * @var RequestInterface + */ + private $request; + + /** + * @var bool + */ + private $store = false; + + /** + * @var StrategyInterface + */ + private $strategy; + /** * @param CacheInterface$cache */ @@ -41,16 +60,30 @@ public function __construct(CacheInterface $cache) */ public function pre(RequestInterface $request, array $options = []): CancellablePromiseInterface { + if (!isset($options[self::class][Options::STRATEGY])) { + return resolve($request); + } + $this->strategy = $options[self::class][Options::STRATEGY]; + if (!($this->strategy instanceof StrategyInterface)) { + return resolve($request); + } + if ($request->getMethod() !== 'GET') { return resolve($request); } + $this->request = $request; + $this->key = $this->determineCacheKey($request->getUri()); return $this->cache->get($this->key)->then(function (string $document) { return resolve( $this->buildResponse($document) ); }, function () use ($request) { + if ($this->strategy->preCheck($request)) { + $this->store = true; + } + return resolve($request); }); } @@ -78,31 +111,44 @@ protected function buildResponse(string $document): Response */ public function post(ResponseInterface $response, array $options = []): CancellablePromiseInterface { + if (!($this->strategy instanceof StrategyInterface)) { + return resolve($response); + } + if (!is_string($this->key)) { return resolve($response); } - $contents = (string)$response->getBody(); - - $document = [ - 'body' => $contents, - 'headers' => $response->getHeaders(), - 'protocol_version' => $response->getProtocolVersion(), - 'reason_phrase' => $response->getReasonPhrase(), - 'status_code' => $response->getStatusCode(), - ]; - - $this->cache->set($this->key, json_encode($document)); - - return resolve( - new Response( - $response->getStatusCode(), - $response->getHeaders(), - $contents, - $response->getProtocolVersion(), - $response->getReasonPhrase() - ) - ); + if ($this->strategy->postCheck($response)) { + $this->store = true; + } + + if (!$this->store) { + return resolve($response); + } + + return $this->getBody($response)->then(function (string $contents) use ($response) { + $document = [ + 'expires_at' => time() + $this->strategy->determineTtl($this->request, $response), + 'body' => $contents, + 'headers' => $response->getHeaders(), + 'protocol_version' => $response->getProtocolVersion(), + 'reason_phrase' => $response->getReasonPhrase(), + 'status_code' => $response->getStatusCode(), + ]; + + $this->cache->set($this->key, json_encode($document)); + + return resolve( + new Response( + $response->getStatusCode(), + $response->getHeaders(), + $contents, + $response->getProtocolVersion(), + $response->getReasonPhrase() + ) + ); + }); } @@ -134,4 +180,17 @@ protected function stripExtraSlashes(string $string): string { return preg_replace('#/+#', '/', $string); } + + protected function getBody(ResponseInterface $response): PromiseInterface + { + if ($response->getBody() instanceof BufferStream) { + return resolve($response->getBody()->getContents()); + } + + if ($response->getBody() instanceof ReadableStreamInterface) { + return BufferedSink::createPromise($response->getBody()); + } + + throw new \Exception('Can\'t get body yet'); + } } diff --git a/src/Options.php b/src/Options.php new file mode 100644 index 0000000..0426e63 --- /dev/null +++ b/src/Options.php @@ -0,0 +1,8 @@ + Date: Sat, 31 Dec 2016 20:05:44 +0100 Subject: [PATCH 02/16] Default TTL option --- src/Options.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Options.php b/src/Options.php index 0426e63..aa412d8 100644 --- a/src/Options.php +++ b/src/Options.php @@ -5,4 +5,5 @@ final class Options { const STRATEGY = StrategyInterface::class; + const DEFAULT_TTL = 'default-ttl'; } From bb612f0da69a07b1f7587fbf0ea19af17238c2ef Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 31 Dec 2016 20:07:07 +0100 Subject: [PATCH 03/16] Default TTL of 0 --- src/StrategyInterface.php | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/src/StrategyInterface.php b/src/StrategyInterface.php index 3536b0d..95d7133 100644 --- a/src/StrategyInterface.php +++ b/src/StrategyInterface.php @@ -7,7 +7,35 @@ interface StrategyInterface { + const DEFAULT_TTL = 0; + + /** + * Check whether to store to cache based on the request + * + * @param RequestInterface $request + * @return bool + */ public function preCheck(RequestInterface $request): bool; + + /** + * Check whether to store to cache based on the response + * + * @param ResponseInterface $response + * @return bool + */ public function postCheck(ResponseInterface $response): bool; - public function determineTtl(RequestInterface $request, ResponseInterface $response): int; + + /** + * Determine what the TTL for the cache entry should be based on the request and response + * + * @param RequestInterface $request + * @param ResponseInterface $response + * @param int $default + * @return int + */ + public function determineTtl( + RequestInterface $request, + ResponseInterface $response, + int $default = self::DEFAULT_TTL + ): int; } From cd7ad70ee195a74e4e60cef92d237e56ff04b91b Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 31 Dec 2016 20:07:54 +0100 Subject: [PATCH 04/16] Added tests for Always --- src/Strategy/Always.php | 11 ++++-- tests/Strategy/AlwaysTest.php | 69 +++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 tests/Strategy/AlwaysTest.php diff --git a/src/Strategy/Always.php b/src/Strategy/Always.php index 3fbe43a..cafb31d 100644 --- a/src/Strategy/Always.php +++ b/src/Strategy/Always.php @@ -8,6 +8,8 @@ final class Always implements StrategyInterface { + const ALWAYS_TTL = 300; + public function preCheck(RequestInterface $request): bool { return true; @@ -18,8 +20,11 @@ public function postCheck(ResponseInterface $response): bool return true; } - public function determineTtl(RequestInterface $request, ResponseInterface $response): int - { - return 0; + public function determineTtl( + RequestInterface $request, + ResponseInterface $response, + int $default = self::ALWAYS_TTL + ): int { + return $default; } } diff --git a/tests/Strategy/AlwaysTest.php b/tests/Strategy/AlwaysTest.php new file mode 100644 index 0000000..caccce7 --- /dev/null +++ b/tests/Strategy/AlwaysTest.php @@ -0,0 +1,69 @@ +preCheck($this->prophesize(RequestInterface::class)->reveal()) + ); + } + + public function testPostCheck() + { + self::assertTrue( + (new Always())->postCheck($this->prophesize(ResponseInterface::class)->reveal()) + ); + } + + public function provideTtl() + { + yield [ + Always::ALWAYS_TTL, + Always::ALWAYS_TTL, + ]; + + yield [ + Always::DEFAULT_TTL, + Always::DEFAULT_TTL, + ]; + + yield [ + 123, + 123, + ]; + } + + /** + * @dataProvider provideTtl + */ + public function testDetermineTtl(int $expectedTtl, int $ttl) + { + self::assertSame( + $expectedTtl, + (new Always())->determineTtl( + $this->prophesize(RequestInterface::class)->reveal(), + $this->prophesize(ResponseInterface::class)->reveal(), + $ttl + ) + ); + } + + public function testDetermineTtlDefault() + { + self::assertSame( + Always::ALWAYS_TTL, + (new Always())->determineTtl( + $this->prophesize(RequestInterface::class)->reveal(), + $this->prophesize(ResponseInterface::class)->reveal() + ) + ); + } +} From 288cd945d7d59b7ee8f8d4e025a6a06d5cff4ee2 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 31 Dec 2016 20:10:34 +0100 Subject: [PATCH 05/16] Updated dependencies --- composer.json | 4 +- composer.lock | 120 +++++++++++++++++++++++++------------------------- 2 files changed, 62 insertions(+), 62 deletions(-) diff --git a/composer.json b/composer.json index f14cf11..c6cc7b1 100644 --- a/composer.json +++ b/composer.json @@ -10,8 +10,8 @@ "require": { "php": "^7.0", "api-clients/middleware": "^1.0", - "guzzlehttp/psr7": "^1.3", - "react/cache": "^0.4.1" + "react/cache": "^0.4.1", + "ringcentral/psr7": "^1.2" }, "require-dev": { "api-clients/test-utilities": "^2.0" diff --git a/composer.lock b/composer.lock index 2f8a050..a98481c 100644 --- a/composer.lock +++ b/composer.lock @@ -4,8 +4,8 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "hash": "2ba6f05340cce89b6093d1a822ac278b", - "content-hash": "2b5982ac19a862966c69079b8f7d16c1", + "hash": "62018b0f3d0d5809054c17c9df1235c8", + "content-hash": "86fb7c110e642134d0ee668b46439403", "packages": [ { "name": "api-clients/middleware", @@ -54,64 +54,6 @@ "description": "Request middleware", "time": "2016-12-05 07:52:20" }, - { - "name": "guzzlehttp/psr7", - "version": "1.3.1", - "source": { - "type": "git", - "url": "https://github.com/guzzle/psr7.git", - "reference": "5c6447c9df362e8f8093bda8f5d8873fe5c7f65b" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/guzzle/psr7/zipball/5c6447c9df362e8f8093bda8f5d8873fe5c7f65b", - "reference": "5c6447c9df362e8f8093bda8f5d8873fe5c7f65b", - "shasum": "" - }, - "require": { - "php": ">=5.4.0", - "psr/http-message": "~1.0" - }, - "provide": { - "psr/http-message-implementation": "1.0" - }, - "require-dev": { - "phpunit/phpunit": "~4.0" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-master": "1.4-dev" - } - }, - "autoload": { - "psr-4": { - "GuzzleHttp\\Psr7\\": "src/" - }, - "files": [ - "src/functions_include.php" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Michael Dowling", - "email": "mtdowling@gmail.com", - "homepage": "https://github.com/mtdowling" - } - ], - "description": "PSR-7 message implementation", - "keywords": [ - "http", - "message", - "stream", - "uri" - ], - "time": "2016-06-24 23:00:38" - }, { "name": "psr/http-message", "version": "1.0.1", @@ -238,6 +180,64 @@ "promises" ], "time": "2016-12-22 14:09:01" + }, + { + "name": "ringcentral/psr7", + "version": "1.2.1", + "source": { + "type": "git", + "url": "https://github.com/ringcentral/psr7.git", + "reference": "2594fb47cdc659f3fcf0aa1559b7355460555303" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/ringcentral/psr7/zipball/2594fb47cdc659f3fcf0aa1559b7355460555303", + "reference": "2594fb47cdc659f3fcf0aa1559b7355460555303", + "shasum": "" + }, + "require": { + "php": ">=5.3", + "psr/http-message": "~1.0" + }, + "provide": { + "psr/http-message-implementation": "1.0" + }, + "require-dev": { + "phpunit/phpunit": "~4.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.0-dev" + } + }, + "autoload": { + "psr-4": { + "RingCentral\\Psr7\\": "src/" + }, + "files": [ + "src/functions_include.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Michael Dowling", + "email": "mtdowling@gmail.com", + "homepage": "https://github.com/mtdowling" + } + ], + "description": "PSR-7 message implementation", + "keywords": [ + "http", + "message", + "stream", + "uri" + ], + "time": "2016-03-25 17:36:49" } ], "packages-dev": [ From a0ff30cbe0da56bce6285ce1c98652b658b111f8 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 31 Dec 2016 20:19:07 +0100 Subject: [PATCH 06/16] Fixed testNotGet --- tests/CacheMiddlewareTest.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/CacheMiddlewareTest.php b/tests/CacheMiddlewareTest.php index 804da94..5af1eed 100644 --- a/tests/CacheMiddlewareTest.php +++ b/tests/CacheMiddlewareTest.php @@ -3,6 +3,8 @@ namespace ApiClients\Tests\Middleware\Cache; use ApiClients\Middleware\Cache\CacheMiddleware; +use ApiClients\Middleware\Cache\Options; +use ApiClients\Middleware\Cache\Strategy\Always; use ApiClients\Tools\TestUtilities\TestCase; use Prophecy\Argument; use Psr\Http\Message\RequestInterface; @@ -27,8 +29,8 @@ public function providerMethod() yield ['LOLCAT']; yield [time()]; yield [mt_rand()]; - yield [mt_rand(0, time())]; - yield [mt_rand(time(), time() * time())]; + yield [random_int(0, time())]; + yield [random_int(time(), time() * time())]; } /** @@ -36,6 +38,12 @@ public function providerMethod() */ public function testNotGet(string $method) { + $options = [ + CacheMiddleware::class => [ + Options::STRATEGY => new Always(), + ], + ]; + $request = $this->prophesize(RequestInterface::class); $request->getMethod()->shouldBeCalled()->willReturn($method); @@ -47,7 +55,7 @@ public function testNotGet(string $method) $this->assertSame( $requestInstance, await( - $middleware->pre($requestInstance), + $middleware->pre($requestInstance, $options), Factory::create() ) ); From 2e652437deb901226953e4b19faafceab92b8cf0 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 31 Dec 2016 23:07:53 +0100 Subject: [PATCH 07/16] Cache option --- src/Options.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Options.php b/src/Options.php index aa412d8..9b3a253 100644 --- a/src/Options.php +++ b/src/Options.php @@ -2,8 +2,11 @@ namespace ApiClients\Middleware\Cache; +use React\Cache\CacheInterface; + final class Options { - const STRATEGY = StrategyInterface::class; + const CACHE = CacheInterface::class; + const STRATEGY = StrategyInterface::class; const DEFAULT_TTL = 'default-ttl'; } From 4f3a2b71e04fbb95c2b58ee2363f92ad323b6521 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 31 Dec 2016 23:34:06 +0100 Subject: [PATCH 08/16] Dropped old tests, will write new ones while reworking the middleware --- tests/CacheMiddlewareTest.php | 143 ++-------------------------------- 1 file changed, 7 insertions(+), 136 deletions(-) diff --git a/tests/CacheMiddlewareTest.php b/tests/CacheMiddlewareTest.php index 5af1eed..823531c 100644 --- a/tests/CacheMiddlewareTest.php +++ b/tests/CacheMiddlewareTest.php @@ -4,20 +4,17 @@ use ApiClients\Middleware\Cache\CacheMiddleware; use ApiClients\Middleware\Cache\Options; -use ApiClients\Middleware\Cache\Strategy\Always; +use ApiClients\Middleware\Cache\StrategyInterface; use ApiClients\Tools\TestUtilities\TestCase; use Prophecy\Argument; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; -use Psr\Http\Message\UriInterface; use React\Cache\CacheInterface; -use function Clue\React\Block\await; use React\EventLoop\Factory; -use React\Promise\FulfilledPromise; -use React\Promise\RejectedPromise; +use function Clue\React\Block\await; use function React\Promise\resolve; -class CacheMiddlewareTest extends TestCase +final class CacheMiddlewareTest extends TestCase { public function providerMethod() { @@ -40,7 +37,8 @@ public function testNotGet(string $method) { $options = [ CacheMiddleware::class => [ - Options::STRATEGY => new Always(), + Options::CACHE => $this->prophesize(CacheInterface::class)->reveal(), + Options::STRATEGY => $this->prophesize(StrategyInterface::class)->reveal(), ], ]; @@ -48,11 +46,9 @@ public function testNotGet(string $method) $request->getMethod()->shouldBeCalled()->willReturn($method); $requestInstance = $request->reveal(); - $cache = $this->prophesize(CacheInterface::class); - $cache->set(Argument::type('string'), Argument::type('string'))->shouldNotBeCalled(); - $middleware = new CacheMiddleware($cache->reveal()); + $middleware = new CacheMiddleware(); - $this->assertSame( + self::assertSame( $requestInstance, await( $middleware->pre($requestInstance, $options), @@ -62,129 +58,4 @@ public function testNotGet(string $method) $middleware->post($this->prophesize(ResponseInterface::class)->reveal()); } - - public function provideUri() - { - $uri = $this->prophesize(UriInterface::class); - $uri->getScheme()->shouldBeCalled()->willReturn('https'); - $uri->getHost()->shouldBeCalled()->willReturn('example.com'); - $uri->getPort()->shouldBeCalled()->willReturn(); - $uri->getPath()->shouldBeCalled()->willReturn('/'); - $uri->getQuery()->shouldBeCalled()->willReturn(''); - - yield [$uri->reveal()]; - - $uri = $this->prophesize(UriInterface::class); - $uri->getScheme()->shouldBeCalled()->willReturn('https'); - $uri->getHost()->shouldBeCalled()->willReturn('example.com'); - $uri->getPort()->shouldBeCalled()->willReturn(); - $uri->getPath()->shouldBeCalled()->willReturn(); - $uri->getQuery()->shouldBeCalled()->willReturn(); - - yield [$uri->reveal()]; - - $uri = $this->prophesize(UriInterface::class); - $uri->getScheme()->shouldBeCalled()->willReturn('https'); - $uri->getHost()->shouldBeCalled()->willReturn('example.com'); - $uri->getPort()->shouldBeCalled()->willReturn(80); - $uri->getPath()->shouldBeCalled()->willReturn('/'); - $uri->getQuery()->shouldBeCalled()->willReturn('?blaat'); - - yield [$uri->reveal()]; - } - - /** - * @dataProvider provideUri - */ - public function testNotInCache(UriInterface $uri) - { - $request = $this->prophesize(RequestInterface::class); - $request->getUri()->shouldBeCalled()->willReturn($uri); - $request->getMethod()->shouldBeCalled()->willReturn('GET'); - - $requestInstance = $request->reveal(); - $cache = $this->prophesize(CacheInterface::class); - $cache->get(Argument::type('string'))->shouldBeCalled()->willReturn(new RejectedPromise()); - $middleware = new CacheMiddleware($cache->reveal()); - - $this->assertSame( - $requestInstance, - await( - $middleware->pre($requestInstance), - Factory::create() - ) - ); - } - - public function testInCache() - { - $uri = $this->prophesize(UriInterface::class)->reveal(); - - $request = $this->prophesize(RequestInterface::class); - $request->getUri()->shouldBeCalled()->willReturn($uri); - $request->getMethod()->shouldBeCalled()->willReturn('GET'); - $requestInstance = $request->reveal(); - - $document = '{"body":"foo","headers":[],"protocol_version":3.0,"reason_phrase":"w00t w00t","status_code":9001}'; - $cache = $this->prophesize(CacheInterface::class); - $cache->get(Argument::type('string'))->shouldBeCalled()->willReturn(new FulfilledPromise($document)); - $middleware = new CacheMiddleware($cache->reveal()); - - $response = await( - $middleware->pre($requestInstance)->then(null, function (ResponseInterface $response) { - return resolve($response); - }), - Factory::create() - ); - - $this->assertSame('foo', (string)$response->getBody()); - $this->assertSame([], $response->getHeaders()); - $this->assertSame(3.0, $response->getProtocolVersion()); - $this->assertSame('w00t w00t', $response->getReasonPhrase()); - $this->assertSame(9001, $response->getStatusCode()); - } - - public function testSaveCache() - { - - $uri = $this->prophesize(UriInterface::class); - $uri->getScheme()->shouldBeCalled()->willReturn('https'); - $uri->getHost()->shouldBeCalled()->willReturn('example.com'); - $uri->getPort()->shouldBeCalled()->willReturn(); - $uri->getPath()->shouldBeCalled()->willReturn('/'); - $uri->getQuery()->shouldBeCalled()->willReturn(''); - - $request = $this->prophesize(RequestInterface::class); - $request->getUri()->shouldBeCalled()->willReturn($uri->reveal()); - $request->getMethod()->shouldBeCalled()->willReturn('GET'); - - $response = $this->prophesize(ResponseInterface::class); - $response->getBody()->shouldBeCalled()->willReturn('foo'); - $response->getHeaders()->shouldBeCalled()->willReturn([]); - $response->getProtocolVersion()->shouldBeCalled()->willReturn(3.0); - $response->getReasonPhrase()->shouldBeCalled()->willReturn('w00t w00t'); - $response->getStatusCode()->shouldBeCalled()->willReturn(9001); - $responseInstance = $response->reveal(); - - $cache = $this->prophesize(CacheInterface::class); - $cache->get(Argument::type('string'))->shouldBeCalled()->willReturn(new RejectedPromise()); - $cache->set(Argument::type('string'), Argument::any('string'))->shouldBeCalled(); - $middleware = new CacheMiddleware($cache->reveal()); - - await( - $middleware->pre($request->reveal()), - Factory::create() - ); - - $processedResponse = await( - $middleware->post($responseInstance), - Factory::create() - ); - - $this->assertSame('foo', (string)$processedResponse->getBody()); - $this->assertSame([], $processedResponse->getHeaders()); - $this->assertSame(3.0, $processedResponse->getProtocolVersion()); - $this->assertSame('w00t w00t', $processedResponse->getReasonPhrase()); - $this->assertSame(9001, $processedResponse->getStatusCode()); - } } From 89ef00cb79f5a2d53acab9d34167e135997d4531 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 31 Dec 2016 23:38:26 +0100 Subject: [PATCH 09/16] Moved cache implementation injection into options --- src/CacheMiddleware.php | 53 ++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index d3c1249..3046eae 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -11,12 +11,11 @@ use React\Cache\CacheInterface; use React\Promise\CancellablePromiseInterface; use React\Promise\PromiseInterface; -use function React\Promise\resolve; -use React\Stream\BufferedSink; -use React\Stream\ReadableStreamInterface; use RingCentral\Psr7\BufferStream; +use function React\Promise\reject; +use function React\Promise\resolve; -class CacheMiddleware implements MiddlewareInterface +final class CacheMiddleware implements MiddlewareInterface { use DefaultPriorityTrait; @@ -45,14 +44,6 @@ class CacheMiddleware implements MiddlewareInterface */ private $strategy; - /** - * @param CacheInterface$cache - */ - public function __construct(CacheInterface $cache) - { - $this->cache = $cache; - } - /** * @param RequestInterface $request * @param array $options @@ -60,11 +51,12 @@ public function __construct(CacheInterface $cache) */ public function pre(RequestInterface $request, array $options = []): CancellablePromiseInterface { - if (!isset($options[self::class][Options::STRATEGY])) { + if (!isset($options[self::class][Options::CACHE]) || !isset($options[self::class][Options::STRATEGY])) { return resolve($request); } + $this->cache = $options[self::class][Options::CACHE]; $this->strategy = $options[self::class][Options::STRATEGY]; - if (!($this->strategy instanceof StrategyInterface)) { + if (!($this->cache instanceof CacheInterface) || !($this->strategy instanceof StrategyInterface)) { return resolve($request); } @@ -76,10 +68,14 @@ public function pre(RequestInterface $request, array $options = []): Cancellable $this->key = $this->determineCacheKey($request->getUri()); return $this->cache->get($this->key)->then(function (string $document) { - return resolve( - $this->buildResponse($document) - ); - }, function () use ($request) { + $document = json_decode($document, true); + + if ($document['expires_at'] > time()) { + return resolve($this->buildResponse($document)); + } + + return reject(); + })->otherwise(function () use ($request) { if ($this->strategy->preCheck($request)) { $this->store = true; } @@ -89,12 +85,11 @@ public function pre(RequestInterface $request, array $options = []): Cancellable } /** - * @param string $document + * @param array $document * @return Response */ - protected function buildResponse(string $document): Response + protected function buildResponse(array $document): Response { - $document = json_decode($document, true); return new Response( $document['status_code'], $document['headers'], @@ -111,7 +106,7 @@ protected function buildResponse(string $document): Response */ public function post(ResponseInterface $response, array $options = []): CancellablePromiseInterface { - if (!($this->strategy instanceof StrategyInterface)) { + if (!($this->cache instanceof CacheInterface) || !($this->strategy instanceof StrategyInterface)) { return resolve($response); } @@ -148,6 +143,8 @@ public function post(ResponseInterface $response, array $options = []): Cancella $response->getReasonPhrase() ) ); + }, function () use ($response) { + return resolve($response); }); } @@ -181,16 +178,18 @@ protected function stripExtraSlashes(string $string): string return preg_replace('#/+#', '/', $string); } + /** + * Get the body, but only if the body has been buffered in already + * + * @param ResponseInterface $response + * @return PromiseInterface + */ protected function getBody(ResponseInterface $response): PromiseInterface { if ($response->getBody() instanceof BufferStream) { return resolve($response->getBody()->getContents()); } - if ($response->getBody() instanceof ReadableStreamInterface) { - return BufferedSink::createPromise($response->getBody()); - } - - throw new \Exception('Can\'t get body yet'); + return reject(); } } From 621620bc70d6de32cb41353fe64daf47cc97e342 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sun, 1 Jan 2017 13:09:27 +0100 Subject: [PATCH 10/16] Glue option --- src/Options.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Options.php b/src/Options.php index 9b3a253..67dd2d3 100644 --- a/src/Options.php +++ b/src/Options.php @@ -9,4 +9,5 @@ final class Options const CACHE = CacheInterface::class; const STRATEGY = StrategyInterface::class; const DEFAULT_TTL = 'default-ttl'; + const GLUE = 'glue'; } From ac41a460fe60ff2d9ebb41b4f44b04a705deb3db Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sun, 1 Jan 2017 13:15:37 +0100 Subject: [PATCH 11/16] Document for handling the (un)serializing of documents into and from the cache --- src/Document.php | 78 +++++++++++++++++++++++++++++++ tests/DocumentTest.php | 102 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+) create mode 100644 src/Document.php create mode 100644 tests/DocumentTest.php diff --git a/src/Document.php b/src/Document.php new file mode 100644 index 0000000..ff67d39 --- /dev/null +++ b/src/Document.php @@ -0,0 +1,78 @@ +response = $response; + $this->expiresAt = $expiresAt; + } + + /** + * @return ResponseInterface + */ + public function getResponse(): ResponseInterface + { + return $this->response; + } + + /** + * @return bool + */ + public function hasExpired(): bool + { + return time() >= $this->expiresAt; + } + + public function __toString(): string + { + $contents = $this->response->getBody()->getContents(); + $stream = new BufferStream(strlen($contents)); + $stream->write($contents); + $this->response = $this->response->withBody($stream); + return json_encode([ + 'status_code' => $this->response->getStatusCode(), + 'headers' => $this->response->getHeaders(), + 'body' => $contents, + 'protocol_version' => $this->response->getProtocolVersion(), + 'reason_phrase' => $this->response->getReasonPhrase(), + 'expires_at' => $this->expiresAt, + ]); + } +} diff --git a/tests/DocumentTest.php b/tests/DocumentTest.php new file mode 100644 index 0000000..7c0dc92 --- /dev/null +++ b/tests/DocumentTest.php @@ -0,0 +1,102 @@ +getStatusCode(), $document->getResponse()->getStatusCode()); + self::assertSame($response->getHeaders(), $document->getResponse()->getHeaders()); + self::assertSame($response->getBody()->getContents(), $document->getResponse()->getBody()->getContents()); + self::assertSame($response->getProtocolVersion(), $document->getResponse()->getProtocolVersion()); + self::assertSame($response->getReasonPhrase(), $document->getResponse()->getReasonPhrase()); + self::assertSame($expired, $document->hasExpired()); + } + + public function testCreateFromResponse() + { + $response = new Response( + 200, + [], + 'foo.bar', + '3.0', + 'OK' + ); + + $document = Document::createFromResponse($response, 1); + self::assertFalse($document->hasExpired()); + self::assertSame($response, $document->getResponse()); + + sleep(2); + + self::assertTrue($document->hasExpired()); + } + + public function testCreateFromResponseNotExpired() + { + $response = new Response( + 200, + [], + 'foo.bar', + '3.0', + 'OK' + ); + + $document = Document::createFromResponse($response, 5); + self::assertFalse($document->hasExpired()); + self::assertSame($response, $document->getResponse()); + + sleep(2); + + self::assertFalse($document->hasExpired()); + } +} From 6beed2d53909bcff8c2838cd01037c8334323fbf Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sun, 1 Jan 2017 18:38:28 +0100 Subject: [PATCH 12/16] Cache key --- src/CacheKey.php | 51 ++++++++++++++++++++++++++++++++++++++++++ tests/CacheKeyTest.php | 39 ++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 src/CacheKey.php create mode 100644 tests/CacheKeyTest.php diff --git a/src/CacheKey.php b/src/CacheKey.php new file mode 100644 index 0000000..0187a11 --- /dev/null +++ b/src/CacheKey.php @@ -0,0 +1,51 @@ +getScheme(), + (string)$uri->getHost(), + (string)$uri->getPort(), + self::chunkUp(md5((string)$uri->getPath()), $glue), + self::chunkUp(md5((string)$uri->getQuery()), $glue), + ] + ), + $glue + ); + } + + /** + * @param string $string + * @param string $glue + * @return string + */ + private static function chunkUp(string $string, string $glue): string + { + return implode($glue, str_split($string, 2)); + } + + /** + * @param string $string + * @return string + */ + private static function stripExtraSlashes(string $string, string $glue): string + { + return preg_replace('#' . $glue . '+#', $glue, $string); + } +} diff --git a/tests/CacheKeyTest.php b/tests/CacheKeyTest.php new file mode 100644 index 0000000..28f5cfe --- /dev/null +++ b/tests/CacheKeyTest.php @@ -0,0 +1,39 @@ + Date: Sun, 1 Jan 2017 22:04:19 +0100 Subject: [PATCH 13/16] Reworked strategies to a single decide method keeping things simpler --- src/Strategy/Always.php | 7 +------ src/StrategyInterface.php | 11 ++--------- tests/Strategy/AlwaysTest.php | 14 +++++--------- 3 files changed, 8 insertions(+), 24 deletions(-) diff --git a/src/Strategy/Always.php b/src/Strategy/Always.php index cafb31d..c4d78b8 100644 --- a/src/Strategy/Always.php +++ b/src/Strategy/Always.php @@ -10,12 +10,7 @@ final class Always implements StrategyInterface { const ALWAYS_TTL = 300; - public function preCheck(RequestInterface $request): bool - { - return true; - } - - public function postCheck(ResponseInterface $response): bool + public function decide(RequestInterface $request, ResponseInterface $response): bool { return true; } diff --git a/src/StrategyInterface.php b/src/StrategyInterface.php index 95d7133..e60cd82 100644 --- a/src/StrategyInterface.php +++ b/src/StrategyInterface.php @@ -10,20 +10,13 @@ interface StrategyInterface const DEFAULT_TTL = 0; /** - * Check whether to store to cache based on the request + * Determine whether to store to cache based on the request & response * * @param RequestInterface $request - * @return bool - */ - public function preCheck(RequestInterface $request): bool; - - /** - * Check whether to store to cache based on the response - * * @param ResponseInterface $response * @return bool */ - public function postCheck(ResponseInterface $response): bool; + public function decide(RequestInterface $request, ResponseInterface $response): bool; /** * Determine what the TTL for the cache entry should be based on the request and response diff --git a/tests/Strategy/AlwaysTest.php b/tests/Strategy/AlwaysTest.php index caccce7..bc2a7db 100644 --- a/tests/Strategy/AlwaysTest.php +++ b/tests/Strategy/AlwaysTest.php @@ -9,17 +9,13 @@ final class AlwaysTest extends TestCase { - public function testPreCheck() + public function testDecide() { self::assertTrue( - (new Always())->preCheck($this->prophesize(RequestInterface::class)->reveal()) - ); - } - - public function testPostCheck() - { - self::assertTrue( - (new Always())->postCheck($this->prophesize(ResponseInterface::class)->reveal()) + (new Always())->decide( + $this->prophesize(RequestInterface::class)->reveal(), + $this->prophesize(ResponseInterface::class)->reveal() + ) ); } From 60e3090bcc039523a6f4c793c5d5d56754ae9ee1 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sun, 1 Jan 2017 23:12:05 +0100 Subject: [PATCH 14/16] Reworked cached middleware to use strategies, the document and cache key classes --- src/CacheMiddleware.php | 117 ++++++++------------------------- tests/CacheMiddlewareTest.php | 118 +++++++++++++++++++++++++++++++++- 2 files changed, 144 insertions(+), 91 deletions(-) diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index 3046eae..b99a0f0 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -4,10 +4,8 @@ use ApiClients\Foundation\Middleware\DefaultPriorityTrait; use ApiClients\Foundation\Middleware\MiddlewareInterface; -use GuzzleHttp\Psr7\Response; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; -use Psr\Http\Message\UriInterface; use React\Cache\CacheInterface; use React\Promise\CancellablePromiseInterface; use React\Promise\PromiseInterface; @@ -19,6 +17,8 @@ final class CacheMiddleware implements MiddlewareInterface { use DefaultPriorityTrait; + const DEFAULT_GLUE = '/'; + /** * @var CacheInterface */ @@ -65,40 +65,24 @@ public function pre(RequestInterface $request, array $options = []): Cancellable } $this->request = $request; + $this->key = CacheKey::create( + $this->request->getUri(), + $options[self::class][Options::GLUE] ?? self::DEFAULT_GLUE + ); - $this->key = $this->determineCacheKey($request->getUri()); - return $this->cache->get($this->key)->then(function (string $document) { - $document = json_decode($document, true); - - if ($document['expires_at'] > time()) { - return resolve($this->buildResponse($document)); - } + return $this->cache->get($this->key)->then(function (string $json) { + $document = Document::createFromString($json); - return reject(); - })->otherwise(function () use ($request) { - if ($this->strategy->preCheck($request)) { - $this->store = true; + if ($document->hasExpired()) { + return resolve($this->request); } - return resolve($request); + return reject($document->getResponse()); + }, function () { + return resolve($this->request); }); } - /** - * @param array $document - * @return Response - */ - protected function buildResponse(array $document): Response - { - return new Response( - $document['status_code'], - $document['headers'], - $document['body'], - $document['protocol_version'], - $document['reason_phrase'] - ); - } - /** * @param ResponseInterface $response * @param array $options @@ -106,88 +90,41 @@ protected function buildResponse(array $document): Response */ public function post(ResponseInterface $response, array $options = []): CancellablePromiseInterface { - if (!($this->cache instanceof CacheInterface) || !($this->strategy instanceof StrategyInterface)) { + if (!($this->request instanceof RequestInterface)) { return resolve($response); } - if (!is_string($this->key)) { - return resolve($response); - } - - if ($this->strategy->postCheck($response)) { - $this->store = true; - } + $this->store = $this->strategy->decide($this->request, $response); if (!$this->store) { return resolve($response); } - return $this->getBody($response)->then(function (string $contents) use ($response) { - $document = [ - 'expires_at' => time() + $this->strategy->determineTtl($this->request, $response), - 'body' => $contents, - 'headers' => $response->getHeaders(), - 'protocol_version' => $response->getProtocolVersion(), - 'reason_phrase' => $response->getReasonPhrase(), - 'status_code' => $response->getStatusCode(), - ]; - - $this->cache->set($this->key, json_encode($document)); - - return resolve( - new Response( - $response->getStatusCode(), - $response->getHeaders(), - $contents, - $response->getProtocolVersion(), - $response->getReasonPhrase() + return $this->hasBody($response)->then(function (ResponseInterface $response) { + $document = Document::createFromResponse( + $response, + $this->strategy->determineTtl( + $this->request, + $response ) ); + + $this->cache->set($this->key, (string)$document); + + return resolve($document->getResponse()); }, function () use ($response) { return resolve($response); }); } - /** - * @param UriInterface $uri - * @return string - */ - protected function determineCacheKey(UriInterface $uri): string - { - return $this->stripExtraSlashes( - implode( - '/', - [ - (string)$uri->getScheme(), - (string)$uri->getHost(), - (string)$uri->getPort(), - (string)$uri->getPath(), - md5((string)$uri->getQuery()), - ] - ) - ); - } - - /** - * @param string $string - * @return string - */ - protected function stripExtraSlashes(string $string): string - { - return preg_replace('#/+#', '/', $string); - } - - /** - * Get the body, but only if the body has been buffered in already - * * @param ResponseInterface $response * @return PromiseInterface */ - protected function getBody(ResponseInterface $response): PromiseInterface + protected function hasBody(ResponseInterface $response): PromiseInterface { if ($response->getBody() instanceof BufferStream) { - return resolve($response->getBody()->getContents()); + return resolve($response); } return reject(); diff --git a/tests/CacheMiddlewareTest.php b/tests/CacheMiddlewareTest.php index 823531c..fe58295 100644 --- a/tests/CacheMiddlewareTest.php +++ b/tests/CacheMiddlewareTest.php @@ -3,6 +3,7 @@ namespace ApiClients\Tests\Middleware\Cache; use ApiClients\Middleware\Cache\CacheMiddleware; +use ApiClients\Middleware\Cache\Document; use ApiClients\Middleware\Cache\Options; use ApiClients\Middleware\Cache\StrategyInterface; use ApiClients\Tools\TestUtilities\TestCase; @@ -11,7 +12,11 @@ use Psr\Http\Message\ResponseInterface; use React\Cache\CacheInterface; use React\EventLoop\Factory; +use RingCentral\Psr7\BufferStream; +use RingCentral\Psr7\Request; +use RingCentral\Psr7\Response; use function Clue\React\Block\await; +use function React\Promise\reject; use function React\Promise\resolve; final class CacheMiddlewareTest extends TestCase @@ -33,7 +38,7 @@ public function providerMethod() /** * @dataProvider providerMethod */ - public function testNotGet(string $method) + public function testPreNoGet(string $method) { $options = [ CacheMiddleware::class => [ @@ -58,4 +63,115 @@ public function testNotGet(string $method) $middleware->post($this->prophesize(ResponseInterface::class)->reveal()); } + + public function testPreGetCache() + { + $documentString = (string)Document::createFromResponse( + new Response(123, [], 'foo.bar'), + 5 + ); + $cache = $this->prophesize(CacheInterface::class); + $cache->get(Argument::type('string'))->shouldBecalled()->willReturn(resolve($documentString)); + + $options = [ + CacheMiddleware::class => [ + Options::CACHE => $cache->reveal(), + Options::STRATEGY => $this->prophesize(StrategyInterface::class)->reveal(), + ], + ]; + + $request = new Request('GET', 'foo.bar'); + + $response = null; + $middleware = new CacheMiddleware(); + $middleware->pre($request, $options)->otherwise(function ($responseObject) use (&$response) { + $response = $responseObject; + }); + self::assertNotNull($response); + + self::assertSame(123, $response->getStatusCode()); + self::assertSame('foo.bar', $response->getBody()->getContents()); + } + + public function testPreGetNoCache() + { + $request = new Request('GET', 'foo.bar'); + + $cache = $this->prophesize(CacheInterface::class); + $cache->get(Argument::type('string'))->shouldBecalled()->willReturn(reject()); + + $options = [ + CacheMiddleware::class => [ + Options::CACHE => $cache->reveal(), + Options::STRATEGY => $this->prophesize(StrategyInterface::class)->reveal(), + ], + ]; + + $middleware = new CacheMiddleware(); + $response = await($middleware->pre($request, $options), Factory::create()); + + self::assertSame($request, $response); + } + + public function testPreGetExpired() + { + $documentString = (string)Document::createFromResponse( + new Response(123, [], 'foo.bar'), + 0 + ); + + sleep(2); + + $cache = $this->prophesize(CacheInterface::class); + $cache->get(Argument::type('string'))->shouldBecalled()->willReturn(resolve($documentString)); + + $options = [ + CacheMiddleware::class => [ + Options::CACHE => $cache->reveal(), + Options::STRATEGY => $this->prophesize(StrategyInterface::class)->reveal(), + ], + ]; + + $request = new Request('GET', 'foo.bar'); + + $middleware = new CacheMiddleware(); + $response = await($middleware->pre($request, $options), Factory::create()); + + self::assertSame($request, $response); + } + + public function testPost() + { + $request = new Request('GET', 'foo.bar'); + + $body = 'foo.bar'; + $stream = new BufferStream(strlen($body)); + $stream->write($body); + $response = (new Response(200, []))->withBody($stream); + + $cache = $this->prophesize(CacheInterface::class); + $cache->get(Argument::type('string'))->shouldBecalled()->willReturn(reject()); + $cache->set( + Argument::type('string'), + Argument::type('string') + )->shouldBecalled(); + + $strategy = $this->prophesize(StrategyInterface::class); + $strategy->determineTtl(Argument::type(RequestInterface::class), Argument::type(ResponseInterface::class))->shouldBeCalled()->willReturn(true); + $strategy->decide(Argument::type(RequestInterface::class), Argument::type(ResponseInterface::class))->shouldBeCalled()->willReturn(true); + + $options = [ + CacheMiddleware::class => [ + Options::CACHE => $cache->reveal(), + Options::STRATEGY => $strategy->reveal(), + ], + ]; + + $middleware = new CacheMiddleware(); + $middleware->pre($request, $options); + $responseObject = await($middleware->post($response, $options), Factory::create()); + + self::assertSame($response->getStatusCode(), $responseObject->getStatusCode()); + self::assertSame($response->getBody()->getContents(), $responseObject->getBody()->getContents()); + } } From f043bebf8a1e59310e6397b5655b018e67c9b860 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Mon, 2 Jan 2017 11:51:22 +0100 Subject: [PATCH 15/16] Fixed test --- tests/CacheMiddlewareTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/CacheMiddlewareTest.php b/tests/CacheMiddlewareTest.php index fe58295..c01f82d 100644 --- a/tests/CacheMiddlewareTest.php +++ b/tests/CacheMiddlewareTest.php @@ -172,6 +172,6 @@ public function testPost() $responseObject = await($middleware->post($response, $options), Factory::create()); self::assertSame($response->getStatusCode(), $responseObject->getStatusCode()); - self::assertSame($response->getBody()->getContents(), $responseObject->getBody()->getContents()); + self::assertSame($body, $responseObject->getBody()->getContents()); } } From 1c1614ce607dd044dac5f712c09a43f80de13a15 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Mon, 2 Jan 2017 14:56:29 +0100 Subject: [PATCH 16/16] Ensure we remove item from cache when expired --- src/CacheMiddleware.php | 1 + tests/CacheMiddlewareTest.php | 1 + 2 files changed, 2 insertions(+) diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index b99a0f0..c068ff4 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -74,6 +74,7 @@ public function pre(RequestInterface $request, array $options = []): Cancellable $document = Document::createFromString($json); if ($document->hasExpired()) { + $this->cache->remove($this->key); return resolve($this->request); } diff --git a/tests/CacheMiddlewareTest.php b/tests/CacheMiddlewareTest.php index c01f82d..a9af981 100644 --- a/tests/CacheMiddlewareTest.php +++ b/tests/CacheMiddlewareTest.php @@ -124,6 +124,7 @@ public function testPreGetExpired() $cache = $this->prophesize(CacheInterface::class); $cache->get(Argument::type('string'))->shouldBecalled()->willReturn(resolve($documentString)); + $cache->remove(Argument::type('string'))->shouldBecalled(); $options = [ CacheMiddleware::class => [