From 9bc373c245ceb51db3506d2b7be4076d81904ecd Mon Sep 17 00:00:00 2001 From: kolzeq Date: Fri, 16 Apr 2021 14:46:46 +0200 Subject: [PATCH] misc - timezone handling When setting timezone, an initial verification check that this doesn't correspond to server, in order to avoid any unneeded timezone conversion. Implementation correction in order to avoid throwing error if server timezone is not recognized. --- .../org/mariadb/jdbc/client/ClientImpl.java | 30 ++-- src/test/java/org/mariadb/jdbc/Common.java | 7 +- .../jdbc/integration/LocalInfileTest.java | 6 +- .../jdbc/integration/MultiHostTest.java | 149 ++++++++++-------- .../integration/PreparedStatementTest.java | 3 +- 5 files changed, 109 insertions(+), 86 deletions(-) diff --git a/src/main/java/org/mariadb/jdbc/client/ClientImpl.java b/src/main/java/org/mariadb/jdbc/client/ClientImpl.java index 05496e3da..200635b06 100644 --- a/src/main/java/org/mariadb/jdbc/client/ClientImpl.java +++ b/src/main/java/org/mariadb/jdbc/client/ClientImpl.java @@ -17,6 +17,7 @@ import java.time.Instant; import java.time.ZoneId; import java.time.ZoneOffset; +import java.time.zone.ZoneRulesException; import java.util.*; import java.util.concurrent.Executor; import java.util.concurrent.locks.ReentrantLock; @@ -359,18 +360,25 @@ public String createSessionVariableQuery(String serverTz) { // force client timezone to connection to ensure result of now(), ... if (conf.timezone() != null && !"disable".equalsIgnoreCase(conf.timezone())) { - ZoneId serverZoneId = ZoneId.of(serverTz).normalized(); + boolean mustSetTimezone = true; ZoneId clientZoneId = ZoneId.of(conf.timezone()).normalized(); - if (!serverZoneId.equals(clientZoneId)) { - serverZoneId = ZoneId.of(serverTz, ZoneId.SHORT_IDS); - if (!serverZoneId.equals(clientZoneId)) { - // to ensure system not having saving time set, prefer fixed offset if possible - if (clientZoneId.getRules().isFixedOffset()) { - ZoneOffset zoneOffset = clientZoneId.getRules().getOffset(Instant.now()); - sb.append(",time_zone='").append(zoneOffset.getId()).append("'"); - } else { - sb.append(",time_zone='").append(conf.timezone()).append("'"); - } + + // try to avoid timezone consideration if server use the same one + try { + if (ZoneId.of(serverTz).normalized().equals(clientZoneId) + || ZoneId.of(serverTz, ZoneId.SHORT_IDS).equals(clientZoneId)) { + mustSetTimezone = false; + } + } catch (ZoneRulesException e) { + // eat + } + + if (mustSetTimezone) { + if (clientZoneId.getRules().isFixedOffset()) { + ZoneOffset zoneOffset = clientZoneId.getRules().getOffset(Instant.now()); + sb.append(",time_zone='").append(zoneOffset.getId()).append("'"); + } else { + sb.append(",time_zone='").append(conf.timezone()).append("'"); } } } diff --git a/src/test/java/org/mariadb/jdbc/Common.java b/src/test/java/org/mariadb/jdbc/Common.java index c7ae0b3c6..93706f629 100644 --- a/src/test/java/org/mariadb/jdbc/Common.java +++ b/src/test/java/org/mariadb/jdbc/Common.java @@ -54,12 +54,7 @@ public class Common { mDefUrl = String.format( "jdbc:mariadb://%s:%s/%s?user=%s&password=%s&restrictedAuth=false&%s", - hostname, - port, - get("DB_DATABASE", prop), - user, - password, - defaultOther); + hostname, port, get("DB_DATABASE", prop), user, password, defaultOther); } catch (IOException io) { io.printStackTrace(); diff --git a/src/test/java/org/mariadb/jdbc/integration/LocalInfileTest.java b/src/test/java/org/mariadb/jdbc/integration/LocalInfileTest.java index 7095a17fe..7b10877d5 100644 --- a/src/test/java/org/mariadb/jdbc/integration/LocalInfileTest.java +++ b/src/test/java/org/mariadb/jdbc/integration/LocalInfileTest.java @@ -73,11 +73,12 @@ public void wrongFile() throws Exception { try (Connection con = createCon("allowLocalInfile")) { Statement stmt = con.createStatement(); assertThrowsContains( - SQLTransientConnectionException.class, + SQLException.class, () -> stmt.execute( "LOAD DATA LOCAL INFILE 'someFile' INTO TABLE LocalInfileInputStreamTest2 (id, test)"), "Could not send file : someFile"); + assertTrue(con.isValid(1)); } } @@ -95,13 +96,14 @@ public void unReadableFile() throws Exception { tempFile.setReadable(false); Statement stmt = con.createStatement(); assertThrowsContains( - SQLTransientConnectionException.class, + SQLException.class, () -> stmt.execute( "LOAD DATA LOCAL INFILE '" + tempFile.getCanonicalPath().replace("\\", "/") + "' INTO TABLE LocalInfileInputStreamTest2 (id, test)"), "Could not send file"); + assertTrue(con.isValid(1)); } } diff --git a/src/test/java/org/mariadb/jdbc/integration/MultiHostTest.java b/src/test/java/org/mariadb/jdbc/integration/MultiHostTest.java index 979cb71c6..de292e211 100644 --- a/src/test/java/org/mariadb/jdbc/integration/MultiHostTest.java +++ b/src/test/java/org/mariadb/jdbc/integration/MultiHostTest.java @@ -8,7 +8,6 @@ import java.io.IOException; import java.sql.*; - import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Test; import org.mariadb.jdbc.*; @@ -106,7 +105,7 @@ public void replicaNotSet() throws Exception { @Test public void masterFailover() throws Exception { Assumptions.assumeTrue( - !"skysql".equals(System.getenv("srv")) && !"skysql-ha".equals(System.getenv("srv"))); + !"skysql".equals(System.getenv("srv")) && !"skysql-ha".equals(System.getenv("srv"))); Configuration conf = Configuration.parse(mDefUrl); HostAddress hostAddress = conf.addresses().get(0); @@ -117,21 +116,21 @@ public void masterFailover() throws Exception { } String url = - mDefUrl.replaceAll( - "//([^/]*)/", - String.format( - "//address=(host=localhost)(port=9999)(type=master),address=(host=localhost)(port=%s)(type=master),address=(host=%s)(port=%s)(type=master)/", - proxy.getLocalPort(), hostAddress.host, hostAddress.port)); + mDefUrl.replaceAll( + "//([^/]*)/", + String.format( + "//address=(host=localhost)(port=9999)(type=master),address=(host=localhost)(port=%s)(type=master),address=(host=%s)(port=%s)(type=master)/", + proxy.getLocalPort(), hostAddress.host, hostAddress.port)); url = url.replaceAll("jdbc:mariadb:", "jdbc:mariadb:sequential:"); if (conf.sslMode() == SslMode.VERIFY_FULL) { url = url.replaceAll("sslMode=verify-full", "sslMode=verify-ca"); } try (Connection con = - (Connection) - DriverManager.getConnection( - url - + "waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=4&connectTimeout=500")) { + (Connection) + DriverManager.getConnection( + url + + "waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=4&connectTimeout=500")) { Statement stmt = con.createStatement(); stmt.execute("SET @con=1"); proxy.restart(50); @@ -144,10 +143,10 @@ public void masterFailover() throws Exception { // with transaction replay try (Connection con = - (Connection) - DriverManager.getConnection( - url - + "transactionReplay=true&waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=4&connectTimeout=500")) { + (Connection) + DriverManager.getConnection( + url + + "transactionReplay=true&waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=4&connectTimeout=500")) { Statement stmt = con.createStatement(); stmt.execute("DROP TABLE IF EXISTS testReplay"); stmt.execute("CREATE TABLE testReplay(id INT)"); @@ -186,8 +185,9 @@ public void masterFailover() throws Exception { @Test public void masterStreamingFailover() throws Exception { Assumptions.assumeTrue( - isMariaDBServer() && - !"skysql".equals(System.getenv("srv")) && !"skysql-ha".equals(System.getenv("srv"))); + isMariaDBServer() + && !"skysql".equals(System.getenv("srv")) + && !"skysql-ha".equals(System.getenv("srv"))); Configuration conf = Configuration.parse(mDefUrl); HostAddress hostAddress = conf.addresses().get(0); @@ -198,47 +198,56 @@ public void masterStreamingFailover() throws Exception { } String url = - mDefUrl.replaceAll( - "//([^/]*)/", - String.format( - "//address=(host=localhost)(port=%s)(type=master)/", - proxy.getLocalPort(), hostAddress.host, hostAddress.port)); + mDefUrl.replaceAll( + "//([^/]*)/", + String.format( + "//address=(host=localhost)(port=%s)(type=master)/", + proxy.getLocalPort(), hostAddress.host, hostAddress.port)); url = url.replaceAll("jdbc:mariadb:", "jdbc:mariadb:sequential:"); if (conf.sslMode() == SslMode.VERIFY_FULL) { url = url.replaceAll("sslMode=verify-full", "sslMode=verify-ca"); } Connection con = - (Connection) - DriverManager.getConnection( - url - + "allowMultiQueries&transactionReplay=true&waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=40&connectTimeout=500&useReadAheadInput=false"); - long threadId = con.getThreadId(); - Statement stmt = con.createStatement(); - stmt.setFetchSize(2); - ResultSet rs = stmt.executeQuery("SELECT * FROM seq_1_to_50; SELECT * FROM seq_1_to_50000"); - rs.next(); - assertEquals(1, rs.getInt(1)); - proxy.restart(50); - Statement stmt2 = con.createStatement(); - assertThrowsContains(SQLException.class, () -> stmt2.executeQuery("SELECT * from mysql.user"), "Socket error during result streaming"); - assertNotEquals(threadId, con.getThreadId()); + (Connection) + DriverManager.getConnection( + url + + "allowMultiQueries&transactionReplay=true&waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=40&connectTimeout=500&useReadAheadInput=false"); + long threadId = con.getThreadId(); + Statement stmt = con.createStatement(); + stmt.setFetchSize(2); + ResultSet rs = stmt.executeQuery("SELECT * FROM seq_1_to_50; SELECT * FROM seq_1_to_50000"); + rs.next(); + assertEquals(1, rs.getInt(1)); + proxy.restart(50); + Statement stmt2 = con.createStatement(); + assertThrowsContains( + SQLException.class, + () -> stmt2.executeQuery("SELECT * from mysql.user"), + "Socket error during result streaming"); + assertNotEquals(threadId, con.getThreadId()); - // additional small test + // additional small test assertEquals(0, con.getNetworkTimeout()); - con.setNetworkTimeout(Runnable::run,10); + con.setNetworkTimeout(Runnable::run, 10); assertEquals(10, con.getNetworkTimeout()); con.setReadOnly(true); - con.close(); - assertThrowsContains(SQLNonTransientConnectionException.class, () -> con.setReadOnly(false), "Connection is closed"); - assertThrowsContains(SQLNonTransientConnectionException.class, () -> con.abort(Runnable::run), "Connection is closed"); + con.close(); + assertThrowsContains( + SQLNonTransientConnectionException.class, + () -> con.setReadOnly(false), + "Connection is closed"); + assertThrowsContains( + SQLNonTransientConnectionException.class, + () -> con.abort(Runnable::run), + "Connection is closed"); Connection con2 = - (Connection) - DriverManager.getConnection( - url - + "allowMultiQueries&transactionReplay=true&waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=40&connectTimeout=500&useReadAheadInput=false"); + (Connection) + DriverManager.getConnection( + url + + "allowMultiQueries&transactionReplay=true&waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=40&connectTimeout=500&useReadAheadInput=false"); con2.abort(Runnable::run); } @@ -301,12 +310,12 @@ public void masterReplicationFailover() throws Exception { } } - @Test public void masterReplicationStreamingFailover() throws Exception { Assumptions.assumeTrue( - isMariaDBServer() && - !"skysql".equals(System.getenv("srv")) && !"skysql-ha".equals(System.getenv("srv"))); + isMariaDBServer() + && !"skysql".equals(System.getenv("srv")) + && !"skysql-ha".equals(System.getenv("srv"))); Configuration conf = Configuration.parse(mDefUrl); HostAddress hostAddress = conf.addresses().get(0); @@ -317,21 +326,21 @@ public void masterReplicationStreamingFailover() throws Exception { } String url = - mDefUrl.replaceAll( - "//([^/]*)/", - String.format( - "//address=(host=localhost)(port=%s)(type=primary),address=(host=%s)(port=%s)(type=replica)/", - proxy.getLocalPort(), hostAddress.host, hostAddress.port, hostname, port)); + mDefUrl.replaceAll( + "//([^/]*)/", + String.format( + "//address=(host=localhost)(port=%s)(type=primary),address=(host=%s)(port=%s)(type=replica)/", + proxy.getLocalPort(), hostAddress.host, hostAddress.port, hostname, port)); url = url.replaceAll("jdbc:mariadb:", "jdbc:mariadb:replication:"); if (conf.sslMode() == SslMode.VERIFY_FULL) { url = url.replaceAll("sslMode=verify-full", "sslMode=verify-ca"); } Connection con = - (Connection) - DriverManager.getConnection( - url - + "allowMultiQueries&transactionReplay=true&waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=40&connectTimeout=500&useReadAheadInput=false"); + (Connection) + DriverManager.getConnection( + url + + "allowMultiQueries&transactionReplay=true&waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=40&connectTimeout=500&useReadAheadInput=false"); long threadId = con.getThreadId(); Statement stmt = con.createStatement(); stmt.setFetchSize(2); @@ -340,28 +349,36 @@ public void masterReplicationStreamingFailover() throws Exception { assertEquals(1, rs.getInt(1)); proxy.restart(50); Statement stmt2 = con.createStatement(); - assertThrowsContains(SQLException.class, () -> stmt2.executeQuery("SELECT * from mysql.user"), "Socket error during result streaming"); + assertThrowsContains( + SQLException.class, + () -> stmt2.executeQuery("SELECT * from mysql.user"), + "Socket error during result streaming"); assertNotEquals(threadId, con.getThreadId()); // additional small test assertEquals(0, con.getNetworkTimeout()); - con.setNetworkTimeout(Runnable::run,10); + con.setNetworkTimeout(Runnable::run, 10); assertEquals(10, con.getNetworkTimeout()); con.setReadOnly(true); con.close(); - assertThrowsContains(SQLNonTransientConnectionException.class, () -> con.setReadOnly(false), "Connection is closed"); - assertThrowsContains(SQLNonTransientConnectionException.class, () -> con.abort(Runnable::run), "Connection is closed"); + assertThrowsContains( + SQLNonTransientConnectionException.class, + () -> con.setReadOnly(false), + "Connection is closed"); + assertThrowsContains( + SQLNonTransientConnectionException.class, + () -> con.abort(Runnable::run), + "Connection is closed"); Connection con2 = - (Connection) - DriverManager.getConnection( - url - + "allowMultiQueries&transactionReplay=true&waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=40&connectTimeout=500&useReadAheadInput=false"); + (Connection) + DriverManager.getConnection( + url + + "allowMultiQueries&transactionReplay=true&waitReconnectTimeout=300&deniedListTimeout=300&retriesAllDown=40&connectTimeout=500&useReadAheadInput=false"); con2.abort(Runnable::run); } - public Connection createProxyConKeep(String opts) throws SQLException { Configuration conf = Configuration.parse(mDefUrl); HostAddress hostAddress = conf.addresses().get(0); diff --git a/src/test/java/org/mariadb/jdbc/integration/PreparedStatementTest.java b/src/test/java/org/mariadb/jdbc/integration/PreparedStatementTest.java index a4feb9c42..a27ce8a56 100644 --- a/src/test/java/org/mariadb/jdbc/integration/PreparedStatementTest.java +++ b/src/test/java/org/mariadb/jdbc/integration/PreparedStatementTest.java @@ -1017,10 +1017,11 @@ public void more2BytesParameters() throws Throwable { st.setInt(i, rnds[i - 1]); } assertThrowsContains( - SQLTransientConnectionException.class, + SQLException.class, () -> st.executeQuery(), "Prepared statement contains too many placeholders"); } + assertTrue(sharedConnBinary.isValid(1)); } private String generateLongText(int len) {