Merge pull request #19 from neos/bugfix/refactor-node-get-child-nodes

BUGFIX: Refactor Node::getChildNodes rector refactoring
neos · May 2, 2023 · b046f7e · b046f7e
2 parents fcb6ae9 + 652ec33
commit b046f7e
Show file tree

Hide file tree

Showing 7 changed files with 166 additions and 39 deletions.
diff --git a/src/ContentRepository90/Rules/NodeGetChildNodesRector.php b/src/ContentRepository90/Rules/NodeGetChildNodesRector.php
@@ -48,22 +48,27 @@ public function refactor(Node $node) : ?Node
         $nodeTypeFilterExpr = null;
         $limitExpr = null;
         $offsetExpr = null;
-        if (count($node->args) >= 1) {
-            $nodeTypeFilterExpr = $node->args[0];
-            assert($nodeTypeFilterExpr instanceof Node\Arg);
-            $nodeTypeFilterExpr = $nodeTypeFilterExpr->value;
-        }
-        if (count($node->args) >= 2) {
-            $limitExpr = $node->args[1];
-            assert($limitExpr instanceof Node\Arg);
-            $limitExpr = $limitExpr->value;
-        }
-        if (count($node->args) >= 3) {
-            $offsetExpr = $node->args[2];
-            assert($offsetExpr instanceof Node\Arg);
-            $offsetExpr = $offsetExpr->value;
-        }
 
+        foreach ($node->args as $index => $arg) {
+            $argumentName = $arg?->name?->name;
+            $namedArgument = $argumentName !== null;
+
+            if (($namedArgument && $argumentName === 'nodeTypeFilter') ||  !$namedArgument && $index === 0) {
+                assert($arg instanceof Node\Arg);
+
+                if ($arg->value instanceof Node\Scalar\String_) {
+                    $nodeTypeFilterExpr = $arg->value;
+                }
+            }
+            if (($namedArgument && $argumentName === 'limit') ||  !$namedArgument && $index === 1) {
+                assert($arg instanceof Node\Arg);
+                $limitExpr = $arg->value;
+            }
+            if (($namedArgument && $argumentName === 'offset') ||  !$namedArgument && $index === 2) {
+                assert($arg instanceof Node\Arg);
+                $offsetExpr = $arg->value;
+            }
+        }
 
         $this->nodesToAddCollector->addNodesBeforeNode(
             [

diff --git a/src/ContentRepository90/Rules/Traits/SubgraphTrait.php b/src/ContentRepository90/Rules/Traits/SubgraphTrait.php
@@ -21,35 +21,29 @@ private function subgraph_findChildNodes(
         Expr $nodeVariable,
         ?Expr $nodeTypeConstraintsFilterString = null,
         ?Expr $limit = null,
-        ?Expr $offset = null
-    ): Expr
-    {
+        ?Expr $offset = null,
+    ): Expr {
+        $args = [];
+
         if ($nodeTypeConstraintsFilterString) {
-            $filter = $this->nodeFactory->createStaticCall(
-                FindChildNodesFilter::class,
-                'nodeTypeConstraints',
-                [
-                    $nodeTypeConstraintsFilterString
-                ]
-            );
-        } else {
-            $filter = $this->nodeFactory->createStaticCall(
-                FindChildNodesFilter::class,
-                'all'
-            );
+            $args = [
+                'nodeTypeConstraints' => $nodeTypeConstraintsFilterString
+            ];
         }
 
         if ($limit || $offset) {
-            $filter = $this->nodeFactory->createMethodCall(
-                $filter,
-                'withPagination',
-                [
-                    $limit,
-                    $offset
-                ]
-            );
+            $args['pagination'] = [
+                'limit' => $limit,
+                'offset' => $offset
+            ];
         }
 
+        $filter = $this->nodeFactory->createStaticCall(
+            FindChildNodesFilter::class,
+            'create',
+            $args
+        );
+
         return $this->nodeFactory->createMethodCall(
             'subgraph',
             'findChildNodes',

diff --git a/tests/Rules/NodeGetChildNodesRector/Fixture/all-with-pagination-named-arguments.php.inc b/tests/Rules/NodeGetChildNodesRector/Fixture/all-with-pagination-named-arguments.php.inc
@@ -0,0 +1,32 @@
+<?php
+
+use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
+
+class SomeClass
+{
+    public function run(Node $node)
+    {
+        foreach ($node->getChildNodes(offset: 100, limit: 10) as $node) {
+        }
+    }
+}
+
+?>
+-----
+<?php
+
+use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
+
+class SomeClass
+{
+    public function run(Node $node)
+    {
+        $subgraph = $this->contentRepositoryRegistry->subgraphForNode($node);
+        // TODO 9.0 migration: Try to remove the iterator_to_array($nodes) call.
+
+        foreach (iterator_to_array($subgraph->findChildNodes($node->nodeAggregateId, \Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindChildNodesFilter::create(pagination: ['limit' => 10, 'offset' => 100]))) as $node) {
+        }
+    }
+}
+
+?>
diff --git a/tests/Rules/NodeGetChildNodesRector/Fixture/all-with-pagination.php.inc b/tests/Rules/NodeGetChildNodesRector/Fixture/all-with-pagination.php.inc
@@ -0,0 +1,32 @@
+<?php
+
+use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
+
+class SomeClass
+{
+    public function run(Node $node)
+    {
+        foreach ($node->getChildNodes(null, 10,100) as $node) {
+        }
+    }
+}
+
+?>
+-----
+<?php
+
+use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
+
+class SomeClass
+{
+    public function run(Node $node)
+    {
+        $subgraph = $this->contentRepositoryRegistry->subgraphForNode($node);
+        // TODO 9.0 migration: Try to remove the iterator_to_array($nodes) call.
+
+        foreach (iterator_to_array($subgraph->findChildNodes($node->nodeAggregateId, \Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindChildNodesFilter::create(pagination: ['limit' => 10, 'offset' => 100]))) as $node) {
+        }
+    }
+}
+
+?>
diff --git a/...ildNodesRector/Fixture/some_class.php.inc → ...deGetChildNodesRector/Fixture/all.php.inc b/...ildNodesRector/Fixture/some_class.php.inc → ...deGetChildNodesRector/Fixture/all.php.inc
@@ -24,7 +24,7 @@ class SomeClass
         $subgraph = $this->contentRepositoryRegistry->subgraphForNode($node);
         // TODO 9.0 migration: Try to remove the iterator_to_array($nodes) call.
 
-        foreach (iterator_to_array($subgraph->findChildNodes($node->nodeAggregateId, \Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindChildNodesFilter::all())) as $node) {
+        foreach (iterator_to_array($subgraph->findChildNodes($node->nodeAggregateId, \Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindChildNodesFilter::create())) as $node) {
         }
     }
 }

diff --git a/tests/Rules/NodeGetChildNodesRector/Fixture/nodetype-filter-with-pagination.php.inc b/tests/Rules/NodeGetChildNodesRector/Fixture/nodetype-filter-with-pagination.php.inc
@@ -0,0 +1,32 @@
+<?php
+
+use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
+
+class SomeClass
+{
+    public function run(Node $node)
+    {
+        foreach ($node->getChildNodes('Neos.Neos:Document', 10, 100) as $node) {
+        }
+    }
+}
+
+?>
+-----
+<?php
+
+use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
+
+class SomeClass
+{
+    public function run(Node $node)
+    {
+        $subgraph = $this->contentRepositoryRegistry->subgraphForNode($node);
+        // TODO 9.0 migration: Try to remove the iterator_to_array($nodes) call.
+
+        foreach (iterator_to_array($subgraph->findChildNodes($node->nodeAggregateId, \Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindChildNodesFilter::create(nodeTypeConstraints: 'Neos.Neos:Document', pagination: ['limit' => 10, 'offset' => 100]))) as $node) {
+        }
+    }
+}
+
+?>
diff --git a/tests/Rules/NodeGetChildNodesRector/Fixture/nodetype-filter.php.inc b/tests/Rules/NodeGetChildNodesRector/Fixture/nodetype-filter.php.inc
@@ -0,0 +1,32 @@
+<?php
+
+use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
+
+class SomeClass
+{
+    public function run(Node $node)
+    {
+        foreach ($node->getChildNodes('Neos.Neos:Document') as $node) {
+        }
+    }
+}
+
+?>
+-----
+<?php
+
+use Neos\ContentRepository\Core\Projection\ContentGraph\Node;
+
+class SomeClass
+{
+    public function run(Node $node)
+    {
+        $subgraph = $this->contentRepositoryRegistry->subgraphForNode($node);
+        // TODO 9.0 migration: Try to remove the iterator_to_array($nodes) call.
+
+        foreach (iterator_to_array($subgraph->findChildNodes($node->nodeAggregateId, \Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindChildNodesFilter::create(nodeTypeConstraints: 'Neos.Neos:Document'))) as $node) {
+        }
+    }
+}
+
+?>