Merge pull request #1739 from ClickHouse/user-agent-string

Adjusting how user agent is calculated
ClickHouse · Jul 19, 2024 · e2cae1c · e2cae1c
2 parents 75b7922 + 7751f51
commit e2cae1c
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 17 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
@@ -436,6 +436,7 @@ public enum ClickHouseClientOption implements ClickHouseOption {
     private static final Map<String, ClickHouseClientOption> options;
 
     static final String UNKNOWN = "unknown";
+    public static final String LATEST_KNOWN_VERSION = "0.6.3";
 
     /**
      * Semantic version of the product.
@@ -480,9 +481,11 @@ public enum ClickHouseClientOption implements ClickHouseOption {
             ver = parts[3];
             PRODUCT_REVISION = ver.substring(0, ver.length() - 1);
         } else { // perhaps try harder by checking version from pom.xml?
-            PRODUCT_VERSION = UNKNOWN;
+            PRODUCT_VERSION = LATEST_KNOWN_VERSION;
             PRODUCT_REVISION = UNKNOWN;
         }
+
+
         CLIENT_OS_INFO = new StringBuilder().append(getSystemConfig("os.name", "O/S")).append('/')
                 .append(getSystemConfig("os.version", UNKNOWN)).toString();
         String javaVersion = System.getProperty("java.vendor.version");
@@ -510,15 +513,18 @@ public enum ClickHouseClientOption implements ClickHouseOption {
      * @param additionalProperty additional property if any
      * @return non-empty user-agent
      */
-    public static final String buildUserAgent(String productName, String additionalProperty) {
-        productName = productName == null || productName.isEmpty() ? (String) PRODUCT_NAME.getEffectiveDefaultValue()
-                : productName.trim();
-        StringBuilder builder = new StringBuilder(productName).append('/').append(PRODUCT_VERSION).append(" (")
-                .append(CLIENT_OS_INFO).append("; ").append(CLIENT_JVM_INFO);
+    public static String buildUserAgent(String productName, String additionalProperty) {
+        productName = productName == null || productName.isEmpty() ? (String) PRODUCT_NAME.getEffectiveDefaultValue() : productName.trim();
+        StringBuilder builder = new StringBuilder(productName).append(PRODUCT_VERSION.isEmpty() ? "" : "/" + PRODUCT_VERSION);
+
+        if (!String.valueOf(PRODUCT_NAME.getDefaultValue()).equals(productName)) {//Append if someone changed the original value
+            builder.append(" ").append(PRODUCT_NAME.getDefaultValue()).append(LATEST_KNOWN_VERSION);
+        }
+        builder.append(" (").append(CLIENT_JVM_INFO);
         if (additionalProperty != null && !additionalProperty.isEmpty()) {
             builder.append("; ").append(additionalProperty.trim());
         }
-        return builder.append("; rv:").append(PRODUCT_REVISION).append(')').toString();
+        return builder.append(")").toString();
     }
 
     /**

diff --git a/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseConfigTest.java b/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseConfigTest.java
@@ -74,7 +74,7 @@ public void testCustomValues() {
     @Test(groups = { "unit" })
     public void testClientInfo() throws UnknownHostException {
         ClickHouseConfig config = new ClickHouseConfig();
-        Assert.assertEquals(config.getProductVersion(), "unknown");
+        Assert.assertEquals(config.getProductVersion(), ClickHouseClientOption.LATEST_KNOWN_VERSION);
         Assert.assertEquals(config.getProductRevision(), "unknown");
         Assert.assertEquals(config.getClientOsInfo(),
                 System.getProperty("os.name") + "/" + System.getProperty("os.version"));
@@ -85,11 +85,10 @@ public void testClientInfo() throws UnknownHostException {
         Assert.assertEquals(config.getClientHost(), InetAddress.getLocalHost().getHostName());
 
         Assert.assertEquals(ClickHouseClientOption.buildUserAgent(null, null),
-                "ClickHouse-JavaClient/unknown (" + System.getProperty("os.name") + "/"
-                        + System.getProperty("os.version") + "; " + System.getProperty("java.vm.name") + "/"
+                "ClickHouse-JavaClient/"+ ClickHouseClientOption.PRODUCT_VERSION + " (" + System.getProperty("java.vm.name") + "/"
                         + System.getProperty("java.vendor.version",
                                 System.getProperty("java.vm.version", System.getProperty("java.version", "unknown")))
-                        + "; rv:unknown)");
+                        + ")");
         Assert.assertEquals(ClickHouseClientOption.buildUserAgent(null, null),
                 ClickHouseClientOption.buildUserAgent("", null));
 

diff --git a/...khouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java b/...khouse-http-client/src/main/java/com/clickhouse/client/http/ClickHouseHttpConnection.java
@@ -420,14 +420,18 @@ protected String getDefaultUserAgent() {
     protected final String getUserAgent() {
         final ClickHouseConfig c = config;
         String name = c.getClientName();
+        String userAgent = getDefaultUserAgent();
+
         if (!ClickHouseClientOption.CLIENT_NAME.getDefaultValue().equals(name)) {
-            return name;
+            return name + " " + userAgent;
         }
 
-        String userAgent = getDefaultUserAgent();
         name = c.getProductName();
-        return ClickHouseClientOption.PRODUCT_NAME.getDefaultValue().equals(name) ? userAgent
-                : new StringBuilder(name).append(userAgent.substring(userAgent.indexOf('/'))).toString();
+        String version = c.getProductVersion();
+        if (!ClickHouseClientOption.PRODUCT_VERSION.equals(version)) {
+            name = name + "/" + c.getProductVersion();
+        }
+        return ClickHouseClientOption.PRODUCT_NAME.getDefaultValue().equals(name) ? userAgent : name + " " + userAgent;
     }
 
     /**

diff --git a/...khouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpClientTest.java b/...khouse-http-client/src/test/java/com/clickhouse/client/http/ClickHouseHttpClientTest.java
@@ -175,7 +175,8 @@ public void testUserAgent() throws Exception {
                      .query("select http_user_agent from system.query_log where query='select ''" + uuid + "'''")
                      .executeAndWait()) {
             String result = response.firstRecord().getValue(0).asString();
-            Assert.assertTrue(result.startsWith("MyCustomClient"));
+            System.out.println(result);
+            Assert.assertTrue(result.startsWith("MyCustomClient ClickHouse-JavaClient/"));
             Assert.assertTrue(result.indexOf("Http") > 0);
         }
 
@@ -193,7 +194,8 @@ public void testUserAgent() throws Exception {
              ClickHouseResponse response = newRequest(client, server)
                      .query("select http_user_agent from system.query_log where query='select ''" + uuid + "'''")
                      .executeAndWait()) {
-            Assert.assertEquals(response.firstRecord().getValue(0).asString(), "MyCustomClient");
+            String result = response.firstRecord().getValue(0).asString();
+            Assert.assertTrue(result.startsWith("MyCustomClient ClickHouse-JavaClient/"));
         }
     }