You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by tr...@apache.org on 2013/11/16 01:27:56 UTC

svn commit: r1542434 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/identifier/IdentifierManager.java oak-jcr/pom.xml oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ReferencesTest.java

Author: tripod
Date: Sat Nov 16 00:27:56 2013
New Revision: 1542434

URL: http://svn.apache.org/r1542434
Log:
OAK-1196 Node.getReferences() should not show references in version storage

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/identifier/IdentifierManager.java
    jackrabbit/oak/trunk/oak-jcr/pom.xml
    jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ReferencesTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/identifier/IdentifierManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/identifier/IdentifierManager.java?rev=1542434&r1=1542433&r2=1542434&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/identifier/IdentifierManager.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/identifier/IdentifierManager.java Sat Nov 16 00:27:56 2013
@@ -46,12 +46,9 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Charsets;
-import com.google.common.base.Predicate;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static org.apache.jackrabbit.oak.api.Type.STRING;
 
 /**
  * TODO document
@@ -98,7 +95,7 @@ public class IdentifierManager {
     public static String getIdentifier(Tree tree) {
         PropertyState property = tree.getProperty(JcrConstants.JCR_UUID);
         if (property != null) {
-            return property.getValue(STRING);
+            return property.getValue(Type.STRING);
         } else if (tree.isRoot()) {
             return "/";
         } else {
@@ -201,26 +198,7 @@ public class IdentifierManager {
                     "SELECT * FROM [nt:base] WHERE PROPERTY([" + pName + "], '" + reference + "') = $uuid",
                     Query.JCR_SQL2, Long.MAX_VALUE, 0, bindings, new NamePathMapper.Default());
 
-            Iterable<String> paths = new ReferencePropertyIterable(result, uuid);
-
-            // todo: integrate into ReferencePropertyIterable
-            if (nodeTypeNames.length > 0) {
-                paths = Iterables.filter(paths, new Predicate<String>() {
-                    @Override
-                    public boolean apply(String path) {
-                        Tree tree = root.getTree(PathUtils.getParentPath(path));
-                        if (tree.exists()) {
-                            for (String ntName : nodeTypeNames) {
-                                if (nodeTypeManager.isNodeType(tree, ntName)) {
-                                    return true;
-                                }
-                            }
-                        }
-                        return false;
-                    }
-                });
-            }
-
+            Iterable<String> paths = new ReferencePropertyIterable(result, uuid, propertyName, nodeTypeNames);
             return Sets.newHashSet(paths);
         } catch (ParseException e) {
             log.error("query failed", e);
@@ -228,15 +206,25 @@ public class IdentifierManager {
         }
     }
 
+    /**
+     * Implements an iterable that is used to collect the paths of the properties from a query result
+     * that contain a reference to the given uuid.
+     */
     private class ReferencePropertyIterable implements Iterable<String> {
 
         private final Result result;
 
         private final String uuid;
 
-        private ReferencePropertyIterable(Result result, String uuid) {
+        private final String propertyName;
+
+        private final String[] nodeTypeNames;
+
+        private ReferencePropertyIterable(Result result, String uuid, String propertyName, String[] nodeTypeNames) {
             this.result = result;
             this.uuid = uuid;
+            this.propertyName = propertyName;
+            this.nodeTypeNames = nodeTypeNames;
         }
 
         @Override
@@ -292,7 +280,7 @@ public class IdentifierManager {
                                         break;
                                     }
                                 }
-                            } else if (uuid.equals(pState.getValue(STRING))) {
+                            } else if (uuid.equals(pState.getValue(Type.STRING))) {
                                 next = PathUtils.concat(rowPath, pState.getName());
                             }
 
@@ -301,12 +289,34 @@ public class IdentifierManager {
                                 break;
                             }
                             rowPath = rows.next().getPath();
-                            iter = root.getTree(rowPath).getProperties().iterator();
+                            // skip references from the version storage (OAK-1196)
+                            if (!rowPath.startsWith("/jcr:system/jcr:versionStorage/")) {
+                                // filter by node type if needed
+                                Tree tree = root.getTree(rowPath);
+                                if (nodeTypeNames.length == 0 || containsNodeType(tree, nodeTypeNames)) {
+                                    // for a fixed property name, we don't need to look for it, but just assume that
+                                    // the search found the correct one
+                                    if (propertyName != null) {
+                                        next = PathUtils.concat(rowPath, propertyName);
+                                    } else {
+                                        iter = root.getTree(rowPath).getProperties().iterator();
+                                    }
+                                }
+                            }
                         }
                     }
                 }
             };
         }
+
+        private boolean containsNodeType(Tree tree, String[] nodeTypeNames) {
+            for (String ntName : nodeTypeNames) {
+                if (nodeTypeManager.isNodeType(tree, ntName)) {
+                    return true;
+                }
+            }
+            return false;
+        }
     }
 
     @CheckForNull

Modified: jackrabbit/oak/trunk/oak-jcr/pom.xml
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/pom.xml?rev=1542434&r1=1542433&r2=1542434&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/pom.xml (original)
+++ jackrabbit/oak/trunk/oak-jcr/pom.xml Sat Nov 16 00:27:56 2013
@@ -52,8 +52,6 @@
 
       org.apache.jackrabbit.oak.jcr.ReferencesTest#testMovedReferences <!-- OAK-1195 -->
       org.apache.jackrabbit.oak.jcr.ReferencesTest#testMovedVersionedReferences <!-- OAK-1195 -->
-      org.apache.jackrabbit.oak.jcr.ReferencesTest#testVersionReferencesV1 <!-- OAK-1196 -->
-      org.apache.jackrabbit.oak.jcr.ReferencesTest#testVersionedReferences <!-- OAK-1196 -->
 
       <!-- Locking : not fully implemented -->
       org.apache.jackrabbit.test.api.lock.LockTest#testNodeLocked

Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ReferencesTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ReferencesTest.java?rev=1542434&r1=1542433&r2=1542434&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ReferencesTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/ReferencesTest.java Sat Nov 16 00:27:56 2013
@@ -48,7 +48,7 @@ public class ReferencesTest extends Abst
         superuser.save();
 
         assertEquals("ref", ref.getPath(), n.getProperty("myref").getNode().getPath());
-        checkReferences("refs", ref, n.getPath() + "/myref");
+        checkReferences("refs", ref.getReferences(), n.getPath() + "/myref");
     }
 
     // OAK-1194 Missing properties in Node.getReferences()
@@ -65,7 +65,24 @@ public class ReferencesTest extends Abst
         assertEquals("ref", ref.getPath(), n.getProperty("myref0").getNode().getPath());
         assertEquals("ref", ref.getPath(), n.getProperty("myref1").getNode().getPath());
 
-        checkReferences("refs", ref, n.getPath() + "/myref0", n.getPath() + "/myref1");
+        checkReferences("refs", ref.getReferences(), n.getPath() + "/myref0", n.getPath() + "/myref1");
+    }
+
+    public void testMultipleReferencesOnSameNode1() throws RepositoryException {
+        Node ref = testRootNode.addNode(nodeName2, testNodeType);
+        ref.addMixin(mixReferenceable);
+        superuser.save();
+
+        Node n = testRootNode.addNode(nodeName1, testNodeType);
+        n.setProperty("myref0", ref);
+        n.setProperty("myref1", ref);
+        superuser.save();
+
+        assertEquals("ref", ref.getPath(), n.getProperty("myref0").getNode().getPath());
+        assertEquals("ref", ref.getPath(), n.getProperty("myref1").getNode().getPath());
+
+        checkReferences("refs", ref.getReferences("myref0"), n.getPath() + "/myref0");
+        checkReferences("refs", ref.getReferences("myref1"), n.getPath() + "/myref1");
     }
 
     public void testMultipleReferences() throws RepositoryException {
@@ -79,7 +96,22 @@ public class ReferencesTest extends Abst
         n1.setProperty("myref", ref);
         superuser.save();
 
-        checkReferences("refs", ref, n0.getPath() + "/myref", n1.getPath() + "/myref");
+        checkReferences("refs", ref.getReferences(), n0.getPath() + "/myref", n1.getPath() + "/myref");
+    }
+
+    public void testMultipleReferences1() throws RepositoryException {
+        Node ref = testRootNode.addNode(nodeName2, testNodeType);
+        ref.addMixin(mixReferenceable);
+        superuser.save();
+
+        Node n0 = testRootNode.addNode(nodeName1, testNodeType);
+        n0.setProperty("myref0", ref);
+        Node n1 = testRootNode.addNode(nodeName3, testNodeType);
+        n1.setProperty("myref1", ref);
+        superuser.save();
+
+        checkReferences("refs", ref.getReferences("myref0"), n0.getPath() + "/myref0");
+        checkReferences("refs", ref.getReferences("myref1"), n1.getPath() + "/myref1");
     }
 
     // OAK-1195 Unable to move referenced mode
@@ -97,7 +129,7 @@ public class ReferencesTest extends Abst
         superuser.save();
         ref = superuser.getNode(newPath);
         assertEquals("ref", ref.getPath(), n.getProperty("myref").getNode().getPath());
-        checkReferences("refs", ref, n.getPath() + "/myref");
+        checkReferences("refs", ref.getReferences(), n.getPath() + "/myref");
     }
 
     public void testMVReferences() throws RepositoryException {
@@ -117,8 +149,8 @@ public class ReferencesTest extends Abst
 
         assertEquals("ref0", ref0.getIdentifier(), n.getProperty("myref").getValues()[0].getString());
         assertEquals("ref1", ref1.getIdentifier(), n.getProperty("myref").getValues()[1].getString());
-        checkReferences("refs", ref0, n.getPath() + "/myref");
-        checkReferences("refs", ref1, n.getPath() + "/myref");
+        checkReferences("refs", ref0.getReferences(), n.getPath() + "/myref");
+        checkReferences("refs", ref1.getReferences(), n.getPath() + "/myref");
     }
 
     public void testVersionReferencesVH() throws RepositoryException {
@@ -133,7 +165,7 @@ public class ReferencesTest extends Abst
         // check if versionable node has references to root version
         assertEquals("Version History", vh.getIdentifier(), n.getProperty(Property.JCR_VERSION_HISTORY).getString());
 
-        checkReferences("Version History", vh, p + "/jcr:versionHistory");
+        checkReferences("Version History", vh.getReferences(), p + "/jcr:versionHistory");
     }
 
     public void testVersionReferencesV0() throws RepositoryException {
@@ -149,7 +181,7 @@ public class ReferencesTest extends Abst
         assertEquals("Root Version", v0.getIdentifier(), n.getProperty(Property.JCR_BASE_VERSION).getString());
         assertEquals("Root Version", v0.getIdentifier(), n.getProperty(Property.JCR_PREDECESSORS).getValues()[0].getString());
 
-        checkReferences("Root Version", v0,
+        checkReferences("Root Version", v0.getReferences(),
                 p + "/jcr:baseVersion",
                 p + "/jcr:predecessors"
         );
@@ -168,7 +200,7 @@ public class ReferencesTest extends Abst
         assertEquals("v1.0", v1.getIdentifier(), n.getProperty(Property.JCR_BASE_VERSION).getString());
         assertEquals("v1.0", v1.getIdentifier(), n.getProperty(Property.JCR_PREDECESSORS).getValues()[0].getString());
 
-        checkReferences("v1.0", v1,
+        checkReferences("v1.0", v1.getReferences(),
                 p + "/jcr:baseVersion",
                 p + "/jcr:predecessors"
         );
@@ -192,7 +224,7 @@ public class ReferencesTest extends Abst
 
         assertEquals("ref", ref.getPath(), frozen.getProperty("myref").getNode().getPath());
 
-        checkReferences("ref in version store", ref, n.getPath() + "/myref");
+        checkReferences("ref in version store", ref.getReferences(), n.getPath() + "/myref");
 
         // also test what happens if node is removed
         n.remove();
@@ -228,14 +260,13 @@ public class ReferencesTest extends Abst
 
         Node frozen = v1.getFrozenNode();
         assertEquals("ref", ref.getPath(), frozen.getProperty("myref").getNode().getPath());
-        checkReferences("ref in version store", ref, n.getPath() + "/myref");
+        checkReferences("ref in version store", ref.getReferences(), n.getPath() + "/myref");
     }
 
-    private static void checkReferences(String msg, Node node, String ... expected) throws RepositoryException {
-        PropertyIterator iter = node.getReferences();
+    private static void checkReferences(String msg, PropertyIterator refs, String ... expected) throws RepositoryException {
         List<String> paths = new LinkedList<String>();
-        while (iter.hasNext()) {
-            paths.add(iter.nextProperty().getPath());
+        while (refs.hasNext()) {
+            paths.add(refs.nextProperty().getPath());
         }
         checkEquals(msg, paths, expected);
     }