Skip to content

Nested set treeRootNode option - fix tricky and improve performance#3045

Open
wilfi00 wants to merge 2 commits intodoctrine-extensions:mainfrom
wilfi00:fix/nested-set-tree-root-node-recover-improvement
Open

Nested set treeRootNode option - fix tricky and improve performance#3045
wilfi00 wants to merge 2 commits intodoctrine-extensions:mainfrom
wilfi00:fix/nested-set-tree-root-node-recover-improvement

Conversation

@wilfi00
Copy link
Copy Markdown

@wilfi00 wilfi00 commented Mar 26, 2026

Hello!

I came across a "bug" when using the recover method with the treeRootNode option:

->recover(
            [
                'treeRootNode' => $rootStructure
            ]
);

In NestedTreeRepository, the code loops through each root node and compares the current node with the one provided in the options. However, comparing objects can be tricky in PHP, and in my case, it wasn't working.

My root node is a Doctrine entity. When I pass this node as an option, it has a fully instantiated User attribute. But getRootNodes() returns an entity with a lazy-loaded User object. Because of this, the comparison was wrongly returning false.

The attribute of the root node I give in option:
image

The attribute of the root node being iterated by NestedTreeRepository:
image

I suggest a refactor to unify the logic whether it's a forest, a specific root node, or a single tree. This way, we avoid the direct object comparison entirely, and it works like a charm.

I am not very familiar with this type of contribution, so feel free to request changes or ask questions 🙏

edit: I've also updated the verify method to apply the same logic.

Thank you,
Johan

wilfi00 added 2 commits March 26, 2026 11:05
Nested set changes in recover method :
* refactor code
* fix tricky comparison with the treeRootNode option
* slighly better performance when using treeRootNodeOption
@wilfi00
Copy link
Copy Markdown
Author

wilfi00 commented Apr 8, 2026

Could I have an answer to this, please? Thanks 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant