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 {