From 1b12389cbc3e8e9f57189d5f57904c4b4cd0da72 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 10 Dec 2015 10:46:46 +0100 Subject: [PATCH 1/7] Added a router listener --- src/CacheBundle.php | 1 - src/DependencyInjection/CacheExtension.php | 10 +- .../Compiler/RouterCompilerPass.php | 45 ------- src/DependencyInjection/Configuration.php | 5 +- src/Routing/Matcher/CacheUrlMatcher.php | 78 ------------ src/Routing/Router.php | 109 ----------------- src/Routing/RouterListener.php | 112 ++++++++++++++++++ 7 files changed, 122 insertions(+), 238 deletions(-) delete mode 100755 src/DependencyInjection/Compiler/RouterCompilerPass.php delete mode 100755 src/Routing/Matcher/CacheUrlMatcher.php delete mode 100755 src/Routing/Router.php create mode 100644 src/Routing/RouterListener.php diff --git a/src/CacheBundle.php b/src/CacheBundle.php index a8d2fc5..2302167 100755 --- a/src/CacheBundle.php +++ b/src/CacheBundle.php @@ -31,7 +31,6 @@ public function build(ContainerBuilder $container) $container->addCompilerPass(new Compiler\SessionSupportCompilerPass()); $container->addCompilerPass(new Compiler\DoctrineSupportCompilerPass()); - $container->addCompilerPass(new Compiler\RouterCompilerPass()); if ($container->getParameter('kernel.debug')) { $container->addCompilerPass(new Compiler\DataCollectorCompilerPass()); diff --git a/src/DependencyInjection/CacheExtension.php b/src/DependencyInjection/CacheExtension.php index 31907ed..2ccbfba 100755 --- a/src/DependencyInjection/CacheExtension.php +++ b/src/DependencyInjection/CacheExtension.php @@ -11,8 +11,8 @@ namespace Cache\CacheBundle\DependencyInjection; -use Cache\CacheBundle\Cache\LoggingCachePool; use Cache\CacheBundle\DataCollector\CacheDataCollector; +use Cache\CacheBundle\Routing\RouterListener; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\HttpKernel\DependencyInjection\Extension; @@ -45,6 +45,14 @@ public function load(array $configs, ContainerBuilder $container) } } + if ($config['router']['enabled']) { + $container->register('cache.router_listener', RouterListener::class) + ->addArgument(new Reference($config['router']['service_id'])) + ->addArgument($config['router']['ttl']) + ->addTag('kernel.event_listener', ['event'=>'kernel.request', 'method'=>'onBeforeRouting', 'priority'=>33]) + ->addTag('kernel.event_listener', ['event'=>'kernel.request', 'method'=>'onAfterRouting', 'priority'=>31]); + } + } public function getAlias() diff --git a/src/DependencyInjection/Compiler/RouterCompilerPass.php b/src/DependencyInjection/Compiler/RouterCompilerPass.php deleted file mode 100755 index 984352e..0000000 --- a/src/DependencyInjection/Compiler/RouterCompilerPass.php +++ /dev/null @@ -1,45 +0,0 @@ - - * - * This source file is subject to the MIT license that is bundled - * with this source code in the file LICENSE. - */ - -namespace Cache\CacheBundle\DependencyInjection\Compiler; - -use Symfony\Component\DependencyInjection\Reference; - -/** - * @author Aaron Scherer - * @author Tobias Nyholm - */ -class RouterCompilerPass extends BaseCompilerPass -{ - /** - * @return void - */ - protected function prepare() - { - if (!$this->container->hasParameter('cache.router')) { - return; - } - - $router = $this->container->getParameter('cache.router'); - - $def = clone $this->container->findDefinition('router'); - $def->setClass('Cache\CacheBundle\Routing\Router'); - $def->addMethodCall('setCache', [new Reference($router['service_id'])]); - $def->addMethodCall('setTtl', [$router['ttl']]); - - $this->container->setDefinition('cache.router', $def); - $this->container->setAlias('router.alias', 'cache.router'); - - if ($router['auto-register']) { - $this->container->setAlias('router', 'cache.router'); - } - } -} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index d09d844..1be0c2f 100755 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -142,10 +142,7 @@ private function addRouterSection() ->defaultFalse() ->end() ->integerNode('ttl') - ->defaultValue(86400) - ->end() - ->booleanNode('auto-register') - ->defaultTrue() + ->defaultValue(604800) ->end() ->scalarNode('service_id') ->isRequired() diff --git a/src/Routing/Matcher/CacheUrlMatcher.php b/src/Routing/Matcher/CacheUrlMatcher.php deleted file mode 100755 index bbf3487..0000000 --- a/src/Routing/Matcher/CacheUrlMatcher.php +++ /dev/null @@ -1,78 +0,0 @@ - - * - * This source file is subject to the MIT license that is bundled - * with this source code in the file LICENSE. - */ - -namespace Cache\CacheBundle\Routing\Matcher; - -use Psr\Cache\CacheItemPoolInterface; -use Symfony\Component\Routing\Matcher\UrlMatcher; -use Symfony\Component\Routing\RequestContext; -use Symfony\Component\Routing\RouteCollection; - -/** - * Class CacheUrlMatcher - * - * @author Aaron Scherer - */ -class CacheUrlMatcher extends UrlMatcher -{ - /** - * @var CacheItemPoolInterface - */ - protected $cachePool; - - /** - * @type int - */ - protected $ttl; - - /** - * CacheUrlMatcher constructor. - * - * @param CacheItemPoolInterface $cachePool - * @param int $ttl - * @param RouteCollection $routes - * @param RequestContext $context - */ - public function __construct( - CacheItemPoolInterface $cachePool, - $ttl, - RouteCollection $routes, - RequestContext $context - ) { - $this->cachePool = $cachePool; - $this->ttl = $ttl; - parent::__construct($routes, $context); - } - - /** - * @param string $pathInfo - * - * @return array - */ - public function match($pathInfo) - { - $host = strtr($this->context->getHost(), '.', '_'); - $method = strtolower($this->context->getMethod()); - $key = 'route_'.$method.'_'.$host.'_'.$pathInfo; - - $cacheItem = $this->cachePool->getItem($key); - if ($cacheItem->isHit()) { - return $cacheItem->get(); - } - - $match = parent::match($pathInfo); - $cacheItem->set($match) - ->expiresAfter($this->ttl); - $this->cachePool->save($cacheItem); - - return $match; - } -} diff --git a/src/Routing/Router.php b/src/Routing/Router.php deleted file mode 100755 index 2637af8..0000000 --- a/src/Routing/Router.php +++ /dev/null @@ -1,109 +0,0 @@ - - * - * This source file is subject to the MIT license that is bundled - * with this source code in the file LICENSE. - */ - -namespace Cache\CacheBundle\Routing; - -use Cache\CacheBundle\Routing\Matcher\CacheUrlMatcher; -use Psr\Cache\CacheItemPoolInterface; -use Symfony\Bundle\FrameworkBundle\Routing\Router as BaseRouter; -use Symfony\Component\DependencyInjection\ContainerInterface; - -/** - * Class Router - * - * @author Aaron Scherer - */ -class Router extends BaseRouter -{ - /** - * @var ContainerInterface - */ - protected $container; - - /** - * @var CacheItemPoolInterface - */ - protected $cache; - - /** - * @type int - */ - protected $ttl; - - /** - * @return CacheUrlMatcher|null|\Symfony\Component\Routing\Matcher\UrlMatcherInterface - */ - public function getMatcher() - { - if (null !== $this->matcher) { - return $this->matcher; - } - - $matcher = new CacheUrlMatcher($this->cache, $this->ttl, $this->getRouteCollection(), $this->context); - - return $this->matcher = $matcher; - } - - /** - * {@inheritdoc} - */ - public function getRouteCollection() - { - $key = 'route_collection'; - - if (null === $this->collection) { - $cacheItem = $this->cache->getItem($key); - if ($cacheItem->isHit()) { - $this->collection = $cacheItem->get(); - } else { - $this->collection = parent::getRouteCollection(); - $cacheItem->set($this->collection); - $cacheItem->expiresAfter($this->getTtl()); - - $this->cache->save($cacheItem); - } - } - - return $this->collection; - } - - /** - * @param CacheItemPoolInterface $cache - * - * @return Router - */ - public function setCache(CacheItemPoolInterface $cache) - { - $this->cache = $cache; - - return $this; - } - - /** - * @return int - */ - protected function getTtl() - { - return $this->ttl; - } - - /** - * @param int $ttl - * - * @return Router - */ - public function setTtl($ttl) - { - $this->ttl = $ttl; - - return $this; - } -} diff --git a/src/Routing/RouterListener.php b/src/Routing/RouterListener.php new file mode 100644 index 0000000..bce655d --- /dev/null +++ b/src/Routing/RouterListener.php @@ -0,0 +1,112 @@ + + */ +class RouterListener +{ + /** + * @var CacheItemPoolInterface + */ + private $cache; + + /** + * @var int + */ + private $ttl; + + /** + * @param CacheItemPoolInterface $cache + * @param int $ttl + */ + public function __construct(CacheItemPoolInterface $cache, $ttl) + { + $this->cache = $cache; + $this->ttl = $ttl; + } + + /** + * Before routing, try to fetch route result from cache. + * + * @param GetResponseEvent $event + */ + public function onBeforeRouting(GetResponseEvent $event) + { + $request = $event->getRequest(); + + if ($request->attributes->has('_controller')) { + // routing is already done + return; + } + + $item = $this->getCacheItem($request); + if (!$item->isHit()) { + return; + } + + $request->attributes->add($item->get()); + } + + /** + * After the routing, make sure to store the result in cache. + * + * @param GetResponseEvent $event + */ + public function onAfterRouting(GetResponseEvent $event) + { + $request = $event->getRequest(); + + if (!$request->attributes->has('_controller')) { + // routing has not taken place + return; + } + + $item = $this->getCacheItem($request); + if ($item->isHit()) { + return; + } + + $item->set($request->attributes->all()); + $this->cache->save($item); + } + + /** + * Get a good key that varies on method, host, path info etc etc. + * + * @param Request $request + * + * @return string + */ + private function createCacheKey(Request $request) + { + $key = 'route_'.$request->getMethod().'_'.$request->getHost().'_'.$request->getPathInfo(); + + return $key; + } + + /** + * @param Request $request + * + * @return \Psr\Cache\CacheItemInterface + */ + private function getCacheItem(Request $request) + { + $key = $this->createCacheKey($request); + if ($this->cache instanceof TaggablePoolInterface) { + $item = $this->cache->getItem($key, ['routing']); + } else { + $item = $this->cache->getItem($key); + } + + return $item; + } +} From fb54b414fa53ab7527a94eca38c4aa5c7ba8afe3 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 10 Dec 2015 11:19:05 +0100 Subject: [PATCH 2/7] added quuery params to cache key --- src/Routing/RouterListener.php | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/Routing/RouterListener.php b/src/Routing/RouterListener.php index bce655d..34fe6d1 100644 --- a/src/Routing/RouterListener.php +++ b/src/Routing/RouterListener.php @@ -88,11 +88,34 @@ public function onAfterRouting(GetResponseEvent $event) */ private function createCacheKey(Request $request) { - $key = 'route_'.$request->getMethod().'_'.$request->getHost().'_'.$request->getPathInfo(); + $key = sprintf('route:%s:%s:%s',$request->getMethod(),$request->getHost(),$request->getPathInfo()); + + // This might be optional + $key.=':'.$this->implodeRecursive('|', $request->query->all()); return $key; } + /** + * @param $separator + * @param array $array + * + * @return string + */ + private function implodeRecursive($separator, array $array) + { + $output = ''; + foreach ($array as $key=>$value) { + if (is_array($value)) { + $output.=sprintf('%s%s[%s]', $separator, $key, $this->implodeRecursive($separator, $value)); + } else { + $output.=$separator.$value; + } + } + + return ltrim($output, $separator); + } + /** * @param Request $request * From 218e080cc6ecb03b901c661ace4911e4b82f37cb Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 10 Dec 2015 11:42:01 +0100 Subject: [PATCH 3/7] we dont need to use query string --- src/Routing/RouterListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Routing/RouterListener.php b/src/Routing/RouterListener.php index 34fe6d1..2c9d515 100644 --- a/src/Routing/RouterListener.php +++ b/src/Routing/RouterListener.php @@ -91,7 +91,7 @@ private function createCacheKey(Request $request) $key = sprintf('route:%s:%s:%s',$request->getMethod(),$request->getHost(),$request->getPathInfo()); // This might be optional - $key.=':'.$this->implodeRecursive('|', $request->query->all()); + //$key.=':'.$this->implodeRecursive('|', $request->query->all()); return $key; } From 2b25a7c8f10a067cea383529d08c8e0ac80cb93c Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 10 Dec 2015 11:48:06 +0100 Subject: [PATCH 4/7] Reduce one cache lookup --- src/Routing/RouterListener.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Routing/RouterListener.php b/src/Routing/RouterListener.php index 2c9d515..ac8b262 100644 --- a/src/Routing/RouterListener.php +++ b/src/Routing/RouterListener.php @@ -54,6 +54,7 @@ public function onBeforeRouting(GetResponseEvent $event) } $request->attributes->add($item->get()); + $request->attributes->set('_cache_hit', true); } /** @@ -70,11 +71,14 @@ public function onAfterRouting(GetResponseEvent $event) return; } - $item = $this->getCacheItem($request); - if ($item->isHit()) { + if ($request->attributes->has('_cache_hit')) { + $request->attributes->remove('_cache_hit'); + // object is in cache all ready return; } + // Save to the cache + $item = $this->getCacheItem($request); $item->set($request->attributes->all()); $this->cache->save($item); } From 15e22acdc98b44dc87b6cc08cabadb9133556dec Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 10 Dec 2015 11:48:31 +0100 Subject: [PATCH 5/7] typo --- src/Routing/RouterListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Routing/RouterListener.php b/src/Routing/RouterListener.php index ac8b262..5c54ee2 100644 --- a/src/Routing/RouterListener.php +++ b/src/Routing/RouterListener.php @@ -73,7 +73,7 @@ public function onAfterRouting(GetResponseEvent $event) if ($request->attributes->has('_cache_hit')) { $request->attributes->remove('_cache_hit'); - // object is in cache all ready + // object is in cache already return; } From e078954711fc863bc82c1a44fc8798a6589b3fd9 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 10 Dec 2015 15:35:46 +0100 Subject: [PATCH 6/7] Added services.yml --- src/DependencyInjection/CacheExtension.php | 24 +++++++++++----------- src/Resources/config/services.yml | 12 +++++++++++ 2 files changed, 24 insertions(+), 12 deletions(-) create mode 100644 src/Resources/config/services.yml diff --git a/src/DependencyInjection/CacheExtension.php b/src/DependencyInjection/CacheExtension.php index 2ccbfba..36e6bde 100755 --- a/src/DependencyInjection/CacheExtension.php +++ b/src/DependencyInjection/CacheExtension.php @@ -11,16 +11,17 @@ namespace Cache\CacheBundle\DependencyInjection; -use Cache\CacheBundle\DataCollector\CacheDataCollector; -use Cache\CacheBundle\Routing\RouterListener; +use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\HttpKernel\DependencyInjection\Extension; +use Symfony\Component\DependencyInjection\Loader; + /** - * Class CacheExtension * * @author Aaron Scherer + * @author Tobias Nyholm */ class CacheExtension extends Extension { @@ -34,10 +35,8 @@ public function load(array $configs, ContainerBuilder $container) { $config = $this->processConfiguration(new Configuration(), $configs); - if ($container->getParameter('kernel.debug')) { - $container->register('data_collector.cache', CacheDataCollector::class) - ->addTag('data_collector', ['template' => CacheDataCollector::TEMPLATE, 'id' => 'cache']); - } + $loader = new Loader\YamlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config')); + $loader->load('services.yml'); foreach (['router', 'session', 'doctrine'] as $section) { if ($config[$section]['enabled']) { @@ -46,13 +45,14 @@ public function load(array $configs, ContainerBuilder $container) } if ($config['router']['enabled']) { - $container->register('cache.router_listener', RouterListener::class) - ->addArgument(new Reference($config['router']['service_id'])) - ->addArgument($config['router']['ttl']) - ->addTag('kernel.event_listener', ['event'=>'kernel.request', 'method'=>'onBeforeRouting', 'priority'=>33]) - ->addTag('kernel.event_listener', ['event'=>'kernel.request', 'method'=>'onAfterRouting', 'priority'=>31]); + $container->getDefinition('cache.router_listener')->replaceArgument(0, new Reference($config['router']['service_id'])); + } else { + $container->removeDefinition('cache.router_listener'); } + if (!$container->getParameter('kernel.debug')) { + $container->removeDefinition('data_collector.cache'); + } } public function getAlias() diff --git a/src/Resources/config/services.yml b/src/Resources/config/services.yml new file mode 100644 index 0000000..504dce5 --- /dev/null +++ b/src/Resources/config/services.yml @@ -0,0 +1,12 @@ +services: + data_collector.cache: + class: Cache\CacheBundle\DataCollector\CacheDataCollector + tags: + - { tag: data_collectorr, template: 'CacheBundle:Collector:cache.html.twig', id: 'cache' } + + cache.router_listener: + class: Cache\CacheBundle\Routing\RouterListener + arguments: [~, %cache.router.ttl%] + tags: + - { tag: kernel.event_listener, event: kernel.request, method: onBeforeRouting, priority: 33 } + - { tag: kernel.event_listener, event: kernel.request, method: onAfterRouting, priority: 31 } From 4a6899ed52e2c12437b4e083956803ff701fa0b2 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Thu, 10 Dec 2015 15:37:24 +0100 Subject: [PATCH 7/7] typo --- src/Resources/config/services.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Resources/config/services.yml b/src/Resources/config/services.yml index 504dce5..c130c6f 100644 --- a/src/Resources/config/services.yml +++ b/src/Resources/config/services.yml @@ -2,11 +2,11 @@ services: data_collector.cache: class: Cache\CacheBundle\DataCollector\CacheDataCollector tags: - - { tag: data_collectorr, template: 'CacheBundle:Collector:cache.html.twig', id: 'cache' } + - { name: data_collectorr, template: 'CacheBundle:Collector:cache.html.twig', id: 'cache' } cache.router_listener: class: Cache\CacheBundle\Routing\RouterListener arguments: [~, %cache.router.ttl%] tags: - - { tag: kernel.event_listener, event: kernel.request, method: onBeforeRouting, priority: 33 } - - { tag: kernel.event_listener, event: kernel.request, method: onAfterRouting, priority: 31 } + - { name: kernel.event_listener, event: kernel.request, method: onBeforeRouting, priority: 33 } + - { name: kernel.event_listener, event: kernel.request, method: onAfterRouting, priority: 31 }