do not expose UUID unless the node is referenceable

[lsmith77]

it seems we already have some logic in place for this https://github.com/jackalope/jackalope-doctrine-dbal/blob/master/src/Jackalope/Transport/DoctrineDBAL/Client.php#L1095

however while working on some issues in PHPCR ODM we noticed that it was possible to set a UUID (this might make sense) and then search by UUID even on a non referenceable node (this results in people being able to write code where all nodes behave like referenceable nodes which can lead to problems when migrating to a different backend).

[dbu]

i think the actual bug here is something else: the property jcr:uuid of mix:referenceable is autocreated mandatory protected INITIALIZE

so i think at least after creation, you should never be allowed to change that value. i just experimented with the jackrabbit implementation in java, and it behaves horribly (jackrabbit 2.6.0):

• create a node with jcr:uuid property, save, load, change uuid, save => ItemNotFoundException in the middle of spi2dav
• add a node, create jcr:uuid property, add mix:referenceable mixin => "RepositoryException: failed to add property {http://www.jcp.org/jcr/1.0}uuid to node /y" - its not checking if the property already exists

when i add the mixin first, i get "ConstraintViolationException: Item is protected" so with jackrabbit, its not possible to manually decide on the uuid.

imho the sane way would be to say:

• if the node is not mix:referenceable, its a violation to add a jcr:uuid property as jcr is the implementation namespace
• if the node is mix:referenceable, you can only set this property on a newly created node that was not flushed. otherwise we have to throw the constraint violation.

[lsmith77]

so with jackrabbit, its not possible to manually decide on the uuid.

not sure I understand .. we have been manually setting the UUID in PHPCR ODM for a while now. or are you saying that it cannot be changed manually?

[dbu]

i think in jackalope-jackrabbit we allow to set it on creation. and the jackrabbit davex remoting allows to set it. later change is prevented by the jackrabbit server side.

but the jackrabbit java code does not allow to ever set the uuid aparently.

[ElectricMaxxx]

In our cases the phpcr-odm creates and sets the uuid, when the node got mix:referenceable property:
https://github.com/doctrine/phpcr-odm/blob/master/lib/Doctrine/ODM/PHPCR/UnitOfWork.php#L3627-L3645

Seeing that i can't believe why the test does not fail for all implementations. Or is there a difference in setting that property between the implementations? Cause for some reasons, the example worked without setting referenceable = true on the document for jackalope-doctrine-dbal.

[dbu]

ah, indeed the semantics should be that when mix:referenceable was newly added there is no jcr:uuid property until save. if you set it manually, you chose the uuid. after the save its autogenerated and becomes immutable.

[dbu]

i think we do have part one, you can just set it. but when you have the mixin, validation should prevent changing that. i think we need a lot more of validating the node types in jackalope. in the beginning we just relied on the jackrabbit backend to do the validation (it does, but only at save) and have nothing for the doctrine transport.

i thought there was a general issue about validation, but only found #171

[ElectricMaxxx]

From my POV the phpc-odm should have nothing to do with the implementations. @dbu you said same to me on an other issue. So i would expect that the uuid generation is a task of the implementation and the phpcr-odm should only do: $session->save()

[dbu]

the problem is that when we create documents that reference each other, we would need several save to get that right, or do a second iteration to link them together. and as all known phpcr implementations support setting the uuid when the node does not yet have a uuid we did it like that...

i guess for this ticket: i would prevent writing jcr:uuid on a node that is not mix:referenceable. as its in the jcr namespace its ok to limit what may be done. this will solve this issue and reveal mistakes.

i tend to want to keep our current logic that you can manually chose the uuid of a new node/newly made referenceable node for convenience. but surely not change it once it exists.

[dantleech]

+1 for the manual assignation of the UUID before the first save, it would be madly inefficient otherwise :)

[ElectricMaxxx]

@dantleech with manually you mean, that the user, in our case phpcr-odm creates the uuid and sets it to node/document?

[dantleech]

yes, as we do now I believe

[dbu]

see jackalope/jackalope#270

[dbu]

hm, note that the jackalope change will prevent setting a uuid on a non-referenceable node, and will never expose a uuid on them. but if you look up the uuid in the database, searching for it will still work. not sure if we should care about that at all - can't really see when this is a problem. if people do horrible hacks with the db its their own loss...