Introducing a new method is a new feature, changing a behavior a "bc break". This PR does both but is classified as bug fix. Triggers a red flag to me, needs to be well discussed/justified.

Regarding the method, it could be private as well.

Regarding changing behavior, every bug fix is a BC break 🙈

isCacheable now ignores the ttl. I agree with @nicolas-grekas for the fact that it needs a discussion.

You're right, I missed that...

But do you agree that a it would be fine to cache a response with s-maxage=0 as long as it will immediately (i.e. on the next request) be revalidated? In other words, even a response that is stale might be worth caching because it might successfully revalidate later on.

In HttpCache, a response fetched from the backend will only be stored when it isCacheable:

https://github.com/symfony/symfony/blob/6b4cfd6/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php#L450

When looked up in the cache, here's the check whether the response is fresh enough:

https://github.com/symfony/symfony/blob/6b4cfd6/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php#L347

I am not sure if I get the root issue. What exactly is the point of caching a response with s-maxage being 0? Why would you want to cache a response if you need to fetch it again with the next incoming request anyway?

It may be expensive for your Controller to generate the complete response, but an If-Modified-Since check may be cheap.

By sending s-maxage=0 and using the HttpCache (for example), you can keep the response body around in the cache and get the (cheap) validation on every subsequent request.

Why's the test failing on PHP 7 😒 ?

You would have to tell the HttpKernel component to make use of version ^2.8.19 of the HttpFoundation component.

I missed that in order to make revalidation possible, the response needs an ETag or Last-Modified header.

If that is present, Response::isCacheable will already return true without even considering the TTL. So, the response will be cached and – as TTL will be expired next time it is looked at – revalidation be performed. 👍

In case you'd be interested, here's a test to safeguard this:

namespace Symfony\Component\HttpKernel\Tests\HttpCache;

class HttpCacheTest ...

    public function testServesResponseWhileFreshAndRevalidatesWithLastModifiedInformation()
    {
        $time = \DateTime::createFromFormat('U', time());

        $this->setNextResponse(200, [], 'Hello World', function (Request $request, Response $response) use ($time) {
            $response->setSharedMaxAge(10);
            $response->headers->set('Last-Modified', $time->format(DATE_RFC2822));
        });

        // prime the cache
        $this->request('GET', '/');

        // next request before s-maxage has expired: Serve from cache without hitting the backend
        $this->request('GET', '/');
        $this->assertHttpKernelIsNotCalled();
        $this->assertEquals(200, $this->response->getStatusCode());
        $this->assertEquals('Hello World', $this->response->getContent());
        $this->assertTraceContains('fresh');

        sleep(15); // expire the cache

        $this->setNextResponse(304, [], '', function (Request $request, Response $response) use ($time) {
            $this->assertEquals($time->format(DATE_RFC2822), $request->headers->get('IF_MODIFIED_SINCE'));
        });

        $this->request('GET', '/');
        $this->assertHttpKernelIsCalled();
        $this->assertEquals(200, $this->response->getStatusCode());
        $this->assertEquals('Hello World', $this->response->getContent());
        $this->assertTraceContains('stale');
        $this->assertTraceContains('valid');
    }

@mpdude Can you submit a PR with just this test then?