Improve X-Drupal-Cache and X-Drupal-Dynamic-Cache headers, even for responses that are not cacheable

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

claudiu.cristea → made their first commit to this issue’s fork.

Updated the change record to match the MR.

Narrowed from 268 to 8 failures.

Moving to NW for the failures.

This would be fantastic to ship at last! 🤞

Needs a new MR against 11.x. Let's finally land this — we now have fantastic new tooling such as StandardPerformanceTest (thanks to 📌 Allow assertions on the number of database queries run during tests RTBC and many follow-ups) that this would be a valuable addition to, especially for real-world sites. Just today, I helped out a colleague in Acquia's Professional Services department, and it'd have been far simpler to narrow down root causes if this would've existed. 📌 Log every individual query in performance tests Needs work landed earlier today too, so I'm convinced we can do this one as well! 😊

I have rerolled to 11.x and have brought down the test failure to 6 :), please review the reroll and suggest path forward to fix the remaining test failures.
Thanks !

Believe the failures are showing that the solution needs to be relooked at.

All test Passed , moving to review !

Hiding patches and old MRs for clarity.

smustgrave → changed the visibility of the branch 2951814-cache-header-10.1.x to hidden.

smustgrave → changed the visibility of the branch 2951814-always-set-x-drupal-cache to hidden.

Updated CR version

Updated issue summary to include release notes, as I think this is a good highlight.

Believe the reroll is correct.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Back to review \o/

Even if this gets merged, this other issue still remains relevant.

Ran test-only feature https://git.drupalcode.org/issue/drupal-2951814/-/jobs/1740771 which shows test coverage

CR is well written with the examples

+1 from me

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Back to RTBC after rebase.

Meh... ofc simple rebase wasn't enough

Drupal\Tests\jsonapi\Functional\UserTest::testGetIndividual
Failed asserting that false is identical to true.

core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php:1030

This should be also caused by the recently merged https://git.drupalcode.org/project/drupal/-/commit/9fc1bc9ac5a785ebf6e88......

Rebase seems good.

(rebased)

This will further improve and simplify cache debugging! Thank you all very much for your hard work on this. Very much looking forward to this functionality!

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Gitlab says MR still applies, back to RTBC.

Checking GL, it does say "Merge conflicts must be resolved." so think this needs another rebase.

mxr576 → changed the visibility of the branch drupal-2951814 to hidden.

mxr576 → changed the visibility of the branch 2951814-always-set-x-drupal-cache to active.

Indeed... :sadpanda: Rebasing again... PHPStorm could auto-resolve all :fingers-crossed:.

 git rebase origin/11.x
Auto-merging core/modules/basic_auth/tests/src/Functional/BasicAuthTest.php
Auto-merging core/modules/jsonapi/tests/src/Functional/NodeTest.php
CONFLICT (content): Merge conflict in core/modules/jsonapi/tests/src/Functional/NodeTest.php
Auto-merging core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php
CONFLICT (content): Merge conflict in core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php
Auto-merging core/modules/layout_builder/tests/src/Functional/LayoutSectionTest.php
Auto-merging core/modules/node/tests/src/Functional/NodeBlockFunctionalTest.php
Auto-merging core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
error: could not apply d8f6085835... Applying https://www.drupal.org/files/issues/2021-02-24/2951814-85.patch
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply d8f6085835... Applying https://www.drupal.org/files/issues/2021-02-24/2951814-85.patch

mxr576 → changed the visibility of the branch 2951814-always-set-x-drupal-cache to hidden.

mxr576 → changed the visibility of the branch drupal-2951814 to active.

believe rebase is good.

Rebased the forked branch ~~ RTBC++

Rebased, there was only one conflict in a test file that PHPStorm could autoresolve easily.

❯ git rebase origin/11.x
Auto-merging core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php
Auto-merging core/modules/language/tests/src/Functional/LanguageBrowserDetectionAcceptLanguageTest.php
Auto-merging core/modules/page_cache/tests/src/Functional/PageCacheTest.php
CONFLICT (content): Merge conflict in core/modules/page_cache/tests/src/Functional/PageCacheTest.php
Auto-merging core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
Auto-merging core/modules/rest/tests/src/Functional/ResourceTestBase.php
Auto-merging core/modules/system/tests/src/Functional/Session/SessionTest.php
Auto-merging core/modules/system/tests/src/Functional/System/ErrorHandlerTest.php
Auto-merging core/modules/user/tests/src/Functional/UserPasswordResetTest.php

May I recommend this feature to 10.4.0 release notes?

This would certainly have saved me hours of debugging. Perhaps this is a bridge too far, but it would also help to identify which modules are adding which tags/contexts.

Updated issue credit. Removed credit for partial rerolls and tried to credit everyone who contributed test fixing & comments that help direct the work.

Committed 19f80f6 and pushed to 11.x. Thanks!

Let's backport this to 10.4.x

Created the backport MR - I wanted a full test run. There were no merge conflicts so I'm going to set to RTBC in the hope it is green. If it is, I will cherry-pick the 11.x commit to 10.4.x

Automatically closed - issue fixed for 2 weeks with no activity.