NodeCreation: childNodes constraints are not evaluated correctly if inserting items into Neos.Neos:ContentCollection

[patricekaufmann]

When upgrading from 2.0.0 to 2.0.1, one template configuration stops working. Extract:

  options:
    template:
      childNodes:
        content:
          name: 'content'
          childNodes:
            text:
              type: 'KaufmannDigital.Nova.NodeTypes.Text:Atom.Text'

Apparently the exception is thrown at Classes/Domain/NodeCreation/NodeCreationService.php L97+

                if (!$parentNode->getNodeType()->allowsChildNodeType($nodeType)) {
                    $caughtExceptions->add(
                        CaughtException::fromException(new \RuntimeException(sprintf('Node type "%s" is not allowed for child nodes of type %s', $template->getType()->getValue(), $parentNode->getNodeType()->getName()), 1686417627173))
                    );
                    continue;
                }

The exception that's coming up:

RuntimeException(Node type "KaufmannDigital.Nova.NodeTypes.Text:Atom.Text" is not allowed for child nodes of type Neos.Neos:ContentCollection, 1686417627173)

I am not 100% certain yet but I think the issue is coming up because in that template we are inserting nodes into generic content collections, however the constraint is checked via allowsChildNodeType in NodeType, which however is not considering special constraints defined for those collections. The constraints are defined exeplicitely:

  childNodes:
    content:
      type: 'Neos.Neos:ContentCollection'
      constraints:
        nodeTypes:
          'KaufmannDigital.Nova.NodeTypes.Text:Atom.Text': TRUE

[mhsdesign]

Thank you for the report. I will investigate this. The regression was caused by #57

[mhsdesign]

Hi, im having trouble reproducing your issue. Can you please give me a nodeTypes definition with more details?

this example will pass in my tests just fine:

'Flowpack.NodeTemplates:Content.ConstrainedChildNodes':
  superTypes:
    'Neos.Neos:Content': true
  childNodes:
    content:
      type: 'Neos.Neos:ContentCollection'
      constraints:
        nodeTypes:
          'Flowpack.NodeTemplates:Content.Text': true
  options:
    template:
      childNodes:
        content:
          name: 'content'
          childNodes:
            text:
              type: 'Flowpack.NodeTemplates:Content.Text'
              properties:
                text: 'Text'

while this fails (did you mean it like this?):

'Flowpack.NodeTemplates:Content.RestrictedContentCollection':
  superTypes:
    'Neos.Neos:ContentCollection': true
  constraints:
    nodeTypes:
      '*': false

'Flowpack.NodeTemplates:Content.ConstrainedChildNodes':
  superTypes:
    'Neos.Neos:Content': true
  childNodes:
    content:
      type: 'Flowpack.NodeTemplates:Content.RestrictedContentCollection'
      constraints:
        nodeTypes:
          'Flowpack.NodeTemplates:Content.Text': true
  options:
    template:
      childNodes:
        content:
          name: 'content'
          childNodes:
            text:
              type: 'Flowpack.NodeTemplates:Content.Text'
              properties:
                text: 'Text'

im not really sure right now, if it is invalid that we fail. The new escr will actually fail too for the last example. https://neos-project.slack.com/archives/C3MCBK6S2/p1687357177485239
Can you confirm, that your node type structure looks like my second example? @patricekaufmann

[patricekaufmann]

Thanks for looking into this so quickly. I cannot confirm that the structure looks like your second example. Not exactly to be precise. While looking at your second example the '*': false part looked familiar. To enforce an allow-list approach on the constraints definitions, we have the following part in a separate package:

'Neos.Neos:ContentCollection':
  constraints:
    nodeTypes:
      # enforce allow-list approach
      '*': FALSE

Once I remove this part, the exceptions are no longer thrown. However now it's possible to insert every node because the default deny no longer applies.

Btw.: We really like the constraints being explicitely allowed rather than being explicitely disallowed. I think that no longer being possible would be somewhat of a downgrade unless there are other ways in the future?

[mhsdesign]

i see, i already thought so.

I already started the bigger discussion in neos/neos-development-collection#4351 if the neos9 behavior (which i replicated) is actually intentional, and that we should consider your usecase.

For now i can imagine two options. Either i revert the constraint right now. (I will do that right away in the case the mentioned discussion is in your favor)

Or change the constraint to something the like:

'Flowpack.NodeTemplates:Content.ConstrainedChildNodes':
  superTypes:
    'Neos.Neos:Content': true
  childNodes:
    content:
      type: 'Neos.Neos:ContentCollection'
      constraints:
        nodeTypes:
          '*': false
          'Flowpack.NodeTemplates:Content.Text': true

It is generally speaking bad praxis to override NodeTypes, so you should use your own extended restricted content collection in other places instead.

But this might not be a realistic option for your project right now.

A super simple work around would be in your case:

'Your.Vendor:ContentCollection.WithText':
  superTypes:
    'Neos.Neos:ContentCollection': true
  constraints:
    nodeTypes:
      'Flowpack.NodeTemplates:Content.Text': true

you can use it like:

  childNodes:
    content:
      type: 'Your.Vendor:ContentCollection.WithText'

Sorry for causing the inconvenience, and thank you for your patience, but not sorry, because its important we discuss this for neos9 - so thank you ;)

I will wait a few days and see how the discussion turns out and think about this myself.
I think reverting the constraint is the most logical choice right now, but let me think about this for a second ;)

[mhsdesign]

Since the new CR was "wrong" i created a fix to align the behavior with neos 8.3 neos/neos-development-collection#4361

Thats why there should be no reason for NodeTemplates to behave incorrectly, i restored the old behavior with #67 which will be released soon.