Merge pull request #1 from Nyholm/patch

cryptiklemur · cryptiklemur · commit a757bd25b093 · 2015-12-07T08:00:12.000Z
Bugfixes and improvements
diff --git a/LICENSE b/LICENSE
diff --git a/src/Cache/LoggingCachePool.php b/src/Cache/LoggingCachePool.php
diff --git 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
@@ -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
@@ -11,12 +11,14 @@
 
 namespace Cache\CacheBundle\DependencyInjection\Compiler;
 
+use Cache\CacheBundle\Cache\LoggingCachePool;
 use Symfony\Component\DependencyInjection\Reference;
 
 /**
  * Class DataCollectorCompilerPass
  *
  * @author Aaron Scherer <aequasi@gmail.com>
+ * @author Tobias Nyholm <tobias.nyholm@gmail.com>
  */
 class DataCollectorCompilerPass extends BaseCompilerPass
 {
@@ -25,23 +27,9 @@ class DataCollectorCompilerPass extends BaseCompilerPass
      */
     protected function prepare()
     {
-        $this->transformLoggableCachePools();
-
-        $instances = $this->container->getParameter($this->getAlias() . '.instance');
-
-        $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));
-        }
-
-        $this->container->setDefinition('data_collector.cache', $definition);
-    }
-
-    private function transformLoggableCachePools()
-    {
+        $collectorDefinition = $this->container->getDefinition('data_collector.cache');
         $serviceIds = $this->container->findTaggedServiceIds('cache.provider');
+
         foreach (array_keys($serviceIds) as $id) {
 
             // Duplicating definition to $originalServiceId.logged
@@ -53,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/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 <aequasi@gmail.com>
+ * @author Tobias Nyholm <tobias.nyholm@gmail.com>
  */
 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
@@ -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
@@ -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
@@ -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/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;
     }
diff --git 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
@@ -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);
     }
 
     /**

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ class CacheDataCollector extends DataCollector`
`29`	`29`	`*`
`30`	`30`	`* @type string`
`31`	`31`	`*/`
`32`		`- const TEMPLATE = 'CacheCacheBundle:Collector:cache.html.twig';`
	`32`	`+ const TEMPLATE = 'CacheBundle:Collector:cache.html.twig';`
`33`	`33`
`34`	`34`	`/**`
`35`	`35`	`* @var LoggingCachePool[]`
Original file line number	Diff line number	Diff line change
`@@ -40,8 +40,8 @@ public function load(array $configs, ContainerBuilder $container)`
`40`	`40`	`}`
`41`	`41`
`42`	`42`	`foreach (['router', 'session', 'doctrine'] as $section) {`
`43`		`- if ($container[$section]['enabled']) {`
`44`		`- $container->setParameter($this->getAlias().'.'.$section, $config[$section]);`
	`43`	`+ if ($config[$section]['enabled']) {`
	`44`	`+ $container->setParameter('cache.'.$section, $config[$section]);`
`45`	`45`	`}`
`46`	`46`	`}`
`47`	`47`
Original file line number	Diff line number	Diff line change
`@@ -11,12 +11,16 @@`
`11`	`11`
`12`	`12`	`namespace Cache\CacheBundle\DependencyInjection\Compiler;`
`13`	`13`
	`14`	`+use Cache\Bridge\DoctrineCacheBridge;`
`14`	`15`	`use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;`
	`16`	`+use Symfony\Component\DependencyInjection\Definition;`
	`17`	`+use Symfony\Component\DependencyInjection\Reference;`
`15`	`18`
`16`	`19`	`/**`
`17`	`20`	`* Class DoctrineSupportCompilerPass`
`18`	`21`	`*`
`19`	`22`	`* @author Aaron Scherer <[email protected]>`
	`23`	`+ * @author Tobias Nyholm <[email protected]>`
`20`	`24`	`*/`
`21`	`25`	`class DoctrineSupportCompilerPass extends BaseCompilerPass`
`22`	`26`	`{`
`@@ -27,7 +31,7 @@ class DoctrineSupportCompilerPass extends BaseCompilerPass`
`27`	`31`	`protected function prepare()`
`28`	`32`	`{`
`29`	`33`	`// If disabled, continue`
`30`		`- if (!$this->container->hasParameter($this->getAlias().'.doctrine')) {`
	`34`	`+ if (!$this->container->hasParameter('cache.doctrine')) {`
`31`	`35`	`return;`
`32`	`36`	`}`
`33`	`37`
`@@ -37,7 +41,7 @@ protected function prepare()`
`37`	`41`	`);`
`38`	`42`	`}`
`39`	`43`
`40`		`- $this->enableDoctrineSupport($this->container->getParameter($this->getAlias().'.doctrine'));`
	`44`	`+ $this->enableDoctrineSupport($this->container->getParameter('cache.doctrine'));`
`41`	`45`	`}`
`42`	`46`
`43`	`47`	`/**`
`@@ -56,15 +60,10 @@ protected function enableDoctrineSupport(array $config)`
`56`	`60`	`continue;`
`57`	`61`	`}`
`58`	`62`
`59`		`- if (!isset($cacheData['instance'])) {`
`60`		`- throw new InvalidConfigurationException(`
`61`		`- sprintf(`
`62`		`- "There was no instance passed. Please specify a instance under the %s type",`
`63`		`- $cacheType`
`64`		`- )`
`65`		`- );`
`66`		`- }`
`67`		`- $cacheDefinitionName = sprintf('%s.instance.%s.bridge', $this->getAlias(), $cacheData['instance']);`
	`63`	`+ $bridgeServiceId = sprintf('cache.provider.doctrine.%s.bridge', $cacheType);`
	`64`	`+ $bridgeDef = $this->container->register($bridgeServiceId, DoctrineCacheBridge::class);`
	`65`	`+ $bridgeDef->addArgument(0, new Reference($cacheData['service_id']))`
	`66`	`+ ->setPublic(false);`
`68`	`67`
`69`	`68`	`foreach ($cacheData[$type] as $manager) {`
`70`	`69`	`$doctrineDefinitionName =`
`@@ -76,7 +75,7 @@ protected function enableDoctrineSupport(array $config)`
`76`	`75`	`);`
`77`	`76`
`78`	`77`	`// Replace the doctrine entity manager cache with our bridge`
`79`		`- $this->container->setAlias($doctrineDefinitionName, $cacheDefinitionName);`
	`78`	`+ $this->container->setAlias($doctrineDefinitionName, $bridgeServiceId);`
`80`	`79`	`}`
`81`	`80`	`}`
`82`	`81`	`}`
Original file line number	Diff line number	Diff line change
`@@ -32,14 +32,14 @@ protected function prepare()`
`32`	`32`
`33`	`33`	`$def = clone $this->container->findDefinition('router');`
`34`	`34`	`$def->setClass('Cache\CacheBundle\Routing\Router');`
`35`		`- $def->addMethodCall('setCache', [new Reference(sprintf('cache.instance.%s', $router['instance']))]);`
	`35`	`+ $def->addMethodCall('setCache', [new Reference($router['service_id'])]);`
`36`	`36`	`$def->addMethodCall('setTtl', [$router['ttl']]);`
`37`	`37`
`38`	`38`	`$this->container->setDefinition('cache.router', $def);`
`39`	`39`	`$this->container->setAlias('router.alias', 'cache.router');`
`40`	`40`
`41`	`41`	`if ($router['auto-register']) {`
`42`		`- $this->container->setAlias('router', 'router.cache');`
	`42`	`+ $this->container->setAlias('router', 'cache.router');`
`43`	`43`	`}`
`44`	`44`	`}`
`45`	`45`	`}`