Merge pull request #151 from FriendsOfSymfony/gv-allow-cache-overwrite

Allow override of request cache headers
FriendsOfSymfony · Oct 21, 2014 · 3a21418 · 3a21418
2 parents 7321b95 + 233420c
commit 3a21418
Show file tree

Hide file tree

Showing 11 changed files with 178 additions and 42 deletions.
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -148,6 +148,15 @@ private function addCacheControlSection(ArrayNodeDefinition $rootNode)
                 ->arrayNode('cache_control')
                     ->fixXmlConfig('rule')
                     ->children()
+                        ->arrayNode('defaults')
+                            ->addDefaultsIfNotSet()
+                            ->children()
+                                ->booleanNode('overwrite')
+                                    ->info('Whether to overwrite existing cache headers')
+                                    ->defaultFalse()
+                                ->end()
+                            ->end()
+                        ->end()
                         ->arrayNode('rules')
                             ->prototype('array')
                                 ->children();
@@ -158,6 +167,11 @@ private function addCacheControlSection(ArrayNodeDefinition $rootNode)
                 ->isRequired()
                 // todo validate there is some header defined
                 ->children()
+                    ->enumNode('overwrite')
+                        ->info('Whether to overwrite cache headers for this rule, defaults to the cache_control.defaults.overwrite setting')
+                        ->values(array('default', true, false))
+                        ->defaultValue('default')
+                    ->end()
                     ->arrayNode('cache_control')
                         ->info('Add the specified cache control directives.')
                         ->children()

diff --git a/DependencyInjection/FOSHttpCacheExtension.php b/DependencyInjection/FOSHttpCacheExtension.php
@@ -108,8 +108,12 @@ private function loadCacheControl(ContainerBuilder $container, array $config)
 
         foreach ($config['rules'] as $rule) {
             $ruleMatcher = $this->parseRuleMatcher($container, $rule['match']);
-            $controlDefinition->addMethodCall('addRule', array($ruleMatcher, $rule['headers']))
-            ;
+
+            if ('default' === $rule['headers']['overwrite']) {
+                $rule['headers']['overwrite'] = $config['defaults']['overwrite'];
+            }
+
+            $controlDefinition->addMethodCall('addRule', array($ruleMatcher, $rule['headers']));
         }
     }
 

diff --git a/EventListener/CacheControlSubscriber.php b/EventListener/CacheControlSubscriber.php
@@ -112,10 +112,10 @@ public function onKernelResponse(FilterResponseEvent $event)
                 $directives = array_intersect_key($options['cache_control'], $this->supportedDirectives);
                 $extraDirectives = array_diff_key($options['cache_control'], $directives);
                 if (!empty($directives)) {
-                    $this->setCache($response, $directives);
+                    $this->setCache($response, $directives, $options['overwrite']);
                 }
                 if (!empty($extraDirectives)) {
-                    $this->setExtraCacheDirectives($response, $extraDirectives);
+                    $this->setExtraCacheDirectives($response, $extraDirectives, $options['overwrite']);
                 }
             }
 
@@ -127,17 +127,32 @@ public function onKernelResponse(FilterResponseEvent $event)
             }
 
             if (!empty($options['vary'])) {
-                $response->setVary(array_merge($response->getVary(), $options['vary']), true); //update if already has vary
+                $response->setVary($options['vary'], $options['overwrite']);
             }
 
-            if (isset($options['last_modified']) && null === $response->getLastModified()) {
+            if (isset($options['last_modified'])
+                && ($options['overwrite'] || null === $response->getLastModified())
+            ) {
                 $response->setLastModified(new \DateTime($options['last_modified']));
             }
         }
     }
 
-    private function setCache(Response $response, array $directives)
+    /**
+     * Set cache headers on response.
+     *
+     * @param Response $response
+     * @param array    $directives
+     * @param boolean  $overwrite  Whether to keep existing cache headers or to overwrite them.
+     */
+    private function setCache(Response $response, array $directives, $overwrite)
     {
+        if ($overwrite) {
+            $response->setCache($directives);
+
+            return;
+        }
+
         if ('no-cache' === $response->headers->get('cache-control')) {
             // this single header is set by default. if its the only thing, we override it.
             $response->setCache($directives);
@@ -165,16 +180,17 @@ private function setCache(Response $response, array $directives)
      *
      * @param Response $response
      * @param array    $controls
+     * @param boolean  $overwrite Whether to keep existing cache headers or to overwrite them.
      */
-    private function setExtraCacheDirectives(Response $response, array $controls)
+    private function setExtraCacheDirectives(Response $response, array $controls, $overwrite)
     {
         $flags = array('must_revalidate', 'proxy_revalidate', 'no_transform', 'no_cache');
         $options = array('stale_if_error', 'stale_while_revalidate');
 
         foreach ($flags as $key) {
             $flag = str_replace('_', '-', $key);
             if (!empty($controls[$key])
-                && !$response->headers->hasCacheControlDirective($flag)
+                && ($overwrite || !$response->headers->hasCacheControlDirective($flag))
             ) {
                 $response->headers->addCacheControlDirective($flag);
             }
@@ -183,9 +199,10 @@ private function setExtraCacheDirectives(Response $response, array $controls)
         foreach ($options as $key) {
             $option = str_replace('_', '-', $key);
             if (isset($controls[$key])
-                && !$response->headers->hasCacheControlDirective($option)
+                && ($overwrite || !$response->headers->hasCacheControlDirective($option))
             ) {
                 $response->headers->addCacheControlDirective($option, $controls[$key]);
+
             }
         }
     }

diff --git a/Resources/doc/features/headers.rst b/Resources/doc/features/headers.rst
@@ -11,13 +11,21 @@ Set caching headers under the ``cache_control`` configuration section,
 which consists of a set of rules. When the request matches all criteria under
 ``match``, the headers under ``headers`` will be set on the response.
 
+A Response may already have cache headers set, e.g. by the controller method.
+By default, the options that already exist are not overwritten, but additional
+headers are added. You can force to overwrite the headers globally by setting
+``cache_control.defaults.overwrite: true`` to true, or on a per rule basis with
+``overwrite: true`` under ``headers``.
+
 For instance:
 
 .. code-block:: yaml
 
     # app/config/config.yml
     fos_http_cache:
         cache_control:
+            defaults:
+                overwrite: true
             rules:
                 # only match login.example.com
                 -
@@ -48,8 +56,9 @@ For instance:
                 # match everything to set defaults
                 -
                     match:
-                      path: ^/
+                        path: ^/
                     headers:
+                        overwrite: false
                         cache_control: { public: true, max_age: 15, s_maxage: 30 }
                         last_modified: "-1 hour"
 

diff --git a/Resources/doc/reference/configuration/headers.rst b/Resources/doc/reference/configuration/headers.rst
@@ -6,17 +6,23 @@ parameters described in the ``match`` section, the headers as defined under
 ``headers`` will be set on the response, if they are not already set. Rules are
 checked in the order specified, where the first match wins.
 
+A global setting and a per rule ``overwrite`` option allow to overwrite the
+cache headers even if they are already set.
+
 .. code-block:: yaml
 
     # app/config/config.yml
     fos_http_cache:
         cache_control:
+            defaults:
+                overwrite: false
             rules:
                 # only match login.example.com
                 -
                     match:
                         host: ^login.example.com$
                     headers:
+                        overwrite: true
                         cache_control:
                             public: false
                             max_age: 0

diff --git a/Tests/Resources/Fixtures/config/full.php b/Tests/Resources/Fixtures/config/full.php
@@ -2,6 +2,9 @@
 
 $container->loadFromExtension('fos_http_cache', array(
     'cache_control' => array(
+        'defaults' => array(
+            'overwrite' => true
+        ),
         'rules' => array(
             array(
                 'match' => array(
@@ -13,6 +16,7 @@
                     'additional_cacheable_status' => array(100, 500),
                 ),
                 'headers' => array(
+                    'overwrite' => false,
                     'cache_control' => array(
                         'max_age' => 1,
                         's_maxage' => 2,

diff --git a/Tests/Resources/Fixtures/config/full.xml b/Tests/Resources/Fixtures/config/full.xml
@@ -3,6 +3,9 @@
 
     <config xmlns="http://example.org/schema/dic/fos_http_cache">
         <cache-control>
+            <defaults>
+                <overwrite>true</overwrite>
+            </defaults>
             <rule>
                 <match
                     path="/abc"
@@ -17,6 +20,7 @@
                     <additional-cacheable-status>500</additional-cacheable-status>
                 </match>
                 <headers last-modified="-1 hour" reverse-proxy-ttl="42">
+                    <overwrite>false</overwrite>
                     <cache-control
                         max-age="1"
                         s-maxage="2"

diff --git a/Tests/Resources/Fixtures/config/full.yml b/Tests/Resources/Fixtures/config/full.yml
@@ -1,6 +1,8 @@
 fos_http_cache:
 
     cache_control:
+        defaults:
+            overwrite: true
         rules:
             -
                 match:
@@ -18,6 +20,7 @@ fos_http_cache:
                         - 100
                         - 500
                 headers:
+                    overwrite: false
                     cache_control:
                         max_age: 1
                         s_maxage: 2

diff --git a/Tests/Unit/DependencyInjection/ConfigurationTest.php b/Tests/Unit/DependencyInjection/ConfigurationTest.php
@@ -49,6 +49,9 @@ public function testSupportsAllConfigFormats()
     {
         $expectedConfiguration = array(
             'cache_control' => array(
+                'defaults' => array(
+                    'overwrite' => true,
+                ),
                 'rules' => array(
                     array(
                         'match' => array(
@@ -62,6 +65,7 @@ public function testSupportsAllConfigFormats()
                             // TODO 'match_response' => '',
                         ),
                         'headers' => array(
+                            'overwrite' => false,
                             'cache_control' => array(
                                 'max_age' => 1,
                                 's_maxage' => 2,
@@ -220,9 +224,13 @@ public function testSplitOptions()
                     'headers' => array(
                         'reverse_proxy_ttl' => null,
                         'vary' => array('Cookie', 'Authorization'),
+                        'overwrite' => 'default',
                     ),
                 ),
             ),
+            'defaults' => array(
+                'overwrite' => false,
+            ),
         );
         $expectedConfiguration['proxy_client'] = array(
             'varnish' => array(