Skip to content

[Tree] Fix recover() not forwarding treeRootNode to verify(), causing unscoped forest verification - #3048

Open
ashrafthamir wants to merge 3 commits into
doctrine-extensions:mainfrom
ashrafthamir:fix/nested-tree-recover-scoped-verify
Open

[Tree] Fix recover() not forwarding treeRootNode to verify(), causing unscoped forest verification#3048
ashrafthamir wants to merge 3 commits into
doctrine-extensions:mainfrom
ashrafthamir:fix/nested-tree-recover-scoped-verify

Conversation

@ashrafthamir

Copy link
Copy Markdown

Problem

recover() accepts a treeRootNode option to scope recovery to a single tree, but the internal verify() call does not forward it. This causes verify() to check the entire forest instead of only the target tree.

If any other tree in the forest has errors, the early-exit optimisation is bypassed and the target tree is unnecessarily recovered. At scale (many roots, large trees), the unscoped verify() iterates every lft/rgt index across all trees, producing a query per index and leading to timeouts.

Solution

Forward treeRootNode to verify(), which already supports it.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a performance/correctness issue in the Tree NestedTreeRepository::recover() flow where recovery scoped to a single root node still performed an unscoped verify() across the entire forest, potentially triggering unnecessary recovery work and large verification queries.

Changes:

  • Forward treeRootNode from recover() into the internal verify() call to keep verification scoped.
  • Add a regression test ensuring recover(['treeRootNode' => ...]) does not get forced to run due to errors in other trees.
  • Document the fix in CHANGELOG.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/Gedmo/Tree/NestedTreeRootRepositoryTest.php Adds a regression test to confirm recovery verification is properly scoped to the specified root tree.
src/Tree/Entity/Repository/NestedTreeRepository.php Forwards treeRootNode into the early-exit verification inside recover().
CHANGELOG.md Notes the bugfix under Unreleased.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/Gedmo/Tree/NestedTreeRootRepositoryTest.php Outdated
ashrafthamir and others added 2 commits August 3, 2026 09:41
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants