jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From alexparvule...@apache.org
Subject svn commit: r1453803 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/ main/java/org/apache/jackrabbit/oak/plugins/index/p2/ test/java/org/apache/jackrabbit/oak/plugins/index/p2/
Date Thu, 07 Mar 2013 12:20:46 GMT
Author: alexparvulescu
Date: Thu Mar  7 12:20:45 2013
New Revision: 1453803

URL: http://svn.apache.org/r1453803
Log:
OAK-666 Property2Index: node type is used when indexing, but ignored when querying

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexLookup.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2Index.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexDiff.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexLookup.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexLookup.java?rev=1453803&r1=1453802&r2=1453803&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexLookup.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexLookup.java
Thu Mar  7 12:20:45 2013
@@ -47,8 +47,8 @@ class NodeTypeIndexLookup implements Jcr
      */
     public boolean isIndexed(String path) {
         Property2IndexLookup lookup = new Property2IndexLookup(root);
-        if (lookup.isIndexed(JCR_PRIMARYTYPE, path)
-                && lookup.isIndexed(JCR_MIXINTYPES, path)) {
+        if (lookup.isIndexed(JCR_PRIMARYTYPE, path, null)
+                && lookup.isIndexed(JCR_MIXINTYPES, path, null)) {
             return true;
         }
 
@@ -68,10 +68,10 @@ class NodeTypeIndexLookup implements Jcr
     public double getCost(Iterable<String> nodeTypes) {
         PropertyValue ntNames = PropertyValues.newName(nodeTypes);
         Property2IndexLookup lookup = new Property2IndexLookup(root);
-        return lookup.getCost(JCR_PRIMARYTYPE, ntNames)
-                + lookup.getCost(JCR_MIXINTYPES, ntNames);
+        return lookup.getCost(null, JCR_PRIMARYTYPE, ntNames)
+                + lookup.getCost(null, JCR_MIXINTYPES, ntNames);
     }
-    
+
     /**
      * Returns the paths that match the given node types.
      *

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2Index.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2Index.java?rev=1453803&r1=1453802&r2=1453803&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2Index.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2Index.java
Thu Mar  7 12:20:45 2013
@@ -114,14 +114,14 @@ class Property2Index implements QueryInd
         for (PropertyRestriction pr : filter.getPropertyRestrictions()) {
             // TODO support indexes on a path
             // currently, only indexes on the root node are supported
-            if (lookup.isIndexed(pr.propertyName, "/")) {
+            if (lookup.isIndexed(pr.propertyName, "/", filter)) {
                 if (pr.firstIncluding && pr.lastIncluding
                     && pr.first != null && pr.first.equals(pr.last)) {
                     // "[property] = $value"
-                    return lookup.getCost(pr.propertyName, pr.first);
+                    return lookup.getCost(filter, pr.propertyName, pr.first);
                 } else if (pr.first == null && pr.last == null) {
                     // "[property] is not null"
-                    return lookup.getCost(pr.propertyName, null);
+                    return lookup.getCost(filter, pr.propertyName, null);
                 }
             }
         }
@@ -137,7 +137,7 @@ class Property2Index implements QueryInd
         for (PropertyRestriction pr : filter.getPropertyRestrictions()) {
             // TODO support indexes on a path
             // currently, only indexes on the root node are supported
-            if (lookup.isIndexed(pr.propertyName, "/")) {
+            if (lookup.isIndexed(pr.propertyName, "/", filter)) {
                 // equality
                 if (pr.firstIncluding && pr.lastIncluding
                     && pr.first != null && pr.first.equals(pr.last)) {
@@ -164,7 +164,7 @@ class Property2Index implements QueryInd
         for (PropertyRestriction pr : filter.getPropertyRestrictions()) {
             // TODO support indexes on a path
             // currently, only indexes on the root node are supported
-            if (lookup.isIndexed(pr.propertyName, "/")) {
+            if (lookup.isIndexed(pr.propertyName, "/", filter)) {
                 if (pr.firstIncluding && pr.lastIncluding
                     && pr.first != null && pr.first.equals(pr.last)) {
                     buff.append(' ').append(pr.propertyName).append('=').append(pr.first);

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexDiff.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexDiff.java?rev=1453803&r1=1453802&r2=1453803&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexDiff.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexDiff.java
Thu Mar  7 12:20:45 2013
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.oak.plugin
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -180,6 +181,7 @@ class Property2IndexDiff implements Inde
         PropertyState appliesTo = builder.getProperty(declaringNodeTypes);
         if (appliesTo != null) {
             typeNames = newArrayList(appliesTo.getValue(Type.STRINGS));
+            Collections.sort(typeNames);
         }
         PropertyState ps = builder.getProperty(propertyNames);
 
@@ -192,8 +194,10 @@ class Property2IndexDiff implements Inde
                 this.indexMap.put(pname, list);
             }
             boolean exists = false;
+            String localPath = getPath();
             for (Property2IndexUpdate piu : list) {
-                if (piu.getPath().equals(getPath())) {
+                if (localPath.equals(piu.getPath())
+                        && typeNames.equals(piu.getNodeTypeNames())) {
                     exists = true;
                     break;
                 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java?rev=1453803&r1=1453802&r2=1453803&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java
Thu Mar  7 12:20:45 2013
@@ -16,7 +16,10 @@
  */
 package org.apache.jackrabbit.oak.plugins.index.p2;
 
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.DECLARING_NODE_TYPES;
 import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.PROPERTY_NAMES;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
 
 import java.util.Iterator;
 import java.util.List;
@@ -27,7 +30,6 @@ import org.apache.jackrabbit.oak.api.Pro
 import org.apache.jackrabbit.oak.api.PropertyValue;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.commons.PathUtils;
-import org.apache.jackrabbit.oak.plugins.index.IndexConstants;
 import org.apache.jackrabbit.oak.plugins.index.p2.strategy.ContentMirrorStoreStrategy;
 import org.apache.jackrabbit.oak.plugins.index.p2.strategy.IndexStoreStrategy;
 import org.apache.jackrabbit.oak.spi.query.Filter;
@@ -73,27 +75,23 @@ public class Property2IndexLookup {
      * @param path lookup path
      * @return true if the property is indexed
      */
-    public boolean isIndexed(String propertyName, String path) {
-        return isIndexed(root, propertyName, path);
-    }
-    
-    private static boolean isIndexed(NodeState root, String propertyName, String path) {
+    public boolean isIndexed(String propertyName, String path, Filter filter) {
+        if(PathUtils.denotesRoot(path)){
+            return getIndexDataNode(root, propertyName, filter) != null;
+        }
         NodeState node = root;
         Iterator<String> it = PathUtils.elements(path).iterator();
-        while (true) {
-            if (getIndexDataNode(node, propertyName) != null) {
+        while (it.hasNext()) {
+            if (getIndexDataNode(node, propertyName, filter) != null) {
                 return true;
             }
-            if (!it.hasNext()) {
-                break;
-            }
             node = node.getChildNode(it.next());
         }
         return false;
     }
-    
+
     public Iterable<String> query(Filter filter, String propertyName, PropertyValue
value) {
-        NodeState state = getIndexDataNode(root, propertyName);
+        NodeState state = getIndexDataNode(root, propertyName, filter);
         if (state == null) {
             throw new IllegalArgumentException("No index for " + propertyName);
         }
@@ -101,8 +99,8 @@ public class Property2IndexLookup {
         return store.query(filter, propertyName, state, values);
     }
 
-    public double getCost(String name, PropertyValue value) {
-        NodeState state = getIndexDataNode(root, name);
+    public double getCost(Filter filter, String name, PropertyValue value) {
+        NodeState state = getIndexDataNode(root, name, filter);
         if (state == null) {
             return Double.POSITIVE_INFINITY;
         }
@@ -115,32 +113,54 @@ public class Property2IndexLookup {
      * applicable index with data.
      * 
      * @param propertyName the property name
+     * @param filter for the node type restriction
      * @return the node where the index data is stored, or null if no index
      *         definition or index data node was found
      */
     @Nullable
-    private static NodeState getIndexDataNode(NodeState node, String propertyName) {
+    private static NodeState getIndexDataNode(NodeState node, String propertyName, Filter
filter) {
         NodeState state = node.getChildNode(INDEX_DEFINITIONS_NAME);
-        if (state != null) {
-            for (ChildNodeEntry entry : state.getChildNodeEntries()) {
-                PropertyState type = entry.getNodeState().getProperty(IndexConstants.TYPE_PROPERTY_NAME);
-                if (type == null || type.isArray() || !Property2Index.TYPE.equals(type.getValue(Type.STRING)))
{
-                    continue;
+        if (state == null) {
+            return null;
+        }
+        String filterNodeType = null;
+        if (filter != null) {
+            filterNodeType = filter.getNodeType();
+        }
+        //keep a fallback to a matching index def that has *no* node type constraints
+        NodeState fallback = null;
+        for (ChildNodeEntry entry : state.getChildNodeEntries()) {
+            NodeState ns = entry.getNodeState();
+            PropertyState type = ns.getProperty(TYPE_PROPERTY_NAME);
+            if (type == null || type.isArray() || !Property2Index.TYPE.equals(type.getValue(Type.STRING)))
{
+                continue;
+            }
+            if (containsValue(ns.getProperty(PROPERTY_NAMES), propertyName)) {
+                if (filterNodeType == null
+                        || containsValue(ns.getProperty(DECLARING_NODE_TYPES),
+                                filterNodeType)) {
+                    return ns.getChildNode(":index");
                 }
-                PropertyState names = entry.getNodeState().getProperty("propertyNames");
-                if (names != null) {
-                    for (int i = 0; i < names.count(); i++) {
-                        if (propertyName.equals(names.getValue(Type.STRING, i))) {
-                            NodeState indexDef = entry.getNodeState();
-                            NodeState index = indexDef.getChildNode(":index");
-                            if (index != null) {
-                                return index;
-                            }
-                        }
-                    }
+                if (ns.getProperty(DECLARING_NODE_TYPES) == null) {
+                    fallback = ns.getChildNode(":index");
+                }
+            }
+        }
+        return fallback;
+    }
+
+    private static boolean containsValue(PropertyState values, String lookup) {
+        if (values == null) {
+            return false;
+        }
+        if (values.isArray()) {
+            for (String v : values.getValue(Type.STRINGS)) {
+                if (lookup.equals(v)) {
+                    return true;
                 }
             }
+            return false;
         }
-        return null;
+        return lookup.equals(values.getValue(Type.STRING));
     }
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexTest.java?rev=1453803&r1=1453802&r2=1453803&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexTest.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexTest.java
Thu Mar  7 12:20:45 2013
@@ -27,6 +27,8 @@ import org.apache.jackrabbit.oak.api.Com
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.plugins.index.IndexHook;
 import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeState;
+import org.apache.jackrabbit.oak.query.index.FilterImpl;
+import org.apache.jackrabbit.oak.spi.query.Filter;
 import org.apache.jackrabbit.oak.spi.query.PropertyValues;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
@@ -79,14 +81,18 @@ public class Property2IndexTest {
         assertEquals(MANY + 2, find(lookup, "foo", null).size());
 
         double cost;
-        cost = lookup.getCost("foo", PropertyValues.newString("xyz"));
+        cost = lookup.getCost(null, "foo", PropertyValues.newString("xyz"));
         assertTrue("cost: " + cost, cost >= MANY);
-        cost = lookup.getCost("foo", null);
+        cost = lookup.getCost(null, "foo", null);
         assertTrue("cost: " + cost, cost >= MANY);
     }
 
+    private static Set<String> find(Property2IndexLookup lookup, String name, String
value, Filter filter) {
+        return Sets.newHashSet(lookup.query(filter, name, value == null ? null : PropertyValues.newString(value)));
+    }
+
     private static Set<String> find(Property2IndexLookup lookup, String name, String
value) {
-        return Sets.newHashSet(lookup.query(null, name, value == null ? null : PropertyValues.newString(value)));
+        return find(lookup, name, value, null);
     }
 
     @Test
@@ -133,6 +139,130 @@ public class Property2IndexTest {
         }
     }
 
+    /**
+     * @see <a href="https://issues.apache.org/jira/browse/OAK-666">OAK-666:
+     *      Property2Index: node type is used when indexing, but ignored when
+     *      querying</a>
+     */
+    @Test
+    public void testCustomConfigNodeType() throws Exception {
+        NodeState root = MemoryNodeState.EMPTY_NODE;
+
+        // Add index definitions
+        NodeBuilder builder = root.builder();
+        NodeBuilder index = builder.child("oak:index");
+        index.child("fooIndex")
+                .setProperty("jcr:primaryType", "oak:queryIndexDefinition",
+                        Type.NAME)
+                .setProperty("type", "p2")
+                .setProperty("propertyNames", Arrays.asList("foo", "extrafoo"),
+                        Type.STRINGS)
+                .setProperty("declaringNodeTypes",
+                        Arrays.asList("nt:unstructured"), Type.STRINGS);
+        index.child("fooIndexFile")
+                .setProperty("jcr:primaryType", "oak:queryIndexDefinition",
+                        Type.NAME)
+                .setProperty("type", "p2")
+                .setProperty("propertyNames", Arrays.asList("foo"),
+                        Type.STRINGS)
+                .setProperty("declaringNodeTypes", Arrays.asList("nt:file"),
+                        Type.STRINGS);
+        NodeState before = builder.getNodeState();
+
+        // Add some content and process it through the property index hook
+        builder = before.builder();
+        builder.child("a").setProperty("jcr:primaryType", "nt:unstructured")
+                .setProperty("foo", "abc");
+        builder.child("b").setProperty("jcr:primaryType", "nt:unstructured")
+                .setProperty("foo", Arrays.asList("abc", "def"), Type.STRINGS);
+        NodeState after = builder.getNodeState();
+
+        // Add an index
+        IndexHook p = new Property2IndexDiff(builder);
+        after.compareAgainstBaseState(before, p);
+        p.apply();
+        p.close();
+
+        NodeState indexedState = builder.getNodeState();
+
+        FilterImpl f = new FilterImpl(null, null);
+        f.setNodeType("nt:unstructured");
+
+        // Query the index
+        Property2IndexLookup lookup = new Property2IndexLookup(indexedState);
+        assertEquals(ImmutableSet.of("a", "b"), find(lookup, "foo", "abc", f));
+        assertEquals(ImmutableSet.of("b"), find(lookup, "foo", "def", f));
+        assertEquals(ImmutableSet.of(), find(lookup, "foo", "ghi", f));
+
+        try {
+            assertEquals(ImmutableSet.of(), find(lookup, "pqr", "foo", f));
+            fail();
+        } catch (IllegalArgumentException e) {
+            // expected: no index for "pqr"
+        }
+    }
+
+    /**
+     * @see <a href="https://issues.apache.org/jira/browse/OAK-666">OAK-666:
+     *      Property2Index: node type is used when indexing, but ignored when
+     *      querying</a>
+     */
+    @Test
+    public void testCustomConfigNodeTypeFallback() throws Exception {
+        NodeState root = MemoryNodeState.EMPTY_NODE;
+
+        // Add index definitions
+        NodeBuilder builder = root.builder();
+        NodeBuilder index = builder.child("oak:index");
+        index.child("fooIndex")
+                .setProperty("jcr:primaryType", "oak:queryIndexDefinition",
+                        Type.NAME)
+                .setProperty("type", "p2")
+                .setProperty("propertyNames", Arrays.asList("foo", "extrafoo"),
+                        Type.STRINGS);
+        index.child("fooIndexFile")
+                .setProperty("jcr:primaryType", "oak:queryIndexDefinition",
+                        Type.NAME)
+                .setProperty("type", "p2")
+                .setProperty("propertyNames", Arrays.asList("foo"),
+                        Type.STRINGS)
+                .setProperty("declaringNodeTypes", Arrays.asList("nt:file"),
+                        Type.STRINGS);
+        NodeState before = builder.getNodeState();
+
+        // Add some content and process it through the property index hook
+        builder = before.builder();
+        builder.child("a").setProperty("jcr:primaryType", "nt:unstructured")
+                .setProperty("foo", "abc");
+        builder.child("b").setProperty("jcr:primaryType", "nt:unstructured")
+                .setProperty("foo", Arrays.asList("abc", "def"), Type.STRINGS);
+        NodeState after = builder.getNodeState();
+
+        // Add an index
+        IndexHook p = new Property2IndexDiff(builder);
+        after.compareAgainstBaseState(before, p);
+        p.apply();
+        p.close();
+
+        NodeState indexedState = builder.getNodeState();
+
+        FilterImpl f = new FilterImpl(null, null);
+        f.setNodeType("nt:unstructured");
+
+        // Query the index
+        Property2IndexLookup lookup = new Property2IndexLookup(indexedState);
+        assertEquals(ImmutableSet.of("a", "b"), find(lookup, "foo", "abc", f));
+        assertEquals(ImmutableSet.of("b"), find(lookup, "foo", "def", f));
+        assertEquals(ImmutableSet.of(), find(lookup, "foo", "ghi", f));
+
+        try {
+            assertEquals(ImmutableSet.of(), find(lookup, "pqr", "foo", f));
+            fail();
+        } catch (IllegalArgumentException e) {
+            // expected: no index for "pqr"
+        }
+    }
+
     @Test
     public void testUnique() throws Exception {
 



Mime
View raw message