Recently I found out that our Neos.Neos:ContentCollection only disallows Documents but allows everything else, even if the NodeType does not inherit from Neos.Neos:Content.
This has never been a big issue in Neos as the UI prohibits other types from being used.
But when working with the CR (headless Neos f.e.) or when analysing the raw nodetype data, the constraint is different and one gets the option to add whatever they want to Content Collections.
But even though I had some tests to verify the behaviour, this broke the Constraint best practices we taught the community because suddenly everything of type Content was allowed, even if they had '*': false in their constraints. So to prevent everyone from adapting their existing project I reverted the change in https://github.com/neos/neos-development-collection/pull/2978.
The problem:
I still want to fix this possibly with 6.0. But this is not so easy, because when disallowing Neos.Neos:Content and allowing something like My.Site:Constraint.MyCollection the inheritance distance of a Content element which implements both types is the same. And our algorithm in https://github.com/neos/neos-development-collection/blob/master/Neos.ContentRepository/Classes/Domain/Model/NodeType.php#L651 says: “If allow and disallow have the same distance, disallow wins.”.
Therefore it’s not easily possible to use constraints without a hack (like creating another abstract Content type).
Example:
This is an example of a structure that would not allow adding child nodes anymore when my initial bugfix is applied and with the current algorithm.
How do we treat the case when allow and disallow have the same weight?
In the past the current behaviour was fine, as we mostly used blacklisting for NodeTypes.
But now that we have indirect whitelisting via the Constraints, this approach creates a problem.
What do you think about the idea to make the above mentioned line configurable as first step? The default would be the known behavior but could be switched to have ‘true’ preference over ‘false’.
A best practice could be to switch the behavior together with applying the additional NodeType constraint from https://github.com/neos/neos-development-collection/commit/b48660b28c1de596e74d4a95b8547d743b5199f1.
I’m scared a switch will make debugging and understanding a project hard. Changing both with 6.0 and including a migration instruction would of course be a viable option.
But depends on what we want of course.
It is obviously breaky and thus labeled for next master.
Probably a migration could be added to adjust constraints '*' : false to Neos.Neos:Content: false for all NodeTypes and childNodes that are or are derived from Neos.Neos:ContentCollection.
To be honest i do not totally understand the problem with the weight:
If a constraint is defined for Neos.Neos:Content in my understanding i would assume this beeing farther away then Vendor.Site:Content.Image maybe we should adjust the distance calculation but i think by counting the number of supertypes we have the informations to order the constraints.
I finally got the point … sorry for the first unreflected answer.
I would suggest to use the order to decide between constraints of the same weight and let the last one win. Actually i always assumed this worked that way and never looked it up im the code.
I also somehow assumed that ContentCollection would extend Content which also is not the case. Maybe that should be changed aswell to reduce the special nodetypes neos has to Content and Document.
Damn. We didn’t see the other PR
Also the breakiness was not so obvious for me, because like you I also had some assumptions about how the algorithm works. And our tests and my test cases were too limited.
I don’t think using the order for a decision works. When all the configurations for one type are merged, I’m not sure if the result is still what the integrator expects. I’m not even sure if we know what the result would be.
That’s why we usually have the positional array sorter in most places, which would be also strange to introduce here.
I didn’t fully understand the benefit of letting ContentCollection inherit from Content? Can you elaborate on that?
Just to reduce the number of special nodetypes. Would be nice to only have Content and Document but probably ContentCollection is really needed aswell as it holds the cache configuration.
We could introduce Constraint as a Neos Concept by having an abstract Neos.Neos:Constraint Supertype that beats others when Constraints are resolved.
Yes I don’t see a good way right now without ContentCollection.
I also thought a bit about having the idea of Constraint as a core concept and including a supertype for it.
But it feels more like a workaround. Inversing the mentioned condition and assuming that every package moves to whitelisting could also simply solve the problem. But we need test cases and real world examples to see which way is the best for the next years to come.
