From ddc831e5ca51b91186695fd42b21031d0d579520 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Wed, 4 Mar 2020 15:23:20 +0100 Subject: [PATCH 01/18] chore: clean up unneeded dependencies --- oak-search-elastic/pom.xml | 163 +++++++------------------------------ 1 file changed, 30 insertions(+), 133 deletions(-) diff --git a/oak-search-elastic/pom.xml b/oak-search-elastic/pom.xml index c83b1d69dfb..5280fce1b2f 100644 --- a/oak-search-elastic/pom.xml +++ b/oak-search-elastic/pom.xml @@ -33,8 +33,8 @@ Oak Elasticsearch integration subproject - 7.1.1 - 8.0.0 + 7.6.0 + 8.4.0 @@ -91,6 +91,33 @@ provided + + + org.elasticsearch.client + elasticsearch-rest-high-level-client + ${elasticsearch.version} + + + org.elasticsearch.client + elasticsearch-rest-client + ${elasticsearch.version} + + + org.elasticsearch + elasticsearch + ${elasticsearch.version} + + + org.elasticsearch + elasticsearch-x-content + ${elasticsearch.version} + + + org.apache.lucene + lucene-core + ${lucene.version} + + org.apache.jackrabbit oak-api @@ -116,11 +143,6 @@ oak-core ${project.version} - - org.apache.jackrabbit - oak-store-document - ${project.version} - org.apache.jackrabbit oak-search @@ -140,26 +162,11 @@ - - ch.qos.logback - logback-classic - test - junit junit test - - org.elasticsearch.client - elasticsearch-rest-high-level-client - ${elasticsearch.version} - - - org.apache.lucene - lucene-core - ${lucene.version} - org.apache.jackrabbit oak-core @@ -167,66 +174,6 @@ tests test - - org.apache.jackrabbit - oak-blob-plugins - ${project.version} - tests - test - - - org.apache.jackrabbit - oak-store-spi - ${project.version} - test-jar - test - - - org.apache.jackrabbit - oak-segment-tar - ${project.version} - test - - - org.apache.jackrabbit - oak-segment-tar - ${project.version} - test-jar - test - - - org.apache.jackrabbit - oak-store-document - ${project.version} - test-jar - test - - - org.apache.jackrabbit - oak-jcr - ${project.version} - test - - - org.apache.jackrabbit - oak-jcr - ${project.version} - test-jar - test - - - org.apache.jackrabbit - oak-commons - ${project.version} - test-jar - test - - - org.apache.jackrabbit - jackrabbit-jcr-tests - ${jackrabbit.version} - test - org.apache.jackrabbit jackrabbit-core @@ -234,67 +181,17 @@ tests test - - org.apache.tika - tika-parsers - ${tika.version} - test - - - commons-logging - commons-logging - - - org.slf4j - slf4j-log4j12 - - - - - org.apache.sling - org.apache.sling.testing.osgi-mock - org.hamcrest - hamcrest-all + hamcrest-core 1.3 test - - org.mockito - mockito-core - test - - - io.dropwizard.metrics - metrics-core - test - - - org.apache.commons - commons-exec - 1.3 - test - - - com.google.code.gson - gson - 2.8.0 - test - com.arakelian docker-junit-rule test - - org.apache.jackrabbit - oak-search - tests - ${project.version} - test - - From faadc22fec99fb3cf5b168d215da3657ded4fcf1 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Wed, 4 Mar 2020 15:35:38 +0100 Subject: [PATCH 02/18] refactor: use java 8 instead of guava --- .../ElasticsearchConnectionFactory.java | 4 +- .../ElasticsearchCoordinateImpl.java | 8 ++-- .../ElasticsearchIndexCoordinateImpl.java | 36 +++++---------- .../ElasticsearchIndexProviderService.java | 20 ++++----- .../index/ElasticsearchDocument.java | 15 +++---- .../ElasticsearchIndexEditorProvider.java | 5 +-- .../query/ElasticsearchIndexProvider.java | 4 +- .../query/ElasticsearchResultRowIterator.java | 45 ++++++++++--------- .../ElasticsearchDockerRule.java | 2 +- .../ElasticsearchManagementRule.java | 4 +- .../ElasticsearchPropertyIndexTest.java | 37 ++++++++------- 11 files changed, 82 insertions(+), 98 deletions(-) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchConnectionFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchConnectionFactory.java index 1894777e5be..f1d554ff7dd 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchConnectionFactory.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchConnectionFactory.java @@ -16,7 +16,6 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch; -import com.google.common.collect.Maps; import org.apache.http.HttpHost; import org.elasticsearch.client.RestClient; import org.elasticsearch.client.RestHighLevelClient; @@ -25,13 +24,14 @@ import java.io.Closeable; import java.io.IOException; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantReadWriteLock; public class ElasticsearchConnectionFactory implements Closeable { private static final Logger LOG = LoggerFactory.getLogger(ElasticsearchConnectionFactory.class); - private final ConcurrentMap clientMap = Maps.newConcurrentMap(); + private final ConcurrentMap clientMap = new ConcurrentHashMap<>(); private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); private final AtomicBoolean isClosed = new AtomicBoolean(); diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinateImpl.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinateImpl.java index 7d99e9b55e6..beeaedabafa 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinateImpl.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinateImpl.java @@ -16,12 +16,12 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch; -import com.google.common.base.Objects; import org.apache.jackrabbit.oak.plugins.index.search.util.ConfigUtil; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.elasticsearch.client.RestHighLevelClient; import java.util.Map; +import java.util.Objects; public class ElasticsearchCoordinateImpl implements ElasticsearchCoordinate { private final ElasticsearchConnectionFactory connectionFactory; @@ -39,10 +39,8 @@ static ElasticsearchCoordinate construct(ElasticsearchConnectionFactory connectionFactory, NodeState indexDefn, Map configMap) { - ElasticsearchCoordinate esCoord; - // index defn is at highest prio - esCoord = readFrom(connectionFactory, indexDefn); + ElasticsearchCoordinate esCoord = readFrom(connectionFactory, indexDefn); if (esCoord != null) { return esCoord; } @@ -106,7 +104,7 @@ public boolean equals(Object o) { @Override public int hashCode() { - return Objects.hashCode(getScheme(), getHost(), getPort()); + return Objects.hash(getScheme(), getHost(), getPort()); } @Override diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinateImpl.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinateImpl.java index 0889365c38e..2e4f94a0087 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinateImpl.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinateImpl.java @@ -16,19 +16,15 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch; -import com.google.common.base.Joiner; -import com.google.common.base.Objects; -import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; import org.elasticsearch.client.RestHighLevelClient; import org.jetbrains.annotations.NotNull; -import java.util.Collections; -import java.util.List; +import java.util.Objects; import java.util.regex.Pattern; import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import static org.elasticsearch.common.Strings.INVALID_FILENAME_CHARS; @@ -41,7 +37,7 @@ ElasticsearchIndexCoordinateImpl(@NotNull ElasticsearchCoordinate esCoord, IndexDefinition indexDefinition) { this.esCoord = esCoord; - esIndexName = getRemoteIndexName(indexDefinition, indexDefinition.getIndexPath()); + esIndexName = getRemoteIndexName(indexDefinition); } @Override @@ -56,7 +52,7 @@ public String getEsIndexName() { @Override public int hashCode() { - return Objects.hashCode(esCoord, esIndexName); + return Objects.hash(esCoord, esIndexName); } @Override @@ -70,14 +66,14 @@ public boolean equals(Object o) { && esIndexName.equals(other.esIndexName); } - private String getRemoteIndexName(IndexDefinition definition, String indexPath) { + private String getRemoteIndexName(IndexDefinition definition) { String suffix = definition.getUniqueId(); if (suffix == null) { suffix = String.valueOf(definition.getReindexCount()); } - return getESSafeIndexName(indexPath + "-" + suffix); + return getESSafeIndexName(definition.getIndexPath() + "-" + suffix); } /** @@ -90,21 +86,13 @@ private String getRemoteIndexName(IndexDefinition definition, String indexPath) * The resulting file name would be truncated to MAX_NAME_LENGTH */ private static String getESSafeIndexName(String indexPath) { - List elements = Lists.newArrayList(PathUtils.elements(indexPath)); - Collections.reverse(elements); - List result = Lists.newArrayListWithCapacity(2); + String name = StreamSupport + .stream(PathUtils.elements(indexPath).spliterator(), false) + .limit(3) //Max 3 nodeNames including oak:index which is the immediate parent for any indexPath + .filter(p -> !"oak:index".equals(p)) + .map(ElasticsearchIndexCoordinateImpl::getESSafeName) + .collect(Collectors.joining("_")); - //Max 3 nodeNames including oak:index which is the immediate parent for any indexPath - for (String e : Iterables.limit(elements, 3)) { - if ("oak:index".equals(e)) { - continue; - } - - result.add(getESSafeName(e)); - } - - Collections.reverse(result); - String name = Joiner.on('_').join(result); if (name.length() > MAX_NAME_LENGTH){ name = name.substring(0, MAX_NAME_LENGTH); } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java index fd6a2a969fb..2e407a3e87e 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java @@ -16,9 +16,6 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch; -import com.google.common.base.Strings; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import org.apache.commons.io.FilenameUtils; import org.apache.felix.scr.annotations.*; import org.apache.jackrabbit.oak.api.jmx.CacheStatsMBean; @@ -41,12 +38,13 @@ import org.slf4j.LoggerFactory; import java.io.File; +import java.util.ArrayList; import java.util.Dictionary; +import java.util.HashMap; import java.util.Hashtable; import java.util.List; import java.util.Map; -import static com.google.common.base.Preconditions.checkNotNull; import static org.apache.commons.io.FileUtils.ONE_MB; import static org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUtils.registerMBean; @@ -123,8 +121,8 @@ private ElasticsearchConnectionFactory connectionFactory = null; - private final List regs = Lists.newArrayList(); - private final List oakRegs = Lists.newArrayList(); + private final List regs = new ArrayList<>(); + private final List oakRegs = new ArrayList<>(); private Whiteboard whiteboard; private File textExtractionDir; @@ -211,15 +209,17 @@ private void initializeExtractedTextCache(Map config, StatisticsProvi void initializeTextExtractionDir(BundleContext bundleContext, Map config) { String textExtractionDir = PropertiesUtil.toString(config.get(PROP_LOCAL_TEXT_EXTRACTION_DIR), null); - if (Strings.isNullOrEmpty(textExtractionDir)) { + if (textExtractionDir == null || textExtractionDir.trim().isEmpty()) { String repoHome = bundleContext.getProperty(REPOSITORY_HOME); if (repoHome != null){ textExtractionDir = FilenameUtils.concat(repoHome, "index"); } } - checkNotNull(textExtractionDir, "Text extraction directory cannot be determined as neither " + - "directory path [%s] nor repository home [%s] defined", PROP_LOCAL_TEXT_EXTRACTION_DIR, REPOSITORY_HOME); + if (textExtractionDir == null) { + throw new IllegalStateException(String.format("Text extraction directory cannot be determined as neither " + + "directory path [%s] nor repository home [%s] defined", PROP_LOCAL_TEXT_EXTRACTION_DIR, REPOSITORY_HOME)); + } this.textExtractionDir = new File(textExtractionDir); } @@ -240,7 +240,7 @@ private void registerExtractedTextProvider(PreExtractedTextProvider provider){ private ElasticsearchIndexCoordinateFactory getElasticsearchIndexCoordinateFactory(Map config) { ElasticsearchIndexCoordinateFactory esIndexCoordFactory; - Map esCfg = Maps.newHashMap(); + Map esCfg = new HashMap<>(); esCfg.put(ElasticsearchCoordinate.SCHEME_PROP, PropertiesUtil.toString(config.get(PROP_ELASTICSEARCH_SCHEME), PROP_ELASTICSEARCH_SCHEME_DEFAULT)); esCfg.put(ElasticsearchCoordinate.HOST_PROP, diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchDocument.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchDocument.java index ea982c8ecb2..524d19911b5 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchDocument.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchDocument.java @@ -16,7 +16,6 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch.index; -import com.google.common.collect.Maps; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.plugins.index.search.FieldNames; import org.elasticsearch.common.Strings; @@ -28,11 +27,11 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.URLEncoder; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; -import static com.google.common.collect.Lists.newArrayList; - public class ElasticsearchDocument { private static final Logger LOG = LoggerFactory.getLogger(ElasticsearchDocument.class); @@ -55,11 +54,11 @@ LOG.warn("Couldn't encode {} as ES id", path); } this.id = id; - this.fulltext = newArrayList(); - this.suggest = newArrayList(); - this.notNullProps = newArrayList(); - this.nullProps = newArrayList(); - this.properties = Maps.newHashMap(); + this.fulltext = new ArrayList<>(); + this.suggest = new ArrayList<>(); + this.notNullProps = new ArrayList<>(); + this.nullProps = new ArrayList<>(); + this.properties = new HashMap<>(); } void addFulltext(String value) { diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java index b73bd054554..f3977ffbd69 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java @@ -30,7 +30,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import static com.google.common.base.Preconditions.checkArgument; import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexConstants.TYPE_ELASTICSEARCH; public class ElasticsearchIndexEditorProvider implements IndexEditorProvider { @@ -49,8 +48,8 @@ public ElasticsearchIndexEditorProvider(@NotNull ElasticsearchIndexCoordinateFac @NotNull NodeBuilder definition, @NotNull NodeState root, @NotNull IndexUpdateCallback callback) throws CommitFailedException { if (TYPE_ELASTICSEARCH.equals(type)) { - checkArgument(callback instanceof ContextAwareCallback, "callback instance not of type " + - "ContextAwareCallback [%s]", callback); + assert callback instanceof ContextAwareCallback : "callback instance not of type " + + "ContextAwareCallback [" + callback + "]"; IndexingContext indexingContext = ((ContextAwareCallback)callback).getIndexingContext(); String indexPath = indexingContext.getIndexPath(); diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java index a09924291af..daac23908a5 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java @@ -16,13 +16,13 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch.query; -import com.google.common.collect.ImmutableList; import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinateFactory; import org.apache.jackrabbit.oak.spi.query.QueryIndex; import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.jetbrains.annotations.NotNull; +import java.util.Collections; import java.util.List; public class ElasticsearchIndexProvider implements QueryIndexProvider { @@ -34,6 +34,6 @@ public ElasticsearchIndexProvider(@NotNull ElasticsearchIndexCoordinateFactory e @Override public @NotNull List getQueryIndexes(NodeState nodeState) { - return ImmutableList.of(new ElasticsearchIndex(esIndexCoordFactory, nodeState)); + return Collections.singletonList(new ElasticsearchIndex(esIndexCoordFactory, nodeState)); } } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java index 27088a93a19..2d9b836bfb8 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java @@ -16,9 +16,6 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch.query; -import com.google.common.collect.AbstractIterator; -import com.google.common.collect.Iterables; -import com.google.common.collect.Queues; import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.commons.PerfLogger; @@ -48,13 +45,15 @@ import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Deque; +import java.util.Iterator; import java.util.List; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BiPredicate; +import java.util.stream.StreamSupport; -import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES; import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; import static org.apache.jackrabbit.oak.api.Type.STRING; @@ -66,7 +65,7 @@ import static org.apache.jackrabbit.util.ISO8601.parse; import static org.elasticsearch.index.query.QueryBuilders.*; -public class ElasticsearchResultRowIterator extends AbstractIterator { +public class ElasticsearchResultRowIterator implements Iterator { private static final Logger LOG = LoggerFactory .getLogger(ElasticsearchResultRowIterator.class); private static final PerfLogger PERF_LOGGER = @@ -83,8 +82,7 @@ private static final int ELASTICSEARCH_QUERY_MAX_BATCH_SIZE = 10000; - - private final Deque queue = Queues.newArrayDeque(); + private final Deque queue = new ArrayDeque<>(); // TODO : find if ES can return dup docs - if so how to avoid // private final Set seenPaths = Sets.newHashSet(); private SearchHit lastDoc; @@ -116,12 +114,13 @@ } @Override - protected FulltextIndex.FulltextResultRow computeNext() { - if (!queue.isEmpty() || loadDocs()) { - return queue.remove(); - } else { - return endOfData(); - } + public boolean hasNext() { + return !queue.isEmpty() || loadDocs(); + } + + @Override + public FulltextIndex.FulltextResultRow next() { + return queue.remove(); } /** @@ -134,9 +133,11 @@ private boolean loadDocs() { return false; } - SearchHit lastDocToRecord = null; + if(indexNode != null) { + throw new IllegalStateException("indexNode cannot be null"); + } - checkState(indexNode != null); + SearchHit lastDocToRecord = null; try { ElasticsearchSearcher searcher = getCurrentSearcher(indexNode); QueryBuilder query = getESRequest(plan, pr); @@ -430,7 +431,6 @@ private static String getLuceneFieldName(@Nullable String p, PlanResult pr) { */ @NotNull private static QueryBuilder performAdditionalWraps(@NotNull List qs) { - checkNotNull(qs); if (qs.size() == 1) { // we don't need to worry about all-negatives in a bool query as // BoolQueryBuilder.adjustPureNegative is on by default anyway @@ -460,8 +460,6 @@ private static QueryBuilder performAdditionalWraps(@NotNull List q * @return true if there where at least one unwrapped NOT. false otherwise. */ private static boolean unwrapMustNot(@NotNull BoolQueryBuilder input, @NotNull BoolQueryBuilder output) { - checkNotNull(input); - checkNotNull(output); boolean unwrapped = false; for (QueryBuilder mustNot : input.mustNot()) { output.mustNot(mustNot); @@ -479,7 +477,10 @@ private static boolean unwrapMustNot(@NotNull BoolQueryBuilder input, @NotNull B } private static void addNonFullTextConstraints(List qs, - IndexPlan plan, PlanResult planResult) { + IndexPlan plan, PlanResult planResult) { + final BiPredicate, String> any = (iterable, value) -> + StreamSupport.stream(iterable.spliterator(), false).anyMatch(value::equals); + Filter filter = plan.getFilter(); IndexDefinition defn = planResult.indexDefinition; if (!filter.matchesAllTypes()) { @@ -509,7 +510,7 @@ private static void addNonFullTextConstraints(List qs, // deduced if (planResult.isPathTransformed()) { String parentPathSegment = planResult.getParentPathSegment(); - if ( ! Iterables.any(PathUtils.elements(parentPathSegment), "*"::equals)) { + if ( ! any.test(PathUtils.elements(parentPathSegment), "*")) { qs.add(newPathQuery(path + parentPathSegment)); } } else { @@ -527,7 +528,7 @@ private static void addNonFullTextConstraints(List qs, // deduced if (planResult.isPathTransformed()) { String parentPathSegment = planResult.getParentPathSegment(); - if ( ! Iterables.any(PathUtils.elements(parentPathSegment), "*"::equals)) { + if ( ! any.test(PathUtils.elements(parentPathSegment), "*")) { qs.add(newPathQuery(getParentPath(path) + parentPathSegment)); } } else { diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchDockerRule.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchDockerRule.java index 204ebe16536..7f33f4f8f40 100644 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchDockerRule.java +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchDockerRule.java @@ -35,7 +35,7 @@ private static final String CONFIG_NAME = "Elasticsearch"; - private static final String VERSION = System.getProperty("elasticsearch.version", "7.1.1"); + private static final String VERSION = System.getProperty("elasticsearch.version", "7.6.0"); private static final String IMAGE = "elasticsearch:" + VERSION; diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchManagementRule.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchManagementRule.java index ce8aa7e8a69..48740991c93 100644 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchManagementRule.java +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchManagementRule.java @@ -16,7 +16,6 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch; -import com.google.common.collect.Sets; import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest; @@ -29,6 +28,7 @@ import org.slf4j.LoggerFactory; import java.io.IOException; +import java.util.HashSet; import java.util.Set; public class ElasticsearchManagementRule extends ExternalResource @@ -39,7 +39,7 @@ private final ElasticsearchConnectionFactory connectionFactory = new ElasticsearchConnectionFactory(); - private final Set indices = Sets.newHashSet(); + private final Set indices = new HashSet<>(); private boolean usingDocker; diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java index fd1b759b75c..6f0e1c7a863 100644 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java @@ -38,10 +38,8 @@ import org.junit.Test; import java.util.Arrays; -import java.util.Collections; -import java.util.Set; -import static com.google.common.collect.ImmutableSet.of; +import static java.util.Collections.singletonList; import static org.apache.derby.vti.XmlVTI.asList; import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME; import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.PROPDEF_PROP_NODE_NAME; @@ -81,8 +79,8 @@ protected void createTestIndexNode() { @Test public void indexSelection() throws Exception { - setIndex("test1", createIndex(of("propa", "propb"))); - setIndex("test2", createIndex(of("propc"))); + setIndex("test1", createIndex("propa", "propb")); + setIndex("test2", createIndex("propc")); Tree test = root.getTree("/").addChild("test"); test.addChild("a").setProperty("propa", "foo"); @@ -105,7 +103,7 @@ public void indexSelection() throws Exception { @Test public void nodeNameViaPropDefinition() throws Exception{ //make index - IndexDefinitionBuilder builder = createIndex(Collections.EMPTY_SET); + IndexDefinitionBuilder builder = createIndex(); builder.includedPaths("/test") .evaluatePathRestrictions() .indexRule("nt:base") @@ -126,19 +124,19 @@ public void nodeNameViaPropDefinition() throws Exception{ String explanation = explain(propabQuery); Assert.assertThat(explanation, containsString("elasticsearch:test1(/oak:index/test1) ")); Assert.assertThat(explanation, containsString("{\"term\":{\":nodeName\":{\"value\":\"foo\",")); - assertQuery(propabQuery, Arrays.asList("/test/foo")); - assertQuery(queryPrefix + "LOCALNAME() = 'bar'", Arrays.asList("/test/sc/bar")); - assertQuery(queryPrefix + "LOCALNAME() LIKE 'foo'", Arrays.asList("/test/foo")); - assertQuery(queryPrefix + "LOCALNAME() LIKE 'camel%'", Arrays.asList("/test/camelCase")); - - assertQuery(queryPrefix + "NAME() = 'bar'", Arrays.asList("/test/sc/bar")); - assertQuery(queryPrefix + "NAME() LIKE 'foo'", Arrays.asList("/test/foo")); - assertQuery(queryPrefix + "NAME() LIKE 'camel%'", Arrays.asList("/test/camelCase")); + assertQuery(propabQuery, singletonList("/test/foo")); + assertQuery(queryPrefix + "LOCALNAME() = 'bar'", singletonList("/test/sc/bar")); + assertQuery(queryPrefix + "LOCALNAME() LIKE 'foo'", singletonList("/test/foo")); + assertQuery(queryPrefix + "LOCALNAME() LIKE 'camel%'", singletonList("/test/camelCase")); + + assertQuery(queryPrefix + "NAME() = 'bar'", singletonList("/test/sc/bar")); + assertQuery(queryPrefix + "NAME() LIKE 'foo'", singletonList("/test/foo")); + assertQuery(queryPrefix + "NAME() LIKE 'camel%'", singletonList("/test/camelCase")); } @Test public void emptyIndex() throws Exception{ - setIndex("test1", createIndex(of("propa", "propb"))); + setIndex("test1", createIndex("propa", "propb")); root.commit(); Tree test = root.getTree("/").addChild("test"); @@ -151,7 +149,7 @@ public void emptyIndex() throws Exception{ @Test public void propertyExistenceQuery() throws Exception { - setIndex("test1", createIndex(of("propa", "propb"))); + setIndex("test1", createIndex("propa", "propb")); Tree test = root.getTree("/").addChild("test"); test.addChild("a").setProperty("propa", "a"); @@ -162,11 +160,12 @@ public void propertyExistenceQuery() throws Exception { assertQuery("select [jcr:path] from [nt:base] where propa is not null", Arrays.asList("/test/a", "/test/b")); } - private static IndexDefinitionBuilder createIndex(Set propNames) { + private static IndexDefinitionBuilder createIndex(String ... propNames) { IndexDefinitionBuilder builder = new ElasticsearchIndexDefinitionBuilder().noAsync(); IndexDefinitionBuilder.IndexRule indexRule = builder.indexRule("nt:base"); - propNames.forEach(propName -> indexRule.property(propName).propertyIndex()); - + for (String propName : propNames) { + indexRule.property(propName).propertyIndex(); + } return builder; } From 4795bb8f10210bc91770c40ea7dadf202d060abf Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Wed, 4 Mar 2020 17:15:05 +0100 Subject: [PATCH 03/18] fix: wrong null check --- .../elasticsearch/query/ElasticsearchResultRowIterator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java index 2d9b836bfb8..1b3b8755ed2 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java @@ -133,7 +133,7 @@ private boolean loadDocs() { return false; } - if(indexNode != null) { + if(indexNode == null) { throw new IllegalStateException("indexNode cannot be null"); } From 23ab5f3b5f780014767e9a903cd196552ebff97f Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Wed, 4 Mar 2020 17:18:32 +0100 Subject: [PATCH 04/18] refactor: replaced docker-junit-rule with testcontainers --- oak-search-elastic/pom.xml | 5 +- .../ElasticsearchDockerRule.java | 78 -------------- .../ElasticsearchManagementRule.java | 101 ------------------ .../ElasticsearchPropertyIndexTest.java | 21 +++- 4 files changed, 19 insertions(+), 186 deletions(-) delete mode 100644 oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchDockerRule.java delete mode 100644 oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchManagementRule.java diff --git a/oak-search-elastic/pom.xml b/oak-search-elastic/pom.xml index 5280fce1b2f..bee7fe54c6a 100644 --- a/oak-search-elastic/pom.xml +++ b/oak-search-elastic/pom.xml @@ -188,8 +188,9 @@ test - com.arakelian - docker-junit-rule + org.testcontainers + elasticsearch + 1.12.5 test diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchDockerRule.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchDockerRule.java deleted file mode 100644 index 7f33f4f8f40..00000000000 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchDockerRule.java +++ /dev/null @@ -1,78 +0,0 @@ -/* - * 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.elasticsearch; - -import com.arakelian.docker.junit.DockerRule; -import com.arakelian.docker.junit.model.ImmutableDockerConfig; -import com.spotify.docker.client.DefaultDockerClient; -import com.spotify.docker.client.auth.FixedRegistryAuthSupplier; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * An Elasticsearch {@link DockerRule}. - */ -class ElasticsearchDockerRule extends DockerRule { - - //Mimic following: - // docker run -p 9200:9200 -e "discovery.type=single-node" docker.elastic.co/elasticsearch/elasticsearch:7.1.1 - - private static final Logger LOG = LoggerFactory.getLogger(ElasticsearchDockerRule.class); - - private static final String CONFIG_NAME = "Elasticsearch"; - - private static final String VERSION = System.getProperty("elasticsearch.version", "7.6.0"); - - private static final String IMAGE = "elasticsearch:" + VERSION; - - private static final boolean DOCKER_AVAILABLE; - - static { - boolean available = false; - try (DefaultDockerClient client = DefaultDockerClient.fromEnv() - .connectTimeoutMillis(5000L).readTimeoutMillis(20000L) - .registryAuthSupplier(new FixedRegistryAuthSupplier()) - .build()) { - client.ping(); - client.pull(IMAGE); - available = true; - } catch (Throwable t) { - LOG.info("Cannot connect to docker or pull image", t); - } - DOCKER_AVAILABLE = available; - } - - ElasticsearchDockerRule() { - super(ImmutableDockerConfig.builder() - .name(CONFIG_NAME) - .image(IMAGE) - .ports("9200") - .allowRunningBetweenUnitTests(true) - .alwaysRemoveContainer(true) - .addStartedListener(container -> container.waitForLog("LicenseService")) - .addContainerConfigurer(builder -> builder.env("discovery.type=single-node")) - .build()); - } - - int getPort() { - return getContainer().getPortBinding("9200/tcp").getPort(); - } - - boolean isDockerAvailable() { - return DOCKER_AVAILABLE; - } -} diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchManagementRule.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchManagementRule.java deleted file mode 100644 index 48740991c93..00000000000 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchManagementRule.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * 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.elasticsearch; - -import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; -import org.apache.jackrabbit.oak.spi.state.NodeState; -import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest; -import org.elasticsearch.client.RequestOptions; -import org.junit.Assume; -import org.junit.rules.ExternalResource; -import org.junit.runner.Description; -import org.junit.runners.model.Statement; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.io.IOException; -import java.util.HashSet; -import java.util.Set; - -public class ElasticsearchManagementRule extends ExternalResource - implements ElasticsearchIndexCoordinateFactory { - private static final Logger LOG = LoggerFactory.getLogger(ElasticsearchManagementRule.class); - - private final ElasticsearchDockerRule elasticsearch = new ElasticsearchDockerRule(); - - private final ElasticsearchConnectionFactory connectionFactory = new ElasticsearchConnectionFactory(); - - private final Set indices = new HashSet<>(); - - private boolean usingDocker; - - @Override - public Statement apply(Statement base, Description description) { - Statement s = super.apply(base, description); - // see if local instance is available... initialize docker rule only if that's not the case - ElasticsearchCoordinate esCoord = ElasticsearchCoordinateImpl.construct(connectionFactory, - null, null); - if (!ElasticsearchTestUtils.isAvailable(esCoord) && elasticsearch.isDockerAvailable()) { - s = elasticsearch.apply(s, description); - usingDocker = true; - } - - return s; - } - - @Override - public ElasticsearchIndexCoordinate getElasticsearchIndexCoordinate(IndexDefinition indexDefinition) { - ElasticsearchCoordinate esCoord = getElasticsearchCoordinate(indexDefinition.getDefinitionNodeState()); - ElasticsearchIndexCoordinate esIdxCoord = new ElasticsearchIndexCoordinateImpl(esCoord, indexDefinition); - indices.add(esIdxCoord); - return esIdxCoord; - } - - @Override - protected void after() { - deletedIndices(); - connectionFactory.close(); - } - - private ElasticsearchCoordinate getElasticsearchCoordinate(NodeState indexDefinition) { - ElasticsearchCoordinate esCoord = ElasticsearchCoordinateImpl.construct(connectionFactory, - indexDefinition, null); - - if (!ElasticsearchTestUtils.isAvailable(esCoord) && usingDocker) { - int port = elasticsearch.getPort(); - esCoord = new ElasticsearchCoordinateImpl(connectionFactory, "http", "localhost", port); - } - - Assume.assumeTrue(ElasticsearchTestUtils.isAvailable(esCoord)); - - return esCoord; - } - - private void deletedIndices() { - indices.forEach(idxCoord -> { - DeleteIndexRequest request = new DeleteIndexRequest(idxCoord.getEsIndexName()); - try { - idxCoord.getClient().indices().delete(request, RequestOptions.DEFAULT); - - LOG.info("Cleaned up index {}", idxCoord.getEsIndexName()); - - } catch (IOException e) { - LOG.warn("Failed to cleanup index {}", idxCoord.getEsIndexName()); - } - }); - } -} \ No newline at end of file diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java index 6f0e1c7a863..deb3c765cc1 100644 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java @@ -34,8 +34,9 @@ import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeStore; import org.junit.Assert; -import org.junit.Rule; +import org.junit.ClassRule; import org.junit.Test; +import org.testcontainers.elasticsearch.ElasticsearchContainer; import java.util.Arrays; @@ -47,14 +48,24 @@ import static org.hamcrest.MatcherAssert.assertThat; public class ElasticsearchPropertyIndexTest extends AbstractQueryTest { - @Rule - public ElasticsearchManagementRule esMgmt = new ElasticsearchManagementRule(); + @ClassRule + public static final ElasticsearchContainer ELASTIC = + new ElasticsearchContainer("docker.elastic.co/elasticsearch/elasticsearch:7.6.0"); + + private ElasticsearchIndexCoordinateFactory eicf = indexDefinition -> { + ElasticsearchCoordinate esCoord = + new ElasticsearchCoordinateImpl( + new ElasticsearchConnectionFactory(), "http", ELASTIC.getContainerIpAddress(), + ELASTIC.getMappedPort(9200) + ); + return new ElasticsearchIndexCoordinateImpl(esCoord, indexDefinition); + }; @Override protected ContentRepository createRepository() { - ElasticsearchIndexEditorProvider editorProvider = new ElasticsearchIndexEditorProvider(esMgmt, + ElasticsearchIndexEditorProvider editorProvider = new ElasticsearchIndexEditorProvider(eicf, new ExtractedTextCache(10* FileUtils.ONE_MB, 100)); - ElasticsearchIndexProvider indexProvider = new ElasticsearchIndexProvider(esMgmt); + ElasticsearchIndexProvider indexProvider = new ElasticsearchIndexProvider(eicf); // remove all indexes to avoid cost competition (essentially a TODO for fixing cost ES cost estimation) NodeBuilder builder = InitialContentHelper.INITIAL_CONTENT.builder(); From 1a03f857aeaabeee3d15233527c6dd748313a89e Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Wed, 4 Mar 2020 17:55:58 +0100 Subject: [PATCH 05/18] chore: removed indirect dependency on derby --- oak-search-elastic/pom.xml | 5 +- .../ElasticsearchPropertyIndexTest.java | 7 ++- .../elasticsearch/ElasticsearchTestUtils.java | 47 ------------------- 3 files changed, 5 insertions(+), 54 deletions(-) delete mode 100644 oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchTestUtils.java diff --git a/oak-search-elastic/pom.xml b/oak-search-elastic/pom.xml index bee7fe54c6a..3722960a86e 100644 --- a/oak-search-elastic/pom.xml +++ b/oak-search-elastic/pom.xml @@ -176,9 +176,8 @@ org.apache.jackrabbit - jackrabbit-core - ${jackrabbit.version} - tests + oak-security-spi + ${project.version} test diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java index deb3c765cc1..462af1926db 100644 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java @@ -41,7 +41,6 @@ import java.util.Arrays; import static java.util.Collections.singletonList; -import static org.apache.derby.vti.XmlVTI.asList; import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME; import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.PROPDEF_PROP_NODE_NAME; import static org.hamcrest.CoreMatchers.containsString; @@ -105,9 +104,9 @@ public void indexSelection() throws Exception { assertThat(explain(propaQuery), containsString("elasticsearch:test1")); assertThat(explain("select [jcr:path] from [nt:base] where [propc] = 'foo'"), containsString("elasticsearch:test2")); - assertQuery(propaQuery, asList("/test/a", "/test/b")); - assertQuery("select [jcr:path] from [nt:base] where [propa] = 'foo2'", asList("/test/c")); - assertQuery("select [jcr:path] from [nt:base] where [propc] = 'foo'", asList("/test/d")); + assertQuery(propaQuery, Arrays.asList("/test/a", "/test/b")); + assertQuery("select [jcr:path] from [nt:base] where [propa] = 'foo2'", Arrays.asList("/test/c")); + assertQuery("select [jcr:path] from [nt:base] where [propc] = 'foo'", Arrays.asList("/test/d")); } //OAK-3825 diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchTestUtils.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchTestUtils.java deleted file mode 100644 index 967c90ba95c..00000000000 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchTestUtils.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * 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.elasticsearch; - -import org.jetbrains.annotations.NotNull; - -import java.net.HttpURLConnection; -import java.net.URL; - -class ElasticsearchTestUtils { - private static String createHealthURL(@NotNull final ElasticsearchCoordinate esCoord) { - return esCoord.getScheme() + "://" + esCoord.getHost() + ":" + esCoord.getPort() + "/_cat/health"; - } - - static boolean isAvailable(final ElasticsearchCoordinate esCoord) { - if (esCoord == null) { - return false; - } - - try { - URL url = new URL(createHealthURL(esCoord)); - - HttpURLConnection con = (HttpURLConnection) url.openConnection(); - con.setRequestMethod("GET"); - - int responseCode = con.getResponseCode(); - - return responseCode == 200; - } catch (Throwable t) { - return false; - } - } -} From 8c7d77ea2ba697bffb9f5e2d89551ecde5893369 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Fri, 6 Mar 2020 09:02:53 +0100 Subject: [PATCH 06/18] refactor: cleaned up some basic stuff --- .../query/ElasticsearchIndexStatistics.java | 9 ++------- .../query/ElasticsearchResultRowIterator.java | 3 --- .../util/TermQueryBuilderFactory.java | 20 ++++++++++++------- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexStatistics.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexStatistics.java index c404174660d..35f85b59b7e 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexStatistics.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexStatistics.java @@ -22,7 +22,6 @@ import org.elasticsearch.client.core.CountRequest; import org.elasticsearch.client.core.CountResponse; import org.elasticsearch.index.query.QueryBuilders; -import org.elasticsearch.search.builder.SearchSourceBuilder; import java.io.IOException; @@ -36,9 +35,7 @@ @Override public int numDocs() { CountRequest countRequest = new CountRequest(); - SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); - searchSourceBuilder.query(QueryBuilders.matchAllQuery()); - countRequest.source(searchSourceBuilder); + countRequest.query(QueryBuilders.matchAllQuery()); try { CountResponse count = elasticsearchIndexCoordinate.getClient().count(countRequest, RequestOptions.DEFAULT); return (int) count.getCount(); @@ -51,9 +48,7 @@ public int numDocs() { @Override public int getDocCountFor(String key) { CountRequest countRequest = new CountRequest(); - SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder(); - searchSourceBuilder.query(QueryBuilders.existsQuery(key)); - countRequest.source(searchSourceBuilder); + countRequest.query(QueryBuilders.existsQuery(key)); try { CountResponse count = elasticsearchIndexCoordinate.getClient().count(countRequest, RequestOptions.DEFAULT); return (int) count.getCount(); diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java index 1b3b8755ed2..9bdd31d97e8 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java @@ -353,9 +353,6 @@ public boolean visit(FullTextTerm term) { private boolean visitTerm(String propertyName, String text, String boost, boolean not) { String p = getLuceneFieldName(propertyName, pr); QueryBuilder q = tokenToQuery(text, p, pr); - if (q == null) { - return false; - } if (boost != null) { q.boost(Float.parseFloat(boost)); } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/util/TermQueryBuilderFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/util/TermQueryBuilderFactory.java index ec4d43a2768..5f807eff09b 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/util/TermQueryBuilderFactory.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/util/TermQueryBuilderFactory.java @@ -17,16 +17,20 @@ package org.apache.jackrabbit.oak.plugins.index.elasticsearch.util; import org.apache.jackrabbit.oak.api.PropertyValue; -import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.plugins.index.search.FieldNames; -import org.apache.jackrabbit.oak.plugins.index.search.PropertyDefinition; import org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndexPlanner; import org.apache.jackrabbit.oak.spi.query.Filter; -import org.elasticsearch.index.query.*; +import org.elasticsearch.index.query.BoolQueryBuilder; +import org.elasticsearch.index.query.ExistsQueryBuilder; +import org.elasticsearch.index.query.PrefixQueryBuilder; +import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.index.query.RangeQueryBuilder; +import org.elasticsearch.index.query.TermQueryBuilder; +import org.elasticsearch.index.query.WildcardQueryBuilder; import org.jetbrains.annotations.NotNull; -import javax.jcr.PropertyType; import java.util.List; import java.util.function.Function; import java.util.stream.Collectors; @@ -35,9 +39,11 @@ import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; import static org.apache.jackrabbit.oak.plugins.index.search.FieldNames.PATH; import static org.apache.jackrabbit.oak.plugins.index.search.FieldNames.PATH_DEPTH; -import static org.elasticsearch.index.query.QueryBuilders.*; +import static org.elasticsearch.index.query.QueryBuilders.boolQuery; +import static org.elasticsearch.index.query.QueryBuilders.prefixQuery; +import static org.elasticsearch.index.query.QueryBuilders.termQuery; +import static org.elasticsearch.index.query.QueryBuilders.wildcardQuery; -@SuppressWarnings("SpellCheckingInspection") public class TermQueryBuilderFactory { /** * Private constructor. @@ -144,7 +150,7 @@ public static TermQueryBuilder newNullPropQuery(String propName) { return newRangeQuery(propertyName, null, last, true, pr.lastIncluding); } else if (pr.list != null) { return newInQuery(propertyName, pr.list.stream() - .map(propToObj::apply) + .map(propToObj) .collect(Collectors.toList())); } else if (pr.isNotNullRestriction()) { // not null. For date lower bound of zero can be used From 7490b19b76332befbbfa6120a40f2a38d620f0b3 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Mon, 9 Mar 2020 17:51:00 +0100 Subject: [PATCH 07/18] refactor: simplified design, improved naming --- ...ltElasticsearchIndexCoordinateFactory.java | 42 ------ ...y.java => ElasticsearchClientFactory.java} | 16 +- .../ElasticsearchCoordinate.java | 97 ++++++++++-- .../ElasticsearchCoordinateImpl.java | 138 ------------------ .../ElasticsearchIndexCoordinate.java | 24 --- .../ElasticsearchIndexCoordinateFactory.java | 23 --- ...java => ElasticsearchIndexDescriptor.java} | 63 +++++--- .../ElasticsearchIndexProviderService.java | 75 +++++----- .../ElasticsearchIndexEditorProvider.java | 10 +- .../index/ElasticsearchIndexWriter.java | 25 ++-- .../ElasticsearchIndexWriterFactory.java | 10 +- .../query/ElasticsearchIndex.java | 14 +- .../query/ElasticsearchIndexNode.java | 27 ++-- .../query/ElasticsearchIndexProvider.java | 10 +- .../query/ElasticsearchIndexStatistics.java | 12 +- .../query/ElasticsearchResultRowIterator.java | 8 +- .../query/ElasticsearchSearcher.java | 19 +-- .../ElasticsearchPropertyIndexTest.java | 22 ++- 18 files changed, 237 insertions(+), 398 deletions(-) delete mode 100644 oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/DefaultElasticsearchIndexCoordinateFactory.java rename oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/{ElasticsearchConnectionFactory.java => ElasticsearchClientFactory.java} (87%) delete mode 100644 oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinateImpl.java delete mode 100644 oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinate.java delete mode 100644 oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinateFactory.java rename oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/{ElasticsearchIndexCoordinateImpl.java => ElasticsearchIndexDescriptor.java} (58%) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/DefaultElasticsearchIndexCoordinateFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/DefaultElasticsearchIndexCoordinateFactory.java deleted file mode 100644 index 5328b53028d..00000000000 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/DefaultElasticsearchIndexCoordinateFactory.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * 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.elasticsearch; - -import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; -import org.apache.jackrabbit.oak.spi.state.NodeState; - -import java.util.Map; - -public class DefaultElasticsearchIndexCoordinateFactory implements ElasticsearchIndexCoordinateFactory { - private final Map config; - private final ElasticsearchConnectionFactory factory; - - DefaultElasticsearchIndexCoordinateFactory(ElasticsearchConnectionFactory factory, Map config) { - this.factory = factory; - this.config = config; - } - - @Override - public ElasticsearchIndexCoordinate getElasticsearchIndexCoordinate(IndexDefinition indexDefinition) { - ElasticsearchCoordinate esCoord = getElasticsearchCoordinate(indexDefinition.getDefinitionNodeState()); - return new ElasticsearchIndexCoordinateImpl(esCoord, indexDefinition); - } - - private ElasticsearchCoordinate getElasticsearchCoordinate(NodeState indexDefinition) { - return ElasticsearchCoordinateImpl.construct(factory, indexDefinition, config); - } -} diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchConnectionFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java similarity index 87% rename from oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchConnectionFactory.java rename to oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java index f1d554ff7dd..1e74c8c668c 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchConnectionFactory.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java @@ -29,14 +29,22 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantReadWriteLock; -public class ElasticsearchConnectionFactory implements Closeable { - private static final Logger LOG = LoggerFactory.getLogger(ElasticsearchConnectionFactory.class); - private final ConcurrentMap clientMap = new ConcurrentHashMap<>(); +public class ElasticsearchClientFactory implements Closeable { + private static final Logger LOG = LoggerFactory.getLogger(ElasticsearchClientFactory.class); + + public static final ElasticsearchClientFactory INSTANCE = new ElasticsearchClientFactory(); + private final ConcurrentMap clientMap = new ConcurrentHashMap<>(); private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); private final AtomicBoolean isClosed = new AtomicBoolean(); - public RestHighLevelClient getConnection(ElasticsearchCoordinate esCoord) { + private ElasticsearchClientFactory() { } + + public static ElasticsearchClientFactory getInstance() { + return INSTANCE; + } + + public RestHighLevelClient getClient(ElasticsearchCoordinate esCoord) { lock.readLock().lock(); try { if (isClosed.get()) { diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java index 97f87b5ef8a..53d36344246 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java @@ -16,18 +16,87 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch; -import org.elasticsearch.client.RestHighLevelClient; - -public interface ElasticsearchCoordinate { - String SCHEME_PROP = "elasticsearch.scheme"; - String DEFAULT_SCHEME = "http"; - String HOST_PROP = "elasticsearch.host"; - String DEFAULT_HOST = "127.0.0.1"; - String PORT_PROP = "elasticsearch.port"; - int DEFAULT_PORT = 9200; - - RestHighLevelClient getClient(); - String getScheme(); - String getHost(); - int getPort(); +import org.jetbrains.annotations.NotNull; + +import java.util.Map; +import java.util.Objects; + +public class ElasticsearchCoordinate { + + protected static final String SCHEME_PROP = "elasticsearch.scheme"; + protected static final String DEFAULT_SCHEME = "http"; + protected static final String HOST_PROP = "elasticsearch.host"; + protected static final String DEFAULT_HOST = "127.0.0.1"; + protected static final String PORT_PROP = "elasticsearch.port"; + protected static final int DEFAULT_PORT = 9200; + + protected static final ElasticsearchCoordinate DEFAULT = + new ElasticsearchCoordinate(DEFAULT_SCHEME, DEFAULT_HOST, DEFAULT_PORT); + + private final String scheme; + private final String host; + private final int port; + + protected ElasticsearchCoordinate(String scheme, String host, Integer port) { + if (scheme == null || host == null || port == null) { + throw new IllegalArgumentException(); + } + this.scheme = scheme; + this.host = host; + this.port = port; + } + + public String getScheme() { + return scheme; + } + + public String getHost() { + return host; + } + + public int getPort() { + return port; + } + + public boolean equals(Object o) { + if (!(o instanceof ElasticsearchCoordinate)) { + return false; + } + + ElasticsearchCoordinate other = (ElasticsearchCoordinate)o; + return hashCode() == other.hashCode() // just to have a quicker comparison + && getScheme().equals(other.getScheme()) + && getHost().equals(other.getHost()) + && getPort() == other.getPort(); + } + + @Override + public int hashCode() { + return Objects.hash(getScheme(), getHost(), getPort()); + } + + @Override + public String toString() { + return getScheme() + "://" + getHost() + ":" + getPort(); + } + + public static ElasticsearchCoordinate build(@NotNull Map config) { + ElasticsearchCoordinate coordinate = null; + Object p = config.get(PORT_PROP); + if (p != null) { + try { + Integer port = Integer.parseInt(p.toString()); + coordinate = build((String) config.get(SCHEME_PROP), (String) config.get(HOST_PROP), port); + } catch (NumberFormatException nfe) { /* ignore */ } + } + return coordinate; + } + + private static ElasticsearchCoordinate build(String scheme, String host, Integer port) { + if (scheme == null || host == null || port == null) { + return null; + } + + return new ElasticsearchCoordinate(scheme, host, port); + } } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinateImpl.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinateImpl.java deleted file mode 100644 index beeaedabafa..00000000000 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinateImpl.java +++ /dev/null @@ -1,138 +0,0 @@ -/* - * 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.elasticsearch; - -import org.apache.jackrabbit.oak.plugins.index.search.util.ConfigUtil; -import org.apache.jackrabbit.oak.spi.state.NodeState; -import org.elasticsearch.client.RestHighLevelClient; - -import java.util.Map; -import java.util.Objects; - -public class ElasticsearchCoordinateImpl implements ElasticsearchCoordinate { - private final ElasticsearchConnectionFactory connectionFactory; - private final String scheme; - private final String host; - private final int port; - - ElasticsearchCoordinateImpl(ElasticsearchConnectionFactory connectionFactory, - String scheme, String host, int port) { - this.connectionFactory = connectionFactory; - this.scheme = scheme; - this.host = host; - this.port = port; - } - - static ElasticsearchCoordinate construct(ElasticsearchConnectionFactory connectionFactory, - NodeState indexDefn, Map configMap) { - // index defn is at highest prio - ElasticsearchCoordinate esCoord = readFrom(connectionFactory, indexDefn); - if (esCoord != null) { - return esCoord; - } - - // command line comes next - esCoord = construct(connectionFactory, - System.getProperty(SCHEME_PROP), System.getProperty(HOST_PROP), Integer.getInteger(PORT_PROP)); - if (esCoord != null) { - return esCoord; - } - - // config map - if (configMap != null) { - Integer port = null; - try { - port = Integer.parseInt(configMap.get(PORT_PROP)); - } catch (NumberFormatException nfe) { - // ignore - } - esCoord = construct(connectionFactory, configMap.get(SCHEME_PROP), configMap.get(HOST_PROP), port); - if (esCoord != null) { - return esCoord; - } - } - - return new ElasticsearchCoordinateImpl(connectionFactory, DEFAULT_SCHEME, DEFAULT_HOST, DEFAULT_PORT); - } - - @Override - public RestHighLevelClient getClient() { - return connectionFactory.getConnection(this); - } - - @Override - public String getScheme() { - return scheme; - } - - @Override - public String getHost() { - return host; - } - - @Override - public int getPort() { - return port; - } - - @Override - public boolean equals(Object o) { - if (!(o instanceof ElasticsearchCoordinate)) { - return false; - } - - ElasticsearchCoordinate other = (ElasticsearchCoordinate)o; - return hashCode() == other.hashCode() // just to have a quicker comparison - && getScheme().equals(other.getScheme()) - && getHost().equals(other.getHost()) - && getPort() == other.getPort(); - } - - @Override - public int hashCode() { - return Objects.hash(getScheme(), getHost(), getPort()); - } - - @Override - public String toString() { - return getScheme() + "://" + getHost() + ":" + getPort(); - } - - private static ElasticsearchCoordinate readFrom(ElasticsearchConnectionFactory factory, NodeState definition) { - if (definition == null - || !definition.hasProperty(SCHEME_PROP) - || !definition.hasProperty(HOST_PROP) - || !definition.hasProperty(PORT_PROP)) { - return null; - } - - String scheme = ConfigUtil.getOptionalValue(definition, SCHEME_PROP, DEFAULT_SCHEME); - String host = ConfigUtil.getOptionalValue(definition, HOST_PROP, DEFAULT_HOST); - int port = ConfigUtil.getOptionalValue(definition, PORT_PROP, DEFAULT_PORT); - - return new ElasticsearchCoordinateImpl(factory, scheme, host, port); - } - - private static ElasticsearchCoordinate construct(ElasticsearchConnectionFactory factory, - String scheme, String host, Integer port) { - if (scheme == null || host == null || port == null) { - return null; - } - - return new ElasticsearchCoordinateImpl(factory, scheme, host, port); - } -} diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinate.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinate.java deleted file mode 100644 index 7e63aa0d8b3..00000000000 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinate.java +++ /dev/null @@ -1,24 +0,0 @@ -/* - * 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.elasticsearch; - -import org.elasticsearch.client.RestHighLevelClient; - -public interface ElasticsearchIndexCoordinate { - RestHighLevelClient getClient(); - String getEsIndexName(); -} diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinateFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinateFactory.java deleted file mode 100644 index 5b9626e21a5..00000000000 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinateFactory.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * 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.elasticsearch; - -import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; - -public interface ElasticsearchIndexCoordinateFactory { - ElasticsearchIndexCoordinate getElasticsearchIndexCoordinate(IndexDefinition indexDefinition); -} diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinateImpl.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexDescriptor.java similarity index 58% rename from oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinateImpl.java rename to oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexDescriptor.java index 2e4f94a0087..a80a4c72901 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexCoordinateImpl.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexDescriptor.java @@ -18,6 +18,7 @@ import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; +import org.apache.jackrabbit.oak.spi.state.NodeState; import org.elasticsearch.client.RestHighLevelClient; import org.jetbrains.annotations.NotNull; @@ -28,42 +29,60 @@ import static org.elasticsearch.common.Strings.INVALID_FILENAME_CHARS; -public class ElasticsearchIndexCoordinateImpl implements ElasticsearchIndexCoordinate { +public class ElasticsearchIndexDescriptor { private static final int MAX_NAME_LENGTH = 255; - private final ElasticsearchCoordinate esCoord; - private final String esIndexName; + private static final String INVALID_CHARS_REGEX = Pattern.quote(INVALID_FILENAME_CHARS + .stream() + .map(Object::toString) + .collect(Collectors.joining(""))); - ElasticsearchIndexCoordinateImpl(@NotNull ElasticsearchCoordinate esCoord, IndexDefinition indexDefinition) { - this.esCoord = esCoord; - esIndexName = getRemoteIndexName(indexDefinition); + private final ElasticsearchCoordinate coordinate; + private final String indexName; + + public ElasticsearchIndexDescriptor(IndexDefinition indexDefinition, @NotNull ElasticsearchCoordinate defaultCoordinate) { + ElasticsearchCoordinate c = build(indexDefinition); + coordinate = c != null ? c : defaultCoordinate; + indexName = getRemoteIndexName(indexDefinition); } - @Override public RestHighLevelClient getClient() { - return esCoord.getClient(); + return ElasticsearchClientFactory.getInstance().getClient(coordinate); } - @Override - public String getEsIndexName() { - return esIndexName; + public String getIndexName() { + return indexName; } @Override public int hashCode() { - return Objects.hash(esCoord, esIndexName); + return Objects.hash(coordinate, indexName); } @Override public boolean equals(Object o) { - if (! (o instanceof ElasticsearchIndexCoordinateImpl)) { - return false; + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ElasticsearchIndexDescriptor that = (ElasticsearchIndexDescriptor) o; + return coordinate.equals(that.coordinate) && + indexName.equals(that.indexName); + } + + private ElasticsearchCoordinate build(IndexDefinition definition) { + final NodeState node = definition.getDefinitionNodeState(); + if (node == null + || !node.hasProperty(ElasticsearchCoordinate.SCHEME_PROP) + || !node.hasProperty(ElasticsearchCoordinate.HOST_PROP) + || !node.hasProperty(ElasticsearchCoordinate.PORT_PROP)) { + return null; } - ElasticsearchIndexCoordinateImpl other = (ElasticsearchIndexCoordinateImpl)o; - return hashCode() == other.hashCode() - && esCoord.equals(other.esCoord) - && esIndexName.equals(other.esIndexName); + + String scheme = node.getString(ElasticsearchCoordinate.SCHEME_PROP); + String host = node.getString(ElasticsearchCoordinate.HOST_PROP); + int port = (int) node.getLong(ElasticsearchCoordinate.PORT_PROP); + + return new ElasticsearchCoordinate(scheme, host, port); } private String getRemoteIndexName(IndexDefinition definition) { @@ -90,7 +109,7 @@ private static String getESSafeIndexName(String indexPath) { .stream(PathUtils.elements(indexPath).spliterator(), false) .limit(3) //Max 3 nodeNames including oak:index which is the immediate parent for any indexPath .filter(p -> !"oak:index".equals(p)) - .map(ElasticsearchIndexCoordinateImpl::getESSafeName) + .map(ElasticsearchIndexDescriptor::getESSafeName) .collect(Collectors.joining("_")); if (name.length() > MAX_NAME_LENGTH){ @@ -104,11 +123,7 @@ private static String getESSafeIndexName(String indexPath) { * Ref: https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-create-index.html */ private static String getESSafeName(String suggestedIndexName) { - String invalidCharsRegex = Pattern.quote(INVALID_FILENAME_CHARS - .stream() - .map(Object::toString) - .collect(Collectors.joining(""))); - return suggestedIndexName.replaceAll(invalidCharsRegex, "").toLowerCase(); + return suggestedIndexName.replaceAll(INVALID_CHARS_REGEX, "").toLowerCase(); } } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java index 2e407a3e87e..aee1d988f66 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java @@ -40,10 +40,10 @@ import java.io.File; import java.util.ArrayList; import java.util.Dictionary; -import java.util.HashMap; import java.util.Hashtable; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import static org.apache.commons.io.FileUtils.ONE_MB; import static org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUtils.registerMBean; @@ -81,25 +81,22 @@ ) private static final String PROP_PRE_EXTRACTED_TEXT_ALWAYS_USE = "alwaysUsePreExtractedCache"; - private static final String PROP_ELASTICSEARCH_SCHEME_DEFAULT = "http"; -// @Property( -// value = PROP_ELASTICSEARCH_SCHEME_DEFAULT, -// label = "Elasticsearch connection scheme" -// ) + @Property( + value = ElasticsearchCoordinate.DEFAULT_SCHEME, + label = "Elasticsearch connection scheme" + ) private static final String PROP_ELASTICSEARCH_SCHEME = "elasticsearch.scheme"; - private static final String PROP_ELASTICSEARCH_HOST_DEFAULT = "localhost"; -// @Property( -// value = PROP_ELASTICSEARCH_HOST_DEFAULT, -// label = "Elasticsearch connection host" -// ) + @Property( + value = ElasticsearchCoordinate.DEFAULT_HOST, + label = "Elasticsearch connection host" + ) private static final String PROP_ELASTICSEARCH_HOST = "elasticsearch.host"; - private static final int PROP_ELASTICSEARCH_PORT_DEFAULT = 9200; -// @Property( -// intValue = PROP_ELASTICSEARCH_PORT_DEFAULT, -// label = "Elasticsearch connection port" -// ) + @Property( + intValue = ElasticsearchCoordinate.DEFAULT_PORT, + label = "Elasticsearch connection port" + ) private static final String PROP_ELASTICSEARCH_PORT = "elasticsearch.port"; @Property( @@ -119,8 +116,6 @@ private ExtractedTextCache extractedTextCache; - private ElasticsearchConnectionFactory connectionFactory = null; - private final List regs = new ArrayList<>(); private final List oakRegs = new ArrayList<>(); @@ -134,11 +129,10 @@ private void activate(BundleContext bundleContext, Map config) { initializeTextExtractionDir(bundleContext, config); initializeExtractedTextCache(config, statisticsProvider); - connectionFactory = new ElasticsearchConnectionFactory(); - ElasticsearchIndexCoordinateFactory esIndexCoordFactory = getElasticsearchIndexCoordinateFactory(config); + final ElasticsearchCoordinate coordinate = getElasticsearchCoordinate(config); - registerIndexProvider(bundleContext, esIndexCoordFactory); - registerIndexEditor(bundleContext, esIndexCoordFactory); + registerIndexProvider(bundleContext, coordinate); + registerIndexEditor(bundleContext, coordinate); } @Deactivate @@ -151,24 +145,23 @@ private void deactivate() { reg.unregister(); } - IOUtils.closeQuietly(connectionFactory); - connectionFactory = null; + IOUtils.closeQuietly(ElasticsearchClientFactory.getInstance()); if (extractedTextCache != null) { extractedTextCache.close(); } } - private void registerIndexProvider(BundleContext bundleContext, ElasticsearchIndexCoordinateFactory esIndexCoordFactory) { - ElasticsearchIndexProvider indexProvider = new ElasticsearchIndexProvider(esIndexCoordFactory); + private void registerIndexProvider(BundleContext bundleContext, ElasticsearchCoordinate coordinate) { + ElasticsearchIndexProvider indexProvider = new ElasticsearchIndexProvider(coordinate); Dictionary props = new Hashtable<>(); props.put("type", ElasticsearchIndexConstants.TYPE_ELASTICSEARCH); regs.add(bundleContext.registerService(IndexEditorProvider.class.getName(), indexProvider, props)); } - private void registerIndexEditor(BundleContext bundleContext, ElasticsearchIndexCoordinateFactory esIndexCoordFactory) { - ElasticsearchIndexEditorProvider editorProvider = new ElasticsearchIndexEditorProvider(esIndexCoordFactory, extractedTextCache); + private void registerIndexEditor(BundleContext bundleContext, ElasticsearchCoordinate coordinate) { + ElasticsearchIndexEditorProvider editorProvider = new ElasticsearchIndexEditorProvider(coordinate, extractedTextCache); Dictionary props = new Hashtable<>(); props.put("type", ElasticsearchIndexConstants.TYPE_ELASTICSEARCH); @@ -238,16 +231,20 @@ private void registerExtractedTextProvider(PreExtractedTextProvider provider){ } } - private ElasticsearchIndexCoordinateFactory getElasticsearchIndexCoordinateFactory(Map config) { - ElasticsearchIndexCoordinateFactory esIndexCoordFactory; - Map esCfg = new HashMap<>(); - esCfg.put(ElasticsearchCoordinate.SCHEME_PROP, - PropertiesUtil.toString(config.get(PROP_ELASTICSEARCH_SCHEME), PROP_ELASTICSEARCH_SCHEME_DEFAULT)); - esCfg.put(ElasticsearchCoordinate.HOST_PROP, - PropertiesUtil.toString(config.get(PROP_ELASTICSEARCH_HOST), PROP_ELASTICSEARCH_HOST_DEFAULT)); - esCfg.put(ElasticsearchCoordinate.PORT_PROP, String.valueOf( - PropertiesUtil.toInteger(config.get(PROP_ELASTICSEARCH_PORT), PROP_ELASTICSEARCH_PORT_DEFAULT))); - esIndexCoordFactory = new DefaultElasticsearchIndexCoordinateFactory(connectionFactory, esCfg); - return esIndexCoordFactory; + private ElasticsearchCoordinate getElasticsearchCoordinate(Map contextConfig) { + // system properties have priority + ElasticsearchCoordinate coordinate = ElasticsearchCoordinate.build(System.getProperties().entrySet() + .stream() + .collect(Collectors.toMap( + e -> String.valueOf(e.getKey()), + e -> String.valueOf(e.getValue())) + ) + ); + + if (coordinate == null) { + coordinate = ElasticsearchCoordinate.build(contextConfig); + } + + return coordinate != null ? coordinate : ElasticsearchCoordinate.DEFAULT; } } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java index f3977ffbd69..28c30f4ae43 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java @@ -21,7 +21,7 @@ import org.apache.jackrabbit.oak.plugins.index.IndexEditorProvider; import org.apache.jackrabbit.oak.plugins.index.IndexUpdateCallback; import org.apache.jackrabbit.oak.plugins.index.IndexingContext; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinateFactory; +import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchCoordinate; import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexDefinition; import org.apache.jackrabbit.oak.plugins.index.search.ExtractedTextCache; import org.apache.jackrabbit.oak.spi.commit.Editor; @@ -34,12 +34,12 @@ public class ElasticsearchIndexEditorProvider implements IndexEditorProvider { - private final ElasticsearchIndexCoordinateFactory esIndexCoordFactory; + private final ElasticsearchCoordinate defaultCoordinate; private final ExtractedTextCache extractedTextCache; - public ElasticsearchIndexEditorProvider(@NotNull ElasticsearchIndexCoordinateFactory esIndexCoordFactory, + public ElasticsearchIndexEditorProvider(@NotNull ElasticsearchCoordinate defaultCoordinate, ExtractedTextCache extractedTextCache) { - this.esIndexCoordFactory = esIndexCoordFactory; + this.defaultCoordinate = defaultCoordinate; this.extractedTextCache = extractedTextCache != null ? extractedTextCache : new ExtractedTextCache(0, 0); } @@ -56,7 +56,7 @@ public ElasticsearchIndexEditorProvider(@NotNull ElasticsearchIndexCoordinateFac ElasticsearchIndexDefinition indexDefinition = new ElasticsearchIndexDefinition(root, definition.getNodeState(), indexPath); - ElasticsearchIndexWriterFactory writerFactory = new ElasticsearchIndexWriterFactory(esIndexCoordFactory); + ElasticsearchIndexWriterFactory writerFactory = new ElasticsearchIndexWriterFactory(defaultCoordinate); ElasticsearchIndexEditorContext context = new ElasticsearchIndexEditorContext(root, definition, indexDefinition, diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java index 348c2a516a7..3ad4f8dd604 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java @@ -16,8 +16,8 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch.index; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinateFactory; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinate; +import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchCoordinate; +import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexDescriptor; import org.apache.jackrabbit.oak.plugins.index.search.FieldNames; import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; import org.apache.jackrabbit.oak.plugins.index.search.spi.editor.FulltextIndexWriter; @@ -26,7 +26,6 @@ import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.client.RequestOptions; -import org.elasticsearch.client.RestHighLevelClient; import org.elasticsearch.client.indices.CreateIndexRequest; import org.elasticsearch.client.indices.CreateIndexResponse; import org.elasticsearch.common.Strings; @@ -49,17 +48,15 @@ public class ElasticsearchIndexWriter implements FulltextIndexWriter { private static final Logger LOG = LoggerFactory.getLogger(ElasticsearchIndexWriter.class); - private final ElasticsearchIndexCoordinate esIndexCoord; - private final RestHighLevelClient client; + private final ElasticsearchIndexDescriptor indexDescriptor; private boolean shouldProvisionIndex; private final boolean isAsync; // TODO: use bulk API - https://www.elastic.co/guide/en/elasticsearch/client/java-api/current/java-docs-bulk-processor.html ElasticsearchIndexWriter(@NotNull IndexDefinition indexDefinition, - ElasticsearchIndexCoordinateFactory esIndexCoordFactory) { - esIndexCoord = esIndexCoordFactory.getElasticsearchIndexCoordinate(indexDefinition); - client = esIndexCoord.getClient(); + @NotNull ElasticsearchCoordinate defaultCoordinate) { + indexDescriptor = new ElasticsearchIndexDescriptor(indexDefinition, defaultCoordinate); // TODO: ES indexing put another bit delay before docs appear in search. // For test without "async" indexing, we can use following hack BUT those where we @@ -72,27 +69,27 @@ @Override public void updateDocument(String path, ElasticsearchDocument doc) throws IOException { provisionIndex(); - IndexRequest request = new IndexRequest(esIndexCoord.getEsIndexName()) + IndexRequest request = new IndexRequest(indexDescriptor.getIndexName()) .id(pathToId(path)) // immediate refresh would slow indexing response such that next // search would see the effect of this indexed doc. Must only get // enabled in tests (hopefully there are no non-async indexes in real life) .setRefreshPolicy(isAsync ? NONE : IMMEDIATE) .source(doc.build(), XContentType.JSON); - IndexResponse response = client.index(request, RequestOptions.DEFAULT); + IndexResponse response = indexDescriptor.getClient().index(request, RequestOptions.DEFAULT); LOG.trace("update {} - {}. Response: {}", path, doc, response); } @Override public void deleteDocuments(String path) throws IOException { provisionIndex(); - DeleteRequest request = new DeleteRequest(esIndexCoord.getEsIndexName()) + DeleteRequest request = new DeleteRequest(indexDescriptor.getIndexName()) .id(pathToId(path)) // immediate refresh would slow indexing response such that next // search would see the effect of this indexed doc. Must only get // enabled in tests (hopefully there are no non-async indexes in real life) .setRefreshPolicy(isAsync ? NONE : IMMEDIATE); - DeleteResponse response = client.delete(request, RequestOptions.DEFAULT); + DeleteResponse response = indexDescriptor.getClient().delete(request, RequestOptions.DEFAULT); LOG.trace("delete {}. Response: {}", path, response); } @@ -119,7 +116,7 @@ private void provisionIndex() throws IOException { } try { - CreateIndexRequest request = new CreateIndexRequest(esIndexCoord.getEsIndexName()); + CreateIndexRequest request = new CreateIndexRequest(indexDescriptor.getIndexName()); // provision settings request.settings(Settings.builder() @@ -157,7 +154,7 @@ private void provisionIndex() throws IOException { request.mapping(mappingBuilder); String requestMsg = Strings.toString(request.toXContent(jsonBuilder(), EMPTY_PARAMS)); - CreateIndexResponse response = client.indices().create(request, RequestOptions.DEFAULT); + CreateIndexResponse response = indexDescriptor.getClient().indices().create(request, RequestOptions.DEFAULT); LOG.info("Updated settings {}. Response acknowledged: {}", requestMsg, response.isAcknowledged()); } finally { diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriterFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriterFactory.java index c9e3ac0c977..8403d34bdc2 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriterFactory.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriterFactory.java @@ -16,21 +16,21 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch.index; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinateFactory; +import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchCoordinate; import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; import org.apache.jackrabbit.oak.plugins.index.search.spi.editor.FulltextIndexWriterFactory; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.jetbrains.annotations.NotNull; public class ElasticsearchIndexWriterFactory implements FulltextIndexWriterFactory { - private final ElasticsearchIndexCoordinateFactory esIndexCoordFactory; + private final ElasticsearchCoordinate defaultCoordinate; - ElasticsearchIndexWriterFactory(@NotNull ElasticsearchIndexCoordinateFactory esIndexCoordFactory) { - this.esIndexCoordFactory = esIndexCoordFactory; + ElasticsearchIndexWriterFactory(@NotNull ElasticsearchCoordinate defaultCoordinate) { + this.defaultCoordinate = defaultCoordinate; } @Override public ElasticsearchIndexWriter newInstance(IndexDefinition definition, NodeBuilder definitionBuilder, boolean reindex) { - return new ElasticsearchIndexWriter(definition, esIndexCoordFactory); + return new ElasticsearchIndexWriter(definition, defaultCoordinate); } } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java index ed6c8f87f34..a63fae69c0e 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java @@ -16,7 +16,7 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch.query; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinateFactory; +import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchCoordinate; import org.apache.jackrabbit.oak.plugins.index.search.IndexNode; import org.apache.jackrabbit.oak.plugins.index.search.SizeEstimator; import org.apache.jackrabbit.oak.plugins.index.search.util.LMSEstimator; @@ -45,11 +45,11 @@ // higher than some threshold below which the query should rather be answered by something else if possible private static final double MIN_COST = 100.1; - private final ElasticsearchIndexCoordinateFactory esIndexCoordFactory; + private final ElasticsearchCoordinate defaultCoordinate; private final NodeState root; - ElasticsearchIndex(@NotNull ElasticsearchIndexCoordinateFactory esIndexCoordFactory, @NotNull NodeState root) { - this.esIndexCoordFactory = esIndexCoordFactory; + ElasticsearchIndex(@NotNull ElasticsearchCoordinate defaultCoordinate, @NotNull NodeState root) { + this.defaultCoordinate = defaultCoordinate; this.root = root; } @@ -85,9 +85,7 @@ protected ElasticsearchIndexNode acquireIndexNode(IndexPlan plan) { @Override protected IndexNode acquireIndexNode(String indexPath) { - ElasticsearchIndexNode elasticsearchIndexNode = ElasticsearchIndexNode.fromIndexPath(root, indexPath); - elasticsearchIndexNode.setFactory(esIndexCoordFactory); - return elasticsearchIndexNode; + return new ElasticsearchIndexNode(root, indexPath, defaultCoordinate); } @Override @@ -104,7 +102,7 @@ public Cursor query(IndexPlan plan, NodeState rootState) { final FulltextIndexPlanner.PlanResult pr = getPlanResult(plan); QueryLimits settings = filter.getQueryLimits(); - Iterator itr = new ElasticsearchResultRowIterator(esIndexCoordFactory, filter, pr, plan, + Iterator itr = new ElasticsearchResultRowIterator(filter, pr, plan, acquireIndexNode(plan), FulltextIndex::shouldInclude, getEstimator(plan.getPlanName())); SizeEstimator sizeEstimator = getSizeEstimator(plan); diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexNode.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexNode.java index 1a90adee0cb..051ef1617c4 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexNode.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexNode.java @@ -16,8 +16,9 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch.query; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinateFactory; +import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchCoordinate; import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexDefinition; +import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexDescriptor; import org.apache.jackrabbit.oak.plugins.index.search.IndexNode; import org.apache.jackrabbit.oak.plugins.index.search.IndexStatistics; import org.apache.jackrabbit.oak.spi.state.NodeState; @@ -28,16 +29,12 @@ public class ElasticsearchIndexNode implements IndexNode { private final ElasticsearchIndexDefinition indexDefinition; - private ElasticsearchIndexCoordinateFactory factory; + private final ElasticsearchIndexDescriptor indexDescriptor; - static ElasticsearchIndexNode fromIndexPath(@NotNull NodeState root, @NotNull String indexPath) { - NodeState indexNS = NodeStateUtils.getNode(root, indexPath); - ElasticsearchIndexDefinition indexDefinition = new ElasticsearchIndexDefinition(root, indexNS, indexPath); - return new ElasticsearchIndexNode(indexDefinition); - } - - private ElasticsearchIndexNode(ElasticsearchIndexDefinition indexDefinition) { - this.indexDefinition = indexDefinition; + protected ElasticsearchIndexNode(@NotNull NodeState root, @NotNull String indexPath, @NotNull ElasticsearchCoordinate defaultCoordinate) { + final NodeState indexNS = NodeStateUtils.getNode(root, indexPath); + this.indexDefinition = new ElasticsearchIndexDefinition(root, indexNS, indexPath); + this.indexDescriptor = new ElasticsearchIndexDescriptor(indexDefinition, defaultCoordinate); } @Override @@ -50,6 +47,10 @@ public ElasticsearchIndexDefinition getDefinition() { return indexDefinition; } + public ElasticsearchIndexDescriptor getIndexDescriptor() { + return indexDescriptor; + } + @Override public int getIndexNodeId() { // TODO: does it matter that we simply return 0 as there's no observation based _refresh_ going on here @@ -59,10 +60,6 @@ public int getIndexNodeId() { @Override public @Nullable IndexStatistics getIndexStatistics() { - return new ElasticsearchIndexStatistics(factory.getElasticsearchIndexCoordinate(indexDefinition)); - } - - public void setFactory(ElasticsearchIndexCoordinateFactory factory) { - this.factory = factory; + return new ElasticsearchIndexStatistics(indexDescriptor); } } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java index daac23908a5..0e76c015f4e 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java @@ -16,7 +16,7 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch.query; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinateFactory; +import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchCoordinate; import org.apache.jackrabbit.oak.spi.query.QueryIndex; import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider; import org.apache.jackrabbit.oak.spi.state.NodeState; @@ -26,14 +26,14 @@ import java.util.List; public class ElasticsearchIndexProvider implements QueryIndexProvider { - private final ElasticsearchIndexCoordinateFactory esIndexCoordFactory; + private final ElasticsearchCoordinate defaultCoordinate; - public ElasticsearchIndexProvider(@NotNull ElasticsearchIndexCoordinateFactory esIndexCoordFactory) { - this.esIndexCoordFactory = esIndexCoordFactory; + public ElasticsearchIndexProvider(ElasticsearchCoordinate defaultCoordinate) { + this.defaultCoordinate = defaultCoordinate; } @Override public @NotNull List getQueryIndexes(NodeState nodeState) { - return Collections.singletonList(new ElasticsearchIndex(esIndexCoordFactory, nodeState)); + return Collections.singletonList(new ElasticsearchIndex(defaultCoordinate, nodeState)); } } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexStatistics.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexStatistics.java index 35f85b59b7e..caf1eef994d 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexStatistics.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexStatistics.java @@ -16,7 +16,7 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch.query; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinate; +import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexDescriptor; import org.apache.jackrabbit.oak.plugins.index.search.IndexStatistics; import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.core.CountRequest; @@ -26,10 +26,10 @@ import java.io.IOException; public class ElasticsearchIndexStatistics implements IndexStatistics { - private final ElasticsearchIndexCoordinate elasticsearchIndexCoordinate; + private final ElasticsearchIndexDescriptor elasticsearchIndexDescriptor; - ElasticsearchIndexStatistics(ElasticsearchIndexCoordinate elasticsearchIndexCoordinate) { - this.elasticsearchIndexCoordinate = elasticsearchIndexCoordinate; + ElasticsearchIndexStatistics(ElasticsearchIndexDescriptor elasticsearchIndexDescriptor) { + this.elasticsearchIndexDescriptor = elasticsearchIndexDescriptor; } @Override @@ -37,7 +37,7 @@ public int numDocs() { CountRequest countRequest = new CountRequest(); countRequest.query(QueryBuilders.matchAllQuery()); try { - CountResponse count = elasticsearchIndexCoordinate.getClient().count(countRequest, RequestOptions.DEFAULT); + CountResponse count = elasticsearchIndexDescriptor.getClient().count(countRequest, RequestOptions.DEFAULT); return (int) count.getCount(); } catch (IOException e) { // ignore failure @@ -50,7 +50,7 @@ public int getDocCountFor(String key) { CountRequest countRequest = new CountRequest(); countRequest.query(QueryBuilders.existsQuery(key)); try { - CountResponse count = elasticsearchIndexCoordinate.getClient().count(countRequest, RequestOptions.DEFAULT); + CountResponse count = elasticsearchIndexDescriptor.getClient().count(countRequest, RequestOptions.DEFAULT); return (int) count.getCount(); } catch (IOException e) { // ignore failure diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java index 9bdd31d97e8..76efcf8cf46 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java @@ -19,7 +19,6 @@ import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.commons.PerfLogger; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinateFactory; import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexDefinition; import org.apache.jackrabbit.oak.plugins.index.search.FieldNames; import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; @@ -89,7 +88,6 @@ private int nextBatchSize = ELASTICSEARCH_QUERY_BATCH_SIZE; private boolean noDocs = false; - private final ElasticsearchIndexCoordinateFactory esIndexCoordFactory; private final Filter filter; private final PlanResult pr; private final IndexPlan plan; @@ -97,14 +95,12 @@ private final RowInclusionPredicate rowInclusionPredicate; private final LMSEstimator estimator; - ElasticsearchResultRowIterator(@NotNull ElasticsearchIndexCoordinateFactory esIndexCoordFactory, - @NotNull Filter filter, + ElasticsearchResultRowIterator(@NotNull Filter filter, @NotNull PlanResult pr, @NotNull IndexPlan plan, ElasticsearchIndexNode indexNode, RowInclusionPredicate rowInclusionPredicate, LMSEstimator estimator) { - this.esIndexCoordFactory = esIndexCoordFactory; this.filter = filter; this.pr = pr; this.plan = plan; @@ -203,7 +199,7 @@ private boolean loadDocs() { } private ElasticsearchSearcher getCurrentSearcher(ElasticsearchIndexNode indexNode) { - return new ElasticsearchSearcher(esIndexCoordFactory, indexNode); + return new ElasticsearchSearcher(indexNode); } private FulltextIndex.FulltextResultRow convertToRow(SearchHit hit) throws IOException { diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchSearcher.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchSearcher.java index 24a7cf6cb30..06e75a81971 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchSearcher.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchSearcher.java @@ -16,14 +16,11 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch.query; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinateFactory; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexCoordinate; -import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexDefinition; +import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexDescriptor; import org.apache.jackrabbit.oak.plugins.index.search.FieldNames; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.client.RequestOptions; -import org.elasticsearch.client.RestHighLevelClient; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.search.builder.SearchSourceBuilder; import org.jetbrains.annotations.NotNull; @@ -31,14 +28,10 @@ import java.io.IOException; public class ElasticsearchSearcher { - private final ElasticsearchIndexCoordinate esIndexCoord; - private final RestHighLevelClient client; + private final ElasticsearchIndexDescriptor indexDescriptor; - ElasticsearchSearcher(@NotNull ElasticsearchIndexCoordinateFactory esIndexCoordFactory, - @NotNull ElasticsearchIndexNode indexNode) { - ElasticsearchIndexDefinition defn = indexNode.getDefinition(); - esIndexCoord = esIndexCoordFactory.getElasticsearchIndexCoordinate(defn); - client = esIndexCoord.getClient(); + ElasticsearchSearcher(@NotNull ElasticsearchIndexNode indexNode) { + indexDescriptor = indexNode.getIndexDescriptor(); } public SearchResponse search(QueryBuilder query, int batchSize) throws IOException { @@ -49,9 +42,9 @@ public SearchResponse search(QueryBuilder query, int batchSize) throws IOExcepti .size(batchSize) ; - SearchRequest request = new SearchRequest(esIndexCoord.getEsIndexName()) + SearchRequest request = new SearchRequest(indexDescriptor.getIndexName()) .source(searchSourceBuilder); - return client.search(request, RequestOptions.DEFAULT); + return indexDescriptor.getClient().search(request, RequestOptions.DEFAULT); } } diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java index 462af1926db..210a0ffabf9 100644 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java @@ -51,20 +51,16 @@ public static final ElasticsearchContainer ELASTIC = new ElasticsearchContainer("docker.elastic.co/elasticsearch/elasticsearch:7.6.0"); - private ElasticsearchIndexCoordinateFactory eicf = indexDefinition -> { - ElasticsearchCoordinate esCoord = - new ElasticsearchCoordinateImpl( - new ElasticsearchConnectionFactory(), "http", ELASTIC.getContainerIpAddress(), - ELASTIC.getMappedPort(9200) - ); - return new ElasticsearchIndexCoordinateImpl(esCoord, indexDefinition); - }; - @Override protected ContentRepository createRepository() { - ElasticsearchIndexEditorProvider editorProvider = new ElasticsearchIndexEditorProvider(eicf, + ElasticsearchCoordinate coordinate = new ElasticsearchCoordinate( + ElasticsearchCoordinate.DEFAULT_SCHEME, + ELASTIC.getContainerIpAddress(), + ELASTIC.getMappedPort(ElasticsearchCoordinate.DEFAULT_PORT) + ); + ElasticsearchIndexEditorProvider editorProvider = new ElasticsearchIndexEditorProvider(coordinate, new ExtractedTextCache(10* FileUtils.ONE_MB, 100)); - ElasticsearchIndexProvider indexProvider = new ElasticsearchIndexProvider(eicf); + ElasticsearchIndexProvider indexProvider = new ElasticsearchIndexProvider(coordinate); // remove all indexes to avoid cost competition (essentially a TODO for fixing cost ES cost estimation) NodeBuilder builder = InitialContentHelper.INITIAL_CONTENT.builder(); @@ -105,8 +101,8 @@ public void indexSelection() throws Exception { assertThat(explain("select [jcr:path] from [nt:base] where [propc] = 'foo'"), containsString("elasticsearch:test2")); assertQuery(propaQuery, Arrays.asList("/test/a", "/test/b")); - assertQuery("select [jcr:path] from [nt:base] where [propa] = 'foo2'", Arrays.asList("/test/c")); - assertQuery("select [jcr:path] from [nt:base] where [propc] = 'foo'", Arrays.asList("/test/d")); + assertQuery("select [jcr:path] from [nt:base] where [propa] = 'foo2'", singletonList("/test/c")); + assertQuery("select [jcr:path] from [nt:base] where [propc] = 'foo'", singletonList("/test/d")); } //OAK-3825 From e36f64d726506982cb5a8daff541c9906533cab5 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Mon, 9 Mar 2020 18:06:28 +0100 Subject: [PATCH 08/18] style: reformat code + optimize imports --- .../ElasticsearchClientFactory.java | 13 +++--- .../ElasticsearchCoordinate.java | 2 +- .../ElasticsearchIndexDescriptor.java | 4 +- .../ElasticsearchIndexProviderService.java | 23 ++++++---- .../ElasticsearchIndexEditorContext.java | 2 +- .../ElasticsearchIndexEditorProvider.java | 2 +- .../index/ElasticsearchIndexWriter.java | 4 +- .../query/ElasticsearchIndex.java | 2 +- .../query/ElasticsearchResultRowIterator.java | 46 +++++++++++++------ .../query/ElasticsearchSearcher.java | 3 +- .../util/TermQueryBuilderFactory.java | 2 +- .../ElasticsearchPropertyIndexTest.java | 10 ++-- 12 files changed, 70 insertions(+), 43 deletions(-) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java index 1e74c8c668c..931741e1545 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java @@ -38,7 +38,8 @@ private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); private final AtomicBoolean isClosed = new AtomicBoolean(); - private ElasticsearchClientFactory() { } + private ElasticsearchClientFactory() { + } public static ElasticsearchClientFactory getInstance() { return INSTANCE; @@ -54,11 +55,11 @@ public RestHighLevelClient getClient(ElasticsearchCoordinate esCoord) { return clientMap.computeIfAbsent(esCoord, elasticsearchCoordinate -> { LOG.info("Creating client {}", elasticsearchCoordinate); return new RestHighLevelClient( - RestClient.builder( - new HttpHost(elasticsearchCoordinate.getHost(), - elasticsearchCoordinate.getPort(), - elasticsearchCoordinate.getScheme()) - )); + RestClient.builder( + new HttpHost(elasticsearchCoordinate.getHost(), + elasticsearchCoordinate.getPort(), + elasticsearchCoordinate.getScheme()) + )); }); } finally { lock.readLock().unlock(); diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java index 53d36344246..4f5141e04bf 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java @@ -63,7 +63,7 @@ public boolean equals(Object o) { return false; } - ElasticsearchCoordinate other = (ElasticsearchCoordinate)o; + ElasticsearchCoordinate other = (ElasticsearchCoordinate) o; return hashCode() == other.hashCode() // just to have a quicker comparison && getScheme().equals(other.getScheme()) && getHost().equals(other.getHost()) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexDescriptor.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexDescriptor.java index a80a4c72901..ca627231275 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexDescriptor.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexDescriptor.java @@ -101,7 +101,7 @@ private String getRemoteIndexName(IndexDefinition definition) { *
  • xy:abc -> xyabc
    • *
  • /oak:index/abc -> abc
    • * - * + *

    * The resulting file name would be truncated to MAX_NAME_LENGTH */ private static String getESSafeIndexName(String indexPath) { @@ -112,7 +112,7 @@ private static String getESSafeIndexName(String indexPath) { .map(ElasticsearchIndexDescriptor::getESSafeName) .collect(Collectors.joining("_")); - if (name.length() > MAX_NAME_LENGTH){ + if (name.length() > MAX_NAME_LENGTH) { name = name.substring(0, MAX_NAME_LENGTH); } return name; diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java index aee1d988f66..25ac621f246 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java @@ -17,7 +17,14 @@ package org.apache.jackrabbit.oak.plugins.index.elasticsearch; import org.apache.commons.io.FilenameUtils; -import org.apache.felix.scr.annotations.*; +import org.apache.felix.scr.annotations.Activate; +import org.apache.felix.scr.annotations.Component; +import org.apache.felix.scr.annotations.Deactivate; +import org.apache.felix.scr.annotations.Property; +import org.apache.felix.scr.annotations.Reference; +import org.apache.felix.scr.annotations.ReferenceCardinality; +import org.apache.felix.scr.annotations.ReferencePolicy; +import org.apache.felix.scr.annotations.ReferencePolicyOption; import org.apache.jackrabbit.oak.api.jmx.CacheStatsMBean; import org.apache.jackrabbit.oak.cache.CacheStats; import org.apache.jackrabbit.oak.commons.IOUtils; @@ -187,11 +194,11 @@ private void initializeExtractedTextCache(Map config, StatisticsProvi alwaysUsePreExtractedCache, textExtractionDir, statisticsProvider); - if (extractedTextProvider != null){ + if (extractedTextProvider != null) { registerExtractedTextProvider(extractedTextProvider); } CacheStats stats = extractedTextCache.getCacheStats(); - if (stats != null){ + if (stats != null) { oakRegs.add(registerMBean(whiteboard, CacheStatsMBean.class, stats, CacheStatsMBean.TYPE, stats.getName())); @@ -204,7 +211,7 @@ void initializeTextExtractionDir(BundleContext bundleContext, Map con String textExtractionDir = PropertiesUtil.toString(config.get(PROP_LOCAL_TEXT_EXTRACTION_DIR), null); if (textExtractionDir == null || textExtractionDir.trim().isEmpty()) { String repoHome = bundleContext.getProperty(REPOSITORY_HOME); - if (repoHome != null){ + if (repoHome != null) { textExtractionDir = FilenameUtils.concat(repoHome, "index"); } } @@ -217,13 +224,13 @@ void initializeTextExtractionDir(BundleContext bundleContext, Map con this.textExtractionDir = new File(textExtractionDir); } - private void registerExtractedTextProvider(PreExtractedTextProvider provider){ - if (extractedTextCache != null){ - if (provider != null){ + private void registerExtractedTextProvider(PreExtractedTextProvider provider) { + if (extractedTextCache != null) { + if (provider != null) { String usage = extractedTextCache.isAlwaysUsePreExtractedCache() ? "always" : "only during reindexing phase"; LOG.info("Registering PreExtractedTextProvider {} with extracted text cache. " + - "It would be used {}", provider, usage); + "It would be used {}", provider, usage); } else { LOG.info("Unregistering PreExtractedTextProvider with extracted text cache"); } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorContext.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorContext.java index ea04cab1332..94e49817ca4 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorContext.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorContext.java @@ -58,6 +58,6 @@ public void enableReindexMode() { @Override public ElasticsearchIndexWriter getWriter() { - return (ElasticsearchIndexWriter)super.getWriter(); + return (ElasticsearchIndexWriter) super.getWriter(); } } \ No newline at end of file diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java index 28c30f4ae43..0878472d9a1 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java @@ -50,7 +50,7 @@ public ElasticsearchIndexEditorProvider(@NotNull ElasticsearchCoordinate default if (TYPE_ELASTICSEARCH.equals(type)) { assert callback instanceof ContextAwareCallback : "callback instance not of type " + "ContextAwareCallback [" + callback + "]"; - IndexingContext indexingContext = ((ContextAwareCallback)callback).getIndexingContext(); + IndexingContext indexingContext = ((ContextAwareCallback) callback).getIndexingContext(); String indexPath = indexingContext.getIndexPath(); ElasticsearchIndexDefinition indexDefinition = diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java index 3ad4f8dd604..0e430d8e391 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java @@ -120,8 +120,8 @@ private void provisionIndex() throws IOException { // provision settings request.settings(Settings.builder() - .put("analysis.analyzer.ancestor_analyzer.type", "custom") - .put("analysis.analyzer.ancestor_analyzer.tokenizer", "path_hierarchy")); + .put("analysis.analyzer.ancestor_analyzer.type", "custom") + .put("analysis.analyzer.ancestor_analyzer.tokenizer", "path_hierarchy")); // provision mappings XContentBuilder mappingBuilder = XContentFactory.jsonBuilder(); diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java index a63fae69c0e..c7cf9a64ede 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java @@ -19,9 +19,9 @@ import org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchCoordinate; import org.apache.jackrabbit.oak.plugins.index.search.IndexNode; import org.apache.jackrabbit.oak.plugins.index.search.SizeEstimator; -import org.apache.jackrabbit.oak.plugins.index.search.util.LMSEstimator; import org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndex; import org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndexPlanner; +import org.apache.jackrabbit.oak.plugins.index.search.util.LMSEstimator; import org.apache.jackrabbit.oak.spi.query.Cursor; import org.apache.jackrabbit.oak.spi.query.Filter; import org.apache.jackrabbit.oak.spi.query.QueryLimits; diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java index 76efcf8cf46..ddc5a53e987 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchResultRowIterator.java @@ -23,13 +23,18 @@ import org.apache.jackrabbit.oak.plugins.index.search.FieldNames; import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; import org.apache.jackrabbit.oak.plugins.index.search.PropertyDefinition; -import org.apache.jackrabbit.oak.plugins.index.search.util.LMSEstimator; import org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndex; import org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndexPlanner.PlanResult; +import org.apache.jackrabbit.oak.plugins.index.search.util.LMSEstimator; import org.apache.jackrabbit.oak.spi.query.Filter; import org.apache.jackrabbit.oak.spi.query.QueryConstants; import org.apache.jackrabbit.oak.spi.query.QueryIndex.IndexPlan; -import org.apache.jackrabbit.oak.spi.query.fulltext.*; +import org.apache.jackrabbit.oak.spi.query.fulltext.FullTextAnd; +import org.apache.jackrabbit.oak.spi.query.fulltext.FullTextContains; +import org.apache.jackrabbit.oak.spi.query.fulltext.FullTextExpression; +import org.apache.jackrabbit.oak.spi.query.fulltext.FullTextOr; +import org.apache.jackrabbit.oak.spi.query.fulltext.FullTextTerm; +import org.apache.jackrabbit.oak.spi.query.fulltext.FullTextVisitor; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.QueryBuilder; @@ -58,11 +63,25 @@ import static org.apache.jackrabbit.oak.api.Type.STRING; import static org.apache.jackrabbit.oak.commons.PathUtils.denotesRoot; import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath; -import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.*; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newAncestorQuery; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newDepthQuery; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newMixinTypeQuery; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newNodeTypeQuery; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newNotNullPropQuery; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newNullPropQuery; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newPathQuery; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newPrefixPathQuery; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newPrefixQuery; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newPropertyRestrictionQuery; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newWildcardPathQuery; +import static org.apache.jackrabbit.oak.plugins.index.elasticsearch.util.TermQueryBuilderFactory.newWildcardQuery; import static org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndex.isNodePath; import static org.apache.jackrabbit.oak.spi.query.QueryConstants.JCR_PATH; import static org.apache.jackrabbit.util.ISO8601.parse; -import static org.elasticsearch.index.query.QueryBuilders.*; +import static org.elasticsearch.index.query.QueryBuilders.boolQuery; +import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; +import static org.elasticsearch.index.query.QueryBuilders.matchQuery; +import static org.elasticsearch.index.query.QueryBuilders.termQuery; public class ElasticsearchResultRowIterator implements Iterator { private static final Logger LOG = LoggerFactory @@ -121,6 +140,7 @@ public boolean hasNext() { /** * Loads the lucene documents in batches + * * @return true if any document is loaded */ private boolean loadDocs() { @@ -129,7 +149,7 @@ private boolean loadDocs() { return false; } - if(indexNode == null) { + if (indexNode == null) { throw new IllegalStateException("indexNode cannot be null"); } @@ -237,7 +257,7 @@ private ElasticsearchSearcher getCurrentSearcher(ElasticsearchIndexNode indexNod /** * Get the Elasticsearch query for the given filter. * - * @param plan index plan containing filter details + * @param plan index plan containing filter details * @param planResult * @return the Lucene query */ @@ -326,7 +346,7 @@ public boolean visit(FullTextAnd and) { if (x instanceof BoolQueryBuilder) { BoolQueryBuilder bq = (BoolQueryBuilder) x; if (bq.mustNot().size() == 1 - // no other clauses + // no other clauses && bq.should().isEmpty() && bq.must().isEmpty() && bq.filter().isEmpty()) { hasMustNot = true; q.mustNot(bq.mustNot().get(0)); @@ -448,7 +468,7 @@ private static QueryBuilder performAdditionalWraps(@NotNull List q /** * unwraps any NOT clauses from the provided boolean query into another boolean query. * - * @param input the query to be analysed for the existence of NOT clauses. Cannot be null. + * @param input the query to be analysed for the existence of NOT clauses. Cannot be null. * @param output the query where the unwrapped NOTs will be saved into. Cannot be null. * @return true if there where at least one unwrapped NOT. false otherwise. */ @@ -470,9 +490,9 @@ private static boolean unwrapMustNot(@NotNull BoolQueryBuilder input, @NotNull B } private static void addNonFullTextConstraints(List qs, - IndexPlan plan, PlanResult planResult) { + IndexPlan plan, PlanResult planResult) { final BiPredicate, String> any = (iterable, value) -> - StreamSupport.stream(iterable.spliterator(), false).anyMatch(value::equals); + StreamSupport.stream(iterable.spliterator(), false).anyMatch(value::equals); Filter filter = plan.getFilter(); IndexDefinition defn = planResult.indexDefinition; @@ -503,7 +523,7 @@ private static void addNonFullTextConstraints(List qs, // deduced if (planResult.isPathTransformed()) { String parentPathSegment = planResult.getParentPathSegment(); - if ( ! any.test(PathUtils.elements(parentPathSegment), "*")) { + if (!any.test(PathUtils.elements(parentPathSegment), "*")) { qs.add(newPathQuery(path + parentPathSegment)); } } else { @@ -521,7 +541,7 @@ private static void addNonFullTextConstraints(List qs, // deduced if (planResult.isPathTransformed()) { String parentPathSegment = planResult.getParentPathSegment(); - if ( ! any.test(PathUtils.elements(parentPathSegment), "*")) { + if (!any.test(PathUtils.elements(parentPathSegment), "*")) { qs.add(newPathQuery(getParentPath(path) + parentPathSegment)); } } else { @@ -651,7 +671,7 @@ private static void addReferenceConstraint(String uuid, List qs) { @Nullable private static QueryBuilder createQuery(String propertyName, Filter.PropertyRestriction pr, - PropertyDefinition defn) { + PropertyDefinition defn) { int propType = FulltextIndex.determinePropertyType(defn, pr); if (pr.isNullRestriction()) { diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchSearcher.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchSearcher.java index 06e75a81971..c69e954b4ec 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchSearcher.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchSearcher.java @@ -39,8 +39,7 @@ public SearchResponse search(QueryBuilder query, int batchSize) throws IOExcepti .query(query) .fetchSource(false) .storedField(FieldNames.PATH) - .size(batchSize) - ; + .size(batchSize); SearchRequest request = new SearchRequest(indexDescriptor.getIndexName()) .source(searchSourceBuilder); diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/util/TermQueryBuilderFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/util/TermQueryBuilderFactory.java index 5f807eff09b..67fa899a73f 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/util/TermQueryBuilderFactory.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/util/TermQueryBuilderFactory.java @@ -86,7 +86,7 @@ public static WildcardQueryBuilder newWildcardPathQuery(@NotNull String value) { return wildcardQuery(PATH, value); } - public static TermQueryBuilder newAncestorQuery(String path){ + public static TermQueryBuilder newAncestorQuery(String path) { return termQuery(FieldNames.ANCESTORS, preparePath(path)); } diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java index 210a0ffabf9..438a906504f 100644 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java @@ -59,7 +59,7 @@ protected ContentRepository createRepository() { ELASTIC.getMappedPort(ElasticsearchCoordinate.DEFAULT_PORT) ); ElasticsearchIndexEditorProvider editorProvider = new ElasticsearchIndexEditorProvider(coordinate, - new ExtractedTextCache(10* FileUtils.ONE_MB, 100)); + new ExtractedTextCache(10 * FileUtils.ONE_MB, 100)); ElasticsearchIndexProvider indexProvider = new ElasticsearchIndexProvider(coordinate); // remove all indexes to avoid cost competition (essentially a TODO for fixing cost ES cost estimation) @@ -107,7 +107,7 @@ public void indexSelection() throws Exception { //OAK-3825 @Test - public void nodeNameViaPropDefinition() throws Exception{ + public void nodeNameViaPropDefinition() throws Exception { //make index IndexDefinitionBuilder builder = createIndex(); builder.includedPaths("/test") @@ -141,7 +141,7 @@ public void nodeNameViaPropDefinition() throws Exception{ } @Test - public void emptyIndex() throws Exception{ + public void emptyIndex() throws Exception { setIndex("test1", createIndex("propa", "propb")); root.commit(); @@ -166,7 +166,7 @@ public void propertyExistenceQuery() throws Exception { assertQuery("select [jcr:path] from [nt:base] where propa is not null", Arrays.asList("/test/a", "/test/b")); } - private static IndexDefinitionBuilder createIndex(String ... propNames) { + private static IndexDefinitionBuilder createIndex(String... propNames) { IndexDefinitionBuilder builder = new ElasticsearchIndexDefinitionBuilder().noAsync(); IndexDefinitionBuilder.IndexRule indexRule = builder.indexRule("nt:base"); for (String propName : propNames) { @@ -179,7 +179,7 @@ private void setIndex(String idxName, IndexDefinitionBuilder builder) { builder.build(root.getTree("/").addChild(INDEX_DEFINITIONS_NAME).addChild(idxName)); } - private String explain(String query){ + private String explain(String query) { String explain = "explain " + query; return executeQuery(explain, "JCR-SQL2").get(0); } From 0cd7c1aba9e0b056e1e34b8578abf7c71aa8067c Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Mon, 9 Mar 2020 18:31:27 +0100 Subject: [PATCH 09/18] refactor: improved ElasticsearchCoordinate#equals --- .../elasticsearch/ElasticsearchCoordinate.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java index 4f5141e04bf..28328a76882 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java @@ -58,16 +58,14 @@ public int getPort() { return port; } + @Override public boolean equals(Object o) { - if (!(o instanceof ElasticsearchCoordinate)) { - return false; - } - - ElasticsearchCoordinate other = (ElasticsearchCoordinate) o; - return hashCode() == other.hashCode() // just to have a quicker comparison - && getScheme().equals(other.getScheme()) - && getHost().equals(other.getHost()) - && getPort() == other.getPort(); + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + ElasticsearchCoordinate that = (ElasticsearchCoordinate) o; + return port == that.port && + Objects.equals(scheme, that.scheme) && + Objects.equals(host, that.host); } @Override From a4022b1d4101e0d06ba4c27c31c914e613e0f480 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Wed, 11 Mar 2020 15:55:26 +0100 Subject: [PATCH 10/18] chore: add basic test around index provider service --- oak-search-elastic/pom.xml | 5 ++ .../ElasticsearchClientFactory.java | 7 --- .../ElasticsearchIndexProviderService.java | 5 +- ...ElasticsearchIndexProviderServiceTest.java | 60 +++++++++++++++++++ 4 files changed, 68 insertions(+), 9 deletions(-) create mode 100644 oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderServiceTest.java diff --git a/oak-search-elastic/pom.xml b/oak-search-elastic/pom.xml index 3722960a86e..f4355ba1d85 100644 --- a/oak-search-elastic/pom.xml +++ b/oak-search-elastic/pom.xml @@ -180,6 +180,11 @@ ${project.version} test + + org.apache.sling + org.apache.sling.testing.osgi-mock + test + org.hamcrest hamcrest-core diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java index 931741e1545..70f07123b8f 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java @@ -26,7 +26,6 @@ import java.io.IOException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantReadWriteLock; public class ElasticsearchClientFactory implements Closeable { @@ -36,7 +35,6 @@ private final ConcurrentMap clientMap = new ConcurrentHashMap<>(); private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); - private final AtomicBoolean isClosed = new AtomicBoolean(); private ElasticsearchClientFactory() { } @@ -48,10 +46,6 @@ public static ElasticsearchClientFactory getInstance() { public RestHighLevelClient getClient(ElasticsearchCoordinate esCoord) { lock.readLock().lock(); try { - if (isClosed.get()) { - throw new IllegalStateException("Already closed"); - } - return clientMap.computeIfAbsent(esCoord, elasticsearchCoordinate -> { LOG.info("Creating client {}", elasticsearchCoordinate); return new RestHighLevelClient( @@ -70,7 +64,6 @@ public RestHighLevelClient getClient(ElasticsearchCoordinate esCoord) { public void close() { lock.writeLock().lock(); try { - isClosed.set(true); clientMap.values().forEach(restHighLevelClient -> { try { restHighLevelClient.close(); diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java index 25ac621f246..4c8bdff4d7b 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java @@ -36,6 +36,7 @@ import org.apache.jackrabbit.oak.plugins.index.fulltext.PreExtractedTextProvider; import org.apache.jackrabbit.oak.plugins.index.search.ExtractedTextCache; import org.apache.jackrabbit.oak.plugins.index.search.TextExtractionStatsMBean; +import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider; import org.apache.jackrabbit.oak.spi.whiteboard.Registration; import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard; import org.apache.jackrabbit.oak.stats.StatisticsProvider; @@ -164,7 +165,7 @@ private void registerIndexProvider(BundleContext bundleContext, ElasticsearchCoo Dictionary props = new Hashtable<>(); props.put("type", ElasticsearchIndexConstants.TYPE_ELASTICSEARCH); - regs.add(bundleContext.registerService(IndexEditorProvider.class.getName(), indexProvider, props)); + regs.add(bundleContext.registerService(QueryIndexProvider.class.getName(), indexProvider, props)); } private void registerIndexEditor(BundleContext bundleContext, ElasticsearchCoordinate coordinate) { @@ -207,7 +208,7 @@ private void initializeExtractedTextCache(Map config, StatisticsProvi } } - void initializeTextExtractionDir(BundleContext bundleContext, Map config) { + private void initializeTextExtractionDir(BundleContext bundleContext, Map config) { String textExtractionDir = PropertiesUtil.toString(config.get(PROP_LOCAL_TEXT_EXTRACTION_DIR), null); if (textExtractionDir == null || textExtractionDir.trim().isEmpty()) { String repoHome = bundleContext.getProperty(REPOSITORY_HOME); diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderServiceTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderServiceTest.java new file mode 100644 index 00000000000..8e9a0cd4213 --- /dev/null +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderServiceTest.java @@ -0,0 +1,60 @@ +package org.apache.jackrabbit.oak.plugins.index.elasticsearch; + +import org.apache.jackrabbit.oak.osgi.OsgiWhiteboard; +import org.apache.jackrabbit.oak.plugins.index.IndexEditorProvider; +import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore; +import org.apache.jackrabbit.oak.spi.mount.MountInfoProvider; +import org.apache.jackrabbit.oak.spi.mount.Mounts; +import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider; +import org.apache.jackrabbit.oak.spi.state.NodeStore; +import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard; +import org.apache.jackrabbit.oak.stats.StatisticsProvider; +import org.apache.sling.testing.mock.osgi.MockOsgi; +import org.apache.sling.testing.mock.osgi.junit.OsgiContext; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.util.Collections; + +import static org.junit.Assert.assertNotNull; + +public class ElasticsearchIndexProviderServiceTest { + + @Rule + public final TemporaryFolder folder = new TemporaryFolder(new File("target")); + + @Rule + public final OsgiContext context = new OsgiContext(); + + private final ElasticsearchIndexProviderService service = new ElasticsearchIndexProviderService(); + + private MountInfoProvider mip; + private Whiteboard wb; + + @Before + public void setUp() { + mip = Mounts.newBuilder().build(); + context.registerService(MountInfoProvider.class, mip); + context.registerService(NodeStore.class, new MemoryNodeStore()); + context.registerService(StatisticsProvider.class, StatisticsProvider.NOOP); + + wb = new OsgiWhiteboard(context.bundleContext()); + MockOsgi.injectServices(service, context.bundleContext()); + } + + @Test + public void defaultSetup() throws Exception { + MockOsgi.activate(service, context.bundleContext(), + Collections.singletonMap("localTextExtractionDir", folder.newFolder("localTextExtractionDir").getAbsolutePath()) + ); + + assertNotNull(context.getService(QueryIndexProvider.class)); + assertNotNull(context.getService(IndexEditorProvider.class)); + + MockOsgi.deactivate(service, context.bundleContext()); + } + +} From 303ed4e7d6946957f543928ff38ca7b18d4a3ba6 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Wed, 11 Mar 2020 16:53:56 +0100 Subject: [PATCH 11/18] feat: remove logic to extract elastic coordinate from index definition --- .../ElasticsearchIndexDescriptor.java | 24 +++---------------- .../ElasticsearchIndexEditorProvider.java | 8 +++---- .../index/ElasticsearchIndexWriter.java | 4 ++-- .../ElasticsearchIndexWriterFactory.java | 8 +++---- .../query/ElasticsearchIndex.java | 8 +++---- .../query/ElasticsearchIndexNode.java | 5 ++-- .../query/ElasticsearchIndexProvider.java | 8 +++---- 7 files changed, 24 insertions(+), 41 deletions(-) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexDescriptor.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexDescriptor.java index ca627231275..8e8dc089154 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexDescriptor.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexDescriptor.java @@ -18,7 +18,6 @@ import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; -import org.apache.jackrabbit.oak.spi.state.NodeState; import org.elasticsearch.client.RestHighLevelClient; import org.jetbrains.annotations.NotNull; @@ -41,10 +40,9 @@ private final ElasticsearchCoordinate coordinate; private final String indexName; - public ElasticsearchIndexDescriptor(IndexDefinition indexDefinition, @NotNull ElasticsearchCoordinate defaultCoordinate) { - ElasticsearchCoordinate c = build(indexDefinition); - coordinate = c != null ? c : defaultCoordinate; - indexName = getRemoteIndexName(indexDefinition); + public ElasticsearchIndexDescriptor(IndexDefinition indexDefinition, @NotNull ElasticsearchCoordinate coordinate) { + this.coordinate = coordinate; + this.indexName = getRemoteIndexName(indexDefinition); } public RestHighLevelClient getClient() { @@ -69,22 +67,6 @@ public boolean equals(Object o) { indexName.equals(that.indexName); } - private ElasticsearchCoordinate build(IndexDefinition definition) { - final NodeState node = definition.getDefinitionNodeState(); - if (node == null - || !node.hasProperty(ElasticsearchCoordinate.SCHEME_PROP) - || !node.hasProperty(ElasticsearchCoordinate.HOST_PROP) - || !node.hasProperty(ElasticsearchCoordinate.PORT_PROP)) { - return null; - } - - String scheme = node.getString(ElasticsearchCoordinate.SCHEME_PROP); - String host = node.getString(ElasticsearchCoordinate.HOST_PROP); - int port = (int) node.getLong(ElasticsearchCoordinate.PORT_PROP); - - return new ElasticsearchCoordinate(scheme, host, port); - } - private String getRemoteIndexName(IndexDefinition definition) { String suffix = definition.getUniqueId(); diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java index 0878472d9a1..0fc00b4e384 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java @@ -34,12 +34,12 @@ public class ElasticsearchIndexEditorProvider implements IndexEditorProvider { - private final ElasticsearchCoordinate defaultCoordinate; + private final ElasticsearchCoordinate elasticsearchCoordinate; private final ExtractedTextCache extractedTextCache; - public ElasticsearchIndexEditorProvider(@NotNull ElasticsearchCoordinate defaultCoordinate, + public ElasticsearchIndexEditorProvider(@NotNull ElasticsearchCoordinate elasticsearchCoordinate, ExtractedTextCache extractedTextCache) { - this.defaultCoordinate = defaultCoordinate; + this.elasticsearchCoordinate = elasticsearchCoordinate; this.extractedTextCache = extractedTextCache != null ? extractedTextCache : new ExtractedTextCache(0, 0); } @@ -56,7 +56,7 @@ public ElasticsearchIndexEditorProvider(@NotNull ElasticsearchCoordinate default ElasticsearchIndexDefinition indexDefinition = new ElasticsearchIndexDefinition(root, definition.getNodeState(), indexPath); - ElasticsearchIndexWriterFactory writerFactory = new ElasticsearchIndexWriterFactory(defaultCoordinate); + ElasticsearchIndexWriterFactory writerFactory = new ElasticsearchIndexWriterFactory(elasticsearchCoordinate); ElasticsearchIndexEditorContext context = new ElasticsearchIndexEditorContext(root, definition, indexDefinition, diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java index 0e430d8e391..cdb5035e535 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java @@ -55,8 +55,8 @@ // TODO: use bulk API - https://www.elastic.co/guide/en/elasticsearch/client/java-api/current/java-docs-bulk-processor.html ElasticsearchIndexWriter(@NotNull IndexDefinition indexDefinition, - @NotNull ElasticsearchCoordinate defaultCoordinate) { - indexDescriptor = new ElasticsearchIndexDescriptor(indexDefinition, defaultCoordinate); + @NotNull ElasticsearchCoordinate elasticsearchCoordinate) { + indexDescriptor = new ElasticsearchIndexDescriptor(indexDefinition, elasticsearchCoordinate); // TODO: ES indexing put another bit delay before docs appear in search. // For test without "async" indexing, we can use following hack BUT those where we diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriterFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriterFactory.java index 8403d34bdc2..eafcbeb3701 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriterFactory.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriterFactory.java @@ -23,14 +23,14 @@ import org.jetbrains.annotations.NotNull; public class ElasticsearchIndexWriterFactory implements FulltextIndexWriterFactory { - private final ElasticsearchCoordinate defaultCoordinate; + private final ElasticsearchCoordinate elasticsearchCoordinate; - ElasticsearchIndexWriterFactory(@NotNull ElasticsearchCoordinate defaultCoordinate) { - this.defaultCoordinate = defaultCoordinate; + ElasticsearchIndexWriterFactory(@NotNull ElasticsearchCoordinate elasticsearchCoordinate) { + this.elasticsearchCoordinate = elasticsearchCoordinate; } @Override public ElasticsearchIndexWriter newInstance(IndexDefinition definition, NodeBuilder definitionBuilder, boolean reindex) { - return new ElasticsearchIndexWriter(definition, defaultCoordinate); + return new ElasticsearchIndexWriter(definition, elasticsearchCoordinate); } } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java index c7cf9a64ede..60e6329f5c9 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndex.java @@ -45,11 +45,11 @@ // higher than some threshold below which the query should rather be answered by something else if possible private static final double MIN_COST = 100.1; - private final ElasticsearchCoordinate defaultCoordinate; + private final ElasticsearchCoordinate elasticsearchCoordinate; private final NodeState root; - ElasticsearchIndex(@NotNull ElasticsearchCoordinate defaultCoordinate, @NotNull NodeState root) { - this.defaultCoordinate = defaultCoordinate; + ElasticsearchIndex(@NotNull ElasticsearchCoordinate elasticsearchCoordinate, @NotNull NodeState root) { + this.elasticsearchCoordinate = elasticsearchCoordinate; this.root = root; } @@ -85,7 +85,7 @@ protected ElasticsearchIndexNode acquireIndexNode(IndexPlan plan) { @Override protected IndexNode acquireIndexNode(String indexPath) { - return new ElasticsearchIndexNode(root, indexPath, defaultCoordinate); + return new ElasticsearchIndexNode(root, indexPath, elasticsearchCoordinate); } @Override diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexNode.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexNode.java index 051ef1617c4..5e0a1f249c9 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexNode.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexNode.java @@ -31,10 +31,11 @@ private final ElasticsearchIndexDefinition indexDefinition; private final ElasticsearchIndexDescriptor indexDescriptor; - protected ElasticsearchIndexNode(@NotNull NodeState root, @NotNull String indexPath, @NotNull ElasticsearchCoordinate defaultCoordinate) { + protected ElasticsearchIndexNode(@NotNull NodeState root, @NotNull String indexPath, + @NotNull ElasticsearchCoordinate elasticsearchCoordinate) { final NodeState indexNS = NodeStateUtils.getNode(root, indexPath); this.indexDefinition = new ElasticsearchIndexDefinition(root, indexNS, indexPath); - this.indexDescriptor = new ElasticsearchIndexDescriptor(indexDefinition, defaultCoordinate); + this.indexDescriptor = new ElasticsearchIndexDescriptor(indexDefinition, elasticsearchCoordinate); } @Override diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java index 0e76c015f4e..7ae849a102b 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/query/ElasticsearchIndexProvider.java @@ -26,14 +26,14 @@ import java.util.List; public class ElasticsearchIndexProvider implements QueryIndexProvider { - private final ElasticsearchCoordinate defaultCoordinate; + private final ElasticsearchCoordinate elasticsearchCoordinate; - public ElasticsearchIndexProvider(ElasticsearchCoordinate defaultCoordinate) { - this.defaultCoordinate = defaultCoordinate; + public ElasticsearchIndexProvider(ElasticsearchCoordinate elasticsearchCoordinate) { + this.elasticsearchCoordinate = elasticsearchCoordinate; } @Override public @NotNull List getQueryIndexes(NodeState nodeState) { - return Collections.singletonList(new ElasticsearchIndex(defaultCoordinate, nodeState)); + return Collections.singletonList(new ElasticsearchIndex(elasticsearchCoordinate, nodeState)); } } From dc0380b826f5b7d4ca608686f9ba6069fd6144a6 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Wed, 11 Mar 2020 17:12:56 +0100 Subject: [PATCH 12/18] minor changes in tests --- .../ElasticsearchIndexProviderServiceTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderServiceTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderServiceTest.java index 8e9a0cd4213..311df6f3d19 100644 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderServiceTest.java +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderServiceTest.java @@ -8,6 +8,7 @@ import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider; import org.apache.jackrabbit.oak.spi.state.NodeStore; import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard; +import org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUtils; import org.apache.jackrabbit.oak.stats.StatisticsProvider; import org.apache.sling.testing.mock.osgi.MockOsgi; import org.apache.sling.testing.mock.osgi.junit.OsgiContext; @@ -31,12 +32,11 @@ private final ElasticsearchIndexProviderService service = new ElasticsearchIndexProviderService(); - private MountInfoProvider mip; private Whiteboard wb; @Before public void setUp() { - mip = Mounts.newBuilder().build(); + MountInfoProvider mip = Mounts.newBuilder().build(); context.registerService(MountInfoProvider.class, mip); context.registerService(NodeStore.class, new MemoryNodeStore()); context.registerService(StatisticsProvider.class, StatisticsProvider.NOOP); @@ -54,6 +54,8 @@ public void defaultSetup() throws Exception { assertNotNull(context.getService(QueryIndexProvider.class)); assertNotNull(context.getService(IndexEditorProvider.class)); + assertNotNull(WhiteboardUtils.getServices(wb, Runnable.class)); + MockOsgi.deactivate(service, context.bundleContext()); } From bb53ff3dbf8e9e2efa0022144c69106fa01539d3 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Wed, 11 Mar 2020 18:22:29 +0100 Subject: [PATCH 13/18] perf: avoid to check if index needs to be provisioned at every update --- .../ElasticsearchIndexEditorContext.java | 8 +- .../index/ElasticsearchIndexWriter.java | 98 ++++++++----------- 2 files changed, 46 insertions(+), 60 deletions(-) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorContext.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorContext.java index 94e49817ca4..b3a8a2bb4a7 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorContext.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorContext.java @@ -26,6 +26,8 @@ import org.apache.jackrabbit.oak.spi.state.NodeState; import org.jetbrains.annotations.Nullable; +import java.io.IOException; + public class ElasticsearchIndexEditorContext extends FulltextIndexEditorContext { ElasticsearchIndexEditorContext(NodeState root, NodeBuilder definition, @Nullable IndexDefinition indexDefinition, @@ -53,7 +55,11 @@ public void enableReindexMode() { // Now, that index definition _might_ have been migrated by super call, it would be ok to // get writer and provision index settings and mappings - getWriter().setProvisioningRequired(); + try { + getWriter().provisionIndex(); + } catch (IOException e) { + throw new IllegalStateException("Unable to provision index", e); + } } @Override diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java index cdb5035e535..ca1987186e6 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexWriter.java @@ -49,7 +49,6 @@ private static final Logger LOG = LoggerFactory.getLogger(ElasticsearchIndexWriter.class); private final ElasticsearchIndexDescriptor indexDescriptor; - private boolean shouldProvisionIndex; private final boolean isAsync; @@ -62,13 +61,10 @@ // For test without "async" indexing, we can use following hack BUT those where we // would setup async, we'd need to find another way. isAsync = indexDefinition.getDefinitionNodeState().getProperty("async") != null; - - shouldProvisionIndex = false; } @Override public void updateDocument(String path, ElasticsearchDocument doc) throws IOException { - provisionIndex(); IndexRequest request = new IndexRequest(indexDescriptor.getIndexName()) .id(pathToId(path)) // immediate refresh would slow indexing response such that next @@ -82,7 +78,6 @@ public void updateDocument(String path, ElasticsearchDocument doc) throws IOExce @Override public void deleteDocuments(String path) throws IOException { - provisionIndex(); DeleteRequest request = new DeleteRequest(indexDescriptor.getIndexName()) .id(pathToId(path)) // immediate refresh would slow indexing response such that next @@ -96,69 +91,54 @@ public void deleteDocuments(String path) throws IOException { @Override public boolean close(long timestamp) throws IOException { - provisionIndex(); // TODO : track index updates and return accordingly // TODO : if/when we do async push, this is where to wait for those ops to complete return false; } - /** - * This method won't immediately provision index. But, provision would be done before - * any updates are sent to the index - */ - void setProvisioningRequired() { - shouldProvisionIndex = true; - } - - private void provisionIndex() throws IOException { - if (!shouldProvisionIndex) { - return; - } - - try { - CreateIndexRequest request = new CreateIndexRequest(indexDescriptor.getIndexName()); - - // provision settings - request.settings(Settings.builder() - .put("analysis.analyzer.ancestor_analyzer.type", "custom") - .put("analysis.analyzer.ancestor_analyzer.tokenizer", "path_hierarchy")); - - // provision mappings - XContentBuilder mappingBuilder = XContentFactory.jsonBuilder(); - mappingBuilder.startObject(); + // TODO: we need to check if the index already exists and in that case we have to figure out if there are + // "breaking changes" in the index definition + protected void provisionIndex() throws IOException { + CreateIndexRequest request = new CreateIndexRequest(indexDescriptor.getIndexName()); + + // provision settings + request.settings(Settings.builder() + .put("analysis.analyzer.ancestor_analyzer.type", "custom") + .put("analysis.analyzer.ancestor_analyzer.tokenizer", "path_hierarchy")); + + // provision mappings + XContentBuilder mappingBuilder = XContentFactory.jsonBuilder(); + mappingBuilder.startObject(); + { + mappingBuilder.startObject("properties"); { - mappingBuilder.startObject("properties"); - { - mappingBuilder.startObject(FieldNames.ANCESTORS) - .field("type", "text") - .field("analyzer", "ancestor_analyzer") - .field("search_analyzer", "keyword") - .field("search_quote_analyzer", "keyword") - .endObject(); - mappingBuilder.startObject(FieldNames.PATH_DEPTH) - .field("type", "integer") - .endObject(); - mappingBuilder.startObject(FieldNames.SUGGEST) - .field("type", "completion") - .endObject(); - mappingBuilder.startObject(FieldNames.NOT_NULL_PROPS) - .field("type", "keyword") - .endObject(); - mappingBuilder.startObject(FieldNames.NULL_PROPS) - .field("type", "keyword") - .endObject(); - } - mappingBuilder.endObject(); + mappingBuilder.startObject(FieldNames.ANCESTORS) + .field("type", "text") + .field("analyzer", "ancestor_analyzer") + .field("search_analyzer", "keyword") + .field("search_quote_analyzer", "keyword") + .endObject(); + mappingBuilder.startObject(FieldNames.PATH_DEPTH) + .field("type", "integer") + .endObject(); + mappingBuilder.startObject(FieldNames.SUGGEST) + .field("type", "completion") + .endObject(); + mappingBuilder.startObject(FieldNames.NOT_NULL_PROPS) + .field("type", "keyword") + .endObject(); + mappingBuilder.startObject(FieldNames.NULL_PROPS) + .field("type", "keyword") + .endObject(); } mappingBuilder.endObject(); - request.mapping(mappingBuilder); + } + mappingBuilder.endObject(); + request.mapping(mappingBuilder); - String requestMsg = Strings.toString(request.toXContent(jsonBuilder(), EMPTY_PARAMS)); - CreateIndexResponse response = indexDescriptor.getClient().indices().create(request, RequestOptions.DEFAULT); + String requestMsg = Strings.toString(request.toXContent(jsonBuilder(), EMPTY_PARAMS)); + CreateIndexResponse response = indexDescriptor.getClient().indices().create(request, RequestOptions.DEFAULT); - LOG.info("Updated settings {}. Response acknowledged: {}", requestMsg, response.isAcknowledged()); - } finally { - shouldProvisionIndex = false; - } + LOG.info("Updated settings {}. Response acknowledged: {}", requestMsg, response.isAcknowledged()); } } From 362879301d0263f115d6cf510bb85c677668a024 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Tue, 17 Mar 2020 15:45:36 +0100 Subject: [PATCH 14/18] fix: removed use of assertions --- .../index/ElasticsearchIndexEditorProvider.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java index 0fc00b4e384..fa875ea5e5a 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/index/ElasticsearchIndexEditorProvider.java @@ -48,8 +48,9 @@ public ElasticsearchIndexEditorProvider(@NotNull ElasticsearchCoordinate elastic @NotNull NodeBuilder definition, @NotNull NodeState root, @NotNull IndexUpdateCallback callback) throws CommitFailedException { if (TYPE_ELASTICSEARCH.equals(type)) { - assert callback instanceof ContextAwareCallback : "callback instance not of type " + - "ContextAwareCallback [" + callback + "]"; + if (!(callback instanceof ContextAwareCallback)) { + throw new IllegalStateException("callback instance not of type ContextAwareCallback [" + callback + "]"); + } IndexingContext indexingContext = ((ContextAwareCallback) callback).getIndexingContext(); String indexPath = indexingContext.getIndexPath(); From 63299c5998b21a126c57a17a070f7d8af16b2897 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Tue, 17 Mar 2020 15:57:53 +0100 Subject: [PATCH 15/18] fix: use ES version from constant instead of hard coding it --- .../index/elasticsearch/ElasticsearchPropertyIndexTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java index 438a906504f..1d63f7fd257 100644 --- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java +++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchPropertyIndexTest.java @@ -33,6 +33,7 @@ import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeStore; +import org.elasticsearch.Version; import org.junit.Assert; import org.junit.ClassRule; import org.junit.Test; @@ -47,9 +48,10 @@ import static org.hamcrest.MatcherAssert.assertThat; public class ElasticsearchPropertyIndexTest extends AbstractQueryTest { + @ClassRule public static final ElasticsearchContainer ELASTIC = - new ElasticsearchContainer("docker.elastic.co/elasticsearch/elasticsearch:7.6.0"); + new ElasticsearchContainer("docker.elastic.co/elasticsearch/elasticsearch:" + Version.CURRENT); @Override protected ContentRepository createRepository() { From d7e6b218514a44f9c9e2d9e23353f91ba18d0bb3 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Thu, 19 Mar 2020 16:19:30 +0100 Subject: [PATCH 16/18] fix: osgi instructions to include elastic + transitive dependencies --- oak-search-elastic/pom.xml | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/oak-search-elastic/pom.xml b/oak-search-elastic/pom.xml index f4355ba1d85..a0ea080928a 100644 --- a/oak-search-elastic/pom.xml +++ b/oak-search-elastic/pom.xml @@ -23,7 +23,7 @@ org.apache.jackrabbit oak-parent - 1.25-SNAPSHOT + 1.25-elastic ../oak-parent/pom.xml @@ -44,13 +44,30 @@ maven-bundle-plugin - - + <_exportcontents> + org.apache.jackrabbit.oak.plugins.index.elasticsearch.ElasticsearchIndexProviderService + + + !com.carrotsearch.randomizedtesting.*, + !com.google.common.geometry.*, + !com.sun.management.*, + !jdk.net.*, + !org.apache.avalon.framework.logger.*, + !org.apache.log.*, + !org.apache.logging.*, + !org.elasticsearch.geometry.*, + !org.joda.convert.*, + !org.locationtech.jts.geom.*, + !org.locationtech.spatial4j.*, + !sun.misc.*, + * + - lucene-core;inline=true - elasticsearch-rest-high-level-client;inline=true + oak-search;inline=true, + elasticsearch;groupId=org.elasticsearch, + *;scope=compile|runtime - !* + true @@ -122,43 +139,51 @@ org.apache.jackrabbit oak-api ${project.version} + provided org.apache.jackrabbit oak-core-spi ${project.version} + provided org.apache.jackrabbit oak-store-spi ${project.version} + provided org.apache.jackrabbit oak-query-spi ${project.version} + provided org.apache.jackrabbit oak-core ${project.version} + provided org.apache.jackrabbit oak-search ${project.version} + provided org.slf4j slf4j-api + provided org.jetbrains annotations + provided From 50ebe6ed2838d9910be884ae368675f238a49af4 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Thu, 19 Mar 2020 16:20:32 +0100 Subject: [PATCH 17/18] fix: disable text extraction init since it goes in error during bundle activation --- .../ElasticsearchIndexProviderService.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java index 4c8bdff4d7b..bdb5d71a796 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java @@ -134,8 +134,8 @@ private void activate(BundleContext bundleContext, Map config) { whiteboard = new OsgiWhiteboard(bundleContext); - initializeTextExtractionDir(bundleContext, config); - initializeExtractedTextCache(config, statisticsProvider); + //initializeTextExtractionDir(bundleContext, config); + //initializeExtractedTextCache(config, statisticsProvider); final ElasticsearchCoordinate coordinate = getElasticsearchCoordinate(config); @@ -174,11 +174,11 @@ private void registerIndexEditor(BundleContext bundleContext, ElasticsearchCoord Dictionary props = new Hashtable<>(); props.put("type", ElasticsearchIndexConstants.TYPE_ELASTICSEARCH); regs.add(bundleContext.registerService(IndexEditorProvider.class.getName(), editorProvider, props)); - oakRegs.add(registerMBean(whiteboard, - TextExtractionStatsMBean.class, - editorProvider.getExtractedTextCache().getStatsMBean(), - TextExtractionStatsMBean.TYPE, - "TextExtraction statistics")); +// oakRegs.add(registerMBean(whiteboard, +// TextExtractionStatsMBean.class, +// editorProvider.getExtractedTextCache().getStatsMBean(), +// TextExtractionStatsMBean.TYPE, +// "TextExtraction statistics")); } private void initializeExtractedTextCache(Map config, StatisticsProvider statisticsProvider) { From dd2fbadc4b400f511ec29a933f64de3c431b1246 Mon Sep 17 00:00:00 2001 From: fabriziofortino Date: Thu, 19 Mar 2020 17:21:42 +0100 Subject: [PATCH 18/18] fix: code review changes --- .../ElasticsearchClientFactory.java | 3 ++- .../ElasticsearchCoordinate.java | 23 ---------------- .../ElasticsearchIndexProviderService.java | 27 ++++++++++++++----- 3 files changed, 23 insertions(+), 30 deletions(-) diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java index 70f07123b8f..13e93fb5327 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchClientFactory.java @@ -31,7 +31,7 @@ public class ElasticsearchClientFactory implements Closeable { private static final Logger LOG = LoggerFactory.getLogger(ElasticsearchClientFactory.class); - public static final ElasticsearchClientFactory INSTANCE = new ElasticsearchClientFactory(); + private static final ElasticsearchClientFactory INSTANCE = new ElasticsearchClientFactory(); private final ConcurrentMap clientMap = new ConcurrentHashMap<>(); private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); @@ -71,6 +71,7 @@ public void close() { LOG.error("Error occurred while closing a connection", e); } }); + clientMap.clear(); } finally { lock.writeLock().unlock(); } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java index 28328a76882..444afec8e9e 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchCoordinate.java @@ -16,9 +16,6 @@ */ package org.apache.jackrabbit.oak.plugins.index.elasticsearch; -import org.jetbrains.annotations.NotNull; - -import java.util.Map; import java.util.Objects; public class ElasticsearchCoordinate { @@ -77,24 +74,4 @@ public int hashCode() { public String toString() { return getScheme() + "://" + getHost() + ":" + getPort(); } - - public static ElasticsearchCoordinate build(@NotNull Map config) { - ElasticsearchCoordinate coordinate = null; - Object p = config.get(PORT_PROP); - if (p != null) { - try { - Integer port = Integer.parseInt(p.toString()); - coordinate = build((String) config.get(SCHEME_PROP), (String) config.get(HOST_PROP), port); - } catch (NumberFormatException nfe) { /* ignore */ } - } - return coordinate; - } - - private static ElasticsearchCoordinate build(String scheme, String host, Integer port) { - if (scheme == null || host == null || port == null) { - return null; - } - - return new ElasticsearchCoordinate(scheme, host, port); - } } diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java index bdb5d71a796..c0967e90788 100644 --- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java +++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elasticsearch/ElasticsearchIndexProviderService.java @@ -35,11 +35,11 @@ import org.apache.jackrabbit.oak.plugins.index.elasticsearch.query.ElasticsearchIndexProvider; import org.apache.jackrabbit.oak.plugins.index.fulltext.PreExtractedTextProvider; import org.apache.jackrabbit.oak.plugins.index.search.ExtractedTextCache; -import org.apache.jackrabbit.oak.plugins.index.search.TextExtractionStatsMBean; import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider; import org.apache.jackrabbit.oak.spi.whiteboard.Registration; import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard; import org.apache.jackrabbit.oak.stats.StatisticsProvider; +import org.jetbrains.annotations.NotNull; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceRegistration; import org.slf4j.Logger; @@ -93,19 +93,19 @@ value = ElasticsearchCoordinate.DEFAULT_SCHEME, label = "Elasticsearch connection scheme" ) - private static final String PROP_ELASTICSEARCH_SCHEME = "elasticsearch.scheme"; + private static final String PROP_ELASTICSEARCH_SCHEME = ElasticsearchCoordinate.SCHEME_PROP; @Property( value = ElasticsearchCoordinate.DEFAULT_HOST, label = "Elasticsearch connection host" ) - private static final String PROP_ELASTICSEARCH_HOST = "elasticsearch.host"; + private static final String PROP_ELASTICSEARCH_HOST = ElasticsearchCoordinate.HOST_PROP; @Property( intValue = ElasticsearchCoordinate.DEFAULT_PORT, label = "Elasticsearch connection port" ) - private static final String PROP_ELASTICSEARCH_PORT = "elasticsearch.port"; + private static final String PROP_ELASTICSEARCH_PORT = ElasticsearchCoordinate.PORT_PROP; @Property( label = "Local text extraction cache path", @@ -241,7 +241,7 @@ private void registerExtractedTextProvider(PreExtractedTextProvider provider) { private ElasticsearchCoordinate getElasticsearchCoordinate(Map contextConfig) { // system properties have priority - ElasticsearchCoordinate coordinate = ElasticsearchCoordinate.build(System.getProperties().entrySet() + ElasticsearchCoordinate coordinate = build(System.getProperties().entrySet() .stream() .collect(Collectors.toMap( e -> String.valueOf(e.getKey()), @@ -250,9 +250,24 @@ private ElasticsearchCoordinate getElasticsearchCoordinate(Map contex ); if (coordinate == null) { - coordinate = ElasticsearchCoordinate.build(contextConfig); + coordinate = build(contextConfig); } return coordinate != null ? coordinate : ElasticsearchCoordinate.DEFAULT; } + + private ElasticsearchCoordinate build(@NotNull Map config) { + ElasticsearchCoordinate coordinate = null; + Object p = config.get(PROP_ELASTICSEARCH_PORT); + if (p != null) { + try { + Integer port = Integer.parseInt(p.toString()); + coordinate = new ElasticsearchCoordinate((String) config.get(PROP_ELASTICSEARCH_SCHEME), + (String) config.get(PROP_ELASTICSEARCH_HOST), port); + } catch (NumberFormatException nfe) { + LOG.warn("{} value ({}) cannot be parsed to a valid number", PROP_ELASTICSEARCH_PORT, p); + } + } + return coordinate; + } }