From 7482f22b5c68e1ceb8262f6298ef512a65d1b8e3 Mon Sep 17 00:00:00 2001 From: Rhys Date: Sun, 16 Jan 2022 18:54:01 +1000 Subject: [PATCH 01/10] fixes #149 invalidate items for all methods on unsafe method --- src/CacheMiddleware.php | 4 ++++ tests/InvalidateCacheTest.php | 37 +++++++++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index 0f6ea2a..48ed998 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -377,6 +377,10 @@ private function invalidateCache(RequestInterface $request, ResponseInterface $r { $this->cacheStorage->delete($request); + foreach (array_diff(array_keys($this->httpMethods), [$request->getMethod()]) as $method) { + $this->cacheStorage->delete($request->withMethod($method)); + } + return $response->withHeader(self::HEADER_INVALIDATION, true); } } diff --git a/tests/InvalidateCacheTest.php b/tests/InvalidateCacheTest.php index 50cf991..a906084 100644 --- a/tests/InvalidateCacheTest.php +++ b/tests/InvalidateCacheTest.php @@ -2,11 +2,14 @@ namespace Kevinrob\GuzzleCache\Tests; +use Cache\Adapter\PHPArray\ArrayCachePool; use GuzzleHttp\Client; use GuzzleHttp\HandlerStack; use GuzzleHttp\Promise\FulfilledPromise; use GuzzleHttp\Psr7\Response; use Kevinrob\GuzzleCache\CacheMiddleware; +use Kevinrob\GuzzleCache\Storage\Psr6CacheStorage; +use Kevinrob\GuzzleCache\Strategy\PrivateCacheStrategy; use PHPUnit\Framework\TestCase; class InvalidateCacheTest extends TestCase @@ -16,15 +19,26 @@ class InvalidateCacheTest extends TestCase */ protected $client; + /** + * @var CacheMiddleware + */ + protected $middelware; + protected function setUp(): void { // Create default HandlerStack $stack = HandlerStack::create(function () { - return new FulfilledPromise(new Response()); + return new FulfilledPromise(new Response(200, [ + 'Cache-Control' => 'private, max-age=300' + ])); }); + $this->middelware = new CacheMiddleware(new PrivateCacheStrategy( + new Psr6CacheStorage(new ArrayCachePool()), + )); + // Add this middleware to the top with `push` - $stack->push(new CacheMiddleware(), 'cache'); + $stack->push($this->middelware, 'cache'); // Initialize the client with the handler option $this->client = new Client(['handler' => $stack]); @@ -47,4 +61,23 @@ public function testInvalidationCacheIfNotValidHttpMethod() $response = $this->client->patch('anything'); $this->assertSame('1', $response->getHeaderLine(CacheMiddleware::HEADER_INVALIDATION)); } + + public function testItInvalidatesForAllHttpMethods() + { + $this->middelware->setHttpMethods([ + 'GET' => true, + 'POST' => true, + ]); + + $this->client->get('anything'); + $this->client->post('anything'); + + $response = $this->client->delete('anything'); + $this->assertSame('1', $response->getHeaderLine(CacheMiddleware::HEADER_INVALIDATION)); + + $response = $this->client->get('anything'); + $this->assertEquals(CacheMiddleware::HEADER_CACHE_MISS, $response->getHeaderLine('X-Kevinrob-Cache')); + $response = $this->client->post('anything'); + $this->assertEquals(CacheMiddleware::HEADER_CACHE_MISS, $response->getHeaderLine('X-Kevinrob-Cache')); + } } From 196397cfcf76afeb5dec1728d7cc03e148dc81e9 Mon Sep 17 00:00:00 2001 From: Rhys Date: Mon, 17 Jan 2022 07:54:13 +1000 Subject: [PATCH 02/10] add unsafe method config --- src/CacheMiddleware.php | 22 ++++++++---- tests/InvalidateCacheTest.php | 67 +++++++++++++++++++++-------------- 2 files changed, 57 insertions(+), 32 deletions(-) diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index 48ed998..3bff6d1 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -49,6 +49,13 @@ class CacheMiddleware */ protected $httpMethods = ['GET' => true]; + /** + * List of unsafe methods, + * + * @var array + */ + protected $unsafeMethods = ['POST', 'DELETE', 'PUT']; + /** * @param CacheStrategyInterface|null $cacheStrategy */ @@ -113,12 +120,15 @@ public function __invoke(callable $handler) { return function (RequestInterface $request, array $options) use (&$handler) { if (!isset($this->httpMethods[strtoupper($request->getMethod())])) { - // No caching for this method allowed - - return $handler($request, $options)->then( - function (ResponseInterface $response) use ($request) { - // Invalidate cache after a call of non-safe method on the same URI - $response = $this->invalidateCache($request, $response); + // No caching for this method allowed + + + return $handler($request, $options)->then( + function (ResponseInterface $response) use ($request) { + if (in_array($request->getMethod(), $this->unsafeMethods)) { + // Invalidate cache after a call of non-safe method on the same URI + $response = $this->invalidateCache($request, $response); + } return $response->withHeader(self::HEADER_CACHE_INFO, self::HEADER_CACHE_MISS); } diff --git a/tests/InvalidateCacheTest.php b/tests/InvalidateCacheTest.php index a906084..3dbd4e0 100644 --- a/tests/InvalidateCacheTest.php +++ b/tests/InvalidateCacheTest.php @@ -26,7 +26,6 @@ class InvalidateCacheTest extends TestCase protected function setUp(): void { - // Create default HandlerStack $stack = HandlerStack::create(function () { return new FulfilledPromise(new Response(200, [ 'Cache-Control' => 'private, max-age=300' @@ -37,47 +36,63 @@ protected function setUp(): void new Psr6CacheStorage(new ArrayCachePool()), )); - // Add this middleware to the top with `push` $stack->push($this->middelware, 'cache'); - // Initialize the client with the handler option $this->client = new Client(['handler' => $stack]); } - public function testInvalidationCacheIfNotValidHttpMethod() + /** + * @dataProvider unsafeMethods + */ + public function testItInvalidatesForUnsafeHttpMethods($unsafeMethod) { - $response = $this->client->get('anything'); - $this->assertSame('', $response->getHeaderLine(CacheMiddleware::HEADER_INVALIDATION)); + $this->middelware->setHttpMethods([ + 'GET' => true, + 'HEAD' => true, + ]); - $response = $this->client->post('anything'); - $this->assertSame('1', $response->getHeaderLine(CacheMiddleware::HEADER_INVALIDATION)); + $this->client->get('resource'); + $this->client->head('resource'); - $response = $this->client->put('anything'); + $response = $this->client->{$unsafeMethod}('resource'); $this->assertSame('1', $response->getHeaderLine(CacheMiddleware::HEADER_INVALIDATION)); - $response = $this->client->delete('anything'); - $this->assertSame('1', $response->getHeaderLine(CacheMiddleware::HEADER_INVALIDATION)); + $response = $this->client->get('resource'); + $this->assertEquals(CacheMiddleware::HEADER_CACHE_MISS, $response->getHeaderLine('X-Kevinrob-Cache')); - $response = $this->client->patch('anything'); - $this->assertSame('1', $response->getHeaderLine(CacheMiddleware::HEADER_INVALIDATION)); + $response = $this->client->head('resource'); + $this->assertEquals(CacheMiddleware::HEADER_CACHE_MISS, $response->getHeaderLine('X-Kevinrob-Cache')); } - public function testItInvalidatesForAllHttpMethods() + /** + * @dataProvider safeMethods + */ + public function testItDoesInvalidatesForSafeHttpMethods($safeMethod) { - $this->middelware->setHttpMethods([ - 'GET' => true, - 'POST' => true, - ]); + $this->client->get('resource'); - $this->client->get('anything'); - $this->client->post('anything'); + $response = $this->client->{$safeMethod}('resource'); + $this->assertSame('', $response->getHeaderLine(CacheMiddleware::HEADER_INVALIDATION)); - $response = $this->client->delete('anything'); - $this->assertSame('1', $response->getHeaderLine(CacheMiddleware::HEADER_INVALIDATION)); + $response = $this->client->get('resource'); + $this->assertEquals(CacheMiddleware::HEADER_CACHE_HIT, $response->getHeaderLine('X-Kevinrob-Cache')); + } - $response = $this->client->get('anything'); - $this->assertEquals(CacheMiddleware::HEADER_CACHE_MISS, $response->getHeaderLine('X-Kevinrob-Cache')); - $response = $this->client->post('anything'); - $this->assertEquals(CacheMiddleware::HEADER_CACHE_MISS, $response->getHeaderLine('X-Kevinrob-Cache')); + public function unsafeMethods() + { + return [ + 'delete' => ['delete'], + 'put' => ['put'], + 'post' => ['post'], + ]; + } + + public function safemethods() + { + return [ + 'options' => ['options'], + 'trace' => ['trace'], + 'head' => ['head'], + ]; } } From 344f079d0cca5d07b16fca331fd28afc98618781 Mon Sep 17 00:00:00 2001 From: Rhys Date: Mon, 17 Jan 2022 07:56:06 +1000 Subject: [PATCH 03/10] cs --- src/CacheMiddleware.php | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index 3bff6d1..3565a2b 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -120,19 +120,18 @@ public function __invoke(callable $handler) { return function (RequestInterface $request, array $options) use (&$handler) { if (!isset($this->httpMethods[strtoupper($request->getMethod())])) { - // No caching for this method allowed - - - return $handler($request, $options)->then( - function (ResponseInterface $response) use ($request) { - if (in_array($request->getMethod(), $this->unsafeMethods)) { - // Invalidate cache after a call of non-safe method on the same URI - $response = $this->invalidateCache($request, $response); - } + // No caching for this method allowed - return $response->withHeader(self::HEADER_CACHE_INFO, self::HEADER_CACHE_MISS); - } - ); + return $handler($request, $options)->then( + function (ResponseInterface $response) use ($request) { + if (in_array($request->getMethod(), $this->unsafeMethods)) { + // Invalidate cache after a call of non-safe method on the same URI + $response = $this->invalidateCache($request, $response); + } + + return $response->withHeader(self::HEADER_CACHE_INFO, self::HEADER_CACHE_MISS); + } + ); } if ($request->hasHeader(self::HEADER_RE_VALIDATION)) { From 77e54f43ca4f9423bc2ccaebbbd3dcb9f0f83264 Mon Sep 17 00:00:00 2001 From: Rhys Date: Mon, 17 Jan 2022 08:01:31 +1000 Subject: [PATCH 04/10] cs --- src/CacheMiddleware.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index 3565a2b..f1d474b 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -122,16 +122,16 @@ public function __invoke(callable $handler) if (!isset($this->httpMethods[strtoupper($request->getMethod())])) { // No caching for this method allowed - return $handler($request, $options)->then( + return $handler($request, $options)->then( function (ResponseInterface $response) use ($request) { - if (in_array($request->getMethod(), $this->unsafeMethods)) { - // Invalidate cache after a call of non-safe method on the same URI - $response = $this->invalidateCache($request, $response); - } - - return $response->withHeader(self::HEADER_CACHE_INFO, self::HEADER_CACHE_MISS); + if (in_array($request->getMethod(), $this->unsafeMethods)) { + // Invalidate cache after a call of non-safe method on the same URI + $response = $this->invalidateCache($request, $response); } - ); + + return $response->withHeader(self::HEADER_CACHE_INFO, self::HEADER_CACHE_MISS); + } + ); } if ($request->hasHeader(self::HEADER_RE_VALIDATION)) { From 80abb16cd65a122fac80ef9f3abea79c653304ae Mon Sep 17 00:00:00 2001 From: Rhys Date: Mon, 17 Jan 2022 08:07:14 +1000 Subject: [PATCH 05/10] add getter and setter, add test for configuration of unsafe methods --- src/CacheMiddleware.php | 13 +++++++++++++ tests/InvalidateCacheTest.php | 13 +++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index f1d474b..749cbd1 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -103,6 +103,19 @@ public function getHttpMethods() return $this->httpMethods; } + /** + * @param array $unsafeMethods + */ + public function setUnsafeMethods(array $unsafeMethods) + { + $this->unsafeMethods = $unsafeMethods; + } + + public function getUnsafeMethods() + { + return $this->unsafeMethods; + } + /** * Will be called at the end of the script. */ diff --git a/tests/InvalidateCacheTest.php b/tests/InvalidateCacheTest.php index 3dbd4e0..41a916e 100644 --- a/tests/InvalidateCacheTest.php +++ b/tests/InvalidateCacheTest.php @@ -78,6 +78,19 @@ public function testItDoesInvalidatesForSafeHttpMethods($safeMethod) $this->assertEquals(CacheMiddleware::HEADER_CACHE_HIT, $response->getHeaderLine('X-Kevinrob-Cache')); } + public function testItAllowsConfiguringUnsafeMethods() + { + $this->middelware->setUnsafeMethods(['HEAD']); + + $this->client->get('resource'); + + $response = $this->client->head('resource'); + $this->assertSame('1', $response->getHeaderLine(CacheMiddleware::HEADER_INVALIDATION)); + + $response = $this->client->get('resource'); + $this->assertEquals(CacheMiddleware::HEADER_CACHE_MISS, $response->getHeaderLine('X-Kevinrob-Cache')); + } + public function unsafeMethods() { return [ From 515d2d31957beba9eb65245264652a8cfb264488 Mon Sep 17 00:00:00 2001 From: Rhys Date: Mon, 17 Jan 2022 08:13:35 +1000 Subject: [PATCH 06/10] use safe methods as defined by spec --- src/CacheMiddleware.php | 21 +++++---------------- tests/InvalidateCacheTest.php | 14 +------------- 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index 749cbd1..f91b62a 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -50,11 +50,13 @@ class CacheMiddleware protected $httpMethods = ['GET' => true]; /** - * List of unsafe methods, + * List of safe methods + * + * https://datatracker.ietf.org/doc/html/rfc7231#section-4.2.1 * * @var array */ - protected $unsafeMethods = ['POST', 'DELETE', 'PUT']; + protected $safeMethods = ['GET', 'HEAD', 'OPTIONS', 'TRACE']; /** * @param CacheStrategyInterface|null $cacheStrategy @@ -103,19 +105,6 @@ public function getHttpMethods() return $this->httpMethods; } - /** - * @param array $unsafeMethods - */ - public function setUnsafeMethods(array $unsafeMethods) - { - $this->unsafeMethods = $unsafeMethods; - } - - public function getUnsafeMethods() - { - return $this->unsafeMethods; - } - /** * Will be called at the end of the script. */ @@ -137,7 +126,7 @@ public function __invoke(callable $handler) return $handler($request, $options)->then( function (ResponseInterface $response) use ($request) { - if (in_array($request->getMethod(), $this->unsafeMethods)) { + if (!in_array($request->getMethod(), $this->safeMethods)) { // Invalidate cache after a call of non-safe method on the same URI $response = $this->invalidateCache($request, $response); } diff --git a/tests/InvalidateCacheTest.php b/tests/InvalidateCacheTest.php index 41a916e..5da43ec 100644 --- a/tests/InvalidateCacheTest.php +++ b/tests/InvalidateCacheTest.php @@ -78,19 +78,6 @@ public function testItDoesInvalidatesForSafeHttpMethods($safeMethod) $this->assertEquals(CacheMiddleware::HEADER_CACHE_HIT, $response->getHeaderLine('X-Kevinrob-Cache')); } - public function testItAllowsConfiguringUnsafeMethods() - { - $this->middelware->setUnsafeMethods(['HEAD']); - - $this->client->get('resource'); - - $response = $this->client->head('resource'); - $this->assertSame('1', $response->getHeaderLine(CacheMiddleware::HEADER_INVALIDATION)); - - $response = $this->client->get('resource'); - $this->assertEquals(CacheMiddleware::HEADER_CACHE_MISS, $response->getHeaderLine('X-Kevinrob-Cache')); - } - public function unsafeMethods() { return [ @@ -103,6 +90,7 @@ public function unsafeMethods() public function safemethods() { return [ + 'get' => ['get'], 'options' => ['options'], 'trace' => ['trace'], 'head' => ['head'], From 5a595880a11ae482ca0ac5699414d775c4f19c40 Mon Sep 17 00:00:00 2001 From: Rhys Date: Mon, 17 Jan 2022 08:16:09 +1000 Subject: [PATCH 07/10] fix test name --- tests/InvalidateCacheTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/InvalidateCacheTest.php b/tests/InvalidateCacheTest.php index 5da43ec..e1c4709 100644 --- a/tests/InvalidateCacheTest.php +++ b/tests/InvalidateCacheTest.php @@ -67,7 +67,7 @@ public function testItInvalidatesForUnsafeHttpMethods($unsafeMethod) /** * @dataProvider safeMethods */ - public function testItDoesInvalidatesForSafeHttpMethods($safeMethod) + public function testItDoesNotInvalidateForSafeHttpMethods($safeMethod) { $this->client->get('resource'); From 7af3d89780beb45d248532f935bd2f29c1c1f1be Mon Sep 17 00:00:00 2001 From: Rhys Date: Mon, 17 Jan 2022 21:05:20 +1000 Subject: [PATCH 08/10] use isset instead of in array, code style, fix typo --- src/CacheMiddleware.php | 8 +++----- tests/InvalidateCacheTest.php | 8 ++++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index f91b62a..1f8cae3 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -56,7 +56,7 @@ class CacheMiddleware * * @var array */ - protected $safeMethods = ['GET', 'HEAD', 'OPTIONS', 'TRACE']; + protected $safeMethods = ['GET' => true, 'HEAD' => true, 'OPTIONS' => true, 'TRACE' => true]; /** * @param CacheStrategyInterface|null $cacheStrategy @@ -126,7 +126,7 @@ public function __invoke(callable $handler) return $handler($request, $options)->then( function (ResponseInterface $response) use ($request) { - if (!in_array($request->getMethod(), $this->safeMethods)) { + if (!isset($this->safeMethods[$request->getMethod()])) { // Invalidate cache after a call of non-safe method on the same URI $response = $this->invalidateCache($request, $response); } @@ -386,9 +386,7 @@ public static function getMiddleware(CacheStrategyInterface $cacheStorage = null */ private function invalidateCache(RequestInterface $request, ResponseInterface $response) { - $this->cacheStorage->delete($request); - - foreach (array_diff(array_keys($this->httpMethods), [$request->getMethod()]) as $method) { + foreach (array_keys($this->httpMethods) as $method) { $this->cacheStorage->delete($request->withMethod($method)); } diff --git a/tests/InvalidateCacheTest.php b/tests/InvalidateCacheTest.php index e1c4709..1ef8fd4 100644 --- a/tests/InvalidateCacheTest.php +++ b/tests/InvalidateCacheTest.php @@ -22,7 +22,7 @@ class InvalidateCacheTest extends TestCase /** * @var CacheMiddleware */ - protected $middelware; + protected $middleware; protected function setUp(): void { @@ -32,11 +32,11 @@ protected function setUp(): void ])); }); - $this->middelware = new CacheMiddleware(new PrivateCacheStrategy( + $this->middleware = new CacheMiddleware(new PrivateCacheStrategy( new Psr6CacheStorage(new ArrayCachePool()), )); - $stack->push($this->middelware, 'cache'); + $stack->push($this->middleware, 'cache'); $this->client = new Client(['handler' => $stack]); } @@ -46,7 +46,7 @@ protected function setUp(): void */ public function testItInvalidatesForUnsafeHttpMethods($unsafeMethod) { - $this->middelware->setHttpMethods([ + $this->middleware->setHttpMethods([ 'GET' => true, 'HEAD' => true, ]); From 2b4a7dab49e276d5f4ed85cd9fb195b823ffcabb Mon Sep 17 00:00:00 2001 From: Rhys Date: Mon, 17 Jan 2022 21:06:50 +1000 Subject: [PATCH 09/10] fix 7.2 trailing comma --- tests/InvalidateCacheTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/InvalidateCacheTest.php b/tests/InvalidateCacheTest.php index 1ef8fd4..d7b2fdf 100644 --- a/tests/InvalidateCacheTest.php +++ b/tests/InvalidateCacheTest.php @@ -33,7 +33,7 @@ protected function setUp(): void }); $this->middleware = new CacheMiddleware(new PrivateCacheStrategy( - new Psr6CacheStorage(new ArrayCachePool()), + new Psr6CacheStorage(new ArrayCachePool()) )); $stack->push($this->middleware, 'cache'); From 1830d780636128d7de68ebc34f0fdf2bee4a7585 Mon Sep 17 00:00:00 2001 From: Rhys Emmerson Date: Wed, 19 Jan 2022 08:38:42 +1000 Subject: [PATCH 10/10] Update src/CacheMiddleware.php Co-authored-by: Kevin Robatel --- src/CacheMiddleware.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CacheMiddleware.php b/src/CacheMiddleware.php index 1f8cae3..a37386e 100644 --- a/src/CacheMiddleware.php +++ b/src/CacheMiddleware.php @@ -124,7 +124,7 @@ public function __invoke(callable $handler) if (!isset($this->httpMethods[strtoupper($request->getMethod())])) { // No caching for this method allowed - return $handler($request, $options)->then( + return $handler($request, $options)->then( function (ResponseInterface $response) use ($request) { if (!isset($this->safeMethods[$request->getMethod()])) { // Invalidate cache after a call of non-safe method on the same URI