Speed up root access on the node read and create paths - #12283
Conversation
Codacy's Analysis Summary0 new issue (≤ 0 issue)
|
There was a problem hiding this comment.
Pull request overview
Updates node creation to assign manual order values under manually ordered roots and centralizes repository/branch validation.
Changes:
- Removes the root exclusion from manual order resolution.
- Refactors branch validation and root lookups.
- Adds regression tests for ordering and invalid contexts.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
modules/itest/itest-core/src/test/java/com/enonic/xp/core/node/NodeServiceImplTest.java |
Tests missing repository and branch handling. |
modules/itest/itest-core/src/test/java/com/enonic/xp/core/node/CreateNodeCommandTest.java |
Tests manual ordering under a root node. |
modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/NodeServiceImpl.java |
Delegates branch validation. |
modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/NodeHelper.java |
Centralizes branch validation. |
modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/node/CreateNodeCommand.java |
Resolves manual order values for root children. |
modules/core/core-repo/src/main/java/com/enonic/xp/repo/impl/branch/storage/BranchServiceImpl.java |
Supports root lookups by ID. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
NodeServiceImplTest covered getById against a missing repository and a missing branch, but there was no equivalent for create, leaving the exception it reports for those cases unspecified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANkY5TN9xn2cFKHk1sfX4B
Every node service method verified that the branch exists before doing its own work, which for create meant a storage lookup of the root node on top of the parent it reads anyway. The lookup is now made only once a parent turns out to be missing, where it tells a branch that does not exist from a parent that does not, and a missing repository keeps being reported as such rather than as the index error underneath. The guard itself moves to NodeHelper so the service and the create command share one copy of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANkY5TN9xn2cFKHk1sfX4B
Resolving a path to a node asks the branch index which node holds that path, and caches the answer. The root path needs neither: the root node keeps the one node id that is known without looking it up, and it can be neither moved nor deleted, so its id is answered directly. A root path that was not cached refreshed the storage index and then searched it, and a cached one resolved inside the atomic block of the cache, which serialized every concurrent lookup of the busiest path in the repository across a storage round trip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANkY5TN9xn2cFKHk1sfX4B
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ANkY5TN9xn2cFKHk1sfX4B
e6b44c7 to
0193375
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12283 +/- ##
=========================================
Coverage 87.01% 87.02%
- Complexity 20754 20758 +4
=========================================
Files 2593 2593
Lines 69055 69063 +8
Branches 5723 5723
=========================================
+ Hits 60091 60100 +9
Misses 6290 6290
+ Partials 2674 2673 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Performance of the two hottest root-node touches, plus a behaviour pin. Four commits:
NodeServiceImplTestcoveredgetByIdagainst a missing repository and branch but notcreate, leaving the exception it reports unspecified. The pin comes first so the next commit demonstrably changes nothing observable.NodeHelperso the service and the create command share one copy.An earlier revision also fixed
resolvePotentialManualOrderValueignoring a manually ordered root; that fix is deliberately dropped from this PR — legacy manual ordering under root stays as it was, and root ordering is handled by the order-key work in #12285 instead.🤖 Generated with Claude Code
https://claude.ai/code/session_01ANkY5TN9xn2cFKHk1sfX4B