diff --git a/README.md b/README.md index 38917f0..ffa86a4 100644 --- a/README.md +++ b/README.md @@ -16,11 +16,29 @@ RestETagBundle uses REST semantics to form a cache invalidation and optimistic c * Versions the resources your URI paths represents and keeps this list in a server side cache. * Increments the version of a path when one of the following methods is used: POST, PUT, PATCH, DELETE -* Increments the version of all parent paths when that of a child in incremented +* Increments the version of all parent and selected "lower" paths when that of a child in incremented * Ensures the tag passed using If-Match matches the ETag in the cache, returns HTTP 412 in case of discrepancy. * Returns HTTP 428 responses when concurrency control is enabled and the appropriate header is missing. -The bundle uses microtime based version IDs to prevent loss of the server side cache causing collisions and sub-second resource locking. +The bundle uses microtime based version IDs to prevent loss of the server side cache causing collisions and sub-second resource locking. Removes all non-printable and non-ascii chars from URLs before using them as cache keys. + +The versioning scheme is pretty straightforward, examples: + + * Modifying `/animals/rabbits/1`: invalidates `/animals`, `/animals/rabbits`, `/animals/rabbits/1`, and (if it exists) `/animals/rabbits/1/relations/owners` + * GET on `/animals/rabbits/2`: this is not effected by the previous example. In addition, this will create a version if none exists yet (without invalidating anything) + * Modifying `/animals/rabbits`: both `/animals` and `/animals/rabbits` get a new version. + So will any existing versions matching the child invalidation constraint (see configuration), eg `/animals/rabbits/findByName` + +The query part of the URL is treated as the last path segment: + + * Modifying `/animals?type=rabbits`: will be interpreted as modification of `/animals/?type=rabbits`. So `/animals` will be invalidated. + * GET on `/animals?type=rabbits`: will be interpreted as GET `/animals/?type=rabbits`. + * Modifying `/animals/rabbits?id=1`: will be interpreted as a modification of `/animals/rabbits/?id=1`. So the old versions of both `/animals` and `/animals/rabbits` are invalidated too. + * GET on `/animals?type=dogs`: will be interpreted as GET `/animals/?type=dogs`. So a modification of `/animals?type=rabbits` will not affect it (but modification of `/animals` will invalidate it). + +The default child invalidation constraint is a negated regular expression: `\/[0-9]+$`. This means a POST to `/animals/rabbits` will by default not invalidate `/animals/rabbits/1` or any paths below it, but will invalidate `/animals/rabbits/findByName`. + +NOTE: The store and retrieve calls are not yet fully optimized and get pretty chatty when using network based caches. You can probably expect best performance from APCu. It won't use that much memory. ## Install And Configure @@ -30,15 +48,28 @@ Concurrency control is enabled by default. To disable: ```yml rest_e_tags: - concurrency_control: false + concurrency_control: false ``` The bundle will work with any Doctrine cache. Use the 'cache' config option to reference the service to be used: ```yml rest_e_tags: - cache: my.doctrine.cache + cache: my.doctrine.cache ``` +You can tweak the default child invalidation constraint (negated, see default above): + +```yml +rest_e_tags: + # Do not invalidate paths that look like they end in UUIDs (nor any paths below them) + child_invalidation_constraint: '\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$' +``` + +```yml +rest_e_tags: + # Always invalidate, skip regex match + child_invalidation_constraint: '' +``` ## License KleijnWeb\RestETagBundle is made available under the terms of the [LGPL, version 3.0](https://spdx.org/licenses/LGPL-3.0.html#licenseText). diff --git a/src/Cache/CacheAdapter.php b/src/Cache/CacheAdapter.php new file mode 100644 index 0000000..94b781e --- /dev/null +++ b/src/Cache/CacheAdapter.php @@ -0,0 +1,212 @@ + + */ +class CacheAdapter +{ + const KEY_VERSION = 1; + const KEY_CHILDREN = 2; + + /** + * @var Cache + */ + private $cache; + + /** + * @var string + */ + private $childInvalidationConstraint; + + /** + * @param Cache $cache + * @param string $childInvalidationConstraint + */ + public function __construct(Cache $cache, $childInvalidationConstraint = '') + { + $this->cache = $cache; + $this->childInvalidationConstraint = $childInvalidationConstraint; + } + + /** + * @param string $childInvalidationConstraint + * + * @return $this + */ + public function setChildInvalidationConstraint($childInvalidationConstraint) + { + $this->childInvalidationConstraint = $childInvalidationConstraint; + + return $this; + } + + /** + * @param Request $request + * + * @return string + */ + public function fetch(Request $request) + { + if (!$record = $this->cache->fetch($this->createKey($request))) { + return ''; + } + + return $record[self::KEY_VERSION]; + } + + /** + * @param Request $request + * + * @return bool + */ + public function contains(Request $request) + { + return $this->containsKey($this->createKey($request)); + } + + /** + * @param string $key + * + * @return bool + */ + public function containsKey($key) + { + return $this->cache->contains($key); + } + + /** + * @param Request $request + * @param string $version + * + * @return mixed + */ + public function update(Request $request, $version) + { + $segments = $this->getSegments($request); + $paths = []; + $path = ''; + foreach ($segments as $segment) { + $path .= "/$segment"; + $paths[] = $path; + } + + foreach ($paths as $i => $path) { + $record = $this->cache->fetch($path); + if ($record) { + $this->invalidateChildren($record[self::KEY_CHILDREN], $version); + } else { + $record = [self::KEY_CHILDREN => []]; + } + $record[self::KEY_VERSION] = $version; + if (isset($paths[$i + 1])) { + $record[self::KEY_CHILDREN][] = $paths[$i + 1]; + } + $this->cache->save($path, $record); + } + + return $version; + } + + /** + * @param Request $request + * @param string $version + * + * @return mixed + */ + public function register(Request $request, $version) + { + $segments = $this->getSegments($request); + $paths = []; + $path = ''; + foreach ($segments as $segment) { + $path .= "/$segment"; + $paths[] = $path; + } + + foreach ($paths as $i => $path) { + $record = $this->cache->fetch($path); + if (!$record) { + $record = [self::KEY_VERSION => $version, self::KEY_CHILDREN => []]; + } + if (isset($paths[$i + 1])) { + $record[self::KEY_CHILDREN][] = $paths[$i + 1]; + } + $this->cache->save($path, $record); + } + $record = [self::KEY_VERSION => $version, self::KEY_CHILDREN => []]; + $this->cache->save($this->createKeyFromSegments($segments), $record); + + return $version; + } + + private function invalidateChildren(array $children, $version) + { + foreach ($children as $child) { + if ($this->childInvalidationConstraint !== '' + && preg_match("/$this->childInvalidationConstraint/", $child) + ) { + // Stop recursive invalidation if it matches + return; + } + $record = $this->cache->fetch($child); + $record[self::KEY_VERSION] = $version; + $this->cache->save($child, $record); + if ($record) { + $this->invalidateChildren($record[self::KEY_CHILDREN], $version); + } + } + } + + /** + * @param Request $request + * + * @return array + */ + private function getSegments(Request $request) + { + $key = $request->getPathInfo(); + $segments = explode('/', ltrim($key, '/')); + if ($query = $request->getQueryString()) { + $segments[] = '?' . $query; + } + + array_walk($segments, function (&$value) { + $value = preg_replace('/[^[:print:]]/', '_', $value); + }); + + return array_filter($segments, function ($value) { + return $value !== ''; + }); + } + + /** + * @param Request $request + * + * @return string + */ + private function createKey(Request $request) + { + return $this->createKeyFromSegments($this->getSegments($request)); + } + + /** + * @param array $segments + * + * @return string + */ + private function createKeyFromSegments(array $segments) + { + return '/' . implode('/', $segments); + } +} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index b01e76b..d465442 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -27,7 +27,8 @@ public function getConfigTreeBuilder() $rootNode ->children() ->booleanNode('concurrency_control')->defaultFalse()->end() - ->scalarNode('cache')->defaultFalse()->end() + ->scalarNode('child_invalidation_constraint')->defaultValue('\/[0-9]+$')->end() + ->scalarNode('cache')->isRequired()->defaultFalse()->end() ; return $treeBuilder; diff --git a/src/DependencyInjection/KleijnWebRestETagExtension.php b/src/DependencyInjection/KleijnWebRestETagExtension.php index c60f5c1..a0d6942 100644 --- a/src/DependencyInjection/KleijnWebRestETagExtension.php +++ b/src/DependencyInjection/KleijnWebRestETagExtension.php @@ -27,6 +27,7 @@ public function load(array $configs, ContainerBuilder $container) $loader = new Loader\YamlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); $loader->load('services.yml'); $container->setParameter('rest_e_tags.concurrency_control', $config['concurrency_control']); + $container->setParameter('rest_e_tags.child_invalidation_constraint', $config['child_invalidation_constraint']); $container->setAlias('rest_e_tags.cache', $config['cache']); } diff --git a/src/EventListener/RequestListener.php b/src/EventListener/RequestListener.php index a71225f..0c9a31f 100644 --- a/src/EventListener/RequestListener.php +++ b/src/EventListener/RequestListener.php @@ -8,7 +8,7 @@ namespace KleijnWeb\RestETagBundle\EventListener; -use Doctrine\Common\Cache\Cache; +use KleijnWeb\RestETagBundle\Cache\CacheAdapter; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Event\GetResponseEvent; @@ -19,9 +19,9 @@ class RequestListener { /** - * @var Cache + * @var CacheAdapter */ - private $cache; + private $cacheAdapter; /** * @var bool @@ -29,12 +29,12 @@ class RequestListener private $concurrencyControl; /** - * @param Cache $cache - * @param bool $concurrencyControl + * @param CacheAdapter $cache + * @param bool $concurrencyControl */ - public function __construct(Cache $cache, $concurrencyControl = true) + public function __construct(CacheAdapter $cache, $concurrencyControl = true) { - $this->cache = $cache; + $this->cacheAdapter = $cache; $this->concurrencyControl = $concurrencyControl; } @@ -57,13 +57,37 @@ public function onKernelRequest(GetResponseEvent $event) * * @return bool */ - public static function isModifyingRequest(Request $request) + public static function isModifyingMethodRequest(Request $request) { $method = strtoupper($request->getMethod()); return in_array($method, ['POST', 'PUT', 'PATCH', 'DELETE']); } + /** + * @param Request $request + * + * @return bool + */ + public static function isIgnoreMethodRequest(Request $request) + { + $method = strtoupper($request->getMethod()); + + return in_array($method, ['OPTIONS', 'HEAD']); + } + + /** + * @param Request $request + * + * @return bool + */ + public static function isSupportedMethodRequest(Request $request) + { + $method = strtoupper($request->getMethod()); + + return in_array($method, ['GET', 'OPTIONS', 'HEAD', 'POST', 'PUT', 'PATCH', 'DELETE']); + } + /** * @param Request $request * @@ -71,7 +95,11 @@ public static function isModifyingRequest(Request $request) */ private function createResponse(Request $request) { - if (!$version = $this->cache->fetch($request->getPathInfo())) { + if (!self::isSupportedMethodRequest($request)) { + return new Response('', Response::HTTP_METHOD_NOT_ALLOWED); + } + + if (!$version = $this->cacheAdapter->fetch($request)) { return null; } $method = strtoupper($request->getMethod()); @@ -81,7 +109,7 @@ private function createResponse(Request $request) if ($ifNoneMatch && $version === $ifNoneMatch) { return new Response('', Response::HTTP_NOT_MODIFIED); } - } elseif ($this->concurrencyControl && self::isModifyingRequest($request)) { + } elseif ($this->concurrencyControl && self::isModifyingMethodRequest($request)) { $ifMatch = $request->headers->get('If-Match'); if (!$ifMatch) { return new Response('', Response::HTTP_PRECONDITION_REQUIRED); diff --git a/src/EventListener/ResponseListener.php b/src/EventListener/ResponseListener.php index 202aa9b..e07c6f3 100644 --- a/src/EventListener/ResponseListener.php +++ b/src/EventListener/ResponseListener.php @@ -8,7 +8,7 @@ namespace KleijnWeb\RestETagBundle\EventListener; -use Doctrine\Common\Cache\Cache; +use KleijnWeb\RestETagBundle\Cache\CacheAdapter; use Symfony\Component\HttpKernel\Event\FilterResponseEvent; /** @@ -17,16 +17,16 @@ class ResponseListener { /** - * @var Cache + * @var CacheAdapter */ - private $cache; + private $cacheAdapter; /** - * @param Cache $cache + * @param CacheAdapter $cache */ - public function __construct(Cache $cache) + public function __construct(CacheAdapter $cache) { - $this->cache = $cache; + $this->cacheAdapter = $cache; } /** @@ -38,19 +38,16 @@ public function onKernelResponse(FilterResponseEvent $event) return; } $request = $event->getRequest(); - $version = $this->cache->fetch($request->getPathInfo()); - $response = $event->getResponse(); - if (RequestListener::isModifyingRequest($request)) { - $version = microtime(true); - $path = $request->getPathInfo(); - $partialPath = ''; - foreach (explode('/', ltrim($path, '/')) as $segment) { - $partialPath .= "/$segment"; - $this->cache->save($partialPath, $version); + + if (RequestListener::isModifyingMethodRequest($request)) { + $version = $this->cacheAdapter->update($request, (string)microtime(true)); + } elseif (!RequestListener::isIgnoreMethodRequest($request)) { + if (!$version = $this->cacheAdapter->fetch($request)) { + $version = $this->cacheAdapter->register($request, (string)microtime(true)); } } - if ($version) { + if (isset($version)) { $response->headers->set('ETag', $version); } } diff --git a/src/Resources/config/services.yml b/src/Resources/config/services.yml index 1519498..50a07ae 100644 --- a/src/Resources/config/services.yml +++ b/src/Resources/config/services.yml @@ -1,12 +1,16 @@ services: - kernel.listener.rest_e_tags.request: + rest_e_tags.kernel.listener.request: class: KleijnWeb\RestETagBundle\EventListener\RequestListener - arguments: ['@rest_e_tags.cache', '%rest_e_tags.concurrency_control%'] + arguments: ['@rest_e_tags.cache_adapter', '%rest_e_tags.concurrency_control%'] tags: - { name: kernel.event_listener, event: kernel.request, method: onKernelRequest } - kernel.listener.rest_e_tags.response: + rest_e_tags.kernel.listener.response: class: KleijnWeb\RestETagBundle\EventListener\ResponseListener - arguments: ['@rest_e_tags.cache' ] + arguments: ['@rest_e_tags.cache_adapter' ] tags: - { name: kernel.event_listener, event: kernel.response, method: onKernelResponse } + + rest_e_tags.cache_adapter: + class: KleijnWeb\RestETagBundle\Cache\CacheAdapter + arguments: ['@rest_e_tags.cache', '%child_invalidation_constraint'] diff --git a/src/Tests/Cache/CacheAdapterTest.php b/src/Tests/Cache/CacheAdapterTest.php new file mode 100644 index 0000000..694fb4c --- /dev/null +++ b/src/Tests/Cache/CacheAdapterTest.php @@ -0,0 +1,271 @@ + + */ +class CacheAdapterTest extends \PHPUnit_Framework_TestCase +{ + /** + * @var CacheAdapter + */ + private $adapter; + + /** + * Create mocks + */ + protected function setUp() + { + $this->adapter = new CacheAdapter(new ArrayCache()); + } + + /** + * @test + */ + public function canSaveAndFetchUsingPath() + { + $uri = "/a/b/cee/dee/eff"; + $version = microtime(true); + $this->adapter->update(Request::create($uri), $version); + $this->assertSame($version, $this->adapter->fetch(Request::create($uri))); + } + + /** + * @test + */ + public function canSaveAndFetchUsingPathAndQuery() + { + $uri = "/a/b/cee/?dee=eff"; + $version = microtime(true); + $this->adapter->update(Request::create($uri), $version); + $this->assertSame($version, $this->adapter->fetch(Request::create($uri))); + } + + /** + * @test + */ + public function willInsertSlashBeforeQuery() + { + $uri = "/a/b/cee?dee=eff"; + $version = microtime(true); + $this->adapter->update(Request::create($uri), $version); + $this->assertSame($version, $this->adapter->fetch(Request::create("/a/b/cee/?dee=eff"))); + } + + /** + * @test + */ + public function willUrlEncodeQuery() + { + $uri = "/a/b/cee/?dee=eff/"; + $version = microtime(true); + $this->adapter->update(Request::create($uri), $version); + $this->assertSame($version, $this->adapter->fetch(Request::create("/a/b/cee/?dee=eff%2F"))); + } + + /** + * @test + */ + public function willFilterEmptySegments() + { + $version1 = microtime(true); + $this->adapter->update(Request::create("/a/b///cee/"), $version1); + $this->assertSame($version1, $this->adapter->fetch(Request::create('/a/b/cee'))); + $version2 = microtime(true); + $this->adapter->update(Request::create("/a/b/cee//?dee=eff"), $version2); + $this->assertSame($version2, $this->adapter->fetch(Request::create('/a/b/cee/?dee=eff'))); + } + + /** + * @test + */ + public function willFilterNonPrintable() + { + $uri = "/a/b/" . chr(6) . "cee/?" . chr(6) . "dee=eff"; + $version = microtime(true); + $this->adapter->update(Request::create($uri), $version); + // The underscores are introduced by parse_url() + $this->assertSame($version, $this->adapter->fetch(Request::create("/a/b/_cee/?_dee=eff"))); + } + + /** + * @test + */ + public function willFilterNonAscii() + { + $uri = "/aنقاط/你好b,世界"; + $version = microtime(true); + $this->adapter->update(Request::create($uri), $version); + // The underscores are introduced by preg_replace() + $this->assertSame($version, $this->adapter->fetch(Request::create("/a________/______b_________"))); + } + + /** + * @test + */ + public function willSaveSegmentsIndividually() + { + $uri = "/a/b/cee/?dee=eff"; + $version = microtime(true); + $this->adapter->update(Request::create($uri), $version); + $this->assertSame($version, $this->adapter->fetch(Request::create("/a"))); + $this->assertSame($version, $this->adapter->fetch(Request::create("/a/b"))); + $this->assertSame($version, $this->adapter->fetch(Request::create("/a/b/cee"))); + $this->assertSame($version, $this->adapter->fetch(Request::create("/a/b/cee/?dee=eff"))); + + $this->assertSame('', $this->adapter->fetch(Request::create("/something/else"))); + } + + /** + * @test + */ + public function canRegisterPathVersion() + { + $uri = "/a/b/cee/?dee=eff"; + $version = microtime(true); + $this->adapter->register(Request::create($uri), $version); + $this->assertSame($version, $this->adapter->fetch(Request::create($uri))); + } + + /** + * @test + */ + public function canConfirmContainsVersionForRequest() + { + $uri = "/a/b/cee/?dee=eff"; + $this->adapter->register(Request::create($uri), microtime(true)); + $this->assertTrue($this->adapter->contains(Request::create($uri))); + } + + /** + * @test + */ + public function updatingRootInvalidatesChildren() + { + $childUri = "/a/b/cee/?dee=eff"; + $childVersion = microtime(true); + $this->adapter->update(Request::create($childUri), $childVersion); + + $parentUri = "/a"; + $parentVersion = microtime(true); + $this->adapter->update(Request::create($parentUri), $parentVersion); + + $this->assertSame($parentVersion, $this->adapter->fetch(Request::create("/a"))); + $this->assertSame($parentVersion, $this->adapter->fetch(Request::create("/a/b"))); + $this->assertSame($parentVersion, $this->adapter->fetch(Request::create("/a/b/cee"))); + $this->assertSame($parentVersion, $this->adapter->fetch(Request::create("/a/b/cee/?dee=eff"))); + } + + /** + * @test + */ + public function updatingParentInvalidatesChildrenAndParents() + { + $childUri = "/a/b/cee/dee/?eff=gee"; + $originalVersion = microtime(true); + $this->adapter->update(Request::create($childUri), $originalVersion); + + $parentUri = "/a/b"; + $newVersion = microtime(true); + $this->adapter->update(Request::create($parentUri), $newVersion); + + $this->assertSame($newVersion, $this->adapter->fetch(Request::create("/a"))); + $this->assertSame($newVersion, $this->adapter->fetch(Request::create("/a/b"))); + $this->assertSame($newVersion, $this->adapter->fetch(Request::create("/a/b/cee"))); + $this->assertSame($newVersion, $this->adapter->fetch(Request::create("/a/b/cee/dee"))); + $this->assertSame($newVersion, $this->adapter->fetch(Request::create("/a/b/cee/dee/?eff=gee"))); + } + + /** + * @test + */ + public function registeredChildrenAreInvalidated() + { + $uri = "/a/b/cee/?dee=eff"; + $this->adapter->register(Request::create($uri), microtime(true)); + $this->assertTrue($this->adapter->contains(Request::create($uri))); + + $parentUri = "/a/b"; + $newVersion = microtime(true); + $this->adapter->update(Request::create($parentUri), $newVersion); + + $this->assertSame($newVersion, $this->adapter->fetch(Request::create("/a"))); + $this->assertSame($newVersion, $this->adapter->fetch(Request::create("/a/b"))); + $this->assertSame($newVersion, $this->adapter->fetch(Request::create("/a/b/cee"))); + $this->assertSame($newVersion, $this->adapter->fetch(Request::create($uri))); + } + + /** + * @test + */ + public function registerWillNotUpdateVersionOfExistingParent() + { + $parentUri = "/a/b"; + $parentVersion = microtime(true); + $this->adapter->update(Request::create($parentUri), $parentVersion); + + $childUri = "/a/b/cee/dee/?eff=gee"; + $childVersion = microtime(true); + $this->adapter->register(Request::create($childUri), $childVersion); + + $this->assertSame($parentVersion, $this->adapter->fetch(Request::create("/a"))); + $this->assertSame($parentVersion, $this->adapter->fetch(Request::create("/a/b"))); + $this->assertSame($childVersion, $this->adapter->fetch(Request::create("/a/b/cee"))); + $this->assertSame($childVersion, $this->adapter->fetch(Request::create("/a/b/cee/dee"))); + $this->assertSame($childVersion, $this->adapter->fetch(Request::create("/a/b/cee/dee/?eff=gee"))); + } + + /** + * @test + */ + public function updateWillUpdateVersionOfExistingParent() + { + $parentUri = "/a/b"; + $parentVersion = microtime(true); + $this->adapter->update(Request::create($parentUri), $parentVersion); + + $childUri = "/a/b/cee/dee/?eff=gee"; + $childVersion = microtime(true); + $this->adapter->update(Request::create($childUri), $childVersion); + + $this->assertSame($childVersion, $this->adapter->fetch(Request::create("/a"))); + $this->assertSame($childVersion, $this->adapter->fetch(Request::create("/a/b"))); + $this->assertSame($childVersion, $this->adapter->fetch(Request::create("/a/b/cee"))); + $this->assertSame($childVersion, $this->adapter->fetch(Request::create("/a/b/cee/dee"))); + $this->assertSame($childVersion, $this->adapter->fetch(Request::create("/a/b/cee/dee/?eff=gee"))); + } + + /** + * @test + */ + public function savingParentInvalidatesParentsAndOnlyChildrenNotMatchingConstraint() + { + $childUri = "/a/b/cee/dee/?eff=gee"; + $originalVersion = microtime(true); + $this->adapter->setChildInvalidationConstraint('\/dee$'); + $this->adapter->update(Request::create($childUri), $originalVersion); + + $parentUri = "/a/b"; + $newVersion = microtime(true); + $this->adapter->update(Request::create($parentUri), $newVersion); + + $this->assertSame($newVersion, $this->adapter->fetch(Request::create("/a"))); + $this->assertSame($newVersion, $this->adapter->fetch(Request::create("/a/b"))); + $this->assertSame($newVersion, $this->adapter->fetch(Request::create("/a/b/cee"))); + $this->assertSame($originalVersion, $this->adapter->fetch(Request::create("/a/b/cee/dee"))); + $this->assertSame($originalVersion, $this->adapter->fetch(Request::create("/a/b/cee/dee/?eff=gee"))); + } +} diff --git a/src/Tests/EventListener/RequestListenerTest.php b/src/Tests/EventListener/RequestListenerTest.php index 4063800..d34aa54 100644 --- a/src/Tests/EventListener/RequestListenerTest.php +++ b/src/Tests/EventListener/RequestListenerTest.php @@ -6,9 +6,10 @@ * file that was distributed with this source code. */ -namespace KleijnWeb\RestETagBundle\Tests\Dev\EventListener; +namespace KleijnWeb\RestETagBundle\Tests\EventListener; use Doctrine\Common\Cache\ArrayCache; +use KleijnWeb\RestETagBundle\Cache\CacheAdapter; use KleijnWeb\RestETagBundle\EventListener\RequestListener; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -31,9 +32,9 @@ class RequestListenerTest extends \PHPUnit_Framework_TestCase private $eventMock; /** - * @var ArrayCache + * @var CacheAdapter */ - private $cache; + private $cacheAdapter; /** * Create mocks @@ -50,18 +51,33 @@ protected function setUp() ->method('isMasterRequest') ->willReturn(true); - $this->cache = new ArrayCache(); - $this->listener = new RequestListener($this->cache, true); + $this->cacheAdapter = new CacheAdapter(new ArrayCache()); + $this->listener = new RequestListener($this->cacheAdapter, true); } /** * @test */ - public function willCreateNotModifiedResponseCacheHasMatch() + public function limitAllowedMethods() { - $version = microtime(true); + $this->eventMock + ->expects($this->once()) + ->method('setResponse') + ->with($this->callback(function (Response $response) { + return $response->getStatusCode() === Response::HTTP_METHOD_NOT_ALLOWED; + })); - $this->cache->save(self::URI, $version); + $this->invokeListener('FAUX'); + $this->assertEmpty($this->cacheAdapter->fetch(self::createRequest())); + } + + /** + * @test + */ + public function willCreateNotModifiedResponseCacheHasMatch() + { + $version = (string)microtime(true); + $this->cacheAdapter->update(self::createRequest(), $version); $this->eventMock ->expects($this->once()) @@ -70,6 +86,7 @@ public function willCreateNotModifiedResponseCacheHasMatch() return $response->getStatusCode() === Response::HTTP_NOT_MODIFIED; })); + $this->invokeListener('GET', ['HTTP_IF_NONE_MATCH' => $version]); } @@ -78,7 +95,7 @@ public function willCreateNotModifiedResponseCacheHasMatch() */ public function willNotCreateResponseWhenCacheIsEmpty() { - $version = microtime(true); + $version = (string)microtime(true); $this->eventMock ->expects($this->never()) @@ -95,13 +112,12 @@ public function willNotCreateResponseWhenVersionDoesNotMatch() $version1 = microtime(true); $version2 = $version1 + 1; - $this->cache->save(self::URI, $version1); - + $this->cacheAdapter->update(self::createRequest(), (string)$version1); $this->eventMock ->expects($this->never()) ->method('setResponse'); - $this->invokeListener('GET', ['HTTP_IF_NONE_MATCH' => $version2]); + $this->invokeListener('GET', ['HTTP_IF_NONE_MATCH' => (string)$version2]); } /** @@ -109,9 +125,9 @@ public function willNotCreateResponseWhenVersionDoesNotMatch() */ public function willCreatePreconditionRequiredWhenHeaderIsMissing() { - $version1 = microtime(true); + $version1 = (string)microtime(true); - $this->cache->save(self::URI, $version1); + $this->cacheAdapter->update(self::createRequest(), $version1); $this->eventMock ->expects($this->once()) @@ -131,7 +147,7 @@ public function willCreatePreconditionFailedWhenVersionMismatch() $version1 = microtime(true); $version2 = $version1 + 1; - $this->cache->save(self::URI, $version1); + $this->cacheAdapter->update(self::createRequest(), (string)$version1); $this->eventMock ->expects($this->once()) @@ -140,16 +156,16 @@ public function willCreatePreconditionFailedWhenVersionMismatch() return $response->getStatusCode() === Response::HTTP_PRECONDITION_FAILED; })); - $this->invokeListener('POST', ['HTTP_IF_MATCH' => $version2]); + $this->invokeListener('POST', ['HTTP_IF_MATCH' => (string)$version2]); } /** * @param string $method - * @param array $server + * @param array $server */ private function invokeListener($method, array $server = []) { - $request = Request::create(self::URI, $method, [], [], [], $server); + $request = self::createRequest($method, $server); $this->eventMock ->expects($this->once()) @@ -158,4 +174,15 @@ private function invokeListener($method, array $server = []) $this->listener->onKernelRequest($this->eventMock); } + + /** + * @param string $method + * @param array $server + * + * @return Request + */ + private static function createRequest($method = "GET", array $server = []) + { + return Request::create(self::URI, $method, [], [], [], $server); + } } diff --git a/src/Tests/EventListener/ResponseListenerTest.php b/src/Tests/EventListener/ResponseListenerTest.php index d4b29cf..d805988 100644 --- a/src/Tests/EventListener/ResponseListenerTest.php +++ b/src/Tests/EventListener/ResponseListenerTest.php @@ -6,9 +6,10 @@ * file that was distributed with this source code. */ -namespace KleijnWeb\RestETagBundle\Tests\Dev\EventListener; +namespace KleijnWeb\RestETagBundle\Tests\EventListener; use Doctrine\Common\Cache\ArrayCache; +use KleijnWeb\RestETagBundle\Cache\CacheAdapter; use KleijnWeb\RestETagBundle\EventListener\ResponseListener; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -31,9 +32,9 @@ class ResponseListenerTest extends \PHPUnit_Framework_TestCase private $eventMock; /** - * @var ArrayCache + * @var CacheAdapter */ - private $cache; + private $cacheAdapter; /** * @var Response @@ -50,26 +51,31 @@ protected function setUp() ->disableOriginalConstructor() ->getMock(); $this->eventMock - ->expects($this->once()) + ->expects($this->any()) ->method('isMasterRequest') ->willReturn(true); $this->eventMock - ->expects($this->once()) + ->expects($this->any()) ->method('getResponse') ->willReturn($this->response = new Response()); - $this->cache = new ArrayCache(); - $this->listener = new ResponseListener($this->cache); + $this->cacheAdapter = new CacheAdapter(new ArrayCache()); + $this->listener = new ResponseListener($this->cacheAdapter); } /** * @test */ - public function willIgnoreGetRequest() + public function getRequestDoesNotModifyVersion() { $this->invokeListener('GET'); - $this->assertFalse($this->cache->fetch(self::URI)); + $originalVersion = $this->cacheAdapter->fetch(self::createRequest()); + + for ($i = 0; $i < 10; ++$i) { + $this->invokeListener('GET'); + $this->assertSame($originalVersion, $this->cacheAdapter->fetch(self::createRequest())); + } } /** @@ -78,7 +84,16 @@ public function willIgnoreGetRequest() public function willIgnoreHeadRequest() { $this->invokeListener('HEAD'); - $this->assertFalse($this->cache->fetch(self::URI)); + $this->assertEmpty($this->cacheAdapter->fetch(self::createRequest())); + } + + /** + * @test + */ + public function willIgnoreOptionsRequest() + { + $this->invokeListener('HEAD'); + $this->assertEmpty($this->cacheAdapter->fetch(self::createRequest())); } /** @@ -96,7 +111,7 @@ public function willSetETagOnModifiedRequest() public function willSaveVersionOnModifiedRequest() { $this->invokeListener('PUT'); - $this->assertRegExp('/\d{10}\.\d+/', (string)$this->cache->fetch(self::URI)); + $this->assertRegExp('/\d{10}\.\d+/', $this->cacheAdapter->fetch(self::createRequest())); } /** @@ -105,23 +120,39 @@ public function willSaveVersionOnModifiedRequest() public function willInvalidateAllParentPaths() { $this->invokeListener('PUT'); - $this->assertTrue($this->cache->contains('/foo')); - $this->assertTrue($this->cache->contains('/foo/bar')); - $this->assertTrue($this->cache->contains('/foo/bar/bah')); + $this->assertTrue($this->cacheAdapter->containsKey('/foo')); + $this->assertTrue($this->cacheAdapter->containsKey('/foo/bar')); + $this->assertTrue($this->cacheAdapter->containsKey('/foo/bar/bah')); } /** * @param string $method + * + * @return Request */ private function invokeListener($method) { $request = Request::create(self::URI, $method); $this->eventMock - ->expects($this->once()) + ->expects($this->any()) ->method('getRequest') ->willReturn($request); $this->listener->onKernelResponse($this->eventMock); + + return $request; + } + + + /** + * @param string $method + * @param array $server + * + * @return Request + */ + private static function createRequest($method = "GET", array $server = []) + { + return Request::create(self::URI, $method, [], [], [], $server); } } diff --git a/src/Tests/Functional/Foo/app/routing.yml b/src/Tests/Functional/Foo/app/routing.yml index 739dfcd..83a1e05 100644 --- a/src/Tests/Functional/Foo/app/routing.yml +++ b/src/Tests/Functional/Foo/app/routing.yml @@ -1,3 +1,7 @@ foobar: path: /foo/bar + defaults: { _controller: FooBundle:Foo:foobar } + +foobardoh: + path: /foo/bar/doh defaults: { _controller: FooBundle:Foo:foobar } \ No newline at end of file diff --git a/src/Tests/Functional/FunctionalTest.php b/src/Tests/Functional/FunctionalTest.php index 1eddd0a..a6c5e80 100644 --- a/src/Tests/Functional/FunctionalTest.php +++ b/src/Tests/Functional/FunctionalTest.php @@ -41,6 +41,53 @@ public function willReturnNotModifiedResponseWhenUsingETag() $this->assertSame(Response::HTTP_NOT_MODIFIED, $response->getStatusCode()); } + /** + * @test + */ + public function willInvalidateWhenPostingToParent() + { + $client = self::createClient(); + $client->disableReboot(); + $childUrl = '/foo/bar/doh'; + $client->request('GET', $childUrl); + $response = $client->getResponse(); + $originalEtag = $response->getEtag(); + $this->assertNotEmpty($originalEtag); + + // Sanity check + $client->request('GET', $childUrl, [], [], ['HTTP_IF_NONE_MATCH' => $originalEtag]); + $response = $client->getResponse(); + $this->assertSame(Response::HTTP_NOT_MODIFIED, $response->getStatusCode()); + + // Validate that when we post to what should be the parent, the resource is marked as modified + $client->request('POST', '/foo/bar'); + $client->request('GET', $childUrl, [], [], ['HTTP_IF_NONE_MATCH' => $originalEtag]); + $this->assertNotSame(Response::HTTP_NOT_MODIFIED, $response->getStatusCode()); + } + + /** + * @test + */ + public function willTreatQueryAsASegment() + { + $client = self::createClient(); + $client->disableReboot(); + $client->request('GET', '/foo/bar?doh=1'); + $response = $client->getResponse(); + $originalEtag = $response->getEtag(); + $this->assertNotEmpty($originalEtag); + + // Sanity check + $client->request('GET', '/foo/bar?doh=1', [], [], ['HTTP_IF_NONE_MATCH' => $originalEtag]); + $response = $client->getResponse(); + $this->assertSame(Response::HTTP_NOT_MODIFIED, $response->getStatusCode()); + + // Validate that when we post to what should be the parent, the resource is marked as modified + $client->request('POST', '/foo/bar'); + $client->request('GET', '/foo/bar?doh=1', [], [], ['HTTP_IF_NONE_MATCH' => $originalEtag]); + $this->assertNotSame(Response::HTTP_NOT_MODIFIED, $response->getStatusCode()); + } + /** * @test */