bug #18164 [HttpKernel] set s-maxage only if all responses are cachea…

…ble (xabbuh)

This PR was merged into the 2.3 branch.

Discussion
----------

[HttpKernel] set s-maxage only if all responses are cacheable

| Q             | A
| ------------- | ---
| Branch        | 2.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #18160
| License       | MIT
| Doc PR        |

Commits
-------

b7d9338 set s-maxage only if all responses are cacheable

src/Symfony/Component/HttpKernel/HttpCache/EsiResponseCacheStrategy.php

@@ -32,6 +32,7 @@ class EsiResponseCacheStrategy implements EsiResponseCacheStrategyInterface
     private $embeddedResponses = 0;
     private $ttls = array();
     private $maxAges = array();
+    private $isNotCacheableResponseEmbedded = false;
 
     /**
      * {@inheritdoc}
@@ -41,8 +42,13 @@ public function add(Response $response)
         if ($response->isValidateable()) {
             $this->cacheable = false;
         } else {
+            $maxAge = $response->getMaxAge();
             $this->ttls[] = $response->getTtl();
-            $this->maxAges[] = $response->getMaxAge();
+            $this->maxAges[] = $maxAge;
+
+            if (null === $maxAge) {
+                $this->isNotCacheableResponseEmbedded = true;
+            }
         }
 
         ++$this->embeddedResponses;
@@ -76,7 +82,9 @@ public function update(Response $response)
         $this->ttls[] = $response->getTtl();
         $this->maxAges[] = $response->getMaxAge();
 
-        if (null !== $maxAge = min($this->maxAges)) {
+        if ($this->isNotCacheableResponseEmbedded) {
+            $response->headers->removeCacheControlDirective('s-maxage');
+        } elseif (null !== $maxAge = min($this->maxAges)) {
             $response->setSharedMaxAge($maxAge);
             $response->headers->set('Age', $maxAge - min($this->ttls));
         }

src/Symfony/Component/HttpKernel/Tests/HttpCache/EsiResponseCacheStrategyTest.php

@@ -0,0 +1,77 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * This code is partially based on the Rack-Cache library by Ryan Tomayko,
+ * which is released under the MIT license.
+ * (based on commit 02d2b48d75bcb63cf1c0c7149c077ad256542801)
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpKernel\Tests\HttpCache;
+
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\HttpCache\EsiResponseCacheStrategy;
+
+class EsiResponseCacheStrategyTest extends \PHPUnit_Framework_TestCase
+{
+    public function testMinimumSharedMaxAgeWins()
+    {
+        $cacheStrategy = new EsiResponseCacheStrategy();
+
+        $response1 = new Response();
+        $response1->setSharedMaxAge(60);
+        $cacheStrategy->add($response1);
+
+        $response2 = new Response();
+        $response2->setSharedMaxAge(3600);
+        $cacheStrategy->add($response2);
+
+        $response = new Response();
+        $response->setSharedMaxAge(86400);
+        $cacheStrategy->update($response);
+
+        $this->assertSame('60', $response->headers->getCacheControlDirective('s-maxage'));
+    }
+
+    public function testSharedMaxAgeNotSetIfNotSetInAnyEmbeddedRequest()
+    {
+        $cacheStrategy = new EsiResponseCacheStrategy();
+
+        $response1 = new Response();
+        $response1->setSharedMaxAge(60);
+        $cacheStrategy->add($response1);
+
+        $response2 = new Response();
+        $cacheStrategy->add($response2);
+
+        $response = new Response();
+        $response->setSharedMaxAge(86400);
+        $cacheStrategy->update($response);
+
+        $this->assertFalse($response->headers->hasCacheControlDirective('s-maxage'));
+    }
+
+    public function testSharedMaxAgeNotSetIfNotSetInMasterRequest()
+    {
+        $cacheStrategy = new EsiResponseCacheStrategy();
+
+        $response1 = new Response();
+        $response1->setSharedMaxAge(60);
+        $cacheStrategy->add($response1);
+
+        $response2 = new Response();
+        $response2->setSharedMaxAge(3600);
+        $cacheStrategy->add($response2);
+
+        $response = new Response();
+        $cacheStrategy->update($response);
+
+        $this->assertFalse($response->headers->hasCacheControlDirective('s-maxage'));
+    }
+}