From 89aae32645c61f059110b797ba2bd90906ec2667 Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 7 Dec 2015 01:04:08 +0100 Subject: [PATCH 1/2] Bugfixes and improvements --- LICENSE | 0 src/Cache/LoggingCachePool.php | 0 src/DataCollector/CacheDataCollector.php | 2 +- src/DependencyInjection/CacheExtension.php | 4 +-- .../Compiler/DataCollectorCompilerPass.php | 17 +++++++------ .../Compiler/DoctrineSupportCompilerPass.php | 23 ++++++++--------- .../Compiler/RouterCompilerPass.php | 4 +-- .../Compiler/SessionSupportCompilerPass.php | 25 +++---------------- src/DependencyInjection/Configuration.php | 10 ++++---- src/Routing/Router.php | 16 +++++++++--- src/Session/SessionHandler.php | 4 +-- 11 files changed, 49 insertions(+), 56 deletions(-) mode change 100644 => 100755 LICENSE mode change 100644 => 100755 src/Cache/LoggingCachePool.php mode change 100644 => 100755 src/DependencyInjection/Configuration.php diff --git a/LICENSE b/LICENSE old mode 100644 new mode 100755 diff --git a/src/Cache/LoggingCachePool.php b/src/Cache/LoggingCachePool.php old mode 100644 new mode 100755 diff --git a/src/DataCollector/CacheDataCollector.php b/src/DataCollector/CacheDataCollector.php index 86aec5f..0e9e16e 100755 --- a/src/DataCollector/CacheDataCollector.php +++ b/src/DataCollector/CacheDataCollector.php @@ -29,7 +29,7 @@ class CacheDataCollector extends DataCollector * * @type string */ - const TEMPLATE = 'CacheCacheBundle:Collector:cache.html.twig'; + const TEMPLATE = 'CacheBundle:Collector:cache.html.twig'; /** * @var LoggingCachePool[] diff --git a/src/DependencyInjection/CacheExtension.php b/src/DependencyInjection/CacheExtension.php index 6a8a9b7..671850a 100755 --- a/src/DependencyInjection/CacheExtension.php +++ b/src/DependencyInjection/CacheExtension.php @@ -40,8 +40,8 @@ public function load(array $configs, ContainerBuilder $container) } foreach (['router', 'session', 'doctrine'] as $section) { - if ($container[$section]['enabled']) { - $container->setParameter($this->getAlias().'.'.$section, $config[$section]); + if ($config[$section]['enabled']) { + $container->setParameter('cache.'.$section, $config[$section]); } } diff --git a/src/DependencyInjection/Compiler/DataCollectorCompilerPass.php b/src/DependencyInjection/Compiler/DataCollectorCompilerPass.php index 72a31eb..3b06508 100755 --- a/src/DependencyInjection/Compiler/DataCollectorCompilerPass.php +++ b/src/DependencyInjection/Compiler/DataCollectorCompilerPass.php @@ -11,12 +11,14 @@ namespace Cache\CacheBundle\DependencyInjection\Compiler; +use Cache\CacheBundle\Cache\LoggingCachePool; use Symfony\Component\DependencyInjection\Reference; /** * Class DataCollectorCompilerPass * * @author Aaron Scherer + * @author Tobias Nyholm */ class DataCollectorCompilerPass extends BaseCompilerPass { @@ -27,18 +29,19 @@ protected function prepare() { $this->transformLoggableCachePools(); - $instances = $this->container->getParameter($this->getAlias() . '.instance'); + $collectorDefinition = $this->container->getDefinition('data_collector.cache'); - $definition = $this->container->getDefinition('data_collector.cache'); - - foreach (array_keys($instances) as $name) { - $instance = new Reference(sprintf("%s.instance.%s", $this->getAlias(), $name)); - $definition->addMethodCall('addInstance', array($name, $instance)); + $serviceIds = $this->container->findTaggedServiceIds('cache.provider'); + foreach (array_keys($serviceIds) as $id) { + $collectorDefinition->addMethodCall('addInstance', [$id, new Reference($id)]); } - $this->container->setDefinition('data_collector.cache', $definition); + $this->container->setDefinition('data_collector.cache', $collectorDefinition); } + /** + * Make all cache providers loggable. + */ private function transformLoggableCachePools() { $serviceIds = $this->container->findTaggedServiceIds('cache.provider'); diff --git a/src/DependencyInjection/Compiler/DoctrineSupportCompilerPass.php b/src/DependencyInjection/Compiler/DoctrineSupportCompilerPass.php index f551811..70403b3 100755 --- a/src/DependencyInjection/Compiler/DoctrineSupportCompilerPass.php +++ b/src/DependencyInjection/Compiler/DoctrineSupportCompilerPass.php @@ -11,12 +11,16 @@ namespace Cache\CacheBundle\DependencyInjection\Compiler; +use Cache\Bridge\DoctrineCacheBridge; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Reference; /** * Class DoctrineSupportCompilerPass * * @author Aaron Scherer + * @author Tobias Nyholm */ class DoctrineSupportCompilerPass extends BaseCompilerPass { @@ -27,7 +31,7 @@ class DoctrineSupportCompilerPass extends BaseCompilerPass protected function prepare() { // If disabled, continue - if (!$this->container->hasParameter($this->getAlias().'.doctrine')) { + if (!$this->container->hasParameter('cache.doctrine')) { return; } @@ -37,7 +41,7 @@ protected function prepare() ); } - $this->enableDoctrineSupport($this->container->getParameter($this->getAlias().'.doctrine')); + $this->enableDoctrineSupport($this->container->getParameter('cache.doctrine')); } /** @@ -56,15 +60,10 @@ protected function enableDoctrineSupport(array $config) continue; } - if (!isset($cacheData['instance'])) { - throw new InvalidConfigurationException( - sprintf( - "There was no instance passed. Please specify a instance under the %s type", - $cacheType - ) - ); - } - $cacheDefinitionName = sprintf('%s.instance.%s.bridge', $this->getAlias(), $cacheData['instance']); + $bridgeServiceId = sprintf('cache.provider.doctrine.%s.bridge', $cacheType); + $bridgeDef = $this->container->register($bridgeServiceId, DoctrineCacheBridge::class); + $bridgeDef->addArgument(0, new Reference($cacheData['service_id'])) + ->setPublic(false); foreach ($cacheData[$type] as $manager) { $doctrineDefinitionName = @@ -76,7 +75,7 @@ protected function enableDoctrineSupport(array $config) ); // Replace the doctrine entity manager cache with our bridge - $this->container->setAlias($doctrineDefinitionName, $cacheDefinitionName); + $this->container->setAlias($doctrineDefinitionName, $bridgeServiceId); } } } diff --git a/src/DependencyInjection/Compiler/RouterCompilerPass.php b/src/DependencyInjection/Compiler/RouterCompilerPass.php index 2ba0c0f..1b59ff0 100755 --- a/src/DependencyInjection/Compiler/RouterCompilerPass.php +++ b/src/DependencyInjection/Compiler/RouterCompilerPass.php @@ -32,14 +32,14 @@ protected function prepare() $def = clone $this->container->findDefinition('router'); $def->setClass('Cache\CacheBundle\Routing\Router'); - $def->addMethodCall('setCache', [new Reference(sprintf('cache.instance.%s', $router['instance']))]); + $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', 'router.cache'); + $this->container->setAlias('router', 'cache.router'); } } } diff --git a/src/DependencyInjection/Compiler/SessionSupportCompilerPass.php b/src/DependencyInjection/Compiler/SessionSupportCompilerPass.php index e3adc5f..7f00933 100755 --- a/src/DependencyInjection/Compiler/SessionSupportCompilerPass.php +++ b/src/DependencyInjection/Compiler/SessionSupportCompilerPass.php @@ -11,6 +11,7 @@ namespace Cache\CacheBundle\DependencyInjection\Compiler; +use Cache\CacheBundle\Session\SessionHandler; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; @@ -49,35 +50,17 @@ protected function prepare() */ private function enableSessionSupport(array $config) { - if (empty($config['instance'])) { - throw new InvalidConfigurationException("Instance must be passed under the `session` config."); - } - - $instance = $config['instance']; - $instances = $this->container->getParameter($this->getAlias().'.instance'); - - if (!isset($instances[$instance])) { - throw new InvalidConfigurationException( - sprintf( - "Failed to hook into the session. The instance \"%s\" doesn't exist!", - $instance - ) - ); - } - // calculate options $sessionOptions = $this->container->getParameter('session.storage.options'); if (isset($sessionOptions['cookie_lifetime']) && !isset($config['cookie_lifetime'])) { $config['cookie_lifetime'] = $sessionOptions['cookie_lifetime']; } // load the session handler - $definition = - new Definition($this->container->getParameter(sprintf('%s.session.handler.class', $this->getAlias()))); - $definition->addArgument(new Reference(sprintf('%s.instance.%s.bridge', $this->getAlias(), $instance))) + $definition = new Definition(SessionHandler::class); + $definition->addArgument(new Reference($config['service_id'])) ->addArgument($config); - $this->container->setDefinition(sprintf('%s.session_handler', $this->getAlias()), $definition); - $this->container->setAlias('cache.session_handler', sprintf('%s.session_handler', $this->getAlias())); + $this->container->setDefinition('cache.session_handler', $definition); $this->container->setAlias('session.handler', 'cache.session_handler'); } diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php old mode 100644 new mode 100755 index 7c876ec..d936695 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -57,7 +57,7 @@ private function addSessionSupportSection() ->booleanNode('enabled') ->defaultFalse() ->end() - ->scalarNode('instance')->end() + ->scalarNode('service_id')->isRequired()->end() ->scalarNode('prefix') ->defaultValue("session_") ->end() @@ -94,7 +94,7 @@ private function addDoctrineSection() ->arrayNode($type) ->canBeUnset() ->children() - ->scalarNode('instance')->end() + ->scalarNode('service_id')->isRequired()->end() ->arrayNode('entity_managers') ->defaultValue(array()) ->beforeNormalization() @@ -142,13 +142,13 @@ private function addRouterSection() ->defaultFalse() ->end() ->integerNode('ttl') - ->isRequired() + ->defaultValue(86400) ->end() ->booleanNode('auto-register') ->defaultTrue() ->end() - ->scalarNode('instance') - ->defaultNull() + ->scalarNode('service_id') + ->isRequired() ->end() ->end(); diff --git a/src/Routing/Router.php b/src/Routing/Router.php index 847ab8a..2637af8 100755 --- a/src/Routing/Router.php +++ b/src/Routing/Router.php @@ -23,8 +23,6 @@ */ class Router extends BaseRouter { - const CACHE_LIFETIME = 604800; // a week - /** * @var ContainerInterface */ @@ -92,8 +90,20 @@ public function setCache(CacheItemPoolInterface $cache) /** * @return int */ - public function getTtl() + protected function getTtl() { return $this->ttl; } + + /** + * @param int $ttl + * + * @return Router + */ + public function setTtl($ttl) + { + $this->ttl = $ttl; + + return $this; + } } diff --git a/src/Session/SessionHandler.php b/src/Session/SessionHandler.php index 8452353..7b35adc 100755 --- a/src/Session/SessionHandler.php +++ b/src/Session/SessionHandler.php @@ -101,9 +101,7 @@ public function write($sessionId, $data) */ public function destroy($sessionId) { - $item = $this->cache->getItem($this->prefix . $sessionId); - - return $this->cache->deleteItem($item); + return $this->cache->deleteItem($this->prefix . $sessionId); } /** From 224e62530f3b624b07799e226c09c3d1ae519a8c Mon Sep 17 00:00:00 2001 From: Tobias Nyholm Date: Mon, 7 Dec 2015 02:09:49 +0100 Subject: [PATCH 2/2] fixes --- .../Compiler/DataCollectorCompilerPass.php | 16 +--------------- src/Routing/Matcher/CacheUrlMatcher.php | 9 +++++---- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/src/DependencyInjection/Compiler/DataCollectorCompilerPass.php b/src/DependencyInjection/Compiler/DataCollectorCompilerPass.php index 3b06508..3343833 100755 --- a/src/DependencyInjection/Compiler/DataCollectorCompilerPass.php +++ b/src/DependencyInjection/Compiler/DataCollectorCompilerPass.php @@ -27,24 +27,9 @@ class DataCollectorCompilerPass extends BaseCompilerPass */ protected function prepare() { - $this->transformLoggableCachePools(); - $collectorDefinition = $this->container->getDefinition('data_collector.cache'); - $serviceIds = $this->container->findTaggedServiceIds('cache.provider'); - foreach (array_keys($serviceIds) as $id) { - $collectorDefinition->addMethodCall('addInstance', [$id, new Reference($id)]); - } - - $this->container->setDefinition('data_collector.cache', $collectorDefinition); - } - /** - * Make all cache providers loggable. - */ - private function transformLoggableCachePools() - { - $serviceIds = $this->container->findTaggedServiceIds('cache.provider'); foreach (array_keys($serviceIds) as $id) { // Duplicating definition to $originalServiceId.logged @@ -56,6 +41,7 @@ private function transformLoggableCachePools() // Overwrite the original service id with the new LoggingCachePool instance $this->container->setAlias($id, $id.'.logger'); + $collectorDefinition->addMethodCall('addInstance', [$id, new Reference($id.'.logger')]); } } } diff --git a/src/Routing/Matcher/CacheUrlMatcher.php b/src/Routing/Matcher/CacheUrlMatcher.php index 07385f2..bbf3487 100755 --- a/src/Routing/Matcher/CacheUrlMatcher.php +++ b/src/Routing/Matcher/CacheUrlMatcher.php @@ -63,14 +63,15 @@ public function match($pathInfo) $method = strtolower($this->context->getMethod()); $key = 'route_'.$method.'_'.$host.'_'.$pathInfo; - if ($this->cachePool->hasItem($key)) { - return $this->cachePool->getItem($key)->get(); + $cacheItem = $this->cachePool->getItem($key); + if ($cacheItem->isHit()) { + return $cacheItem->get(); } $match = parent::match($pathInfo); - $item = $this->cachePool->getItem($key); - $item->set($match) + $cacheItem->set($match) ->expiresAfter($this->ttl); + $this->cachePool->save($cacheItem); return $match; }