From 635323c9f4bce637dbb95e8c105ea172cadb47eb Mon Sep 17 00:00:00 2001 From: Andrej Rypo Date: Mon, 5 Feb 2024 10:03:13 +0100 Subject: [PATCH] Builder message consolidation when no root found + more edge-case tests --- src/MaterializedPath/Support/ShadowNode.php | 9 +- src/MaterializedPath/TreeBuilder.php | 8 +- src/Recursive/TreeBuilder.php | 9 +- tests/mptree.phpt | 134 ++++++++++++++++---- tests/recursive.phpt | 112 +++++++++++----- tests/setup.php | 14 ++ 6 files changed, 216 insertions(+), 70 deletions(-) diff --git a/src/MaterializedPath/Support/ShadowNode.php b/src/MaterializedPath/Support/ShadowNode.php index 1194493..4bf34d7 100644 --- a/src/MaterializedPath/Support/ShadowNode.php +++ b/src/MaterializedPath/Support/ShadowNode.php @@ -8,7 +8,6 @@ use Dakujem\Oliva\MovableNodeContract; use Dakujem\Oliva\Node; use Dakujem\Oliva\TreeNodeContract; -use LogicException; /** * Shadow node used internally when building materialized path trees. @@ -18,13 +17,9 @@ final class ShadowNode extends Node implements MovableNodeContract { public function __construct( - ?MovableNodeContract $node, - ?ShadowNode $parent = null, + ?MovableNodeContract $node ) { - parent::__construct( - data: $node, - parent: $parent, - ); + parent::__construct(data: $node); } /** diff --git a/src/MaterializedPath/TreeBuilder.php b/src/MaterializedPath/TreeBuilder.php index dc1490f..4ebff14 100644 --- a/src/MaterializedPath/TreeBuilder.php +++ b/src/MaterializedPath/TreeBuilder.php @@ -73,7 +73,7 @@ public function build(iterable $input): TreeNodeContract $result = $this->processInput($input); $root = $result->root(); if (null === $root) { - throw (new InvalidInputData('Corrupted input, no tree created.')) + throw (new InvalidInputData('No root node found in the input collection.')) ->tag('result', $result); } return $root; @@ -157,9 +157,6 @@ private function buildShadowTree( return $register->pull([]); } - /** - * Recursion. - */ private function connectNode(ShadowNode $node, array $vector, Register $register): void { $existingNode = $register->pull($vector); @@ -181,6 +178,9 @@ private function connectNode(ShadowNode $node, array $vector, Register $register $this->connectAncestry($node, $vector, $register); } + /** + * Recursive. + */ private function connectAncestry(ShadowNode $node, array $vector, Register $register): void { // When the node is a root itself, abort recursion. diff --git a/src/Recursive/TreeBuilder.php b/src/Recursive/TreeBuilder.php index b89d952..83197f2 100644 --- a/src/Recursive/TreeBuilder.php +++ b/src/Recursive/TreeBuilder.php @@ -4,7 +4,6 @@ namespace Dakujem\Oliva\Recursive; -use Dakujem\Oliva\Exceptions\ConfigurationIssue; use Dakujem\Oliva\Exceptions\ExtractorReturnValueIssue; use Dakujem\Oliva\Exceptions\InvalidInputData; use Dakujem\Oliva\Exceptions\InvalidNodeFactoryReturnValue; @@ -78,10 +77,8 @@ public function __construct( int|string|null $parentReference, int|string|null $selfReference, ): bool => $parentReference === $root; - } elseif (is_callable($root)) { - $this->root = $root; } else { - throw new ConfigurationIssue('Invalid argument: The root detector must either be a string|int|null to compare against the parent refs, or a callable that returns truthy if a root is detected.'); + $this->root = $root; } } @@ -170,7 +167,7 @@ private function processData( } if (!$rootFound) { - throw (new InvalidInputData('No root node found in the data.')) + throw (new InvalidInputData('No root node found in the input collection.')) ->tag('nodes', $nodeRegister); } @@ -189,6 +186,8 @@ private function processData( } /** + * Recursive. + * * @param array $nodeRegister * @param array> $childRegister */ diff --git a/tests/mptree.phpt b/tests/mptree.phpt index d0e3b2f..e17823b 100644 --- a/tests/mptree.phpt +++ b/tests/mptree.phpt @@ -5,18 +5,18 @@ declare(strict_types=1); namespace Dakujem\Test; use Dakujem\Oliva\Exceptions\ExtractorReturnValueIssue; +use Dakujem\Oliva\Exceptions\InternalLogicException; use Dakujem\Oliva\Exceptions\InvalidInputData; use Dakujem\Oliva\Exceptions\InvalidNodeFactoryReturnValue; use Dakujem\Oliva\Exceptions\InvalidTreePath; -use Dakujem\Oliva\Iterator\PreOrderTraversal; use Dakujem\Oliva\MaterializedPath\Path; use Dakujem\Oliva\MaterializedPath\Support\AlmostThere; +use Dakujem\Oliva\MaterializedPath\Support\ShadowNode; use Dakujem\Oliva\MaterializedPath\TreeBuilder; 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'; @@ -31,20 +31,6 @@ class Item } (function () { - $toArray = function (TreeNodeContract $root) { - $it = new PreOrderTraversal($root, fn( - TreeNodeContract $node, - array $vector, - int $seq, - int $counter, - ): string => '>' . implode('.', $vector)); - return array_map(function (Node $item): string { - $data = $item->data(); - return null !== $data ? "[$data->id]" : 'root'; - }, iterator_to_array($it)); - }; - - $data = [ new Item(id: 1, path: '000'), new Item(id: 2, path: '001'), @@ -72,6 +58,8 @@ class Item Assert::type(Node::class, $almost->root()); Assert::null($almost->root()?->data()); Assert::type(Item::class, Seed::firstOf($almost->root()?->children())?->data()); + Assert::type(ShadowNode::class, $almost->shadow()); + Assert::same($almost->root(), $almost->shadow()->data()); Assert::same([ '>' => 'root', @@ -83,7 +71,7 @@ class Item '>3' => '[6]', '>3.0' => '[5]', '>5' => '[9]', // note the index `4` being skipped - this is expected, because the node with ID 7 is not connected to the root and is omitted during reconstruction by the shadow tree, but the index is not changed - ], $toArray($almost->root())); + ], TreeTesterTool::visualize($almost->root())); $vectorExtractor = Path::fixed( @@ -103,7 +91,7 @@ class Item // an empty input can not result in any tree Assert::throws(function () use ($builder) { $builder->build([]); - }, InvalidInputData::class, 'Corrupted input, no tree created.'); + }, InvalidInputData::class, 'No root node found in the input collection.'); $failingBuilder = new TreeBuilder(fn() => null, fn() => []); @@ -127,9 +115,9 @@ class Item Assert::throws(function () use ($duplicateVector) { $duplicateVector->build([null, null]); }, InvalidInputData::class, 'Duplicate node vector: any'); +})(); - - +(function () { $collection = [ new Item(id: 0, path: ''), // the root new Item(id: 1, path: '.0'), @@ -167,7 +155,7 @@ class Item '>3' => '[6]', '>3.0' => '[5]', '>5' => '[9]', // note the index `4` being skipped - this is expected - ], $toArray($root)); + ], TreeTesterTool::visualize($root)); Tree::reindexTree($root, fn(Node $node) => $node->data()->id, null); @@ -181,7 +169,7 @@ class Item '>6' => '[6]', '>6.5' => '[5]', '>9' => '[9]', - ], $toArray($root)); + ], TreeTesterTool::visualize($root)); Tree::reindexTree($root, null, fn(Node $a, Node $b) => $a->data()->path <=> $b->data()->path); Assert::same([ @@ -194,5 +182,105 @@ class Item '>6.5' => '[5]', // .2.0 '>3' => '[3]', // .3 '>9' => '[9]', // .9 - ], $toArray($root)); + ], TreeTesterTool::visualize($root)); +})(); + +(function () { + $builder = new TreeBuilder( + node: fn(?string $path) => new Node($path), + vector: Path::fixed( + 3, + fn(?string $path) => $path, + ), + ); + $almost = $builder->processInput( + input: [], + ); + Assert::same(null, $almost->shadow()); + Assert::same(null, $almost->root()); + + Assert::throws( + function () use ($builder) { + $builder->build( + input: [], + ); + }, + InvalidInputData::class, + 'No root node found in the input collection.', + ); + + $root = $builder->build( + input: [null], + ); + Assert::type(Node::class, $root); + Assert::same(null, $root->data()); + $root = $builder->build( + input: [''], + ); + Assert::type(Node::class, $root); + Assert::same('', $root->data()); + + Assert::throws( + function () use ($builder) { + $builder->build( + input: ['000'], + ); + }, + InvalidInputData::class, + 'No root node found in the input collection.', + ); + + // Here no root node will be found. + Assert::throws( + function () use ($builder) { + $builder->build( + input: ['007000', '007', '007001'], + ); + }, + InvalidInputData::class, + 'No root node found in the input collection.', + ); + + // However, a shadow tree will be returned and the node can be accessed, as the first shadow-child in these cases: + $almost = $builder->processInput( + input: ['000'], + ); + $node = $almost->shadow()->child(0)->data(); + Assert::type(Node::class, $node); + Assert::same('000', $node->data()); + + $almost = $builder->processInput( + input: ['007000', '007', '007001'], + ); + $node = $almost->shadow()->child(0)->data(); + Assert::type(Node::class, $node); + Assert::same('007', $node->data()); + Assert::count(2, $node->children()); + Assert::same('007000', $node->child(0)->data()); + Assert::same('007001', $node->child(1)->data()); +})(); + +(function () { + $shadow = new ShadowNode(null); + Assert::same(null, $shadow->data()); + + $shadow = new ShadowNode($node = new Node(null)); + Assert::same($node, $shadow->data()); + + $parent = new ShadowNode(new Node('root')); + $shadow->setParent($parent); + Assert::same($parent, $shadow->parent()); + Assert::same('root', $shadow->parent()?->data()?->data()); + + Assert::same([], $shadow->children()); + $shadow->addChild(new ShadowNode(new Node('child')), 0); + Assert::count(1, $shadow->children()); + Assert::same('child', $shadow->child(0)?->data()?->data()); + + Assert::throws(function () use ($shadow) { + $shadow->addChild(new Node('another')); + }, InternalLogicException::class, 'Invalid use of a shadow node. Only shadow nodes can be children of shadow nodes.'); + 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 diff --git a/tests/recursive.phpt b/tests/recursive.phpt index 7c42f5a..a8b28d8 100644 --- a/tests/recursive.phpt +++ b/tests/recursive.phpt @@ -4,13 +4,12 @@ declare(strict_types=1); namespace Dakujem\Test; +use Dakujem\Oliva\Exceptions\ExtractorReturnValueIssue; use Dakujem\Oliva\Exceptions\InvalidInputData; use Dakujem\Oliva\Iterator\Filter; -use Dakujem\Oliva\Iterator\PreOrderTraversal; use Dakujem\Oliva\Node; use Dakujem\Oliva\Recursive\TreeBuilder; use Dakujem\Oliva\Seed; -use Dakujem\Oliva\TreeNodeContract; use Tester\Assert; require_once __DIR__ . '/setup.php'; @@ -18,34 +17,21 @@ require_once __DIR__ . '/setup.php'; class Item { public function __construct( - public int $id, - public ?int $parent, + public mixed $id, + public mixed $parent, ) { } } (function () { - $toArray = function (TreeNodeContract $root, callable $mod = null) { - $it = new PreOrderTraversal($root, fn( - TreeNodeContract $node, - array $vector, - int $seq, - int $counter, - ): string => '>' . implode('.', $vector)); - return array_map(function (Node $item): string { - $data = $item->data(); - return null !== $data ? "[$data->id]" : 'root'; - }, iterator_to_array($mod ? $mod($it) : $it)); - }; - $data = [ new Item(id: 0, parent: null), ]; $builder = new TreeBuilder( - node: fn(?Item $item) => new Node($item), - selfRef: fn(?Item $item) => $item?->id, - parentRef: fn(?Item $item) => $item?->parent, + node: fn(?Item $item): Node => new Node($item), + selfRef: fn(?Item $item): int => $item?->id, + parentRef: fn(?Item $item): ?int => $item?->parent, ); $tree = $builder->build( @@ -54,7 +40,7 @@ class Item Assert::same([ '>' => '[0]', - ], $toArray($tree)); + ], TreeTesterTool::visualize($tree)); $data = [ @@ -68,12 +54,6 @@ class Item new Item(id: 3, parent: 4), ]; - $builder = new TreeBuilder( - node: fn(?Item $item) => new Node($item), - selfRef: fn(?Item $item) => $item?->id, - parentRef: fn(?Item $item) => $item?->parent, - ); - $tree = $builder->build( input: $data, ); @@ -87,7 +67,7 @@ class Item '>1' => '[5]', '>1.0' => '[6]', '>2' => '[3]', - ], $toArray($tree)); + ], TreeTesterTool::visualize($tree)); //new Filter($it, Seed::omitNull()); @@ -100,7 +80,7 @@ class Item '>1' => '[5]', '>1.0' => '[6]', '>2' => '[3]', - ], $toArray($tree, $withoutRoot)); + ], TreeTesterTool::visualize($tree, $withoutRoot)); $filter = new Filter($collection = [ new Node(null), @@ -115,11 +95,81 @@ class Item Assert::throws( fn() => $builder->build([]), InvalidInputData::class, - 'No root node found in the data.', + 'No root node found in the input collection.', ); Assert::throws( fn() => $builder->build([new Item(id: 7, parent: 42)]), InvalidInputData::class, - 'No root node found in the data.', + 'No root node found in the input collection.', + ); +})(); + + +(function () { + $builder = new TreeBuilder( + node: fn(?Item $item): Node => new Node($item), + selfRef: fn(?Item $item): mixed => $item?->id, + parentRef: fn(?Item $item): mixed => $item?->parent, + root: fn(?Item $item): bool => $item->id === 'unknown', + ); + + /** @var Node $root */ + $root = $builder->build([ + new Item(id: 'frodo', parent: 'unknown'), + new Item(id: 'sam', parent: 'unknown'), + new Item(id: 'gandalf', parent: 'unknown'), + new Item(id: 'unknown', parent: 'unknown'), + ]); + + Assert::same('unknown', $root->data()?->id); + Assert::same('unknown', $root->data()?->parent); + Assert::count(3, $root->children()); + + Assert::same([ + '>' => '[unknown]', + '>0' => '[frodo]', + '>1' => '[sam]', + '>2' => '[gandalf]', + ], TreeTesterTool::visualize($root)); +})(); + +(function () { + $builder = new TreeBuilder( + node: fn(?Item $item): Node => new Node($item), + selfRef: fn(?Item $item): mixed => $item?->id, + parentRef: fn(?Item $item): mixed => $item?->parent, + ); + + Assert::throws( + function () use ($builder) { + $builder->build([ + new Item(id: null, parent: null), + ]); + }, + ExtractorReturnValueIssue::class, + 'Invalid "self reference" returned by the extractor. Requires a int|string unique to the given node.', + ); + + Assert::throws( + function () use ($builder) { + $builder->build([ + new Item(id: 123, parent: 4.2), + ]); + }, + ExtractorReturnValueIssue::class, + 'Invalid "parent reference" returned by the extractor. Requires a int|string uniquely pointing to "self reference" of another node, or `null`.', + ); + + + Assert::throws( + function () use ($builder) { + $builder->build([ + new Item(id: 123, parent: null), + new Item(id: 42, parent: 123), + new Item(id: 42, parent: 5), + ]); + }, + ExtractorReturnValueIssue::class, + 'Duplicate node reference: 42', ); })(); diff --git a/tests/setup.php b/tests/setup.php index a1f5d22..66c5b5d 100644 --- a/tests/setup.php +++ b/tests/setup.php @@ -50,6 +50,20 @@ public static function reduce( } return $carry; } + + public static function visualize(TreeNodeContract $root, callable $iteratorDecorator = null):array + { + $it = new PreOrderTraversal($root, fn( + TreeNodeContract $node, + array $vector, + int $seq, + int $counter, + ): string => '>' . implode('.', $vector)); + return array_map(function (Node $item): string { + $data = $item->data(); + return null !== $data ? "[$data->id]" : 'root'; + }, iterator_to_array($iteratorDecorator ? $iteratorDecorator($it) : $it)); + } } final class Manipulator