Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/reference/ReferenceEditor.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/reference/ReferenceEditor.java (revision 1692365) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/reference/ReferenceEditor.java (working copy) @@ -17,12 +17,11 @@ package org.apache.jackrabbit.oak.plugins.index.reference; import static com.google.common.collect.ImmutableSet.of; -import static com.google.common.collect.Iterables.addAll; import static com.google.common.collect.Maps.newHashMap; import static com.google.common.collect.Sets.newHashSet; +import static java.util.Collections.emptySet; import static javax.jcr.PropertyType.REFERENCE; import static javax.jcr.PropertyType.WEAKREFERENCE; -import static org.apache.jackrabbit.JcrConstants.JCR_SYSTEM; import static org.apache.jackrabbit.JcrConstants.JCR_UUID; import static org.apache.jackrabbit.oak.api.CommitFailedException.INTEGRITY; import static org.apache.jackrabbit.oak.api.Type.STRING; @@ -32,12 +31,14 @@ import static org.apache.jackrabbit.oak.plugins.index.reference.NodeReferenceConstants.REF_NAME; import static org.apache.jackrabbit.oak.plugins.index.reference.NodeReferenceConstants.WEAK_REF_NAME; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE; -import static org.apache.jackrabbit.oak.plugins.version.VersionConstants.SYSTEM_PATHS; +import static org.apache.jackrabbit.oak.plugins.version.VersionConstants.VERSION_STORE_PATH; import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import com.google.common.collect.Sets; + import org.apache.jackrabbit.oak.api.CommitFailedException; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.plugins.index.IndexEditor; @@ -69,12 +70,6 @@ private final NodeBuilder definition; /** - * the uuid of the current node, null if the node doesn't have this - * property. - */ - private final String uuid; - - /** * > */ private final Map> newRefs; @@ -101,18 +96,8 @@ private final Set rmIds; /** - * set of ids that changed. This can happen when a node with the same name - * is deleted and added again - * - */ - private final Set discardedIds; - - private final Set versionStoreIds; - - /** * set of ids that were added during this commit. we need it to reconcile * moves - * */ private final Set newIds; @@ -128,31 +113,25 @@ this.path = "/"; this.definition = definition; this.root = root; - this.uuid = null; this.newRefs = newHashMap(); this.rmRefs = newHashMap(); this.newWeakRefs = newHashMap(); this.rmWeakRefs = newHashMap(); this.rmIds = newHashSet(); - this.discardedIds = newHashSet(); - this.versionStoreIds = newHashSet(); this.newIds = newHashSet(); } - private ReferenceEditor(ReferenceEditor parent, String name, String uuid) { + private ReferenceEditor(ReferenceEditor parent, String name) { this.parent = parent; this.name = name; this.path = null; this.definition = parent.definition; this.root = parent.root; - this.uuid = uuid; this.newRefs = parent.newRefs; this.rmRefs = parent.rmRefs; this.newWeakRefs = parent.newWeakRefs; this.rmWeakRefs = parent.rmWeakRefs; this.rmIds = parent.rmIds; - this.discardedIds = parent.discardedIds; - this.versionStoreIds = parent.versionStoreIds; this.newIds = parent.newIds; this.isReindex = parent.isReindex; } @@ -179,27 +158,11 @@ public void leave(NodeState before, NodeState after) throws CommitFailedException { if (parent == null) { - Set offending = newHashSet(rmIds); - offending.removeAll(rmRefs.keySet()); - offending.removeAll(newIds); - if (!offending.isEmpty()) { - throw new CommitFailedException(INTEGRITY, 1, - "Unable to delete referenced node"); - } - rmIds.addAll(discardedIds); - - // remove ids that are actually deleted (that exist in the rmRefs.keySet()) - versionStoreIds.removeAll(rmRefs.keySet()); - rmIds.addAll(versionStoreIds); - // update references for (Entry> ref : rmRefs.entrySet()) { String uuid = ref.getKey(); - if (rmIds.contains(uuid)) { - continue; - } Set rm = ref.getValue(); - Set add = newHashSet(); + Set add = emptySet(); if (newRefs.containsKey(uuid)) { add = newRefs.remove(uuid); } @@ -207,22 +170,19 @@ } for (Entry> ref : newRefs.entrySet()) { String uuid = ref.getKey(); - if (rmIds.contains(uuid)) { - continue; - } Set add = ref.getValue(); - Set rm = newHashSet(); + Set rm = emptySet(); update(definition, REF_NAME, uuid, add, rm); } + checkReferentialIntegrity(root, definition.getNodeState(), + Sets.difference(rmIds, newIds)); + // update weak references for (Entry> ref : rmWeakRefs.entrySet()) { String uuid = ref.getKey(); - if (rmIds.contains(uuid)) { - continue; - } Set rm = ref.getValue(); - Set add = newHashSet(); + Set add = emptySet(); if (newWeakRefs.containsKey(uuid)) { add = newWeakRefs.remove(uuid); } @@ -230,11 +190,8 @@ } for (Entry> ref : newWeakRefs.entrySet()) { String uuid = ref.getKey(); - if (rmIds.contains(uuid)) { - continue; - } Set add = ref.getValue(); - Set rm = newHashSet(); + Set rm = emptySet(); update(definition, WEAK_REF_NAME, uuid, add, rm); } } @@ -250,9 +207,7 @@ if (before != null) { if (before.getType().tag() == REFERENCE) { - if (isVersionStorePath(getPath())) { - addAll(versionStoreIds, before.getValue(STRINGS)); - } else { + if (!isVersionStorePath(getPath())) { put(rmRefs, before.getValue(STRINGS), concat(getPath(), before.getName())); } @@ -263,17 +218,12 @@ } if (JCR_UUID.equals(before.getName())) { // node remove + add -> changed uuid - String beforeUuid = before.getValue(STRING); - if (beforeUuid != null && !beforeUuid.equals(uuid)) { - discardedIds.add(beforeUuid); - } + rmIds.add(before.getValue(STRING)); } } if (after != null) { if (after.getType().tag() == REFERENCE) { - if (isVersionStorePath(getPath())) { - addAll(versionStoreIds, after.getValue(STRINGS)); - } else { + if (!isVersionStorePath(getPath())) { put(newRefs, after.getValue(STRINGS), concat(getPath(), after.getName())); } @@ -282,6 +232,10 @@ put(newWeakRefs, after.getValue(STRINGS), concat(getPath(), after.getName())); } + if (JCR_UUID.equals(after.getName())) { + // node remove + add -> changed uuid + newIds.add(after.getValue(STRING)); + } } } @@ -296,13 +250,13 @@ if (!isReindex && uuid != null) { newIds.add(uuid); } - return new ReferenceEditor(this, name, uuid); + return new ReferenceEditor(this, name); } @Override public Editor childNodeChanged(String name, NodeState before, NodeState after) { - return new ReferenceEditor(this, name, after.getString(JCR_UUID)); + return new ReferenceEditor(this, name); } @Override @@ -309,26 +263,17 @@ public Editor childNodeDeleted(String name, NodeState before) throws CommitFailedException { String uuid = before.getString(JCR_UUID); - if (uuid != null && check(root, definition.getNodeState(), REF_NAME, uuid)) { + if (uuid != null) { rmIds.add(uuid); } - return new ReferenceEditor(this, name, uuid); + return new ReferenceEditor(this, name); } // ---------- Utils ----------------------------------------- private static boolean isVersionStorePath(String oakPath) { - if (oakPath == null) { - return false; - } - if (oakPath.indexOf(JCR_SYSTEM) == 1) { - for (String p : SYSTEM_PATHS) { - if (oakPath.startsWith(p)) { - return true; - } - } - } - return false; + return oakPath != null + && oakPath.startsWith(VERSION_STORE_PATH); } private static void put(Map> map, @@ -357,9 +302,23 @@ } } - private static boolean check(NodeState root, NodeState definition, String name, String key) { + private static boolean hasReferences(NodeState root, + NodeState definition, + String name, + String key) { return definition.hasChildNode(name) && STORE.count(root, definition, name, of(key), 1) > 0; } + private static void checkReferentialIntegrity(NodeState root, + NodeState definition, + Set idsOfRemovedNodes) + throws CommitFailedException { + for (String id : idsOfRemovedNodes) { + if (hasReferences(root, definition, REF_NAME, id)) { + throw new CommitFailedException(INTEGRITY, 1, + "Unable to delete referenced node"); + } + } + } } Index: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ReferencesTest.java =================================================================== --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ReferencesTest.java (revision 1692365) +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ReferencesTest.java (working copy) @@ -484,10 +484,6 @@ } public void testRemoveReferenced2() throws RepositoryException { - if (true) { - // FIXME OAK-3130 - return; - } Node ref = testRootNode.addNode(nodeName1, testNodeType); ref.addMixin(mixReferenceable); superuser.save(); @@ -511,10 +507,6 @@ } public void testRemoveReferenced3() throws RepositoryException { - if (true) { - // FIXME OAK-3130 - return; - } Node ref = testRootNode.addNode(nodeName1, testNodeType); ref.addMixin(mixReferenceable); superuser.save(); @@ -537,6 +529,51 @@ } } + public void testRecreateWithDifferentUUID() throws RepositoryException { + Node ref = testRootNode.addNode(nodeName1, testNodeType); + ref.addMixin(mixReferenceable); + superuser.save(); + String uuid = ref.getIdentifier(); + + Node n1 = testRootNode.addNode(nodeName2, testNodeType); + n1.setProperty("ref", ref); + assertEquals(PropertyType.REFERENCE, n1.getProperty("ref").getType()); + superuser.save(); + + // recreate + ref.remove(); + ref = testRootNode.addNode(nodeName1, testNodeType); + ref.addMixin(mixReferenceable); + assertFalse(uuid.equals(ref.getIdentifier())); + try { + superuser.save(); + fail("must fail with ReferentialIntegrityException"); + } catch (ReferentialIntegrityException e) { + // expected + } + } + + public void testRecreateNonReferenceable() throws RepositoryException { + Node ref = testRootNode.addNode(nodeName1, testNodeType); + ref.addMixin(mixReferenceable); + superuser.save(); + + Node n1 = testRootNode.addNode(nodeName2, testNodeType); + n1.setProperty("ref", ref); + assertEquals(PropertyType.REFERENCE, n1.getProperty("ref").getType()); + superuser.save(); + + // recreate + ref.remove(); + testRootNode.addNode(nodeName1, testNodeType); + try { + superuser.save(); + fail("must fail with ReferentialIntegrityException"); + } catch (ReferentialIntegrityException e) { + // expected + } + } + private static void checkReferences(String msg, PropertyIterator refs, String ... expected) throws RepositoryException { List paths = new LinkedList(); while (refs.hasNext()) { Index: oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionTest.java =================================================================== --- oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionTest.java (revision 1692365) +++ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/version/VersionTest.java (working copy) @@ -16,9 +16,12 @@ */ package org.apache.jackrabbit.oak.jcr.version; +import java.util.Set; + import javax.jcr.Node; import javax.jcr.NodeIterator; import javax.jcr.Property; +import javax.jcr.PropertyIterator; import javax.jcr.ReferentialIntegrityException; import javax.jcr.RepositoryException; import javax.jcr.query.Query; @@ -27,9 +30,15 @@ import javax.jcr.version.Version; import javax.jcr.version.VersionManager; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; + import org.apache.jackrabbit.test.AbstractJCRTest; import org.apache.jackrabbit.test.NotExecutableException; +import static java.util.Collections.emptySet; +import static org.apache.jackrabbit.oak.commons.PathUtils.concat; + /** * VersionTest performs tests on JCR Version nodes. */ @@ -111,8 +120,6 @@ VersionManager vMgr = superuser.getWorkspace().getVersionManager(); vMgr.checkin(n.getPath()); - // comment out the next line and the test will fail - vMgr.checkout(n.getPath()); Version v = vMgr.getBaseVersion(n.getPath()); try { @@ -121,5 +128,53 @@ } catch (ReferentialIntegrityException e) { // expected } + + vMgr.checkout(n.getPath()); + v = vMgr.getBaseVersion(n.getPath()); + try { + v.getContainingHistory().removeVersion(v.getName()); + fail("removeVersion() must fail with ReferentialIntegrityException"); + } catch (ReferentialIntegrityException e) { + // expected + } } + + // OAK-3130 + public void testVersionReferences() throws RepositoryException { + Node n = testRootNode.addNode(nodeName1, testNodeType); + n.addMixin(mixVersionable); + superuser.save(); + + VersionManager vMgr = superuser.getWorkspace().getVersionManager(); + Version v = vMgr.checkin(n.getPath()); + Version rootVersion = v.getContainingHistory().getRootVersion(); + System.out.println(rootVersion); + System.out.println(v); + + Set refs = getReferencingPaths(rootVersion); + // the rootVersion actually has referencing property + // from the version 'v' created by checkin() (jcr:predecessors + // points to the rootVersion), but for compatibility with + // Jackrabbit 2.x it is not returned by Node.getReferences() + assertEquals("references mismatch", emptySet(), refs); + + refs = getReferencingPaths(v); + // Similar to above, the version is actually also referenced + // from the rootVersion's jcr:successors property, but for + // compatibility reasons it is not returned + Set expected = ImmutableSet.of( + concat(n.getPath(), jcrBaseVersion) + ); + assertEquals("references mismatch", expected, refs); + } + + private static Set getReferencingPaths(Node n) + throws RepositoryException { + Set refs = Sets.newHashSet(); + PropertyIterator it = n.getReferences(); + while (it.hasNext()) { + refs.add(it.nextProperty().getPath()); + } + return refs; + } }