Resolve #2973356 "Route access check cacheability ignored"
Closes #2973356
Merge request reports
Activity
mentioned in merge request !8198 (closed)
33 33 * Tests dynamic page cache. 34 34 */ 35 35 public function testDynamicPageCache(): void { 36 $node_type = $this->drupalCreateContentType(); 37 $node = $this->drupalCreateNode(['type' => $node_type->id()]); 36 38 $this->drupalLogin($this->drupalCreateUser([ 37 39 'access toolbar', 38 40 'access announcements', 39 41 ])); 40 // Front-page is visited right after login. This was an invalid expectation because on default installations the user lands on the
user/[id]
after login and that hasuser
cache context now bubbling up from\Drupal\user\UserAccessControlHandler::checkAccess()
as it should have been.// Users can view own profiles at all times. elseif ($account->id() == $entity->id()) { return AccessResult::allowed()->cachePerUser(); }
173 173 $this->assertSession()->pageTextNotContains($label); 174 174 $this->assertCacheContexts(['languages:language_content', 'languages:language_interface', 'theme', 'url.query_args:' . MainContentViewSubscriber::WRAPPER_FORMAT, 'url.site', 'user', 'route']); 175 175 176 // Ensure that a page that does not have a node context can still be cached, 177 // the front page is the user page which is already cached from the login 178 // request above. 176 // Ensure that a page that does not have a node context can still be cached. 177 \Drupal::service('module_installer')->install(['dynamic_page_cache_test']); 104 104 * Tests that the toolbar workspace switcher doesn't disable the page cache. 105 105 */ 106 106 public function testToolbarSwitcherDynamicPageCache() { 107 $node_type = $this->drupalCreateContentType(); 108 $node = $this->drupalCreateNode(['type' => $node_type->id()]); 107 109 $this->drupalLogin($this->drupalCreateUser([ 108 110 'access toolbar', 109 111 'view any workspace', 110 112 ])); 111 // Front-page is visited right after login. - Resolved by Dezső Biczó
added 1 commit
- 5f84b6de - Try to lower the weight of DynamicPageCacheSubscriber instead.
added 1 commit
- db7a5847 - Revert "Try to lower the weight of DynamicPageCacheSubscriber instead."
- Resolved by Dezső Biczó
3045 3045 'url.query_args:resourceVersion', 3046 3046 'url.site', 3047 3047 ]; 3048 $this->assertResourceErrorResponse(501, 'JSON:API does not support filtering on revisions other than the latest version because a secure Drupal core API does not yet exist to do so.', $rel_working_copy_collection_url_filtered, $actual_response, FALSE, ['http_response'], $filtered_collection_expected_cache_contexts); 3048 $this->assertResourceErrorResponse(501, 'JSON:API does not support filtering on revisions other than the latest version because a secure Drupal core API does not yet exist to do so.', $rel_working_copy_collection_url_filtered, $actual_response, FALSE, ['http_response'], $filtered_collection_expected_cache_contexts, FALSE, 'UNCACHEABLE'); I see this gets reverted in your spin-off here, but I'm wondering where this is coming from in the first place.
This could be also "by design" like here: !8221 (comment 321053)
I see this gets reverted in your spin-off here, but I'm wondering where this is coming from in the first place.
FTR, there is no revert in the spin-off anymore.
Hm, my first guess, this is works as designed... and it just reveals a problem that can be solved independently form this issue.
Route enhancers should not abort page executions by throwing a
\Drupal\Core\Http\Exception\CacheableHttpException
. See\Drupal\jsonapi\Revisions\ResourceVersionRouteEnhancer::enhance()
The "JSON:API does not support filtering on revisions other than the latest version because a secure Drupal core API does not yet exist to do so." message is bubbling up from there.
Even the interface doc says nothing about expected exceptions, see
\Drupal\Core\Routing\EnhancerInterface
@bbrala Do you have an opinion on this?
Edited by Dezső Biczóchanged this line in version 29 of the diff
My solution for this riddle is in this MR now: !8221 (9d267378)
Whether enhancers should do this or not... could be discussed in a follow up issue.
mentioned in commit issue/drupal-3231707@ba760665
mentioned in commit issue/drupal-3231707@1c07aef9
added 25 commits
-
db7a5847...1c814ad4 - 20 commits from branch
project:11.x
- a2f3597f - Add failing test
- 7e4dd645 - Cherry-picked fix from !8198 (closed)
- 267ef900 - Try to lower the weight of DynamicPageCacheSubscriber instead.
- e7a65ad2 - Revert "Try to lower the weight of DynamicPageCacheSubscriber instead."
- 1c946e77 - save point
Toggle commit list-
db7a5847...1c814ad4 - 20 commits from branch