fix custom ttl with symfony HttpCache to work regardless of s-maxage

[dbu]

fix #576

[dbu]

when i only add this, the test starts failing because write is never called.

[alexander-schranz]

For reference here I added a comment to: sulu/sulu#7441 (comment) which also is little bit related to new flag CustomTtlListener(fallbackToSmaxage: false) from #575

Copy of the comment: so changes here are understandable in future:

The only part which we may need to recheck is the CustomTtlListener makes now private requests always public, specially in case of $fallbackToSmaxAge = false.

If we want that private responses are never cached also when CustomTtlListener is used we would require:

diff --git a/src/SymfonyCache/CustomTtlListener.php b/src/SymfonyCache/CustomTtlListener.php
index 4be17d9..9ff303e 100644
--- a/src/SymfonyCache/CustomTtlListener.php
+++ b/src/SymfonyCache/CustomTtlListener.php
@@ -77,12 +77,18 @@ class CustomTtlListener implements EventSubscriberInterface
             ? $response->headers->getCacheControlDirective('s-maxage')
             : 'false'
         ;
+
         $response->headers->set(static::SMAXAGE_BACKUP, $backup);
+        $isPrivate = $response->headers->getCacheControlDirective('private');
+        // calling setTtl will make a response public, so we need to set it back to private if it was private before
         $response->setTtl(
             $response->headers->has($this->ttlHeader)
                 ? $response->headers->get($this->ttlHeader)
                 : 0
         );
+        if ($isPrivate) {
+            $response->setPrivate();
+        }
     }

     /**

Or alternative us addCacheControlDirective instead of setTtl:

diff --git a/src/SymfonyCache/CustomTtlListener.php b/src/SymfonyCache/CustomTtlListener.php
index 4be17d9..31126f3 100644
--- a/src/SymfonyCache/CustomTtlListener.php
+++ b/src/SymfonyCache/CustomTtlListener.php
@@ -77,8 +77,11 @@ class CustomTtlListener implements EventSubscriberInterface
             ? $response->headers->getCacheControlDirective('s-maxage')
             : 'false'
         ;
+
         $response->headers->set(static::SMAXAGE_BACKUP, $backup);
-        $response->setTtl(
+
+        $response->headers->addCacheControlDirective(
+            's-maxage',
             $response->headers->has($this->ttlHeader)
                 ? $response->headers->get($this->ttlHeader)
                 : 0

What do you think would be expected behaviour for:

$response = new Response();
$response->setPrivate(0);
$response->header->set('X-Reverse-Proxy-TTL', 1200);

Current version (2.15.3) would not cache it. With #577 and CustomTtlListener(fallbackToSmaxage: false) it will now be cached. But I think explicit set private responses should not be cached even when a custom ttl is set? As example private response with a s-maxage defined is also not cached.

[dbu]

For reference here I added a comment to: sulu/sulu#7441 (comment) which also is little bit related to new flag CustomTtlListener(fallbackToSmaxage: false) from #575

uh, very good catch! i change the code to use setCacheControlDirective. i guess until now this mistake was hidden by the listener not being called at all for a private response. i think we should keep that behaviour.

indeed targeted cache control would be very useful to avoid this mess: symfony/symfony#47288 (we could also provide that functionality with FOSHttpCache rules, but first should try to do the pull request to core symfony)

[alexander-schranz]

@dbu retested the change and looks good for me.