From 4167f9b35fdfdd40baca063d4a3de0f08f2396d8 Mon Sep 17 00:00:00 2001 From: Martin Ficzel Date: Tue, 7 Nov 2023 22:10:46 +0100 Subject: [PATCH] FEATURE: Improve flowQuery `find` and `children` operation The flowQuery operations find and children are fixed optimized for the new cr to use a combined query for nodetype and property criteria. The number of performed db-queries is the number of contextNodes times the number of filterGroups (comma separated parts) In addition the `find` operation now can also handle single property criteria and does not rely on having an instanceof filter first. --- .../Classes/Filter/NodeFilterCriteria.php | 16 ++ .../Filter/NodeFilterCriteriaGroup.php | 32 ++++ .../Filter/NodeFilterCriteriaGroupFactory.php | 150 ++++++++++++++++++ .../FlowQueryOperations/ChildrenOperation.php | 20 ++- .../FlowQueryOperations/FindOperation.php | 30 ++++ .../NodeFilterCriteriaGroupFactoryTest.php | 147 +++++++++++++++++ .../Features/Fusion/FlowQuery.feature | 2 + 7 files changed, 396 insertions(+), 1 deletion(-) create mode 100644 Neos.ContentRepository.NodeAccess/Classes/Filter/NodeFilterCriteria.php create mode 100644 Neos.ContentRepository.NodeAccess/Classes/Filter/NodeFilterCriteriaGroup.php create mode 100644 Neos.ContentRepository.NodeAccess/Classes/Filter/NodeFilterCriteriaGroupFactory.php create mode 100644 Neos.ContentRepository.NodeAccess/Tests/Unit/Filter/NodeFilterCriteriaGroupFactoryTest.php diff --git a/Neos.ContentRepository.NodeAccess/Classes/Filter/NodeFilterCriteria.php b/Neos.ContentRepository.NodeAccess/Classes/Filter/NodeFilterCriteria.php new file mode 100644 index 00000000000..f36189650d7 --- /dev/null +++ b/Neos.ContentRepository.NodeAccess/Classes/Filter/NodeFilterCriteria.php @@ -0,0 +1,16 @@ + + */ +readonly class NodeFilterCriteriaGroup implements \IteratorAggregate +{ + /** + * @var array + */ + protected array $criteria; + + public function __construct(NodeFilterCriteria ...$criteria) + { + $this->criteria = array_values($criteria); + } + + /** + * @return Traversable + */ + public function getIterator(): Traversable + { + return new \ArrayIterator($this->criteria); + } + +} diff --git a/Neos.ContentRepository.NodeAccess/Classes/Filter/NodeFilterCriteriaGroupFactory.php b/Neos.ContentRepository.NodeAccess/Classes/Filter/NodeFilterCriteriaGroupFactory.php new file mode 100644 index 00000000000..7f493576012 --- /dev/null +++ b/Neos.ContentRepository.NodeAccess/Classes/Filter/NodeFilterCriteriaGroupFactory.php @@ -0,0 +1,150 @@ +withAdditionalNodeTypeName(NodeTypeName::fromString($operand)); + break; + case '!instanceof': + $disallowedNodeTypeNames = $disallowedNodeTypeNames->withAdditionalNodeTypeName(NodeTypeName::fromString($operand)); + break; + case '=': + $propertyCriteria[] = PropertyValueEquals::create(PropertyName::fromString($propertyPath), $operand, true); + break; + case '!=': + $propertyCriteria[] = NegateCriteria::create(PropertyValueEquals::create(PropertyName::fromString($propertyPath), $operand, true)); + break; + case '^=': + $propertyCriteria[] = PropertyValueStartsWith::create(PropertyName::fromString($propertyPath), $operand, true); + break; + case '$=': + $propertyCriteria[] = PropertyValueEndsWith::create(PropertyName::fromString($propertyPath), $operand, true); + break; + case '*=': + $propertyCriteria[] = PropertyValueContains::create(PropertyName::fromString($propertyPath), $operand, true); + break; + case '=~': + $propertyCriteria[] = PropertyValueEquals::create(PropertyName::fromString($propertyPath), $operand, false); + break; + case '!=~': + $propertyCriteria[] = NegateCriteria::create(PropertyValueEquals::create(PropertyName::fromString($propertyPath), $operand, false)); + break; + case '^=~': + $propertyCriteria[] = PropertyValueStartsWith::create(PropertyName::fromString($propertyPath), $operand, false); + break; + case '$=~': + $propertyCriteria[] = PropertyValueEndsWith::create(PropertyName::fromString($propertyPath), $operand, false); + break; + case '*=~': + $propertyCriteria[] = PropertyValueContains::create(PropertyName::fromString($propertyPath), $operand, false); + break; + case '>': + $propertyCriteria[] = PropertyValueGreaterThan::create(PropertyName::fromString($propertyPath), $operand); + break; + case '>=': + $propertyCriteria[] = PropertyValueGreaterThanOrEqual::create(PropertyName::fromString($propertyPath), $operand); + break; + case '<': + $propertyCriteria[] = PropertyValueLessThan::create(PropertyName::fromString($propertyPath), $operand); + break; + case '<=': + $propertyCriteria[] = PropertyValueLessThanOrEqual::create(PropertyName::fromString($propertyPath), $operand); + break; + default: + return null; + } + } + + if (count($propertyCriteria) > 1) { + $propertyCriteriaCombined = array_shift($propertyCriteria); + while ($other = array_shift($propertyCriteria)) { + $propertyCriteriaCombined = AndCriteria::create($propertyCriteriaCombined, $other); + } + } elseif (count($propertyCriteria) == 1) { + $propertyCriteriaCombined = $propertyCriteria[0]; + } else { + $propertyCriteriaCombined = null; + } + + $filterCriteria[] = new NodeFilterCriteria( + ($allowedNodeTypeNames->isEmpty() && $disallowedNodeTypeNames->isEmpty()) ? null : NodeTypeCriteria::create($allowedNodeTypeNames, $disallowedNodeTypeNames), + $propertyCriteriaCombined + ); + } else { + return null; + } + } + return new NodeFilterCriteriaGroup(...$filterCriteria); + } + return null; + } +} diff --git a/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/ChildrenOperation.php b/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/ChildrenOperation.php index e2a07d15b64..21658707143 100644 --- a/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/ChildrenOperation.php +++ b/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/ChildrenOperation.php @@ -13,9 +13,11 @@ use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindChildNodesFilter; use Neos\ContentRepository\Core\Projection\ContentGraph\NodePath; -use Neos\ContentRepository\Core\SharedModel\Node\NodeName; +use Neos\ContentRepository\Core\Projection\ContentGraph\Nodes; use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\NodeType\NodeTypeCriteria; use Neos\ContentRepository\Core\NodeType\NodeTypeNames; +use Neos\ContentRepository\NodeAccess\Filter\NodeFilterCriteriaGroup; +use Neos\ContentRepository\NodeAccess\Filter\NodeFilterCriteriaGroupFactory; use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry; use Neos\Eel\FlowQuery\FizzleParser; use Neos\Eel\FlowQuery\FlowQuery; @@ -75,6 +77,22 @@ public function evaluate(FlowQuery $flowQuery, array $arguments) $output = []; $outputNodeAggregateIds = []; if (isset($arguments[0]) && !empty($arguments[0])) { + // optimized cr query for instanceof and attribute filters + $nodeFilterCriteriaGroup = NodeFilterCriteriaGroupFactory::createFromFizzleExpressionString($arguments[0]); + if ($nodeFilterCriteriaGroup instanceof NodeFilterCriteriaGroup) { + $result = Nodes::createEmpty(); + foreach ($nodeFilterCriteriaGroup as $nodeFilterCriteria) { + $findChildNodesFilter = FindChildNodesFilter::create(nodeTypes: $nodeFilterCriteria->nodeTypeCriteria, propertyValue: $nodeFilterCriteria->propertyValueCriteria); + foreach ($flowQuery->getContext() as $contextNode) { + $subgraph = $this->contentRepositoryRegistry->subgraphForNode($contextNode); + $descendantNodes = $subgraph->findChildNodes($contextNode->nodeAggregateId, $findChildNodesFilter); + $result = $result->merge($descendantNodes); + } + } + $flowQuery->setContext(iterator_to_array($result->getIterator())); + return; + } + $parsedFilter = FizzleParser::parseFilterGroup($arguments[0]); if ($this->earlyOptimizationOfFilters($flowQuery, $parsedFilter)) { return; diff --git a/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/FindOperation.php b/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/FindOperation.php index 2efce7928d1..c8c8aacdd62 100644 --- a/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/FindOperation.php +++ b/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/FindOperation.php @@ -20,7 +20,10 @@ use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\NodeType\NodeTypeCriteria; use Neos\ContentRepository\Core\Projection\ContentGraph\Node; use Neos\ContentRepository\Core\Projection\ContentGraph\NodePath; +use Neos\ContentRepository\Core\Projection\ContentGraph\Nodes; use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId; +use Neos\ContentRepository\NodeAccess\Filter\NodeFilterCriteriaGroup; +use Neos\ContentRepository\NodeAccess\Filter\NodeFilterCriteriaGroupFactory; use Neos\ContentRepositoryRegistry\ContentRepositoryRegistry; use Neos\Eel\FlowQuery\FizzleParser; use Neos\Eel\FlowQuery\FlowQuery; @@ -63,6 +66,8 @@ */ class FindOperation extends AbstractOperation { + use CreateNodeHashTrait; + /** * {@inheritdoc} * @@ -120,6 +125,31 @@ public function evaluate(FlowQuery $flowQuery, array $arguments): void $selectorAndFilter = $arguments[0]; + // optimized cr query for instanceof and attribute filters + $nodeFilterCriteriaGroup = NodeFilterCriteriaGroupFactory::createFromFizzleExpressionString($selectorAndFilter); + if ($nodeFilterCriteriaGroup instanceof NodeFilterCriteriaGroup) { + $result = Nodes::createEmpty(); + foreach ($nodeFilterCriteriaGroup as $nodeFilterCriteria) { + $findDescendantNodesFilter = FindDescendantNodesFilter::create(nodeTypes: $nodeFilterCriteria->nodeTypeCriteria, propertyValue: $nodeFilterCriteria->propertyValueCriteria); + foreach ($contextNodes as $contextNode) { + $subgraph = $this->contentRepositoryRegistry->subgraphForNode($contextNode); + $descendantNodes = $subgraph->findDescendantNodes($contextNode->nodeAggregateId, $findDescendantNodesFilter); + $result = $result->merge($descendantNodes); + } + } + + $nodesByHash = []; + foreach ($result as $node) { + $hash = $this->createNodeHash($node); + if (!array_key_exists($hash, $nodesByHash)) { + $nodesByHash[$hash] = $node; + } + } + $flowQuery->setContext(array_values($nodesByHash)); + return; + } + + $firstContextNode = reset($contextNodes); assert($firstContextNode instanceof Node); diff --git a/Neos.ContentRepository.NodeAccess/Tests/Unit/Filter/NodeFilterCriteriaGroupFactoryTest.php b/Neos.ContentRepository.NodeAccess/Tests/Unit/Filter/NodeFilterCriteriaGroupFactoryTest.php new file mode 100644 index 00000000000..50f80f29be3 --- /dev/null +++ b/Neos.ContentRepository.NodeAccess/Tests/Unit/Filter/NodeFilterCriteriaGroupFactoryTest.php @@ -0,0 +1,147 @@ + ['[instanceof Foo:Bar]', [["types" => "Foo:Bar", "properties" => ""]]]; + yield 'single NotInstanceOf' => ['[!instanceof Foo:Bar]', [["types" => "!Foo:Bar", "properties" => ""]]]; + yield 'instanceOf OR combination' => ['[instanceof Foo:Bar],[instanceof Foo:Baz]', [["types" => "Foo:Bar", "properties" => ""], ["types" => "Foo:Baz", "properties" => ""]]]; + yield 'instanceOf AND combination' => ['[instanceof Foo:Bar][instanceof Foo:Baz]', [["types" => "Foo:Bar,Foo:Baz", "properties" => ""]]]; + yield 'notInstanceOf OR combination' => ['[instanceof Foo:Bar],[!instanceof Foo:Baz]', [["types" => "Foo:Bar", "properties" => ""], ["types" => "!Foo:Baz", "properties" => ""]]]; + yield 'notInstanceOf AND combination' => ['[instanceof Foo:Bar][!instanceof Foo:Baz]', [["types" => "Foo:Bar,!Foo:Baz", "properties" => ""]]]; + + yield 'single PropertyEquals filter (case sensitive)' => ['[foo="bar"]', [["types" => "", "properties" => "foo=bar"]]]; + yield 'single PropertyEquals filter (case insensitive)' => ['[foo=~"bar"]', [["types" => "", "properties" => "foo=~bar"]]]; + yield 'single PropertyNotEquals filter (case sensitive)' => ['[foo!="bar"]', [["types" => "", "properties" => "!(foo=bar)"]]]; + yield 'single PropertyNotEquals filter (case insensitive)' => ['[foo!=~"bar"]', [["types" => "", "properties" => "!(foo=~bar)"]]]; + yield 'single PropertyContains filter (case sensitive)' => ['[foo*="bar"]', [["types" => "", "properties" => "foo*=bar"]]]; + yield 'single PropertyContains filter (case insensitive)' => ['[foo*=~"bar"]', [["types" => "", "properties" => "foo*=~bar"]]]; + yield 'single PropertyStartsWith filter (case sensitive)' => ['[foo^="bar"]', [["types" => "", "properties" => "foo^=bar"]]]; + yield 'single PropertyStartsWith filter (case insensitive)' => ['[foo^=~"bar"]', [["types" => "", "properties" => "foo^=~bar"]]]; + yield 'single PropertyEndsWith filter (case sensitive)' => ['[foo$="bar"]', [["types" => "", "properties" => "foo$=bar"]]]; + yield 'single PropertyEndsWith filter (case insensitive)' => ['[foo$=~"bar"]', [["types" => "", "properties" => "foo$=~bar"]]]; + + yield 'single PropertyGreaterThan filter' => ['[foo > 123]', [["types" => "", "properties" => "foo>123"]]]; + yield 'single PropertyGreaterThanOrEqual filter' => ['[foo >= 123]', [["types" => "", "properties" => "foo>=123"]]]; + yield 'single PropertyLessThan filter ' => ['[foo < 123]', [["types" => "", "properties" => "foo<123"]]]; + yield 'single PropertyLessThanOrEqual filter' => ['[foo <= 123]', [["types" => "", "properties" => "foo<=123"]]]; + + yield 'multiple Property AND filter' => ['[foo="bar"][bar="baz"]', [["types" => "", "properties" => "(foo=bar&&bar=baz)"]]]; + yield 'multiple Property OR filter' => ['[foo="bar"],[bar="baz"]', [["types" => "", "properties" => "foo=bar"], ["types" => "", "properties" => "bar=baz"]]]; + + yield 'combination of Property AND NodeTypeFilter' => ['[instanceof Foo:Bar][bar="baz"]', [["types" => "Foo:Bar", "properties" => "bar=baz"]]]; + yield 'combination of Property OR NodeTypeFilter' => ['[instanceof Foo:Bar],[bar="baz"]', [["types" => "Foo:Bar", "properties" => ""], ["types" => "", "properties" => "bar=baz"]]]; + } + + /** + * @dataProvider nodeFilterCriteriaGroupFactoryDataProvider + */ + public function testNodeFilterCriteriaGroupFactory(string $fizzleExpresssion, array $expectation): void + { + $nodeFilterCriteria = NodeFilterCriteriaGroupFactory::createFromFizzleExpressionString($fizzleExpresssion); + $this->assertInstanceOf(NodeFilterCriteriaGroup::class, $nodeFilterCriteria); + $this->assertSame($expectation, self::nodeFilterCriteriaGroupToArray($nodeFilterCriteria)); + } + + public function nodeFilterCriteriaGroupAreNotCreatedForUnknownFilterExpressionsDataProvider(): \Generator + { + yield 'absolute node path' => ['//foo/bar']; + yield 'relative node path' => ['foo/bar/baz']; + yield 'node id' => ['#4d39e8b8-cd05-49ca-bd64-5efc4ea176e9']; + + yield 'mixture of valid and invalid parts' => ['//foo/bar,//foo/baz']; + yield 'multiple pathes' => ['foo/bar/baz, bar/baz/bam']; + yield 'multiple ids' => ['#4d39e8b8-cd05-49ca-bd64-5efc4ea176e9,#4d39e8b8-cd05-49ca-bd64-5efc4ea17619']; + } + + /** + * @dataProvider nodeFilterCriteriaGroupAreNotCreatedForUnknownFilterExpressionsDataProvider + */ + public function testNodeFilterCriteriaGroupAreNotCreatedForUnknownFilterExpressions(string $fizzleExpresssion): void + { + $nodeFilterCriteria = NodeFilterCriteriaGroupFactory::createFromFizzleExpressionString($fizzleExpresssion); + $this->assertNull( $nodeFilterCriteria); + } + + private static function nodeFilterCriteriaGroupToArray(NodeFilterCriteriaGroup $criteriaGroup): array + { + return array_map( + fn(NodeFilterCriteria $criteria) => self::nodeFilterCriteriaToArray($criteria), + iterator_to_array($criteriaGroup->getIterator()) + ); + } + + private static function nodeFilterCriteriaToArray(NodeFilterCriteria $criteria): array + { + return [ + 'types' => $criteria->nodeTypeCriteria ? self::nodeTypeCriteriaToString( $criteria->nodeTypeCriteria) : '', + 'properties' => $criteria->propertyValueCriteria ? self::propertyValueCriteriaToString($criteria->propertyValueCriteria): '' + ]; + } + + private static function nodeTypeCriteriaToString(NodeTypeCriteria $criteria): string + { + $resultParts = []; + foreach($criteria->explicitlyAllowedNodeTypeNames as $allowedNodeTypeName) { + $resultParts[] = $allowedNodeTypeName->value; + } + foreach($criteria->explicitlyDisallowedNodeTypeNames as $disallowedNodeTypeName) { + $resultParts[] = '!' . $disallowedNodeTypeName->value; + } + return implode(',', $resultParts); + } + + private static function propertyValueCriteriaToString(PropertyValueCriteriaInterface $criteria): string + { + return match ($criteria::class) { + AndCriteria::class => '(' . self::propertyValueCriteriaToString($criteria->criteria1) . '&&' . self::propertyValueCriteriaToString($criteria->criteria2) . ')', + OrCriteria::class => '(' . self::propertyValueCriteriaToString($criteria->criteria1) . '||' . self::propertyValueCriteriaToString($criteria->criteria2) . ')', + NegateCriteria::class => '!(' . self::propertyValueCriteriaToString($criteria->criteria) . ')', + PropertyValueStartsWith::class => $criteria->propertyName->value . '^=' . ($criteria->caseSensitive ? '' : '~') . $criteria->value, + PropertyValueEndsWith::class => $criteria->propertyName->value . '$=' . ($criteria->caseSensitive ? '' : '~') . $criteria->value, + PropertyValueContains::class => $criteria->propertyName->value . '*=' . ($criteria->caseSensitive ? '' : '~') . $criteria->value, + PropertyValueEquals::class => $criteria->propertyName->value . '=' . ($criteria->caseSensitive ? '' : '~') . $criteria->value, + PropertyValueGreaterThan::class => $criteria->propertyName->value . '>' . $criteria->value, + PropertyValueLessThan::class => $criteria->propertyName->value . '<' . $criteria->value, + PropertyValueGreaterThanOrEqual::class => $criteria->propertyName->value . '>=' . $criteria->value, + PropertyValueLessThanOrEqual::class => $criteria->propertyName->value . '<=' . $criteria->value, + default => throw new \InvalidArgumentException('type ' . $criteria::class . ' was not hancled'()) + }; + } +} diff --git a/Neos.Neos/Tests/Behavior/Features/Fusion/FlowQuery.feature b/Neos.Neos/Tests/Behavior/Features/Fusion/FlowQuery.feature index 07835b41d8a..276c123229b 100644 --- a/Neos.Neos/Tests/Behavior/Features/Fusion/FlowQuery.feature +++ b/Neos.Neos/Tests/Behavior/Features/Fusion/FlowQuery.feature @@ -352,6 +352,7 @@ Feature: Tests for the "Neos.ContentRepository" Flow Query methods. """fusion test = Neos.Fusion:DataStructure { typeFilter = ${q(node).find('[instanceof Neos.Neos:Test.DocumentType2]').get()} + propertyFilter = ${q(node).find('[uriPathSegment*="1b"]').get()} combinedFilter = ${q(node).find('[instanceof Neos.Neos:Test.DocumentType2][uriPathSegment*="b1"]').get()} identifier = ${q(node).find('#a1b1a').get()} name = ${q(node).find('a1b').get()} @@ -363,6 +364,7 @@ Feature: Tests for the "Neos.ContentRepository" Flow Query methods. Then I expect the following Fusion rendering result: """ typeFilter: a1a,a1a2,a1b2,a1a3,a1a4,a1a5,a1a6,a1b1a + propertyFilter: a1b,a1b1,a1b2,a1b3,a1b1a,a1b1b combinedFilter: a1b1a identifier: a1b1a name: a1b