Issue #3484714: Increase route build speed - #11
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRoute generation now caches JSON:API resource type names by entity type. A kernel test verifies that multiple node-based Views use the cached resource type and return the expected entity data. ChangesResource type cache
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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 #11 +/- ##
===========================================
+ Coverage 94.11% 94.28% +0.16%
===========================================
Files 3 3
Lines 136 140 +4
===========================================
+ Hits 128 132 +4
Misses 8 8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/src/Kernel/JsonapiViewsResourceKernelTest.php (1)
249-294: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftMake the cache hit observable.
This test validates response correctness only. A route builder that recomputes resource types for each View produces the same responses.
Decorate or replace
ResourceTypeRepositoryInterfacebefore the router rebuild. Assert thatget()runs once pernodebundle across both Views. This protects the route-build performance contract from regression.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/src/Kernel/JsonapiViewsResourceKernelTest.php` around lines 249 - 294, Update testMultipleViewsShareEntityTypeResourceTypeCache() to decorate or replace ResourceTypeRepositoryInterface before rebuilding the router, track get() invocations by node bundle, and assert each bundle is resolved only once across both Views. Retain the existing response assertions while making the cache-hit behavior observable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/src/Kernel/JsonapiViewsResourceKernelTest.php`:
- Around line 249-294: Update
testMultipleViewsShareEntityTypeResourceTypeCache() to decorate or replace
ResourceTypeRepositoryInterface before rebuilding the router, track get()
invocations by node bundle, and assert each bundle is resolved only once across
both Views. Retain the existing response assertions while making the cache-hit
behavior observable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ffc01900-2f14-472b-9eff-660fa8224b00
📒 Files selected for processing (2)
src/Routing/Routes.phptests/src/Kernel/JsonapiViewsResourceKernelTest.php
2124d34 to
6245b5e
Compare
Mirrors Drupal.org issue #3484714.
During database updates, when Drupal's cache backend can't persist writes, route building recomputed JSON:API resource type names for every view instead of reusing them across views sharing an entity type. On one reported site (46 views, 13 content types) this took over 45 minutes.
Caches resource type names per entity type during route building, and adds test coverage for views sharing an entity type (previously untested). Measured at the reported scale: 637 calls to
ResourceTypeRepository::get()before, 13 after.Tests green on D10 and D11.
Summary by CodeRabbit
Bug Fixes
Tests