Merge pull request #354 from FriendsOfSymfony/rule-matcher

Add rule matchers for cacheable and must invalidate

Author: dbu

CHANGELOG.md

@@ -32,6 +32,17 @@ Changelog
 
 * Deprecated methods have been removed.
 
+### Rule matcher
+
+* **BC break:** The `match_response` and `additional_cacheable_status` 
+  configuration parameters were removed for individual match rules. 
+
+* **BC break:** The second argument of the `RuleMatcher` constructor was changed 
+  to take a `ResponseMatcherInterface`.
+  
+* Cacheable status codes are now configured globally 
+  (`cacheable.response.additional_status`).
+
 1.3.7
 -----
 

Resources/doc/features/headers.rst

@@ -41,7 +41,6 @@ This is an example configuration. For more, see the
                 -
                     match:
                         attributes: { _controller: ^AcmeBundle:Default:.* }
-                        additional_cacheable_status: [400]
                     headers:
                         cache_control: { public: true, max_age: 15, s_maxage: 30 }
                         last_modified: "-1 hour"

Resources/doc/includes/safe.rst

@@ -1,4 +1,4 @@
 1. the HTTP request matches *all* criteria defined under ``match``
 2. the HTTP request is :term:`safe` (GET or HEAD)
 3. the HTTP response is considered :term:`cacheable` (override with
-   :ref:`additional_cacheable_status` and :ref:`match_response`).
+   :ref:`additional_status`).

Resources/doc/reference/configuration.rst

@@ -15,5 +15,6 @@ for the bundle.
     configuration/user-context
     configuration/flash-message
     configuration/debug
+    configuration/cacheable
     configuration/match
     configuration/test

Resources/doc/reference/configuration/cacheable.rst

@@ -0,0 +1,49 @@
+cacheable
+=========
+
+``response``
+------------
+
+Configure which responses are considered :term:`cacheable`. This bundle will
+only set Cache-Control headers, including tags etc., on cacheable responses.
+
+.. _additional_status:
+
+``additional_status``
+^^^^^^^^^^^^^^^^^^^^^
+
+**type**: ``array``
+
+Following `RFC 7231`_, by default responses are considered :term:`cacheable`
+if they have status code 200, 203, 204, 206, 300, 301, 404, 405, 410, 414 or 501.
+You can add status codes to this list by setting ``additional_status``:
+
+.. code-block:: yaml
+
+    # app/config/config.yml
+    fos_http_cache:
+        cacheable:
+            response:
+                additional_status:
+                    - 100
+                    - 500
+
+``expression``
+^^^^^^^^^^^^^^
+
+**type**: ``string``
+
+An ExpressionLanguage expression to decide whether the response is considered
+cacheable. The expression can access the Response object with the response variable.
+
+.. code-block:: yaml
+
+    # app/config/config.yml
+    fos_http_cache:
+        cacheable:
+            response:
+                expression: "response.getStatusCode() >= 300"
+
+You cannot set both ``expression`` and ``additional_status``.
+
+.. _RFC 7231: https://tools.ietf.org/html/rfc7231#section-6.1

Resources/doc/reference/configuration/headers.rst

@@ -34,7 +34,6 @@ cache headers even if they are already set:
                 -
                     match:
                         attributes: { _controller: ^Acme\\TestBundle\\Controller\\DefaultController::.* }
-                        additional_cacheable_status: [400]
                     headers:
                         cache_control:
                             public: true

Resources/doc/reference/configuration/match.rst

@@ -121,50 +121,6 @@ regular expressions.
     match:
         attributes: { _controller: ^AcmeBundle:Default:.* }
 
-.. _additional_cacheable_status:
-
-``additional_cacheable_status``
--------------------------------
-
-**type**: ``array``
-
-By default, a rule will only match cacheable status codes: 200, 203, 300, 301,
-302, 404 and 410 (as described in the `RFC 7231`_).
-
-`additional_cacheable_status` let you define a list of additional HTTP
-status codes of the response for which to also apply the rule.
-
-.. code-block:: yaml
-
-    match:
-        additional_cacheable_status: [400, 403]
-
-.. _match_response:
-
-``match_response``
-------------------
-
-**type**: ``string``
-
-.. note::
-
-    ``match_response`` :ref:`requires the ExpressionLanguage component <requirements>`.
-
-An ExpressionLanguage expression to decide whether the response should have
-the effect applied. If not set, headers are applied if the request is
-:term:`safe`. The expression can access the ``Response`` object with the
-``response`` variable. For example, to handle all failed requests, you can do:
-
-.. code-block:: yaml
-
-    -
-        match:
-            match_response: response.getStatusCode() >= 400
-        # ...
-
-You cannot set both ``match_response`` and ``additional_cacheable_status``
-inside the same rule.
-
 .. _Trusting Proxies: http://symfony.com/doc/current/components/http_foundation/trusting_proxies.html
 .. _controllers as services: http://symfony.com/doc/current/cookbook/controller/service.html
 .. _RFC 7231: http://tools.ietf.org/html/rfc7231#page-48

Resources/doc/reference/glossary.rst

@@ -4,15 +4,13 @@ Glossary
 .. glossary::
 
     Cacheable
-        A *response* is considered cacheable when the status code is one of
-        200, 203, 300, 301, 302, 404, 410. This range of status codes can be
-        extended with :ref:`additional_cacheable_status` or overridden with
-        :ref:`match_response`.
+        According to `RFC 7231`_, a *response* is considered cacheable when its
+        status code is one of 200, 203, 204, 206, 300, 301, 404, 405, 410, 414
+        or 501.
 
     Safe
         A *request* is safe if its HTTP method is GET or HEAD. Safe methods
         only retrieve data and do not change the application state, and
         therefore can be served with a response from the cache.
 
-
-
+.. _RFC 7231: https://tools.ietf.org/html/rfc7231#section-6.1

src/DependencyInjection/Configuration.php

@@ -111,6 +111,7 @@ function ($v) {
                 })
         ;
 
+        $this->addCacheableResponseSection($rootNode);
         $this->addCacheControlSection($rootNode);
         $this->addProxyClientSection($rootNode);
         $this->addCacheManagerSection($rootNode);
@@ -124,6 +125,37 @@ function ($v) {
         return $treeBuilder;
     }
 
+    private function addCacheableResponseSection(ArrayNodeDefinition $rootNode)
+    {
+        $rootNode
+            ->children()
+                ->arrayNode('cacheable')
+                    ->addDefaultsIfNotSet()
+                    ->children()
+                        ->arrayNode('response')
+                            ->addDefaultsIfNotSet()
+                            ->children()
+                                ->arrayNode('additional_status')
+                                    ->prototype('scalar')->end()
+                                    ->info('Additional response HTTP status codes that will be considered cacheable.')
+                                ->end()
+                                ->scalarNode('expression')
+                                    ->defaultNull()
+                                    ->info('Expression to decide whether response is cacheable.')
+                            ->end()
+                        ->end()
+
+                        ->validate()
+                            ->ifTrue(function ($v) {
+                                return !empty($v['additional_status']) && !empty($v['expression']);
+                            })
+                            ->thenInvalid('You may not set both additional_status and expression.')
+                        ->end()
+                    ->end()
+                ->end()
+            ->end();
+    }
+
     /**
      * Cache header control main section.
      *
@@ -223,12 +255,6 @@ private function addMatch(NodeBuilder $rules)
                 ->fixXmlConfig('method')
                 ->fixXmlConfig('ip')
                 ->fixXmlConfig('attribute')
-                ->validate()
-                    ->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');
@@ -265,14 +291,6 @@ private function addMatch(NodeBuilder $rules)
                         ->prototype('scalar')->end()
                         ->info('Regular expressions on request attributes.')
                     ->end()
-                    ->arrayNode('additional_cacheable_status')
-                        ->prototype('scalar')->end()
-                        ->info('Additional response HTTP status codes that will match.')
-                    ->end()
-                    ->scalarNode('match_response')
-                        ->defaultNull()
-                        ->info('Expression to decide whether response should be matched. Replaces HTTP code check and additional_cacheable_status.')
-                    ->end()
                 ->end()
             ->end()
         ;

src/DependencyInjection/FOSHttpCacheExtension.php

@@ -13,6 +13,7 @@
 
 use FOS\HttpCache\ProxyClient\HttpDispatcher;
 use FOS\HttpCacheBundle\DependencyInjection\Compiler\HashGeneratorPass;
+use FOS\HttpCacheBundle\Http\ResponseMatcher\ExpressionResponseMatcher;
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
@@ -53,6 +54,8 @@ public function load(array $configs, ContainerBuilder $container)
             $loader->load('cache_control_listener.xml');
         }
 
+        $this->loadCacheable($container, $config['cacheable']);
+
         if (!empty($config['cache_control'])) {
             $this->loadCacheControl($container, $config['cache_control']);
         }
@@ -131,6 +134,22 @@ public function load(array $configs, ContainerBuilder $container)
         }
     }
 
+    private function loadCacheable(ContainerBuilder $container, array $config)
+    {
+        $definition = $container->getDefinition($this->getAlias().'.response_matcher.cacheable');
+
+        // Change CacheableResponseMatcher to ExpressionResponseMatcher
+        if ($config['response']['expression']) {
+            $definition->setClass(ExpressionResponseMatcher::class)
+                ->setArguments([$config['response']['expression']]);
+        } else {
+            $container->setParameter(
+                $this->getAlias().'.cacheable.response.additional_status',
+                $config['response']['additional_status']
+            );
+        }
+    }
+
     /**
      * @param ContainerBuilder $container
      * @param array            $config
@@ -156,44 +175,14 @@ private function parseRuleMatcher(ContainerBuilder $container, array $match)
     {
         $match['ips'] = (empty($match['ips'])) ? null : $match['ips'];
 
-        $requestMatcher = $this->createRequestMatcher(
+        return $this->createRequestMatcher(
             $container,
             $match['path'],
             $match['host'],
             $match['methods'],
             $match['ips'],
             $match['attributes']
         );
-
-        $extraCriteria = [];
-        foreach (['additional_cacheable_status', 'match_response'] as $extra) {
-            if (isset($match[$extra])) {
-                $extraCriteria[$extra] = $match[$extra];
-            }
-        }
-
-        return $this->createRuleMatcher(
-            $container,
-            $requestMatcher,
-            $extraCriteria
-        );
-    }
-
-    private function createRuleMatcher(ContainerBuilder $container, Reference $requestMatcher, array $extraCriteria)
-    {
-        $arguments = [(string) $requestMatcher, $extraCriteria];
-        $serialized = serialize($arguments);
-        $id = $this->getAlias().'.rule_matcher.'.md5($serialized).sha1($serialized);
-
-        if (!$container->hasDefinition($id)) {
-            $container
-                ->setDefinition($id, new DefinitionDecorator($this->getAlias().'.rule_matcher'))
-                ->replaceArgument(0, $requestMatcher)
-                ->replaceArgument(1, $extraCriteria)
-            ;
-        }
-
-        return new Reference($id);
     }
 
     private function loadUserContext(ContainerBuilder $container, XmlFileLoader $loader, array $config)

src/EventListener/AbstractRuleListener.php

@@ -11,43 +11,41 @@
 
 namespace FOS\HttpCacheBundle\EventListener;
 
-use FOS\HttpCacheBundle\Http\RuleMatcherInterface;
 use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpFoundation\RequestMatcherInterface;
 
 abstract class AbstractRuleListener
 {
     /**
-     * @var array List of arrays with RuleMatcher, settings array
+     * @var array List of arrays with RequestMatcher, settings array
      */
     private $rulesMap = [];
 
     /**
      * Add a rule matcher with a list of header directives to apply if the
      * request and response are matched.
      *
-     * @param RuleMatcherInterface $ruleMatcher The headers apply to responses matched by this matcher
-     * @param array                $settings    An array of header configuration
+     * @param RequestMatcherInterface $requestMatcher The headers apply to responses matched by this matcher
+     * @param array                   $settings       An array of header configuration
      */
     public function addRule(
-        RuleMatcherInterface $ruleMatcher,
+        RequestMatcherInterface $requestMatcher,
         array $settings = []
     ) {
-        $this->rulesMap[] = [$ruleMatcher, $settings];
+        $this->rulesMap[] = [$requestMatcher, $settings];
     }
 
     /**
      * Return the settings for the current request if any rule matches.
      *
-     * @param Request  $request
-     * @param Response $response
+     * @param Request $request
      *
      * @return array|false Settings to apply or false if no rule matched
      */
-    protected function matchRule(Request $request, Response $response)
+    protected function matchRule(Request $request)
     {
         foreach ($this->rulesMap as $elements) {
-            if ($elements[0]->matches($request, $response)) {
+            if ($elements[0]->matches($request)) {
                 return $elements[1];
             }
         }