Introduce "Vary" page cache response policy

Regrettably this approach clearly violates the ResponsePolicyInterface contract. While not stated explicitly in the documentation, the check() method is certainly not expected to actually change the response.

Rerolled patch for Drupal core 8.7.6

> Site needs to display local sales office to people depending on their originating country. Many CDN providers (Cloudflare, Akamai etc.) sent this information in HTTP header.

Afaik this was the original path during Drupal 8 development, but it was replaced by the current approach of using URLS and the support fro the `_format` query parameters. This was done since there is only bad or hardly proper support for the Vary http headers across proxies and CDNs, so using the query parameters makes the url is different and cachable.

I guess that decision could be re-evaluated now, since a couple of years passed, but that's the status quo.

+++ b/core/modules/page_cache/src/StackMiddleware/PageCache.php
--- /dev/null
+++ b/core/modules/page_cache/src/StackMiddleware/PageCache.php.rej

I think this file made it into the latest patch on accident.

Rerolled patch #7 on Drupal 9.1.x and addressed comment #10.

Tagging novice for the minor test fixes for the failures in https://www.drupal.org/pift-ci-job/1850261.

A couple of questions, not necessarily novice their answers.

1. +++ b/core/lib/Drupal/Core/PageCache/ResponsePolicy/Vary.php
   @@ -0,0 +1,50 @@
   +  public function add($header) {
   +    if (!in_array($header, $this->vary)) {
   +      $this->vary[] = $header;
   ...
   +      $response->setVary($this->vary);
   

   setVary has a $replace flag available to it. Should we expose that on the interface for add? Which leads to the next question, do we need to have an interface for this Vary class, or is that not necessary?

2. +++ b/core/modules/page_cache/src/StackMiddleware/PageCache.php
   @@ -334,6 +348,14 @@ protected function get(Request $request, $allow_invalid = FALSE) {
   +    $vary = $response->getVary();
   +    if ($vary) {
   

   There is a hasVary method on the response. We should use it.

Test fixes of #11.

Tested #15 on 9.1, 'Vary' page cache response has been added

Drupal core 9.2.3 and Composer 2.1.5.

I run "composer update" after add the patch into composer.json, but I got an error:

Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2020-10-16/3023104-15_0.patch                                                                                                                     
  [Exception]                                                                                                                               
  Cannot apply patch Introduce Vary page cache response policy for D9 (https://www.drupal.org/files/issues/2020-10-16/3023104-15_0.patch)!  

In my composer.json file

"drupal/core": {
    ... ...,
   "Introduce Vary page cache response policy for D9": "https://www.drupal.org/files/issues/2020-10-16/3023104-15_0.patch"
}

Thank you very much!

Re-rolled #15 for 9.3

Re-rolled patch #21, Fixed custom command failed. Added interdiff for same.

Fixed the fail test, Please have a look.

Questions/comments in #14 still not addressed.

As the author of the

ResponsePolicyInterface

, I'd like to point out that #4 is not addressed neither. This approach is invalid, please try to find a different extension point to solve the issue.

Contrib blocker, see: #3222748-30: Create a page if given internal path is not valid

Addressed point #14.2 of comment #14, keeping the status needs work for comment #14.1.

Any update on this issue?
Will this be included in core?

#2430335: Browser language detection is not cache aware has been updated to use a responsesubscriber that allows setting Vary headers. Wonder if those changes also need partial split into this issue?

From #4...

Regrettably this approach clearly violates the ResponsePolicyInterface contract. While not stated explicitly in the documentation, the check() method is certainly not expected to actually change the response.

I don't think this is resolved with the current approaches still, as suggested in #26.
I have solved this in my own project with an event subscriber for KernelEvents::RESPONSE. I think that would be a relatively simple change to make here. Would that address the concerns of #4?

Sorry @bbrala - this is nearly the same thing you are suggesting as well... but I do think that issue #2430335 would do most of what this issue is trying to do from what I can see.

As commented in https://www.drupal.org/project/drupal/issues/2972483, I've developed the module page_cache_vary until the issue is fixed in the core page_cache module.

It can retrieve cache vary headers by caching them per URL, so that the cost of computing the vary headers is as minimal as possible.

If this solution is good enough, I can try to integrate it directly in Drupal, let me know what you think.

Talked with catch avout a possible angle of attack which would make this viable (possobel) from a variation and perdormance perspective. Will post that sometime soon.

See here: https://www.drupal.org/project/drupal/issues/2972483 a module that might help in the meantime: https://www.drupal.org/project/page_cache_vary