[
https://issues.apache.org/jira/browse/JCR-3371?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13407037#comment-13407037
]
Randall Hauch commented on JCR-3371:
------------------------------------
Apologies for the wrong patch format. Your additional changes are good, except
for one thing that I'm not sure of. In the new
'testRemoveMixinFromSharedNode()" method, the last few lines of the proposed
try block are
// If this happens, then b1 nor b2 shouldn't be shareable anymore
// ...
assertFalse(b1.isNodeType(mixShareable));
assertFalse(b2.isNodeType(mixShareable));
assertFalse(b2.isSame(b1));
where 'b1' and 'b2' are nodes that were in the same shared set before the
removal of the mixin. While it seems logical to have these checks, they do seem
to go beyond what section 14.15 "RemoveMixin" in the specification requires,
which actually states that the implementation "may either throw a
ConstraintViolationException or allow the removal and change the subgraph in
some implementation-specific manner."
I can imagine one possible behavior allowed by the spec but not allowed by this
test would be if the implementation, upon the client removing the
'mix:shareable' mixin from node b1, simply removed the mixin as requested and
then treated that as an "unlinking" of that node from the shared set. Per the
spec, the implementation could replace b1 with a copy of the shared content, or
even leave b1 as a simple node with no children. IIUC, the implementation does
not require changing any of the other nodes in the shared set (which in this
case consists only of b2).
Therefore, I think the test should not assume/require that b2 is no longer
shareable, and that the *second-to-last line* listed above should not be
included in the change.
> TCK test for shareable nodes incorrectly assumes the 'mix:shareable' mixin
> cannot be removed
> --------------------------------------------------------------------------------------------
>
> Key: JCR-3371
> URL: https://issues.apache.org/jira/browse/JCR-3371
> Project: Jackrabbit Content Repository
> Issue Type: Task
> Components: jackrabbit-jcr-tests, JCR 2.0, test
> Affects Versions: 2.5
> Reporter: Randall Hauch
> Assignee: Julian Reschke
> Fix For: 2.5.1, 2.6
>
> Attachments: JCR-3371-CorrectedShareableNodeTest-logic.patch,
> JCR-3371.patch
>
>
> The ShareableNodeTest.testRemoveMixin() assumes that removing the
> "mix:shareable" mixin will *always* throw an UnsupportedOperationException.
> This is not only checking for the incorrect exception, section 14.15
> "RemoveMixin" specifically states that it *is* possible for an implementation
> to support removing the 'mix:shareable' mixin:
> "If an attempt is made to remove the mix:shareable mixin node type from a
> node in a
> shared set the implementation may either throw a
> ConstraintViolationException or allow
> the removal and change the subgraph in some implementation-specific
> manner.
> One possibility is to replace the node with a copy that has no children
> (if this does not
> violate the node type restrictions of that node). Another possibility is
> to give the node
> a copy of all of its descendants (unless the resulting copy operation
> would be unfeasible,
> as would be the case if a share cycle were involved)."
> Thus, even though it is possible for an implementation not to allow removing
> the "mix:shareable" mixin, the test shouldn't expect that an implementation
> will throw an exception. For example, one particularly easy thing for an
> implementation to support is removing 'mix:shareable' if and only if there
> are no other shared nodes (e.g., the "getSharedSet().getSize() == 1").
> This test could be removed, or perhaps it might be possible to test that
> removing the mixin (and saving) will either succeed or will throws only a
> ConstraintViolationException.
> I marked as critical because the TCK test prevents other implementations from
> correctly proving compatibility.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
