addressed comments

Author: felicity3786

gateway-ha/pom.xml

@@ -76,6 +76,12 @@
             <artifactId>guice</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>com.h2database</groupId>
+            <artifactId>h2</artifactId>
+            <version>1.4.192</version>
+        </dependency>
+
         <dependency>
             <groupId>com.mysql</groupId>
             <artifactId>mysql-connector-j</artifactId>
@@ -95,6 +101,12 @@
             <classifier>jdk11</classifier>
         </dependency>
 
+        <dependency>
+            <groupId>com.oracle.database.jdbc</groupId>
+            <artifactId>ojdbc11</artifactId>
+            <version>23.7.0.25.01</version>
+        </dependency>
+
         <dependency>
             <groupId>com.squareup.okhttp3</groupId>
             <artifactId>okhttp</artifactId>
@@ -272,16 +284,14 @@
         </dependency>
 
         <dependency>
-            <groupId>org.weakref</groupId>
-            <artifactId>jmxutils</artifactId>
+            <groupId>org.postgresql</groupId>
+            <artifactId>postgresql</artifactId>
+            <version>42.7.5</version>
         </dependency>
 
         <dependency>
-            <groupId>com.oracle.database.jdbc</groupId>
-            <artifactId>ojdbc11-production</artifactId>
-            <version>23.7.0.25.01</version>
-            <type>pom</type>
-            <scope>runtime</scope>
+            <groupId>org.weakref</groupId>
+            <artifactId>jmxutils</artifactId>
         </dependency>
 
         <!-- Required for ClusterStatsJdbcMonitor -->
@@ -313,21 +323,7 @@
             <scope>runtime</scope>
         </dependency>
 
-        <dependency>
-            <groupId>org.postgresql</groupId>
-            <artifactId>postgresql</artifactId>
-            <version>42.7.5</version>
-            <scope>runtime</scope>
-        </dependency>
-
         <!-- Test deps -->
-        <dependency>
-            <groupId>com.h2database</groupId>
-            <artifactId>h2</artifactId>
-            <version>1.4.192</version>
-            <scope>test</scope>
-        </dependency>
-
         <dependency>
             <groupId>com.squareup.okhttp3</groupId>
             <artifactId>mockwebserver</artifactId>

gateway-ha/src/main/java/io/trino/gateway/ha/config/DataStoreConfiguration.java

@@ -18,6 +18,8 @@
 import java.sql.SQLException;
 import java.util.Enumeration;
 
+import static java.util.Objects.requireNonNull;
+
 public class DataStoreConfiguration
 {
     private String jdbcUrl;
@@ -34,7 +36,7 @@ public class DataStoreConfiguration
 
     public DataStoreConfiguration(String jdbcUrl, String user, String password, String driver, Integer queryHistoryHoursRetention, boolean runMigrationsEnabled)
     {
-        this.jdbcUrl = jdbcUrl;
+        this.jdbcUrl = requireNonNull(jdbcUrl, "jdbc must be set");
         this.user = user;
         this.password = password;
         this.driver = driver;
@@ -119,27 +121,16 @@ public DataStoreType getDataStoreType()
         if (dataStoreType != null) {
             return dataStoreType;
         }
-        if (jdbcUrl == null) {
-            throw new IllegalStateException("jdbcUrl must be set");
-        }
-
+        String jdbcUrl = getJdbcUrl();
         try {
             Enumeration<Driver> drivers = DriverManager.getDrivers();
             while (drivers.hasMoreElements()) {
                 Driver driver = drivers.nextElement();
                 if (driver.acceptsURL(jdbcUrl)) {
-                    String driverClass = driver.getClass().getName();
-                    if (driverClass.contains("oracle")) {
-                        return DataStoreType.ORACLE;
-                    }
-                    if (driverClass.contains("mysql")) {
-                        return DataStoreType.MYSQL;
-                    }
-                    if (driverClass.contains("postgresql")) {
-                        return DataStoreType.POSTGRES;
-                    }
-                    if (driverClass.contains("h2")) {
-                        return DataStoreType.H2;
+                    for (DataStoreType dataStoreType : DataStoreType.values()) {
+                        if (dataStoreType.getDriverClass().equals(driver.getClass())) {
+                            return dataStoreType;
+                        }
                     }
                     break;
                 }

gateway-ha/src/main/java/io/trino/gateway/ha/config/DataStoreType.java

@@ -13,9 +13,26 @@
  */
 package io.trino.gateway.ha.config;
 
+import java.sql.Driver;
+
 public enum DataStoreType {
-    ORACLE,
-    MYSQL,
-    POSTGRES,
-    H2
+    ORACLE(oracle.jdbc.driver.OracleDriver.class),
+    MYSQL(com.mysql.cj.jdbc.Driver.class),
+    POSTGRES(org.postgresql.Driver.class),
+    H2(org.h2.Driver.class);
+
+    private final Class<? extends Driver> driverClass;
+
+    DataStoreType(Class<? extends Driver> driverClass)
+    {
+        this.driverClass = driverClass;
+    }
+
+    /**
+     * Returns the JDBC Driver class associated with this data store type.
+     */
+    public Class<? extends Driver> getDriverClass()
+    {
+        return driverClass;
+    }
 }

gateway-ha/src/main/java/io/trino/gateway/ha/persistence/JdbcPropertiesProviderFactory.java

@@ -13,13 +13,15 @@
  */
 package io.trino.gateway.ha.persistence;
 
+import com.google.common.collect.ImmutableList;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import io.trino.gateway.ha.config.DataStoreConfiguration;
 
 import java.util.List;
 
 import static java.lang.String.format;
+import static java.util.Objects.requireNonNull;
 
 @Singleton
 public class JdbcPropertiesProviderFactory
@@ -29,7 +31,7 @@ public class JdbcPropertiesProviderFactory
     @Inject
     public JdbcPropertiesProviderFactory(List<JdbcPropertiesProvider> orderedProviders)
     {
-        this.providers = List.copyOf(orderedProviders);
+        this.providers = ImmutableList.copyOf(requireNonNull(orderedProviders, "providers is null"));
     }
 
     public JdbcPropertiesProvider forConfig(DataStoreConfiguration config)

gateway-ha/src/test/java/io/trino/gateway/ha/persistence/TestJdbcPropertiesProviderFactory.java

@@ -22,13 +22,11 @@
 import io.trino.gateway.ha.config.MySqlConfiguration;
 import io.trino.gateway.ha.module.RouterBaseModule;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.TestInstance;
 
 import java.util.Properties;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-@TestInstance(TestInstance.Lifecycle.PER_CLASS)
 final class TestJdbcPropertiesProviderFactory
 {
     private static JdbcPropertiesProviderFactory factoryFor(DataStoreConfiguration dbConfig)
@@ -68,9 +66,9 @@ private DataStoreConfiguration makeH2Config()
     @Test
     void testBasicProviderWhenH2()
     {
-        var cfg = makeH2Config();
-        var factory = factoryFor(cfg);
-        var provider = factory.forConfig(cfg);
+        DataStoreConfiguration cfg = makeH2Config();
+        JdbcPropertiesProviderFactory factory = factoryFor(cfg);
+        DefaultJdbcPropertiesProvider provider = (DefaultJdbcPropertiesProvider) factory.forConfig(cfg);
         assertThat(provider).isInstanceOf(DefaultJdbcPropertiesProvider.class);
 
         Properties properties = provider.getProperties(cfg);
@@ -83,10 +81,9 @@ void testBasicProviderWhenH2()
     @Test
     void testMysqlProviderWhenSslDisabled()
     {
-        var cfg = makeMySqlConfig(SslMode.DISABLED);
-
-        var factory = factoryFor(cfg);
-        var provider = factory.forConfig(cfg);
+        DataStoreConfiguration cfg = makeMySqlConfig(SslMode.DISABLED);
+        JdbcPropertiesProviderFactory factory = factoryFor(cfg);
+        JdbcPropertiesProvider provider = factory.forConfig(cfg);
         assertThat(provider).isInstanceOf(MySqlJdbcPropertiesProvider.class);
 
         Properties properties = provider.getProperties(cfg);
@@ -106,19 +103,19 @@ void testMysqlProviderWhenSslDisabled()
     @Test
     void testMysqlProviderWhenVerifyCa()
     {
-        var cfg = makeMySqlConfig(SslMode.VERIFY_CA);
-        var factory = factoryFor(cfg);
-        var mySqlConfiguration = cfg.getMySqlConfiguration();
-        mySqlConfiguration.setClientCertificateKeyStoreUrl("file:/tmp/cli.p12")
+        DataStoreConfiguration cfg = makeMySqlConfig(SslMode.VERIFY_CA);
+        JdbcPropertiesProviderFactory factory = factoryFor(cfg);
+        MySqlConfiguration mysqlConfig = cfg.getMySqlConfiguration();
+        mysqlConfig.setClientCertificateKeyStoreUrl("file:/tmp/cli.p12")
                 .setClientCertificateKeyStorePassword("cpw")
                 .setClientCertificateKeyStoreType("PKCS12")
                 .setTrustCertificateKeyStoreUrl("file:/tmp/trs.p12")
                 .setTrustCertificateKeyStorePassword("tpw");
 
-        var provider = factory.forConfig(cfg);
+        JdbcPropertiesProvider provider = factory.forConfig(cfg);
         assertThat(provider).isInstanceOf(MySqlJdbcPropertiesProvider.class);
 
-        var properties = provider.getProperties(cfg);
+        Properties properties = provider.getProperties(cfg);
         assertThat(properties.getProperty("user")).isEqualTo("root");
         assertThat(properties.getProperty(PropertyKey.sslMode.getKeyName()))
                 .isEqualTo("VERIFY_CA");