[WIP] Prevent creating same name siblings

[fazy]

Added "clone" test cases for existing, non-corresponding nodes in the destination workspace.

Attempting to clone over an existing node with a different UUID should result in PHPCR\ItemExistsException being thrown, with or without removeExisting=true.

As discussed here:
https://groups.google.com/forum/?fromgroups=#!topic/symfony-cmf-devs/Ro12vnxInG8

[fazy]

This test currently fails, until we manage to fix the problem in jackalope-jackrabbit.

[fazy]

I'm not 100% sure about this change. The JCR documentation states that "The clone method clones both referenceable and non-referenceable nodes", but it doesn't explicitly says how "removeExisting=true" should behave for non-referenceable nodes.

From our discussion on the symfony-cmf-devs list I got the impression that we probably shouldn't allow removeExisting for non-referenceable nodes. However, the alternative (remove and replace the existing node) is equally arguable.

[dbu]

from how i read that section, i would say that if either node is not referenceable then it has no identifier, hence removeExisting has no effect. the application would have to remove the target manually first. (unless same-name siblings are allowed, that is. probably this test first should check that capability and skip if that capability is supported by the repository. in that case a new same-name sibling would be the correct thing to happen.)

[dbu]

or set the expected exception in code instead of the annotation in case same-name is not supported, otherwise expect that there are 2 children with that name now - that would be forward compatible.

[fazy]

So for a non-referenceable node, removeExisting should have no effect (but should throw an exception in that case?)

For the case with same name siblings/forward compatibility, I would hesitate here, because if we support SNS then there are many more tests that should be done.

[dbu]

if there is something with the same name and we do not support SNS, we would need the exception yes. otherwise never (f.e. if internally uuid are assigned even for not referenceable nodes - then a new uuid would have to be created. but i think jackrabbit handles that for us, right?)

the thing about forward compatible is that if SNS are allowed, this test is completely wrong as no exception is to be assumed. lets check the capability and mark the test skipped if SNS are supported, saying that this needs to be done with an implementation that actually supports SNS. doing the test that is never run would be weird...

[fazy]

Ok, I've skipped the tests that would probably be wrong if SNS support is enabled. All of those tests would need to look for different behaviour with/without SNS.

To make this possible, I needed to change the Jackalope-Jackrabbit client so that it doesn't claim to support SNS, see this PR.

[dbu]

cool, thanks. is this now ready to merge? if it fails with jackalope-jackrabbit because we did not merge the bugfix, that is ok. hiding the error does not make it go away :-)

[fazy]

Unfortunately I think there's 1 more test (non referenceable source, referenceable node at dest path) which still needs to be written. Can do first thing tomorrow.

[dbu]

ok, then i will wait with merging.

[fazy]

I believe this is all the tests now so should be ok to merge. However, I'll work on the Jackalope-Jackrabbit back end this morning, and knowing me I'll probably discover something wrong with the tests. ;)

[fazy]

I'm happy with this now...

[dbu]

thanks!