From 04f5b5d972541446371e6623e5da0794f6372db3 Mon Sep 17 00:00:00 2001
From: Andrej Rypo <xrypak@gmail.com>
Date: Wed, 27 Mar 2024 13:28:58 +0100
Subject: [PATCH] Squish repeated ShadowNode reconstruction bug

- fixes cases when repeated calls to `ShadowNode::reconstructRealTree` would cause index collisions
---
 changelog.md                                |  1 +
 src/MaterializedPath/Support/ShadowNode.php |  5 +--
 tests/mptree.phpt                           | 38 ++++++++++++++++++++-
 3 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/changelog.md b/changelog.md
index 0ac7e01..a532f6f 100644
--- a/changelog.md
+++ b/changelog.md
@@ -11,6 +11,7 @@ Please report any issues.
 
 - Added `Seed::chain` method for iterable collection chaining: the new method takes over `Seed::merged` which becomes its alias for backward compatibility.
 - Deprecated `Seed::merged`, use `Seed::chain` instead.
+- Fixed repeated calls to `ShadowNode::reconstructRealTree` causing index collisions.
 
 
 ## v1.0
diff --git a/src/MaterializedPath/Support/ShadowNode.php b/src/MaterializedPath/Support/ShadowNode.php
index 4bf34d7..dc268c0 100644
--- a/src/MaterializedPath/Support/ShadowNode.php
+++ b/src/MaterializedPath/Support/ShadowNode.php
@@ -34,11 +34,12 @@ public function __construct(
     public function reconstructRealTree(): ?TreeNodeContract
     {
         $realNode = $this->realNode();
+        $realNode?->removeChildren();
         /** @var self $child */
         foreach ($this->children() as $index => $child) {
             $realChild = $child->realNode();
-            if (null !== $realNode && null !== $realChild) {
-                $realNode->addChild($realChild, $index);
+            if (null !== $realChild) {
+                $realNode?->addChild($realChild, $index);
                 $realChild->setParent($realNode);
             }
             $child->reconstructRealTree();
diff --git a/tests/mptree.phpt b/tests/mptree.phpt
index e17823b..3e0f746 100644
--- a/tests/mptree.phpt
+++ b/tests/mptree.phpt
@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace Dakujem\Test;
 
+use Dakujem\Oliva\DataNodeContract;
 use Dakujem\Oliva\Exceptions\ExtractorReturnValueIssue;
 use Dakujem\Oliva\Exceptions\InternalLogicException;
 use Dakujem\Oliva\Exceptions\InvalidInputData;
@@ -17,6 +18,7 @@ use Dakujem\Oliva\MovableNodeContract;
 use Dakujem\Oliva\Node;
 use Dakujem\Oliva\Seed;
 use Dakujem\Oliva\Tree;
+use Dakujem\Oliva\TreeNodeContract;
 use Tester\Assert;
 
 require_once __DIR__ . '/setup.php';
@@ -258,6 +260,12 @@ class Item
     Assert::count(2, $node->children());
     Assert::same('007000', $node->child(0)->data());
     Assert::same('007001', $node->child(1)->data());
+
+    // The reconstruction works even when the tree contains nodes not connected to the root.
+    Assert::same(null, $almost->root());
+    Assert::same(null, $almost->shadow()->data());
+    Assert::same($node, $almost->shadow()->child(0)->child(0)->data()->parent());
+    Assert::same($node, $almost->shadow()->child(0)->child(1)->data()->parent());
 })();
 
 (function () {
@@ -283,4 +291,32 @@ class Item
     Assert::throws(function () use ($shadow) {
         $shadow->setParent(new Node('new parent'));
     }, InternalLogicException::class, 'Invalid use of a shadow node. Only shadow nodes can be parents of shadow nodes.');
-})();
\ No newline at end of file
+})();
+
+// This test checks an edge-case where reconstructing a shadow root led to an index collision.
+(function () {
+    $builder = new TreeBuilder(
+        node: fn(?string $path) => new Node($path),
+        vector: Path::fixed(
+            3,
+            fn(?string $path) => $path,
+        ),
+    );
+
+    $normalizer = function (TreeNodeContract $node) use (&$normalizer): array {
+        return [
+            'data' => $node instanceof DataNodeContract ? $node->data() : null,
+            'children' => array_map(fn($child) => $normalizer($child), $node->children()),
+        ];
+    };
+
+    $almost = $builder->processInput(
+        input: ['', '007000', '006', '007', '007001', '005001', '004', '005001009',],
+    );
+
+    Assert::same(
+        $normalizer($almost->root()),
+        $normalizer($almost->shadow()->reconstructRealTree()),
+    );
+})();
+