From a3d4704a433ca8342b28203ff7c485afc6d0d1c4 Mon Sep 17 00:00:00 2001 From: Bastian Waidelich Date: Thu, 23 Nov 2023 16:26:49 +0100 Subject: [PATCH 1/4] BUGFIX: Respect position when ordering nodes Related: #4769 --- .../src/Domain/Repository/ContentGraph.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php index 3e39613f33b..c1b2c7e72d0 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php @@ -406,7 +406,8 @@ private function createChildNodeAggregateQuery(): string AND r.dimensionspacepointhash = ph.dimensionspacepointhash WHERE p.nodeaggregateid = :parentNodeAggregateId AND ph.contentstreamid = :contentStreamId - AND ch.contentstreamid = :contentStreamId'; + AND ch.contentstreamid = :contentStreamId + ORDER BY ch.position'; } /** From 633c2708d46eeb35395689ef9896313b17619bc3 Mon Sep 17 00:00:00 2001 From: bwaidelich Date: Thu, 23 Nov 2023 18:03:22 +0100 Subject: [PATCH 2/4] move ordering out of createChildNodeAggregateQuery() --- .../src/Domain/Repository/ContentGraph.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php index c1b2c7e72d0..4ff7b71b4e3 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/ContentGraph.php @@ -317,7 +317,7 @@ public function findChildNodeAggregates( ): iterable { $connection = $this->client->getConnection(); - $query = $this->createChildNodeAggregateQuery(); + $query = $this->createChildNodeAggregateQuery() . ' ORDER BY ch.position'; $parameters = [ 'parentNodeAggregateId' => $parentNodeAggregateId->value, @@ -344,7 +344,8 @@ public function findChildNodeAggregatesByName( $connection = $this->client->getConnection(); $query = $this->createChildNodeAggregateQuery() . ' - AND ch.name = :relationName'; + AND ch.name = :relationName + ORDER BY ch.position'; $parameters = [ 'contentStreamId' => $contentStreamId->value, @@ -371,7 +372,8 @@ public function findTetheredChildNodeAggregates( $connection = $this->client->getConnection(); $query = $this->createChildNodeAggregateQuery() . ' - AND c.classification = :tetheredClassification'; + AND c.classification = :tetheredClassification + ORDER BY ch.position'; $parameters = [ 'contentStreamId' => $contentStreamId->value, @@ -406,8 +408,7 @@ private function createChildNodeAggregateQuery(): string AND r.dimensionspacepointhash = ph.dimensionspacepointhash WHERE p.nodeaggregateid = :parentNodeAggregateId AND ph.contentstreamid = :contentStreamId - AND ch.contentstreamid = :contentStreamId - ORDER BY ch.position'; + AND ch.contentstreamid = :contentStreamId'; } /** From 034646f758a27bc28c604547c7bd4f157e8b5054 Mon Sep 17 00:00:00 2001 From: bwaidelich Date: Thu, 23 Nov 2023 18:03:46 +0100 Subject: [PATCH 3/4] Relax tests where no deterministic ordering is possible --- .../Bootstrap/GenericCommandExecutionAndEventPublication.php | 3 ++- .../Classes/Behavior/Features/Bootstrap/NodeTraversalTrait.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php index 8f9680a2736..32e5de9f0fd 100644 --- a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php +++ b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/GenericCommandExecutionAndEventPublication.php @@ -220,7 +220,8 @@ public function eventNumberIs(int $eventNumber, string $eventType, TableNode $pa $key = $assertionTableRow['Key']; $actualValue = Arrays::getValueByPath($actualEventPayload, $key); - if ($key === 'affectedDimensionSpacePoints') { + // Note: For dimension space points we switch to an array comparison because the order is not deterministic (@see https://github.com/neos/neos-development-collection/issues/4769) + if ($key === 'affectedDimensionSpacePoints' || $key === 'affectedOccupiedDimensionSpacePoints') { $expected = DimensionSpacePointSet::fromJsonString($assertionTableRow['Expected']); $actual = DimensionSpacePointSet::fromArray($actualValue); Assert::assertTrue($expected->equals($actual), 'Actual Dimension Space Point set "' . json_encode($actualValue) . '" does not match expected Dimension Space Point set "' . $assertionTableRow['Expected'] . '"'); diff --git a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/NodeTraversalTrait.php b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/NodeTraversalTrait.php index 1e0323c69d2..2d4f54daa4d 100644 --- a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/NodeTraversalTrait.php +++ b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/NodeTraversalTrait.php @@ -263,7 +263,8 @@ public function iExecuteTheFindDescendantNodesQueryIExpectTheFollowingNodes(stri $subgraph = $this->getCurrentSubgraph(); $actualNodeIds = array_map(static fn(Node $node) => $node->nodeAggregateId->value, iterator_to_array($subgraph->findDescendantNodes($entryNodeAggregateId, $filter))); - Assert::assertSame($expectedNodeIds, $actualNodeIds, 'findDescendantNodes returned an unexpected result'); + // Note: In contrast to other similar checks, in this case we use assertEquals() instead of assertSame() because the order of descendant nodes is not completely deterministic (@see https://github.com/neos/neos-development-collection/issues/4769) + Assert::assertEquals($expectedNodeIds, $actualNodeIds, 'findDescendantNodes returned an unexpected result'); $actualCount = $subgraph->countDescendantNodes($entryNodeAggregateId, CountDescendantNodesFilter::fromFindDescendantNodesFilter($filter)); Assert::assertSame($expectedTotalCount ?? count($expectedNodeIds), $actualCount, 'countDescendantNodes returned an unexpected result'); } From 8af009d0a3afbdc617923883378ef52e9dc20678 Mon Sep 17 00:00:00 2001 From: bwaidelich Date: Thu, 23 Nov 2023 18:50:30 +0100 Subject: [PATCH 4/4] Use assertEqualsCanonicalizing to compare expected descendant nodes --- .../Behavior/Features/Bootstrap/NodeTraversalTrait.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/NodeTraversalTrait.php b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/NodeTraversalTrait.php index 2d4f54daa4d..d0b9d016695 100644 --- a/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/NodeTraversalTrait.php +++ b/Neos.ContentRepository.TestSuite/Classes/Behavior/Features/Bootstrap/NodeTraversalTrait.php @@ -263,8 +263,8 @@ public function iExecuteTheFindDescendantNodesQueryIExpectTheFollowingNodes(stri $subgraph = $this->getCurrentSubgraph(); $actualNodeIds = array_map(static fn(Node $node) => $node->nodeAggregateId->value, iterator_to_array($subgraph->findDescendantNodes($entryNodeAggregateId, $filter))); - // Note: In contrast to other similar checks, in this case we use assertEquals() instead of assertSame() because the order of descendant nodes is not completely deterministic (@see https://github.com/neos/neos-development-collection/issues/4769) - Assert::assertEquals($expectedNodeIds, $actualNodeIds, 'findDescendantNodes returned an unexpected result'); + // Note: In contrast to other similar checks, in this case we use assertEqualsCanonicalizing() instead of assertSame() because the order of descendant nodes is not completely deterministic (@see https://github.com/neos/neos-development-collection/issues/4769) + Assert::assertEqualsCanonicalizing($expectedNodeIds, $actualNodeIds, 'findDescendantNodes returned an unexpected result'); $actualCount = $subgraph->countDescendantNodes($entryNodeAggregateId, CountDescendantNodesFilter::fromFindDescendantNodesFilter($filter)); Assert::assertSame($expectedTotalCount ?? count($expectedNodeIds), $actualCount, 'countDescendantNodes returned an unexpected result'); }