opendev/gerrit
Code Issues Proposed changes

ElasticConfiguration: Extract constants to statics

Browse Source 
Create static constants for the elasticsearch section name, the
keys, and default values.

Make them package visible so that they can be used in tests.

Update the configuration tests to use them.

Change-Id: I150e584f82825a25ae32604481b4b8e919850cb1
This commit is contained in:
David Pursehouse
2018-07-04 13:34:33 +09:00
parent 479c50ba28
commit 12a75b2815
2 changed files with 29 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
View File

@@ -35,8 +35,16 @@ import org.slf4j.LoggerFactory;
class ElasticConfiguration { class ElasticConfiguration {
private static final Logger log = LoggerFactory.getLogger(ElasticConfiguration.class); private static final Logger log = LoggerFactory.getLogger(ElasticConfiguration.class);
private static final String DEFAULT_PORT = "9200"; static final String SECTION_ELASTICSEARCH = "elasticsearch";
private static final String DEFAULT_USERNAME = "elastic"; static final String KEY_PASSWORD = "password";
static final String KEY_USERNAME = "username";
static final String KEY_MAX_RETRY_TIMEOUT = "maxRetryTimeout";
static final String KEY_PREFIX = "prefix";
static final String KEY_SERVER = "server";
static final String DEFAULT_PORT = "9200";
static final String DEFAULT_USERNAME = "elastic";
static final int DEFAULT_MAX_RETRY_TIMEOUT_MS = 30000;
static final TimeUnit MAX_RETRY_TIMEOUT_UNIT = TimeUnit.MILLISECONDS;
private final Config cfg; private final Config cfg;
private final List<HttpHost> hosts; private final List<HttpHost> hosts;
@@ -49,17 +57,23 @@ class ElasticConfiguration {
@Inject @Inject
ElasticConfiguration(@GerritServerConfig Config cfg) { ElasticConfiguration(@GerritServerConfig Config cfg) {
this.cfg = cfg; this.cfg = cfg;
this.password = cfg.getString("elasticsearch", null, "password"); this.password = cfg.getString(SECTION_ELASTICSEARCH, null, KEY_PASSWORD);
this.username = this.username =
password == null password == null
? null ? null
: firstNonNull(cfg.getString("elasticsearch", null, "username"), DEFAULT_USERNAME); : firstNonNull(
cfg.getString(SECTION_ELASTICSEARCH, null, KEY_USERNAME), DEFAULT_USERNAME);
this.maxRetryTimeout = this.maxRetryTimeout =
(int) (int)
cfg.getTimeUnit("elasticsearch", null, "maxRetryTimeout", 30000, TimeUnit.MILLISECONDS); cfg.getTimeUnit(
this.prefix = Strings.nullToEmpty(cfg.getString("elasticsearch", null, "prefix")); SECTION_ELASTICSEARCH,
null,
KEY_MAX_RETRY_TIMEOUT,
DEFAULT_MAX_RETRY_TIMEOUT_MS,
MAX_RETRY_TIMEOUT_UNIT);
this.prefix = Strings.nullToEmpty(cfg.getString(SECTION_ELASTICSEARCH, null, KEY_PREFIX));
this.hosts = new ArrayList<>(); this.hosts = new ArrayList<>();
for (String server : cfg.getStringList("elasticsearch", null, "server")) { for (String server : cfg.getStringList(SECTION_ELASTICSEARCH, null, KEY_SERVER)) {
try { try {
URI uri = new URI(server); URI uri = new URI(server);
int port = uri.getPort(); int port = uri.getPort();

14
gerrit-elasticsearch/src/test/java/com/google/gerrit/elasticsearch/ElasticConfigurationTest.java
View File

@@ -15,6 +15,8 @@
package com.google.gerrit.elasticsearch; package com.google.gerrit.elasticsearch;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.elasticsearch.ElasticConfiguration.KEY_SERVER;
import static com.google.gerrit.elasticsearch.ElasticConfiguration.SECTION_ELASTICSEARCH;
import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toList;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
@@ -31,7 +33,7 @@ public class ElasticConfigurationTest {
@Test @Test
public void singleServer() throws Exception { public void singleServer() throws Exception {
Config cfg = new Config(); Config cfg = new Config();
cfg.setString("elasticsearch", null, "server", "http://elastic:1234"); cfg.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "http://elastic:1234");
ElasticConfiguration esCfg = new ElasticConfiguration(cfg); ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
assertHosts(esCfg, "http://elastic:1234"); assertHosts(esCfg, "http://elastic:1234");
} }
@@ -39,7 +41,7 @@ public class ElasticConfigurationTest {
@Test @Test
public void serverWithoutPortSpecified() throws Exception { public void serverWithoutPortSpecified() throws Exception {
Config cfg = new Config(); Config cfg = new Config();
cfg.setString("elasticsearch", null, "server", "http://elastic"); cfg.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "http://elastic");
ElasticConfiguration esCfg = new ElasticConfiguration(cfg); ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
assertHosts(esCfg, "http://elastic:9200"); assertHosts(esCfg, "http://elastic:9200");
} }
@@ -48,9 +50,9 @@ public class ElasticConfigurationTest {
public void multipleServers() throws Exception { public void multipleServers() throws Exception {
Config cfg = new Config(); Config cfg = new Config();
cfg.setStringList( cfg.setStringList(
"elasticsearch", SECTION_ELASTICSEARCH,
null, null,
"server", KEY_SERVER,
ImmutableList.of("http://elastic1:1234", "http://elastic2:1234")); ImmutableList.of("http://elastic1:1234", "http://elastic2:1234"));
ElasticConfiguration esCfg = new ElasticConfiguration(cfg); ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
assertHosts(esCfg, "http://elastic1:1234", "http://elastic2:1234"); assertHosts(esCfg, "http://elastic1:1234", "http://elastic2:1234");
@@ -64,7 +66,7 @@ public class ElasticConfigurationTest {
@Test @Test
public void singleServerInvalid() throws Exception { public void singleServerInvalid() throws Exception {
Config cfg = new Config(); Config cfg = new Config();
cfg.setString("elasticsearch", null, "server", "foo"); cfg.setString(SECTION_ELASTICSEARCH, null, KEY_SERVER, "foo");
assertProvisionException(cfg); assertProvisionException(cfg);
} }
@@ -72,7 +74,7 @@ public class ElasticConfigurationTest {
public void multipleServersIncludingInvalid() throws Exception { public void multipleServersIncludingInvalid() throws Exception {
Config cfg = new Config(); Config cfg = new Config();
cfg.setStringList( cfg.setStringList(
"elasticsearch", null, "server", ImmutableList.of("http://elastic1:1234", "foo")); SECTION_ELASTICSEARCH, null, KEY_SERVER, ImmutableList.of("http://elastic1:1234", "foo"));
ElasticConfiguration esCfg = new ElasticConfiguration(cfg); ElasticConfiguration esCfg = new ElasticConfiguration(cfg);
assertHosts(esCfg, "http://elastic1:1234"); assertHosts(esCfg, "http://elastic1:1234");
} }