Index: oak-api/src/main/java/org/apache/jackrabbit/oak/api/StrictPathRestriction.java =================================================================== --- oak-api/src/main/java/org/apache/jackrabbit/oak/api/StrictPathRestriction.java (nonexistent) +++ oak-api/src/main/java/org/apache/jackrabbit/oak/api/StrictPathRestriction.java (working copy) @@ -0,0 +1,31 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.api; + +import java.util.Locale; + +public enum StrictPathRestriction { + ENABLE, + WARN, + DISABLE; + + public static StrictPathRestriction stringToEnum(String strictPathRestrictionInString) { + // OAK-260 : Locale english is being used explicitly. (DISABLE[in turkish] = DİSABLE[in English]--- Mind the dot above I) + return StrictPathRestriction.valueOf(strictPathRestrictionInString.toUpperCase(Locale.ENGLISH)); + } + +} Index: oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java =================================================================== --- oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java (revision 1862608) +++ oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/QueryEngineSettingsMBean.java (working copy) @@ -87,6 +87,32 @@ void setFastQuerySize(boolean fastQuerySize); /** + * Whether Path restrictions are enabled while figuring out index plan + * + * @return true if enabled + */ + String getStrictPathRestriction(); + + /** + * Whether path restrictions of indexes (excludedPaths / includedPaths) are taken into account during query execution, + * for Lucene indexes. When enabled, only indexes are considered if the index path restriction is compatible with the + * query path restrictions. When disabled, only the queryPaths of the index is taken into account. + * + * @param pathRestriction Set path restriction: Expected value is either of ENABLE/DISABLE/WARN + * ENABLE: enable path restriction- Index won't be used if index definition path restrictions are not compatible with query's path restriction + * DISABLE: path restrictions are not taken into account while querying + * WARN: path restrictions are not taken into account but a warning will be logged if query path restrictions are not compatible with index path restrictions + */ + @Description("Set path restriction: Expected value is either of ENABLE/DISABLE/WARN. " + + "ENABLE: enable path restriction- Index won't be used if index definition path restrictions are not compatible with query's path restriction. " + + "DISABLE: path restrictions are not taken into account while querying. " + + "WARN: path restrictions are not taken into account but a warning will be logged if query path restrictions are not compatible with index path restrictions." + ) + void setStrictPathRestriction( + @Name("pathRestriction") + String pathRestriction); + + /** * Set or remove a query validator pattern. * * @param key the key Index: oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/package-info.java =================================================================== --- oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/package-info.java (revision 1862608) +++ oak-api/src/main/java/org/apache/jackrabbit/oak/api/jmx/package-info.java (working copy) @@ -15,7 +15,7 @@ * limitations under the License. */ -@Version("4.8.0") +@Version("4.9.0") package org.apache.jackrabbit.oak.api.jmx; import org.osgi.annotation.versioning.Version; Index: oak-api/src/main/java/org/apache/jackrabbit/oak/api/package-info.java =================================================================== --- oak-api/src/main/java/org/apache/jackrabbit/oak/api/package-info.java (revision 1862608) +++ oak-api/src/main/java/org/apache/jackrabbit/oak/api/package-info.java (working copy) @@ -18,7 +18,7 @@ /** * Oak repository API */ -@Version("3.1.2") +@Version("3.2.0") package org.apache.jackrabbit.oak.api; import org.osgi.annotation.versioning.Version; Index: oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java (revision 1862608) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java (working copy) @@ -411,6 +411,7 @@ this.queryEngineSettings.setFullTextComparisonWithoutIndex(settings.getFullTextComparisonWithoutIndex()); this.queryEngineSettings.setLimitInMemory(settings.getLimitInMemory()); this.queryEngineSettings.setLimitReads(settings.getLimitReads()); + this.queryEngineSettings.setStrictPathRestriction(settings.getStrictPathRestriction()); return this; } @@ -933,6 +934,14 @@ settings.setFastQuerySize(fastQuerySize); } + public String getStrictPathRestriction() { + return settings.getStrictPathRestriction(); + } + + public void setStrictPathRestriction(String strictPathRestriction) { + settings.setStrictPathRestriction(strictPathRestriction); + } + @Override public void setQueryValidatorPattern(String key, String pattern, String comment, boolean failQuery) { settings.getQueryValidator().setPattern(key, pattern, comment, failQuery); Index: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java (revision 1862608) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java (working copy) @@ -18,6 +18,7 @@ */ package org.apache.jackrabbit.oak.query; +import org.apache.jackrabbit.oak.api.StrictPathRestriction; import org.apache.jackrabbit.oak.api.jmx.QueryEngineSettingsMBean; import org.apache.jackrabbit.oak.query.stats.QueryStatsMBean; import org.apache.jackrabbit.oak.query.stats.QueryStatsMBeanImpl; @@ -76,6 +77,8 @@ public static final boolean DEFAULT_FAST_QUERY_SIZE = Boolean.getBoolean(OAK_FAST_QUERY_SIZE); private boolean fastQuerySize = DEFAULT_FAST_QUERY_SIZE; + private StrictPathRestriction strictPathRestriction = StrictPathRestriction.DISABLE; + private final QueryStatsMBeanImpl queryStats = new QueryStatsMBeanImpl(this); /** @@ -134,6 +137,14 @@ System.setProperty(OAK_FAST_QUERY_SIZE, String.valueOf(fastQuerySize)); } + public String getStrictPathRestriction() { + return strictPathRestriction.name(); + } + + public void setStrictPathRestriction(String strictPathRestriction) { + this.strictPathRestriction = StrictPathRestriction.stringToEnum(strictPathRestriction); + } + public void setFullTextComparisonWithoutIndex(boolean fullTextComparisonWithoutIndex) { this.fullTextComparisonWithoutIndex = fullTextComparisonWithoutIndex; } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettingsService.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettingsService.java (revision 1862608) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettingsService.java (working copy) @@ -68,6 +68,16 @@ "Note: even if enabled, getSize may still return -1 if the index used does not support the feature." ) boolean fastQuerySize() default false; + + @AttributeDefinition( + name = "Enable Strict Path restrictions for indexes to be used", + description = "Whether path restrictions of indexes (excludedPaths / includedPaths) are taken into" + + "account during query execution, for Lucene indexes. When enabled, only indexes are considered if" + + "the index path restriction is compatible with the query path restrictions. When disabled, only" + + "the queryPaths of the index is taken into account." + ) + String getStrictPathRestrictionsForIndexes() default DISABLED_STRICT_PATH_RESTRICTION; + } // should be the same as QueryEngineSettings.DEFAULT_QUERY_LIMIT_IN_MEMORY @@ -82,6 +92,7 @@ static final String QUERY_FAIL_TRAVERSAL = "queryFailTraversal"; static final String QUERY_FAST_QUERY_SIZE = "fastQuerySize"; + static final String DISABLED_STRICT_PATH_RESTRICTION = "DISABLE"; private final Logger log = LoggerFactory.getLogger(getClass()); @@ -114,6 +125,8 @@ boolean fastQuerySizeSysProp = QueryEngineSettings.DEFAULT_FAST_QUERY_SIZE; boolean fastQuerySizeFromConfig = config.fastQuerySize(); queryEngineSettings.setFastQuerySize(fastQuerySizeFromConfig || fastQuerySizeSysProp); + + queryEngineSettings.setStrictPathRestriction(config.getStrictPathRestrictionsForIndexes()); log.info("Initialize QueryEngine settings {}", queryEngineSettings); } Index: oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java (revision 1862608) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java (working copy) @@ -37,6 +37,7 @@ import org.apache.jackrabbit.oak.namepath.NamePathMapper; import org.apache.jackrabbit.oak.plugins.index.counter.jmx.NodeCounter; import org.apache.jackrabbit.oak.plugins.memory.PropertyValues; +import org.apache.jackrabbit.oak.plugins.observation.filter.UniversalFilter.Selector; import org.apache.jackrabbit.oak.query.QueryOptions.Traversal; import org.apache.jackrabbit.oak.query.ast.AndImpl; import org.apache.jackrabbit.oak.query.ast.AstVisitorBase; @@ -133,8 +134,17 @@ SourceImpl source; private String statement; final HashMap bindVariableMap = new HashMap(); + + /** + * The map of indexes (each selector uses one index) + */ final HashMap selectorIndexes = new HashMap(); + + /** + * The list of selectors of this query. For a join, there can be multiple selectors. + */ final ArrayList selectors = new ArrayList(); + ConstraintImpl constraint; /** @@ -492,9 +502,40 @@ return new ResultImpl(this); } + /** + * If one of the indexes wants a warning to be logged due to path mismatch, + * then get the warning message. Otherwise, return null. + * + * @return null (in the normal case) or the list of index plan names (if + * some index wants a warning to be logged) + */ + private String getWarningForPathFilterMismatch() { + StringBuilder buff = null; + for (SelectorImpl s : selectors) { + if (s.getExecutionPlan() != null && + s.getExecutionPlan().getIndexPlan() != null && + s.getExecutionPlan().getIndexPlan().logWarningForPathFilterMismatch()) { + if (buff == null) { + buff = new StringBuilder(); + } + if (buff.length() > 0) { + buff.append(", "); + } + buff.append(s.getExecutionPlan().getIndexPlanName()); + } + } + return buff == null ? null : buff.toString(); + } + @Override public Iterator getRows() { prepare(); + String warn = getWarningForPathFilterMismatch(); + if (warn != null) { + LOG.warn("Index definition of index used have path restrictions and query won't return nodes from " + + "those restricted paths; query={}, plan={}", statement, warn); + } + if (explain) { String plan = getPlan(); if (measure) { Index: oak-core/src/main/java/org/apache/jackrabbit/oak/query/plan/SelectorExecutionPlan.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/query/plan/SelectorExecutionPlan.java (revision 1862608) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/query/plan/SelectorExecutionPlan.java (working copy) @@ -17,7 +17,9 @@ package org.apache.jackrabbit.oak.query.plan; import org.apache.jackrabbit.oak.query.ast.SelectorImpl; +import org.apache.jackrabbit.oak.query.index.FilterImpl; import org.apache.jackrabbit.oak.spi.query.QueryIndex; +import org.apache.jackrabbit.oak.spi.query.QueryIndex.AdvancedQueryIndex; import org.apache.jackrabbit.oak.spi.query.QueryIndex.IndexPlan; /** @@ -56,5 +58,18 @@ public IndexPlan getIndexPlan() { return plan; } + + /** + * Get the index name, or index type (may not always be the exact index name). + * + * @return the name + */ + public String getIndexPlanName() { + if (plan != null) { + return plan.getPlanName(); + } else { + return index.getIndexName(); + } + } } Index: oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java =================================================================== --- oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java (revision 1862608) +++ oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java (working copy) @@ -86,6 +86,7 @@ import org.apache.jackrabbit.oak.InitialContentHelper; import org.apache.jackrabbit.oak.plugins.nodetype.write.NodeTypeRegistry; import org.apache.jackrabbit.oak.query.AbstractQueryTest; +import org.apache.jackrabbit.oak.query.QueryEngineSettings; import org.apache.jackrabbit.oak.spi.commit.CommitInfo; import org.apache.jackrabbit.oak.spi.commit.EmptyHook; import org.apache.jackrabbit.oak.spi.commit.Observer; @@ -173,6 +174,8 @@ private ResultCountingIndexProvider queryIndexProvider; + private QueryEngineSettings queryEngineSettings = new QueryEngineSettings(); + @After public void after() { new ExecutorCloser(executorService).close(); @@ -199,6 +202,7 @@ .with(optionalEditorProvider) .with(new PropertyIndexEditorProvider()) .with(new NodeTypeIndexProvider()) + .with(queryEngineSettings) .createContentRepository(); } @@ -865,32 +869,6 @@ assertQuery("select [jcr:path] from [nt:base] where [propa] = 10", asList("/test/a", "/test/a/b")); } - @Test - public void pathExclude() throws Exception{ - Tree idx = createIndex("test1", of("propa", "propb")); - idx.setProperty(createProperty(PROP_EXCLUDED_PATHS, of("/test/a"), Type.STRINGS)); - //Do not provide type information - root.commit(); - - Tree test = root.getTree("/").addChild("test"); - test.addChild("a").setProperty("propa", 10); - test.addChild("a").addChild("b").setProperty("propa", 10); - test.addChild("c").setProperty("propa", 10); - root.commit(); - - assertThat(explain("select [jcr:path] from [nt:base] where [propa] = 10"), containsString("lucene:test1")); - - assertQuery("select [jcr:path] from [nt:base] where [propa] = 10", asList("/test/c")); - - //Make some change and then check - test = root.getTree("/").getChild("test"); - test.addChild("a").addChild("e").setProperty("propa", 10); - test.addChild("f").setProperty("propa", 10); - root.commit(); - - assertQuery("select [jcr:path] from [nt:base] where [propa] = 10", asList("/test/c", "/test/f")); - } - //OAK-4516 @Test public void wildcardQueryToLookupUnanalyzedText() throws Exception { Index: oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneStrictPathRestrictionEnableTest.java =================================================================== --- oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneStrictPathRestrictionEnableTest.java (nonexistent) +++ oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneStrictPathRestrictionEnableTest.java (working copy) @@ -0,0 +1,244 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.jackrabbit.oak.plugins.index.lucene; + +import org.apache.commons.io.FileUtils; +import org.apache.jackrabbit.JcrConstants; +import org.apache.jackrabbit.oak.InitialContentHelper; +import org.apache.jackrabbit.oak.Oak; +import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.api.ContentRepository; +import org.apache.jackrabbit.oak.api.StrictPathRestriction; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser; +import org.apache.jackrabbit.oak.plugins.index.lucene.directory.CopyOnReadDirectory; +import org.apache.jackrabbit.oak.plugins.index.nodetype.NodeTypeIndexProvider; +import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexEditorProvider; +import org.apache.jackrabbit.oak.plugins.index.search.ExtractedTextCache; +import org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants; +import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; +import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore; +import org.apache.jackrabbit.oak.plugins.memory.PropertyStates; +import org.apache.jackrabbit.oak.query.AbstractQueryTest; +import org.apache.jackrabbit.oak.query.QueryEngineSettings; +import org.apache.jackrabbit.oak.spi.commit.Observer; +import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider; +import org.apache.jackrabbit.oak.spi.state.NodeStore; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FSDirectory; +import org.apache.lucene.store.FilterDirectory; +import org.junit.After; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; +import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import static com.google.common.collect.ImmutableSet.of; +import static java.util.Arrays.asList; +import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME; +import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE; +import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME; +import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME; +import static org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty; +import static org.apache.jackrabbit.oak.spi.filter.PathFilter.PROP_EXCLUDED_PATHS; +import static org.apache.jackrabbit.oak.spi.filter.PathFilter.PROP_INCLUDED_PATHS; +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; + +@SuppressWarnings("ArraysAsListWithZeroOrOneArgument") +public class LuceneStrictPathRestrictionEnableTest extends AbstractQueryTest { + + private ExecutorService executorService = Executors.newFixedThreadPool(2); + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(new File("target")); + + private String corDir = null; + private String cowDir = null; + + private LuceneIndexEditorProvider editorProvider; + + private TestUtil.OptionalEditorProvider optionalEditorProvider = new TestUtil.OptionalEditorProvider(); + + private NodeStore nodeStore; + + private LuceneIndexProvider provider; + + private ResultCountingIndexProvider queryIndexProvider; + + private QueryEngineSettings queryEngineSettings = new QueryEngineSettings(); + + @After + public void after() { + new ExecutorCloser(executorService).close(); + IndexDefinition.setDisableStoredIndexDefinition(false); + } + + @Override + protected void createTestIndexNode() throws Exception { + setTraversalEnabled(false); + } + + @Override + protected ContentRepository createRepository() { + IndexCopier copier = createIndexCopier(); + editorProvider = new LuceneIndexEditorProvider(copier, new ExtractedTextCache(10 * FileUtils.ONE_MB, 100)); + provider = new LuceneIndexProvider(copier); + queryIndexProvider = new ResultCountingIndexProvider(provider); + nodeStore = new MemoryNodeStore(InitialContentHelper.INITIAL_CONTENT); + queryEngineSettings.setStrictPathRestriction(StrictPathRestriction.ENABLE.name()); + return new Oak(nodeStore) + .with(new OpenSecurityProvider()) + .with(queryIndexProvider) + .with((Observer) provider) + .with(editorProvider) + .with(optionalEditorProvider) + .with(new PropertyIndexEditorProvider()) + .with(new NodeTypeIndexProvider()) + .with(queryEngineSettings) + .createContentRepository(); + } + + private IndexCopier createIndexCopier() { + try { + return new IndexCopier(executorService, temporaryFolder.getRoot()) { + @Override + public Directory wrapForRead(String indexPath, LuceneIndexDefinition definition, + Directory remote, String dirName) throws IOException { + Directory ret = super.wrapForRead(indexPath, definition, remote, dirName); + corDir = getFSDirPath(ret); + return ret; + } + + @Override + public Directory wrapForWrite(LuceneIndexDefinition definition, + Directory remote, boolean reindexMode, String dirName, + COWDirectoryTracker cowDirectoryTracker) throws IOException { + Directory ret = super.wrapForWrite(definition, remote, reindexMode, dirName, cowDirectoryTracker); + cowDir = getFSDirPath(ret); + return ret; + } + + private String getFSDirPath(Directory dir) { + if (dir instanceof CopyOnReadDirectory) { + dir = ((CopyOnReadDirectory) dir).getLocal(); + } + + dir = unwrap(dir); + + if (dir instanceof FSDirectory) { + return ((FSDirectory) dir).getDirectory().getAbsolutePath(); + } + return null; + } + + private Directory unwrap(Directory dir) { + if (dir instanceof FilterDirectory) { + return unwrap(((FilterDirectory) dir).getDelegate()); + } + return dir; + } + + }; + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @After + public void shutdownExecutor() { + executorService.shutdown(); + } + + @Test + public void pathIncludeWithPathRestrictionsEnabled() throws Exception { + + Tree idx = createIndex("test1", of("propa", "propb")); + idx.setProperty(createProperty(PROP_INCLUDED_PATHS, of("/test/a"), Type.STRINGS)); + root.commit(); + + Tree test = root.getTree("/").addChild("test"); + test.addChild("a").setProperty("propa", 10); + test.addChild("a").addChild("b").setProperty("propa", 10); + test.addChild("c").setProperty("propa", 10); + root.commit(); + + assertFalse(explain("select [jcr:path] from [nt:base] where [propa] = 10").contains("lucene:test1")); + assertThat(explain("select [jcr:path] from [nt:base] where [propa] = 10 and isDescendantNode('/test/a')"), containsString("lucene:test1")); + assertQuery("select [jcr:path] from [nt:base] where [propa] = 10 and isDescendantNode('/test/a')", asList("/test/a/b")); + + } + + @Test + public void pathExcludeWithPathRestrictionsEnabled() throws Exception { + Tree idx = createIndex("test1", of("propa", "propb")); + idx.setProperty(createProperty(PROP_EXCLUDED_PATHS, of("/test/a"), Type.STRINGS)); + root.commit(); + + Tree test = root.getTree("/").addChild("test"); + test.addChild("a").setProperty("propa", 10); + test.addChild("a").addChild("b").setProperty("propa", 10); + test.addChild("c").setProperty("propa", 10); + test.addChild("c").addChild("d").setProperty("propa", 10); + root.commit(); + + assertFalse(explain("select [jcr:path] from [nt:base] where [propa] = 10").contains("lucene:test1")); + assertThat(explain("select [jcr:path] from [nt:base] where [propa] = 10 and isDescendantNode('/test/c')"), containsString("lucene:test1")); + + assertQuery("select [jcr:path] from [nt:base] where [propa] = 10 and isDescendantNode('/test/c')", asList("/test/c/d")); + + //Make some change and then check + Tree testc = root.getTree("/").getChild("test").getChild("c"); + testc.addChild("e").addChild("f").setProperty("propa", 10); + root.commit(); + assertQuery("select [jcr:path] from [nt:base] where [propa] = 10 and isDescendantNode('/test/c')", asList("/test/c/d", "/test/c/e/f")); + assertThat(explain("select [jcr:path] from [nt:base] where [propa] = 10 and isDescendantNode('/test/c') and not(isDescendantNode('/test/c/e'))"), containsString("lucene:test1")); + assertQuery("select [jcr:path] from [nt:base] where [propa] = 10 and isDescendantNode('/test/c') and not(isDescendantNode('/test/c/e'))", asList("/test/c/d")); + } + + private String explain(String query) { + String explain = "explain " + query; + return executeQuery(explain, "JCR-SQL2").get(0); + } + + private Tree createIndex(String name, Set propNames) throws CommitFailedException { + Tree index = root.getTree("/"); + return createIndex(index, name, propNames); + } + + public static Tree createIndex(Tree index, String name, Set propNames) throws CommitFailedException { + Tree def = index.addChild(INDEX_DEFINITIONS_NAME).addChild(name); + def.setProperty(JcrConstants.JCR_PRIMARYTYPE, + INDEX_DEFINITIONS_NODE_TYPE, Type.NAME); + def.setProperty(TYPE_PROPERTY_NAME, LuceneIndexConstants.TYPE_LUCENE); + def.setProperty(REINDEX_PROPERTY_NAME, true); + def.setProperty(FulltextIndexConstants.FULL_TEXT_ENABLED, false); + def.setProperty(PropertyStates.createProperty(FulltextIndexConstants.INCLUDE_PROPERTY_NAMES, propNames, Type.STRINGS)); + def.setProperty(LuceneIndexConstants.SAVE_DIR_LISTING, true); + return index.getChild(INDEX_DEFINITIONS_NAME).getChild(name); + } + +} Index: oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneStrictPathRestrictionWarnTest.java =================================================================== --- oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneStrictPathRestrictionWarnTest.java (nonexistent) +++ oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneStrictPathRestrictionWarnTest.java (working copy) @@ -0,0 +1,280 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.jackrabbit.oak.plugins.index.lucene; + +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import org.apache.commons.io.FileUtils; +import org.apache.jackrabbit.JcrConstants; +import org.apache.jackrabbit.oak.InitialContentHelper; +import org.apache.jackrabbit.oak.Oak; +import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.api.ContentRepository; +import org.apache.jackrabbit.oak.api.StrictPathRestriction; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser; +import org.apache.jackrabbit.oak.plugins.index.lucene.directory.CopyOnReadDirectory; +import org.apache.jackrabbit.oak.plugins.index.nodetype.NodeTypeIndexProvider; +import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexEditorProvider; +import org.apache.jackrabbit.oak.plugins.index.search.ExtractedTextCache; +import org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants; +import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; +import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore; +import org.apache.jackrabbit.oak.plugins.memory.PropertyStates; +import org.apache.jackrabbit.oak.query.AbstractQueryTest; +import org.apache.jackrabbit.oak.query.QueryEngineSettings; +import org.apache.jackrabbit.oak.spi.commit.Observer; +import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider; +import org.apache.jackrabbit.oak.spi.state.NodeStore; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FSDirectory; +import org.apache.lucene.store.FilterDirectory; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.io.IOException; +import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import static com.google.common.collect.ImmutableSet.of; +import static java.util.Arrays.asList; +import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME; +import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE; +import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME; +import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME; +import static org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty; +import static org.apache.jackrabbit.oak.spi.filter.PathFilter.PROP_EXCLUDED_PATHS; +import static org.apache.jackrabbit.oak.spi.filter.PathFilter.PROP_INCLUDED_PATHS; +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +@SuppressWarnings("ArraysAsListWithZeroOrOneArgument") +public class LuceneStrictPathRestrictionWarnTest extends AbstractQueryTest { + + private ExecutorService executorService = Executors.newFixedThreadPool(2); + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(new File("target")); + + ListAppender listAppender = null; + + private String corDir = null; + private String cowDir = null; + + private LuceneIndexEditorProvider editorProvider; + + private TestUtil.OptionalEditorProvider optionalEditorProvider = new TestUtil.OptionalEditorProvider(); + + private NodeStore nodeStore; + + private LuceneIndexProvider provider; + + private ResultCountingIndexProvider queryIndexProvider; + + private QueryEngineSettings queryEngineSettings = new QueryEngineSettings(); + + private final String warnMessage = "Index definition of index used have path restrictions and query won't return nodes from " + + "those restricted paths"; + + private final String queryImplLogger = "org.apache.jackrabbit.oak.query.QueryImpl"; + + @Before + public void loggingAppenderStart() { + LoggerContext context = (LoggerContext) LoggerFactory.getILoggerFactory(); + listAppender = new ListAppender<>(); + listAppender.start(); + context.getLogger(queryImplLogger).addAppender(listAppender); + } + + @After + public void loggingAppenderStop() { + listAppender.stop(); + } + + @After + public void after() { + new ExecutorCloser(executorService).close(); + IndexDefinition.setDisableStoredIndexDefinition(false); + } + + @Override + protected void createTestIndexNode() throws Exception { + setTraversalEnabled(false); + } + + @Override + protected ContentRepository createRepository() { + IndexCopier copier = createIndexCopier(); + editorProvider = new LuceneIndexEditorProvider(copier, new ExtractedTextCache(10 * FileUtils.ONE_MB, 100)); + provider = new LuceneIndexProvider(copier); + queryIndexProvider = new ResultCountingIndexProvider(provider); + nodeStore = new MemoryNodeStore(InitialContentHelper.INITIAL_CONTENT); + queryEngineSettings.setStrictPathRestriction(StrictPathRestriction.WARN.name()); + return new Oak(nodeStore) + .with(new OpenSecurityProvider()) + .with(queryIndexProvider) + .with((Observer) provider) + .with(editorProvider) + .with(optionalEditorProvider) + .with(new PropertyIndexEditorProvider()) + .with(new NodeTypeIndexProvider()) + .with(queryEngineSettings) + .createContentRepository(); + } + + private IndexCopier createIndexCopier() { + try { + return new IndexCopier(executorService, temporaryFolder.getRoot()) { + @Override + public Directory wrapForRead(String indexPath, LuceneIndexDefinition definition, + Directory remote, String dirName) throws IOException { + Directory ret = super.wrapForRead(indexPath, definition, remote, dirName); + corDir = getFSDirPath(ret); + return ret; + } + + @Override + public Directory wrapForWrite(LuceneIndexDefinition definition, + Directory remote, boolean reindexMode, String dirName, + COWDirectoryTracker cowDirectoryTracker) throws IOException { + Directory ret = super.wrapForWrite(definition, remote, reindexMode, dirName, cowDirectoryTracker); + cowDir = getFSDirPath(ret); + return ret; + } + + private String getFSDirPath(Directory dir) { + if (dir instanceof CopyOnReadDirectory) { + dir = ((CopyOnReadDirectory) dir).getLocal(); + } + + dir = unwrap(dir); + + if (dir instanceof FSDirectory) { + return ((FSDirectory) dir).getDirectory().getAbsolutePath(); + } + return null; + } + + private Directory unwrap(Directory dir) { + if (dir instanceof FilterDirectory) { + return unwrap(((FilterDirectory) dir).getDelegate()); + } + return dir; + } + + }; + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @After + public void shutdownExecutor() { + executorService.shutdown(); + } + + @Test + public void pathIncludeWithPathRestrictionsWarn() throws Exception { + + Tree idx = createIndex("test1", of("propa", "propb")); + idx.setProperty(createProperty(PROP_INCLUDED_PATHS, of("/test/a"), Type.STRINGS)); + //Do not provide type information + root.commit(); + + Tree test = root.getTree("/").addChild("test"); + test.addChild("a").setProperty("propa", 10); + test.addChild("a").addChild("b").setProperty("propa", 10); + test.addChild("c").setProperty("propa", 10); + root.commit(); + + assertThat(explain("select [jcr:path] from [nt:base] where [propa] = 10 and isDescendantNode('/test/a')"), containsString("lucene:test1")); + assertQuery("select [jcr:path] from [nt:base] where [propa] = 10 and isDescendantNode('/test/a')", asList("/test/a/b")); + // List appender should not have any warn logs as we are searching under right descendant as per path restrictions + assertFalse(isWarnMessagePresent(listAppender)); + assertTrue(explain("select [jcr:path] from [nt:base] where [propa] = 10").contains("lucene:test1")); + // List appender now will have warn log as we are searching under root(/) but index definition have include path restriction. + assertTrue(isWarnMessagePresent(listAppender)); + } + + @Test + public void pathExcludeWithPathRestrictionsWarn() throws Exception { + Tree idx = createIndex("test1", of("propa", "propb")); + idx.setProperty(createProperty(PROP_EXCLUDED_PATHS, of("/test/a"), Type.STRINGS)); + //Do not provide type information + root.commit(); + + Tree test = root.getTree("/").addChild("test"); + test.addChild("a").setProperty("propa", 10); + test.addChild("a").addChild("b").setProperty("propa", 10); + test.addChild("c").setProperty("propa", 10); + test.addChild("c").addChild("d").setProperty("propa", 10); + root.commit(); + + assertThat(explain("select [jcr:path] from [nt:base] where [propa] = 10 and isDescendantNode('/test/c')"), containsString("lucene:test1")); + assertQuery("select [jcr:path] from [nt:base] where [propa] = 10 and isDescendantNode('/test/c')", asList("/test/c/d")); + // List appender should not have any warn logs as we are searching under right descendant as per path restrictions + assertFalse(isWarnMessagePresent(listAppender)); + assertTrue(explain("select [jcr:path] from [nt:base] where [propa] = 10").contains("lucene:test1")); + // List appender now will have warn log as we are searching under root(/) but index definition have exclude path restriction. + assertTrue(isWarnMessagePresent(listAppender)); + } + + private boolean isWarnMessagePresent(ListAppender listAppender) { + for (ILoggingEvent loggingEvent : listAppender.list) { + if (loggingEvent.getMessage().contains(warnMessage)) { +System.out.println(loggingEvent.toString()); + return true; + } + } + return false; + } + + private String explain(String query) { + String explain = "explain " + query; + return executeQuery(explain, "JCR-SQL2").get(0); + } + + private Tree createIndex(String name, Set propNames) throws CommitFailedException { + Tree index = root.getTree("/"); + return createIndex(index, name, propNames); + } + + public static Tree createIndex(Tree index, String name, Set propNames) throws CommitFailedException { + Tree def = index.addChild(INDEX_DEFINITIONS_NAME).addChild(name); + def.setProperty(JcrConstants.JCR_PRIMARYTYPE, + INDEX_DEFINITIONS_NODE_TYPE, Type.NAME); + def.setProperty(TYPE_PROPERTY_NAME, LuceneIndexConstants.TYPE_LUCENE); + def.setProperty(REINDEX_PROPERTY_NAME, true); + def.setProperty(FulltextIndexConstants.FULL_TEXT_ENABLED, false); + def.setProperty(PropertyStates.createProperty(FulltextIndexConstants.INCLUDE_PROPERTY_NAMES, propNames, Type.STRINGS)); + def.setProperty(LuceneIndexConstants.SAVE_DIR_LISTING, true); + return index.getChild(INDEX_DEFINITIONS_NAME).getChild(name); + } + +} Index: oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/QueryIndex.java =================================================================== --- oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/QueryIndex.java (revision 1862608) +++ oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/QueryIndex.java (working copy) @@ -348,6 +348,10 @@ * @return if it is deprecated */ boolean isDeprecated(); + + default boolean logWarningForPathFilterMismatch() { + return false; + } /** * A builder for index plans. @@ -369,6 +373,7 @@ protected Map attributes = Maps.newHashMap(); protected String planName; protected boolean deprecated; + protected boolean logWarningForPathFilterMismatch; public Builder setCostPerExecution(double costPerExecution) { this.costPerExecution = costPerExecution; @@ -394,6 +399,11 @@ this.isDelayed = isDelayed; return this; } + + public Builder setLogWarningForPathFilterMismatch(boolean value) { + this.logWarningForPathFilterMismatch = value; + return this; + } public Builder setFulltextIndex(boolean isFulltextIndex) { this.isFulltextIndex = isFulltextIndex; @@ -480,6 +490,7 @@ private final String planName = Builder.this.planName; private final boolean deprecated = Builder.this.deprecated; + private final boolean logWarningForPathFilterMismatch = Builder.this.logWarningForPathFilterMismatch; @Override public String toString() { @@ -496,7 +507,8 @@ + " propertyRestriction : %s," + " pathPrefix : %s," + " deprecated : %s," - + " supportsPathRestriction : %s }", + + " supportsPathRestriction : %s," + + " logWarningForPathFilterMismatch : %s }", costPerExecution, costPerEntry, estimatedEntryCount, @@ -509,7 +521,8 @@ propRestriction, pathPrefix, deprecated, - supportsPathRestriction + supportsPathRestriction, + logWarningForPathFilterMismatch ); } @@ -606,6 +619,11 @@ public boolean isDeprecated() { return deprecated; } + + @Override + public boolean logWarningForPathFilterMismatch() { + return logWarningForPathFilterMismatch; + } }; } Index: oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/QueryLimits.java =================================================================== --- oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/QueryLimits.java (revision 1862608) +++ oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/QueryLimits.java (working copy) @@ -18,6 +18,8 @@ */ package org.apache.jackrabbit.oak.spi.query; +import org.apache.jackrabbit.oak.api.StrictPathRestriction; + public interface QueryLimits { long getLimitInMemory(); @@ -28,4 +30,8 @@ boolean getFailTraversal(); + default String getStrictPathRestriction() { + return StrictPathRestriction.DISABLE.name(); + } + } Index: oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java =================================================================== --- oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java (revision 1862608) +++ oak-query-spi/src/main/java/org/apache/jackrabbit/oak/spi/query/package-info.java (working copy) @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -@Version("1.2.0") +@Version("1.3.0") package org.apache.jackrabbit.oak.spi.query; import org.osgi.annotation.versioning.Version; Index: oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java =================================================================== --- oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java (revision 1862608) +++ oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java (working copy) @@ -34,6 +34,7 @@ import com.google.common.collect.Multimap; import org.apache.jackrabbit.JcrConstants; import org.apache.jackrabbit.oak.api.PropertyValue; +import org.apache.jackrabbit.oak.api.StrictPathRestriction; import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.plugins.index.IndexConstants; @@ -44,6 +45,7 @@ import org.apache.jackrabbit.oak.plugins.index.search.IndexNode; import org.apache.jackrabbit.oak.plugins.index.search.IndexStatistics; import org.apache.jackrabbit.oak.plugins.index.search.PropertyDefinition; +import org.apache.jackrabbit.oak.spi.filter.PathFilter; import org.apache.jackrabbit.oak.spi.query.Filter; import org.apache.jackrabbit.oak.spi.query.Filter.PropertyRestriction; import org.apache.jackrabbit.oak.spi.query.QueryConstants; @@ -152,6 +154,9 @@ if (wrongIndex()) { return null; } + if (filter.getQueryLimits().getStrictPathRestriction().equals(StrictPathRestriction.ENABLE.name()) && !isPlanWithValidPathFilter()) { + return null; + } FullTextExpression ft = filter.getFullTextConstraint(); @@ -286,6 +291,11 @@ costPerEntryFactor += sortOrder.size(); IndexPlan.Builder plan = defaultPlan(); + + if (filter.getQueryLimits().getStrictPathRestriction().equals(StrictPathRestriction.WARN.name()) && !isPlanWithValidPathFilter()) { + plan.setLogWarningForPathFilterMismatch(true); + } + if (plan == null) { return null; } @@ -333,6 +343,12 @@ return null; } + private boolean isPlanWithValidPathFilter() { + String pathFilter = filter.getPath(); + PathFilter definitionPathFilter = definition.getPathFilter(); + return definitionPathFilter.areAllDescendantsIncluded(pathFilter); + } + private boolean matchesValuePattern(PropertyRestriction pr, PropertyDefinition pd) { if (!pd.valuePattern.matchesAll()){ //So we have a valuePattern defined. So determine if