Details

    • Bug
    • Status: Open
    • Major
    • Resolution: Unresolved
    • 1.42.0
    • None
    • None
    • None

    Description

      Node types may have mandatory child nodes.
      Those constraints are not enforced when creating new nodes or deleting the mandatory child node if the mandatory child node has no name constraint (i.e. is residual). It works well if the mandatory child node has a name constraint as well.

      Attachments

        Issue Links

        Activity

          If we introduce a configuration option I would rather do it to optionally enable 3. not 1. IMHO no one will ever use a configuration option to enable 1 (because there is just no convincing reason to do so)

          kwin Konrad Windszus added a comment - If we introduce a configuration option I would rather do it to optionally enable 3. not 1. IMHO no one will ever use a configuration option to enable 1 (because there is just no convincing reason to do so)
          miroslav Miroslav Smiljanic added a comment - - edited

          +1 for documenting the deviation from the spec. 

          I think we should keep the current behaviour. In addition expose configuration option (not enabled by default) that will mandate behaviour as per spec.

          Migration to the structure required by the spec can be implemented as oak-run command. 

          miroslav Miroslav Smiljanic added a comment - - edited +1 for documenting the deviation from the spec.  I think we should keep the current behaviour. In addition expose configuration option (not enabled by default) that will mandate behaviour as per spec. Migration to the structure required by the spec can be implemented as oak-run command. 

          3. could break consumers relying on the non-enforcing part. IMHO it is too late to change anything about the current behaviour. So I am in favour of option 2.

          kwin Konrad Windszus added a comment - 3. could break consumers relying on the non-enforcing part. IMHO it is too late to change anything about the current behaviour. So I am in favour of option 2.
          reschke Julian Reschke added a comment -

          FWIW, this (disallowing mandatory on residual definitions) might be a change in JCR 2.0; it doesn't seem to be present in https://www.adobe.io/experience-manager/reference-materials/spec/jcr/1.0/index.html

          reschke Julian Reschke added a comment - FWIW, this (disallowing mandatory on residual definitions) might be a change in JCR 2.0; it doesn't seem to be present in https://www.adobe.io/experience-manager/reference-materials/spec/jcr/1.0/index.html
          reschke Julian Reschke added a comment - - edited

          So Oak (and presumably Jackrabbit) support node types (combining mandatory+residual constraints) that aren't allowed per the JCR spec.

          Options:

          1. Change the implementations to reject these node type definitions. That would be the "correct" approach, but would apparently break existing installations (see JCRVLT-549 where this came up first). It would also require some form of migration for existing content. (not realistic, just added for completeness)
          2. Keep the current implementation (node type is accepted, but constraint is not enforced). (this has the best backwards compat story, as we do not change anything)
          3. Accept the node type, and actually try to enforce it.

          I think we should investigate the last option ("fixing it"), because there's presumably a reason why people use these node definitions.

          In any case (except for the first option), we need to document this deviation from the spec properly.

          reschke Julian Reschke added a comment - - edited So Oak (and presumably Jackrabbit) support node types (combining mandatory+residual constraints) that aren't allowed per the JCR spec. Options: Change the implementations to reject these node type definitions. That would be the "correct" approach, but would apparently break existing installations (see JCRVLT-549 where this came up first). It would also require some form of migration for existing content. ( not realistic, just added for completeness ) Keep the current implementation (node type is accepted, but constraint is not enforced). (this has the best backwards compat story, as we do not change anything) Accept the node type, and actually try to enforce it. I think we should investigate the last option ("fixing it"), because there's presumably a reason why people use these node definitions. In any case (except for the first option), we need to document this deviation from the spec properly.
          kwin Konrad Windszus added a comment - - edited

          Haven't tested in JR2 but according to JCR 2.0 this is a violation of the spec (and should potentially be covered in the TCK)

          Update: not so sure anymore after I read https://www.adobe.io/experience-manager/reference-materials/spec/jcr/2.0/3_Repository_Model.html#3.7.2.4%20Mandatory

          3.7.2.4.3 Mandatory and Residual Definitions
          In repositories that support residual definitions, an item cannot be both mandatory and residual (see §3.7.2.1.2 Item Definition Name and Residual Definitions).

          kwin Konrad Windszus added a comment - - edited Haven't tested in JR2 but according to JCR 2.0 this is a violation of the spec (and should potentially be covered in the TCK) Update: not so sure anymore after I read https://www.adobe.io/experience-manager/reference-materials/spec/jcr/2.0/3_Repository_Model.html#3.7.2.4%20Mandatory 3.7.2.4.3 Mandatory and Residual Definitions In repositories that support residual definitions, an item cannot be both mandatory and residual (see §3.7.2.1.2 Item Definition Name and Residual Definitions).
          reschke Julian Reschke added a comment -

          Interesting.

          Does this fail in Jackrabbit as well?

          reschke Julian Reschke added a comment - Interesting. Does this fail in Jackrabbit as well?
          kwin Konrad Windszus added a comment - I added a failing test in https://github.com/apache/jackrabbit-oak/pull/414/files .

          People

            Unassigned Unassigned
            kwin Konrad Windszus

            Dates

              Created:
              Updated:

              Slack