Issue #3202583: Views cache settings not added to headers - #10
Open
Decipher wants to merge 2 commits into
Open
Conversation
…ders Derive the JSON:API response's cacheable metadata from the view's own render array via CacheableMetadata::createFromRenderArray(), which already aggregates cache tags, contexts, and max-age from every Views plugin and handler that implements CacheableDependencyInterface. Fixes missing cache contexts (e.g. `user`, for contextual filters defaulting to the current user) that were causing the same cached response to be served across different users. Patch: https://www.drupal.org/files/issues/2024-11-20/view_cache_dependency-3202583-13.patch Applied by deciphered on behalf of yahyaalhamad, credited as author. Fixes #3202583
Keep the original access-check-before-execution ordering rather than patch #13's approach (execute first, reuse one cacheable metadata object for both the 403 and 200 paths) — that runs the view's query even for requests that end up denied. The 403 path now derives its own cacheability via $view->getDisplay()->getCacheMetadata() instead, the same aggregation, without requiring execution. This also adds cache contexts (not just the static config:views.view.<id> tag) to the denial response for the first time, since access checks can themselves depend on contexts the view contributes (e.g. `user`). Also removes the now-fully-dead BubbleableMetadata::createFromObject() handling in the 200 path (patch #13 worked around it rather than removing it), and generalizes JsonapiViewsResourceTest::assertCacheContext() to walk the full cache-context ancestor chain: this fix surfaces broader contexts (bare `url`, from Views' own exposed-form/pager plugins) that Drupal's cache context optimizer uses to collapse the narrower explicit url.query_args:* contexts this module adds, which the test helper only checked one level of (added in #3587949 for the narrower url.query_args case). Fixes #3202583
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughViews resource responses now derive cacheability from display metadata, rendered output, and bubbled render metadata. Functional cache-context assertions now support hierarchical contexts. The spelling dictionary includes ChangesViews response cacheability
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ViewsResource
participant ViewDisplay
participant RenderContext
ViewsResource->>ViewDisplay: check access or execute view
ViewDisplay->>RenderContext: render preview and bubble metadata
RenderContext->>ViewsResource: return render result and bubbled metadata
ViewsResource->>ViewsResource: attach cache metadata to response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.x-1.x #10 +/- ##
===========================================
- Coverage 94.11% 93.98% -0.14%
===========================================
Files 3 3
Lines 136 133 -3
===========================================
- Hits 128 125 -3
Misses 8 8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirrors Drupal.org issue #3202583.
Two commits:
view_cache_dependency-3202583-13.patch, comment 11) — deriving the JSON:API response's cacheable metadata from the view's own render array viaCacheableMetadata::createFromRenderArray().BubbleableMetadatahandling, and generalizes the test suite'sassertCacheContext()helper to walk the full cache-context ancestor chain (this fix surfaces a broaderurlcontext that collapses the narrowerurl.query_args:*contexts under Drupal's cache-context optimizer, which the helper didn't previously recognize).Verified on Drupal 10 and 11, and via the gitlab.local DrupalCI-mirror pipeline (real DB service): pipeline #1314, all green.
Summary by CodeRabbit