Cleanup changes pulled out of PR #12368 (#12672)

This commit contains the cleanup needed for the new integration test framework.

Changes:
- Fix log lines, misspellings, docs, etc.
- Allow the use of some of Druid's "JSON config" objects in tests
- Fix minor bug in `BaseNodeRoleWatcher`

.gitignore

@@ -13,15 +13,16 @@ target
 .settings/
 .vscode
 *.log
-*.DS_Store
+.DS_Store
 _site
 dependency-reduced-pom.xml
 LICENSE.BINARY
 NOTICE.BINARY
 README.BINARY
 README
 *.lock
-**/.pmd
-**/.pmdruleset.xml
+.pmd
+.pmdruleset.xml
 .java-version
 integration-tests/gen-scripts/
+bin/

.travis.yml

@@ -34,8 +34,8 @@ env:
     - DOCKER_IP=127.0.0.1  # for integration tests
     - MVN="mvn -B"
     - > # Various options to make execution of maven goals faster (e.g., mvn install)
-      MAVEN_SKIP="-Pskip-static-checks -Ddruid.console.skip=true -Dmaven.javadoc.skip=true"
-    - MAVEN_SKIP_TESTS="-Pskip-tests"
+      MAVEN_SKIP="-P skip-static-checks -Ddruid.console.skip=true -Dmaven.javadoc.skip=true"
+    - MAVEN_SKIP_TESTS="-P skip-tests"
 
 addons:
   apt:
@@ -46,7 +46,7 @@ addons:
 # Add various options to make 'mvn install' fast and skip javascript compile (-Ddruid.console.skip=true) since it is not
 # needed. Depending on network speeds, "mvn -q install" may take longer than the default 10 minute timeout to print any
 # output.  To compensate, use travis_wait to extend the timeout.
-install: ./check_test_suite.py && travis_terminate 0  || echo 'Running maven install...' && MAVEN_OPTS='-Xmx3000m' travis_wait 15 ${MVN} clean install -q -ff -pl '!distribution' ${MAVEN_SKIP} ${MAVEN_SKIP_TESTS} -T1C && ${MVN} install -q -ff -pl 'distribution' ${MAVEN_SKIP} ${MAVEN_SKIP_TESTS}
+install: ./check_test_suite.py && travis_terminate 0  || echo 'Running Maven install...' && MAVEN_OPTS='-Xmx3000m' travis_wait 15 ${MVN} clean install -q -ff -pl '!distribution' ${MAVEN_SKIP} ${MAVEN_SKIP_TESTS} -T1C && ${MVN} install -q -ff -pl 'distribution' ${MAVEN_SKIP} ${MAVEN_SKIP_TESTS}
 
 # There are 3 stages of tests
 # 1. Tests - phase 1

core/src/main/java/org/apache/druid/guice/LifecycleModule.java

@@ -61,7 +61,7 @@ public class LifecycleModule implements Module
    * is materialized and injected, meaning that objects are not actually instantiated in dependency order.
    * Registering with the LifecyceModule, on the other hand, will instantiate the objects after the normal object
    * graph has already been instantiated, meaning that objects will be created in dependency order and this will
-   * only actually instantiate something that wasn't actually dependend upon.
+   * only actually instantiate something that wasn't actually depended upon.
    *
    * @param clazz the class to instantiate
    * @return this, for chaining.

core/src/main/java/org/apache/druid/metadata/MetadataStorageConnectorConfig.java

@@ -22,6 +22,7 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import org.apache.druid.java.util.common.StringUtils;
 
+import java.util.Map;
 import java.util.Properties;
 
 /**
@@ -49,6 +50,30 @@ public class MetadataStorageConnectorConfig
   @JsonProperty("dbcp")
   private Properties dbcpProperties;
 
+  public static MetadataStorageConnectorConfig create(
+      String connectUri,
+      String user,
+      String password,
+      Map<String, Object> properties
+  )
+  {
+    MetadataStorageConnectorConfig config = new MetadataStorageConnectorConfig();
+    if (connectUri != null) {
+      config.connectURI = connectUri;
+    }
+    if (user != null) {
+      config.user = user;
+    }
+    if (password != null) {
+      config.passwordProvider = () -> password;
+    }
+    if (properties != null) {
+      config.dbcpProperties = new Properties();
+      config.dbcpProperties.putAll(properties);
+    }
+    return config;
+  }
+
   @JsonProperty
   public boolean isCreateTables()
   {

core/src/test/java/org/apache/druid/metadata/MetadataStorageConnectorConfigTest.java

@@ -20,10 +20,12 @@
 package org.apache.druid.metadata;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableMap;
 import org.junit.Assert;
 import org.junit.Test;
 
 import java.io.IOException;
+import java.util.Map;
 import java.util.Properties;
 
 public class MetadataStorageConnectorConfigTest
@@ -187,4 +189,19 @@ private void testDbcpPropertiesFile(
     Assert.assertEquals(dbcpProperties.getProperty("maxConnLifetimeMillis"), "1200000");
     Assert.assertEquals(dbcpProperties.getProperty("defaultQueryTimeout"), "30000");
   }
+
+  @Test
+  public void testCreate()
+  {
+    Map<String, Object> props = ImmutableMap.of("key", "value");
+    MetadataStorageConnectorConfig config = MetadataStorageConnectorConfig.create(
+        "connectURI",
+        "user",
+        "pwd",
+        props);
+    Assert.assertEquals("connectURI", config.getConnectURI());
+    Assert.assertEquals("user", config.getUser());
+    Assert.assertEquals("pwd", config.getPassword());
+    Assert.assertEquals(1, config.getDbcpProperties().size());
+  }
 }

distribution/docker/druid.sh

@@ -20,11 +20,11 @@
 #
 
 # NOTE: this is a 'run' script for the stock tarball
-# It takes 1 required argument (the name of the service,
+# It takes one required argument (the name of the service,
 # e.g. 'broker', 'historical' etc). Any additional arguments
 # are passed to that service.
 #
-# It accepts 'JAVA_OPTS' as an environment variable
+# This script accepts JAVA_OPTS as an environment variable
 #
 # Additional env vars:
 # - DRUID_LOG4J -- set the entire log4j.xml verbatim

extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnector.java

@@ -61,19 +61,24 @@ public MySQLConnector(
   )
   {
     super(config, dbTables);
-    log.info("Loading \"MySQL\" metadata connector driver %s", driverConfig.getDriverClassName());
-    tryLoadDriverClass(driverConfig.getDriverClassName(), true);
+    this.dbi = createDBI(config.get(), driverConfig, connectorSslConfig, getValidationQuery());
 
     if (driverConfig.getDriverClassName().contains("mysql")) {
       myTransientExceptionClass = tryLoadDriverClass(MYSQL_TRANSIENT_EXCEPTION_CLASS_NAME, false);
     } else {
       myTransientExceptionClass = null;
     }
+  }
+
+  public static DBI createDBI(MetadataStorageConnectorConfig config, MySQLConnectorDriverConfig driverConfig, MySQLConnectorSslConfig connectorSslConfig, String validationQuery)
+  {
+    log.info("Loading \"MySQL\" metadata connector driver %s", driverConfig.getDriverClassName());
+    tryLoadDriverClass(driverConfig.getDriverClassName(), true);
 
-    final BasicDataSource datasource = getDatasource();
+    final BasicDataSource datasource = makeDatasource(config, validationQuery);
     // MySQL driver is classloader isolated as part of the extension
     // so we need to help JDBC find the driver
-    datasource.setDriverClassLoader(getClass().getClassLoader());
+    datasource.setDriverClassLoader(MySQLConnector.class.getClassLoader());
     datasource.setDriverClassName(driverConfig.getDriverClassName());
     datasource.addConnectionProperty("useSSL", String.valueOf(connectorSslConfig.isUseSSL()));
     if (connectorSslConfig.isUseSSL()) {
@@ -141,9 +146,10 @@ public MySQLConnector(
     // use double-quotes for quoting columns, so we can write SQL that works with most databases
     datasource.setConnectionInitSqls(ImmutableList.of("SET sql_mode='ANSI_QUOTES'"));
 
-    this.dbi = new DBI(datasource);
+    DBI dbi = new DBI(datasource);
 
     log.info("Configured MySQL as metadata storage");
+    return dbi;
   }
 
   @Override
@@ -252,10 +258,10 @@ public DBI getDBI()
   }
 
   @Nullable
-  private Class<?> tryLoadDriverClass(String className, boolean failIfNotFound)
+  private static Class<?> tryLoadDriverClass(String className, boolean failIfNotFound)
   {
     try {
-      return Class.forName(className, false, getClass().getClassLoader());
+      return Class.forName(className, false, MySQLConnector.class.getClassLoader());
     }
     catch (ClassNotFoundException e) {
       if (failIfNotFound) {

extensions-core/mysql-metadata-storage/src/main/java/org/apache/druid/metadata/storage/mysql/MySQLConnectorDriverConfig.java

@@ -20,13 +20,25 @@
 package org.apache.druid.metadata.storage.mysql;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Strings;
 
 import java.util.Objects;
 
 public class MySQLConnectorDriverConfig
 {
+  public static final String MYSQL_DRIVER = "com.mysql.jdbc.Driver";
+
   @JsonProperty
-  private String driverClassName = "com.mysql.jdbc.Driver";
+  private String driverClassName = MYSQL_DRIVER;
+
+  public static MySQLConnectorDriverConfig create(String driverClassName)
+  {
+    MySQLConnectorDriverConfig config = new MySQLConnectorDriverConfig();
+    if (!Strings.isNullOrEmpty(driverClassName)) {
+      config.driverClassName = driverClassName;
+    }
+    return config;
+  }
 
   @JsonProperty
   public String getDriverClassName()

extensions-core/mysql-metadata-storage/src/test/java/org/apache/druid/metadata/storage/mysql/MySQLConnectorDriverConfigTest.java

@@ -22,6 +22,8 @@
 import nl.jqno.equalsverifier.EqualsVerifier;
 import org.junit.Test;
 
+import static org.junit.Assert.assertEquals;
+
 public class MySQLConnectorDriverConfigTest
 {
   @Test
@@ -33,4 +35,13 @@ public void testEqualsAndHashcode()
                   .withNonnullFields("driverClassName")
                   .verify();
   }
+
+  @Test
+  public void testCreate()
+  {
+    MySQLConnectorDriverConfig config = MySQLConnectorDriverConfig.create(null);
+    assertEquals(MySQLConnectorDriverConfig.MYSQL_DRIVER, config.getDriverClassName());
+    config = MySQLConnectorDriverConfig.create("myDriver");
+    assertEquals("myDriver", config.getDriverClassName());
+  }
 }

server/src/main/java/org/apache/druid/client/FilteredHttpServerInventoryViewProvider.java

@@ -36,20 +36,20 @@ public class FilteredHttpServerInventoryViewProvider implements FilteredServerIn
   @JacksonInject
   @NotNull
   @EscalatedClient
-  HttpClient httpClient = null;
+  HttpClient httpClient;
 
   @JacksonInject
   @NotNull
   @Smile
-  ObjectMapper smileMapper = null;
+  ObjectMapper smileMapper;
 
   @JacksonInject
   @NotNull
-  HttpServerInventoryViewConfig config = null;
+  HttpServerInventoryViewConfig config;
 
   @JacksonInject
   @NotNull
-  private DruidNodeDiscoveryProvider druidNodeDiscoveryProvider = null;
+  private DruidNodeDiscoveryProvider druidNodeDiscoveryProvider;
 
   @Override
   public HttpServerInventoryView get()

server/src/main/java/org/apache/druid/curator/CuratorConfig.java

@@ -28,6 +28,8 @@
 
 public class CuratorConfig
 {
+  public static final String CONFIG_PREFIX = "druid.zk.service";
+
   static final String HOST = "host";
   @JsonProperty(HOST)
   private String zkHosts = "localhost";
@@ -56,6 +58,13 @@ public class CuratorConfig
   @JsonProperty("authScheme")
   private String authScheme = "digest";
 
+  public static CuratorConfig create(String hosts)
+  {
+    CuratorConfig config = new CuratorConfig();
+    config.zkHosts = hosts;
+    return config;
+  }
+
   public String getZkHosts()
   {
     return zkHosts;