bug #23130 Keep s-maxage when expiry and validation are used in combination (mpdude)

This PR was merged into the 2.7 branch.

Discussion
----------

Keep s-maxage when expiry and validation are used in combination

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

(Symfony) docs say that [expiration wins over validation](https://symfony.com/doc/current/http_cache/validation.html). So,

a) when both the master and embedded response are public with an s-maxage, the result should be public as well and use the lower s-maxage of both, *also* in the case that the embedded response carries validation headers. (The cache may use those for revalidating the embedded response once it has become stale, but that does not impact expiration-based caching of the combined response.)

b) when both the master and embedded response are public with an s-maxage, the result should be public as well and use the lower s-maxage of both, *also* in the case that the master response carries validation headers. However, those *must not* be passed on to the client: They do not apply to the combined response, but may only be used by the cache itself to revalidate the (raw) master response.

Commits
-------

09bcbc70e7 Embedding a response that combines expiration and validation, that should not defeat expiration on the combined response
This commit is contained in:
Fabien Potencier 2017-06-14 15:21:38 -07:00
commit 3c2b1ff929
2 changed files with 45 additions and 1 deletions

View File

@ -39,7 +39,7 @@ class ResponseCacheStrategy implements ResponseCacheStrategyInterface
*/
public function add(Response $response)
{
if ($response->isValidateable() || !$response->isCacheable()) {
if (!$response->isFresh() || !$response->isCacheable()) {
$this->cacheable = false;
} else {
$maxAge = $response->getMaxAge();
@ -70,6 +70,9 @@ class ResponseCacheStrategy implements ResponseCacheStrategyInterface
if ($response->isValidateable()) {
$response->setEtag(null);
$response->setLastModified(null);
}
if (!$response->isFresh()) {
$this->cacheable = false;
}

View File

@ -178,4 +178,45 @@ class ResponseCacheStrategyTest extends TestCase
// Not sure if we should pass "max-age: 60" in this case, as long as the response is private and
// that's the more conservative of both the master and embedded response...?
}
public function testResponseIsExiprableWhenEmbeddedResponseCombinesExpiryAndValidation()
{
/* When "expiration wins over validation" (https://symfony.com/doc/current/http_cache/validation.html)
* and both the main and embedded response provide s-maxage, then the more restricting value of both
* should be fine, regardless of whether the embedded response can be validated later on or must be
* completely regenerated.
*/
$cacheStrategy = new ResponseCacheStrategy();
$masterResponse = new Response();
$masterResponse->setSharedMaxAge(3600);
$embeddedResponse = new Response();
$embeddedResponse->setSharedMaxAge(60);
$embeddedResponse->setEtag('foo');
$cacheStrategy->add($embeddedResponse);
$cacheStrategy->update($masterResponse);
$this->assertSame('60', $masterResponse->headers->getCacheControlDirective('s-maxage'));
}
public function testResponseIsExpirableButNotValidateableWhenMasterResponseCombinesExpirationAndValidation()
{
$cacheStrategy = new ResponseCacheStrategy();
$masterResponse = new Response();
$masterResponse->setSharedMaxAge(3600);
$masterResponse->setEtag('foo');
$masterResponse->setLastModified(new \DateTime());
$embeddedResponse = new Response();
$embeddedResponse->setSharedMaxAge(60);
$cacheStrategy->add($embeddedResponse);
$cacheStrategy->update($masterResponse);
$this->assertSame('60', $masterResponse->headers->getCacheControlDirective('s-maxage'));
$this->assertFalse($masterResponse->isValidateable());
}
}