This is an automated email from the ASF dual-hosted git repository.
fortino pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 87035907d2 OAK-10617: oak-search-elastic fix deadlock with
includePathRestrictions=false and multiple filtered results (#1276)
87035907d2 is described below
commit 87035907d26d89495f4c47faf827ac8e9f91f34f
Author: Fabrizio Fortino <[email protected]>
AuthorDate: Tue Jan 23 14:13:05 2024 +0100
OAK-10617: oak-search-elastic fix deadlock with
includePathRestrictions=false and multiple filtered results (#1276)
* OAK-10617: oak-search-elastic fix deadlock with
includePathRestrictions=false and multiple filtered results
* OAK-10617: always apply path restrictions in ES
* OAK-10617: remove use of guava from modified classes
* OAK-10617: updated docs
* OAK-10617: simplify logic to check if all listeners have process at least
one element
* OAK-10617: async iterator queue is now bounded to the read limit
* OAK-10617: small improvement in logic to check if all listeners have
process at least one element
* OAK-10617: minor improvement
* OAK-10617: minor improvement: use a BitSet to keep track of search hit
listeners
* OAK-10617: annotate ElasticResponseListener with @ProviderType
---
oak-doc/src/site/markdown/query/elastic.md | 5 +-
oak-search-elastic/pom.xml | 5 +
.../index/elastic/ElasticIndexDefinition.java | 5 +
.../index/elastic/query/ElasticRequestHandler.java | 68 +++++++------
.../query/async/ElasticResponseListener.java | 8 +-
.../query/async/ElasticResultRowAsyncIterator.java | 30 ++++--
.../facets/ElasticSecureFacetAsyncProvider.java | 3 +-
...ElasticIndexDescendantSpellcheckCommonTest.java | 8 ++
.../index/IndexPathRestrictionCommonTest.java | 105 ++++++++++-----------
9 files changed, 135 insertions(+), 102 deletions(-)
diff --git a/oak-doc/src/site/markdown/query/elastic.md
b/oak-doc/src/site/markdown/query/elastic.md
index 867470e505..2f6669d320 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 a0fd53acc4..277d451a97 100644
--- a/oak-search-elastic/pom.xml
+++ b/oak-search-elastic/pom.xml
@@ -101,6 +101,11 @@
<artifactId>org.apache.felix.scr.annotations</artifactId>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>org.osgi</groupId>
+ <artifactId>org.osgi.annotation.versioning</artifactId>
+ <scope>provided</scope>
+ </dependency>
<dependency>
<groupId>org.osgi</groupId>
<artifactId>org.osgi.service.component.annotations</artifactId>
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 82d0374fdd..95b3290bfe 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 class ElasticIndexDefinition extends IndexDefinition
{
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 f04d38c2d2..2935ec45ce 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 @@ public class ElasticRequestHandler {
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 b49875a729..f3a770f6fe 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._types.aggregations.Aggregate;
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<String> DEFAULT_SOURCE_FIELDS = Collections.singleton(FieldNames.PATH);
+ Set<String> DEFAULT_SOURCE_FIELDS = Set.of(FieldNames.PATH);
/**
* Returns the source fields this listener is interested on
@@ -68,8 +69,9 @@ public interface ElasticResponseListener {
/**
* 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<ObjectNode> searchHit);
+ boolean on(Hit<ObjectNode> 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 b350762fe8..b0da1da4f7 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.Logger;
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.Set;
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<FulltextResultRow> queue = new
LinkedBlockingQueue<>();
+ private final BlockingQueue<FulltextResultRow> queue;
private final ElasticIndexNode indexNode;
private final IndexPlan indexPlan;
@@ -96,6 +98,9 @@ public class ElasticResultRowAsyncIterator implements
ElasticQueryIterator, Elas
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 class ElasticResultRowAsyncIterator implements
ElasticQueryIterator, Elas
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 class ElasticResultRowAsyncIterator implements
ElasticQueryIterator, Elas
}
@Override
- public void on(Hit<ObjectNode> searchHit) {
+ public boolean on(Hit<ObjectNode> 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 class ElasticResultRowAsyncIterator implements
ElasticQueryIterator, Elas
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 class ElasticResultRowAsyncIterator implements
ElasticQueryIterator, Elas
}
LOG.trace("Emitting {} search hits, for a total of {} scanned
results", searchHits.size(), scannedRows);
+
+ BitSet listenersWithHits = new
BitSet(searchHitListeners.size());
+
for (Hit<ObjectNode> 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 a02a6afcb1..dec588c62d 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 @@ class ElasticSecureFacetAsyncProvider implements
ElasticFacetProvider, ElasticRe
}
@Override
- public void on(Hit<ObjectNode> searchHit) {
+ public boolean on(Hit<ObjectNode> searchHit) {
final String path = elasticResponseHandler.getPath(searchHit);
if (path != null && isAccessible.test(path)) {
for (String field: facetFields) {
@@ -90,6 +90,7 @@ class ElasticSecureFacetAsyncProvider implements
ElasticFacetProvider, ElasticRe
}
}
}
+ 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 a428a54e0c..eb2e0c60fc 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.Oak;
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 @@ public class ElasticIndexDescendantSpellcheckCommonTest
extends IndexDescendantS
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 f28c59bb78..be2e2203b3 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.Test;
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 abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
// 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<String, Boolean> expectedMap = new LinkedHashMap<>();
expectedMap.put("/test/a", true);
@@ -107,12 +121,12 @@ public abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
}
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 abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
// 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 abstract class IndexPathRestrictionCommonTest {
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<String, Boolean> expectedMap = new LinkedHashMap<>();
@@ -327,7 +335,7 @@ public abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
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<String, Boolean> expectedMap = new LinkedHashMap<>();
@@ -377,7 +384,7 @@ public abstract class IndexPathRestrictionCommonTest {
// (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 abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
f.restrictPath("/test/a/j:c", Filter.PathRestriction.EXACT);
Map<String, Boolean> 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<String, Boolean> 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 abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
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 abstract class IndexPathRestrictionCommonTest {
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 @@ public abstract class IndexPathRestrictionCommonTest {
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 @@ public abstract class IndexPathRestrictionCommonTest {
}, 4000 * 5);
}
- private long getEstimatedCount(Filter f) {
- List<QueryIndex.IndexPlan> 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 @@ public abstract class IndexPathRestrictionCommonTest {
try {
customLogs.starting();
Cursor cursor = index.query(plan, root);
- Set<String> paths = Sets.newHashSet();
+
+ Set<String> 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<String> expectedLogEntries =
expectedPathMapForPostPathFilterLogEntries.keySet()
- .stream()
- .map(path ->
getExpectedLogEntryForPostPathFiltering(path,
expectedPathMapForPostPathFilterLogEntries.get(path)))
- .collect(Collectors.toList());
+ List<String> 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);
}
