JCRVLT-828: Fix handling of target node UUID for IdConflictPolicy.LEGACY (#398)
diff --git a/vault-core-it/vault-core-integration-tests/src/main/java/org/apache/jackrabbit/vault/packaging/integration/ReferenceableIdentifiersImportIT.java b/vault-core-it/vault-core-integration-tests/src/main/java/org/apache/jackrabbit/vault/packaging/integration/ReferenceableIdentifiersImportIT.java
index 7694e50..7c10672 100644
--- a/vault-core-it/vault-core-integration-tests/src/main/java/org/apache/jackrabbit/vault/packaging/integration/ReferenceableIdentifiersImportIT.java
+++ b/vault-core-it/vault-core-integration-tests/src/main/java/org/apache/jackrabbit/vault/packaging/integration/ReferenceableIdentifiersImportIT.java
@@ -533,12 +533,6 @@
assertIdConflictPolicyBehaviour(IdConflictPolicy.LEGACY, TARGET_STATE.NO_CONFLICT_TARGET_UNCHANGED, ID_KEPT, NA);
}
- @Test
- @Ignore("JCRVLT-828")
- public void testInstallPackageConflictTargetPresent_LEGACY() throws Exception {
- assertIdConflictPolicyBehaviour(IdConflictPolicy.LEGACY, TARGET_STATE.CONFLICT_TARGET_PRESENT, ID_KEPT, NA);
- }
-
// postcondition: exception
private void assertIdConflictPolicyBehaviour(IdConflictPolicy policy, TARGET_STATE dstState, Class<?> expectedException,
Class<?> expectedRootCause) throws Exception {
@@ -697,4 +691,38 @@
options.setIdConflictPolicy(policy);
extractVaultPackage("/test-packages/referenceable-dup.zip", options);
}
-}
\ No newline at end of file
+
+ @Test
+ public void testUuidPreservedWithLegacyPolicy() throws Exception {
+ // 1. Install package with UUID from package
+ extractVaultPackageStrict("/test-packages/referenceable.zip");
+ Node node = admin.getNode("/tmp/referenceable");
+ String originalUuid = node.getIdentifier();
+ assertEquals(UUID_REFERENCEABLE, originalUuid);
+
+ // 2. Change the UUID manually (simulating an existing node with different UUID)
+ // This simulates the scenario where node exists with UUID1, package has UUID2
+ node.remove();
+ admin.save();
+ node = JcrUtils.getOrCreateByPath("/tmp/referenceable", null, JcrConstants.NT_UNSTRUCTURED, admin, true);
+ node.addMixin(JcrConstants.MIX_VERSIONABLE);
+ node.setProperty(PROPERTY_NAME, PROPERTY_VALUE); // Keep same property
+ admin.save();
+ String currentUuid = node.getIdentifier();
+ assertNotEquals("Node should have different UUID than package", UUID_REFERENCEABLE, currentUuid);
+
+ // 3. Re-import with LEGACY policy and UPDATE_PROPERTIES mode
+ // Expected: UUID should be preserved (not replaced with package UUID)
+ // Bug: UUID gets replaced unconditionally at line 996
+ ImportOptions opts = getDefaultOptions();
+ opts.setIdConflictPolicy(IdConflictPolicy.LEGACY);
+ opts.setImportMode(ImportMode.REPLACE);
+ extractVaultPackage("/test-packages/referenceable.zip", opts);
+
+ // 4. Verify UUID is preserved (not replaced)
+ node = admin.getNode("/tmp/referenceable");
+ String finalUuid = node.getIdentifier();
+ assertEquals("UUID should be preserved with LEGACY policy", currentUuid, finalUuid);
+ assertNotEquals("UUID should NOT be replaced with package UUID", UUID_REFERENCEABLE, finalUuid);
+ }
+}
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java
index bdab1ff..7555d7e 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java
@@ -906,16 +906,20 @@
} else {
log.warn("To-be imported node and existing conflicting node have different parents. Will create new identifier for the former. ({})",
newNodePath);
- preprocessedProperties.removeIf(p -> p.getName().equals(NameConstants.JCR_UUID)
- || p.getName().equals(NameConstants.JCR_BASEVERSION)
+ preprocessedProperties.removeIf(p -> p.getName().equals(NameConstants.JCR_UUID)
+ || p.getName().equals(NameConstants.JCR_BASEVERSION)
|| p.getName().equals(NameConstants.JCR_PREDECESSORS)
|| p.getName().equals(NameConstants.JCR_SUCCESSORS)
|| p.getName().equals(NameConstants.JCR_VERSIONHISTORY));
}
}
}
- } catch (ItemNotFoundException e) {
- // ignore
+ } catch (ItemNotFoundException expected) {
+ // LEGACY mode: no node with same ID present, but target node exists: ignore the ID from the package being imported
+ if (existingNode != null && idConflictPolicy == IdConflictPolicy.LEGACY && existingNode.isNodeType(JcrConstants.MIX_REFERENCEABLE)) {
+ log.debug("IdConflictPolicy.LEGACY - ignoring Identifier {} from imported package at {} but keep existing identifier {}", identifier.get(), docViewNode.getName(), existingNode.getIdentifier());
+ preprocessedProperties.removeIf(p -> p.getName().equals(NameConstants.JCR_UUID));
+ }
}
}
}
@@ -1039,6 +1043,9 @@
newMixins.add(mixin);
}
}
+
+ boolean wasReferenceable = node.isNodeType(JcrConstants.MIX_REFERENCEABLE);
+
// remove mixins not in package (only for mode = replace)
if (importMode == ImportMode.REPLACE) {
for (NodeType mix : node.getMixinNodeTypes()) {
@@ -1049,12 +1056,20 @@
|| acHandling == AccessControlHandling.CLEAR
|| acHandling == AccessControlHandling.OVERWRITE) {
vs.ensureCheckedOut();
- node.removeMixin(name);
+
+ // can't remove a mixin which would remove the UUID in LEGACY Mode
+ if (idConflictPolicy == idConflictPolicy.LEGACY && wasReferenceable && mix.isNodeType(JcrConstants.MIX_REFERENCEABLE)) {
+ log.debug("idConflictPolicy.LEGACY: not removing mixin " + mix.getName() + ", so that UUID can be preserved.");
+ } else {
+ node.removeMixin(name);
+ }
+
updatedNode = node;
}
}
}
}
+
// add remaining mixins (for all import modes)
for (String mixin : newMixins) {
vs.ensureCheckedOut();
@@ -1080,7 +1095,7 @@
}
EffectiveNodeType effectiveNodeType = EffectiveNodeType.ofNode(node);
Collection<DocViewProperty2> unprotectedProperties = removeProtectedProperties(ni.getProperties(), effectiveNodeType, node.getPath(), PROTECTED_PROPERTIES_CONSIDERED_FOR_UPDATED_NODES);
-
+
// add/modify properties contained in package
if (setProperties(node, ni,unprotectedProperties, importMode == ImportMode.REPLACE|| importMode == ImportMode.UPDATE || importMode == ImportMode.UPDATE_PROPERTIES, vs)) {
updatedNode = node;
