Optimize Tree::link method

changelog.md

@@ -11,6 +11,9 @@ Please report any issues.
 
 - `Node::addChild()` method (through the implementation in `MovableNodeTrait::addChild`) now allows adding the same node
   under the same key without having an effect, previously it would throw the `ChildKeyCollision` exception
+- `Tree::link()` method now allows for turning off the duplicate node handling mechanism,
+  which prevents adding the same child multiple times
+  - used for performance optimization when linking multiple children in a sequence
 
 
 ## v1.2.x

src/Tree.php

@@ -27,25 +27,36 @@ final class Tree
      * that link will be broken and the original parent will be returned.
      *
      * Null is returned in all other cases.
+     *
+     * Note:
+     *   Because of the node lookup, this method may be very slow if linking to a node with a large number
+     *   of children with the duplicate linking handling mechanism on.
+     *   In such cases, making sure there are no duplicates beforehand,
+     *   the `$handleDuplicateLinking` parameter may be set to `false`.
      */
     public static function link(
         MovableNodeContract $node,
         MovableNodeContract $parent,
         string|int|null $key = null,
+        bool $handleDuplicateLinking = true,
     ): ?MovableNodeContract {
-        // If the current parent is different, first detach the node.
         $currentParent = $node->parent();
-        if ($currentParent === $parent) {
-            // Already linked, but check the link the other way around.
-            self::adoptChild($parent, $node, $key);
-            return null;
-        }
-        if (null !== $currentParent) {
+
+        // If the node already has a parent, but it is different from the target one, detach the node first.
+        if (null !== $currentParent && $currentParent !== $parent) {
             $originalParent = self::unlink($node);
+            $currentParent = null;
         }
-        $node->setParent($parent);
 
-        self::adoptChild($parent, $node, $key);
+        // Set the parent reference (child-to-parent link).
+        if (null === $currentParent) {
+            $node->setParent($parent);
+        }
+
+        // Create the parent-to-child link.
+        self::adoptChild($parent, $node, $key, $handleDuplicateLinking);
+
+        // If a parent was unlinked during the process, return it.
         return $originalParent ?? null;
     }
 
@@ -155,13 +166,18 @@ public static function reindexTree(
 
     /**
      * @internal
+     * Note: $parent->childKey($child) is slow on nodes with many children when repeatedly called.
      */
     private static function adoptChild(
         MovableNodeContract $parent,
         MovableNodeContract $child,
-        string|int|null $key = null
+        string|int|null $key,
+        bool $handleDuplicateLinking,
     ): void {
-        $existing = $parent->childKey($child);
+        // Note:
+        // The duplicate adding prevention is very slow for nodes with many siblings due to the child node lookup
+        // (done by the `childKey` method), if not optimized internally.
+        $existing = $handleDuplicateLinking ? $parent->childKey($child) : null;
         if (
             null !== $existing &&
             $parent->child($existing) === $child &&

tests/tree.phpt

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Dakujem\Test;
 
+use Dakujem\Oliva\Exceptions\ChildKeyCollision;
 use Dakujem\Oliva\Exceptions\NodeNotMovable;
 use Dakujem\Oliva\Node;
 use Dakujem\Oliva\Simple\NodeBuilder;
@@ -187,3 +188,42 @@ require_once __DIR__ . '/setup.php';
     Assert::same(['two' => $node2, 'three' => $node1], $parent->children());
 })();
 
+
+(function () {
+    //
+    // Duplicate linking of the same node without duplicate handling does not prevent adding the same node multiple times.
+    //
+
+    $node = new Node(null);
+    $parent = new Node(null);
+    Tree::link($node, $parent); // default index `0`
+
+    // this will add the same node duplicate under `1`,
+    // behaves like an array push
+    Tree::link($node, $parent, handleDuplicateLinking: false);
+    Assert::same([0 => $node, 1 => $node], $parent->children());
+    // this will add the same node yet again under `foo`
+    Tree::link($node, $parent, 'foo', handleDuplicateLinking: false);
+    Assert::same([0 => $node, 1 => $node, 'foo' => $node], $parent->children());
+
+    $node1 = new Node(null);
+    $node2 = new Node(null);
+    $children = ['one' => $node1, 'two' => $node2];
+    $parent = new Node(null, children: $children);
+    Assert::same($children, $parent->children()); // sanity check
+
+    // Attempting to add the same node under the same key without the duplicate node handling mechanism still has no effect.
+    // Also, attempting to add the same node under a different key without the duplicate node handling mechanism
+    // still results in a ChildKeyCollision exception.
+    Tree::link($node1, $parent, 'one', handleDuplicateLinking: false);
+    Assert::same($children, $parent->children());
+    Assert::throws(function () use ($parent, $node1) {
+        Tree::link($node1, $parent, 'two', handleDuplicateLinking: false);
+    }, ChildKeyCollision::class);
+
+    // Without the duplicate node handling mechanism, this call will allow adding the node as a duplicate child,
+    // possibly causing issues in the tree structure, but improving performance of a large number of subsequent calls.
+    Tree::link($node1, $parent, 'three', handleDuplicateLinking: false);
+    Assert::same(['one' => $node1, 'two' => $node2, 'three' => $node1], $parent->children());
+})();
+