Skip to content

Commit 24925bb

Browse files
chernserchernser
committed
fixed Driver issues with USE_TIME_ZONE. Added more test
1 parent 7f17786 commit 24925bb

File tree

7 files changed

+81
-25
lines changed

7 files changed

+81
-25
lines changed

‎clickhouse-jdbc/src/test/java/com/clickhouse/jdbc/ClickHouseDriverTest.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,13 @@
11
package com.clickhouse.jdbc;
22

3-
import java.sql.Connection;
4-
import java.sql.SQLException;
5-
63
import com.clickhouse.client.ClickHouseProtocol;
7-
84
import com.clickhouse.client.ClickHouseServerForTest;
9-
import com.clickhouse.client.config.ClickHouseDefaults;
10-
import com.clickhouse.jdbc.internal.ClickHouseJdbcUrlParser;
115
import org.testng.Assert;
126
import org.testng.annotations.Test;
137

8+
import java.sql.Connection;
9+
import java.sql.SQLException;
10+
1411
public class ClickHouseDriverTest extends JdbcIntegrationTest {
1512
@Test(groups = "integration")
1613
public void testAcceptUrl() throws SQLException {
@@ -24,13 +21,21 @@ public void testAcceptUrl() throws SQLException {
2421

2522
@Test(groups = "integration")
2623
public void testConnect() throws SQLException {
27-
if (isCloud()) return; //TODO: testConnect - Revisit, see: https://github.com/ClickHouse/clickhouse-java/issues/1747
24+
if (isCloud()) {
25+
return; //
26+
}
27+
2828
System.setProperty("clickhouse.jdbc.v1","true");
2929
String address = getServerAddress(ClickHouseProtocol.HTTP, true);
3030
ClickHouseDriver driver = new ClickHouseDriver();
3131
Connection conn = driver.connect("jdbc:clickhouse://default:" + ClickHouseServerForTest.getPassword() + "@" + address, null);
3232
conn.close();
33+
System.setProperty("clickhouse.jdbc.v1","false");
34+
ClickHouseDriver driver2 = new ClickHouseDriver();
35+
Connection conn2 = driver2.connect("jdbc:clickhouse://default:" + ClickHouseServerForTest.getPassword() + "@" + address, null);
36+
conn2.close();
3337
}
38+
3439
@Test(groups = "integration")
3540
public void testV2Driver() {
3641
System.setProperty("clickhouse.jdbc.v1","false");

‎client-v2/src/main/java/com/clickhouse/client/api/Client.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -766,6 +766,8 @@ public Builder useServerTimeZone(boolean useServerTimeZone) {
766766
*/
767767
public Builder useTimeZone(String timeZone) {
768768
this.configuration.put(ClientConfigProperties.USE_TIMEZONE.getKey(), timeZone);
769+
// switch using server timezone to false
770+
this.configuration.put(ClientConfigProperties.USE_SERVER_TIMEZONE.getKey(), String.valueOf(Boolean.FALSE));
769771
return this;
770772
}
771773

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/ConnectionImpl.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
import java.sql.Struct;
3737
import java.time.Duration;
3838
import java.time.temporal.ChronoUnit;
39-
import java.util.Arrays;
4039
import java.util.Calendar;
4140
import java.util.Collections;
4241
import java.util.HashSet;

‎jdbc-v2/src/main/java/com/clickhouse/jdbc/Driver.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,8 +164,8 @@ public static int[] parseVersion(String version) {
164164

165165
Matcher matcher = pattern.matcher(version);
166166
if (matcher.find()) {
167-
short major = Short.parseShort(matcher.group(1));
168-
short minor = Short.parseShort(matcher.group(2));
167+
int major = Integer.parseInt(matcher.group(1));
168+
int minor = Integer.parseInt(matcher.group(2));
169169
int patch = Integer.parseInt(matcher.group(3));
170170
int majorVersion = (major << 16) | minor;
171171
return new int[]{majorVersion, patch};

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
import com.clickhouse.jdbc.Driver;
88
import com.clickhouse.jdbc.DriverProperties;
99
import com.google.common.collect.ImmutableMap;
10+
import org.slf4j.Logger;
11+
import org.slf4j.LoggerFactory;
1012

1113
import java.io.UnsupportedEncodingException;
1214
import java.net.URI;
@@ -24,6 +26,7 @@
2426
import java.util.stream.Collectors;
2527

2628
public class JdbcConfiguration {
29+
public static final Logger LOG = LoggerFactory.getLogger(JdbcConfiguration.class);
2730

2831
private static final String PREFIX_CLICKHOUSE = "jdbc:clickhouse:";
2932
private static final String PREFIX_CLICKHOUSE_SHORT = "jdbc:ch:";
@@ -61,19 +64,20 @@ public boolean isIgnoreUnsupportedRequests() {
6164
* @param info - Driver and Client properties.
6265
*/
6366
public JdbcConfiguration(String url, Properties info) throws SQLException {
64-
this.disableFrameworkDetection = info != null && Boolean.parseBoolean(info.getProperty("disable_frameworks_detection", "false"));
67+
final Properties props = info == null ? new Properties() : info;
68+
this.disableFrameworkDetection = Boolean.parseBoolean(props.getProperty("disable_frameworks_detection", "false"));
6569
this.clientProperties = new HashMap<>();
6670
this.driverProperties = new HashMap<>();
6771

6872
Map<String, String> urlProperties = parseUrl(url);
6973
String tmpConnectionUrl = urlProperties.remove(PARSE_URL_CONN_URL_PROP);
70-
initProperties(urlProperties, info);
74+
initProperties(urlProperties, props);
7175

7276
// after initializing all properties - set final connection URL
73-
boolean useSSLInfo = Boolean.parseBoolean(info.getProperty(DriverProperties.SECURE_CONNECTION.getKey(), "false"));
77+
boolean useSSLInfo = Boolean.parseBoolean(props.getProperty(DriverProperties.SECURE_CONNECTION.getKey(), "false"));
7478
boolean useSSLUrlProperties = Boolean.parseBoolean(urlProperties.getOrDefault(DriverProperties.SECURE_CONNECTION.getKey(), "false"));
7579
boolean useSSL = useSSLInfo || useSSLUrlProperties;
76-
String bearerToken = info.getProperty(ClientConfigProperties.BEARERTOKEN_AUTH.getKey(), null);
80+
String bearerToken = props.getProperty(ClientConfigProperties.BEARERTOKEN_AUTH.getKey(), null);
7781
if (bearerToken != null) {
7882
clientProperties.put(ClientConfigProperties.BEARERTOKEN_AUTH.getKey(), bearerToken);
7983
}
@@ -233,6 +237,12 @@ private void initProperties(Map<String, String> urlProperties, Properties provid
233237

234238
props.putAll(urlProperties);
235239

240+
if (props.containsKey(ClientConfigProperties.USE_TIMEZONE.getKey())) {
241+
if (!props.containsKey(ClientConfigProperties.USE_SERVER_TIMEZONE.getKey())) {
242+
props.put(ClientConfigProperties.USE_SERVER_TIMEZONE.getKey(), String.valueOf(Boolean.FALSE));
243+
}
244+
}
245+
236246
// Process all properties
237247
Map<String, DriverPropertyInfo> propertyInfos = new HashMap<>();
238248

‎jdbc-v2/src/test/java/com/clickhouse/jdbc/ConnectionTest.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import java.util.Arrays;
4040
import java.util.Base64;
4141
import java.util.Collections;
42-
import java.util.List;
4342
import java.util.Properties;
4443
import java.util.UUID;
4544
import java.util.concurrent.ExecutorService;
@@ -962,4 +961,14 @@ public void testClientInfoProperties() throws Exception {
962961
assertNull(conn.getClientInfo("unknown"));
963962
}
964963
}
964+
965+
@Test(groups = {"integration"})
966+
public void testUseUserTimeZone() throws Exception {
967+
Properties props = new Properties();
968+
props.put(ClientConfigProperties.USE_TIMEZONE.getKey(), "America/New_York");
969+
try (Connection conn = getJdbcConnection(props)) {
970+
//
971+
}
972+
973+
}
965974
}

‎jdbc-v2/src/test/java/com/clickhouse/jdbc/DriverTest.java‎

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
import org.testng.annotations.Test;
77

88
import java.sql.DriverManager;
9+
import java.sql.DriverPropertyInfo;
910
import java.sql.SQLException;
11+
import java.util.HashMap;
12+
import java.util.Map;
1013
import java.util.Properties;
11-
import java.util.regex.Matcher;
12-
import java.util.regex.Pattern;
1314

1415
import static org.testng.AssertJUnit.assertTrue;
15-
import static org.testng.internal.junit.ArrayAsserts.assertArrayEquals;
1616

1717

1818
public class DriverTest extends JdbcIntegrationTest {
@@ -52,20 +52,51 @@ public void testAcceptsURL() {
5252
}
5353
}
5454

55-
@Test(groups = {"integration"})
56-
public void testGetPropertyInfo() {
55+
@Test(groups = {"integration"}, dataProvider = "testGetPropertyInfoDP")
56+
public void testGetPropertyInfo(String url, Properties props, Map<String, String> checkProperties) {
57+
final Map<String, String> checkPropertiesCopy = new HashMap<>(checkProperties);
5758
try {
5859
Driver driver = new Driver();
59-
driver.getPropertyInfo(getEndpointString(), new Properties());
60-
Assert.assertEquals(driver.getPropertyInfo(getEndpointString(), new Properties()).length, 7);
61-
Properties sample = new Properties();
62-
sample.setProperty("testing", "true");
63-
Assert.assertEquals(driver.getPropertyInfo(getEndpointString(), sample).length, 7);
60+
DriverPropertyInfo[] properties = driver.getPropertyInfo(url, props);
61+
62+
for (DriverPropertyInfo property : properties) {
63+
Object expectedValue = checkPropertiesCopy.remove(property.name);
64+
if (expectedValue != null) {
65+
Assert.assertEquals(property.value, expectedValue);
66+
} else {
67+
for (DriverProperties driverProp : DriverProperties.values()) {
68+
if (driverProp.getKey().equalsIgnoreCase(property.name)) {
69+
Assert.assertEquals(property.value, driverProp.getDefaultValue());
70+
}
71+
}
72+
for (ClientConfigProperties clientProp : ClientConfigProperties.values()) {
73+
if (clientProp.getKey().equalsIgnoreCase(property.name)) {
74+
Assert.assertEquals(property.value, clientProp.getDefaultValue());
75+
}
76+
}
77+
}
78+
}
79+
80+
Assert.assertTrue(checkPropertiesCopy.isEmpty(), "Not checked properties: " + checkProperties);
6481
} catch (SQLException e) {
6582
Assert.fail("Failed to get property info", e);
6683
}
6784
}
6885

86+
@DataProvider(name = "testGetPropertyInfoDP")
87+
public Object[][] testGetPropertyInfoDP() {
88+
return new Object[][]{
89+
{"jdbc:ch://localhost:8123/?async=true", null, Map.of(ClientConfigProperties.ASYNC_OPERATIONS.getKey(), "true")},
90+
{"jdbc:ch://localhost:8123/?connection_ttl=10000&max_threads_per_client=100", null, Map.of(ClientConfigProperties.CONNECTION_TTL.getKey(), "10000",
91+
ClientConfigProperties.MAX_THREADS_PER_CLIENT.getKey(), "100")},
92+
{"jdbc:ch://localhost:8123/?client_retry_on_failures=NoHttpResponse,SocketTimeout", null, Map.of(ClientConfigProperties.CLIENT_RETRY_ON_FAILURE.getKey(), "NoHttpResponse,SocketTimeout")},
93+
{"jdbc:ch://localhost:8123/?connection_ttl=10000&client_retry_on_failures=NoHttpResponse,SocketTimeout&max_threads_per_client=100",
94+
null, Map.of(ClientConfigProperties.CLIENT_RETRY_ON_FAILURE.getKey(), "NoHttpResponse,SocketTimeout",
95+
ClientConfigProperties.CONNECTION_TTL.getKey(), "10000",
96+
ClientConfigProperties.MAX_THREADS_PER_CLIENT.getKey(), "100")}
97+
};
98+
}
99+
69100
@Test(groups = {"integration"})
70101
public void testGetDriverVersion() {
71102
Driver driver = new Driver();

0 commit comments

Comments
 (0)