VersionManager.checkin(...) descends below any versionable nodes within that subgraph

When checking in a 'mix:versionable' node, the VersionManager implementation creates a new version in the node's version history, and copies the versionable state of that node into that new version via a single 'nt:frozenNode'. The versionable state includes all versionable properties and children, as dictated by the property & child node definitions' On Parent Versioning (OPV) value.

Our JcrVersionManager class implements this behavior by creating the 'nt:frozenNode' inside the JcrVersionManager.checkin(...) method, and then calling 'versionNodeAt(...)' on all children. The 'versionNodeAt(...)' method calls itself recursively for all children where the child node definition has an OPV of COPY or VERSION, though it skips and returns for any child where the definition has an OPV of IGNORE, INITIALIZE, or COMPUTE.

However, the 'versionNodeAt(...)' method should not be recursive when the child node has a child node definition of an OPV of VERSION and the child node is 'mix:versionable'. Instead, it should create the 'nt:versionedChild' node (as it does not) and simply return rather than break out of the switch. In other words, the 'break' should be changed to 'return' at https://github.com/ModeShape/modeshape/blob/ffefa17a/modeshape-jcr/src/main/java/org/modeshape/jcr/JcrVersionManager.java#L434

I've changed this locally, and it does not affect the outcome of any of the JCR TCK tests, which means none of the TCK tests really check this. Because of this, I would like to have some verification that my interpretation is correct.

For example, consider this subgraph:

+ A mix:versionable
  + B
  + C
     + D mix:versionable
        + E

If node A is checked in, then the implementation should create a new version in the version history for A under '/jcr:system/jcr:versionStorage', and this new version node V should contain a single 'nt:frozenNode' node that is the snapshot of the versionable subgraph of A. Thus, nodes B and C will be copied into this frozen node, and node D will have a 'nt:versionedChild' node created under the version's representation/copy of C.

Without the proposed change, the 'nt:versionedChild' representation of D will actually contain a full copy of the state of D. This is not correct, since the whole point of the 'nt:versionedChild' node is to REFERENCE the actual version history of D. See the image in the JSR-283 specification.