From 8b9163930bbeecd664a71f8c9b866e90fd2dfb62 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 26 Dec 2024 10:23:16 -0800 Subject: [PATCH 1/7] fixed client name in client-v2 --- .../com/clickhouse/client/api/Client.java | 38 ++---------- .../api/internal/HttpAPIClientHelper.java | 56 +++++++++++++++-- .../clickhouse/client/HttpTransportTests.java | 62 ++++++++++++++++++- 3 files changed, 115 insertions(+), 41 deletions(-) diff --git a/client-v2/src/main/java/com/clickhouse/client/api/Client.java b/client-v2/src/main/java/com/clickhouse/client/api/Client.java index 6b9e118df..82ba0273f 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/Client.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/Client.java @@ -186,6 +186,7 @@ private Client(Set endpoints, Map configuration, boolean } this.columnToMethodMatchingStrategy = columnToMethodMatchingStrategy; + updateServerContext(); } @@ -1147,41 +1148,7 @@ private void setDefaults() { if (!configuration.containsKey(ClientConfigProperties.USE_HTTP_COMPRESSION.getKey())) { useHttpCompression(false); } - - String userAgent = configuration.getOrDefault(ClientConfigProperties.HTTP_HEADER_PREFIX + HttpHeaders.USER_AGENT.toUpperCase(Locale.US), ""); - String clientName = configuration.getOrDefault(ClientConfigProperties.CLIENT_NAME.getKey(), ""); - httpHeader(HttpHeaders.USER_AGENT, buildUserAgent(userAgent.isEmpty() ? clientName : userAgent)); - } - - private static String buildUserAgent(String customUserAgent) { - - StringBuilder userAgent = new StringBuilder(); - if (customUserAgent != null && !customUserAgent.isEmpty()) { - userAgent.append(customUserAgent).append(" "); - } - - userAgent.append(CLIENT_USER_AGENT); - - String clientVersion = Client.class.getPackage().getImplementationVersion(); - if (clientVersion == null) { - clientVersion = LATEST_ARTIFACT_VERSION; - } - userAgent.append(clientVersion); - - userAgent.append(" ("); - userAgent.append(System.getProperty("os.name")); - userAgent.append("; "); - userAgent.append("jvm:").append(System.getProperty("java.version")); - userAgent.append("; "); - - userAgent.setLength(userAgent.length() - 2); - userAgent.append(')'); - - return userAgent.toString(); } - - public static final String LATEST_ARTIFACT_VERSION = "0.7.1-patch1"; - public static final String CLIENT_USER_AGENT = "clickhouse-java-v2/"; } private ClickHouseNode getServerNode() { @@ -2186,6 +2153,9 @@ public void updateClientName(String name) { this.configuration.put(ClientConfigProperties.CLIENT_NAME.getKey(), name); } + public static final String LATEST_ARTIFACT_VERSION = "~0.7.2"; + public static final String CLIENT_USER_AGENT = "clickhouse-java-v2/"; + private Collection unmodifiableDbRolesView = Collections.emptyList(); /** diff --git a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java index 5a3fa227b..104134a89 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java @@ -34,6 +34,7 @@ import org.apache.hc.core5.http.HttpEntity; import org.apache.hc.core5.http.HttpHeaders; import org.apache.hc.core5.http.HttpHost; +import org.apache.hc.core5.http.HttpRequest; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.NoHttpResponseException; import org.apache.hc.core5.http.config.CharCodingConfig; @@ -69,6 +70,7 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -92,14 +94,12 @@ public class HttpAPIClientHelper { private final Set defaultRetryCauses; - private String httpClientUserAgentPart; + private String defaultUserAgent; public HttpAPIClientHelper(Map configuration) { this.chConfiguration = configuration; this.httpClient = createHttpClient(); - this.httpClientUserAgentPart = this.httpClient.getClass().getPackage().getImplementationTitle() + "/" + this.httpClient.getClass().getPackage().getImplementationVersion(); - RequestConfig.Builder reqConfBuilder = RequestConfig.custom(); MapUtils.applyLong(chConfiguration, "connection_request_timeout", (t) -> reqConfBuilder @@ -116,6 +116,8 @@ public HttpAPIClientHelper(Map configuration) { if (defaultRetryCauses.contains(ClientFaultCause.None)) { defaultRetryCauses.removeIf(c -> c != ClientFaultCause.None); } + + this.defaultUserAgent = buildDefaultUserAgent(); } /** @@ -472,9 +474,9 @@ private void addHeaders(HttpPost req, Map chConfig, Map chConfig, Map requestConfig) { for (Map.Entry entry : chConfig.entrySet()) { if (entry.getKey().startsWith(ClientConfigProperties.SERVER_SETTING_PREFIX)) { @@ -646,6 +648,50 @@ public static Map parseUrlParameters(URL url) { return params; } + + private void correctUserAgentHeader(HttpRequest request) { + Header userAgentHeader = request.getLastHeader(HttpHeaders.USER_AGENT); + request.removeHeaders(HttpHeaders.USER_AGENT); + + String clientName = chConfiguration.getOrDefault(ClientConfigProperties.CLIENT_NAME.getKey(), ""); + + String userAgentValue = defaultUserAgent; + if (userAgentHeader == null && clientName != null && !clientName.isEmpty()) { + userAgentValue = clientName + " " + defaultUserAgent; + } else if (userAgentHeader != null) { + userAgentValue = userAgentHeader.getValue() + " " + defaultUserAgent; + } + + request.setHeader(HttpHeaders.USER_AGENT, userAgentValue); + } + + private String buildDefaultUserAgent() { + StringBuilder userAgent = new StringBuilder(); + userAgent.append(Client.CLIENT_USER_AGENT); + + String clientVersion = Client.class.getPackage().getImplementationVersion(); + if (clientVersion == null) { + clientVersion = Client.LATEST_ARTIFACT_VERSION; + } + userAgent.append(clientVersion); + + userAgent.append(" ("); + userAgent.append(System.getProperty("os.name")); + userAgent.append("; "); + userAgent.append("jvm:").append(System.getProperty("java.version")); + userAgent.append("; "); + + userAgent.setLength(userAgent.length() - 2); + userAgent.append(')'); + + userAgent.append(" ") + .append(this.httpClient.getClass().getPackage().getImplementationTitle()) + .append('/') + .append(this.httpClient.getClass().getPackage().getImplementationVersion()); + + return userAgent.toString(); + } + public void close() { httpClient.close(CloseMode.IMMEDIATE); } diff --git a/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java b/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java index 0b7cba523..958d51b55 100644 --- a/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java @@ -928,13 +928,71 @@ public void testJWTWithCloud() throws Exception { } } + + @Test(dataProvider = "testClientNameDataProvider") + public void testClientName(String clientName, boolean setWithUpdate, String userAgentHeader, boolean setForRequest) throws Exception { + + final String initialClientName = setWithUpdate ? "init clientName" : clientName; + final String initialUserAgentHeader = setForRequest ? "init userAgentHeader" : userAgentHeader; + final String clientReferer = "http://localhost/webpage"; + + Client.Builder builder = newClient(); + if (initialClientName != null) { + builder.setClientName(initialClientName); + } + if (initialUserAgentHeader != null) { + builder.httpHeader(HttpHeaders.USER_AGENT, initialUserAgentHeader); + } + try (Client client = builder.build()) { + String expectedClientNameStartsWith = initialClientName == null || initialUserAgentHeader != null ? initialUserAgentHeader : initialClientName; + + if (setWithUpdate) { + client.updateClientName(clientName); + expectedClientNameStartsWith = initialUserAgentHeader == null ? clientName : initialUserAgentHeader; + } + + String qId = UUID.randomUUID().toString(); + QuerySettings settings = new QuerySettings() + .httpHeader(HttpHeaders.REFERER, clientReferer) + .setQueryId(qId); + + if (setForRequest) { + settings.httpHeader(HttpHeaders.USER_AGENT, userAgentHeader); + expectedClientNameStartsWith = userAgentHeader; + } + + client.query("SELECT 1", settings).get().close(); + client.execute("SYSTEM FLUSH LOGS").get().close(); + + List logRecords = client.queryAll("SELECT query_id, client_name, http_user_agent, http_referer " + + " FROM system.query_log WHERE query_id = '" + settings.getQueryId() + "'"); + Assert.assertEquals(logRecords.get(0).getString("query_id"), settings.getQueryId()); + final String logUserAgent = logRecords.get(0).getString("http_user_agent"); + Assert.assertTrue(logUserAgent.startsWith(expectedClientNameStartsWith), + "Expected to start with \"" + expectedClientNameStartsWith + "\" but values was \"" + logUserAgent + "\"" ); + Assert.assertTrue(logUserAgent.contains(Client.CLIENT_USER_AGENT), "Expected to contain client v2 version but value was \"" + logUserAgent + "\""); + Assert.assertEquals(logRecords.get(0).getString("http_referer"), clientReferer); + Assert.assertEquals(logRecords.get(0).getString("client_name"), ""); // http client can't set this field + } + } + + @DataProvider(name = "testClientNameDataProvider") + public static Object[][] testClientName() { + return new Object[][] { + {"test-product (app 1.0)", false, null, false}, // only client name set + {"test-product (app 1.0)", false, "final product (app 1.1)", false}, // http header set and overrides client name + {"test-product (app 1.0)", true, null, false}, // client name set thru Client#updateClientName + {"test-product (app 1.0)", true, "final product (app 1.1)", true}, // custom UserAgent header overrides client name + }; + } + protected Client.Builder newClient() { ClickHouseNode node = getServer(ClickHouseProtocol.HTTP); boolean isSecure = isCloud(); return new Client.Builder() .addEndpoint(Protocol.HTTP, node.getHost(), node.getPort(), isSecure) -// .setUsername("default") -// .setPassword(ClickHouseServerForTest.getPassword()) + .setUsername("default") + .setPassword(ClickHouseServerForTest.getPassword()) .compressClientRequest(false) .setDefaultDatabase(ClickHouseServerForTest.getDatabase()) .serverSetting(ServerSettings.WAIT_END_OF_QUERY, "1") From 5694bfaeb33f05f53115f99df53cd40d78942292 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 26 Dec 2024 17:49:39 -0800 Subject: [PATCH 2/7] fixed client name in jdbc-v2 --- .../api/internal/HttpAPIClientHelper.java | 2 +- .../com/clickhouse/jdbc/ConnectionImpl.java | 7 ++- .../main/java/com/clickhouse/jdbc/Driver.java | 6 ++- .../jdbc/internal/JdbcConfiguration.java | 14 ++++++ .../com/clickhouse/jdbc/ConnectionTest.java | 47 ++++++++++++++++--- 5 files changed, 65 insertions(+), 11 deletions(-) diff --git a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java index 104134a89..7583d5d5b 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java @@ -685,7 +685,7 @@ private String buildDefaultUserAgent() { userAgent.append(')'); userAgent.append(" ") - .append(this.httpClient.getClass().getPackage().getImplementationTitle()) + .append(this.httpClient.getClass().getPackage().getImplementationTitle().replaceAll(" ", "-")) .append('/') .append(this.httpClient.getClass().getPackage().getImplementationVersion()); diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java index 519fcf586..9e7b706bc 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java @@ -435,10 +435,13 @@ public boolean isValid(int timeout) throws SQLException { return true; } + private String appName = ""; + @Override public void setClientInfo(String name, String value) throws SQLClientInfoException { if (ClientInfoProperties.APPLICATION_NAME.getKey().equals(name)) { - client.updateClientName(value); + config.updateUserClient(value, client); + appName = value; } // TODO: generate warning for unknown properties } @@ -471,7 +474,7 @@ public void setClientInfo(Properties properties) throws SQLClientInfoException { public String getClientInfo(String name) throws SQLException { checkOpen(); if (ClientInfoProperties.APPLICATION_NAME.getKey().equals(name)) { - return client.getConfiguration().get(ClientConfigProperties.CLIENT_NAME.getKey()); + return appName; } else { return null; } diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java index 4231172c8..484589655 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java @@ -54,13 +54,17 @@ public static String getFrameworksDetected() { } } + public static final String DRIVER_CLIENT_NAME = "jdbc-v2/"; + + private static final String LAST_KNOWN_VERSION = "~0.7.2"; + static { log.debug("Initializing ClickHouse JDBC driver V2"); String tempDriverVersion = Driver.class.getPackage().getImplementationVersion(); //If the version is not available, set it to 1.0 if (tempDriverVersion == null || tempDriverVersion.isEmpty()) { log.warn("ClickHouse JDBC driver version is not available"); - tempDriverVersion = "1.0.0"; + tempDriverVersion = LAST_KNOWN_VERSION; } driverVersion = tempDriverVersion; diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java index 9edcdd1cb..b05a98701 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcConfiguration.java +++ b/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.jdbc.Driver; import java.net.URI; import java.sql.DriverPropertyInfo; @@ -242,4 +243,17 @@ public Client.Builder applyClientProperties(Client.Builder builder) { return builder; } + + public void updateUserClient(String clientName, Client client) { + client.updateClientName((clientName == null || clientName.isEmpty() ? "" : clientName) + ' ' + getDefaultClientName()); + } + + public static String getDefaultClientName() { + StringBuilder jdbcName = new StringBuilder(); + jdbcName.append(Driver.DRIVER_CLIENT_NAME) + .append(Driver.driverVersion); + + return jdbcName.toString(); + } + } diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java index 52a7c5c32..204510319 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java @@ -4,15 +4,19 @@ import java.util.Properties; import java.util.Properties; +import java.util.UUID; import com.clickhouse.client.ClickHouseNode; import com.clickhouse.client.ClickHouseProtocol; +import com.clickhouse.client.api.Client; import com.clickhouse.client.api.ClientConfigProperties; import com.clickhouse.client.api.ServerException; import com.clickhouse.client.api.internal.ServerSettings; import com.clickhouse.jdbc.internal.ClientInfoProperties; import com.clickhouse.jdbc.internal.DriverProperties; +import com.clickhouse.jdbc.internal.JdbcUtils; import org.testng.Assert; +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import static org.testng.Assert.assertEquals; @@ -233,15 +237,44 @@ public void isValidTest() throws SQLException { Assert.assertTrue(localConnection.isValid(0)); } - @Test(groups = { "integration" }) - public void setAndGetClientInfoTest() throws SQLException { - Connection localConnection = this.getJdbcConnection(); - localConnection.setClientInfo("custom-property", "client-name"); - Assert.assertNull(localConnection.getClientInfo("custom-property")); - localConnection.setClientInfo(ClientInfoProperties.APPLICATION_NAME.getKey(), "client-name"); - Assert.assertEquals(localConnection.getClientInfo(ClientInfoProperties.APPLICATION_NAME.getKey()), "client-name"); + @Test(groups = { "integration" }, dataProvider = "setAndGetClientInfoTestDataProvider") + public void setAndGetClientInfoTest(String clientName) throws SQLException { + final String unsupportedProperty = "custom-unsupported-property"; + try (Connection localConnection = this.getJdbcConnection(); + Statement stmt = localConnection.createStatement()) { + localConnection.setClientInfo(unsupportedProperty, "i-am-unsupported-property"); + Assert.assertNull(localConnection.getClientInfo("custom-property")); + localConnection.setClientInfo(ClientInfoProperties.APPLICATION_NAME.getKey(), clientName); + Assert.assertEquals(localConnection.getClientInfo(ClientInfoProperties.APPLICATION_NAME.getKey()), clientName); + Assert.assertNull(localConnection.getClientInfo(unsupportedProperty)); + + final String testQuery = "SELECT '" + UUID.randomUUID() + "'"; + stmt.execute(testQuery); + stmt.execute("SYSTEM FLUSH LOGS"); + + final String logQuery ="SELECT http_user_agent " + + " FROM system.query_log WHERE query = '" + testQuery.replaceAll("'", "\\\\'") + "'"; + try (ResultSet rs = stmt.executeQuery(logQuery)) { + Assert.assertTrue(rs.next()); + String userAgent = rs.getString("http_user_agent"); + System.out.println(userAgent); + if (clientName != null && !clientName.isEmpty()) { + Assert.assertTrue(userAgent.startsWith(clientName), "Expected to start with '" + clientName + "' but value was '" + userAgent + "'"); + } + Assert.assertTrue(userAgent.contains(Client.CLIENT_USER_AGENT), "Expected to contain '" + Client.CLIENT_USER_AGENT + "' but value was '" + userAgent + "'"); + Assert.assertTrue(userAgent.contains(Driver.DRIVER_CLIENT_NAME), "Expected to contain '" + Driver.DRIVER_CLIENT_NAME + "' but value was '" + userAgent + "'"); + } + } } + @DataProvider(name = "setAndGetClientInfoTestDataProvider") + public static Object[][] setAndGetClientInfoTestDataProvider() { + return new Object[][] { + {"product (version 1.0)"}, + {null}, + {""} + }; + } @Test(groups = { "integration" }) public void createArrayOfTest() throws SQLException { From 6332bd50a9037e8524cf4bdcb3069e22be375385 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 26 Dec 2024 19:08:12 -0800 Subject: [PATCH 3/7] fixed product_name as alias to client_name in client-v2 --- .../com/clickhouse/client/api/Client.java | 10 +- .../client/api/ClientConfigProperties.java | 11 +- .../client/api/insert/InsertSettings.java | 6 +- .../api/internal/HttpAPIClientHelper.java | 15 +- .../client/api/query/QuerySettings.java | 4 + .../com/clickhouse/client/ClientTests.java | 15 ++ .../clickhouse/client/HttpTransportTests.java | 161 +++++++++++------- .../com/clickhouse/client/SettingsTests.java | 1 + jdbc-v2/pom.xml | 2 +- .../java/com/clickhouse/jdbc/DriverTest.java | 4 +- 10 files changed, 152 insertions(+), 77 deletions(-) diff --git a/client-v2/src/main/java/com/clickhouse/client/api/Client.java b/client-v2/src/main/java/com/clickhouse/client/api/Client.java index 82ba0273f..e1645ca9b 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/Client.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/Client.java @@ -26,10 +26,8 @@ import com.clickhouse.client.api.internal.ClickHouseLZ4OutputStream; import com.clickhouse.client.api.internal.ClientStatisticsHolder; import com.clickhouse.client.api.internal.ClientV1AdaptorHelper; -import com.clickhouse.client.api.internal.EnvUtils; import com.clickhouse.client.api.internal.HttpAPIClientHelper; import com.clickhouse.client.api.internal.MapUtils; -import com.clickhouse.client.api.internal.ServerSettings; import com.clickhouse.client.api.internal.SettingsConverter; import com.clickhouse.client.api.internal.TableSchemaParser; import com.clickhouse.client.api.internal.ValidationUtils; @@ -76,7 +74,6 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.StringJoiner; @@ -360,6 +357,9 @@ public Builder addEndpoint(Protocol protocol, String host, int port, boolean sec */ public Builder setOption(String key, String value) { this.configuration.put(key, value); + if (key.equals(ClientConfigProperties.PRODUCT_NAME.getKey())) { + setClientName(value); + } return this; } @@ -978,7 +978,9 @@ public Builder setClientName(String clientName) { * @return same instance of the builder */ public Builder setOptions(Map options) { - this.configuration.putAll(options); + for (Map.Entry entry : options.entrySet()) { + setOption(entry.getKey(), entry.getValue()); + } return this; } diff --git a/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java b/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java index 6a0e30af7..268f0e093 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java @@ -115,7 +115,16 @@ public enum ClientConfigProperties { CLIENT_RETRY_ON_FAILURE("client_retry_on_failures"), - CLIENT_NAME("client_name"); + CLIENT_NAME("client_name"), + + /** + * An old alias to {@link ClientConfigProperties#CLIENT_NAME}. Using the last one is preferred. + */ + @Deprecated + PRODUCT_NAME("product_name"), + + + ; private String key; diff --git a/client-v2/src/main/java/com/clickhouse/client/api/insert/InsertSettings.java b/client-v2/src/main/java/com/clickhouse/client/api/insert/InsertSettings.java index b664d1937..de38fbafc 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/insert/InsertSettings.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/insert/InsertSettings.java @@ -48,8 +48,12 @@ public Object getOption(String option) { * @param option - configuration option name * @param value - configuration option value */ - public void setOption(String option, Object value) { + public InsertSettings setOption(String option, Object value) { rawSettings.put(option, value); + if (option.equals(ClientConfigProperties.PRODUCT_NAME.getKey())) { + rawSettings.put(ClientConfigProperties.CLIENT_NAME.getKey(), value); + } + return this; } /** diff --git a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java index 7583d5d5b..69411ee35 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java @@ -70,14 +70,10 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.function.Function; -import java.util.function.Supplier; - -import static com.clickhouse.client.api.ClientConfigProperties.SOCKET_TCP_NO_DELAY_OPT; public class HttpAPIClientHelper { private static final Logger LOG = LoggerFactory.getLogger(Client.class); @@ -474,7 +470,7 @@ private void addHeaders(HttpPost req, Map chConfig, Map chConfig, Map requestConfig) { @@ -649,12 +645,17 @@ public static Map parseUrlParameters(URL url) { } - private void correctUserAgentHeader(HttpRequest request) { + private void correctUserAgentHeader(HttpRequest request, Map requestConfig) { Header userAgentHeader = request.getLastHeader(HttpHeaders.USER_AGENT); request.removeHeaders(HttpHeaders.USER_AGENT); String clientName = chConfiguration.getOrDefault(ClientConfigProperties.CLIENT_NAME.getKey(), ""); - + if (requestConfig != null) { + String reqClientName = (String) requestConfig.get(ClientConfigProperties.CLIENT_NAME.getKey()); + if (reqClientName != null && !reqClientName.isEmpty()) { + clientName = reqClientName; + } + } String userAgentValue = defaultUserAgent; if (userAgentHeader == null && clientName != null && !clientName.isEmpty()) { userAgentValue = clientName + " " + defaultUserAgent; diff --git a/client-v2/src/main/java/com/clickhouse/client/api/query/QuerySettings.java b/client-v2/src/main/java/com/clickhouse/client/api/query/QuerySettings.java index d2ef36690..9140b6739 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/query/QuerySettings.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/query/QuerySettings.java @@ -35,6 +35,10 @@ public QuerySettings() { */ public QuerySettings setOption(String option, Object value) { rawSettings.put(option, value); + if (option.equals(ClientConfigProperties.PRODUCT_NAME.getKey())) { + rawSettings.put(ClientConfigProperties.CLIENT_NAME.getKey(), value); + } + return this; } diff --git a/client-v2/src/test/java/com/clickhouse/client/ClientTests.java b/client-v2/src/test/java/com/clickhouse/client/ClientTests.java index 0e56fc617..db8796a0f 100644 --- a/client-v2/src/test/java/com/clickhouse/client/ClientTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/ClientTests.java @@ -1,6 +1,7 @@ package com.clickhouse.client; import com.clickhouse.client.api.Client; +import com.clickhouse.client.api.ClientConfigProperties; import com.clickhouse.client.api.ClientException; import com.clickhouse.client.api.enums.Protocol; import com.clickhouse.client.api.query.GenericRecord; @@ -14,7 +15,9 @@ import org.testng.annotations.Test; import java.net.ConnectException; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -115,6 +118,18 @@ public void testPingFailure() { } } + @Test + public void testSetOptions() { + Map options = new HashMap<>(); + String productName = "my product_name (version 1.0)"; + options.put(ClickHouseClientOption.PRODUCT_NAME.getKey(), productName); + try (Client client = newClient() + .setOptions(options).build()) { + + Assert.assertEquals(client.getConfiguration().get(ClickHouseClientOption.PRODUCT_NAME.getKey()), productName); + } + } + protected Client.Builder newClient() { ClickHouseNode node = getServer(ClickHouseProtocol.HTTP); boolean isSecure = isCloud(); diff --git a/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java b/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java index 958d51b55..dace656c5 100644 --- a/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java @@ -1,6 +1,7 @@ package com.clickhouse.client; import com.clickhouse.client.api.Client; +import com.clickhouse.client.api.ClientConfigProperties; import com.clickhouse.client.api.ClientException; import com.clickhouse.client.api.ClientFaultCause; import com.clickhouse.client.api.ConnectionInitiationException; @@ -831,9 +832,105 @@ public void testUserAgentHasCompleteProductName(String clientName, Pattern userA @DataProvider(name = "testUserAgentHasCompleteProductName_dataProvider") public static Object[][] testUserAgentHasCompleteProductName_dataProvider() { return new Object[][] { - { "", Pattern.compile("clickhouse-java-v2\\/.+ \\(.+\\) Apache HttpClient\\/[\\d\\.]+$") }, - { "test-client/1.0", Pattern.compile("test-client/1.0 clickhouse-java-v2\\/.+ \\(.+\\) Apache HttpClient\\/[\\d\\.]+$")}, - { "test-client/", Pattern.compile("test-client/ clickhouse-java-v2\\/.+ \\(.+\\) Apache HttpClient\\/[\\d\\.]+$")}}; + { "", Pattern.compile("clickhouse-java-v2\\/.+ \\(.+\\) Apache-HttpClient\\/[\\d\\.]+$") }, + { "test-client/1.0", Pattern.compile("test-client/1.0 clickhouse-java-v2\\/.+ \\(.+\\) Apache-HttpClient\\/[\\d\\.]+$")}, + { "test-client/", Pattern.compile("test-client/ clickhouse-java-v2\\/.+ \\(.+\\) Apache-HttpClient\\/[\\d\\.]+$")}}; + } + + @Test(dataProvider = "testClientNameDataProvider") + public void testClientName(String clientName, boolean setWithUpdate, String userAgentHeader, boolean setForRequest) throws Exception { + + final String initialClientName = setWithUpdate ? "init clientName" : clientName; + final String initialUserAgentHeader = setForRequest ? "init userAgentHeader" : userAgentHeader; + final String clientReferer = "http://localhost/webpage"; + + Client.Builder builder = newClient(); + if (initialClientName != null) { + builder.setClientName(initialClientName); + } + if (initialUserAgentHeader != null) { + builder.httpHeader(HttpHeaders.USER_AGENT, initialUserAgentHeader); + } + try (Client client = builder.build()) { + String expectedClientNameStartsWith = initialClientName == null || initialUserAgentHeader != null ? initialUserAgentHeader : initialClientName; + + if (setWithUpdate) { + client.updateClientName(clientName); + expectedClientNameStartsWith = initialUserAgentHeader == null ? clientName : initialUserAgentHeader; + } + + String qId = UUID.randomUUID().toString(); + QuerySettings settings = new QuerySettings() + .httpHeader(HttpHeaders.REFERER, clientReferer) + .setQueryId(qId); + + if (setForRequest) { + settings.httpHeader(HttpHeaders.USER_AGENT, userAgentHeader); + expectedClientNameStartsWith = userAgentHeader; + } + + client.query("SELECT 1", settings).get().close(); + client.execute("SYSTEM FLUSH LOGS").get().close(); + + List logRecords = client.queryAll("SELECT query_id, client_name, http_user_agent, http_referer " + + " FROM system.query_log WHERE query_id = '" + settings.getQueryId() + "'"); + Assert.assertEquals(logRecords.get(0).getString("query_id"), settings.getQueryId()); + final String logUserAgent = logRecords.get(0).getString("http_user_agent"); + Assert.assertTrue(logUserAgent.startsWith(expectedClientNameStartsWith), + "Expected to start with \"" + expectedClientNameStartsWith + "\" but values was \"" + logUserAgent + "\"" ); + Assert.assertTrue(logUserAgent.contains(Client.CLIENT_USER_AGENT), "Expected to contain client v2 version but value was \"" + logUserAgent + "\""); + Assert.assertEquals(logRecords.get(0).getString("http_referer"), clientReferer); + Assert.assertEquals(logRecords.get(0).getString("client_name"), ""); // http client can't set this field + } + } + + @DataProvider(name = "testClientNameDataProvider") + public static Object[][] testClientName() { + return new Object[][] { + {"test-product (app 1.0)", false, null, false}, // only client name set + {"test-product (app 1.0)", false, "final product (app 1.1)", false}, // http header set and overrides client name + {"test-product (app 1.0)", true, null, false}, // client name set thru Client#updateClientName + {"test-product (app 1.0)", true, "final product (app 1.1)", true}, // custom UserAgent header overrides client name + }; + } + + @Test(dataProvider = "testClientNameThruRawOptionsDataProvider") + public void testClientNameThruRawOptions(String property, String value, boolean setInClient) throws Exception { + Client.Builder builder = newClient(); + if (setInClient) { + builder.setOption(property, value); + } + try (Client client = builder.build()) { + + String qId = UUID.randomUUID().toString(); + QuerySettings settings = new QuerySettings() + .setQueryId(qId); + + if (!setInClient) { + settings.setOption(property, value); + } + + client.query("SELECT 1", settings).get().close(); + client.execute("SYSTEM FLUSH LOGS").get().close(); + + List logRecords = client.queryAll("SELECT query_id, client_name, http_user_agent, http_referer " + + " FROM system.query_log WHERE query_id = '" + settings.getQueryId() + "'"); + Assert.assertEquals(logRecords.get(0).getString("query_id"), settings.getQueryId()); + final String logUserAgent = logRecords.get(0).getString("http_user_agent"); + Assert.assertTrue(logUserAgent.startsWith(value), + "Expected to start with \"" + value + "\" but values was \"" + logUserAgent + "\"" ); + Assert.assertTrue(logUserAgent.contains(Client.CLIENT_USER_AGENT), "Expected to contain client v2 version but value was \"" + logUserAgent + "\""); + } + } + + @DataProvider(name = "testClientNameThruRawOptionsDataProvider") + public Object[][] testClientNameThruRawOptionsDataProvider() { + return new Object[][] { + {ClientConfigProperties.PRODUCT_NAME.getKey(), "my product 1", true}, + {ClientConfigProperties.CLIENT_NAME.getKey(), "my product 2", true}, + {ClientConfigProperties.PRODUCT_NAME.getKey(), "my product 1", false}, + {ClientConfigProperties.CLIENT_NAME.getKey(), "my product 2", false}, + }; } @Test(groups = { "integration" }) @@ -928,64 +1025,6 @@ public void testJWTWithCloud() throws Exception { } } - - @Test(dataProvider = "testClientNameDataProvider") - public void testClientName(String clientName, boolean setWithUpdate, String userAgentHeader, boolean setForRequest) throws Exception { - - final String initialClientName = setWithUpdate ? "init clientName" : clientName; - final String initialUserAgentHeader = setForRequest ? "init userAgentHeader" : userAgentHeader; - final String clientReferer = "http://localhost/webpage"; - - Client.Builder builder = newClient(); - if (initialClientName != null) { - builder.setClientName(initialClientName); - } - if (initialUserAgentHeader != null) { - builder.httpHeader(HttpHeaders.USER_AGENT, initialUserAgentHeader); - } - try (Client client = builder.build()) { - String expectedClientNameStartsWith = initialClientName == null || initialUserAgentHeader != null ? initialUserAgentHeader : initialClientName; - - if (setWithUpdate) { - client.updateClientName(clientName); - expectedClientNameStartsWith = initialUserAgentHeader == null ? clientName : initialUserAgentHeader; - } - - String qId = UUID.randomUUID().toString(); - QuerySettings settings = new QuerySettings() - .httpHeader(HttpHeaders.REFERER, clientReferer) - .setQueryId(qId); - - if (setForRequest) { - settings.httpHeader(HttpHeaders.USER_AGENT, userAgentHeader); - expectedClientNameStartsWith = userAgentHeader; - } - - client.query("SELECT 1", settings).get().close(); - client.execute("SYSTEM FLUSH LOGS").get().close(); - - List logRecords = client.queryAll("SELECT query_id, client_name, http_user_agent, http_referer " + - " FROM system.query_log WHERE query_id = '" + settings.getQueryId() + "'"); - Assert.assertEquals(logRecords.get(0).getString("query_id"), settings.getQueryId()); - final String logUserAgent = logRecords.get(0).getString("http_user_agent"); - Assert.assertTrue(logUserAgent.startsWith(expectedClientNameStartsWith), - "Expected to start with \"" + expectedClientNameStartsWith + "\" but values was \"" + logUserAgent + "\"" ); - Assert.assertTrue(logUserAgent.contains(Client.CLIENT_USER_AGENT), "Expected to contain client v2 version but value was \"" + logUserAgent + "\""); - Assert.assertEquals(logRecords.get(0).getString("http_referer"), clientReferer); - Assert.assertEquals(logRecords.get(0).getString("client_name"), ""); // http client can't set this field - } - } - - @DataProvider(name = "testClientNameDataProvider") - public static Object[][] testClientName() { - return new Object[][] { - {"test-product (app 1.0)", false, null, false}, // only client name set - {"test-product (app 1.0)", false, "final product (app 1.1)", false}, // http header set and overrides client name - {"test-product (app 1.0)", true, null, false}, // client name set thru Client#updateClientName - {"test-product (app 1.0)", true, "final product (app 1.1)", true}, // custom UserAgent header overrides client name - }; - } - protected Client.Builder newClient() { ClickHouseNode node = getServer(ClickHouseProtocol.HTTP); boolean isSecure = isCloud(); diff --git a/client-v2/src/test/java/com/clickhouse/client/SettingsTests.java b/client-v2/src/test/java/com/clickhouse/client/SettingsTests.java index e99fdc480..befd6929f 100644 --- a/client-v2/src/test/java/com/clickhouse/client/SettingsTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/SettingsTests.java @@ -1,5 +1,6 @@ package com.clickhouse.client; +import com.clickhouse.client.api.Client; import com.clickhouse.client.api.ClientConfigProperties; import org.testng.Assert; import org.testng.annotations.Test; diff --git a/jdbc-v2/pom.xml b/jdbc-v2/pom.xml index 0ed68ca61..530c70128 100644 --- a/jdbc-v2/pom.xml +++ b/jdbc-v2/pom.xml @@ -12,7 +12,7 @@ jdbc-v2 jar - ClickHouse JDBC Driver + JDBC Driver V2 JDBC driver for ClickHouse https://github.com/ClickHouse/clickhouse-java/tree/main/jdbc-v2 diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java index 048d0c3c9..f76343679 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java @@ -63,13 +63,13 @@ public void testGetPropertyInfo() { @Test(groups = { "integration" }) public void testGetMajorVersion() { Driver driver = new Driver(); - Assert.assertEquals(driver.getMajorVersion(), 1); + Assert.assertEquals(driver.getMajorVersion(), 0); } @Test(groups = { "integration" }) public void testGetMinorVersion() { Driver driver = new Driver(); - Assert.assertEquals(driver.getMinorVersion(), 0); + Assert.assertEquals(driver.getMinorVersion(), 7); } @Test(groups = { "integration" }) From 32a009a73fdc2b36aace0c2491514f3d42c589a7 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 26 Dec 2024 19:14:01 -0800 Subject: [PATCH 4/7] minor javadoc fix --- client-v2/src/main/java/com/clickhouse/client/api/Client.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client-v2/src/main/java/com/clickhouse/client/api/Client.java b/client-v2/src/main/java/com/clickhouse/client/api/Client.java index e1645ca9b..1f55c53fd 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/Client.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/Client.java @@ -818,7 +818,7 @@ public Builder setSharedOperationExecutor(ExecutorService executorService) { /** * Set size of a buffers that are used to read/write data from the server. It is mainly used to copy data from * a socket to application memory and visa-versa. Setting is applied for both read and write operations. - * Default is 8192 bytes. + * Default is 300,000 bytes. * * @param size - size in bytes * @return From b5d52f4a264eb62e75858f8e290a4a88e25133e7 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 26 Dec 2024 20:03:01 -0800 Subject: [PATCH 5/7] fix cleaning connection --- .../com/clickhouse/jdbc/StatementImpl.java | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java index d7b3dd92b..916446a70 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java @@ -143,10 +143,29 @@ public ResultSet executeQuery(String sql) throws SQLException { return executeQuery(sql, new QuerySettings().setDatabase(schema)); } + private void closePreviousResultSet() { + if (currentResultSet != null) { + LOG.debug("Previous result set is open [resultSet = " + currentResultSet + "]"); + // Closing request blindly assuming that user do not care about it anymore (DDL request for example) + try { + currentResultSet.close(); + } catch (Exception e) { + LOG.error("Failed to close previous result set", e); + } finally { + currentResultSet = null; + } + } + } + public ResultSetImpl executeQuery(String sql, QuerySettings settings) throws SQLException { checkClosed(); + // Closing before trying to do next request. Otherwise, deadlock because previous connection will not be + // release before this one completes. + closePreviousResultSet(); + QuerySettings mergedSettings = QuerySettings.merge(connection.getDefaultQuerySettings(), settings); + if (mergedSettings.getQueryId() != null) { lastQueryId = mergedSettings.getQueryId(); } else { @@ -165,17 +184,6 @@ public ResultSetImpl executeQuery(String sql, QuerySettings settings) throws SQL } ClickHouseBinaryFormatReader reader = connection.client.newBinaryFormatReader(response); - if (currentResultSet != null) { - LOG.debug("Previous result set is open [resultSet = " + currentResultSet + "]"); - // Closing request blindly assuming that user do not care about it anymore (DDL request for example) - try { - currentResultSet.close(); - } catch (Exception e) { - LOG.error("Failed to close previous result set", e); - } finally { - currentResultSet = null; - } - } currentResultSet = new ResultSetImpl(this, response, reader); metrics = response.getMetrics(); } catch (Exception e) { @@ -199,6 +207,10 @@ public int executeUpdate(String sql, QuerySettings settings) throws SQLException throw new SQLException("executeUpdate() cannot be called with a SELECT/SHOW/DESCRIBE/EXPLAIN statement", ExceptionUtils.SQL_STATE_SQL_ERROR); } + // Closing before trying to do next request. Otherwise, deadlock because previous connection will not be + // release before this one completes. + closePreviousResultSet(); + QuerySettings mergedSettings = QuerySettings.merge(connection.getDefaultQuerySettings(), settings); if (mergedSettings.getQueryId() != null) { From b0a89340ce5d1ec1b827c5122301d9cf0e790b34 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Thu, 26 Dec 2024 21:42:56 -0800 Subject: [PATCH 6/7] implemented version file --- clickhouse-client/pom.xml | 6 ++++ .../client/config/ClickHouseClientOption.java | 31 ++++++++++++------- .../clickhouse-client-version.properties | 2 ++ .../client/ClickHouseConfigTest.java | 5 ++- .../client/http/ClickHouseHttpClientTest.java | 1 + .../clickhouse-jdbc-version.properties | 2 ++ client-v2/pom.xml | 6 ++++ .../com/clickhouse/client/api/Client.java | 24 +++++++++++++- .../api/internal/HttpAPIClientHelper.java | 3 +- .../resources/client-v2-version.properties | 2 ++ .../clickhouse/client/HttpTransportTests.java | 2 +- jdbc-v2/pom.xml | 6 ++++ .../main/java/com/clickhouse/jdbc/Driver.java | 22 ++++++++----- .../com/clickhouse/jdbc/StatementImpl.java | 2 +- .../main/resources/jdbc-v2-version.properties | 2 ++ .../java/com/clickhouse/jdbc/DriverTest.java | 9 +++--- pom.xml | 14 +++++++++ 17 files changed, 109 insertions(+), 30 deletions(-) create mode 100644 clickhouse-client/src/main/resources/clickhouse-client-version.properties create mode 100644 clickhouse-jdbc/src/main/resources/clickhouse-jdbc-version.properties create mode 100644 client-v2/src/main/resources/client-v2-version.properties create mode 100644 jdbc-v2/src/main/resources/jdbc-v2-version.properties diff --git a/clickhouse-client/pom.xml b/clickhouse-client/pom.xml index d2bf525e3..71d5f5bc4 100644 --- a/clickhouse-client/pom.xml +++ b/clickhouse-client/pom.xml @@ -69,6 +69,12 @@ + + + src/main/resources + true + + org.apache.maven.plugins diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/config/ClickHouseClientOption.java b/clickhouse-client/src/main/java/com/clickhouse/client/config/ClickHouseClientOption.java index 3c8fbb3bd..b87ecdd72 100644 --- a/clickhouse-client/src/main/java/com/clickhouse/client/config/ClickHouseClientOption.java +++ b/clickhouse-client/src/main/java/com/clickhouse/client/config/ClickHouseClientOption.java @@ -1,11 +1,13 @@ package com.clickhouse.client.config; +import java.io.InputStream; import java.io.Serializable; import java.net.InetAddress; import java.net.UnknownHostException; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Properties; import com.clickhouse.config.ClickHouseOption; import com.clickhouse.data.ClickHouseChecker; @@ -492,18 +494,21 @@ public enum ClickHouseClientOption implements ClickHouseOption { options = Collections.unmodifiableMap(map); // (revision: ) - String ver = ClickHouseClientOption.class.getPackage().getImplementationVersion(); - String[] parts = ver == null || ver.isEmpty() ? null : ver.split("\\s"); - if (parts != null && parts.length == 4 && parts[1].length() > 0 && parts[3].length() > 1 - && ver.charAt(ver.length() - 1) == ')') { - PRODUCT_VERSION = parts[1]; - ver = parts[3]; - PRODUCT_REVISION = ver.substring(0, ver.length() - 1); - } else { // perhaps try harder by checking version from pom.xml? - PRODUCT_VERSION = LATEST_KNOWN_VERSION; - PRODUCT_REVISION = UNKNOWN; + String tmpVersion = "unknown"; + try (InputStream in = Thread.currentThread().getContextClassLoader().getResourceAsStream("clickhouse-client-version.properties")) { + Properties p = new Properties(); + p.load(in); + + String tmp = p.getProperty("version"); + if (tmp != null && !tmp.isEmpty() && !tmp.equals("${revision}")) { + tmpVersion = tmp; + } + } catch (Exception e) { + // ignore } + PRODUCT_VERSION = tmpVersion; + PRODUCT_REVISION = UNKNOWN; CLIENT_OS_INFO = new StringBuilder().append(getSystemConfig("os.name", "O/S")).append('/') .append(getSystemConfig("os.version", UNKNOWN)).toString(); @@ -514,12 +519,14 @@ public enum ClickHouseClientOption implements ClickHouseOption { CLIENT_JVM_INFO = new StringBuilder().append(getSystemConfig("java.vm.name", "Java")).append('/') .append(javaVersion).toString(); CLIENT_USER = getSystemConfig("user.name", UNKNOWN); + + String host = null; try { - ver = InetAddress.getLocalHost().getHostName(); + host = InetAddress.getLocalHost().getHostName(); } catch (UnknownHostException e1) { // ignore } - CLIENT_HOST = ver == null || ver.isEmpty() ? UNKNOWN : ver; + CLIENT_HOST = host == null || host.isEmpty() ? UNKNOWN : host; } /** diff --git a/clickhouse-client/src/main/resources/clickhouse-client-version.properties b/clickhouse-client/src/main/resources/clickhouse-client-version.properties new file mode 100644 index 000000000..a54c8f97d --- /dev/null +++ b/clickhouse-client/src/main/resources/clickhouse-client-version.properties @@ -0,0 +1,2 @@ +version=${revision} +build.date=${build_timestamp} \ No newline at end of file diff --git a/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseConfigTest.java b/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseConfigTest.java index 1fa2bf3af..9d7b5c98b 100644 --- a/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseConfigTest.java +++ b/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseConfigTest.java @@ -12,6 +12,8 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; public class ClickHouseConfigTest { @Test(groups = { "unit" }) @@ -74,7 +76,8 @@ public void testCustomValues() { @Test(groups = { "unit" }) public void testClientInfo() throws UnknownHostException { ClickHouseConfig config = new ClickHouseConfig(); - Assert.assertEquals(config.getProductVersion(), ClickHouseClientOption.LATEST_KNOWN_VERSION); + Matcher versioMatcher = Pattern.compile("(^|\\\\.[\\\\d]+)+.*").matcher(config.getProductVersion()); + Assert.assertTrue(versioMatcher.matches()); Assert.assertEquals(config.getProductRevision(), "unknown"); Assert.assertEquals(config.getClientOsInfo(), System.getProperty("os.name") + "/" + System.getProperty("os.version")); diff --git a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpClientTest.java b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpClientTest.java index c51a97409..dbc305705 100644 --- a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpClientTest.java +++ b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpClientTest.java @@ -174,6 +174,7 @@ private void testUserAgent(ClickHouseOption option, String optionValue) throws E .executeAndWait()) { if (response.records().iterator().hasNext()) { String result = response.firstRecord().getValue(0).asString(); + System.out.println(result); Assert.assertTrue(result.startsWith(optionValue + " ClickHouse-JavaClient/")); Assert.assertTrue(result.indexOf("Http") > 0); break; diff --git a/clickhouse-jdbc/src/main/resources/clickhouse-jdbc-version.properties b/clickhouse-jdbc/src/main/resources/clickhouse-jdbc-version.properties new file mode 100644 index 000000000..a54c8f97d --- /dev/null +++ b/clickhouse-jdbc/src/main/resources/clickhouse-jdbc-version.properties @@ -0,0 +1,2 @@ +version=${revision} +build.date=${build_timestamp} \ No newline at end of file diff --git a/client-v2/pom.xml b/client-v2/pom.xml index bf57b1f21..1a7fa5a52 100644 --- a/client-v2/pom.xml +++ b/client-v2/pom.xml @@ -138,6 +138,12 @@ + + + src/main/resources + true + + org.apache.maven.plugins diff --git a/client-v2/src/main/java/com/clickhouse/client/api/Client.java b/client-v2/src/main/java/com/clickhouse/client/api/Client.java index 1f55c53fd..7b3cacd29 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/Client.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/Client.java @@ -75,6 +75,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Properties; import java.util.Set; import java.util.StringJoiner; import java.util.TimeZone; @@ -2139,6 +2140,10 @@ public String getServerVersion() { return this.serverVersion; } + public String getClientVersion() { + return clientVersion; + } + /** * Sets list of DB roles that should be applied to each query. * @@ -2155,11 +2160,28 @@ public void updateClientName(String name) { this.configuration.put(ClientConfigProperties.CLIENT_NAME.getKey(), name); } - public static final String LATEST_ARTIFACT_VERSION = "~0.7.2"; + public static final String clientVersion = getPackageVersion(); public static final String CLIENT_USER_AGENT = "clickhouse-java-v2/"; private Collection unmodifiableDbRolesView = Collections.emptyList(); + private static String getPackageVersion() { + String version = "unknown"; + try (InputStream in = Thread.currentThread().getContextClassLoader().getResourceAsStream("client-v2-version.properties")) { + Properties p = new Properties(); + p.load(in); + + String tmp = p.getProperty("version"); + if (tmp != null && !tmp.isEmpty() && !tmp.equals("${revision}")) { + version = tmp; + } + } catch (Exception e) { + LOG.error("Failed to load version file", e); + } + + return version; + } + /** * Returns list of DB roles that should be applied to each query. * diff --git a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java index 69411ee35..9d2c310df 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java @@ -672,8 +672,9 @@ private String buildDefaultUserAgent() { String clientVersion = Client.class.getPackage().getImplementationVersion(); if (clientVersion == null) { - clientVersion = Client.LATEST_ARTIFACT_VERSION; + clientVersion = Client.clientVersion; } + userAgent.append(clientVersion); userAgent.append(" ("); diff --git a/client-v2/src/main/resources/client-v2-version.properties b/client-v2/src/main/resources/client-v2-version.properties new file mode 100644 index 000000000..a54c8f97d --- /dev/null +++ b/client-v2/src/main/resources/client-v2-version.properties @@ -0,0 +1,2 @@ +version=${revision} +build.date=${build_timestamp} \ No newline at end of file diff --git a/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java b/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java index dace656c5..322e3f16a 100644 --- a/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/HttpTransportTests.java @@ -819,7 +819,7 @@ public void testUserAgentHasCompleteProductName(String clientName, Pattern userA Assert.assertFalse(logRecords.isEmpty(), "No records found in query log"); for (GenericRecord record : logRecords) { - + System.out.println(record.getString("http_user_agent")); Assert.assertTrue(userAgentPattern.matcher(record.getString("http_user_agent")).matches(), record.getString("http_user_agent") + " doesn't match \"" + userAgentPattern.pattern() + "\""); diff --git a/jdbc-v2/pom.xml b/jdbc-v2/pom.xml index 530c70128..05cef34f9 100644 --- a/jdbc-v2/pom.xml +++ b/jdbc-v2/pom.xml @@ -96,6 +96,12 @@ + + + src/main/resources + true + + org.apache.maven.plugins diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java index 484589655..b76b17259 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java @@ -7,6 +7,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.InputStream; import java.sql.Connection; import java.sql.DriverManager; import java.sql.DriverPropertyInfo; @@ -56,18 +57,23 @@ public static String getFrameworksDetected() { public static final String DRIVER_CLIENT_NAME = "jdbc-v2/"; - private static final String LAST_KNOWN_VERSION = "~0.7.2"; - static { log.debug("Initializing ClickHouse JDBC driver V2"); - String tempDriverVersion = Driver.class.getPackage().getImplementationVersion(); - //If the version is not available, set it to 1.0 - if (tempDriverVersion == null || tempDriverVersion.isEmpty()) { - log.warn("ClickHouse JDBC driver version is not available"); - tempDriverVersion = LAST_KNOWN_VERSION; + + String tmpDriverVersion = "unknown"; + try (InputStream in = Thread.currentThread().getContextClassLoader().getResourceAsStream("jdbc-v2-version.properties")) { + Properties p = new Properties(); + p.load(in); + + String tmp = p.getProperty("version"); + if (tmp != null && !tmp.isEmpty() && !tmp.equals("${revision}")) { + tmpDriverVersion = tmp; + } + } catch (Exception e) { + log.error("Failed to load version file", e); } - driverVersion = tempDriverVersion; + driverVersion = tmpDriverVersion; log.info("ClickHouse JDBC driver version: {}", driverVersion); int tmpMajorVersion; diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java index 916446a70..0756fe847 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java @@ -162,7 +162,7 @@ public ResultSetImpl executeQuery(String sql, QuerySettings settings) throws SQL // Closing before trying to do next request. Otherwise, deadlock because previous connection will not be // release before this one completes. closePreviousResultSet(); - + QuerySettings mergedSettings = QuerySettings.merge(connection.getDefaultQuerySettings(), settings); diff --git a/jdbc-v2/src/main/resources/jdbc-v2-version.properties b/jdbc-v2/src/main/resources/jdbc-v2-version.properties new file mode 100644 index 000000000..a54c8f97d --- /dev/null +++ b/jdbc-v2/src/main/resources/jdbc-v2-version.properties @@ -0,0 +1,2 @@ +version=${revision} +build.date=${build_timestamp} \ No newline at end of file diff --git a/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java b/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java index f76343679..c5bad1a18 100644 --- a/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java +++ b/jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java @@ -1,14 +1,13 @@ package com.clickhouse.jdbc; -import java.sql.DriverManager; -import java.sql.SQLException; -import java.util.Properties; - import com.clickhouse.client.api.ClientConfigProperties; -import org.testcontainers.shaded.com.fasterxml.jackson.databind.annotation.JsonAppend; import org.testng.Assert; import org.testng.annotations.Test; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.util.Properties; + public class DriverTest extends JdbcIntegrationTest { @Test(groups = { "integration" }) diff --git a/pom.xml b/pom.xml index dedc39527..bffd4fcf4 100644 --- a/pom.xml +++ b/pom.xml @@ -143,6 +143,7 @@ 3.2.5 3.1.0 2.16.0 + 3.3.1 1.37 1.8 @@ -156,6 +157,9 @@ https://sonarcloud.io 0.6.2 **/*0*.java,**/data/*Value.java,**/data/array/*Value.java,**/stream/*Stream.java + + ${maven.build.timestamp} + MM-dd-yyyy HH:mm @@ -560,6 +564,11 @@ true + + org.apache.maven.plugins + maven-resources-plugin + ${resource-plugin.version} + @@ -641,6 +650,11 @@ org.apache.maven.plugins maven-failsafe-plugin + + + org.apache.maven.plugins + maven-resources-plugin + From 262251cbb5fbb5aad5ad6974f958ebc0df8a68c5 Mon Sep 17 00:00:00 2001 From: Sergey Chernov Date: Fri, 27 Dec 2024 10:38:12 -0800 Subject: [PATCH 7/7] removed duplication and stdout --- .../client/config/ClickHouseClientOption.java | 33 ++++++++++--------- .../client/http/ClickHouseHttpClientTest.java | 1 - .../com/clickhouse/client/api/Client.java | 20 ++--------- .../api/internal/HttpAPIClientHelper.java | 6 ++-- .../main/java/com/clickhouse/jdbc/Driver.java | 16 ++------- 5 files changed, 24 insertions(+), 52 deletions(-) diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/config/ClickHouseClientOption.java b/clickhouse-client/src/main/java/com/clickhouse/client/config/ClickHouseClientOption.java index b87ecdd72..96540ba61 100644 --- a/clickhouse-client/src/main/java/com/clickhouse/client/config/ClickHouseClientOption.java +++ b/clickhouse-client/src/main/java/com/clickhouse/client/config/ClickHouseClientOption.java @@ -493,21 +493,7 @@ public enum ClickHouseClientOption implements ClickHouseOption { } options = Collections.unmodifiableMap(map); - // (revision: ) - String tmpVersion = "unknown"; - try (InputStream in = Thread.currentThread().getContextClassLoader().getResourceAsStream("clickhouse-client-version.properties")) { - Properties p = new Properties(); - p.load(in); - - String tmp = p.getProperty("version"); - if (tmp != null && !tmp.isEmpty() && !tmp.equals("${revision}")) { - tmpVersion = tmp; - } - } catch (Exception e) { - // ignore - } - - PRODUCT_VERSION = tmpVersion; + PRODUCT_VERSION = readVersionFromResource("clickhouse-client-version.properties"); PRODUCT_REVISION = UNKNOWN; CLIENT_OS_INFO = new StringBuilder().append(getSystemConfig("os.name", "O/S")).append('/') @@ -529,6 +515,23 @@ public enum ClickHouseClientOption implements ClickHouseOption { CLIENT_HOST = host == null || host.isEmpty() ? UNKNOWN : host; } + public static String readVersionFromResource(String resourceFilePath) { + // TODO: move to client-v2 when client-v1 is deprecated completely + String tmpVersion = "unknown"; + try (InputStream in = Thread.currentThread().getContextClassLoader().getResourceAsStream(resourceFilePath)) { + Properties p = new Properties(); + p.load(in); + + String tmp = p.getProperty("version"); + if (tmp != null && !tmp.isEmpty() && !tmp.equals("${revision}")) { + tmpVersion = tmp; + } + } catch (Exception e) { + // ignore + } + return tmpVersion; + } + /** * Builds user-agent based on given product name. The user-agent will be * something look like diff --git a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpClientTest.java b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpClientTest.java index dbc305705..c51a97409 100644 --- a/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpClientTest.java +++ b/clickhouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpClientTest.java @@ -174,7 +174,6 @@ private void testUserAgent(ClickHouseOption option, String optionValue) throws E .executeAndWait()) { if (response.records().iterator().hasNext()) { String result = response.firstRecord().getValue(0).asString(); - System.out.println(result); Assert.assertTrue(result.startsWith(optionValue + " ClickHouse-JavaClient/")); Assert.assertTrue(result.indexOf("Http") > 0); break; diff --git a/client-v2/src/main/java/com/clickhouse/client/api/Client.java b/client-v2/src/main/java/com/clickhouse/client/api/Client.java index 7b3cacd29..45bc90a30 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/Client.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/Client.java @@ -2160,28 +2160,12 @@ public void updateClientName(String name) { this.configuration.put(ClientConfigProperties.CLIENT_NAME.getKey(), name); } - public static final String clientVersion = getPackageVersion(); + public static final String clientVersion = + ClickHouseClientOption.readVersionFromResource("client-v2-version.properties"); public static final String CLIENT_USER_AGENT = "clickhouse-java-v2/"; private Collection unmodifiableDbRolesView = Collections.emptyList(); - private static String getPackageVersion() { - String version = "unknown"; - try (InputStream in = Thread.currentThread().getContextClassLoader().getResourceAsStream("client-v2-version.properties")) { - Properties p = new Properties(); - p.load(in); - - String tmp = p.getProperty("version"); - if (tmp != null && !tmp.isEmpty() && !tmp.equals("${revision}")) { - version = tmp; - } - } catch (Exception e) { - LOG.error("Failed to load version file", e); - } - - return version; - } - /** * Returns list of DB roles that should be applied to each query. * diff --git a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java index 9d2c310df..efe53b63b 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java @@ -646,6 +646,7 @@ public static Map parseUrlParameters(URL url) { private void correctUserAgentHeader(HttpRequest request, Map requestConfig) { + //TODO: implement cache for user-agent Header userAgentHeader = request.getLastHeader(HttpHeaders.USER_AGENT); request.removeHeaders(HttpHeaders.USER_AGENT); @@ -670,10 +671,7 @@ private String buildDefaultUserAgent() { StringBuilder userAgent = new StringBuilder(); userAgent.append(Client.CLIENT_USER_AGENT); - String clientVersion = Client.class.getPackage().getImplementationVersion(); - if (clientVersion == null) { - clientVersion = Client.clientVersion; - } + String clientVersion = Client.clientVersion; userAgent.append(clientVersion); diff --git a/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java b/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java index b76b17259..6ba7319ef 100644 --- a/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java +++ b/jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java @@ -2,6 +2,7 @@ import com.clickhouse.client.api.ClientConfigProperties; +import com.clickhouse.client.config.ClickHouseClientOption; import com.clickhouse.jdbc.internal.JdbcConfiguration; import com.clickhouse.jdbc.internal.ExceptionUtils; import org.slf4j.Logger; @@ -60,20 +61,7 @@ public static String getFrameworksDetected() { static { log.debug("Initializing ClickHouse JDBC driver V2"); - String tmpDriverVersion = "unknown"; - try (InputStream in = Thread.currentThread().getContextClassLoader().getResourceAsStream("jdbc-v2-version.properties")) { - Properties p = new Properties(); - p.load(in); - - String tmp = p.getProperty("version"); - if (tmp != null && !tmp.isEmpty() && !tmp.equals("${revision}")) { - tmpDriverVersion = tmp; - } - } catch (Exception e) { - log.error("Failed to load version file", e); - } - - driverVersion = tmpDriverVersion; + driverVersion = ClickHouseClientOption.readVersionFromResource("jdbc-v2-version.properties"); log.info("ClickHouse JDBC driver version: {}", driverVersion); int tmpMajorVersion;