Author: angela
Date: Wed Apr 1 10:36:00 2015
New Revision: 1670603
URL: http://svn.apache.org/r1670603
Log:
OAK-2246 : UUID collision check is not does not work in transient space
Modified:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/xml/ImportTest.java
Modified:
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java?rev=1670603&r1=1670602&r2=1670603&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java
Wed Apr 1 10:36:00 2015
@@ -47,6 +47,7 @@ import com.google.common.base.Predicates
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.oak.api.ContentSession;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Root;
import org.apache.jackrabbit.oak.api.Tree;
@@ -82,26 +83,10 @@ public class ImporterImpl implements Imp
private final String userID;
private final AccessManager accessManager;
- /**
- * There are two IdentifierManagers used.
- *
- * 1) currentStateIdManager - Associated with current root on which all
import
- * operations are being performed
- *
- * 2) baseStateIdManager - Associated with the initial root on which
- * no modifications are performed
- */
- private final IdentifierManager currentStateIdManager;
- private final IdentifierManager baseStateIdManager;
-
private final EffectiveNodeTypeProvider effectiveNodeTypeProvider;
private final DefinitionProvider definitionProvider;
- /**
- * Set of newly created uuid from nodes which are
- * created in this import
- */
- private final Set<String> uuids = new HashSet<String>();
+ private final IdResolver idLookup;
private final Stack<Tree> parents;
@@ -171,8 +156,7 @@ public class ImporterImpl implements Imp
accessManager = sessionContext.getAccessManager();
- currentStateIdManager = new IdentifierManager(root);
- baseStateIdManager = new
IdentifierManager(sd.getContentSession().getLatestRoot());
+ idLookup = new IdResolver(root, sd.getContentSession());
refTracker = new ReferenceChangeTracker();
@@ -465,24 +449,7 @@ public class ImporterImpl implements Imp
}
} else {
- //1. First check from base state that tree corresponding to
- //this id exist
- Tree conflicting = baseStateIdManager.getTree(id);
-
- if (conflicting == null) {
- //1.a. Check if id is found in newly created nodes
- if (uuids.contains(id)) {
- conflicting = currentStateIdManager.getTree(id);
- }
- } else {
- //1.b Re obtain the conflicting tree from Id Manager
- //associated with current root. Such that any operation
- //on it gets reflected in later operations
- //In case a tree with same id was removed earlier then it
- //would return null
- conflicting = currentStateIdManager.getTree(id);
- }
-
+ Tree conflicting = idLookup.getConflictingTree(id);
if (conflicting != null && conflicting.exists()) {
// resolve uuid conflict
tree = resolveUUIDConflict(parent, conflicting, id,
nodeInfo);
@@ -522,22 +489,7 @@ public class ImporterImpl implements Imp
}
}
- collectUUIDs(parent);
- }
-
- private void collectUUIDs(Tree tree) {
- if (tree == null) {
- return;
- }
-
- String uuid = TreeUtil.getString(tree, JcrConstants.JCR_UUID);
- if (uuid != null) {
- uuids.add(uuid);
- }
-
- for (Tree child : tree.getChildren()) {
- collectUUIDs(child);
- }
+ idLookup.rememberImportedUUIDs(parent);
}
@Override
@@ -621,4 +573,78 @@ public class ImporterImpl implements Imp
tree.setProperty(prop);
}
}
+
+ /**
+ * Resolves 'uuid' property values to {@code Tree} objects and optionally
+ * keeps track of newly imported UUIDs.
+ */
+ private static final class IdResolver {
+ /**
+ * There are two IdentifierManagers used.
+ *
+ * 1) currentStateIdManager - Associated with current root on which
all import
+ * operations are being performed
+ *
+ * 2) baseStateIdManager - Associated with the initial root on which
+ * no modifications are performed
+ */
+ private final IdentifierManager currentStateIdManager;
+ private final IdentifierManager baseStateIdManager;
+
+ /**
+ * Set of newly created uuid from nodes which are
+ * created in this import, which are only remembered if the editing
+ * session doesn't have any pending transient changes preventing this
+ * performance optimisation from working properly (see OAK-2246).
+ */
+ private final Set<String> importedUUIDs;
+
+ private IdResolver(@Nonnull Root root, @Nonnull ContentSession
contentSession) {
+ currentStateIdManager = new IdentifierManager(root);
+ baseStateIdManager = new
IdentifierManager(contentSession.getLatestRoot());
+
+ if (!root.hasPendingChanges()) {
+ importedUUIDs = new HashSet<String>();
+ } else {
+ importedUUIDs = null;
+ }
+ }
+
+
+ @CheckForNull
+ private Tree getConflictingTree(@Nonnull String id) {
+ //1. First check from base state that tree corresponding to
+ //this id exist
+ Tree conflicting = baseStateIdManager.getTree(id);
+ if (conflicting == null && importedUUIDs != null) {
+ //1.a. Check if id is found in newly created nodes
+ if (importedUUIDs.contains(id)) {
+ conflicting = currentStateIdManager.getTree(id);
+ }
+ } else {
+ //1.b Re obtain the conflicting tree from Id Manager
+ //associated with current root. Such that any operation
+ //on it gets reflected in later operations
+ //In case a tree with same id was removed earlier then it
+ //would return null
+ conflicting = currentStateIdManager.getTree(id);
+ }
+ return conflicting;
+ }
+
+ private void rememberImportedUUIDs(@CheckForNull Tree tree) {
+ if (tree == null || importedUUIDs == null) {
+ return;
+ }
+
+ String uuid = TreeUtil.getString(tree, JcrConstants.JCR_UUID);
+ if (uuid != null) {
+ importedUUIDs.add(uuid);
+ }
+
+ for (Tree child : tree.getChildren()) {
+ rememberImportedUUIDs(child);
+ }
+ }
+ }
}
Modified:
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/xml/ImportTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/xml/ImportTest.java?rev=1670603&r1=1670602&r2=1670603&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/xml/ImportTest.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/xml/ImportTest.java
Wed Apr 1 10:36:00 2015
@@ -18,45 +18,212 @@ package org.apache.jackrabbit.oak.jcr.xm
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
import java.io.OutputStream;
import javax.jcr.ImportUUIDBehavior;
+import javax.jcr.ItemExistsException;
import javax.jcr.Node;
+import javax.jcr.RepositoryException;
+import javax.jcr.nodetype.ConstraintViolationException;
import org.apache.jackrabbit.test.AbstractJCRTest;
public class ImportTest extends AbstractJCRTest {
- public void testReplaceUUID() throws Exception {
+ private String uuid;
+ private String path;
+ private String siblingPath;
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+
Node node = testRootNode.addNode(nodeName1);
node.addMixin(mixReferenceable);
- superuser.save();
+ Node sibling = testRootNode.addNode(nodeName2);
+
+ uuid = node.getIdentifier();
+ path = node.getPath();
+ siblingPath = sibling.getPath();
+ }
+
+ private InputStream getImportStream() throws RepositoryException,
IOException {
OutputStream out = new ByteArrayOutputStream();
- superuser.exportSystemView(node.getPath(), out, true, false);
+ superuser.exportSystemView(path, out, true, false);
+ return new ByteArrayInputStream(out.toString().getBytes());
+ }
+
+ public void testReplaceUUID() throws Exception {
+ superuser.save();
+
+ superuser.importXML(siblingPath, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_REPLACE_EXISTING);
+ superuser.save();
+
+ // original node must have been replaced (but no child node added)
+ assertTrue(testRootNode.hasNode(nodeName1));
+ Node n2 = testRootNode.getNode(nodeName1);
+ assertTrue(n2.isNodeType(mixReferenceable));
+ assertEquals(uuid, n2.getIdentifier());
+
+ Node sibling = superuser.getNode(siblingPath);
+ assertFalse(sibling.hasNode(nodeName1));
+ }
+
+ /**
+ * @see <a
href="https://issues.apache.org/jira/browse/OAK-2246";>OAK-2246</a>
+ */
+ public void testTransientReplaceUUID() throws Exception {
+ superuser.importXML(path, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_REPLACE_EXISTING);
+ superuser.save();
- superuser.importXML(node.getPath(), new
ByteArrayInputStream(out.toString().getBytes()),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_REPLACE_EXISTING);
+ // original node must have been replaced (but no child node added)
+ superuser.importXML(siblingPath, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_REPLACE_EXISTING);
superuser.save();
+ // original node must have been replaced (but no child node added)
assertTrue(testRootNode.hasNode(nodeName1));
Node n2 = testRootNode.getNode(nodeName1);
assertTrue(n2.isNodeType(mixReferenceable));
+ assertEquals(uuid, n2.getIdentifier());
+
+ Node sibling = superuser.getNode(siblingPath);
+ assertFalse(sibling.hasNode(nodeName1));
+ }
+
+ public void testReplaceUUIDSameTree() throws Exception {
+ superuser.save();
+
+ superuser.importXML(path, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_REPLACE_EXISTING);
+ superuser.save();
+
+ // original node must have been replaced (but no child node added)
+ assertTrue(testRootNode.hasNode(nodeName1));
+ Node n2 = testRootNode.getNode(nodeName1);
+ assertTrue(n2.isNodeType(mixReferenceable));
+ assertEquals(uuid, n2.getIdentifier());
assertFalse(n2.hasNode(nodeName1));
}
-// TODO : FIXME (OAK-2246)
-// public void testTransientReplaceUUID() throws Exception {
-// Node node = testRootNode.addNode(nodeName1);
-// node.addMixin(mixReferenceable);
-//
-// OutputStream out = new ByteArrayOutputStream();
-// superuser.exportSystemView(node.getPath(), out, true, false);
-//
-// superuser.importXML(node.getPath(), new
ByteArrayInputStream(out.toString().getBytes()),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_REPLACE_EXISTING);
-// superuser.save();
-//
-// assertTrue(testRootNode.hasNode(nodeName1));
-// Node n2 = testRootNode.getNode(nodeName1);
-// assertTrue(n2.isNodeType(mixReferenceable));
-// assertFalse(n2.hasNode(nodeName1));
-// }
+ /**
+ * @see <a
href="https://issues.apache.org/jira/browse/OAK-2246";>OAK-2246</a>
+ */
+ public void testTransientReplaceUUIDSameTree() throws Exception {
+ superuser.importXML(path, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_REPLACE_EXISTING);
+ superuser.save();
+
+ // original node must have been replaced (but no child node added)
+ assertTrue(testRootNode.hasNode(nodeName1));
+ Node n2 = testRootNode.getNode(nodeName1);
+ assertTrue(n2.isNodeType(mixReferenceable));
+ assertEquals(uuid, n2.getIdentifier());
+ assertFalse(n2.hasNode(nodeName1));
+ }
+
+ public void testRemoveUUID() throws Exception {
+ superuser.save();
+
+ superuser.importXML(siblingPath, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING);
+ superuser.save();
+
+ // original node must have been removed
+ assertFalse(testRootNode.hasNode(nodeName1));
+
+ Node sibling = superuser.getNode(siblingPath);
+ assertTrue(sibling.hasNode(nodeName1));
+
+ Node imported = sibling.getNode(nodeName1);
+ assertTrue(imported.isNodeType(mixReferenceable));
+ assertEquals(uuid, imported.getIdentifier());
+ }
+
+ public void testTransientRemoveUUID() throws Exception {
+ superuser.importXML(siblingPath, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING);
+ superuser.save();
+
+ // original node must have been removed
+ assertFalse(testRootNode.hasNode(nodeName1));
+
+ Node sibling = superuser.getNode(siblingPath);
+ assertTrue(sibling.hasNode(nodeName1));
+
+ Node imported = sibling.getNode(nodeName1);
+ assertTrue(imported.isNodeType(mixReferenceable));
+ assertEquals(uuid, imported.getIdentifier());
+ }
+
+ public void testRemoveUUIDSameTree() throws Exception {
+ superuser.save();
+
+ try {
+ superuser.importXML(path, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING);
+ fail("ConstraintViolationException expected");
+ } catch (ConstraintViolationException e) {
+ // success
+ }
+ }
+
+ public void testTransientRemoveUUIDSameTree() throws Exception {
+ try {
+ superuser.importXML(path, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING);
+ fail("ConstraintViolationException expected");
+ } catch (ConstraintViolationException e) {
+ // success
+ }
+ }
+
+ public void testCreateNewUUID() throws Exception {
+ superuser.save();
+
+ superuser.importXML(siblingPath, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_CREATE_NEW);
+ superuser.save();
+
+ // original node must still exist
+ assertTrue(testRootNode.hasNode(nodeName1));
+
+ // verify the import produced the expected new node
+ Node sibling = superuser.getNode(siblingPath);
+ assertTrue(sibling.hasNode(nodeName1));
+
+ Node imported = sibling.getNode(nodeName1);
+ assertTrue(imported.isNodeType(mixReferenceable));
+ assertFalse(uuid.equals(imported.getIdentifier()));
+ }
+
+ public void testTransientCreateNewUUID() throws Exception {
+ superuser.importXML(siblingPath, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_CREATE_NEW);
+ superuser.save();
+
+ // original node must still exist
+ assertTrue(testRootNode.hasNode(nodeName1));
+
+ // verify the import produced the expected new node
+ Node sibling = superuser.getNode(siblingPath);
+ assertTrue(sibling.hasNode(nodeName1));
+
+ Node imported = sibling.getNode(nodeName1);
+ assertTrue(imported.isNodeType(mixReferenceable));
+ assertFalse(uuid.equals(imported.getIdentifier()));
+ }
+
+ public void testThrow() throws Exception {
+ superuser.save();
+
+ try {
+ superuser.importXML(siblingPath, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW);
+ fail("ItemExistsException expected");
+ } catch (ItemExistsException e) {
+ // success
+ }
+ }
+
+ public void testTransientThrow() throws Exception {
+ try {
+ superuser.importXML(siblingPath, getImportStream(),
ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW);
+ fail("ItemExistsException expected");
+ } catch (ItemExistsException e) {
+ // success
+ }
+ }
}
\ No newline at end of file
