!!! TASK: Followup #4520 Introduce NodeType::tetheredNodeTypeDefinitions

[mhsdesign]

Introduces new value objects TetheredNodeTypeDefinitions and TetheredNodeTypeDefinition (ported from #4999) to simplify NodeType and NodeTypeManager api.

Upgrade instructions

From 8.3

- foreach ($nodeType->getAutoCreatedChildNodes() as $rawNodeName => $childNodeType) {
-     $nodeName = NodeName::fromString($rawNodeName);
+ foreach ($nodeType->tetheredNodeTypeDefinitions as $tetheredNodeTypeDefinition) {
+     $nodeName = $tetheredNodeTypeDefinition->name;
+     $childNodeType = $nodeTypeManager->getNodeType($tetheredNodeTypeDefinition->nodeTypeName);

- $nodeType->hasAutoCreatedChildNode($nodeName);
+ $nodeType->tetheredNodeTypeDefinitions->contain($nodeName);

- $parentsNodeType->getTypeOfAutoCreatedChildNode($nodeName)->name;
+ $parentsNodeType->tetheredNodeTypeDefinitions->get($nodeName)->nodeTypeName;

- $parentsNodeType->getTypeOfAutoCreatedChildNode($nodeName);
+ $nodeTypeManager->getNodeType($parentsNodeType->tetheredNodeTypeDefinitions->get($nodeName));

- $grandParentsNodeType->allowsGrandchildNodeType($parentNodeName->value, $nodeType);
+ $nodeTypeManager->isNodeTypeAllowedAsChildToTetheredNode($grandParentsNodeType->name, $parentNodeName, $nodeType->name);

For upgrade from 9.0-dev please note that the temporary methods: NodeTypeManager::getTetheredNodesConfigurationForNodeType, NodeTypeManager::getTypeOfTetheredNode, NodeType::hasTetheredNode and NodeType::getNodeTypeNameOfTetheredNode have been removed again and must be replaced with NodeType::$tetheredNodeTypeDefinitions

Review instructions

Ui part: neos/neos-ui#3782

Rector migrations will be provided as part of neos/rector#57

As said here #4520 (comment) writing

$nodeTypeManager->getTetheredNodesConfigurationForNodeType($nodeTypeManager->getNodeType($nodeTypeName))

feels not right, the nodeTypeManger should just operate on NodeTypeNames.

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the lowest maintained branch
• Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
• Reviewer - The first section explains the change briefly for change-logs
• Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

[mhsdesign]

As alternative trick we can as well make these hardcore mehtods internal like who really understands isNodeTypeAllowedAsChildToTetheredNode ?? :D Usages are limited to Neos.Neos and FlowpackNodeTemplates

[mhsdesign]

Todo these rector migration would need to be adjusted so they dont pass the $nodeType to the methods, but $nodeType->name:

• NodeTypeGetAutoCreatedChildNodesRector
• NodeTypeGetTypeOfAutoCreatedChildNodeRector
• NodeTypeAllowsGrandchildNodeTypeRector

[mhsdesign]

@bwaidelich as you also currently refactor the NodeTypeManager (#4999) ... do you think we should also incorporate this change (i yet have to fix ci but just want to know if its worth the effort)

[bwaidelich]

do you think we should also incorporate this change

We'll have to since Node::nodeType will most probably be removed

[dlubitz]

Looks good in general. I stumled over the method names. Should we change them as we changed the parameter from NodeType to NodeTypeName?

E.g:
isNodeTypeAllowedAsChildToTetheredNode =>isNodeTypeNameAllowedAsChildToTetheredNode

[mhsdesign]

Should we change them as we changed the parameter from NodeType to NodeTypeName?

good point but i dont think so as the thing the user of the api tries to solve if a NodeType is allowed or not ... that the node type is identified via NodeTypeName can be left out in the method name i think.

---

Another thing i just remembered is that we want to be rather forgiving (see #4954) so i dont know if all the new requireNodeType call would do us good or if they would be a pita in the future. Have to think if it makes sense to return false for booleans or just null otherwise.

[bwaidelich]

isNodeTypeAllowedAsChildToTetheredNode =>isNodeTypeNameAllowedAsChildToTetheredNode

That doesn't make sense to me. Yes, we pass in a node name but this method is about the allowed node type

[mhsdesign]

i applied a little hack here which is allowed even in php 8.3 and also documented. Its temporary but ugly.

All done to be forwards compatible with #4999, i just backported the TetheredNodeTypeDefinitions vo and NodeType::tetheredNodeTypeDefinitions

cc @bwaidelich

[mhsdesign]

see also #4523 where i explain the hack and https://www.php.net/manual/en/language.oop5.properties.php

Readonly properties cannot be unset() once they are initialized. However, it is possible to unset a readonly property prior to initialization, from the scope where the property has been declared.

[mhsdesign]

we (christian basti me) discussed that we want to go with this hack for now with the vision that the nodeType will be at one point a simple model like:

public function __construct(
  // ...
  public readonly TetheredNodeTypeDefinitions $tetheredNodeTypeDefinitions,
  // ...
) {}

[mhsdesign]

As stated in #4228 (comment) we will probably NOT have that "planned"/ beloved followup in Neos 9 as this is a bigger one and requires lots of work.

The assumption was that the followup will go in and we can remove the __get hack again. We said for the (at that time) less likely event that the followup doesnt come, we will revert this and fallback to regular getters.

But there is one elephant in the room i was not aware of: Php8.4. It will come with support for such computed properties (see https://stitcher.io/blog/new-in-php-84#property-hooks-rfc) meaning we would have what we want at real language level supported.

The question is, can we live with this hack for the time until php 8.4 when we can refactor it to use the real get hook?
My worst fear, that php wouldnt support our syntax forever is gone now at least :D

Wdyt cc @kitsunet

[kitsunet]

👀