[HttpKernel] Make #[Cache] respect all explicit cache directives set in controller

Author: ayyoub-afwallah

EventListener/CacheAttributeListener.php

@@ -13,6 +13,7 @@
 
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
+use Symfony\Component\HttpFoundation\HeaderBag;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Attribute\Cache;
@@ -126,29 +127,40 @@ public function onKernelResponse(ResponseEvent $event)
         // Check if the response has a Vary header that should be considered, ignoring cases where
         // it's only 'Accept-Language' and the request has the '_vary_by_language' attribute
         $hasVary = ['Accept-Language'] === $response->getVary() ? !$request->attributes->get('_vary_by_language') : $response->hasVary();
+        //Check if cache-control directive was set manually in cacheControl (not auto computed)
+        $hasCacheControlDirective = new class($response->headers) extends HeaderBag {
+            public function __construct(private parent $headerBag)
+            {
+            }
+
+            public function __invoke(string $key): bool
+            {
+                return \array_key_exists($key, $this->headerBag->cacheControl);
+            }
+        };
 
         foreach (array_reverse($attributes) as $cache) {
-            if (null !== $cache->smaxage && !$response->headers->hasCacheControlDirective('s-maxage')) {
+            if (null !== $cache->smaxage && !$hasCacheControlDirective('s-maxage')) {
                 $response->setSharedMaxAge($this->toSeconds($cache->smaxage));
             }
 
             if ($cache->mustRevalidate) {
                 $response->headers->addCacheControlDirective('must-revalidate');
             }
 
-            if (null !== $cache->maxage && !$response->headers->hasCacheControlDirective('max-age')) {
+            if (null !== $cache->maxage && !$hasCacheControlDirective('max-age')) {
                 $response->setMaxAge($this->toSeconds($cache->maxage));
             }
 
-            if (null !== $cache->maxStale && !$response->headers->hasCacheControlDirective('max-stale')) {
+            if (null !== $cache->maxStale && !$hasCacheControlDirective('max-stale')) {
                 $response->headers->addCacheControlDirective('max-stale', $this->toSeconds($cache->maxStale));
             }
 
-            if (null !== $cache->staleWhileRevalidate && !$response->headers->hasCacheControlDirective('stale-while-revalidate')) {
+            if (null !== $cache->staleWhileRevalidate && !$hasCacheControlDirective('stale-while-revalidate')) {
                 $response->headers->addCacheControlDirective('stale-while-revalidate', $this->toSeconds($cache->staleWhileRevalidate));
             }
 
-            if (null !== $cache->staleIfError && !$response->headers->hasCacheControlDirective('stale-if-error')) {
+            if (null !== $cache->staleIfError && !$hasCacheControlDirective('stale-if-error')) {
                 $response->headers->addCacheControlDirective('stale-if-error', $this->toSeconds($cache->staleIfError));
             }
 
@@ -161,12 +173,14 @@ public function onKernelResponse(ResponseEvent $event)
             }
         }
 
+        $hasPublicOrPrivateCacheControlDirective = $hasCacheControlDirective('public') || $hasCacheControlDirective('private');
+
         foreach ($attributes as $cache) {
-            if (true === $cache->public) {
+            if (true === $cache->public && !$hasPublicOrPrivateCacheControlDirective) {
                 $response->setPublic();
             }
 
-            if (false === $cache->public) {
+            if (false === $cache->public && !$hasPublicOrPrivateCacheControlDirective) {
                 $response->setPrivate();
             }
         }

Tests/EventListener/CacheAttributeListenerTest.php

@@ -336,6 +336,30 @@ public function testHasRelevantVaryHeaderBehavior(array $responseVary, array $ca
         $this->assertSame($expectedVary, $response->getVary());
     }
 
+    public function testAttributeRespectsExplicitPrivateFromController()
+    {
+        $request = $this->createRequest(new Cache(public: true));
+        $response = new Response();
+        $response->setPrivate();
+
+        $this->listener->onKernelResponse($this->createEventMock($request, $response));
+
+        $this->assertTrue($response->headers->hasCacheControlDirective('private'));
+        $this->assertFalse($response->headers->hasCacheControlDirective('public'));
+    }
+
+    public function testAttributeRespectsExplicitPublicFromController()
+    {
+        $request = $this->createRequest(new Cache(public: false));
+        $response = new Response();
+        $response->setPublic();
+
+        $this->listener->onKernelResponse($this->createEventMock($request, $response));
+
+        $this->assertTrue($response->headers->hasCacheControlDirective('public'));
+        $this->assertFalse($response->headers->hasCacheControlDirective('private'));
+    }
+
     public static function provideVaryHeaderScenarios(): \Traversable
     {
         yield 'no vary headers at all' => [