Skip to content

[jdbc-v2] Set default ports, parse multiple endpoints #2488

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[jdbc-v2] Set default ports, parse multiple endpoints #2488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 51 additions & 16 deletions jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.jdbc.Driver;
import com.google.common.collect.ImmutableMap;

Expand All @@ -11,11 +12,14 @@
import java.nio.charset.StandardCharsets;
import java.sql.DriverPropertyInfo;
import java.sql.SQLException;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

Expand All @@ -38,7 +42,7 @@ public Map<String, String> getClientProperties() {

private final Map<String, String> driverProperties;

private final String connectionUrl;
private final Set<String> connectionURLs;

public boolean isDisableFrameworkDetection() {
return disableFrameworkDetection;
Expand Down Expand Up @@ -74,7 +78,7 @@ public JdbcConfiguration(String url, Properties info) throws SQLException {
clientProperties.put(ClientConfigProperties.BEARERTOKEN_AUTH.getKey(), bearerToken);
}

this.connectionUrl = createConnectionURL(tmpConnectionUrl, useSSL);
this.connectionURLs = createConnectionURLs(tmpConnectionUrl, useSSL);
this.isIgnoreUnsupportedRequests = Boolean.parseBoolean(getDriverProperty(DriverProperties.IGNORE_UNSUPPORTED_VALUES.getKey(), "false"));
}

Expand Down Expand Up @@ -109,31 +113,62 @@ public static boolean acceptsURL(String url) throws SQLException {
}
}

public String getConnectionUrl() {
return connectionUrl;
Set<String> getConnectionURLs() {
return Collections.unmodifiableSet(connectionURLs);
}

/**
* Returns normalized URL that can be passed as parameter to Client#addEndpoint().
* Returned url has only schema and authority and doesn't have query parameters or path.
* Returns normalized URLs that can be passed as parameter to Client#addEndpoint().
* Returned urls have only schema and authority and doesn't have query parameters or path.
* JDBC URL should have only a single path parameter to specify database name.
* Note: Some BI tools do not let pass JDBC URL, so ssl is passed as property.
* @param url - JDBC url
* @param url - JDBC url without path and query parameters
* @param ssl - if SSL protocol should be used
* @return URL without JDBC prefix
* @return URLs to be used as endpoints
*/
private static String createConnectionURL(String url, boolean ssl) throws SQLException {
String adjustedURL = ssl && url.startsWith("http://")
? "https://" + url.substring(7)
: url;
private static Set<String> createConnectionURLs(String url, boolean ssl) throws SQLException {
try {
URI tmp = URI.create(adjustedURL);
return tmp.toASCIIString();
URI tmp = URI.create(url);
String authority = tmp.getRawAuthority();
if (!authority.contains(",")) {
return Collections.singleton(checkAndAdjustURL(tmp, ssl));
}
// split() with single comma character is not expensive
String[] hosts = authority.split(",");
if (hosts.length == 0) {
throw new IllegalArgumentException("No endpoints in URL '" + url + "'");
}
String proto = tmp.getScheme();
return Arrays.stream(hosts)
.map(h -> checkAndAdjustURL(URI.create(proto + "://" + h), ssl))
.collect(Collectors.toSet());
} catch (IllegalArgumentException iae) {
throw new SQLException("Failed to parse URL '" + url + "'", iae);
}
}

private static String checkAndAdjustURL(URI url, boolean ssl)
throws IllegalArgumentException
{
if (url.getAuthority() == null
|| url.getAuthority().trim().isEmpty()
|| url.getAuthority().startsWith(":"))
{
throw new IllegalArgumentException(
"URL has invalid authority '" + url.getAuthority() + "'");
}
String asciiString = url.toASCIIString();
String sslProtoAdjustedURL = ssl && asciiString.startsWith("http://")
? "https://" + asciiString.substring(7)
: asciiString;
if (url.getPort() > -1) {
return sslProtoAdjustedURL;
}
return ssl || asciiString.startsWith("https://")
? sslProtoAdjustedURL + ":" + String.valueOf(ClickHouseHttpProto.DEFAULT_HTTPS_PORT)
: sslProtoAdjustedURL + ":" + String.valueOf(ClickHouseHttpProto.DEFAULT_HTTP_PORT);
}

private static String stripJDBCPrefix(String url) {
if (url.startsWith(PREFIX_CLICKHOUSE)) {
return url.substring(PREFIX_CLICKHOUSE.length());
Expand Down Expand Up @@ -266,8 +301,8 @@ public String getDriverProperty(String key, String defaultValue) {
}

public Client.Builder applyClientProperties(Client.Builder builder) {
builder.addEndpoint(connectionUrl)
.setOptions(clientProperties);
builder.setOptions(clientProperties);
connectionURLs.forEach(builder::addEndpoint);
return builder;
}

Expand Down
97 changes: 83 additions & 14 deletions jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/JdbcConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,37 +1,43 @@
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;

import java.sql.DriverPropertyInfo;
import java.sql.SQLException;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Properties;
import java.util.Set;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertThrows;

public class JdbcConfigurationTest {

private static final JdbcConfigurationTestData[] VALID_URLs = new JdbcConfigurationTestData[] {
private static final JdbcConfigurationTestData[] VALID_URLs = new JdbcConfigurationTestData[] {
new JdbcConfigurationTestData("jdbc:ch://localhost"),
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://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")
Expand Down Expand Up @@ -108,17 +114,61 @@ public class JdbcConfigurationTest {
"database", "default",
"custom_header1", "\"role 1,3,4\",'val2',val3",
"param1", "value1"
))
)),
new JdbcConfigurationTestData("jdbc:clickhouse://foo:8443,bar:8123")
.withExpectedConnectionURLs(
Set.of(
"http://foo:8443",
"http://bar:8123")),
new JdbcConfigurationTestData("jdbc:ch://foo.bar,baz.qux")
.withExpectedConnectionURLs(
Set.of(
"http://foo.bar:8123",
"http://baz.qux:8123")),
new JdbcConfigurationTestData("jdbc:ch:https://foo.bar,baz.qux:4242")
.withExpectedConnectionURLs(
Set.of(
"https://foo.bar:8443",
"https://baz.qux:4242")),
new JdbcConfigurationTestData("jdbc:ch:http://foo.bar,baz.qux:4242")
.withAdditionalConnectionParameters(
Map.of(JdbcConfiguration.USE_SSL_PROP, "true"))
.withAdditionalExpectedClientProperties(
Map.of(JdbcConfiguration.USE_SSL_PROP, "true"))
.withExpectedConnectionURLs(
Set.of(
"https://foo.bar:8443",
"https://baz.qux:4242")),
new JdbcConfigurationTestData("jdbc:ch://127.0.0.1,baz.qux")
.withExpectedConnectionURLs(
Set.of(
"http://127.0.0.1:8123",
"http://baz.qux:8123")),
new JdbcConfigurationTestData("jdbc:ch://foo,bar,baz")
.withExpectedConnectionURLs(
Set.of(
"http://foo:8123",
"http://bar:8123",
"http://baz:8123"))
};

@SuppressWarnings("deprecation")
@Test(dataProvider = "validURLTestData")
public void testParseURLValid(String jdbcURL, Properties properties,
String connectionUrl, Map<String, String> expectedClientProps)
public void testParseURLValidAndApply(String jdbcURL, Properties properties,
Set<String> connectionURLs, Map<String, String> expectedClientProps)
throws Exception
{
JdbcConfiguration configuration = new JdbcConfiguration(jdbcURL, properties);
assertEquals(configuration.getConnectionUrl(), connectionUrl);
assertEquals(configuration.getConnectionURLs(), connectionURLs);
assertEquals(configuration.clientProperties, expectedClientProps);
Client.Builder bob = new Client.Builder();
configuration.applyClientProperties(bob);
Client client = bob.build();
assertEquals(
client.getEndpoints(),
connectionURLs);
expectedClientProps.entrySet().forEach(
e -> assertEquals(client.getConfiguration().get(e.getKey()), e.getValue()));
}

@Test(dataProvider = "invalidURLs")
Expand All @@ -142,7 +192,7 @@ public void testConfigurationProperties() throws Exception {
ClientConfigProperties.commaSeparated(Arrays.asList("http_headers=header1=3,header2=4")));
String url = "jdbc:clickhouse://localhost:8123/clickhouse?client_name=test_application&database=default1";
JdbcConfiguration configuration = new JdbcConfiguration(url, properties);
assertEquals(configuration.getConnectionUrl(), "http://localhost:8123");
assertEquals(configuration.getConnectionURLs().iterator().next(), "http://localhost:8123");
DriverPropertyInfo p = configuration.getDriverPropertyInfo().stream()
.filter(dpi -> ClientConfigProperties.DATABASE.getKey().equals(dpi.name))
.findAny()
Expand All @@ -156,7 +206,7 @@ public Object[][] createValidConnectionURLTestData() {
.map(d -> new Object[] {
d.url,
d.connectionParameters,
d.expectedConnectionURL,
d.expectedConnectionURLs,
d.expectedClientProperties
})
.toArray(Object[][]::new);
Expand Down Expand Up @@ -198,6 +248,17 @@ public Object[][] createInvalidConnectionURLs() {
{ "jdbc:clickhouse://foo.bar?x&y=z" },
{ "jdbc:clickhouse://foo.bar?x==&y=z" },
{ "jdbc:clickhouse://localhost?☺=value1" },
// would have to compare split() result length with number of commas
// { "jdbc:clickhouse://foo,bar,/" },
{ "jdbc:clickhouse://foo,,bar" },
{ "jdbc:clickhouse://,foo,bar" },
{ "jdbc:clickhouse://,," },
{ "jdbc:clickhouse://,%20," },
{ "jdbc:clickhouse://foo,bar, /" },
{ "jdbc:clickhouse://foo,bar,%20/" },
{ "jdbc:clickhouse://foo,bar,:8123" },
{ "jdbc:clickhouse://foo,bar,:8123/db" },
{ "jdbc:clickhouse://[::1],[::2]" }
};
}

Expand All @@ -210,18 +271,19 @@ 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;
private String expectedConnectionURL;
private Set<String> expectedConnectionURLs;
private final Map<String, String> expectedClientProperties;

JdbcConfigurationTestData(String url) {
this.url = Objects.requireNonNull(url);
this.connectionParameters = new Properties();
this.connectionParameters.putAll(DEFAULT_CONNECTION_PARAMS);
this.expectedConnectionURL = DEFAULT_EXPECTED_CONNECTION_URL;
this.expectedConnectionURLs = Collections.singleton(
DEFAULT_EXPECTED_CONNECTION_URL);
this.expectedClientProperties = new HashMap<>(
DEFAULT_EXPECTED_CLIENT_PROPERTIES); // modifiable
}
Expand All @@ -236,7 +298,14 @@ JdbcConfigurationTestData withAdditionalConnectionParameters(
JdbcConfigurationTestData withExpectedConnectionURL(
String expectedConnectionURL)
{
this.expectedConnectionURL = Objects.requireNonNull(expectedConnectionURL);
this.expectedConnectionURLs = Collections.singleton(expectedConnectionURL);
return this;
}

JdbcConfigurationTestData withExpectedConnectionURLs(
Collection<String> newExpectedConnectionURLs)
{
this.expectedConnectionURLs = new HashSet<>(newExpectedConnectionURLs);
return this;
}

Expand Down