From 18013dd5ccc363299676fa016ae155c83a80c726 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Tue, 1 Jul 2025 11:40:59 -0400 Subject: [PATCH 01/16] feat: Enhance `NestedSetsBehavior` class to support multi-tree operations and improve node movement logic. --- src/NestedSetsBehavior.php | 188 +++++++++++------- .../stub/ExtendableNestedSetsBehavior.php | 4 +- 2 files changed, 117 insertions(+), 75 deletions(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index 5f12921..28505ec 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -6,7 +6,9 @@ use LogicException; use yii\base\{Behavior, NotSupportedException}; -use yii\db\{ActiveQuery, ActiveRecord, Exception, Expression}; +use yii\db\{ActiveQuery, ActiveRecord, Connection, Exception, Expression}; + +use function sprintf; /** * Nested set behavior for managing hierarchical data in {@see ActiveRecord} models. @@ -119,6 +121,8 @@ class NestedSetsBehavior extends Behavior */ public string|false $treeAttribute = false; + private Connection|null $db = null; + /** * Handles post-deletion updates for the nested set structure. * @@ -247,17 +251,19 @@ public function afterInsert(): void */ public function afterUpdate(): void { + $treeValue = $this->treeAttribute !== false ? $this->getOwner()->getAttribute($this->treeAttribute) : null; + match (true) { $this->operation === self::OPERATION_APPEND_TO && $this->node !== null => - $this->moveNode($this->node, $this->node->getAttribute($this->rightAttribute), 1), + $this->moveNode($this->node, $treeValue, $this->node->getAttribute($this->rightAttribute), 1), $this->operation === self::OPERATION_INSERT_AFTER && $this->node !== null => - $this->moveNode($this->node, $this->node->getAttribute($this->rightAttribute) + 1, 0), + $this->moveNode($this->node, $treeValue, $this->node->getAttribute($this->rightAttribute) + 1, 0), $this->operation === self::OPERATION_INSERT_BEFORE && $this->node !== null => - $this->moveNode($this->node, $this->node->getAttribute($this->leftAttribute), 0), + $this->moveNode($this->node, $treeValue, $this->node->getAttribute($this->leftAttribute), 0), $this->operation === self::OPERATION_MAKE_ROOT => - $this->moveNodeAsRoot(), + $this->moveNodeAsRoot($treeValue), $this->operation === self::OPERATION_PREPEND_TO && $this->node !== null => - $this->moveNode($this->node, $this->node->getAttribute($this->leftAttribute) + 1, 1), + $this->moveNode($this->node, $treeValue, $this->node->getAttribute($this->leftAttribute) + 1, 1), default => null, }; @@ -1177,24 +1183,22 @@ protected function deleteWithChildrenInternal(): bool|int * and supports both root and non-root node moves. * * @param ActiveRecord $node Node to be moved within the nested set tree. + * @param mixed $treeValue Tree attribute value to which the node will be moved, or `false` if not applicable. * @param int $value Left attribute value indicating the new position for the node. * @param int $depth Depth offset to apply to the node and its descendants after the move. */ - protected function moveNode(ActiveRecord $node, int $value, int $depth): void + protected function moveNode(ActiveRecord $node, mixed $treeValue, int $value, int $depth): void { $db = $this->getOwner()::getDb(); - $leftValue = $this->getOwner()->getAttribute($this->leftAttribute); - $rightValue = $this->getOwner()->getAttribute($this->rightAttribute); - $depthValue = $this->getOwner()->getAttribute($this->depthAttribute); $depthAttribute = $db->quoteColumnName($this->depthAttribute); + $depthValue = $this->getOwner()->getAttribute($this->depthAttribute); + $leftValue = $this->getOwner()->getAttribute($this->leftAttribute); $nodeDepthValue = $node->getAttribute($this->depthAttribute); + $rightValue = $this->getOwner()->getAttribute($this->rightAttribute); - $depth = $nodeDepthValue - $depthValue + $depth; + $depthValue = $nodeDepthValue - $depthValue + $depth; - if ( - $this->treeAttribute === false || - $this->getOwner()->getAttribute($this->treeAttribute) === $node->getAttribute($this->treeAttribute) - ) { + if ($this->treeAttribute === false || $treeValue === $node->getAttribute($this->treeAttribute)) { $delta = $rightValue - $leftValue + 1; $this->shiftLeftRightAttribute($value, $delta); @@ -1221,7 +1225,7 @@ protected function moveNode(ActiveRecord $node, int $value, int $depth): void $this->applyTreeAttributeCondition($condition); $this->getOwner()::updateAll( [ - $this->depthAttribute => new Expression($depthAttribute . sprintf('%+d', $depth)), + $this->depthAttribute => new Expression($depthAttribute . sprintf('%+d', $depthValue)), ], $condition, ); @@ -1252,8 +1256,6 @@ protected function moveNode(ActiveRecord $node, int $value, int $depth): void $this->shiftLeftRightAttribute($rightValue, -$delta); } else { - $leftAttribute = $db->quoteColumnName($this->leftAttribute); - $rightAttribute = $db->quoteColumnName($this->rightAttribute); $nodeRootValue = $node->getAttribute($this->treeAttribute); foreach ([$this->leftAttribute, $this->rightAttribute] as $attribute) { @@ -1277,31 +1279,13 @@ protected function moveNode(ActiveRecord $node, int $value, int $depth): void ); } - $delta = $value - $leftValue; - - $this->getOwner()::updateAll( - [ - $this->leftAttribute => new Expression($leftAttribute . sprintf('%+d', $delta)), - $this->rightAttribute => new Expression($rightAttribute . sprintf('%+d', $delta)), - $this->depthAttribute => new Expression($depthAttribute . sprintf('%+d', $depth)), - $this->treeAttribute => $nodeRootValue, - ], - [ - 'and', - [ - '>=', - $this->leftAttribute, - $leftValue, - ], - [ - '<=', - $this->rightAttribute, - $rightValue, - ], - [ - $this->treeAttribute => $this->getOwner()->getAttribute($this->treeAttribute), - ], - ], + $this->moveSubtreeToTargetTree( + $nodeRootValue, + $treeValue, + $depthValue, + $leftValue, + ($value - $leftValue), + $rightValue, ); $this->shiftLeftRightAttribute($rightValue, $leftValue - $rightValue - 1); } @@ -1324,40 +1308,23 @@ protected function moveNode(ActiveRecord $node, int $value, int $depth): void * - Shifts left and right boundaries of the node and its descendants to start from 1. * - Shifts left/right values of remaining nodes to close the gap left by the moved subtree. * - Updates the tree attribute to the new root identifier if multi-tree is enabled. + * + * @param mixed $treeValue Tree attribute value to which the node will be moved, or `false` if not applicable. */ - protected function moveNodeAsRoot(): void + protected function moveNodeAsRoot(mixed $treeValue): void { - $db = $this->getOwner()::getDb(); + $depthValue = $this->getOwner()->getAttribute($this->depthAttribute); $leftValue = $this->getOwner()->getAttribute($this->leftAttribute); + $nodeRootValue = $this->getOwner()->getPrimaryKey(); $rightValue = $this->getOwner()->getAttribute($this->rightAttribute); - $depthValue = $this->getOwner()->getAttribute($this->depthAttribute); - $treeValue = $this->treeAttribute !== false ? $this->getOwner()->getAttribute($this->treeAttribute) : null; - $leftAttribute = $db->quoteColumnName($this->leftAttribute); - $rightAttribute = $db->quoteColumnName($this->rightAttribute); - $depthAttribute = $db->quoteColumnName($this->depthAttribute); - $this->getOwner()::updateAll( - [ - $this->leftAttribute => new Expression($leftAttribute . sprintf('%+d', 1 - $leftValue)), - $this->rightAttribute => new Expression($rightAttribute . sprintf('%+d', 1 - $leftValue)), - $this->depthAttribute => new Expression($depthAttribute . sprintf('%+d', -$depthValue)), - $this->treeAttribute => $this->getOwner()->getPrimaryKey(), - ], - [ - 'and', - [ - '>=', - $this->leftAttribute, - $leftValue, - ], - [ - '<=', - $this->rightAttribute, - $rightValue, - ], - [ - $this->treeAttribute => $treeValue, - ], - ], + + $this->moveSubtreeToTargetTree( + $nodeRootValue, + $treeValue, + -$depthValue, + $leftValue, + (1 - $leftValue), + $rightValue, ); $this->shiftLeftRightAttribute($rightValue, $leftValue - $rightValue - 1); } @@ -1394,6 +1361,22 @@ protected function shiftLeftRightAttribute(int $value, int $delta): void } } + /** + * Retrieves and caches the {@see Connection} object associated with the owner model. + * + * The connection is resolved on first access and stored for subsequent calls to improve performance and avoid + * redundant lookups. + * + * This method is used internally by operations that require direct database access, such as bulk updates or + * structural modifications to the nested set tree. + * + * @return Connection Database connection instance for the owner model. + */ + private function getDb(): Connection + { + return $this->db ??= $this->getOwner()::getDb(); + } + /** * Returns the {@see ActiveRecord} instance to which this behavior is currently attached. * @@ -1417,4 +1400,63 @@ private function getOwner(): ActiveRecord return $this->owner; } + + /** + * Moves a subtree to a different tree or position within a multi-tree nested set structure. + * + * Updates the left, right, and depth attributes, as well as the tree attribute, for all nodes in the specified + * subtree. + * + * This method is used internally when moving a node and its descendants across trees or to a new root, ensuring + * that all affected nodes are updated in a single bulk operation. + * + * This operation is essential for maintaining the integrity of the nested set structure when reorganizing nodes + * between trees or promoting a node to root in a multi-tree configuration. + * + * @param mixed $nodeRootValue Value to assign to the tree attribute for all nodes in the moved subtree. + * @param mixed $treeValue Current tree attribute value of the nodes being moved. + * @param int $depth Depth offset to apply to all nodes in the subtree. + * @param int $leftValue Left boundary value of the subtree to move. + * @param int $positionOffset Amount to shift left and right attribute values for the subtree. + * @param int $rightValue Right boundary value of the subtree to move. + */ + private function moveSubtreeToTargetTree( + mixed $nodeRootValue, + mixed $treeValue, + int $depth, + int $leftValue, + int $positionOffset, + int $rightValue, + ): void { + $this->getOwner()::updateAll( + [ + $this->leftAttribute => new Expression( + $this->getDb()->quoteColumnName($this->leftAttribute) . sprintf('%+d', $positionOffset), + ), + $this->rightAttribute => new Expression( + $this->getDb()->quoteColumnName($this->rightAttribute) . sprintf('%+d', $positionOffset), + ), + $this->depthAttribute => new Expression( + $this->getDb()->quoteColumnName($this->depthAttribute) . sprintf('%+d', $depth), + ), + $this->treeAttribute => $nodeRootValue, + ], + [ + 'and', + [ + '>=', + $this->leftAttribute, + $leftValue, + ], + [ + '<=', + $this->rightAttribute, + $rightValue, + ], + [ + $this->treeAttribute => $treeValue, + ], + ], + ); + } } diff --git a/tests/support/stub/ExtendableNestedSetsBehavior.php b/tests/support/stub/ExtendableNestedSetsBehavior.php index 9f56be0..63e2fd9 100644 --- a/tests/support/stub/ExtendableNestedSetsBehavior.php +++ b/tests/support/stub/ExtendableNestedSetsBehavior.php @@ -54,14 +54,14 @@ public function exposedMoveNode(ActiveRecord $node, int $value, int $depth): voi { $this->calledMethods['moveNode'] = true; - $this->moveNode($node, $value, $depth); + $this->moveNode($node, null, $value, $depth); } public function exposedMoveNodeAsRoot(): void { $this->calledMethods['moveNodeAsRoot'] = true; - $this->moveNodeAsRoot(); + $this->moveNodeAsRoot(null); } public function exposedShiftLeftRightAttribute(int $value, int $delta): void From 94cac4114bf73ceb6fa747844c6f4d9588b171c9 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Tue, 1 Jul 2025 15:41:26 +0000 Subject: [PATCH 02/16] Apply fixes from StyleCI --- src/NestedSetsBehavior.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index 28505ec..b4aea85 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -1284,7 +1284,7 @@ protected function moveNode(ActiveRecord $node, mixed $treeValue, int $value, in $treeValue, $depthValue, $leftValue, - ($value - $leftValue), + $value - $leftValue, $rightValue, ); $this->shiftLeftRightAttribute($rightValue, $leftValue - $rightValue - 1); @@ -1323,7 +1323,7 @@ protected function moveNodeAsRoot(mixed $treeValue): void $treeValue, -$depthValue, $leftValue, - (1 - $leftValue), + 1 - $leftValue, $rightValue, ); $this->shiftLeftRightAttribute($rightValue, $leftValue - $rightValue - 1); From d587eff891090c20e03315c16522959193d1a6a6 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Tue, 1 Jul 2025 11:49:20 -0400 Subject: [PATCH 03/16] fix: Update parameter names in `moveSubtreeToTargetTree` method for clarity and consistency. --- src/NestedSetsBehavior.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index b4aea85..1b9ba20 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -1413,16 +1413,16 @@ private function getOwner(): ActiveRecord * This operation is essential for maintaining the integrity of the nested set structure when reorganizing nodes * between trees or promoting a node to root in a multi-tree configuration. * - * @param mixed $nodeRootValue Value to assign to the tree attribute for all nodes in the moved subtree. - * @param mixed $treeValue Current tree attribute value of the nodes being moved. + * @param mixed $newTreeValue Value to assign to the tree attribute for all nodes in the moved subtree. + * @param mixed $currentTreeValue Current tree attribute value of the nodes being moved. * @param int $depth Depth offset to apply to all nodes in the subtree. * @param int $leftValue Left boundary value of the subtree to move. * @param int $positionOffset Amount to shift left and right attribute values for the subtree. * @param int $rightValue Right boundary value of the subtree to move. */ private function moveSubtreeToTargetTree( - mixed $nodeRootValue, - mixed $treeValue, + mixed $newTreeValue, + mixed $currentTreeValue, int $depth, int $leftValue, int $positionOffset, @@ -1439,7 +1439,7 @@ private function moveSubtreeToTargetTree( $this->depthAttribute => new Expression( $this->getDb()->quoteColumnName($this->depthAttribute) . sprintf('%+d', $depth), ), - $this->treeAttribute => $nodeRootValue, + $this->treeAttribute => $newTreeValue ], [ 'and', @@ -1454,7 +1454,7 @@ private function moveSubtreeToTargetTree( $rightValue, ], [ - $this->treeAttribute => $treeValue, + $this->treeAttribute => $currentTreeValue ], ], ); From 0a4141f623f9319ce152674edb44811bf2ed1b76 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Tue, 1 Jul 2025 15:49:39 +0000 Subject: [PATCH 04/16] Apply fixes from StyleCI --- src/NestedSetsBehavior.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index 1b9ba20..f2887ef 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -1439,7 +1439,7 @@ private function moveSubtreeToTargetTree( $this->depthAttribute => new Expression( $this->getDb()->quoteColumnName($this->depthAttribute) . sprintf('%+d', $depth), ), - $this->treeAttribute => $newTreeValue + $this->treeAttribute => $newTreeValue, ], [ 'and', @@ -1454,7 +1454,7 @@ private function moveSubtreeToTargetTree( $rightValue, ], [ - $this->treeAttribute => $currentTreeValue + $this->treeAttribute => $currentTreeValue, ], ], ); From 958fb1cd32e238fe68762eef5b2543fabb25ca9a Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 04:57:37 -0400 Subject: [PATCH 05/16] feat: Introduce NodeContext class for improved node movement handling and refactor moveNode method to use context. --- src/NestedSetsBehavior.php | 196 +++++++++++------- src/NodeContext.php | 146 +++++++++++++ .../stub/ExtendableNestedSetsBehavior.php | 9 +- 3 files changed, 273 insertions(+), 78 deletions(-) create mode 100644 src/NodeContext.php diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index f2887ef..4b2b0bd 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -5,6 +5,7 @@ namespace yii2\extensions\nestedsets; use LogicException; +use RuntimeException; use yii\base\{Behavior, NotSupportedException}; use yii\db\{ActiveQuery, ActiveRecord, Connection, Exception, Expression}; @@ -121,6 +122,12 @@ class NestedSetsBehavior extends Behavior */ public string|false $treeAttribute = false; + /** + * Database connection instance used for executing queries. + * + * This property is used to access the database connection associated with the attached {@see ActiveRecord} model, + * allowing the behavior to perform database operations such as updates and queries. + */ private Connection|null $db = null; /** @@ -251,24 +258,23 @@ public function afterInsert(): void */ public function afterUpdate(): void { - $treeValue = $this->treeAttribute !== false ? $this->getOwner()->getAttribute($this->treeAttribute) : null; + $currentOwnerTreeValue = $this->getTreeValue($this->getOwner()); - match (true) { - $this->operation === self::OPERATION_APPEND_TO && $this->node !== null => - $this->moveNode($this->node, $treeValue, $this->node->getAttribute($this->rightAttribute), 1), - $this->operation === self::OPERATION_INSERT_AFTER && $this->node !== null => - $this->moveNode($this->node, $treeValue, $this->node->getAttribute($this->rightAttribute) + 1, 0), - $this->operation === self::OPERATION_INSERT_BEFORE && $this->node !== null => - $this->moveNode($this->node, $treeValue, $this->node->getAttribute($this->leftAttribute), 0), - $this->operation === self::OPERATION_MAKE_ROOT => - $this->moveNodeAsRoot($treeValue), - $this->operation === self::OPERATION_PREPEND_TO && $this->node !== null => - $this->moveNode($this->node, $treeValue, $this->node->getAttribute($this->leftAttribute) + 1, 1), - default => null, - }; + if ($this->operation === self::OPERATION_MAKE_ROOT) { + $this->moveNodeAsRoot($currentOwnerTreeValue); + $this->resetOperationState(); + return; + } - $this->operation = null; - $this->node = null; + if ($this->node === null) { + $this->resetOperationState(); + return; + } + + $context = $this->createMoveContext($this->node, $this->operation); + + $this->moveNode($context); + $this->resetOperationState(); } /** @@ -1165,47 +1171,32 @@ protected function deleteWithChildrenInternal(): bool|int } /** - * Moves the current node to a new position within the nested set tree structure. - * - * Updates the left, right, and depth attributes of the node and its descendants to reflect the new position, - * ensuring the integrity of the tree after the move operation. - * - * This method supports both single-tree and multi-tree configurations, handling node movement within the same tree - * or across different trees as needed. + * Executes node movement using the provided context. * - * The method performs the following operations. - * - Adjusts left and right boundaries for affected nodes. - * - Handles tree attribute updates when moving nodes across trees. - * - Shifts left and right attribute values to make space for the moved node and its subtree. - * - Updates the depth of the node and its descendants based on the new position. - * - * This method is called internally during node movement operations such as append, prepend, insert before/after, - * and supports both root and non-root node moves. + * Handles both same-tree and cross-tree movements, determining the strategy based on tree attribute configuration + * and tree values from the context. * - * @param ActiveRecord $node Node to be moved within the nested set tree. - * @param mixed $treeValue Tree attribute value to which the node will be moved, or `false` if not applicable. - * @param int $value Left attribute value indicating the new position for the node. - * @param int $depth Depth offset to apply to the node and its descendants after the move. + * @param NodeContext $context Immutable context containing all movement data. */ - protected function moveNode(ActiveRecord $node, mixed $treeValue, int $value, int $depth): void + protected function moveNode(NodeContext $context): void { - $db = $this->getOwner()::getDb(); - $depthAttribute = $db->quoteColumnName($this->depthAttribute); - $depthValue = $this->getOwner()->getAttribute($this->depthAttribute); - $leftValue = $this->getOwner()->getAttribute($this->leftAttribute); - $nodeDepthValue = $node->getAttribute($this->depthAttribute); - $rightValue = $this->getOwner()->getAttribute($this->rightAttribute); + $currentOwnerTreeValue = $this->getTreeValue($this->getOwner()); + $targetNodeTreeValue = $context->getTargetTreeValue($this->treeAttribute); + $targetNodeDepthValue = $context->getTargetDepth($this->depthAttribute); + $ownerDepthValue = $this->getOwner()->getAttribute($this->depthAttribute); + $ownerLeftValue = $this->getOwner()->getAttribute($this->leftAttribute); + $ownerRightValue = $this->getOwner()->getAttribute($this->rightAttribute); - $depthValue = $nodeDepthValue - $depthValue + $depth; + $depthOffset = $targetNodeDepthValue - $ownerDepthValue + $context->depthLevelDelta; - if ($this->treeAttribute === false || $treeValue === $node->getAttribute($this->treeAttribute)) { - $delta = $rightValue - $leftValue + 1; + if ($this->treeAttribute === false || $targetNodeTreeValue === $currentOwnerTreeValue) { + $subtreeSize = $ownerRightValue - $ownerLeftValue + 1; - $this->shiftLeftRightAttribute($value, $delta); + $this->shiftLeftRightAttribute($context->targetPositionValue, $subtreeSize); - if ($leftValue >= $value) { - $leftValue += $delta; - $rightValue += $delta; + if ($ownerLeftValue >= $context->targetPositionValue) { + $ownerLeftValue += $subtreeSize; + $ownerRightValue += $subtreeSize; } $condition = [ @@ -1213,19 +1204,21 @@ protected function moveNode(ActiveRecord $node, mixed $treeValue, int $value, in [ '>=', $this->leftAttribute, - $leftValue, + $ownerLeftValue, ], [ '<=', $this->rightAttribute, - $rightValue, + $ownerRightValue, ], ]; $this->applyTreeAttributeCondition($condition); $this->getOwner()::updateAll( [ - $this->depthAttribute => new Expression($depthAttribute . sprintf('%+d', $depthValue)), + $this->depthAttribute => new Expression( + $this->getDb()->quoteColumnName($this->depthAttribute) . sprintf('%+d', $depthOffset), + ), ], $condition, ); @@ -1235,11 +1228,11 @@ protected function moveNode(ActiveRecord $node, mixed $treeValue, int $value, in 'and', [ '>=', - $attribute, $leftValue, + $attribute, $ownerLeftValue, ], [ '<=', - $attribute, $rightValue, + $attribute, $ownerRightValue, ], ]; @@ -1247,22 +1240,20 @@ protected function moveNode(ActiveRecord $node, mixed $treeValue, int $value, in $this->getOwner()::updateAll( [ $attribute => new Expression( - $db->quoteColumnName($attribute) . sprintf('%+d', $value - $leftValue), + $this->getDb()->quoteColumnName($attribute) . sprintf('%+d', $context->targetPositionValue - $ownerLeftValue), ), ], $condition, ); } - $this->shiftLeftRightAttribute($rightValue, -$delta); + $this->shiftLeftRightAttribute($ownerRightValue, -$subtreeSize); } else { - $nodeRootValue = $node->getAttribute($this->treeAttribute); - foreach ([$this->leftAttribute, $this->rightAttribute] as $attribute) { $this->getOwner()::updateAll( [ $attribute => new Expression( - $db->quoteColumnName($attribute) . sprintf('%+d', $rightValue - $leftValue + 1), + $this->getDb()->quoteColumnName($attribute) . sprintf('%+d', $ownerRightValue - $ownerLeftValue + 1), ), ], [ @@ -1270,24 +1261,24 @@ protected function moveNode(ActiveRecord $node, mixed $treeValue, int $value, in [ '>=', $attribute, - $value, + $context->targetPositionValue, ], [ - $this->treeAttribute => $nodeRootValue, + $this->treeAttribute => $targetNodeTreeValue, ], ], ); } $this->moveSubtreeToTargetTree( - $nodeRootValue, - $treeValue, - $depthValue, - $leftValue, - $value - $leftValue, - $rightValue, + $targetNodeTreeValue, + $currentOwnerTreeValue, + $depthOffset, + $ownerLeftValue, + $context->targetPositionValue - $ownerLeftValue, + $ownerRightValue, ); - $this->shiftLeftRightAttribute($rightValue, $leftValue - $rightValue - 1); + $this->shiftLeftRightAttribute($ownerRightValue, $ownerLeftValue - $ownerRightValue - 1); } } @@ -1346,21 +1337,40 @@ protected function moveNodeAsRoot(mixed $treeValue): void */ protected function shiftLeftRightAttribute(int $value, int $delta): void { - $db = $this->getOwner()::getDb(); - foreach ([$this->leftAttribute, $this->rightAttribute] as $attribute) { $condition = ['>=', $attribute, $value]; $this->applyTreeAttributeCondition($condition); $this->getOwner()::updateAll( [ - $attribute => new Expression($db->quoteColumnName($attribute) . sprintf('%+d', $delta)), + $attribute => new Expression($this->getDb()->quoteColumnName($attribute) . sprintf('%+d', $delta)), ], $condition, ); } } + /** + * Creates a typed movement context based on operation and target node. + * + * @param ActiveRecord $targetNode Target node for the operation. + * @param string|null $operation Operation type to perform. + * + * @throws RuntimeException if a runtime error prevents the operation from completing successfully. + * + * @return NodeContext New instance with the specified parameters for the operation. + */ + private function createMoveContext(ActiveRecord $targetNode, string|null $operation): NodeContext + { + return match ($operation) { + self::OPERATION_APPEND_TO => NodeContext::forAppendTo($targetNode, $this->rightAttribute), + self::OPERATION_INSERT_AFTER => NodeContext::forInsertAfter($targetNode, $this->rightAttribute), + self::OPERATION_INSERT_BEFORE => NodeContext::forInsertBefore($targetNode, $this->leftAttribute), + self::OPERATION_PREPEND_TO => NodeContext::forPrependTo($targetNode, $this->leftAttribute), + default => throw new RuntimeException("Unsupported operation: {$operation}"), + }; + } + /** * Retrieves and caches the {@see Connection} object associated with the owner model. * @@ -1401,6 +1411,27 @@ private function getOwner(): ActiveRecord return $this->owner; } + /** + * Retrieves the tree attribute value from the specified {@see ActiveRecord} instance. + * + * Extracts the tree identifier value from the given model instance when multi-tree support is enabled, providing + * a centralized method for accessing tree attribute values throughout the behavior. + * + * The method is used internally by movement operations, tree validation, and condition building to ensure proper + * tree scoping and maintain data integrity across different tree contexts. + * + * @param ActiveRecord|null $activeRecord Model instance from which to extract the tree value, or `null` if not + * available. + * + * @return mixed Tree attribute value if multi-tree support is enabled and the record exists, `null` otherwise. + */ + private function getTreeValue(ActiveRecord|null $activeRecord): mixed + { + return $activeRecord !== null && $this->treeAttribute !== false + ? $activeRecord->getAttribute($this->treeAttribute) + : null; + } + /** * Moves a subtree to a different tree or position within a multi-tree nested set structure. * @@ -1413,16 +1444,16 @@ private function getOwner(): ActiveRecord * This operation is essential for maintaining the integrity of the nested set structure when reorganizing nodes * between trees or promoting a node to root in a multi-tree configuration. * - * @param mixed $newTreeValue Value to assign to the tree attribute for all nodes in the moved subtree. - * @param mixed $currentTreeValue Current tree attribute value of the nodes being moved. + * @param mixed $targetNodeTreeValue Value to assign to the tree attribute for all nodes in the moved subtree. + * @param mixed $currentOwnerTreeValue Current tree attribute value of the nodes being moved. * @param int $depth Depth offset to apply to all nodes in the subtree. * @param int $leftValue Left boundary value of the subtree to move. * @param int $positionOffset Amount to shift left and right attribute values for the subtree. * @param int $rightValue Right boundary value of the subtree to move. */ private function moveSubtreeToTargetTree( - mixed $newTreeValue, - mixed $currentTreeValue, + mixed $targetNodeTreeValue, + mixed $currentOwnerTreeValue, int $depth, int $leftValue, int $positionOffset, @@ -1439,7 +1470,7 @@ private function moveSubtreeToTargetTree( $this->depthAttribute => new Expression( $this->getDb()->quoteColumnName($this->depthAttribute) . sprintf('%+d', $depth), ), - $this->treeAttribute => $newTreeValue, + $this->treeAttribute => $targetNodeTreeValue, ], [ 'and', @@ -1454,9 +1485,20 @@ private function moveSubtreeToTargetTree( $rightValue, ], [ - $this->treeAttribute => $currentTreeValue, + $this->treeAttribute => $currentOwnerTreeValue, ], ], ); } + + /** + * Resets the internal operation state after completing a nested set operation. + * + * Clears the current operation type and target node reference to prepare for subsequent operations.. + */ + private function resetOperationState(): void + { + $this->operation = null; + $this->node = null; + } } diff --git a/src/NodeContext.php b/src/NodeContext.php new file mode 100644 index 0000000..7095ae5 --- /dev/null +++ b/src/NodeContext.php @@ -0,0 +1,146 @@ +getAttribute($rightAttribute); + + return new self( + targetNode: $targetNode, + operation: NestedSetsBehavior::OPERATION_APPEND_TO, + targetPositionValue: $rightValue, + depthLevelDelta: 1, + ); + } + + /** + * Creates context for insert-after operation (next sibling). + * + * @param ActiveRecord $targetNode Target node to insert after. + * @param string $rightAttribute Name of the right attribute. + * + * @return self New instance with the specified parameters for insert-after operation. + */ + public static function forInsertAfter(ActiveRecord $targetNode, string $rightAttribute): self + { + /** @phpstan-var int $rightValue */ + $rightValue = $targetNode->getAttribute($rightAttribute); + + return new self( + targetNode: $targetNode, + operation: NestedSetsBehavior::OPERATION_INSERT_AFTER, + targetPositionValue: $rightValue + 1, + depthLevelDelta: 0, + ); + } + + /** + * Creates context for insert-before operation (previous sibling). + * + * @param ActiveRecord $targetNode Target node to insert before. + * @param string $leftAttribute Name of the left attribute. + * + * @return self New instance with the specified parameters for insert-before operation. + */ + public static function forInsertBefore(ActiveRecord $targetNode, string $leftAttribute): self + { + /** @var int $leftValue */ + $leftValue = $targetNode->getAttribute($leftAttribute); + + return new self( + targetNode: $targetNode, + operation: NestedSetsBehavior::OPERATION_INSERT_BEFORE, + targetPositionValue: $leftValue, + depthLevelDelta: 0, + ); + } + + /** + * Creates context for prepend-to operation (first child). + * + * @param ActiveRecord $targetNode Target node to prepend to. + * @param string $leftAttribute Name of the left attribute. + * + * @return self New instance with the specified parameters for prepend-to operation. + */ + public static function forPrependTo(ActiveRecord $targetNode, string $leftAttribute): self + { + /** @var int $leftValue */ + $leftValue = $targetNode->getAttribute($leftAttribute); + + return new self( + targetNode: $targetNode, + operation: NestedSetsBehavior::OPERATION_PREPEND_TO, + targetPositionValue: $leftValue + 1, + depthLevelDelta: 1, + ); + } + + /** + * Returns the depth value of the target node. + * + * @param string $depthAttribute Name of the depth attribute. + * + * @return int Depth value of the target node. + */ + public function getTargetDepth(string $depthAttribute): int + { + /** @var int $depth */ + $depth = $this->targetNode->getAttribute($depthAttribute); + + return $depth; + } + + /** + * Returns the tree attribute value of the target node. + * + * @param string|false $treeAttribute Name of the tree attribute or `false` if disabled. + * + * @return mixed Tree attribute value or `null` if tree attribute is disabled. + */ + public function getTargetTreeValue(string|false $treeAttribute): mixed + { + return $treeAttribute !== false + ? $this->targetNode->getAttribute($treeAttribute) + : null; + } +} diff --git a/tests/support/stub/ExtendableNestedSetsBehavior.php b/tests/support/stub/ExtendableNestedSetsBehavior.php index 63e2fd9..b0b41bb 100644 --- a/tests/support/stub/ExtendableNestedSetsBehavior.php +++ b/tests/support/stub/ExtendableNestedSetsBehavior.php @@ -54,7 +54,14 @@ public function exposedMoveNode(ActiveRecord $node, int $value, int $depth): voi { $this->calledMethods['moveNode'] = true; - $this->moveNode($node, null, $value, $depth); + // Create a mock context for testing compatibility + $context = new \yii2\extensions\nestedsets\NodeContext( + $node, + 'appendTo', + 0, + 0 + ); + $this->moveNode($context); } public function exposedMoveNodeAsRoot(): void From 5173b83df98809e76bed6e601a61148004a55df7 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Wed, 2 Jul 2025 08:58:19 +0000 Subject: [PATCH 06/16] Apply fixes from StyleCI --- src/NodeContext.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/NodeContext.php b/src/NodeContext.php index 7095ae5..1edcb7f 100644 --- a/src/NodeContext.php +++ b/src/NodeContext.php @@ -29,7 +29,8 @@ public function __construct( public readonly string $operation, public readonly int $targetPositionValue, public readonly int $depthLevelDelta, - ) {} + ) { + } /** * Creates context for append-to operation (last child). @@ -125,15 +126,13 @@ public static function forPrependTo(ActiveRecord $targetNode, string $leftAttrib public function getTargetDepth(string $depthAttribute): int { /** @var int $depth */ - $depth = $this->targetNode->getAttribute($depthAttribute); - - return $depth; + return $this->targetNode->getAttribute($depthAttribute); } /** * Returns the tree attribute value of the target node. * - * @param string|false $treeAttribute Name of the tree attribute or `false` if disabled. + * @param false|string $treeAttribute Name of the tree attribute or `false` if disabled. * * @return mixed Tree attribute value or `null` if tree attribute is disabled. */ From 7a1a1ad0b595322ec2b9b4a6f82e63061d99a863 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 05:03:11 -0400 Subject: [PATCH 07/16] fix: Ensure type casting for depth attribute in `NodeContext` and correct method call in `ExtendableNestedSetsBehavior`. --- src/NodeContext.php | 3 +-- tests/support/stub/ExtendableNestedSetsBehavior.php | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/NodeContext.php b/src/NodeContext.php index 1edcb7f..8d9988f 100644 --- a/src/NodeContext.php +++ b/src/NodeContext.php @@ -125,8 +125,7 @@ public static function forPrependTo(ActiveRecord $targetNode, string $leftAttrib */ public function getTargetDepth(string $depthAttribute): int { - /** @var int $depth */ - return $this->targetNode->getAttribute($depthAttribute); + return (int) $this->targetNode->getAttribute($depthAttribute); } /** diff --git a/tests/support/stub/ExtendableNestedSetsBehavior.php b/tests/support/stub/ExtendableNestedSetsBehavior.php index b0b41bb..5f97e33 100644 --- a/tests/support/stub/ExtendableNestedSetsBehavior.php +++ b/tests/support/stub/ExtendableNestedSetsBehavior.php @@ -59,7 +59,7 @@ public function exposedMoveNode(ActiveRecord $node, int $value, int $depth): voi $node, 'appendTo', 0, - 0 + 0, ); $this->moveNode($context); } From 3419c88e95d722049334efec083598953324b16b Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 06:04:58 -0400 Subject: [PATCH 08/16] refactor: Simplify constructor and improve PHPStan type annotations in NodeContext. --- src/NestedSetsBehavior.php | 2 +- src/NodeContext.php | 23 +++++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index 4b2b0bd..65bc146 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -34,7 +34,6 @@ * - Supports custom attribute names for left, right, depth, and tree columns. * * @phpstan-template T of ActiveRecord - * * @phpstan-extends Behavior * * @property int $depth @@ -301,6 +300,7 @@ public function afterUpdate(): void * $category->appendTo($parentCategory); * ``` * + * @phpstan-param T $node * @phpstan-param array|null $attributes */ public function appendTo(ActiveRecord $node, bool $runValidation = true, array|null $attributes = null): bool diff --git a/src/NodeContext.php b/src/NodeContext.php index 8d9988f..526b5bf 100644 --- a/src/NodeContext.php +++ b/src/NodeContext.php @@ -29,8 +29,7 @@ public function __construct( public readonly string $operation, public readonly int $targetPositionValue, public readonly int $depthLevelDelta, - ) { - } + ) {} /** * Creates context for append-to operation (last child). @@ -39,16 +38,15 @@ public function __construct( * @param string $rightAttribute Name of the right attribute. * * @return self New instance with the specified parameters for append-to operation. + * + * @phpstan-param 'rgt' $rightAttribute */ public static function forAppendTo(ActiveRecord $targetNode, string $rightAttribute): self { - /** @phpstan-var int $rightValue */ - $rightValue = $targetNode->getAttribute($rightAttribute); - return new self( targetNode: $targetNode, operation: NestedSetsBehavior::OPERATION_APPEND_TO, - targetPositionValue: $rightValue, + targetPositionValue: $targetNode->getAttribute($rightAttribute), depthLevelDelta: 1, ); } @@ -60,10 +58,11 @@ public static function forAppendTo(ActiveRecord $targetNode, string $rightAttrib * @param string $rightAttribute Name of the right attribute. * * @return self New instance with the specified parameters for insert-after operation. + * + * @phpstan-param 'rgt' $rightAttribute */ public static function forInsertAfter(ActiveRecord $targetNode, string $rightAttribute): self { - /** @phpstan-var int $rightValue */ $rightValue = $targetNode->getAttribute($rightAttribute); return new self( @@ -81,10 +80,11 @@ public static function forInsertAfter(ActiveRecord $targetNode, string $rightAtt * @param string $leftAttribute Name of the left attribute. * * @return self New instance with the specified parameters for insert-before operation. + * + * @phpstan-param 'lft' $leftAttribute */ public static function forInsertBefore(ActiveRecord $targetNode, string $leftAttribute): self { - /** @var int $leftValue */ $leftValue = $targetNode->getAttribute($leftAttribute); return new self( @@ -102,10 +102,11 @@ public static function forInsertBefore(ActiveRecord $targetNode, string $leftAtt * @param string $leftAttribute Name of the left attribute. * * @return self New instance with the specified parameters for prepend-to operation. + * + * @phpstan-param 'lft' $leftAttribute */ public static function forPrependTo(ActiveRecord $targetNode, string $leftAttribute): self { - /** @var int $leftValue */ $leftValue = $targetNode->getAttribute($leftAttribute); return new self( @@ -122,10 +123,12 @@ public static function forPrependTo(ActiveRecord $targetNode, string $leftAttrib * @param string $depthAttribute Name of the depth attribute. * * @return int Depth value of the target node. + * + * @phpstan-param 'depth' $depthAttribute */ public function getTargetDepth(string $depthAttribute): int { - return (int) $this->targetNode->getAttribute($depthAttribute); + return $this->targetNode->getAttribute($depthAttribute); } /** From c1ff7aa379d6f861edbd66734e77e90e1124c8bb Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Wed, 2 Jul 2025 10:05:23 +0000 Subject: [PATCH 09/16] Apply fixes from StyleCI --- src/NestedSetsBehavior.php | 1 + src/NodeContext.php | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index 65bc146..dfe5861 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -34,6 +34,7 @@ * - Supports custom attribute names for left, right, depth, and tree columns. * * @phpstan-template T of ActiveRecord + * * @phpstan-extends Behavior * * @property int $depth diff --git a/src/NodeContext.php b/src/NodeContext.php index 526b5bf..45131a5 100644 --- a/src/NodeContext.php +++ b/src/NodeContext.php @@ -29,7 +29,8 @@ public function __construct( public readonly string $operation, public readonly int $targetPositionValue, public readonly int $depthLevelDelta, - ) {} + ) { + } /** * Creates context for append-to operation (last child). From 165548b9b5d2f108bb18428534012ae5b1860a65 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 06:08:01 -0400 Subject: [PATCH 10/16] refactor: Simplify constructor syntax in NodeContext class. --- .styleci.yml | 2 ++ src/NodeContext.php | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.styleci.yml b/.styleci.yml index 9fae20e..ac75a55 100644 --- a/.styleci.yml +++ b/.styleci.yml @@ -85,3 +85,5 @@ enabled: disabled: - function_declaration - new_with_parentheses + - psr12_braces + - psr12_class_definition diff --git a/src/NodeContext.php b/src/NodeContext.php index 45131a5..526b5bf 100644 --- a/src/NodeContext.php +++ b/src/NodeContext.php @@ -29,8 +29,7 @@ public function __construct( public readonly string $operation, public readonly int $targetPositionValue, public readonly int $depthLevelDelta, - ) { - } + ) {} /** * Creates context for append-to operation (last child). From 74f5a6cbfb73f56dd47b35a50f44b5114fb80542 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 06:13:16 -0400 Subject: [PATCH 11/16] refactor: Remove redundant `resetOperationState` calls in `NestedSetsBehavior` class. --- src/NestedSetsBehavior.php | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index dfe5861..90aad1e 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -262,19 +262,16 @@ public function afterUpdate(): void if ($this->operation === self::OPERATION_MAKE_ROOT) { $this->moveNodeAsRoot($currentOwnerTreeValue); - $this->resetOperationState(); return; } if ($this->node === null) { - $this->resetOperationState(); return; } $context = $this->createMoveContext($this->node, $this->operation); $this->moveNode($context); - $this->resetOperationState(); } /** @@ -1491,15 +1488,4 @@ private function moveSubtreeToTargetTree( ], ); } - - /** - * Resets the internal operation state after completing a nested set operation. - * - * Clears the current operation type and target node reference to prepare for subsequent operations.. - */ - private function resetOperationState(): void - { - $this->operation = null; - $this->node = null; - } } From 8e5e264d46bfd7aae9094e35cf325e45895d3856 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 06:59:59 -0400 Subject: [PATCH 12/16] refactor: Replace direct parameter passing with NodeContext in beforeInsertNode method calls. --- src/NestedSetsBehavior.php | 59 +++++++++++-------- src/NodeContext.php | 26 +++++--- tests/NestedSetsBehaviorTest.php | 19 +----- .../stub/ExtendableNestedSetsBehavior.php | 3 +- 4 files changed, 55 insertions(+), 52 deletions(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index 90aad1e..bd5a063 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -378,22 +378,18 @@ public function beforeDelete(): void public function beforeInsert(): void { match (true) { - $this->operation === self::OPERATION_APPEND_TO && $this->node !== null => $this->beforeInsertNode( - $this->node->getAttribute($this->rightAttribute), - 1, + $this->operation === self::OPERATION_APPEND_TO && $this->node !== null => $this->beforeInsertNodeWithContext( + NodeContext::forAppendTo($this->node, $this->rightAttribute) ), - $this->operation === self::OPERATION_INSERT_AFTER && $this->node !== null => $this->beforeInsertNode( - $this->node->getAttribute($this->rightAttribute) + 1, - 0, + $this->operation === self::OPERATION_INSERT_AFTER && $this->node !== null => $this->beforeInsertNodeWithContext( + NodeContext::forInsertAfter($this->node, $this->rightAttribute) ), - $this->operation === self::OPERATION_INSERT_BEFORE && $this->node !== null => $this->beforeInsertNode( - $this->node->getAttribute($this->leftAttribute), - 0, + $this->operation === self::OPERATION_INSERT_BEFORE && $this->node !== null => $this->beforeInsertNodeWithContext( + NodeContext::forInsertBefore($this->node, $this->leftAttribute) ), $this->operation === self::OPERATION_MAKE_ROOT => $this->beforeInsertRootNode(), - $this->operation === self::OPERATION_PREPEND_TO && $this->node !== null => $this->beforeInsertNode( - $this->node->getAttribute($this->leftAttribute) + 1, - 1, + $this->operation === self::OPERATION_PREPEND_TO && $this->node !== null => $this->beforeInsertNodeWithContext( + NodeContext::forPrependTo($this->node, $this->leftAttribute) ), default => throw new NotSupportedException( 'Method "' . get_class($this->getOwner()) . '::insert" is not supported for inserting new nodes.', @@ -1066,12 +1062,12 @@ protected function applyTreeAttributeCondition(array &$condition): void * This method is called internally before inserting a node as a child or sibling, ensuring the nested set * attributes are correctly initialized. * - * @param int|null $value Left attribute value for the new node, or `null` if not applicable. + * @param int $value Left attribute value for the new node, or `null` if not applicable. * @param int $depth Depth offset relative to the target node (`0` for sibling, `1` for child). * * @throws Exception if an unexpected error occurs during execution. */ - protected function beforeInsertNode(int|null $value, int $depth): void + protected function beforeInsertNode(int $value, int $depth): void { if ($this->node?->getIsNewRecord() === true) { throw new Exception('Can not create a node when the target node is new record.'); @@ -1081,10 +1077,6 @@ protected function beforeInsertNode(int|null $value, int $depth): void throw new Exception('Can not create a node when the target node is root.'); } - if ($value === null) { - throw new Exception('Value cannot be \'null\' in \'beforeInsertNode()\' method.'); - } - $this->getOwner()->setAttribute($this->leftAttribute, $value); $this->getOwner()->setAttribute($this->rightAttribute, $value + 1); @@ -1099,6 +1091,24 @@ protected function beforeInsertNode(int|null $value, int $depth): void $this->shiftLeftRightAttribute($value, 2); } + /** + * Prepares the current node for insertion using a NodeContext. + * + * Sets the left, right, and depth attributes of the node to be inserted, based on the provided context which + * encapsulates the target node, operation type, and calculated values. + * + * This method delegates to {@see beforeInsertNode()} using the values from the NodeContext, ensuring consistency + * and reducing code duplication. + * + * @param NodeContext $context Immutable context containing all necessary data for node insertion. + * + * @throws Exception if an unexpected error occurs during execution. + */ + protected function beforeInsertNodeWithContext(NodeContext $context): void + { + $this->beforeInsertNode($context->getTargetPositionValue(), $context->getDepthLevelDelta()); + } + /** * Prepares the current node for insertion as a root node in the nested set tree. * @@ -1185,14 +1195,14 @@ protected function moveNode(NodeContext $context): void $ownerLeftValue = $this->getOwner()->getAttribute($this->leftAttribute); $ownerRightValue = $this->getOwner()->getAttribute($this->rightAttribute); - $depthOffset = $targetNodeDepthValue - $ownerDepthValue + $context->depthLevelDelta; + $depthOffset = $targetNodeDepthValue - $ownerDepthValue + $context->getDepthLevelDelta(); if ($this->treeAttribute === false || $targetNodeTreeValue === $currentOwnerTreeValue) { $subtreeSize = $ownerRightValue - $ownerLeftValue + 1; - $this->shiftLeftRightAttribute($context->targetPositionValue, $subtreeSize); + $this->shiftLeftRightAttribute($context->getTargetPositionValue(), $subtreeSize); - if ($ownerLeftValue >= $context->targetPositionValue) { + if ($ownerLeftValue >= $context->getTargetPositionValue()) { $ownerLeftValue += $subtreeSize; $ownerRightValue += $subtreeSize; } @@ -1238,7 +1248,8 @@ protected function moveNode(NodeContext $context): void $this->getOwner()::updateAll( [ $attribute => new Expression( - $this->getDb()->quoteColumnName($attribute) . sprintf('%+d', $context->targetPositionValue - $ownerLeftValue), + $this->getDb()->quoteColumnName($attribute) . + sprintf('%+d', $context->getTargetPositionValue() - $ownerLeftValue), ), ], $condition, @@ -1259,7 +1270,7 @@ protected function moveNode(NodeContext $context): void [ '>=', $attribute, - $context->targetPositionValue, + $context->getTargetPositionValue(), ], [ $this->treeAttribute => $targetNodeTreeValue, @@ -1273,7 +1284,7 @@ protected function moveNode(NodeContext $context): void $currentOwnerTreeValue, $depthOffset, $ownerLeftValue, - $context->targetPositionValue - $ownerLeftValue, + $context->getTargetPositionValue() - $ownerLeftValue, $ownerRightValue, ); $this->shiftLeftRightAttribute($ownerRightValue, $ownerLeftValue - $ownerRightValue - 1); diff --git a/src/NodeContext.php b/src/NodeContext.php index 526b5bf..9ad918c 100644 --- a/src/NodeContext.php +++ b/src/NodeContext.php @@ -25,10 +25,9 @@ final class NodeContext { public function __construct( - public readonly ActiveRecord $targetNode, - public readonly string $operation, - public readonly int $targetPositionValue, - public readonly int $depthLevelDelta, + private readonly ActiveRecord $targetNode, + private readonly int|null $targetPositionValue, + private readonly int $depthLevelDelta, ) {} /** @@ -45,7 +44,6 @@ public static function forAppendTo(ActiveRecord $targetNode, string $rightAttrib { return new self( targetNode: $targetNode, - operation: NestedSetsBehavior::OPERATION_APPEND_TO, targetPositionValue: $targetNode->getAttribute($rightAttribute), depthLevelDelta: 1, ); @@ -67,7 +65,6 @@ public static function forInsertAfter(ActiveRecord $targetNode, string $rightAtt return new self( targetNode: $targetNode, - operation: NestedSetsBehavior::OPERATION_INSERT_AFTER, targetPositionValue: $rightValue + 1, depthLevelDelta: 0, ); @@ -89,7 +86,6 @@ public static function forInsertBefore(ActiveRecord $targetNode, string $leftAtt return new self( targetNode: $targetNode, - operation: NestedSetsBehavior::OPERATION_INSERT_BEFORE, targetPositionValue: $leftValue, depthLevelDelta: 0, ); @@ -111,12 +107,16 @@ public static function forPrependTo(ActiveRecord $targetNode, string $leftAttrib return new self( targetNode: $targetNode, - operation: NestedSetsBehavior::OPERATION_PREPEND_TO, targetPositionValue: $leftValue + 1, depthLevelDelta: 1, ); } + public function getDepthLevelDelta(): int + { + return $this->depthLevelDelta; + } + /** * Returns the depth value of the target node. * @@ -131,6 +131,16 @@ public function getTargetDepth(string $depthAttribute): int return $this->targetNode->getAttribute($depthAttribute); } + /** + * Returns the target position value used for node movement. + * + * @return int Target position value, or 0 if not set. + */ + public function getTargetPositionValue(): int + { + return $this->targetPositionValue ?? 0; + } + /** * Returns the tree attribute value of the target node. * diff --git a/tests/NestedSetsBehaviorTest.php b/tests/NestedSetsBehaviorTest.php index 59a5c42..2c3a5e3 100644 --- a/tests/NestedSetsBehaviorTest.php +++ b/tests/NestedSetsBehaviorTest.php @@ -1601,7 +1601,7 @@ public function testSetNodeToNullAndCallBeforeInsertNodeSetsLftRgtAndDepth(): vo $this->createDatabase(); $behavior = new class extends NestedSetsBehavior { - public function callBeforeInsertNode(int|null $value, int $depth): void + public function callBeforeInsertNode(int $value, int $depth): void { $this->beforeInsertNode($value, $depth); } @@ -1741,23 +1741,6 @@ public function testReturnShiftedLeftRightAttributesWhenChildAppendedToRoot(): v ); } - public function testThrowExceptionWhenAppendToParentWithNullRightValue(): void - { - $this->createDatabase(); - - $parentNode = new Tree(['name' => 'Parent Node']); - - $parentNode->makeRoot(); - $parentNode->setAttribute('rgt', null); - - $childNode = new Tree(['name' => 'Child Node']); - - $this->expectException(Exception::class); - $this->expectExceptionMessage("Value cannot be 'null' in 'beforeInsertNode()' method."); - - $childNode->appendTo($parentNode); - } - public function testAppendToWithRunValidationParameterUsingStrictValidation(): void { $this->generateFixtureTree(); diff --git a/tests/support/stub/ExtendableNestedSetsBehavior.php b/tests/support/stub/ExtendableNestedSetsBehavior.php index 5f97e33..7f43a5f 100644 --- a/tests/support/stub/ExtendableNestedSetsBehavior.php +++ b/tests/support/stub/ExtendableNestedSetsBehavior.php @@ -29,7 +29,7 @@ public function exposedApplyTreeAttributeCondition(array &$condition): void $this->applyTreeAttributeCondition($condition); } - public function exposedBeforeInsertNode(int|null $value, int $depth): void + public function exposedBeforeInsertNode(int $value, int $depth): void { $this->calledMethods['beforeInsertNode'] = true; @@ -57,7 +57,6 @@ public function exposedMoveNode(ActiveRecord $node, int $value, int $depth): voi // Create a mock context for testing compatibility $context = new \yii2\extensions\nestedsets\NodeContext( $node, - 'appendTo', 0, 0, ); From 7754328087f1a493b768e6974548fe5388f1326d Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 07:03:06 -0400 Subject: [PATCH 13/16] refactor: Ensure consistent parameter passing to beforeInsertNodeWithContext in NestedSetsBehavior. --- src/NestedSetsBehavior.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index bd5a063..c889ad5 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -379,17 +379,17 @@ public function beforeInsert(): void { match (true) { $this->operation === self::OPERATION_APPEND_TO && $this->node !== null => $this->beforeInsertNodeWithContext( - NodeContext::forAppendTo($this->node, $this->rightAttribute) + NodeContext::forAppendTo($this->node, $this->rightAttribute), ), $this->operation === self::OPERATION_INSERT_AFTER && $this->node !== null => $this->beforeInsertNodeWithContext( - NodeContext::forInsertAfter($this->node, $this->rightAttribute) + NodeContext::forInsertAfter($this->node, $this->rightAttribute), ), $this->operation === self::OPERATION_INSERT_BEFORE && $this->node !== null => $this->beforeInsertNodeWithContext( - NodeContext::forInsertBefore($this->node, $this->leftAttribute) + NodeContext::forInsertBefore($this->node, $this->leftAttribute), ), $this->operation === self::OPERATION_MAKE_ROOT => $this->beforeInsertRootNode(), $this->operation === self::OPERATION_PREPEND_TO && $this->node !== null => $this->beforeInsertNodeWithContext( - NodeContext::forPrependTo($this->node, $this->leftAttribute) + NodeContext::forPrependTo($this->node, $this->leftAttribute), ), default => throw new NotSupportedException( 'Method "' . get_class($this->getOwner()) . '::insert" is not supported for inserting new nodes.', From 1643781c6c87131ba5f8bd30f4b3e60cf514fa77 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 07:10:07 -0400 Subject: [PATCH 14/16] refactor: Cast targetPositionValue to int in getTargetPositionValue method. --- src/NodeContext.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NodeContext.php b/src/NodeContext.php index 9ad918c..60c6d4b 100644 --- a/src/NodeContext.php +++ b/src/NodeContext.php @@ -138,7 +138,7 @@ public function getTargetDepth(string $depthAttribute): int */ public function getTargetPositionValue(): int { - return $this->targetPositionValue ?? 0; + return (int) $this->targetPositionValue; } /** From 7db92b2726e4213c3fcb4c4d2673c4e66c14b78f Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 07:24:43 -0400 Subject: [PATCH 15/16] refactor: Update `NodeContext` properties to public and remove redundant `getter` methods. --- src/NestedSetsBehavior.php | 22 +++++++++++----------- src/NodeContext.php | 19 ++----------------- 2 files changed, 13 insertions(+), 28 deletions(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index c889ad5..0a0c6bc 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -377,6 +377,10 @@ public function beforeDelete(): void */ public function beforeInsert(): void { + if ($this->node?->getIsNewRecord() === true) { + throw new Exception('Can not create a node when the target node is new record.'); + } + match (true) { $this->operation === self::OPERATION_APPEND_TO && $this->node !== null => $this->beforeInsertNodeWithContext( NodeContext::forAppendTo($this->node, $this->rightAttribute), @@ -1069,10 +1073,6 @@ protected function applyTreeAttributeCondition(array &$condition): void */ protected function beforeInsertNode(int $value, int $depth): void { - if ($this->node?->getIsNewRecord() === true) { - throw new Exception('Can not create a node when the target node is new record.'); - } - if ($depth === 0 && $this->node?->isRoot() === true) { throw new Exception('Can not create a node when the target node is root.'); } @@ -1106,7 +1106,7 @@ protected function beforeInsertNode(int $value, int $depth): void */ protected function beforeInsertNodeWithContext(NodeContext $context): void { - $this->beforeInsertNode($context->getTargetPositionValue(), $context->getDepthLevelDelta()); + $this->beforeInsertNode($context->targetPositionValue, $context->depthLevelDelta); } /** @@ -1195,14 +1195,14 @@ protected function moveNode(NodeContext $context): void $ownerLeftValue = $this->getOwner()->getAttribute($this->leftAttribute); $ownerRightValue = $this->getOwner()->getAttribute($this->rightAttribute); - $depthOffset = $targetNodeDepthValue - $ownerDepthValue + $context->getDepthLevelDelta(); + $depthOffset = $targetNodeDepthValue - $ownerDepthValue + $context->depthLevelDelta; if ($this->treeAttribute === false || $targetNodeTreeValue === $currentOwnerTreeValue) { $subtreeSize = $ownerRightValue - $ownerLeftValue + 1; - $this->shiftLeftRightAttribute($context->getTargetPositionValue(), $subtreeSize); + $this->shiftLeftRightAttribute($context->targetPositionValue, $subtreeSize); - if ($ownerLeftValue >= $context->getTargetPositionValue()) { + if ($ownerLeftValue >= $context->targetPositionValue) { $ownerLeftValue += $subtreeSize; $ownerRightValue += $subtreeSize; } @@ -1249,7 +1249,7 @@ protected function moveNode(NodeContext $context): void [ $attribute => new Expression( $this->getDb()->quoteColumnName($attribute) . - sprintf('%+d', $context->getTargetPositionValue() - $ownerLeftValue), + sprintf('%+d', $context->targetPositionValue - $ownerLeftValue), ), ], $condition, @@ -1270,7 +1270,7 @@ protected function moveNode(NodeContext $context): void [ '>=', $attribute, - $context->getTargetPositionValue(), + $context->targetPositionValue, ], [ $this->treeAttribute => $targetNodeTreeValue, @@ -1284,7 +1284,7 @@ protected function moveNode(NodeContext $context): void $currentOwnerTreeValue, $depthOffset, $ownerLeftValue, - $context->getTargetPositionValue() - $ownerLeftValue, + $context->targetPositionValue - $ownerLeftValue, $ownerRightValue, ); $this->shiftLeftRightAttribute($ownerRightValue, $ownerLeftValue - $ownerRightValue - 1); diff --git a/src/NodeContext.php b/src/NodeContext.php index 60c6d4b..211d79a 100644 --- a/src/NodeContext.php +++ b/src/NodeContext.php @@ -26,8 +26,8 @@ final class NodeContext { public function __construct( private readonly ActiveRecord $targetNode, - private readonly int|null $targetPositionValue, - private readonly int $depthLevelDelta, + public readonly int $targetPositionValue, + public readonly int $depthLevelDelta, ) {} /** @@ -112,11 +112,6 @@ public static function forPrependTo(ActiveRecord $targetNode, string $leftAttrib ); } - public function getDepthLevelDelta(): int - { - return $this->depthLevelDelta; - } - /** * Returns the depth value of the target node. * @@ -131,16 +126,6 @@ public function getTargetDepth(string $depthAttribute): int return $this->targetNode->getAttribute($depthAttribute); } - /** - * Returns the target position value used for node movement. - * - * @return int Target position value, or 0 if not set. - */ - public function getTargetPositionValue(): int - { - return (int) $this->targetPositionValue; - } - /** * Returns the tree attribute value of the target node. * From ecb689d3a589a1fe6e69581ebe10c6ea08e80cf1 Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Wed, 2 Jul 2025 07:31:26 -0400 Subject: [PATCH 16/16] refactor: Move `beforeInsertNodeWithContext` method to private visibility and update documentation. --- src/NestedSetsBehavior.php | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/NestedSetsBehavior.php b/src/NestedSetsBehavior.php index 0a0c6bc..6b2ed20 100644 --- a/src/NestedSetsBehavior.php +++ b/src/NestedSetsBehavior.php @@ -1091,24 +1091,6 @@ protected function beforeInsertNode(int $value, int $depth): void $this->shiftLeftRightAttribute($value, 2); } - /** - * Prepares the current node for insertion using a NodeContext. - * - * Sets the left, right, and depth attributes of the node to be inserted, based on the provided context which - * encapsulates the target node, operation type, and calculated values. - * - * This method delegates to {@see beforeInsertNode()} using the values from the NodeContext, ensuring consistency - * and reducing code duplication. - * - * @param NodeContext $context Immutable context containing all necessary data for node insertion. - * - * @throws Exception if an unexpected error occurs during execution. - */ - protected function beforeInsertNodeWithContext(NodeContext $context): void - { - $this->beforeInsertNode($context->targetPositionValue, $context->depthLevelDelta); - } - /** * Prepares the current node for insertion as a root node in the nested set tree. * @@ -1359,6 +1341,24 @@ protected function shiftLeftRightAttribute(int $value, int $delta): void } } + /** + * Prepares the current node for insertion using a NodeContext. + * + * Sets the left, right, and depth attributes of the node to be inserted, based on the provided context which + * encapsulates the target node, operation type, and calculated values. + * + * This method delegates to {@see beforeInsertNode()} using the values from the NodeContext, ensuring consistency + * and reducing code duplication. + * + * @param NodeContext $context Immutable context containing all necessary data for node insertion. + * + * @throws Exception if an unexpected error occurs during execution. + */ + private function beforeInsertNodeWithContext(NodeContext $context): void + { + $this->beforeInsertNode($context->targetPositionValue, $context->depthLevelDelta); + } + /** * Creates a typed movement context based on operation and target node. *