From 390c042ab4deb35304aea69a0e64911e94e46807 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 30 May 2018 10:18:38 +0900 Subject: [PATCH] AbstractElasticIndex: Move generation of index name to ElasticConfiguration ElasticConfiguration already reads all of the other configurations, so move reading of the prefix there too. Move generation of the indexName from AbstractElasticIndex to a helper method in ElasticConfiguration. AbstractElasticIndex no longer needs the @GerritServerConfig annotated Config, and now just gets ElasticConfiguration injected. Modify ElasticConfiguration to keep a reference to the @GerritServerConfig annotated Config, and provide an accessor method. ElasticVersionManager also no longer needs to get the @GerritServerConfig annotated. It just gets the ElasticConfiguration which it uses to get the prefix, and passes the underlying Config up to the superclass which uses it to get the onlineReindex flag. Change-Id: Ie4696b4d5a4e0bc58d3b2175f46002d0cc65940c --- .../gerrit/elasticsearch/AbstractElasticIndex.java | 12 ++---------- .../gerrit/elasticsearch/ElasticAccountIndex.java | 4 +--- .../gerrit/elasticsearch/ElasticChangeIndex.java | 4 +--- .../gerrit/elasticsearch/ElasticConfiguration.java | 14 ++++++++++++++ .../gerrit/elasticsearch/ElasticGroupIndex.java | 4 +--- .../elasticsearch/ElasticVersionManager.java | 9 +++------ 6 files changed, 22 insertions(+), 25 deletions(-) diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java index f401b7eb4e..48cb35dac4 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java @@ -18,12 +18,10 @@ import static com.google.gson.FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES; import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.commons.codec.binary.Base64.decodeBase64; -import com.google.common.base.Strings; import com.google.common.collect.FluentIterable; import com.google.common.io.CharStreams; import com.google.gerrit.elasticsearch.builders.SearchSourceBuilder; import com.google.gerrit.elasticsearch.bulk.DeleteRequest; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.index.Index; import com.google.gerrit.server.index.IndexUtils; @@ -48,7 +46,6 @@ import org.apache.http.HttpEntity; import org.apache.http.HttpStatus; import org.apache.http.entity.ContentType; import org.apache.http.nio.entity.NStringEntity; -import org.eclipse.jgit.lib.Config; import org.elasticsearch.client.Response; import org.elasticsearch.client.RestClient; @@ -91,7 +88,7 @@ abstract class AbstractElasticIndex implements Index { protected final ElasticQueryBuilder queryBuilder; AbstractElasticIndex( - @GerritServerConfig Config cfg, + ElasticConfiguration cfg, SitePaths sitePaths, Schema schema, ElasticRestClientBuilder clientBuilder, @@ -100,12 +97,7 @@ abstract class AbstractElasticIndex implements Index { this.schema = schema; this.gson = new GsonBuilder().setFieldNamingPolicy(LOWER_CASE_WITH_UNDERSCORES).create(); this.queryBuilder = new ElasticQueryBuilder(); - this.indexName = - String.format( - "%s%s_%04d", - Strings.nullToEmpty(cfg.getString("elasticsearch", null, "prefix")), - indexName, - schema.getVersion()); + this.indexName = cfg.getIndexName(indexName, schema.getVersion()); this.indexNameRaw = indexName; this.client = clientBuilder.build(); } diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java index cddbe02979..538f49cfaf 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java @@ -27,7 +27,6 @@ import com.google.gerrit.elasticsearch.bulk.UpdateRequest; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountState; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.index.QueryOptions; @@ -53,7 +52,6 @@ import java.util.List; import java.util.Set; import org.apache.http.HttpStatus; import org.apache.http.StatusLine; -import org.eclipse.jgit.lib.Config; import org.elasticsearch.client.Response; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -78,7 +76,7 @@ public class ElasticAccountIndex extends AbstractElasticIndex accountCache, ElasticRestClientBuilder clientBuilder, diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java index 59660ea2ca..bca52f057e 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java @@ -42,7 +42,6 @@ import com.google.gerrit.reviewdb.client.Change.Id; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ReviewerSet; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.index.FieldDef.FillArgs; import com.google.gerrit.server.index.IndexUtils; @@ -74,7 +73,6 @@ import java.util.Set; import org.apache.commons.codec.binary.Base64; import org.apache.http.HttpStatus; import org.apache.http.StatusLine; -import org.eclipse.jgit.lib.Config; import org.elasticsearch.client.Response; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -107,7 +105,7 @@ public class ElasticChangeIndex extends AbstractElasticIndex db, ChangeData.Factory changeDataFactory, FillArgs fillArgs, diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java index 4cca280362..84dae7f238 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java @@ -15,6 +15,7 @@ package com.google.gerrit.elasticsearch; import com.google.common.base.MoreObjects; +import com.google.common.base.Strings; import com.google.gerrit.server.config.GerritServerConfig; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -32,6 +33,8 @@ class ElasticConfiguration { private static final String DEFAULT_PORT = "9200"; private static final String DEFAULT_PROTOCOL = "http"; + private final Config cfg; + final List urls; final String username; final String password; @@ -41,9 +44,11 @@ class ElasticConfiguration { final TimeUnit maxConnectionIdleUnit = TimeUnit.MILLISECONDS; final int maxTotalConnection; final int readTimeout; + final String prefix; @Inject ElasticConfiguration(@GerritServerConfig Config cfg) { + this.cfg = cfg; this.username = cfg.getString("elasticsearch", null, "username"); this.password = cfg.getString("elasticsearch", null, "password"); this.requestCompression = cfg.getBoolean("elasticsearch", null, "requestCompression", false); @@ -55,6 +60,7 @@ class ElasticConfiguration { this.maxTotalConnection = cfg.getInt("elasticsearch", null, "maxTotalConnection", 1); this.readTimeout = (int) cfg.getTimeUnit("elasticsearch", null, "readTimeout", 3000, TimeUnit.MICROSECONDS); + this.prefix = Strings.nullToEmpty(cfg.getString("elasticsearch", null, "prefix")); Set subsections = cfg.getSubsections("elasticsearch"); if (subsections.isEmpty()) { @@ -74,6 +80,14 @@ class ElasticConfiguration { } } + public Config getConfig() { + return cfg; + } + + public String getIndexName(String name, int schemaVersion) { + return String.format("%s%s_%04d", prefix, name, schemaVersion); + } + private String getString(Config cfg, String subsection, String name, String defaultValue) { return MoreObjects.firstNonNull(cfg.getString("elasticsearch", subsection, name), defaultValue); } diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java index d9c0b6280d..04695c7e3e 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java @@ -24,7 +24,6 @@ import com.google.gerrit.elasticsearch.bulk.IndexRequest; import com.google.gerrit.elasticsearch.bulk.UpdateRequest; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.server.account.GroupCache; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.index.IndexUtils; import com.google.gerrit.server.index.QueryOptions; @@ -50,7 +49,6 @@ import java.util.List; import java.util.Set; import org.apache.http.HttpStatus; import org.apache.http.StatusLine; -import org.eclipse.jgit.lib.Config; import org.elasticsearch.client.Response; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -75,7 +73,7 @@ public class ElasticGroupIndex extends AbstractElasticIndex groupCache, ElasticRestClientBuilder clientBuilder, diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticVersionManager.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticVersionManager.java index 3286c422da..d2ad2faf86 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticVersionManager.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticVersionManager.java @@ -14,10 +14,8 @@ package com.google.gerrit.elasticsearch; -import com.google.common.base.Strings; import com.google.common.primitives.Ints; import com.google.gerrit.extensions.events.LifecycleListener; -import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.index.AbstractVersionManager; import com.google.gerrit.server.index.GerritIndexStatus; @@ -29,7 +27,6 @@ import com.google.inject.Singleton; import java.io.IOException; import java.util.Collection; import java.util.TreeMap; -import org.eclipse.jgit.lib.Config; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -42,13 +39,13 @@ public class ElasticVersionManager extends AbstractVersionManager implements Lif @Inject ElasticVersionManager( - @GerritServerConfig Config cfg, + ElasticConfiguration cfg, SitePaths sitePaths, Collection> defs, ElasticIndexVersionDiscovery versionDiscovery) { - super(cfg, sitePaths, defs); + super(cfg.getConfig(), sitePaths, defs); this.versionDiscovery = versionDiscovery; - prefix = Strings.nullToEmpty(cfg.getString("elasticsearch", null, "prefix")); + prefix = cfg.prefix; } @Override