From a4d7230acce6354ddf00de034005bdda232160e1 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 | 125 ++++++++++++++++ .../FlowQueryOperations/ChildrenOperation.php | 137 +++--------------- .../FlowQueryOperations/FindOperation.php | 34 ++++- .../NodeFilterCriteriaGroupFactoryTest.php | 121 ++++++++++++++++ .../Features/Fusion/FlowQuery.feature | 2 + 7 files changed, 349 insertions(+), 118 deletions(-) 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..6681651715d --- /dev/null +++ b/Neos.ContentRepository.NodeAccess/Classes/Filter/NodeFilterCriteriaGroupFactory.php @@ -0,0 +1,125 @@ +withAdditionalNodeTypeName(NodeTypeName::fromString($operand)); + break; + case '!instanceof': + $disallowedNodeTypeNames = $disallowedNodeTypeNames->withAdditionalNodeTypeName(NodeTypeName::fromString($operand)); + break; + case '=': + $propertyCriteria[] = PropertyValueEquals::create(PropertyName::fromString($propertyPath), $operand); + break; + case '!=': + $propertyCriteria[] = NegateCriteria::create(PropertyValueEquals::create(PropertyName::fromString($propertyPath), $operand)); + break; + case '^=': + $propertyCriteria[] = PropertyValueStartsWith::create(PropertyName::fromString($propertyPath), $operand); + break; + case '$=': + $propertyCriteria[] = PropertyValueEndsWith::create(PropertyName::fromString($propertyPath), $operand); + break; + case '*=': + $propertyCriteria[] = PropertyValueContains::create(PropertyName::fromString($propertyPath), $operand); + 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 370fefd9861..4b73af5aa27 100644 --- a/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/ChildrenOperation.php +++ b/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/ChildrenOperation.php @@ -12,10 +12,13 @@ */ use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindChildNodesFilter; -use Neos\ContentRepository\Core\Projection\ContentGraph\NodePath; +use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindDescendantNodesFilter; +use Neos\ContentRepository\Core\Projection\ContentGraph\Nodes; use Neos\ContentRepository\Core\SharedModel\Node\NodeName; 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; @@ -74,15 +77,28 @@ public function evaluate(FlowQuery $flowQuery, array $arguments) { $output = []; $outputNodeAggregateIds = []; + $contextNodes = $flowQuery->getContext(); + if (isset($arguments[0]) && !empty($arguments[0])) { - $parsedFilter = FizzleParser::parseFilterGroup($arguments[0]); - if ($this->earlyOptimizationOfFilters($flowQuery, $parsedFilter)) { + // 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 ($contextNodes 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; } } /** @var Node $contextNode */ - foreach ($flowQuery->getContext() as $contextNode) { + foreach ($contextNodes as $contextNode) { $childNodes = $this->contentRepositoryRegistry->subgraphForNode($contextNode) ->findChildNodes($contextNode->nodeAggregateId, FindChildNodesFilter::create()); foreach ($childNodes as $childNode) { @@ -98,117 +114,4 @@ public function evaluate(FlowQuery $flowQuery, array $arguments) $flowQuery->pushOperation('filter', $arguments); } } - - /** - * Optimize for typical use cases, filter by node name and filter - * by NodeType (instanceof). These cases are now optimized and will - * only load the nodes that match the filters. - * - * @param FlowQuery $flowQuery - * @param array $parsedFilter - * @return boolean - * @throws \Neos\Eel\Exception - */ - protected function earlyOptimizationOfFilters(FlowQuery $flowQuery, array $parsedFilter) - { - $optimized = false; - $output = []; - $outputNodeAggregateIds = []; - foreach ($parsedFilter['Filters'] as $filter) { - $instanceOfFilters = []; - $attributeFilters = []; - if (isset($filter['AttributeFilters'])) { - foreach ($filter['AttributeFilters'] as $attributeFilter) { - if ($attributeFilter['Operator'] === 'instanceof' && $attributeFilter['Identifier'] === null) { - $instanceOfFilters[] = $attributeFilter; - } else { - $attributeFilters[] = $attributeFilter; - } - } - } - - // Only apply optimization if there's a property name filter or an instanceof filter - // or another filter already did optimization - if ((isset($filter['PropertyNameFilter']) || isset($filter['PathFilter'])) - || count($instanceOfFilters) > 0 || $optimized === true) { - $optimized = true; - $filteredOutput = []; - $filteredOutputNodeIdentifiers = []; - // Optimize property name filter if present - if (isset($filter['PropertyNameFilter']) || isset($filter['PathFilter'])) { - $nodePath = $filter['PropertyNameFilter'] ?? $filter['PathFilter']; - /** @var Node $contextNode */ - foreach ($flowQuery->getContext() as $contextNode) { - $resolvedNode = $this->contentRepositoryRegistry->subgraphForNode($contextNode) - ->findNodeByPath( - NodePath::fromString($nodePath), - $contextNode->nodeAggregateId, - ); - - if (!is_null($resolvedNode) && !isset($filteredOutputNodeIdentifiers[ - $resolvedNode->nodeAggregateId->value - ])) { - $filteredOutput[] = $resolvedNode; - $filteredOutputNodeIdentifiers[$resolvedNode->nodeAggregateId->value] = true; - } - } - } elseif (count($instanceOfFilters) > 0) { - // Optimize node type filter if present - $allowedNodeTypes = array_map(function ($instanceOfFilter) { - return $instanceOfFilter['Operand']; - }, $instanceOfFilters); - /** @var Node $contextNode */ - foreach ($flowQuery->getContext() as $contextNode) { - $childNodes = $this->contentRepositoryRegistry->subgraphForNode($contextNode) - ->findChildNodes( - $contextNode->nodeAggregateId, - FindChildNodesFilter::create( - nodeTypes: NodeTypeCriteria::create( - NodeTypeNames::fromStringArray($allowedNodeTypes), - NodeTypeNames::createEmpty() - ) - ) - ); - - foreach ($childNodes as $childNode) { - if (!isset($filteredOutputNodeIdentifiers[ - $childNode->nodeAggregateId->value - ])) { - $filteredOutput[] = $childNode; - $filteredOutputNodeIdentifiers[$childNode->nodeAggregateId->value] = true; - } - } - } - } - - // Apply attribute filters if present - if (isset($filter['AttributeFilters'])) { - $attributeFilters = array_reduce($filter['AttributeFilters'], function ( - $filters, - $attributeFilter - ) { - return $filters . $attributeFilter['text']; - }); - $filteredFlowQuery = new FlowQuery($filteredOutput); - $filteredFlowQuery->pushOperation('filter', [$attributeFilters]); - $filteredOutput = iterator_to_array($filteredFlowQuery); - } - - // Add filtered nodes to output - /** @var Node $filteredNode */ - foreach ($filteredOutput as $filteredNode) { - /** @phpstan-ignore-next-line undefined behaviour https://github.com/neos/neos-development-collection/issues/4507#issuecomment-1784123143 */ - if (!isset($outputNodeAggregateIds[$filteredNode->nodeAggregateId->value])) { - $output[] = $filteredNode; - } - } - } - } - - if ($optimized === true) { - $flowQuery->setContext($output); - } - - return $optimized; - } } diff --git a/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/FindOperation.php b/Neos.ContentRepository.NodeAccess/Classes/FlowQueryOperations/FindOperation.php index 2efce7928d1..5ab1e136241 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} * @@ -112,6 +117,7 @@ public function canEvaluate($context) */ public function evaluate(FlowQuery $flowQuery, array $arguments): void { + /** @var array $contextNodes */ $contextNodes = $flowQuery->getContext(); if (count($contextNodes) === 0 || empty($arguments[0])) { @@ -120,6 +126,32 @@ public function evaluate(FlowQuery $flowQuery, array $arguments): void $selectorAndFilter = $arguments[0]; + $parsedFilter = FizzleParser::parseFilterGroup($selectorAndFilter); + + // optimized cr query for instanceof and attribute filters + $nodeFilterCriteriaGroup = NodeFilterCriteriaGroupFactory::createFromParsedFizzleExpression($parsedFilter); + 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); @@ -180,7 +212,7 @@ public function evaluate(FlowQuery $flowQuery, array $arguments): void $uniqueResult = []; $usedKeys = []; foreach ($result as $item) { - $identifier = $item->subgraphIdentity->contentStreamId->value . '@' . $item->subgraphIdentity->dimensionSpacePoint->hash . '@' . $item->nodeAggregateId->value; + $identifier = $this->createNodeHash($item); if (!isset($usedKeys[$identifier])) { $uniqueResult[] = $item; $usedKeys[$identifier] = $identifier; 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..4f2af64fde4 --- /dev/null +++ b/Neos.ContentRepository.NodeAccess/Tests/Unit/Filter/NodeFilterCriteriaGroupFactoryTest.php @@ -0,0 +1,121 @@ + ['[instanceof Foo:Bar]', [["types" => "Foo:Bar", "properties" => ""]]]; + yield 'singleNotInstanceOf' => ['[!instanceof Foo:Bar]', [["types" => "!Foo:Bar", "properties" => ""]]]; + yield 'instanceOfOrCombination' => ['[instanceof Foo:Bar],[instanceof Foo:Baz]', [["types" => "Foo:Bar", "properties" => ""], ["types" => "Foo:Baz", "properties" => ""]]]; + yield 'instanceOfAndCombination' => ['[instanceof Foo:Bar][instanceof Foo:Baz]', [["types" => "Foo:Bar,Foo:Baz", "properties" => ""]]]; + yield 'notInstanceOfOrCombination' => ['[instanceof Foo:Bar],[!instanceof Foo:Baz]', [["types" => "Foo:Bar", "properties" => ""], ["types" => "!Foo:Baz", "properties" => ""]]]; + yield 'notInstanceOfAndCombination' => ['[instanceof Foo:Bar][!instanceof Foo:Baz]', [["types" => "Foo:Bar,!Foo:Baz", "properties" => ""]]]; + + yield 'singlePropertyEqualsFilter' => ['[foo="bar"]', [["types" => "", "properties" => "foo=bar"]]]; + yield 'singlePropertyNotEqualsFilter' => ['[foo!="bar"]', [["types" => "", "properties" => "!(foo=bar)"]]]; + yield 'singlePropertyContainsFilter' => ['[foo*="bar"]', [["types" => "", "properties" => "foo*=bar"]]]; + yield 'singlePropertyStartsWithFilter' => ['[foo^="bar"]', [["types" => "", "properties" => "foo^=bar"]]]; + yield 'singlePropertyEndsWithFilter' => ['[foo$="bar"]', [["types" => "", "properties" => "foo$=bar"]]]; + yield 'singlePropertyGreaterThanFilter' => ['[foo > 123]', [["types" => "", "properties" => "foo>123"]]]; + yield 'singlePropertyGreaterThanOrEqualFilter' => ['[foo >= 123]', [["types" => "", "properties" => "foo>=123"]]]; + yield 'singlePropertyLessThanFilter' => ['[foo < 123]', [["types" => "", "properties" => "foo<123"]]]; + yield 'singlePropertyLessThanOrEqualFilter' => ['[foo <= 123]', [["types" => "", "properties" => "foo<=123"]]]; + + yield 'multiplePropertyAndFilter' => ['[foo="bar"][bar="baz"]', [["types" => "", "properties" => "(foo=bar&&bar=baz)"]]]; + yield 'multiplePropertyOrFilter' => ['[foo="bar"],[bar="baz"]', [["types" => "", "properties" => "foo=bar"], ["types" => "", "properties" => "bar=baz"]]]; + + yield 'combinationOfPropertyAndNodeTypeFilterAnd' => ['[instanceof Foo:Bar][bar="baz"]', [["types" => "Foo:Bar", "properties" => "bar=baz"]]]; + yield 'combinationOfPropertyAndNodeTypeFilterOr' => ['[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)); + } + + 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->value, + PropertyValueEndsWith::class => $criteria->propertyName->value . '$=' . $criteria->value, + PropertyValueContains::class => $criteria->propertyName->value . '*=' . $criteria->value, + PropertyValueEquals::class => $criteria->propertyName->value . '=' . $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