Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java (revision da183012241c77b3428d21fc510da8125ee2fd32) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexEditor.java (revision ) @@ -40,6 +40,7 @@ import org.apache.jackrabbit.oak.plugins.index.IndexConstants; import org.apache.jackrabbit.oak.plugins.index.IndexEditor; import org.apache.jackrabbit.oak.plugins.index.IndexUpdateCallback; +import org.apache.jackrabbit.oak.plugins.index.PathFilter; import org.apache.jackrabbit.oak.plugins.index.property.strategy.ContentMirrorStoreStrategy; import org.apache.jackrabbit.oak.plugins.index.property.strategy.IndexStoreStrategy; import org.apache.jackrabbit.oak.plugins.index.property.strategy.UniqueEntryStoreStrategy; @@ -110,6 +111,10 @@ private final IndexUpdateCallback updateCallback; + private final PathFilter pathFilter; + + private final PathFilter.Result pathFilterResult; + public PropertyIndexEditor(NodeBuilder definition, NodeState root, IndexUpdateCallback updateCallback) { this.parent = null; @@ -117,6 +122,8 @@ this.path = "/"; this.definition = definition; this.root = root; + pathFilter = PathFilter.from(definition); + pathFilterResult = getPathFilterResult(); //initPropertyNames(definition); @@ -147,7 +154,7 @@ this.updateCallback = updateCallback; } - PropertyIndexEditor(PropertyIndexEditor parent, String name) { + PropertyIndexEditor(PropertyIndexEditor parent, String name, PathFilter.Result pathFilterResult) { this.parent = parent; this.name = name; this.path = null; @@ -157,6 +164,8 @@ this.typePredicate = parent.typePredicate; this.keysToCheckForUniqueness = parent.keysToCheckForUniqueness; this.updateCallback = parent.updateCallback; + this.pathFilter = parent.pathFilter; + this.pathFilterResult = pathFilterResult; } /** @@ -228,6 +237,12 @@ @Override public void leave(NodeState before, NodeState after) throws CommitFailedException { + + if (pathFilterResult != PathFilter.Result.INCLUDE) { + checkUniquenessConstraints(); + return; + } + // apply the type restrictions if (typePredicate != null) { if (typeChanged) { @@ -276,13 +291,17 @@ } } + checkUniquenessConstraints(); + } + + private void checkUniquenessConstraints() throws CommitFailedException { if (parent == null) { // make sure that the index node exist, even with no content definition.child(INDEX_CONTENT_NODE_NAME); boolean uniqueIndex = keysToCheckForUniqueness != null; // check uniqueness constraints when leaving the root - if (uniqueIndex && + if (uniqueIndex && !keysToCheckForUniqueness.isEmpty()) { NodeState indexMeta = definition.getNodeState(); String failed = getFirstDuplicate( @@ -292,12 +311,12 @@ "Uniqueness constraint violated at path [%s] for one of the " + "property in %s having value %s", getPath(), propertyNames, failed); - throw new CommitFailedException(CONSTRAINT, 30, msg); + throw new CommitFailedException(CONSTRAINT, 30, msg); } } } } - + /** * From a set of keys, get those that already exist in the index. * @@ -378,24 +397,43 @@ * @param name the name of the child node * @return an instance of the PropertyIndexEditor */ - PropertyIndexEditor getChildIndexEditor(@Nonnull PropertyIndexEditor parent, @Nonnull String name) { - return new PropertyIndexEditor(parent, name); + PropertyIndexEditor getChildIndexEditor(@Nonnull PropertyIndexEditor parent, @Nonnull String name, PathFilter.Result filterResult) { + return new PropertyIndexEditor(parent, name, filterResult); } @Override public Editor childNodeAdded(String name, NodeState after) { - return getChildIndexEditor(this, name); + PathFilter.Result filterResult = getPathFilterResult(name); + if (filterResult != PathFilter.Result.EXCLUDE) { + return getChildIndexEditor(this, name, filterResult); - } + } + return null; + } @Override public Editor childNodeChanged( String name, NodeState before, NodeState after) { - return getChildIndexEditor(this, name); + PathFilter.Result filterResult = getPathFilterResult(name); + if (filterResult != PathFilter.Result.EXCLUDE) { + return getChildIndexEditor(this, name, filterResult); - } + } + return null; + } @Override public Editor childNodeDeleted(String name, NodeState before) { - return getChildIndexEditor(this, name); + PathFilter.Result filterResult = getPathFilterResult(name); + if (filterResult == PathFilter.Result.EXCLUDE) { + return null; - } + } + return getChildIndexEditor(this, name, filterResult); + } + private PathFilter.Result getPathFilterResult() { + return pathFilter.doFiler(getPath()); + } + + private PathFilter.Result getPathFilterResult(String childNodeName) { + return pathFilter.doFiler(concat(getPath(), childNodeName)); + } } \ No newline at end of file Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java (revision da183012241c77b3428d21fc510da8125ee2fd32) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java (revision ) @@ -29,8 +29,12 @@ import java.util.Set; +import com.google.common.collect.Iterables; +import org.apache.jackrabbit.oak.api.PropertyState; 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.property.strategy.ContentMirrorStoreStrategy; import org.apache.jackrabbit.oak.plugins.index.property.strategy.IndexStoreStrategy; import org.apache.jackrabbit.oak.plugins.index.property.strategy.UniqueEntryStoreStrategy; @@ -48,12 +52,16 @@ import org.apache.jackrabbit.oak.spi.query.Filter; import org.apache.jackrabbit.oak.spi.query.Filter.PropertyRestriction; import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Plan for querying a given property index using a given filter. */ public class PropertyIndexPlan { + private static final Logger log = LoggerFactory.getLogger(PropertyIndexPlan.class); + /** * The cost overhead to use the index in number of read operations. */ @@ -94,11 +102,15 @@ private final int depth; + private final String[] queryPaths; + PropertyIndexPlan(String name, NodeState root, NodeState definition, Filter filter) { this.name = name; this.root = root; this.definition = definition; this.properties = newHashSet(definition.getNames(PROPERTY_NAMES)); + PropertyState ps = definition.getProperty(IndexConstants.QUERY_PATHS); + queryPaths = ps == null ? null : Iterables.toArray(ps.getValue(Type.STRINGS), String.class); if (definition.getBoolean(UNIQUE_PROPERTY_NAME)) { this.strategy = UNIQUE; @@ -118,7 +130,7 @@ Set bestValues = emptySet(); int bestDepth = 1; - if (matchesNodeTypes) { + if (matchesNodeTypes && checkForQueryPaths()) { for (String property : properties) { PropertyRestriction restriction = filter.getPropertyRestriction(property); @@ -174,6 +186,28 @@ this.depth = bestDepth; this.values = bestValues; this.cost = COST_OVERHEAD + bestCost; + } + + /** + * Check if there is a mismatch between QueryPaths associated with index + * and path restriction specified in query + + * @return true if QueryPaths and path restrictions do not have any conflict + */ + private boolean checkForQueryPaths() { + if (queryPaths == null){ + //No explicit value specified. Assume '/' which results in true + return true; + } + + String pathRestriction = filter.getPath(); + for (String queryPath : queryPaths){ + if (queryPath.equals(pathRestriction) || PathUtils.isAncestor(queryPath, pathRestriction)){ + return true; + } + } + + return false; } private Set findMultiProperty(OrImpl or) { Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java (revision da183012241c77b3428d21fc510da8125ee2fd32) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java (revision ) @@ -16,6 +16,8 @@ */ package org.apache.jackrabbit.oak.plugins.index.property; +import static com.google.common.collect.ImmutableSet.of; +import static java.util.Arrays.asList; import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; import static org.apache.jackrabbit.JcrConstants.JCR_SYSTEM; import static org.apache.jackrabbit.JcrConstants.NT_BASE; @@ -23,14 +25,19 @@ import static org.apache.jackrabbit.JcrConstants.NT_UNSTRUCTURED; import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME; import static org.apache.jackrabbit.oak.plugins.index.IndexUtils.createIndexDefinition; +import static org.apache.jackrabbit.oak.plugins.index.PathFilter.PROP_EXCLUDED_PATHS; +import static org.apache.jackrabbit.oak.plugins.index.PathFilter.PROP_INCLUDED_PATHS; import static org.apache.jackrabbit.oak.plugins.index.counter.NodeCounterEditor.COUNT_PROPERTY_NAME; import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; +import static org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty; import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.JCR_NODE_TYPES; import static org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent.INITIAL_CONTENT; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.junit.matchers.JUnitMatchers.containsString; import java.util.Arrays; import java.util.Set; @@ -43,11 +50,13 @@ import ch.qos.logback.core.spi.FilterReply; import com.google.common.collect.Iterables; import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.plugins.index.IndexUpdateProvider; import org.apache.jackrabbit.oak.plugins.index.property.strategy.ContentMirrorStoreStrategy; import org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState; import org.apache.jackrabbit.oak.query.QueryEngineSettings; +import org.apache.jackrabbit.oak.query.ast.Operator; import org.apache.jackrabbit.oak.query.ast.SelectorImpl; import org.apache.jackrabbit.oak.query.index.FilterImpl; import org.apache.jackrabbit.oak.query.index.TraversingIndex; @@ -563,6 +572,130 @@ assertTrue("Warning should not be logged for traversing " + testDataSize, appender.list.isEmpty()); deregisterAppender(appender); + } + + @Test + public void testPathInclude() throws Exception { + NodeState root = INITIAL_CONTENT; + + // Add index definition + NodeBuilder builder = root.builder(); + NodeBuilder index = createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME), "foo", + true, false, ImmutableSet.of("foo"), null); + index.setProperty(createProperty(PROP_INCLUDED_PATHS, of("/test/a"), Type.STRINGS)); + NodeState before = builder.getNodeState(); + + // Add some content and process it through the property index hook + builder.child("test").child("a").setProperty("foo", "abc"); + builder.child("test").child("b").setProperty("foo", "abc"); + NodeState after = builder.getNodeState(); + + NodeState indexed = HOOK.processCommit(before, after, CommitInfo.EMPTY); + + FilterImpl f = createFilter(indexed, NT_BASE); + + // Query the index + PropertyIndexLookup lookup = new PropertyIndexLookup(indexed); + assertEquals(ImmutableSet.of("test/a"), find(lookup, "foo", "abc", f)); + } + + @Test + public void testPathExclude() throws Exception { + NodeState root = INITIAL_CONTENT; + + // Add index definition + NodeBuilder builder = root.builder(); + NodeBuilder index = createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME), "foo", + true, false, ImmutableSet.of("foo"), null); + index.setProperty(createProperty(PROP_EXCLUDED_PATHS, of("/test/a"), Type.STRINGS)); + NodeState before = builder.getNodeState(); + + // Add some content and process it through the property index hook + builder.child("test").child("a").setProperty("foo", "abc"); + builder.child("test").child("b").setProperty("foo", "abc"); + NodeState after = builder.getNodeState(); + + NodeState indexed = HOOK.processCommit(before, after, CommitInfo.EMPTY); + + FilterImpl f = createFilter(indexed, NT_BASE); + + // Query the index + PropertyIndexLookup lookup = new PropertyIndexLookup(indexed); + assertEquals(ImmutableSet.of("test/b"), find(lookup, "foo", "abc", f)); + } + + @Test + public void testPathIncludeExclude() throws Exception { + NodeState root = INITIAL_CONTENT; + + // Add index definition + NodeBuilder builder = root.builder(); + NodeBuilder index = createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME), "foo", + true, false, ImmutableSet.of("foo"), null); + index.setProperty(createProperty(PROP_INCLUDED_PATHS, of("/test/a"), Type.STRINGS)); + index.setProperty(createProperty(PROP_EXCLUDED_PATHS, of("/test/a/b"), Type.STRINGS)); + NodeState before = builder.getNodeState(); + + // Add some content and process it through the property index hook + builder.child("test").child("a").setProperty("foo", "abc"); + builder.child("test").child("a").child("b").setProperty("foo", "abc"); + NodeState after = builder.getNodeState(); + + NodeState indexed = HOOK.processCommit(before, after, CommitInfo.EMPTY); + + FilterImpl f = createFilter(indexed, NT_BASE); + + // Query the index + PropertyIndexLookup lookup = new PropertyIndexLookup(indexed); + assertEquals(ImmutableSet.of("test/a"), find(lookup, "foo", "abc", f)); + } + + @Test + public void testPathExcludeInclude() throws Exception{ + NodeState root = INITIAL_CONTENT; + + // Add index definition + NodeBuilder builder = root.builder(); + NodeBuilder index = createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME), "foo", + true, false, ImmutableSet.of("foo"), null); + index.setProperty(createProperty(PROP_INCLUDED_PATHS, of("/test/a/b"), Type.STRINGS)); + index.setProperty(createProperty(PROP_EXCLUDED_PATHS, of("/test/a"), Type.STRINGS)); + NodeState before = builder.getNodeState(); + + // Add some content and process it through the property index hook + builder.child("test").child("a").setProperty("foo", "abc"); + builder.child("test").child("a").child("b").setProperty("foo", "abc"); + NodeState after = builder.getNodeState(); + + try { + HOOK.processCommit(before, after, CommitInfo.EMPTY); + assertTrue(false); + } catch (IllegalStateException expected) {} + } + + @Test + public void testPathMismatch() throws Exception { + NodeState root = INITIAL_CONTENT; + + // Add index definition + NodeBuilder builder = root.builder(); + NodeBuilder index = createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME), "foo", + true, false, ImmutableSet.of("foo"), null); + index.setProperty(createProperty(PROP_INCLUDED_PATHS, of("/test/a"), Type.STRINGS)); + index.setProperty(createProperty(PROP_EXCLUDED_PATHS, of("/test/a/b"), Type.STRINGS)); + NodeState before = builder.getNodeState(); + + // Add some content and process it through the property index hook + builder.child("test").child("a").setProperty("foo", "abc"); + builder.child("test").child("a").child("b").setProperty("foo", "abc"); + NodeState after = builder.getNodeState(); + + NodeState indexed = HOOK.processCommit(before, after, CommitInfo.EMPTY); + + FilterImpl f = createFilter(indexed, NT_BASE); + f.restrictPath("/test2", Filter.PathRestriction.ALL_CHILDREN); + PropertyIndexPlan plan = new PropertyIndexPlan("plan", root, index.getNodeState(), f); + assertTrue(Double.POSITIVE_INFINITY == plan.getCost()); } private int getResultSize(NodeState indexed, String name, String value){ Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/OrderedPropertyIndexEditor.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/OrderedPropertyIndexEditor.java (revision da183012241c77b3428d21fc510da8125ee2fd32) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/OrderedPropertyIndexEditor.java (revision ) @@ -27,6 +27,7 @@ import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.plugins.index.IndexConstants; import org.apache.jackrabbit.oak.plugins.index.IndexUpdateCallback; +import org.apache.jackrabbit.oak.plugins.index.PathFilter; import org.apache.jackrabbit.oak.plugins.index.property.OrderedIndex.OrderDirection; import org.apache.jackrabbit.oak.plugins.index.property.strategy.IndexStoreStrategy; import org.apache.jackrabbit.oak.plugins.index.property.strategy.OrderedContentMirrorStoreStrategy; @@ -101,8 +102,8 @@ swl = new StopwatchLogger(OrderedPropertyIndexEditor.class); } - OrderedPropertyIndexEditor(OrderedPropertyIndexEditor parent, String name) { - super(parent, name); + OrderedPropertyIndexEditor(OrderedPropertyIndexEditor parent, String name, PathFilter.Result pathFilterResult) { + super(parent, name, pathFilterResult); this.propertyNames = parent.getPropertyNames(); this.direction = parent.getDirection(); this.swl = parent.swl; @@ -133,8 +134,8 @@ @Override PropertyIndexEditor getChildIndexEditor(@Nonnull PropertyIndexEditor parent, - @Nonnull String name) { - return new OrderedPropertyIndexEditor(this, name); + @Nonnull String name, PathFilter.Result pathFilterResult) { + return new OrderedPropertyIndexEditor(this, name, pathFilterResult); } /** Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/Oak2077QueriesTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/Oak2077QueriesTest.java (revision da183012241c77b3428d21fc510da8125ee2fd32) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/Oak2077QueriesTest.java (revision ) @@ -49,6 +49,7 @@ import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.plugins.index.IndexUpdateCallback; import org.apache.jackrabbit.oak.plugins.index.IndexUtils; +import org.apache.jackrabbit.oak.plugins.index.PathFilter; import org.apache.jackrabbit.oak.plugins.index.property.OrderedIndex.OrderDirection; import org.apache.jackrabbit.oak.plugins.index.property.strategy.IndexStoreStrategy; import org.apache.jackrabbit.oak.plugins.index.property.strategy.OrderedContentMirrorStoreStrategy; @@ -169,8 +170,8 @@ this.rnd = rnd; } - public SeededPropertyIndexEditor(SeededPropertyIndexEditor parent, String name) { - super(parent, name); + public SeededPropertyIndexEditor(SeededPropertyIndexEditor parent, String name, PathFilter.Result pathFilterResult) { + super(parent, name, pathFilterResult); this.rnd = parent.rnd; } @@ -185,8 +186,8 @@ } @Override - PropertyIndexEditor getChildIndexEditor(PropertyIndexEditor parent, String name) { - return new SeededPropertyIndexEditor(this, name); + PropertyIndexEditor getChildIndexEditor(PropertyIndexEditor parent, String name, PathFilter.Result pathFilterResult) { + return new SeededPropertyIndexEditor(this, name, pathFilterResult); } } @@ -314,7 +315,7 @@ /** * truncate the {@link AbstractQueryTest#TEST_INDEX_NAME} index at the 4th element of the * provided lane returning the previous value - * + * * @param lane the desired lane. Must be 0 <= {@code lane} < {@link OrderedIndex#LANES} * @param inexistent the derired value to be injected * @return the value before the change