Merge pull request #4466 from neos/nodeTypeFallbackHandling

!!! FEATURE: `NodeTypeManager` remove NodeType fallback handling
neos · Sep 29, 2023 · d0067c7 · d0067c7
2 parents 4de3469 + 2d3bc0a
commit d0067c7
Show file tree

Hide file tree

Showing 59 changed files with 357 additions and 426 deletions.
diff --git a/.composer.json b/.composer.json
@@ -15,7 +15,7 @@
   "scripts": {
     "lint:phpcs-psr12": "../../bin/phpcs --colors --standard=PSR12 ./Neos.ContentGraph.DoctrineDbalAdapter/src ./Neos.ContentGraph.PostgreSQLAdapter/src ./Neos.ContentRepository.BehavioralTests/Classes ./Neos.ContentRepository.TestSuite/Classes ./Neos.ContentRepository.Core/Classes ./Neos.Neos/Classes",
     "lint:phpcs": [
-      "@lint:phpcs-psr12 --exclude=Generic.Files.LineLength"
+      "@lint:phpcs-psr12 --exclude=Generic.Files.LineLength,PSR1.Files.SideEffects"
     ],
     "lint:phpstan": "../../bin/phpstan analyse",
     "lint:distributionintegrity": "[ -d 'Neos.ContentRepository' ] && { echo 'Package Neos.ContentRepository should not exist.' 1>&2; exit 1; } || exit 0;",

diff --git a/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/NodeFactory.php b/Neos.ContentGraph.DoctrineDbalAdapter/src/Domain/Repository/NodeFactory.php
@@ -57,13 +57,16 @@ public function __construct(
 
     /**
      * @param array<string,string> $nodeRow Node Row from projection (<prefix>_node table)
-     * @throws NodeTypeNotFoundException
      */
     public function mapNodeRowToNode(
         array $nodeRow,
         DimensionSpacePoint $dimensionSpacePoint,
         VisibilityConstraints $visibilityConstraints
     ): Node {
+        $nodeType = $this->nodeTypeManager->hasNodeType($nodeRow['nodetypename'])
+            ? $this->nodeTypeManager->getNodeType($nodeRow['nodetypename'])
+            : null;
+
         return new Node(
             ContentSubgraphIdentity::create(
                 $this->contentRepositoryId,
@@ -75,7 +78,7 @@ public function mapNodeRowToNode(
             OriginDimensionSpacePoint::fromJsonString($nodeRow['origindimensionspacepoint']),
             NodeAggregateClassification::from($nodeRow['classification']),
             NodeTypeName::fromString($nodeRow['nodetypename']),
-            $this->nodeTypeManager->getNodeType($nodeRow['nodetypename']),
+            $nodeType,
             $this->createPropertyCollectionFromJsonString($nodeRow['properties']),
             isset($nodeRow['name']) ? NodeName::fromString($nodeRow['name']) : null,
             Timestamps::create(

diff --git a/Neos.ContentGraph.PostgreSQLAdapter/src/Domain/Repository/NodeFactory.php b/Neos.ContentGraph.PostgreSQLAdapter/src/Domain/Repository/NodeFactory.php
@@ -23,6 +23,7 @@
 use Neos\ContentRepository\Core\Projection\ContentGraph\Subtree;
 use Neos\ContentRepository\Core\Projection\ContentGraph\Subtrees;
 use Neos\ContentRepository\Core\Projection\ContentGraph\Timestamps;
+use Neos\ContentRepository\Core\SharedModel\Exception\NodeTypeNotFoundException;
 use Neos\ContentRepository\Core\SharedModel\Node\ReferenceName;
 use Neos\ContentRepository\Core\SharedModel\Workspace\ContentStreamId;
 use Neos\ContentRepository\Core\SharedModel\Node\NodeAggregateId;
@@ -71,8 +72,11 @@ public function mapNodeRowToNode(
         ?DimensionSpacePoint $dimensionSpacePoint = null,
         ?ContentStreamId $contentStreamId = null
     ): Node {
-        $nodeType = $this->nodeTypeManager->getNodeType($nodeRow['nodetypename']);
-        $result = new Node(
+        $nodeType = $this->nodeTypeManager->hasNodeType($nodeRow['nodetypename'])
+            ? $this->nodeTypeManager->getNodeType($nodeRow['nodetypename'])
+            : null;
+
+        return new Node(
             ContentSubgraphIdentity::create(
                 $this->contentRepositoryId,
                 $contentStreamId ?: ContentStreamId::fromString($nodeRow['contentstreamid']),
@@ -97,8 +101,6 @@ public function mapNodeRowToNode(
                 isset($nodeRow['originallastmodified']) ? self::parseDateTimeString($nodeRow['originallastmodified']) : null,
             ),
         );
-
-        return $result;
     }
 
     /**

diff --git a/...epository.BehavioralTests/Classes/TestSuite/Behavior/CRBehavioralTestsSubjectProvider.php b/...epository.BehavioralTests/Classes/TestSuite/Behavior/CRBehavioralTestsSubjectProvider.php
@@ -74,16 +74,6 @@ public function usingTheFollowingContentDimensions(TableNode $contentDimensions)
     public function usingTheFollowingNodeTypes(PyStringNode $serializedNodeTypesConfiguration): void
     {
         GherkinPyStringNodeBasedNodeTypeManagerFactory::initializeWithPyStringNode($serializedNodeTypesConfiguration);
-        GherkinPyStringNodeBasedNodeTypeManagerFactory::$fallbackNodeTypeName = null;
-    }
-
-    /**
-     * @Given /^using the following node types with fallback to "([^"]*)":$/
-     */
-    public function usingTheFollowingNodeTypesWithFallback(string $fallbackNodeTypeName, PyStringNode $serializedNodeTypesConfiguration): void
-    {
-        GherkinPyStringNodeBasedNodeTypeManagerFactory::initializeWithPyStringNode($serializedNodeTypesConfiguration);
-        GherkinPyStringNodeBasedNodeTypeManagerFactory::$fallbackNodeTypeName = $fallbackNodeTypeName;
     }
 
     /**
@@ -136,30 +126,6 @@ public function iChangeTheNodeTypesInContentRepositoryTo(
         }
     }
 
-    /**
-     * @Given /^I change the node types in content repository "([^"]*)" with fallback "([^"]*)" to:$/
-     */
-    public function iChangeTheNodeTypesInContentRepositoryWithFallbackTo(
-        string $contentRepositoryId,
-        string $fallbackNodeTypeName,
-        PyStringNode $serializedNodeTypesConfiguration
-    ): void {
-        if (!array_key_exists($contentRepositoryId, $this->contentRepositories)) {
-            throw new \DomainException('undeclared content repository ' . $contentRepositoryId);
-        } else {
-            $contentRepository = $this->contentRepositories[$contentRepositoryId];
-            GherkinPyStringNodeBasedNodeTypeManagerFactory::initializeWithPyStringNode(
-                $serializedNodeTypesConfiguration,
-                $fallbackNodeTypeName
-            );
-            GherkinTableNodeBasedContentDimensionSourceFactory::$contentDimensionsToUse = $contentRepository->getContentDimensionSource();
-            $this->contentRepositories[$contentRepositoryId] = $this->createContentRepository(ContentRepositoryId::fromString($contentRepositoryId));
-            if ($this->currentContentRepository->id->value === $contentRepositoryId) {
-                $this->currentContentRepository = $this->contentRepositories[$contentRepositoryId];
-            }
-        }
-    }
-
     protected function setUpContentRepository(ContentRepositoryId $contentRepositoryId): ContentRepository
     {
         /**

diff --git a/...vioralTests/Classes/TestSuite/Behavior/GherkinPyStringNodeBasedNodeTypeManagerFactory.php b/...vioralTests/Classes/TestSuite/Behavior/GherkinPyStringNodeBasedNodeTypeManagerFactory.php
@@ -31,8 +31,6 @@ final class GherkinPyStringNodeBasedNodeTypeManagerFactory implements NodeTypeMa
 {
     public static ?NodeTypeManager $nodeTypesToUse = null;
 
-    public static ?string $fallbackNodeTypeName = null;
-
     /**
      * @param array<string,mixed> $options
      */
@@ -44,7 +42,7 @@ public function build(ContentRepositoryId $contentRepositoryId, array $options):
         return self::$nodeTypesToUse;
     }
 
-    public static function initializeWithPyStringNode(PyStringNode $nodeTypesToUse, ?string $fallbackNodeTypeName = null): void
+    public static function initializeWithPyStringNode(PyStringNode $nodeTypesToUse): void
     {
         self::$nodeTypesToUse = new NodeTypeManager(
             fn (): array => Yaml::parse($nodeTypesToUse->getRaw()),
@@ -54,12 +52,11 @@ public function create(NodeType $nodeType): NodeLabelGeneratorInterface
                     return new class implements NodeLabelGeneratorInterface {
                         public function getLabel(Node $node): string
                         {
-                            return $node->nodeType->getLabel();
+                            return $node->nodeTypeName->value;
                         }
                     };
                 }
-            },
-            $fallbackNodeTypeName
+            }
         );
     }
 

diff --git a/...ests/Tests/Behavior/Features/EventSourced/Migration/NodeTypeAdjustment_Dimensions.feature b/...ests/Tests/Behavior/Features/EventSourced/Migration/NodeTypeAdjustment_Dimensions.feature
@@ -50,11 +50,8 @@ Feature: Adjust node types with a node migration
     # Actual Test
     ########################
     # we remove the Document node type (which still exists in the CR)
-    When I change the node types in content repository "default" with fallback "Neos.ContentRepository:Fallback" to:
+    When I change the node types in content repository "default" to:
     """yaml
-    # !!fallback node is needed!! - TODO DISCUSS
-    'Neos.ContentRepository:Fallback': []
-
     'Neos.ContentRepository:Root':
       constraints:
         nodeTypes:

diff --git a/...ts/Tests/Behavior/Features/EventSourced/Migration/NodeTypeAdjustment_NoDimensions.feature b/...ts/Tests/Behavior/Features/EventSourced/Migration/NodeTypeAdjustment_NoDimensions.feature
@@ -48,11 +48,8 @@ Feature: Adjust node types with a node migration
     # Actual Test
     ########################
     # we remove the Document node type (which still exists in the CR)
-    And I change the node types in content repository "default" with fallback "Neos.ContentRepository:Fallback" to:
+    And I change the node types in content repository "default" to:
     """yaml
-    # !!fallback node is needed!! - TODO DISCUSS
-    'Neos.ContentRepository:Fallback': []
-
     'Neos.ContentRepository:Root':
       constraints:
         nodeTypes:

diff --git a/...itory.BehavioralTests/Tests/Behavior/Features/StructureAdjustment/UnknownNodeType.feature b/...itory.BehavioralTests/Tests/Behavior/Features/StructureAdjustment/UnknownNodeType.feature
@@ -5,11 +5,10 @@ Feature: Unknown node types
 
   Background:
     Given using no content dimensions
-    And using the following node types with fallback to "Neos.ContentRepository:Fallback":
+    And using the following node types:
     """yaml
     'Neos.ContentRepository:Root': []
     'Neos.ContentRepository.Testing:Document': []
-    'Neos.ContentRepository:Fallback': []
     """
     And using identifier "default", I define a content repository
     And I am in content repository "default"
@@ -40,10 +39,9 @@ Feature: Unknown node types
     Then I expect no needed structure adjustments for type "Neos.ContentRepository.Testing:Document"
 
   Scenario: When removing "Neos.ContentRepository.Testing:Document", we find a missing node type.
-    Given I change the node types in content repository "default" with fallback "Neos.ContentRepository:Fallback" to:
+    Given I change the node types in content repository "default" to:
     """yaml
     'Neos.ContentRepository:Root': []
-    'Neos.ContentRepository:Fallback': []
     """
     Then I expect the following structure adjustments for type "Neos.ContentRepository.Testing:Document":
       | Type              | nodeAggregateId |

diff --git a/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php b/Neos.ContentRepository.Core/Classes/NodeType/NodeType.php
@@ -223,15 +223,6 @@ protected function applyPostprocessing(array $fullConfiguration): array
         return $fullConfiguration;
     }
 
-    /**
-     * Returns the name of this node type
-     * @deprecated use "name" property directly
-     */
-    public function getName(): string
-    {
-        return $this->name->value;
-    }
-
     /**
      * Return boolean true if marked abstract
      */
@@ -663,7 +654,7 @@ protected function traverseSuperTypes(
         string $constraintNodeTypeName,
         int $distance
     ): ?int {
-        if ($currentNodeType->getName() === $constraintNodeTypeName) {
+        if ($currentNodeType->name->value === $constraintNodeTypeName) {
             return $distance;
         }
 

diff --git a/Neos.ContentRepository.Core/Classes/NodeType/NodeTypeManager.php b/Neos.ContentRepository.Core/Classes/NodeType/NodeTypeManager.php
@@ -40,8 +40,7 @@ class NodeTypeManager
 
     public function __construct(
         private readonly \Closure $nodeTypeConfigLoader,
-        private readonly NodeLabelGeneratorFactoryInterface $nodeLabelGeneratorFactory,
-        private readonly ?string $fallbackNodeTypeName
+        private readonly NodeLabelGeneratorFactoryInterface $nodeLabelGeneratorFactory
     ) {
     }
 
@@ -120,28 +119,13 @@ public function getNodeType(string|NodeTypeName $nodeTypeName): NodeType
             return $this->cachedNodeTypes[$nodeTypeName];
         }
 
-        if ($this->fallbackNodeTypeName === null) {
-            throw new NodeTypeNotFoundException(
-                sprintf(
-                    'The node type "%s" is not available and no fallback NodeType is configured.',
-                    $nodeTypeName
-                ),
-                1316598370
-            );
-        }
-
-        if (!$this->hasNodeType($this->fallbackNodeTypeName)) {
-            throw new NodeTypeNotFoundException(
-                sprintf(
-                    'The node type "%s" is not available and the configured fallback NodeType "%s" is not available.',
-                    $nodeTypeName,
-                    $this->fallbackNodeTypeName
-                ),
-                1438166322
-            );
-        }
-
-        return $this->getNodeType($this->fallbackNodeTypeName);
+        throw new NodeTypeNotFoundException(
+            sprintf(
+                'The node type "%s" is not available',
+                $nodeTypeName
+            ),
+            1316598370
+        );
     }
 
     /**

diff --git a/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/Node.php b/Neos.ContentRepository.Core/Classes/Projection/ContentGraph/Node.php
@@ -34,13 +34,13 @@
  *
  * @api Note: The constructor is not part of the public API
  */
-final class Node
+final readonly class Node
 {
     /**
      * @internal
      */
     public function __construct(
-        public readonly ContentSubgraphIdentity $subgraphIdentity,
+        public ContentSubgraphIdentity $subgraphIdentity,
         /**
          * NodeAggregateId (identifier) of this node
          * This is part of the node's "Read Model" identity, whis is defined by:
@@ -50,15 +50,23 @@ public function __construct(
          * With the above information, you can fetch a Subgraph using {@see ContentGraphInterface::getSubgraph()}.
          * or {@see \Neos\ContentRepositoryRegistry\ContentRepositoryRegistry::subgraphForNode()}
          */
-        public readonly NodeAggregateId $nodeAggregateId,
+        public NodeAggregateId $nodeAggregateId,
         /**
          * returns the DimensionSpacePoint the node is at home in. Usually needed to address a Node in a NodeAggregate
          * in order to update it.
          */
-        public readonly OriginDimensionSpacePoint $originDimensionSpacePoint,
-        public readonly NodeAggregateClassification $classification,
-        public readonly NodeTypeName $nodeTypeName,
-        public readonly NodeType $nodeType,
+        public OriginDimensionSpacePoint $originDimensionSpacePoint,
+        public NodeAggregateClassification $classification,
+        /**
+         * The node's node type name; always set, even if unknown to the NodeTypeManager
+         */
+        public NodeTypeName $nodeTypeName,
+        /**
+         * The node's node type, null if unknown to the NodeTypeManager
+         * @deprecated Don't rely on this too much, as the capabilities of the NodeType here will probably change a lot;
+         * Ask the {@see NodeTypeManager} instead
+         */
+        public ?NodeType $nodeType,
         /**
          * Returns all properties of this node. References are NOT part of this API;
          * there you need to check getReference() and getReferences().
@@ -67,9 +75,9 @@ public function __construct(
          *
          * @param PropertyCollection $properties Property values, indexed by their name
          */
-        public readonly PropertyCollection $properties,
-        public readonly ?NodeName $nodeName,
-        public readonly Timestamps $timestamps,
+        public PropertyCollection $properties,
+        public ?NodeName $nodeName,
+        public Timestamps $timestamps,
     ) {
     }
 
@@ -108,7 +116,7 @@ public function hasProperty(string $propertyName): bool
      */
     public function getLabel(): string
     {
-        return $this->nodeType->getNodeLabelGenerator()->getLabel($this);
+        return $this->nodeType?->getNodeLabelGenerator()->getLabel($this) ?: $this->nodeTypeName->value;
     }
 
     public function equals(Node $other): bool