diff --git a/oak-doc/src/site/markdown/query/elastic.md b/oak-doc/src/site/markdown/query/elastic.md index 867470e505b..2f6669d3208 100644 --- a/oak-doc/src/site/markdown/query/elastic.md +++ b/oak-doc/src/site/markdown/query/elastic.md @@ -33,9 +33,8 @@ however there are differences: * Indexes are NOT automatically built when needed: They can be built by setting the `reindex` property to `true` or by using the `oak-run` tool. We recommend to build them using the `oak-run` tool. -* `evaluatePathRestrictions` is only checked at query time (to keep the compatibility with Lucene). The parent paths are - always indexed. Changing this flag won't require a reindex then. It's strongly suggested to enable it. This control - might be removed in the future. +* `evaluatePathRestrictions` cannot be disabled. The parent paths are always indexed. Queries with path restrictions are + evaluated at index level when possible, otherwise they are evaluated at repository level. * `codec` is ignored. * `compatVersion` is ignored. * `useIfExists` is ignored. diff --git a/oak-search-elastic/pom.xml b/oak-search-elastic/pom.xml index a0fd53acc4a..277d451a979 100644 --- a/oak-search-elastic/pom.xml +++ b/oak-search-elastic/pom.xml @@ -101,6 +101,11 @@ org.apache.felix.scr.annotations provided + + org.osgi + org.osgi.annotation.versioning + provided + org.osgi org.osgi.service.component.annotations diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDefinition.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDefinition.java index 82d0374fdd4..95b3290bfea 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDefinition.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDefinition.java @@ -59,6 +59,9 @@ public class ElasticIndexDefinition extends IndexDefinition { public static final String QUERY_FETCH_SIZES = "queryFetchSizes"; public static final Long[] QUERY_FETCH_SIZES_DEFAULT = new Long[]{10L, 100L, 1000L}; + public static final String QUERY_TIMEOUT_MS = "queryTimeoutMs"; + public static final long QUERY_TIMEOUT_MS_DEFAULT = 60000; + public static final String TRACK_TOTAL_HITS = "trackTotalHits"; public static final Integer TRACK_TOTAL_HITS_DEFAULT = 10000; @@ -138,6 +141,7 @@ public class ElasticIndexDefinition extends IndexDefinition { public final int numberOfShards; public final int numberOfReplicas; public final int[] queryFetchSizes; + public final long queryTimeoutMs; public final Integer trackTotalHits; public final String dynamicMapping; public final boolean failOnError; @@ -162,6 +166,7 @@ public ElasticIndexDefinition(NodeState root, NodeState defn, String indexPath, this.similarityTagsBoost = getOptionalValue(defn, SIMILARITY_TAGS_BOOST, SIMILARITY_TAGS_BOOST_DEFAULT); this.queryFetchSizes = Arrays.stream(getOptionalValues(defn, QUERY_FETCH_SIZES, Type.LONGS, Long.class, QUERY_FETCH_SIZES_DEFAULT)) .mapToInt(Long::intValue).toArray(); + this.queryTimeoutMs = getOptionalValue(defn, QUERY_TIMEOUT_MS, QUERY_TIMEOUT_MS_DEFAULT); this.trackTotalHits = getOptionalValue(defn, TRACK_TOTAL_HITS, TRACK_TOTAL_HITS_DEFAULT); this.dynamicMapping = getOptionalValue(defn, DYNAMIC_MAPPING, DYNAMIC_MAPPING_DEFAULT); this.failOnError = getOptionalValue(defn, FAIL_ON_ERROR, diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java index f04d38c2d24..2935ec45ce9 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java @@ -596,49 +596,47 @@ private List nonFullTextConstraints(IndexPlan plan, PlanResult planResult nodeTypeConstraints.ifPresent(queries::add); } - if (elasticIndexDefinition.evaluatePathRestrictions()) { - String path = FulltextIndex.getPathRestriction(plan); - switch (filter.getPathRestriction()) { - case ALL_CHILDREN: - if (!"/".equals(path)) { - queries.add(newAncestorQuery(path)); + String path = FulltextIndex.getPathRestriction(plan); + switch (filter.getPathRestriction()) { + case ALL_CHILDREN: + if (!"/".equals(path)) { + queries.add(newAncestorQuery(path)); + } + break; + case DIRECT_CHILDREN: + queries.add(Query.of(q -> q.bool(b -> b.must(newAncestorQuery(path)).must(newDepthQuery(path, planResult))))); + break; + case EXACT: + // For transformed paths, we can only add path restriction if absolute path to property can be deduced + if (planResult.isPathTransformed()) { + String parentPathSegment = planResult.getParentPathSegment(); + if (!any.test(PathUtils.elements(parentPathSegment), "*")) { + queries.add(newPathQuery(path + parentPathSegment)); } - break; - case DIRECT_CHILDREN: - queries.add(Query.of(q -> q.bool(b -> b.must(newAncestorQuery(path)).must(newDepthQuery(path, planResult))))); - break; - case EXACT: + } else { + queries.add(newPathQuery(path)); + } + break; + case PARENT: + if (PathUtils.denotesRoot(path)) { + // there's no parent of the root node + // we add a path that can not possibly occur because there + // is no way to say "match no documents" in Lucene + queries.add(newPathQuery("///")); + } else { // For transformed paths, we can only add path restriction if absolute path to property can be deduced if (planResult.isPathTransformed()) { String parentPathSegment = planResult.getParentPathSegment(); if (!any.test(PathUtils.elements(parentPathSegment), "*")) { - queries.add(newPathQuery(path + parentPathSegment)); + queries.add(newPathQuery(PathUtils.getParentPath(path) + parentPathSegment)); } } else { - queries.add(newPathQuery(path)); + queries.add(newPathQuery(PathUtils.getParentPath(path))); } - break; - case PARENT: - if (PathUtils.denotesRoot(path)) { - // there's no parent of the root node - // we add a path that can not possibly occur because there - // is no way to say "match no documents" in Lucene - queries.add(newPathQuery("///")); - } else { - // For transformed paths, we can only add path restriction if absolute path to property can be deduced - if (planResult.isPathTransformed()) { - String parentPathSegment = planResult.getParentPathSegment(); - if (!any.test(PathUtils.elements(parentPathSegment), "*")) { - queries.add(newPathQuery(PathUtils.getParentPath(path) + parentPathSegment)); - } - } else { - queries.add(newPathQuery(PathUtils.getParentPath(path))); - } - } - break; - case NO_RESTRICTION: - break; - } + } + break; + case NO_RESTRICTION: + break; } for (Filter.PropertyRestriction pr : filter.getPropertyRestrictions()) { diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/ElasticResponseListener.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/ElasticResponseListener.java index b49875a729a..f3a770f6fec 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/ElasticResponseListener.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/ElasticResponseListener.java @@ -20,17 +20,18 @@ import co.elastic.clients.elasticsearch.core.search.Hit; import com.fasterxml.jackson.databind.node.ObjectNode; import org.apache.jackrabbit.oak.plugins.index.search.FieldNames; +import org.osgi.annotation.versioning.ProviderType; -import java.util.Collections; import java.util.Map; import java.util.Set; /** * Generic listener of Elastic response */ +@ProviderType public interface ElasticResponseListener { - Set DEFAULT_SOURCE_FIELDS = Collections.singleton(FieldNames.PATH); + Set DEFAULT_SOURCE_FIELDS = Set.of(FieldNames.PATH); /** * Returns the source fields this listener is interested on @@ -68,8 +69,9 @@ default void startData(long totalHits) { /*empty*/ } /** * This method is called for each {@link Hit} retrieved * @param searchHit a search result + * @return true if the search hit was successfully processed, false otherwise */ - void on(Hit searchHit); + boolean on(Hit searchHit); } /** diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/ElasticResultRowAsyncIterator.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/ElasticResultRowAsyncIterator.java index b350762fe85..b0da1da4f78 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/ElasticResultRowAsyncIterator.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/ElasticResultRowAsyncIterator.java @@ -41,6 +41,7 @@ import org.slf4j.LoggerFactory; import java.util.ArrayList; +import java.util.BitSet; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -48,6 +49,7 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -69,7 +71,7 @@ public class ElasticResultRowAsyncIterator implements ElasticQueryIterator, Elas private static final FulltextResultRow POISON_PILL = new FulltextResultRow("___OAK_POISON_PILL___", 0d, Collections.emptyMap(), null, null); - private final BlockingQueue queue = new LinkedBlockingQueue<>(); + private final BlockingQueue queue; private final ElasticIndexNode indexNode; private final IndexPlan indexPlan; @@ -96,6 +98,9 @@ public ElasticResultRowAsyncIterator(@NotNull ElasticIndexNode indexNode, this.rowInclusionPredicate = rowInclusionPredicate; this.metricHandler = metricHandler; this.elasticFacetProvider = elasticRequestHandler.getAsyncFacetProvider(indexNode.getConnection(), elasticResponseHandler); + // set the queue size to the limit of the query. This is to avoid to load too many results in memory in case the + // consumer is slow to process them + this.queue = new LinkedBlockingQueue<>((int) indexPlan.getFilter().getQueryLimits().getLimitReads()); this.elasticQueryScanner = initScanner(); } @@ -108,7 +113,7 @@ public boolean hasNext() { elasticQueryScanner.scan(); } try { - nextRow = queue.take(); + nextRow = queue.poll(indexNode.getDefinition().queryTimeoutMs, TimeUnit.MILLISECONDS); } catch (InterruptedException e) { Thread.currentThread().interrupt(); // restore interrupt status throw new IllegalStateException("Error reading next result from Elastic", e); @@ -144,12 +149,12 @@ public FulltextResultRow next() { } @Override - public void on(Hit searchHit) { + public boolean on(Hit searchHit) { final String path = elasticResponseHandler.getPath(searchHit); if (path != null) { if (rowInclusionPredicate != null && !rowInclusionPredicate.test(path)) { LOG.trace("Path {} not included because of hierarchy inclusion rules", path); - return; + return false; } LOG.trace("Path {} satisfies hierarchy inclusion rules", path); try { @@ -159,7 +164,9 @@ public void on(Hit searchHit) { Thread.currentThread().interrupt(); // restore interrupt status throw new IllegalStateException("Error producing results into the iterator queue", e); } + return true; } + return false; } @Override @@ -333,16 +340,25 @@ public void onSuccess(SearchResponse searchResponse) { } LOG.trace("Emitting {} search hits, for a total of {} scanned results", searchHits.size(), scannedRows); + + BitSet listenersWithHits = new BitSet(searchHitListeners.size()); + for (Hit hit : searchHits) { - for (SearchHitListener l : searchHitListeners) { - l.on(hit); + for (int index = 0; index < searchHitListeners.size(); index++) { + SearchHitListener l = searchHitListeners.get(index); + if (l.on(hit)) { + listenersWithHits.set(index); + } } } + // if any listener has not processed any hit, it means we need to load more data since there could be + // listeners waiting for some results before triggering a new scan + boolean areAllListenersProcessed = listenersWithHits.cardinality() == searchHitListeners.size(); if (!anyDataLeft.get()) { LOG.trace("No data left: closing scanner, notifying listeners"); close(); - } else if (fullScan) { + } else if (fullScan || !areAllListenersProcessed) { scan(); } } else { diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/facets/ElasticSecureFacetAsyncProvider.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/facets/ElasticSecureFacetAsyncProvider.java index a02a6afcb17..dec588c62da 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/facets/ElasticSecureFacetAsyncProvider.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/async/facets/ElasticSecureFacetAsyncProvider.java @@ -71,7 +71,7 @@ public boolean isFullScan() { } @Override - public void on(Hit searchHit) { + public boolean on(Hit searchHit) { final String path = elasticResponseHandler.getPath(searchHit); if (path != null && isAccessible.test(path)) { for (String field: facetFields) { @@ -90,6 +90,7 @@ public void on(Hit searchHit) { } } } + return true; } @Override diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDescendantSpellcheckCommonTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDescendantSpellcheckCommonTest.java index a428a54e0ca..eb2e0c60fc0 100644 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDescendantSpellcheckCommonTest.java +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDescendantSpellcheckCommonTest.java @@ -20,6 +20,8 @@ import org.apache.jackrabbit.oak.jcr.Jcr; import org.apache.jackrabbit.oak.plugins.index.IndexDescendantSpellcheckCommonTest; import org.junit.ClassRule; +import org.junit.Ignore; +import org.junit.Test; import javax.jcr.Repository; @@ -38,4 +40,10 @@ protected Repository createJcrRepository() { return jcr.createRepository(); } + @Override + @Test + @Ignore("OAK-10617: path restrictions are always applied. This test is not applicable for Elastic") + public void descendantSuggestionRequirePathRestrictionIndex() throws Exception { + super.descendantSuggestionRequirePathRestrictionIndex(); + } } diff --git a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexPathRestrictionCommonTest.java b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexPathRestrictionCommonTest.java index f28c59bb78a..be2e2203b3d 100644 --- a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexPathRestrictionCommonTest.java +++ b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/IndexPathRestrictionCommonTest.java @@ -16,8 +16,6 @@ */ package org.apache.jackrabbit.oak.plugins.index; -import org.apache.jackrabbit.guava.common.collect.ImmutableSet; -import org.apache.jackrabbit.guava.common.collect.Sets; import org.apache.jackrabbit.oak.InitialContentHelper; import org.apache.jackrabbit.oak.commons.junit.LogCustomizer; import org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndex; @@ -43,13 +41,13 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; -import static org.apache.jackrabbit.guava.common.collect.ImmutableSet.of; import static org.apache.jackrabbit.JcrConstants.NT_BASE; import static org.junit.Assert.assertEquals; @@ -86,10 +84,26 @@ public void setup() throws Exception { commit(); } + @Test + public void restrictedPathOnLargeDataset() throws Exception { + setupTestData(evaluatePathRestrictionsInIndex); + + // create a significant number of nodes under /a + NodeBuilder aRootBuilder = rootBuilder.child("a"); + for (int i = 0; i < 1000; i++) { + aRootBuilder.child("a" + i).child("j:c").setProperty("foo", "bar"); + } + commit(); + + FilterImpl f = createFilter(root, NT_BASE); + f.restrictProperty("j:c/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test", Filter.PathRestriction.DIRECT_CHILDREN); + validateResult(f, Set.of("/test/a"), null); + } + @Test public void pathTransformationWithNoPathRestriction() throws Exception { setupTestData(evaluatePathRestrictionsInIndex); - FilterImpl f; // NOTE : The expected results here might seem "incorrect" // for example : /test doesn't really satisfy the condition j:c/@foo=bar but still is in the result set @@ -98,7 +112,7 @@ public void pathTransformationWithNoPathRestriction() throws Exception { // Refer FullTextIndexCommonTest#pathTransformationsWithNoPathRestrictions for seeing the e2e behaviour when executing an xpath query. // //*[j:c/foo = 'bar'] -> foo:bar -> transform 1 level up - f = createFilter(root, NT_BASE); + FilterImpl f = createFilter(root, NT_BASE); f.restrictProperty("j:c/foo", Operator.EQUAL, PropertyValues.newString("bar")); Map expectedMap = new LinkedHashMap<>(); expectedMap.put("/test/a", true); @@ -107,12 +121,12 @@ public void pathTransformationWithNoPathRestriction() throws Exception { expectedMap.put("/tmp/a", true); expectedMap.put("/tmp", true); expectedMap.put("/tmp/c/d", true); - validateResult(f, ImmutableSet.of("/test", "/test/a", "/test/c/d", "/tmp", "/tmp/a", "/tmp/c/d"), expectedMap); + validateResult(f, Set.of("/test", "/test/a", "/test/c/d", "/tmp", "/tmp/a", "/tmp/c/d"), expectedMap); // //*[*/foo = 'bar'] -> foo:bar -> transform 1 level up f = createFilter(root, NT_BASE); f.restrictProperty("*/foo", Operator.EQUAL, PropertyValues.newString("bar")); - validateResult(f, ImmutableSet.of("/test", "/test/a", "/test/c/d", "/tmp", "/tmp/a", "/tmp/c/d"), expectedMap); + validateResult(f, Set.of("/test", "/test/a", "/test/c/d", "/tmp", "/tmp/a", "/tmp/c/d"), expectedMap); // //*[d/*/foo = 'bar'] -> foo:bar -> transform 2 level up f = createFilter(root, NT_BASE); @@ -123,10 +137,9 @@ public void pathTransformationWithNoPathRestriction() throws Exception { expectedMap2.put("/test/c", true); expectedMap2.put("/tmp", true); expectedMap2.put("/tmp/c", true); - validateResult(f, of("/", "/test", "/test/c", "/tmp", "/tmp/c"), expectedMap2); + validateResult(f, Set.of("/", "/test", "/test/c", "/tmp", "/tmp/c"), expectedMap2); } - @Test public void entryCountWithNoPathRestriction() throws Exception { IndexDefinitionBuilder idxBuilder = @@ -144,10 +157,8 @@ public void entryCountWithNoPathRestriction() throws Exception { } commit(); - FilterImpl f; - // //*[foo = 'bar'] - f = createFilter(root, NT_BASE); + FilterImpl f = createFilter(root, NT_BASE); f.restrictProperty("foo", Operator.EQUAL, PropertyValues.newString("bar")); // equality check: we assume FulltextIndexPlanner#DEFAULT_PROPERTY_WEIGHT different values int cost = count / FulltextIndexPlanner.DEFAULT_PROPERTY_WEIGHT; @@ -183,11 +194,9 @@ public void pathTransformationWithAllChildrenPathRestriction() throws Exception testRootBuilder.child("d").child("e").child("j:c").setProperty("foo", "bar"); commit(); - FilterImpl f; - // Validation 1 // /jcr:root/test//*[j:c/foo = 'bar'] -> foo:bar :ancestors:/test -> transform 1 level up - f = createFilter(root, NT_BASE); + FilterImpl f = createFilter(root, NT_BASE); f.restrictProperty("j:c/foo", Operator.EQUAL, PropertyValues.newString("bar")); f.restrictPath("/test", Filter.PathRestriction.ALL_CHILDREN); @@ -212,7 +221,7 @@ public void pathTransformationWithAllChildrenPathRestriction() throws Exception expectedMap1.put("/tmp/c/d", false); expectedMap1.put("/test/d/e", true); } - validateResult(f, of("/test/a", "/test/c/d", "/test/d/e"), expectedMap1); + validateResult(f, Set.of("/test/a", "/test/c/d", "/test/d/e"), expectedMap1); // Validation 2 // /jcr:root/test//*[*/foo = 'bar'] -> foo:bar :ancestors:/test -> transform 1 level up @@ -241,7 +250,7 @@ public void pathTransformationWithAllChildrenPathRestriction() throws Exception expectedMap2.put("/tmp/c/d", false); expectedMap2.put("/test/d/e", true); } - validateResult(f, of("/test/a", "/test/c/d", "/test/d/e"), expectedMap2); + validateResult(f, Set.of("/test/a", "/test/c/d", "/test/d/e"), expectedMap2); // Validation 3 // /jcr:root/test//*[d/*/foo = 'bar'] -> foo:bar :ancestors:/test -> transform 2 level up @@ -277,7 +286,7 @@ public void pathTransformationWithAllChildrenPathRestriction() throws Exception // This is handled later in the QueryEngine and the final result // of query like /jcr:root/test//*[d/*/foo = 'bar'] will not contain /test/d and only return /test/c. // This test is demonstrated at QueryEngine level in FullTextIndexCommonTest#pathTransformationsWithPathRestrictions - validateResult(f, of("/test/c", "/test/d"), expectedMap3); + validateResult(f, Set.of("/test/c", "/test/d"), expectedMap3); // Validation 4 // /jcr:root/test//*[foo = 'bar'] -> foo:bar :ancestors:/test -> no transformation @@ -304,16 +313,15 @@ public void pathTransformationWithAllChildrenPathRestriction() throws Exception expectedMap4.put("/tmp/c/d/j:c", true); expectedMap4.put("/test/d/e/j:c", false); } - validateResult(f, of("/tmp/a/j:c", "/tmp/b", "/tmp/c/d/j:c"), expectedMap4); + validateResult(f, Set.of("/tmp/a/j:c", "/tmp/b", "/tmp/c/d/j:c"), expectedMap4); } @Test public void pathTransformationWithDirectChildrenPathRestriction() throws Exception { setupTestData(evaluatePathRestrictionsInIndex); - FilterImpl f; // /jcr:root/test/*[j:c/foo = 'bar'] -> foo:bar :ancestors:/test :depth:[3 TO 3] -> transform 1 level up - f = createFilter(root, NT_BASE); + FilterImpl f = createFilter(root, NT_BASE); f.restrictProperty("j:c/foo", Operator.EQUAL, PropertyValues.newString("bar")); f.restrictPath("/test", Filter.PathRestriction.DIRECT_CHILDREN); Map expectedMap = new LinkedHashMap<>(); @@ -327,7 +335,7 @@ public void pathTransformationWithDirectChildrenPathRestriction() throws Excepti expectedMap.put("/tmp", false); expectedMap.put("/tmp/c/d", false); } - validateResult(f, of("/test/a"), expectedMap); + validateResult(f, Set.of("/test/a"), expectedMap); // /jcr:root/test/*[*/foo = 'bar'] -> foo:bar :ancestors:/test :depth:[3 TO 3] -> transform 1 level up f = createFilter(root, NT_BASE); @@ -344,7 +352,7 @@ public void pathTransformationWithDirectChildrenPathRestriction() throws Excepti expectedMap2.put("/tmp", false); expectedMap2.put("/tmp/c/d", false); } - validateResult(f, of("/test/a"), expectedMap2); + validateResult(f, Set.of("/test/a"), expectedMap2); // /jcr:root/test/*[d/*/foo = 'bar'] -> foo:bar :ancestors:/test :depth:[4 TO 4] -> transform 2 level up f = createFilter(root, NT_BASE); @@ -360,16 +368,15 @@ public void pathTransformationWithDirectChildrenPathRestriction() throws Excepti expectedMap3.put("/tmp", false); expectedMap3.put("/tmp/c", false); } - validateResult(f, of("/test/c"), expectedMap3); + validateResult(f, Set.of("/test/c"), expectedMap3); } @Test public void pathTransformationWithExactPathRestriction() throws Exception { setupTestData(evaluatePathRestrictionsInIndex); - FilterImpl f; // /jcr:root/test/a[j:c/foo = 'bar'] -> foo:bar :path:/test/a/j:c -> transform 1 level up - f = createFilter(root, NT_BASE); + FilterImpl f = createFilter(root, NT_BASE); f.restrictProperty("j:c/foo", Operator.EQUAL, PropertyValues.newString("bar")); f.restrictPath("/test/a", Filter.PathRestriction.EXACT); Map expectedMap = new LinkedHashMap<>(); @@ -377,7 +384,7 @@ public void pathTransformationWithExactPathRestriction() throws Exception { // (because we don't need ancestor query here, a simple term query on path term(:path) works which is always indexed) // so expectedMap here would be same for both expectedMap.put("/test/a", true); - validateResult(f, of("/test/a"), expectedMap); + validateResult(f, Set.of("/test/a"), expectedMap); // /jcr:root/test/a[*/foo = 'bar'] -> foo:bar -> transform 1 level up + filter path restriction f = createFilter(root, NT_BASE); @@ -393,7 +400,7 @@ public void pathTransformationWithExactPathRestriction() throws Exception { expectedMap2.put("/tmp/a", false); expectedMap2.put("/tmp", false); expectedMap2.put("/tmp/c/d", false); - validateResult(f, of("/test/a"), expectedMap2); + validateResult(f, Set.of("/test/a"), expectedMap2); // /jcr:root/test/c[d/*/foo = 'bar'] -> foo:bar -> transform 2 level up + filter path restriction f = createFilter(root, NT_BASE); @@ -408,7 +415,7 @@ public void pathTransformationWithExactPathRestriction() throws Exception { expectedMap3.put("/test/c", true); expectedMap3.put("/tmp", false); expectedMap3.put("/tmp/c", false); - validateResult(f, of("/test/c"), expectedMap3); + validateResult(f, Set.of("/test/c"), expectedMap3); // /jcr:root/test/a/j:c[foo = 'bar'] -> foo:bar :path:/test/a/j:c -> No Transformation @@ -417,24 +424,22 @@ public void pathTransformationWithExactPathRestriction() throws Exception { f.restrictPath("/test/a/j:c", Filter.PathRestriction.EXACT); Map expectedMap4 = new LinkedHashMap<>(); expectedMap4.put("/test/a/j:c", true); - validateResult(f, of("/test/a/j:c"), expectedMap4); + validateResult(f, Set.of("/test/a/j:c"), expectedMap4); } @Test public void pathTransformationWithParentFilter() throws Exception { setupTestData(evaluatePathRestrictionsInIndex); - FilterImpl f; - // /jcr:root/test/a/b/j:c/..[j:c/foo = 'bar'] -> foo:bar :path:/test/a/b/j:c -> transform 1 level up - f = createFilter(root, NT_BASE); + FilterImpl f = createFilter(root, NT_BASE); f.restrictProperty("j:c/foo", Operator.EQUAL, PropertyValues.newString("bar")); f.restrictPath("/test/c/d/j:c", Filter.PathRestriction.PARENT); Map expectedMap = new LinkedHashMap<>(); // In case of PARENT path restriction, index can still serve path restriction even if evaluatePathRestrictions is false // (because we don't need ancestor query here, a simple term query on path term(:path) works which is always indexed) expectedMap.put("/test/c/d", true); - validateResult(f, of("/test/c/d"), expectedMap); + validateResult(f, Set.of("/test/c/d"), expectedMap); // /jcr:root/test/a/b/j:c/..[*/foo = 'bar'] -> foo:bar -> transform 1 level up f = createFilter(root, NT_BASE); @@ -452,7 +457,7 @@ public void pathTransformationWithParentFilter() throws Exception { expectedMap2.put("/tmp/a", true); expectedMap2.put("/tmp", true); expectedMap2.put("/tmp/c/d", true); - validateResult(f, ImmutableSet.of("/test", "/test/a", "/test/c/d", "/tmp", "/tmp/a", "/tmp/c/d"), expectedMap2); + validateResult(f, Set.of("/test", "/test/a", "/test/c/d", "/tmp", "/tmp/a", "/tmp/c/d"), expectedMap2); // /jcr:root/test/c/d/..[d/*/foo = 'bar'] -> foo:bar -> transform 2 level up f = createFilter(root, NT_BASE); @@ -465,7 +470,7 @@ public void pathTransformationWithParentFilter() throws Exception { expectedMap3.put("/test/c", true); expectedMap3.put("/tmp", true); expectedMap3.put("/tmp/c", true); - validateResult(f, of("/", "/test", "/test/c", "/tmp", "/tmp/c"), expectedMap3); + validateResult(f, Set.of("/", "/test", "/test/c", "/tmp", "/tmp/c"), expectedMap3); // /jcr:root/test/c/d/..[foo = 'bar'] -> foo:bar :path:/test/c/d -> No Transformation @@ -476,7 +481,7 @@ public void pathTransformationWithParentFilter() throws Exception { expectedMap4.put("/tmp2/a", true); - validateResult(f, of("/tmp2/a"), expectedMap4); + validateResult(f, Set.of("/tmp2/a"), expectedMap4); } private void setupTestData(boolean evaluatePathRestrictionsInIndex) throws Exception { @@ -509,7 +514,6 @@ private void setupTestData(boolean evaluatePathRestrictionsInIndex) throws Excep commit(); } - private void commit() throws Exception { // This part of code is used by both lucene and elastic tests. // The index definitions in these tests don't have async property set @@ -536,13 +540,6 @@ private void validateEstimatedCount(Filter f, long expectedCount) { }, 4000 * 5); } - private long getEstimatedCount(Filter f) { - List plans = index.getPlans(f, null, root); - assertEquals("Only one plan must show up", 1, plans.size()); - QueryIndex.IndexPlan plan = plans.get(0); - return plan.getEstimatedEntryCount(); - } - private static FilterImpl createFilter(NodeState root, String nodeTypeName) { NodeTypeInfoProvider nodeTypes = new NodeStateNodeTypeInfoProvider(root); NodeTypeInfo type = nodeTypes.getNodeTypeInfo(nodeTypeName); @@ -571,25 +568,27 @@ private void validateResult(Filter f, Set expected, Map try { customLogs.starting(); Cursor cursor = index.query(plan, root); - Set paths = Sets.newHashSet(); + + Set paths = new HashSet<>(); while (cursor.hasNext()) { paths.add(cursor.next().getPath()); } - assertEquals("Expected number log entries for post path filtering", expectedPathMapForPostPathFilterLogEntries.size(), customLogs.getLogs().size()); + if (expectedPathMapForPostPathFilterLogEntries != null) { + assertEquals("Expected number log entries for post path filtering", expectedPathMapForPostPathFilterLogEntries.size(), customLogs.getLogs().size()); - List expectedLogEntries = expectedPathMapForPostPathFilterLogEntries.keySet() - .stream() - .map(path -> getExpectedLogEntryForPostPathFiltering(path, expectedPathMapForPostPathFilterLogEntries.get(path))) - .collect(Collectors.toList()); + List expectedLogEntries = expectedPathMapForPostPathFilterLogEntries.keySet() + .stream() + .map(path -> getExpectedLogEntryForPostPathFiltering(path, expectedPathMapForPostPathFilterLogEntries.get(path))) + .collect(Collectors.toList()); - assertEquals("Expected log entries for post path filtering", expectedLogEntries.toString(), customLogs.getLogs().toString()); + assertEquals("Expected log entries for post path filtering", expectedLogEntries.toString(), customLogs.getLogs().toString()); + } assertEquals(f.toString(), expected, paths); } finally { customLogs.finished(); } - }, 3000 * 3); }