refactor InvalidateRoute to be explicit about expression. fix #117

FriendsOfSymfony · Aug 2, 2014 · 3697d9f · 3697d9f
1 parent bb52839
commit 3697d9f
Show file tree

Hide file tree

Showing 13 changed files with 86 additions and 47 deletions.
diff --git a/Configuration/InvalidateRoute.php b/Configuration/InvalidateRoute.php
@@ -12,6 +12,7 @@
 namespace FOS\HttpCacheBundle\Configuration;
 
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\ConfigurationAnnotation;
+use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 
 /**
  * @Annotation
@@ -54,6 +55,27 @@ public function getName()
      */
     public function setParams($params)
     {
+        if (!is_array($params)) {
+            throw new \RuntimeException('InvalidateRoute params must be an array');
+        }
+        foreach ($params as $name => $value) {
+            if (is_array($value)) {
+                if (1 !== count($value) || !isset($value['expression'])) {
+                    throw new \RuntimeException(sprintf(
+                        '@InvalidateRoute param %s must be string or {"expression"="<expression>"}',
+                        $name,
+                        print_r($value, true)
+                    ));
+                }
+                if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) {
+                    throw new InvalidConfigurationException(sprintf(
+                        '@InvalidateRoute param %s uses an expression but the ExpressionLanguage is not available.',
+                        $name
+                    ));
+                }
+            }
+        }
+
         $this->params = $params;
     }
 

diff --git a/Configuration/Tag.php b/Configuration/Tag.php
@@ -13,6 +13,7 @@
 
 use FOS\HttpCacheBundle\Exception\InvalidTagException;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\ConfigurationAnnotation;
+use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 
 /**
  * @Annotation
@@ -32,6 +33,9 @@ public function setValue($data)
      */
     public function setExpression($expression)
     {
+        if (!class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage')) {
+            throw new InvalidConfigurationException('@Tag param %s uses an expression but the ExpressionLanguage is not available.');
+        }
         $this->expression = $expression;
     }
 

diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -181,6 +181,10 @@ private function addMatch(NodeBuilder $rules)
                     ->ifTrue(function ($v) {return !empty($v['additional_cacheable_status']) && !empty($v['match_response']);})
                     ->thenInvalid('You may not set both additional_cacheable_status and match_response.')
                 ->end()
+                ->validate()
+                    ->ifTrue(function ($v) {return !empty($v['match_response']) && !class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage');})
+                    ->thenInvalid('Configured a match_response but ExpressionLanugage is not available')
+                ->end()
                 ->children()
                     ->scalarNode('path')
                         ->defaultNull()
@@ -324,7 +328,7 @@ private function addTagSection(ArrayNodeDefinition $rootNode)
                         ->enumNode('enabled')
                             ->values(array(true, false, 'auto'))
                             ->defaultValue('auto')
-                            ->info('Allows to disable the listener for tag annotations when your project does not use the annotations. Enabled by default if you have expression language and the cache manager.')
+                            ->info('Allows to disable the event subscriber for tag configuration and annotations when your project does not use the annotations. Enabled by default if you configured the cache manager.')
                         ->end()
                         ->scalarNode('header')
                             ->defaultValue('X-Cache-Tags')
@@ -334,6 +338,10 @@ private function addTagSection(ArrayNodeDefinition $rootNode)
                             ->prototype('array')
                                 ->fixXmlConfig('tag')
                                 ->fixXmlConfig('tag_expression')
+                                ->validate()
+                                    ->ifTrue(function ($v) {return !empty($v['tag_expressions']) && !class_exists('Symfony\Component\ExpressionLanguage\ExpressionLanguage');})
+                                    ->thenInvalid('Configured a tag_expression but ExpressionLanugage is not available')
+                                ->end()
                                 ->children();
 
         $this->addMatch($rules);
@@ -361,7 +369,7 @@ private function addInvalidationSection(ArrayNodeDefinition $rootNode)
                         ->enumNode('enabled')
                             ->values(array(true, false, 'auto'))
                             ->defaultValue('auto')
-                            ->info('Allows to disable the listener for invalidation annotations when your project does not use the annotations. Enabled by default if you have expression language and the cache manager.')
+                            ->info('Allows to disable the listener for invalidation. Enabled by default if the cache manager is configured. When disabled, the cache manager is no longer flushed automatically.')
                         ->end()
                         ->arrayNode('rules')
                             ->info('Set what requests should invalidate which target routes.')

diff --git a/DependencyInjection/FOSHttpCacheExtension.php b/DependencyInjection/FOSHttpCacheExtension.php
@@ -64,14 +64,9 @@ public function load(array $configs, ContainerBuilder $container)
 
         if ($config['tags']['enabled']) {
             // true or auto
-            if (class_exists('\Symfony\Component\ExpressionLanguage\ExpressionLanguage')) {
-                $loader->load('tag_listener.xml');
-                if (!empty($config['tags']['rules'])) {
-                    $this->loadTagRules($container, $config['tags']['rules']);
-                }
-            } elseif (true === $config['tags']['enabled']) {
-                // silently skip if set to auto
-                throw new InvalidConfigurationException('The TagSubscriber requires symfony/expression-language and needs the cache_manager to be configured');
+            $loader->load('tag_listener.xml');
+            if (!empty($config['tags']['rules'])) {
+                $this->loadTagRules($container, $config['tags']['rules']);
             }
 
             $tagsHeader = $config['tags']['header'];
@@ -81,15 +76,9 @@ public function load(array $configs, ContainerBuilder $container)
         }
 
         if ($config['invalidation']['enabled']) {
-            // true or auto
-            if (class_exists('\Symfony\Component\ExpressionLanguage\ExpressionLanguage')) {
-                $loader->load('invalidation_listener.xml');
-                if (!empty($config['invalidation']['rules'])) {
-                    $this->loadInvalidatorRules($container, $config['invalidation']['rules']);
-                }
-            } elseif (true === $config['invalidation']['enabled']) {
-                // silently skip if set to auto
-                throw new InvalidConfigurationException('The InvalidationSubscriber requires symfony/expression-language and needs the cache_manager to be configured');
+            $loader->load('invalidation_listener.xml');
+            if (!empty($config['invalidation']['rules'])) {
+                $this->loadInvalidatorRules($container, $config['invalidation']['rules']);
             }
         }
 

diff --git a/EventListener/InvalidationSubscriber.php b/EventListener/InvalidationSubscriber.php
@@ -209,11 +209,8 @@ private function invalidateRoutes(array $routes, Request $request)
             if (null !== $route->getParams()) {
                 // Iterate over route params and try to evaluate their values
                 foreach ($route->getParams() as $key => $value) {
-                    try {
-                        $value = $this->getExpressionLanguage()->evaluate($value, $request->attributes->all());
-                    } catch (SyntaxError $e) {
-                        // If a syntax error occurred, we assume the param was
-                        // no expression
+                    if (is_array($value)) {
+                        $value = $this->getExpressionLanguage()->evaluate($value['expression'], $request->attributes->all());
                     }
 
                     $params[$key] = $value;

diff --git a/EventListener/TagSubscriber.php b/EventListener/TagSubscriber.php
@@ -47,7 +47,7 @@ public function __construct(
         ExpressionLanguage $expressionLanguage = null
     ) {
         $this->cacheManager = $cacheManager;
-        $this->expressionLanguage = $expressionLanguage ?: new ExpressionLanguage();
+        $this->expressionLanguage = $expressionLanguage;
     }
 
     /**
@@ -146,10 +146,24 @@ private function getAnnotationTags(Request $request)
      */
     private function evaluateTag($expression, Request $request)
     {
-        return $this->expressionLanguage->evaluate(
+        return $this->getExpressionLanguage()->evaluate(
             $expression,
             $request->attributes->all()
         );
     }
 
+    /**
+     * Delay instantiating the expression language instance until we need it,
+     * to support a setup with only symfony 2.3.
+     *
+     * @return ExpressionLanguage
+     */
+    private function getExpressionLanguage()
+    {
+        if (!$this->expressionLanguage) {
+            $this->expressionLanguage = new ExpressionLanguage();
+        }
+
+        return $this->expressionLanguage;
+    }
 }
diff --git a/Exception/InvalidTagException.php b/Exception/InvalidTagException.php
@@ -15,6 +15,6 @@ class InvalidTagException extends \InvalidArgumentException
 {
     public function __construct($tag, $char)
     {
-        parent::__construct(sprintf('Tag %s is invalid because it contains %s'));
+        parent::__construct(sprintf('Tag %s is invalid because it contains %s', $tag, $char));
     }
 }
diff --git a/Resources/doc/features/invalidation.rst b/Resources/doc/features/invalidation.rst
@@ -76,11 +76,12 @@ invalidation from your controllers::
     use FOS\HttpCacheBundle\Configuration\InvalidatePath;
 
     /**
-     * @InvalidatePath("/posts")
-     * @InvalidatePath("/posts/latest")
+     * @InvalidatePath("/articles")
+     * @InvalidatePath("/articles/latest")
      * @InvalidateRoute("overview", params={"type" = "latest"})")
+     * @InvalidateRoute("detail", params={"id" = {"expression"="id"}})")
      */
-    public function editAction()
+    public function editAction($id)
     {
     }
 

diff --git a/Resources/doc/includes/enabled.rst b/Resources/doc/includes/enabled.rst
@@ -3,5 +3,5 @@ enabled
 
 **type**: ``enum``, **default**: ``auto``, **options**: ``true``, ``false``, ``auto``
 
-Enabled by default if :ref:`ExpressionLanguage is installed <expression language requirement>`
-and you have :doc:`configured a proxy client </reference/configuration/proxy-client>`.
+Enabled by default if you have configured the cache manager with
+:doc:`a proxy client </reference/configuration/proxy-client>`.
diff --git a/Resources/doc/reference/annotations.rst b/Resources/doc/reference/annotations.rst
@@ -6,8 +6,9 @@ actions are executed.
 
 .. note::
 
-    Annotations need the SensioFrameworkExtraBundle. Some features also need
-    the ExpressionLanguage. Make sure to
+    Annotations need the SensioFrameworkExtraBundle including registering the
+    Doctrine AnnotationsRegistry. Some features also need the
+    ExpressionLanguage. Make sure to
     :ref:`installed the dependencies first <requirements>`.
 
 .. _invalidatepath:
@@ -20,8 +21,8 @@ Invalidate a path::
     use FOS\HttpCacheBundle\Configuration\InvalidatePath;
 
     /**
-     * @InvalidatePath("/posts")
-     * @InvalidatePath("/posts/latest")
+     * @InvalidatePath("/articles")
+     * @InvalidatePath("/articles/latest")
      */
     public function editAction()
     {
@@ -39,25 +40,25 @@ Invalidate a route with parameters::
     use FOS\HttpCacheBundle\Configuration\InvalidateRoute;
 
     /**
-     * @InvalidateRoute("posts")
-     * @InvalidateRoute("posts", params={"type" = "latest"})
+     * @InvalidateRoute("articles")
+     * @InvalidateRoute("articles", params={"type" = "latest"})
      */
     public function editAction()
     {
     }
 
-You can also use expressions_ in the route parameter values::
+You can also use expressions_ in the route parameter values. This obviously
+:ref:`requires the ExpressionLanguage component <requirements>`. To invalidate
+route ``articles`` with the ``number`` parameter set to ``123``, do::
 
     /**
-     * @InvalidateRoute("posts", params={"number" = "id"})
+     * @InvalidateRoute("articles", params={"number" = {"expression"="id"}})
      */
-    public function editAction(Request $request)
+    public function editAction(Request $request, $id)
     {
         // Assume $request->attributes->get('id') returns 123
     }
 
-Route ``posts`` will now be invalidated with value ``123`` for param ``number``.
-
 See :doc:`/features/invalidation` for more information.
 
 .. _tag:
@@ -106,8 +107,9 @@ If you prefer, you can combine tags in one annotation::
      * @Tag({"news", "news-list"})
      */
 
-You can also use expressions_ in tags. This will set tag ``news-123`` on the
-response::
+You can also use expressions_ in tags. This obviously
+:ref:`requires the ExpressionLanguage component <requirements>`. The following
+example sets the tag ``news-123`` on the Response::
 
     /**
      * @Tag(expression="'news-'~id")

diff --git a/Tests/Functional/EventListener/InvalidationSubscriberTest.php b/Tests/Functional/EventListener/InvalidationSubscriberTest.php
@@ -23,6 +23,7 @@ public function testInvalidateRoute()
             'fos_http_cache.cache_manager',
             '\FOS\HttpCacheBundle\CacheManager'
         )
+            ->shouldReceive('invalidateRoute')->once()->with('test_noncached', array())
             ->shouldReceive('invalidateRoute')->once()->with('test_cached', array('id' => 'myhardcodedid'))
             ->shouldReceive('invalidateRoute')->once()->with('tag_one', array('id' => '42'))
             ->shouldReceive('flush')->once()

diff --git a/Tests/Functional/Fixtures/Controller/InvalidationController.php b/Tests/Functional/Fixtures/Controller/InvalidationController.php
@@ -19,8 +19,9 @@
 class InvalidationController extends Controller
 {
     /**
+     * @InvalidateRoute("test_noncached")
      * @InvalidateRoute("test_cached", params={"id" = "myhardcodedid"})
-     * @InvalidateRoute("tag_one", params={"id" = "id"})
+     * @InvalidateRoute("tag_one", params={"id" = {"expression"="id"}})
      */
     public function itemAction($id)
     {

diff --git a/Tests/Unit/EventListener/InvalidationSubscriberTest.php b/Tests/Unit/EventListener/InvalidationSubscriberTest.php
@@ -143,7 +143,7 @@ public function testInvalidateRoute()
         $request->attributes->set('request_id', 123);
         $request->attributes->set('_invalidate_route', array(
             new InvalidateRoute(array('name' => 'some_route')),
-            new InvalidateRoute(array('name' => 'other_route', 'params' => array('id' => 'request_id')))
+            new InvalidateRoute(array('name' => 'other_route', 'params' => array('id' => array('expression' => 'request_id'))))
         ));
 
         $event = $this->getEvent($request);