From 4060c2315f28774fb5f0ed1eaf73a597944c8b01 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sun, 14 Jan 2024 13:15:04 +0100 Subject: [PATCH 1/5] TASK Document NodeAggregate --- .../src/Domain/Repository/NodeFactory.php | 15 ++++++++------- .../Projection/ContentGraph/NodeAggregate.php | 9 ++++++--- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/NodeFactory.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/NodeFactory.php index 3e91d0fc05a..52b86afce16 100644 --- a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/NodeFactory.php +++ b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/NodeFactory.php @@ -176,7 +176,7 @@ public function mapNodeRowsToNodeAggregate( $rawNodeName = ''; $rawNodeAggregateClassification = ''; $occupiedDimensionSpacePoints = []; - $nodesByOccupiedDimensionSpacePoints = []; + $nodesByOccupiedDimensionSpacePoint = []; $coveredDimensionSpacePoints = []; $nodesByCoveredDimensionSpacePoints = []; $coverageByOccupants = []; @@ -186,9 +186,9 @@ public function mapNodeRowsToNodeAggregate( foreach ($nodeRows as $nodeRow) { // A node can occupy exactly one DSP and cover multiple ones... $occupiedDimensionSpacePoint = $this->dimensionSpacePointRepository->getOriginDimensionSpacePointByHash($nodeRow['origindimensionspacepointhash']); - if (!isset($nodesByOccupiedDimensionSpacePoints[$occupiedDimensionSpacePoint->hash])) { + if (!isset($nodesByOccupiedDimensionSpacePoint[$occupiedDimensionSpacePoint->hash])) { // ... so we handle occupation exactly once ... - $nodesByOccupiedDimensionSpacePoints[$occupiedDimensionSpacePoint->hash] = $this->mapNodeRowToNode( + $nodesByOccupiedDimensionSpacePoint[$occupiedDimensionSpacePoint->hash] = $this->mapNodeRowToNode( $nodeRow, $workspaceName, $contentStreamId, @@ -211,7 +211,7 @@ public function mapNodeRowsToNodeAggregate( = $coveredDimensionSpacePoint; $occupationByCovering[$coveredDimensionSpacePoint->hash] = $occupiedDimensionSpacePoint; $nodesByCoveredDimensionSpacePoints[$coveredDimensionSpacePoint->hash] - = $nodesByOccupiedDimensionSpacePoints[$occupiedDimensionSpacePoint->hash]; + = $nodesByOccupiedDimensionSpacePoint[$occupiedDimensionSpacePoint->hash]; // ... as we do for explicit subtree tags foreach (self::extractNodeTagsFromJson($nodeRow['subtreetags'])->withoutInherited() as $explicitTag) { $dimensionSpacePointsBySubtreeTags = $dimensionSpacePointsBySubtreeTags->withSubtreeTagAndDimensionSpacePoint($explicitTag, $coveredDimensionSpacePoint); @@ -220,8 +220,9 @@ public function mapNodeRowsToNodeAggregate( ksort($occupiedDimensionSpacePoints); ksort($coveredDimensionSpacePoints); - /** @var Node $primaryNode a nodeAggregate only exists if it at least contains one node. */ - $primaryNode = current($nodesByOccupiedDimensionSpacePoints); + // a nodeAggregate only exists if it at least contains one node + assert($nodesByOccupiedDimensionSpacePoint !== []); + $primaryNode = current($nodesByOccupiedDimensionSpacePoint); return new NodeAggregate( $primaryNode->subgraphIdentity->contentStreamId, @@ -230,7 +231,7 @@ public function mapNodeRowsToNodeAggregate( NodeTypeName::fromString($rawNodeTypeName), $rawNodeName ? NodeName::fromString($rawNodeName) : null, new OriginDimensionSpacePointSet($occupiedDimensionSpacePoints), - $nodesByOccupiedDimensionSpacePoints, + $nodesByOccupiedDimensionSpacePoint, CoverageByOrigin::fromArray($coverageByOccupants), new DimensionSpacePointSet($coveredDimensionSpacePoints), $nodesByCoveredDimensionSpacePoints, diff --git a/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php b/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php index d75692f9369..38bbbe5a40c 100644 --- a/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php +++ b/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php @@ -58,10 +58,10 @@ * @param NodeTypeName $nodeTypeName name of the node type of this aggregate * @param NodeName|null $nodeName optional name of this aggregate * @param OriginDimensionSpacePointSet $occupiedDimensionSpacePoints dimension space points this aggregate occupies - * @param array $nodesByOccupiedDimensionSpacePoint + * @param non-empty-array $nodesByOccupiedDimensionSpacePoint At least one node will be occupied. * @param CoverageByOrigin $coverageByOccupant - * @param DimensionSpacePointSet $coveredDimensionSpacePoints - * @param array $nodesByCoveredDimensionSpacePoint + * @param DimensionSpacePointSet $coveredDimensionSpacePoints At least one node will be covered. + * @param non-empty-array $nodesByCoveredDimensionSpacePoint * @param OriginByCoverage $occupationByCovered * @param DimensionSpacePointsBySubtreeTags $dimensionSpacePointsBySubtreeTags dimension space points for every subtree tag this aggregate is *explicitly* tagged with (excluding inherited tags) */ @@ -80,6 +80,9 @@ public function __construct( private OriginByCoverage $occupationByCovered, private DimensionSpacePointsBySubtreeTags $dimensionSpacePointsBySubtreeTags, ) { + // this nodeAggregate can only exist if it at least contains one node. + assert($this->nodesByOccupiedDimensionSpacePoint !== []); + assert($this->nodesByCoveredDimensionSpacePoint !== []); } public function occupiesDimensionSpacePoint(OriginDimensionSpacePoint $originDimensionSpacePoint): bool From 47ad79987f9736527803890e4b41c4d51dfef81b Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Fri, 2 Feb 2024 20:11:06 +0100 Subject: [PATCH 2/5] TASK: Declare NodeAggregate constructor as internal --- .../Classes/Projection/ContentGraph/NodeAggregate.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php b/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php index 38bbbe5a40c..dae8ad0d0ae 100644 --- a/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php +++ b/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php @@ -47,11 +47,12 @@ * This interface is called *Readable* because it exposes read operations on the set of nodes inside * a single NodeAggregate; often used for constraint checks (in command handlers). * - * @api + * @api except its constructor. */ final readonly class NodeAggregate { /** + * @internal * @param ContentStreamId $contentStreamId ID of the content stream of this node aggregate * @param NodeAggregateId $nodeAggregateId ID of this node aggregate * @param NodeAggregateClassification $classification whether this aggregate represents a root, regular or tethered node From 3febbd651309926fb0bfcc04f76068aa0faf0ccf Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Sat, 16 Mar 2024 19:33:28 +0100 Subject: [PATCH 3/5] TASK: Assert that `getRootGeneralizations` is never empty as part of the interface This removes manual assertions on the calling site which are never expected. --- .../InterDimensionalVariationGraph.php | 13 +++++++++++-- .../Classes/Domain/Service/SiteServiceInternals.php | 6 ------ .../Classes/View/FusionExceptionViewInternals.php | 6 ------ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/Neos.ContentRepository.Core/Classes/DimensionSpace/InterDimensionalVariationGraph.php b/Neos.ContentRepository.Core/Classes/DimensionSpace/InterDimensionalVariationGraph.php index 52725f6ec9e..df47ebd4f0a 100644 --- a/Neos.ContentRepository.Core/Classes/DimensionSpace/InterDimensionalVariationGraph.php +++ b/Neos.ContentRepository.Core/Classes/DimensionSpace/InterDimensionalVariationGraph.php @@ -117,7 +117,10 @@ public function getWeightedDimensionSpacePointByHash(string $hash): ?WeightedDim /** * Returns the root generalizations indexed by hash * - * @return array + * Even in a zero-dimensional content repository the array will have at least one entry + * of an empty dimension space point {@see DimensionSpacePoint::createWithoutDimensions()} + * + * @return non-empty-array */ public function getRootGeneralizations(): array { @@ -127,7 +130,13 @@ public function getRootGeneralizations(): array $rootGeneralizations[$dimensionSpacePointHash] = $weightedDimensionSpacePoint->dimensionSpacePoint; } } - + if (empty($rootGeneralizations)) { + // safeguard, should not happen here: + throw new \RuntimeException( + 'The dimension space is empty, please check your configuration.', + 1710613747 + ); + } return $rootGeneralizations; } diff --git a/Neos.Neos/Classes/Domain/Service/SiteServiceInternals.php b/Neos.Neos/Classes/Domain/Service/SiteServiceInternals.php index ae5d75bb1e5..d18cc4c0aae 100644 --- a/Neos.Neos/Classes/Domain/Service/SiteServiceInternals.php +++ b/Neos.Neos/Classes/Domain/Service/SiteServiceInternals.php @@ -106,12 +106,6 @@ public function createSiteNodeIfNotExists(Site $site, string $nodeTypeName): voi } $rootDimensionSpacePoints = $this->interDimensionalVariationGraph->getRootGeneralizations(); - if (empty($rootDimensionSpacePoints)) { - throw new \InvalidArgumentException( - 'The dimension space is empty, please check your configuration.', - 1651957153 - ); - } $arbitraryRootDimensionSpacePoint = array_shift($rootDimensionSpacePoints); $siteNodeAggregateId = NodeAggregateId::create(); diff --git a/Neos.Neos/Classes/View/FusionExceptionViewInternals.php b/Neos.Neos/Classes/View/FusionExceptionViewInternals.php index afc4cbd176b..554c47b6131 100644 --- a/Neos.Neos/Classes/View/FusionExceptionViewInternals.php +++ b/Neos.Neos/Classes/View/FusionExceptionViewInternals.php @@ -31,12 +31,6 @@ public function __construct( public function getArbitraryDimensionSpacePoint(): DimensionSpacePoint { $rootDimensionSpacePoints = $this->interDimensionalVariationGraph->getRootGeneralizations(); - if (empty($rootDimensionSpacePoints)) { - throw new \InvalidArgumentException( - 'The dimension space is empty, please check your configuration.', - 1651957153 - ); - } $arbitraryRootDimensionSpacePoint = array_shift($rootDimensionSpacePoints); return $arbitraryRootDimensionSpacePoint; } From b205bf03b2c79ef1ca177b5c273325c29ea7dacb Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Tue, 2 Apr 2024 23:13:36 +0200 Subject: [PATCH 4/5] TASK: Make `NodeAggregate::getNodes` internal --- .../Projection/ContentGraph/NodeAggregate.php | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php b/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php index dae8ad0d0ae..95474721744 100644 --- a/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php +++ b/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php @@ -91,16 +91,6 @@ public function occupiesDimensionSpacePoint(OriginDimensionSpacePoint $originDim return $this->occupiedDimensionSpacePoints->contains($originDimensionSpacePoint); } - /** - * Returns the nodes belonging to this aggregate, i.e. the "real materialized" node rows. - * - * @return iterable - */ - public function getNodes(): iterable - { - return array_values($this->nodesByOccupiedDimensionSpacePoint); - } - public function getNodeByOccupiedDimensionSpacePoint( OriginDimensionSpacePoint $occupiedDimensionSpacePoint ): Node { @@ -168,4 +158,15 @@ public function getDimensionSpacePointsTaggedWith(SubtreeTag $subtreeTag): Dimen { return $this->dimensionSpacePointsBySubtreeTags->forSubtreeTag($subtreeTag); } + + /** + * Returns the nodes belonging to this aggregate, i.e. the "real materialized" node rows. + * + * @internal Using this method to access all occupied nodes or possibly extract a single arbitrary node is not intended for use outside the core. + * @return iterable + */ + public function getNodes(): iterable + { + return array_values($this->nodesByOccupiedDimensionSpacePoint); + } } From bb6312fd68ab318ee1708b01968e17ec7eb51062 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Wed, 3 Apr 2024 21:43:01 +0200 Subject: [PATCH 5/5] TASK: Revert assertion inside of aggregate `__construct` --- .../Classes/Projection/ContentGraph/NodeAggregate.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php b/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php index 95474721744..3cc7f298ab4 100644 --- a/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php +++ b/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodeAggregate.php @@ -61,8 +61,8 @@ * @param OriginDimensionSpacePointSet $occupiedDimensionSpacePoints dimension space points this aggregate occupies * @param non-empty-array $nodesByOccupiedDimensionSpacePoint At least one node will be occupied. * @param CoverageByOrigin $coverageByOccupant - * @param DimensionSpacePointSet $coveredDimensionSpacePoints At least one node will be covered. - * @param non-empty-array $nodesByCoveredDimensionSpacePoint + * @param DimensionSpacePointSet $coveredDimensionSpacePoints This node aggregate will cover at least one dimension space. + * @param non-empty-array $nodesByCoveredDimensionSpacePoint At least one node will be covered. * @param OriginByCoverage $occupationByCovered * @param DimensionSpacePointsBySubtreeTags $dimensionSpacePointsBySubtreeTags dimension space points for every subtree tag this aggregate is *explicitly* tagged with (excluding inherited tags) */ @@ -81,9 +81,6 @@ public function __construct( private OriginByCoverage $occupationByCovered, private DimensionSpacePointsBySubtreeTags $dimensionSpacePointsBySubtreeTags, ) { - // this nodeAggregate can only exist if it at least contains one node. - assert($this->nodesByOccupiedDimensionSpacePoint !== []); - assert($this->nodesByCoveredDimensionSpacePoint !== []); } public function occupiesDimensionSpacePoint(OriginDimensionSpacePoint $originDimensionSpacePoint): bool