Merge pull request #2489 from enqueue/jdbc_url_default_ports

[jdbc-v2] Adds default port and restricts multiple endpoints

Author: chernser

jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java

@@ -2,6 +2,7 @@
 
 import com.clickhouse.client.api.Client;
 import com.clickhouse.client.api.ClientConfigProperties;
+import com.clickhouse.client.api.http.ClickHouseHttpProto;
 import com.clickhouse.data.ClickHouseDataType;
 import com.clickhouse.jdbc.Driver;
 import com.google.common.collect.ImmutableMap;
@@ -129,7 +130,14 @@ private static String createConnectionURL(String url, boolean ssl) throws SQLExc
             : url;
         try {
             URI tmp = URI.create(adjustedURL);
-            return tmp.toASCIIString();
+            String asciiString = tmp.toASCIIString();
+            if (tmp.getPort() < 0) {
+                String port = ssl || adjustedURL.startsWith("https")
+                    ? ":" + ClickHouseHttpProto.DEFAULT_HTTPS_PORT
+                    : ":" + ClickHouseHttpProto.DEFAULT_HTTP_PORT;
+                asciiString += port;
+            }
+            return asciiString;
         } catch (IllegalArgumentException iae) {
             throw new SQLException("Failed to parse URL '" + url + "'", iae);
         }
@@ -171,6 +179,9 @@ private Map<String, String> parseUrl(String url) throws SQLException {
             throw new SQLException(
                 "Invalid authority part JDBC URL '" + url + "'");
         }
+        if (uri.getAuthority().contains(",")) {
+            throw new SQLException("Multiple endpoints not supported");
+        }
         properties.put(PARSE_URL_CONN_URL_PROP, uri.getScheme() + "://"
             + uri.getRawAuthority()); // will be parsed again later
         if (uri.getPath() != null

jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/JdbcConfigurationTest.java

@@ -1,6 +1,8 @@
 package com.clickhouse.jdbc.internal;
 
+import com.clickhouse.client.api.Client;
 import com.clickhouse.client.api.ClientConfigProperties;
+
 import org.testng.Assert;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
@@ -23,15 +25,17 @@ public class JdbcConfigurationTest {
         new JdbcConfigurationTestData("jdbc:clickhouse://localhost"),
         new JdbcConfigurationTestData("jdbc:clickhouse:http://localhost"),
         new JdbcConfigurationTestData("jdbc:clickhouse:https://localhost")
-            .withExpectedConnectionURL("https://localhost"),
+            .withExpectedConnectionURL("https://localhost:8443"),
+        new JdbcConfigurationTestData("jdbc:clickhouse:https://localhost:8123")
+            .withExpectedConnectionURL("https://localhost:8123"),
         new JdbcConfigurationTestData("jdbc:clickhouse://localhost")
             .withAdditionalConnectionParameters(
                 Map.of(JdbcConfiguration.USE_SSL_PROP, "true"))
-            .withExpectedConnectionURL("https://localhost")
+            .withExpectedConnectionURL("https://localhost:8443")
             .withAdditionalExpectedClientProperties(
                 Map.of("ssl", "true")),
         new JdbcConfigurationTestData("jdbc:clickhouse://[::1]")
-            .withExpectedConnectionURL("http://[::1]"),
+            .withExpectedConnectionURL("http://[::1]:8123"),
         new JdbcConfigurationTestData("jdbc:clickhouse://[::1]:8123")
             .withExpectedConnectionURL("http://[::1]:8123"),
         new JdbcConfigurationTestData("jdbc:clickhouse://localhost:8443")
@@ -111,14 +115,22 @@ public class JdbcConfigurationTest {
                     ))
     };
 
+    @SuppressWarnings("deprecation")
     @Test(dataProvider = "validURLTestData")
     public void testParseURLValid(String jdbcURL, Properties properties,
-        String connectionUrl, Map<String, String> expectedClientProps)
+        String connectionURL, Map<String, String> expectedClientProps)
             throws Exception
     {
         JdbcConfiguration configuration = new JdbcConfiguration(jdbcURL, properties);
-        assertEquals(configuration.getConnectionUrl(), connectionUrl);
+        assertEquals(configuration.getConnectionUrl(), connectionURL);
         assertEquals(configuration.clientProperties, expectedClientProps);
+        Client.Builder bob = new Client.Builder();
+        configuration.applyClientProperties(bob);
+        Client client = bob.build();
+        assertEquals(client.getEndpoints().size(), 1);
+        assertEquals(
+            client.getEndpoints().iterator().next(),
+            connectionURL);
     }
 
     @Test(dataProvider = "invalidURLs")
@@ -198,6 +210,9 @@ public Object[][] createInvalidConnectionURLs() {
             { "jdbc:clickhouse://foo.bar?x&y=z" },
             { "jdbc:clickhouse://foo.bar?x==&y=z" },
             { "jdbc:clickhouse://localhost?☺=value1" },
+            // multiple endpoints are invalid
+            { "jdbc:clickhouse://foo,bar" },
+            { "jdbc:clickhouse://foo,bar.com:8123" },
         };
     }
 
@@ -210,7 +225,7 @@ private static final class JdbcConfigurationTestData {
             Map.of( "user", "default", "password", "");
 
         private static final String DEFAULT_EXPECTED_CONNECTION_URL =
-            "http://localhost";
+            "http://localhost:8123";
 
         private final String url;
         private final Properties connectionParameters;