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 withs-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.

InHttpCache, a response fetched from the backend will only be stored when itisCacheable:

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 withs-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 sendings-maxage=0 and using the HttpCache (for example), you can keep the response body around in the cacheand get the (cheap) validation onevery 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 anETag orLast-Modified header.

If that is present,Response::isCacheable will already returntrue 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:

namespaceSymfony\Component\HttpKernel\Tests\HttpCache;class HttpCacheTest ...    publicfunctiontestServesResponseWhileFreshAndRevalidatesWithLastModifiedInformation()    {$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?