NodeController::revisionOverview is uncacheable

Came here from Bug smash initiative. I am still able to replicate this issue on D11 and tried initial level of debugging.

Wondering why CacheableMetadata::createFromRenderArray($username)->addCacheableDependency($node) or CacheableMetadata::createFromRenderArray($username) are not working as expected. Checked type of argument that i am passing and also looked into multiple other ways of defining it, still x-drupal-dynamic-cache is not coming on response headers.

Might need some more insights here from the community.

Might need some more insights here from the community, thats why moving it to Needs review status.

Is it designed that way or needs fix ? note i am aware of related issue https://www.drupal.org/project/drupal/issues/3232018 → and from my POV possibly we need to take some action on this issue, since it is affecting in many more pages with x-drupal-dynamic-cache: UNCACHEABLE in response headers.

This doesn't need more info, the bug has been reproduced in #32 and also previously, it does need a conversion to an MR though at least.

By default, x-drupal-dynamic-cache header is present when revision oveview page is accessed and its value is 'UNCACHEABLE'.

Now, If revision overview page is accessed after applying patch #17, x-drupal-dynamic-cache header wont be there. This is due tho the following condition in onResponse() method of DynamicPageCacheSubscriber.

    // Don't cache the response if the Dynamic Page Cache request & response
    // policies are not met.
    // @see onRequest()
    if ($this->requestPolicyResults[$request] === RequestPolicyInterface::DENY || $this->responsePolicy->check($response, $request) === ResponsePolicyInterface::DENY) {
      return;
    }

$this->responsePolicy->check() calls several policy rules and Drupal\dynamic_page_cache\PageCache\ResponsePolicy\DenyAdminRoutes rule returns 'deny' for revison overview route as its an admin route. (_admin_route: true). So x-drupal-dynamic-cache header is omitted.

Rebased against 11.x, removed the ->addCacheableDependency($node) addition (out of scope), and moved testing to a dedicated test case since we need to disable the use_admin_theme setting.

1) Drupal\Tests\node\Functional\NodeRevisionsUiTest::testNodeRevisionsCacheability
Behat\Mink\Exception\ExpectationException: Current response header "X-Drupal-Dynamic-Cache" is "UNCACHEABLE (poor cacheability)", but "MISS" expected.
/builds/issue/drupal-3227637/vendor/behat/mink/src/WebAssert.php:888
/builds/issue/drupal-3227637/vendor/behat/mink/src/WebAssert.php:180
/builds/issue/drupal-3227637/core/tests/Drupal/Tests/WebAssert.php:969
/builds/issue/drupal-3227637/core/modules/node/tests/src/Functional/NodeRevisionsUiTest.php:229
FAILURES!
Tests: 5, Assertions: 52, Failures: 1.

Shows test coverage

Change seems straight forward and gone through a number of reviews. Believe feedback has been addressed.

Committed and pushed 44f56c2c1b8 to 11.x and 371936c49aa to 11.1.x. Thanks!