JCRVLT-767: vlt: potential incorrect identifier comparison #348
Conversation
vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java
Outdated
Show resolved
Hide resolved
| // if this is mix:ref and ni specifies an identifier, | ||
| // try to set it | ||
| try { | ||
| uuidProp = node.setProperty(Property.JCR_UUID, identifier.get()); |
There was a problem hiding this comment.
isn't the uuid already set through the sysview import?
There was a problem hiding this comment.
I thought we get here in case we skipped that...?
…etection of referenceable node)
…t fail on classic Jackrabbit by checking reh repo name desc as well)
| boolean isUuidProtected = false; | ||
| if (node.hasProperty(Property.JCR_UUID)) { | ||
| isUuidProtected = node.getProperty(Property.JCR_UUID).getDefinition().isProtected(); | ||
| } else if (! session.getRepository().getDescriptor(Repository.REP_NAME_DESC).contains("Oak")) { |
There was a problem hiding this comment.
why else if? Shouldn't this also overwrite isUuidProtected from line 995 potentially?
There was a problem hiding this comment.
good question; tests passed with Jackrabbit; will have another look
There was a problem hiding this comment.
might have to do with https://jackrabbit.apache.org/oak/docs/differences.html#Identifiers - but code is unclear and needs fixing
There was a problem hiding this comment.
Should better wirh 223bb00
…ode that detects non-Oak repos)
| if (! session.getRepository().getDescriptor(Repository.REP_NAME_DESC).contains("Oak")) { | ||
| // workaround for non-Oak repos that may not be able to add | ||
| // mixin:referencable *after* setting jcr:uuid | ||
| isUuidProtected = true; |
There was a problem hiding this comment.
The original idea was to encapsulate impl specific differences in https://github.com/apache/jackrabbit-filevault/blob/master/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/spi/ServiceProvider.java.
There was a problem hiding this comment.
Thanks for the pointer; let's do more research on the "setting of jcr:uuid" first, and the outcome of that would inform what information we would want to add there.
(work in progress)
This appears to work, except in Jackrabbit, which is unhappy about the sequence "setProperty( ... uid ...)" followed by "addMixing(mix:ref)".