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 al...@apache.org on 2013/03/07 13:20:46 UTC
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/
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 {