* [tarantool-patches] [PATCH] jdbc: add connection timeout configuration and handling @ 2018-10-12 15:47 Sergei Kalashnikov 2018-10-22 4:21 ` [tarantool-patches] " Alexander Turenko 0 siblings, 1 reply; 6+ messages in thread From: Sergei Kalashnikov @ 2018-10-12 15:47 UTC (permalink / raw) To: tarantool-patches; +Cc: alexander.turenko, Sergei Kalashnikov Added connection property `socketTimeout` to allow user control over network timeout before actual connection is returned by the driver. This is only done for default socket provider. The default timeout is is left to be infinite. Implemented `Connection.setNetworkTimeout` API to make it possible to change the maximum amount of time to wait for server replies after the connection is established. The connection that has timed out is forced to close. New subsequent operations requested on such connection must fail right away. The corresponding checks are embedded into relevant APIs. Closes #38 --- https://github.com/tarantool/tarantool-java/issues/38 https://github.com/ztarvos/tarantool-java/commits/gh-38-add-connect-timeout .../java/org/tarantool/TarantoolConnection.java | 21 ++ .../java/org/tarantool/jdbc/SQLConnection.java | 53 +++- .../org/tarantool/jdbc/SQLDatabaseMetadata.java | 97 +++++--- src/main/java/org/tarantool/jdbc/SQLDriver.java | 169 +++++++++++-- .../org/tarantool/jdbc/SQLPreparedStatement.java | 66 ++++- src/main/java/org/tarantool/jdbc/SQLStatement.java | 84 +++++-- .../java/org/tarantool/jdbc/JdbcConnectionIT.java | 61 +++++ .../org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java | 30 +++ .../java/org/tarantool/jdbc/JdbcDriverTest.java | 276 +++++++++++++++++++++ .../tarantool/jdbc/JdbcExceptionHandlingTest.java | 158 ++++++++++++ .../tarantool/jdbc/JdbcPreparedStatementIT.java | 94 +++++++ .../java/org/tarantool/jdbc/JdbcStatementIT.java | 30 +++ 12 files changed, 1036 insertions(+), 103 deletions(-) create mode 100644 src/test/java/org/tarantool/jdbc/JdbcDriverTest.java diff --git a/src/main/java/org/tarantool/TarantoolConnection.java b/src/main/java/org/tarantool/TarantoolConnection.java index be94995..b817988 100644 --- a/src/main/java/org/tarantool/TarantoolConnection.java +++ b/src/main/java/org/tarantool/TarantoolConnection.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.Socket; +import java.net.SocketException; import java.nio.ByteBuffer; import java.util.List; import java.util.Map; @@ -80,4 +81,24 @@ public class TarantoolConnection extends TarantoolBase<List<?>> implements Taran public boolean isClosed() { return socket.isClosed(); } + + /** + * Sets given timeout value on underlying socket. + * + * @param timeout Timeout in milliseconds. + * @throws SocketException If failed. + */ + public void setSocketTimeout(int timeout) throws SocketException { + socket.setSoTimeout(timeout); + } + + /** + * Retrieves timeout value from underlying socket. + * + * @return Timeout in milliseconds. + * @throws SocketException If failed. + */ + public int getSocketTimeout() throws SocketException { + return socket.getSoTimeout(); + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLConnection.java b/src/main/java/org/tarantool/jdbc/SQLConnection.java index de28850..16ffa4e 100644 --- a/src/main/java/org/tarantool/jdbc/SQLConnection.java +++ b/src/main/java/org/tarantool/jdbc/SQLConnection.java @@ -1,5 +1,7 @@ package org.tarantool.jdbc; +import java.io.IOException; +import java.net.SocketException; import java.sql.Array; import java.sql.Blob; import java.sql.CallableStatement; @@ -20,6 +22,7 @@ import java.util.Map; import java.util.Properties; import java.util.concurrent.Executor; +import org.tarantool.CommunicationException; import org.tarantool.TarantoolConnection; @SuppressWarnings("Since15") @@ -36,12 +39,14 @@ public class SQLConnection implements Connection { @Override public Statement createStatement() throws SQLException { - return new SQLStatement(connection, this); + checkNotClosed(); + return new SQLStatement(this); } @Override public PreparedStatement prepareStatement(String sql) throws SQLException { - return new SQLPreparedStatement(connection, this, sql); + checkNotClosed(); + return new SQLPreparedStatement(this, sql); } @Override @@ -89,6 +94,7 @@ public class SQLConnection implements Connection { @Override public DatabaseMetaData getMetaData() throws SQLException { + checkNotClosed(); return new SQLDatabaseMetadata(this); } @@ -293,15 +299,28 @@ public class SQLConnection implements Connection { @Override public void setNetworkTimeout(Executor executor, int milliseconds) throws SQLException { - throw new SQLFeatureNotSupportedException(); + checkNotClosed(); + + if (milliseconds < 0) + throw new SQLException("Network timeout cannot be negative."); + + try { + connection.setSocketTimeout(milliseconds); + } catch (SocketException e) { + throw new SQLException("Failed to set socket timeout: timeout=" + milliseconds, e); + } } @Override public int getNetworkTimeout() throws SQLException { - throw new SQLFeatureNotSupportedException(); + checkNotClosed(); + try { + return connection.getSocketTimeout(); + } catch (SocketException e) { + throw new SQLException("Failed to retrieve socket timeout", e); + } } - @Override public <T> T unwrap(Class<T> iface) throws SQLException { throw new SQLFeatureNotSupportedException(); @@ -311,4 +330,28 @@ public class SQLConnection implements Connection { public boolean isWrapperFor(Class<?> iface) throws SQLException { throw new SQLFeatureNotSupportedException(); } + + /** + * @throws SQLException If connection is closed. + */ + protected void checkNotClosed() throws SQLException { + if (isClosed()) + throw new SQLException("Connection is closed."); + } + + /** + * Inspects passed exception and closes the connection if appropriate. + * + * @param e Exception to process. + */ + protected void handleException(Exception e) { + if (CommunicationException.class.isAssignableFrom(e.getClass()) || + IOException.class.isAssignableFrom(e.getClass())) { + try { + close(); + } catch (SQLException ignored) { + // No-op. + } + } + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java index c8879c9..04c598d 100644 --- a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java +++ b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java @@ -672,23 +672,30 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { @Override public ResultSet getTables(String catalog, String schemaPattern, String tableNamePattern, String[] types) - throws SQLException { - if (types != null && !Arrays.asList(types).contains("TABLE")) { - return new SQLResultSet(JDBCBridge.EMPTY); - } - String[] parts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); - List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); - List<List<Object>> rows = new ArrayList<List<Object>>(); - for (List<Object> space : spaces) { - String name = (String) space.get(NAME_IDX); - Map flags = (Map) space.get(FLAGS_IDX); - if (flags != null && flags.containsKey("sql") && like(name, parts)) { - rows.add(Arrays.asList(name, "TABLE", flags.get("sql"))); + throws SQLException { + connection.checkNotClosed(); + try { + if (types != null && !Arrays.asList(types).contains("TABLE")) { + return new SQLResultSet(JDBCBridge.EMPTY); + } + String[] parts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); + List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); + List<List<Object>> rows = new ArrayList<List<Object>>(); + for (List<Object> space : spaces) { + String name = (String) space.get(NAME_IDX); + Map flags = (Map) space.get(FLAGS_IDX); + if (flags != null && flags.containsKey("sql") && like(name, parts)) { + rows.add(Arrays.asList(name, "TABLE", flags.get("sql"))); + } } + return new SQLNullResultSet(JDBCBridge.mock(Arrays.asList("TABLE_NAME", "TABLE_TYPE", "REMARKS", + //nulls + "TABLE_CAT", "TABLE_SCHEM", "TABLE_TYPE", "TYPE_CAT", "TYPE_SCHEM", "TYPE_NAME", "SELF_REFERENCING_COL_NAME", "REF_GENERATION"), rows)); + } catch (Exception e) { + connection.handleException(e); + throw new SQLException("Failed to retrieve table(s) description: " + + "tableNamePattern=\"" + tableNamePattern + "\".", e); } - return new SQLNullResultSet(JDBCBridge.mock(Arrays.asList("TABLE_NAME", "TABLE_TYPE", "REMARKS", - //nulls - "TABLE_CAT", "TABLE_SCHEM", "TABLE_TYPE", "TYPE_CAT", "TYPE_SCHEM", "TYPE_NAME", "SELF_REFERENCING_COL_NAME", "REF_GENERATION"), rows)); } @Override @@ -713,32 +720,40 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { @Override public ResultSet getColumns(String catalog, String schemaPattern, String tableNamePattern, String columnNamePattern) throws SQLException { - String[] tableParts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); - String[] colParts = columnNamePattern == null ? new String[]{""} : columnNamePattern.split("%"); - List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); - List<List<Object>> rows = new ArrayList<List<Object>>(); - for (List<Object> space : spaces) { - String tableName = (String) space.get(NAME_IDX); - Map flags = (Map) space.get(FLAGS_IDX); - if (flags != null && flags.containsKey("sql") && like(tableName, tableParts)) { - List<Map<String, Object>> format = (List<Map<String, Object>>) space.get(FORMAT_IDX); - for (int columnIdx = 1; columnIdx <= format.size(); columnIdx++) { - Map<String, Object> f = format.get(columnIdx - 1); - String columnName = (String) f.get("name"); - String dbType = (String) f.get("type"); - if (like(columnName, colParts)) { - rows.add(Arrays.<Object>asList(tableName, columnName, columnIdx, Types.OTHER, dbType, 10, 1, "YES", Types.OTHER, "NO", "NO")); + connection.checkNotClosed(); + try { + String[] tableParts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); + String[] colParts = columnNamePattern == null ? new String[]{""} : columnNamePattern.split("%"); + List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); + List<List<Object>> rows = new ArrayList<List<Object>>(); + for (List<Object> space : spaces) { + String tableName = (String) space.get(NAME_IDX); + Map flags = (Map) space.get(FLAGS_IDX); + if (flags != null && flags.containsKey("sql") && like(tableName, tableParts)) { + List<Map<String, Object>> format = (List<Map<String, Object>>) space.get(FORMAT_IDX); + for (int columnIdx = 1; columnIdx <= format.size(); columnIdx++) { + Map<String, Object> f = format.get(columnIdx - 1); + String columnName = (String) f.get("name"); + String dbType = (String) f.get("type"); + if (like(columnName, colParts)) { + rows.add(Arrays.<Object>asList(tableName, columnName, columnIdx, Types.OTHER, dbType, 10, 1, "YES", Types.OTHER, "NO", "NO")); + } } } } - } - return new SQLNullResultSet((JDBCBridge.mock( - Arrays.asList("TABLE_NAME", "COLUMN_NAME", "ORDINAL_POSITION", "DATA_TYPE", "TYPE_NAME", "NUM_PREC_RADIX", "NULLABLE", "IS_NULLABLE", "SOURCE_DATA_TYPE", "IS_AUTOINCREMENT", "IS_GENERATEDCOLUMN", - //nulls - "TABLE_CAT", "TABLE_SCHEM", "COLUMN_SIZE", "BUFFER_LENGTH", "DECIMAL_DIGITS", "REMARKS", "COLUMN_DEF", "SQL_DATA_TYPE", "SQL_DATETIME_SUB", "CHAR_OCTET_LENGTH", "SCOPE_CATALOG", "SCOPE_SCHEMA", "SCOPE_TABLE" - ), - rows))); + return new SQLNullResultSet((JDBCBridge.mock( + Arrays.asList("TABLE_NAME", "COLUMN_NAME", "ORDINAL_POSITION", "DATA_TYPE", "TYPE_NAME", "NUM_PREC_RADIX", "NULLABLE", "IS_NULLABLE", "SOURCE_DATA_TYPE", "IS_AUTOINCREMENT", "IS_GENERATEDCOLUMN", + //nulls + "TABLE_CAT", "TABLE_SCHEM", "COLUMN_SIZE", "BUFFER_LENGTH", "DECIMAL_DIGITS", "REMARKS", "COLUMN_DEF", "SQL_DATA_TYPE", "SQL_DATETIME_SUB", "CHAR_OCTET_LENGTH", "SCOPE_CATALOG", "SCOPE_SCHEMA", "SCOPE_TABLE" + ), + rows))); + } catch (Exception e) { + connection.handleException(e); + throw new SQLException("Error processing table column metadata: " + + "tableNamePattern=\"" + tableNamePattern + "\"; " + + "columnNamePattern=\"" + columnNamePattern + "\".", e); + } } @Override @@ -766,6 +781,8 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { @Override public ResultSet getPrimaryKeys(String catalog, String schema, String table) throws SQLException { + connection.checkNotClosed(); + final List<String> colNames = Arrays.asList( "TABLE_CAT", "TABLE_SCHEM", "TABLE_NAME", "COLUMN_NAME", "KEY_SEQ", "PK_NAME"); @@ -809,9 +826,9 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { } }); return new SQLNullResultSet((JDBCBridge.mock(colNames, rows))); - } - catch (Throwable t) { - throw new SQLException("Error processing metadata for table \"" + table + "\".", t); + } catch (Exception e) { + connection.handleException(e); + throw new SQLException("Error processing metadata for table \"" + table + "\".", e); } } diff --git a/src/main/java/org/tarantool/jdbc/SQLDriver.java b/src/main/java/org/tarantool/jdbc/SQLDriver.java index 6867997..87e7dca 100644 --- a/src/main/java/org/tarantool/jdbc/SQLDriver.java +++ b/src/main/java/org/tarantool/jdbc/SQLDriver.java @@ -3,6 +3,7 @@ package org.tarantool.jdbc; import java.io.IOException; import java.net.InetSocketAddress; import java.net.Socket; +import java.net.SocketException; import java.net.URI; import java.sql.Connection; import java.sql.Driver; @@ -32,7 +33,14 @@ public class SQLDriver implements Driver { public static final String PROP_SOCKET_PROVIDER = "socketProvider"; public static final String PROP_USER = "user"; public static final String PROP_PASSWORD = "password"; + public static final String PROP_SOCKET_TIMEOUT = "socketTimeout"; + // Define default values once here. + final static Properties defaults = new Properties() {{ + setProperty(PROP_HOST, "localhost"); + setProperty(PROP_PORT, "3301"); + setProperty(PROP_SOCKET_TIMEOUT, "0"); + }}; protected Map<String, SQLSocketProvider> providerCache = new ConcurrentHashMap<String, SQLSocketProvider>(); @@ -45,22 +53,42 @@ public class SQLDriver implements Driver { if (providerClassName != null) { socket = getSocketFromProvider(uri, urlProperties, providerClassName); } else { - socket = createAndConnectDefaultSocket(urlProperties); + // Passing the socket to allow unit tests to mock it. + socket = connectAndSetupDefaultSocket(urlProperties, new Socket()); } try { - TarantoolConnection connection = new TarantoolConnection(urlProperties.getProperty(PROP_USER), urlProperties.getProperty(PROP_PASSWORD), socket) {{ + TarantoolConnection connection = new TarantoolConnection( + urlProperties.getProperty(PROP_USER), + urlProperties.getProperty(PROP_PASSWORD), + socket) {{ msgPackLite = SQLMsgPackLite.INSTANCE; }}; - return new SQLConnection(connection, url, info); - } catch (IOException e) { - throw new SQLException("Couldn't initiate connection. Provider class name is " + providerClassName, e); + return new SQLConnection(connection, url, urlProperties); + } catch (Exception e) { + try { + socket.close(); + } catch (IOException ignored) { + // No-op. + } + throw new SQLException("Couldn't initiate connection using " + diagProperties(urlProperties), e); } - } - protected Properties parseQueryString(URI uri, Properties info) { - Properties urlProperties = new Properties(info); + protected Properties parseQueryString(URI uri, Properties info) throws SQLException { + Properties urlProperties = new Properties(defaults); + + String userInfo = uri.getUserInfo(); + if (userInfo != null) { + // Get user and password from the corresponding part of the URI, i.e. before @ sign. + int i = userInfo.indexOf(':'); + if (i < 0) { + urlProperties.setProperty(PROP_USER, userInfo); + } else { + urlProperties.setProperty(PROP_USER, userInfo.substring(0, i)); + urlProperties.setProperty(PROP_PASSWORD, userInfo.substring(i + 1)); + } + } if (uri.getQuery() != null) { String[] parts = uri.getQuery().split("&"); for (String part : parts) { @@ -72,24 +100,77 @@ public class SQLDriver implements Driver { } } } - urlProperties.put(PROP_HOST, uri.getHost() == null ? "localhost" : uri.getHost()); - urlProperties.put(PROP_PORT, uri.getPort() < 1 ? "3301" : uri.getPort()); + if (uri.getHost() != null) { + // Default values are pre-put above. + urlProperties.setProperty(PROP_HOST, uri.getHost()); + } + if (uri.getPort() >= 0) { + // We need to convert port to string otherwise getProperty() will not see it. + urlProperties.setProperty(PROP_PORT, String.valueOf(uri.getPort())); + } + if (info != null) + urlProperties.putAll(info); + + // Validate properties. + int port; + try { + port = Integer.parseInt(urlProperties.getProperty(PROP_PORT)); + } catch (Exception e) { + throw new SQLException("Port must be a valid number."); + } + if (port <= 0 || port > 65535) { + throw new SQLException("Port is out of range: " + port); + } + int timeout; + try { + timeout = Integer.parseInt(urlProperties.getProperty(PROP_SOCKET_TIMEOUT)); + } catch (Exception e) { + throw new SQLException("Timeout must be a valid number."); + } + if (timeout < 0) { + throw new SQLException("Timeout must not be negative."); + } return urlProperties; } - protected Socket createAndConnectDefaultSocket(Properties properties) throws SQLException { - Socket socket; - socket = new Socket(); + /** + * Connects and setup given socket according to connection properties. + * + * Note: socket parameter mainly exists to enable mocking for unit testing. + * + * @param properties Connection properties. + * @param socket Fresh socket instance to setup. + * @return Connected socket. + * @throws SQLException If failed. + */ + protected Socket connectAndSetupDefaultSocket(Properties properties, Socket socket) throws SQLException { + int timeout = Integer.parseInt(properties.getProperty(PROP_SOCKET_TIMEOUT)); try { - socket.connect(new InetSocketAddress(properties.getProperty(PROP_HOST, "localhost"), Integer.parseInt(properties.getProperty(PROP_PORT, "3301")))); - } catch (Exception e) { - throw new SQLException("Couldn't connect to tarantool using" + properties, e); + socket.connect(new InetSocketAddress( + properties.getProperty(PROP_HOST), + Integer.parseInt(properties.getProperty(PROP_PORT))), + timeout); + } catch (IOException e) { + throw new SQLException("Couldn't connect to tarantool using " + diagProperties(properties), e); + } + // Setup socket further. + if (timeout > 0) { + try { + socket.setSoTimeout(timeout); + } catch (SocketException e) { + try { + socket.close(); + } catch (IOException ignored) { + // No-op. + } + throw new SQLException("Couldn't set socket timeout. timeout=" + timeout, e); + } } return socket; } protected Socket getSocketFromProvider(URI uri, Properties urlProperties, String providerClassName) - throws SQLException { + throws SQLException { Socket socket; SQLSocketProvider sqlSocketProvider = providerCache.get(providerClassName); if (sqlSocketProvider == null) { @@ -103,12 +184,23 @@ public class SQLDriver implements Driver { providerCache.put(providerClassName, sqlSocketProvider); } } catch (Exception e) { - throw new SQLException("Couldn't initiate socket provider " + providerClassName, e); + throw new SQLException("Couldn't instantiate socket provider: " + providerClassName, e); + } + if (sqlSocketProvider == null) { + throw new SQLException(String.format("Socket provider %s does not implement %s", + providerClassName, SQLSocketProvider.class.getCanonicalName())); } } } } - socket = sqlSocketProvider.getConnectedSocket(uri, urlProperties); + try { + socket = sqlSocketProvider.getConnectedSocket(uri, urlProperties); + } catch (Exception e) { + throw new SQLException("Socket provider has failed to connect using " + diagProperties(urlProperties), e); + } + if (socket == null) + throw new SQLException("Socket provider returned null socket: " + diagProperties(urlProperties)); + return socket; } @@ -121,16 +213,15 @@ public class SQLDriver implements Driver { public DriverPropertyInfo[] getPropertyInfo(String url, Properties info) throws SQLException { try { URI uri = new URI(url); - Properties properties = parseQueryString(uri, new Properties(info == null ? new Properties() : info)); + Properties properties = parseQueryString(uri, info); DriverPropertyInfo host = new DriverPropertyInfo(PROP_HOST, properties.getProperty(PROP_HOST)); host.required = true; - host.description = "Tarantool sever host"; + host.description = "Tarantool server host"; DriverPropertyInfo port = new DriverPropertyInfo(PROP_PORT, properties.getProperty(PROP_PORT)); port.required = true; - port.description = "Tarantool sever port"; - + port.description = "Tarantool server port"; DriverPropertyInfo user = new DriverPropertyInfo(PROP_USER, properties.getProperty(PROP_USER)); user.required = false; @@ -140,12 +231,20 @@ public class SQLDriver implements Driver { password.required = false; password.description = "password"; - DriverPropertyInfo socketProvider = new DriverPropertyInfo(PROP_SOCKET_PROVIDER, properties.getProperty(PROP_SOCKET_PROVIDER)); + DriverPropertyInfo socketProvider = new DriverPropertyInfo( + PROP_SOCKET_PROVIDER, properties.getProperty(PROP_SOCKET_PROVIDER)); + socketProvider.required = false; socketProvider.description = "SocketProvider class implements org.tarantool.jdbc.SQLSocketProvider"; + DriverPropertyInfo socketTimeout = new DriverPropertyInfo( + PROP_SOCKET_TIMEOUT, properties.getProperty(PROP_SOCKET_TIMEOUT)); - return new DriverPropertyInfo[]{host, port, user, password, socketProvider}; + socketTimeout.required = false; + socketTimeout.description = "The number of milliseconds to wait before a timeout is occurred on a socket" + + " connect or read. The default value is 0, which means infinite timeout."; + + return new DriverPropertyInfo[]{host, port, user, password, socketProvider, socketTimeout}; } catch (Exception e) { throw new SQLException(e); } @@ -171,5 +270,23 @@ public class SQLDriver implements Driver { throw new SQLFeatureNotSupportedException(); } - + /** + * Builds a string representation of given connection properties + * along with their sanitized values. + * + * @param props Connection properties. + * @return Comma-separated pairs of property names and values. + */ + protected static String diagProperties(Properties props) { + StringBuilder sb = new StringBuilder(); + for (Map.Entry<Object, Object> e : props.entrySet()) { + if (sb.length() > 0) + sb.append(", "); + sb.append(e.getKey()); + sb.append('='); + sb.append(PROP_USER.equals(e.getKey()) || PROP_PASSWORD.equals(e.getKey()) ? + "*****" : e.getValue().toString()); + } + return sb.toString(); + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java b/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java index 23d1073..c7a3961 100644 --- a/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java +++ b/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java @@ -20,27 +20,36 @@ import java.sql.SQLFeatureNotSupportedException; import java.sql.SQLXML; import java.sql.Time; import java.sql.Timestamp; +import java.util.Arrays; import java.util.Calendar; import java.util.HashMap; import java.util.Map; import org.tarantool.JDBCBridge; -import org.tarantool.TarantoolConnection; public class SQLPreparedStatement extends SQLStatement implements PreparedStatement { + final static String INVALID_CALL_MSG = "The method cannot be called on a PreparedStatement."; final String sql; final Map<Integer, Object> params; - public SQLPreparedStatement(TarantoolConnection connection, SQLConnection sqlConnection, String sql) { - super(connection, sqlConnection); + public SQLPreparedStatement(SQLConnection connection, String sql) { + super(connection); this.sql = sql; this.params = new HashMap<Integer, Object>(); } @Override public ResultSet executeQuery() throws SQLException { - return new SQLResultSet(JDBCBridge.query(connection, sql, getParams())); + connection.checkNotClosed(); + discardLastResults(); + Object[] args = getParams(); + try { + return new SQLResultSet(JDBCBridge.query(connection.connection, sql, args)); + } catch (Exception e) { + connection.handleException(e); + throw new SQLException(formatError(sql, args), e); + } } protected Object[] getParams() throws SQLException { @@ -49,7 +58,7 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem if (params.containsKey(i)) { objects[i - 1] = params.get(i); } else { - throw new SQLException("Parameter " + i + "is not"); + throw new SQLException("Parameter " + i + " is missing"); } } return objects; @@ -57,8 +66,15 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem @Override public int executeUpdate() throws SQLException { - return JDBCBridge.update(connection, sql, getParams()); - + connection.checkNotClosed(); + discardLastResults(); + Object[] args = getParams(); + try { + return JDBCBridge.update(connection.connection, sql, args); + } catch (Exception e) { + connection.handleException(e); + throw new SQLException(formatError(sql, args), e); + } } @Override @@ -163,7 +179,15 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem @Override public boolean execute() throws SQLException { - return false; + connection.checkNotClosed(); + discardLastResults(); + Object[] args = getParams(); + try { + return handleResult(JDBCBridge.execute(connection.connection, sql, args)); + } catch (Exception e) { + connection.handleException(e); + throw new SQLException(formatError(sql, args), e); + } } @Override @@ -336,10 +360,34 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem throw new SQLFeatureNotSupportedException(); } - @Override public void addBatch() throws SQLException { throw new SQLFeatureNotSupportedException(); } + @Override + public ResultSet executeQuery(String sql) throws SQLException { + throw new SQLException(INVALID_CALL_MSG); + } + + @Override + public int executeUpdate(String sql) throws SQLException { + throw new SQLException(INVALID_CALL_MSG); + } + + @Override + public boolean execute(String sql) throws SQLException { + throw new SQLException(INVALID_CALL_MSG); + } + + /** + * Provides error message that contains parameters of failed SQL statement. + * + * @param sql SQL Text. + * @param params Parameters of the SQL statement. + * @return Formatted error message. + */ + private static String formatError(String sql, Object[] params) { + return "Failed to execute SQL: " + sql + ", params: " + Arrays.deepToString(params); + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLStatement.java b/src/main/java/org/tarantool/jdbc/SQLStatement.java index 141ae52..8142687 100644 --- a/src/main/java/org/tarantool/jdbc/SQLStatement.java +++ b/src/main/java/org/tarantool/jdbc/SQLStatement.java @@ -8,33 +8,40 @@ import java.sql.SQLWarning; import java.sql.Statement; import org.tarantool.JDBCBridge; -import org.tarantool.TarantoolConnection; @SuppressWarnings("Since15") public class SQLStatement implements Statement { - protected final TarantoolConnection connection; - protected final SQLConnection sqlConnection; + protected final SQLConnection connection; private SQLResultSet resultSet; private int updateCount; private int maxRows; - protected SQLStatement(TarantoolConnection connection, SQLConnection sqlConnection) { - this.connection = connection; - this.sqlConnection = sqlConnection; + protected SQLStatement(SQLConnection sqlConnection) { + this.connection = sqlConnection; } @Override public ResultSet executeQuery(String sql) throws SQLException { - resultSet = new SQLResultSet(JDBCBridge.query(connection, sql)); - updateCount = -1; - return resultSet; + connection.checkNotClosed(); + discardLastResults(); + try { + return new SQLResultSet(JDBCBridge.query(connection.connection, sql)); + } catch (Exception e) { + connection.handleException(e); + throw new SQLException("Failed to execute SQL: " + sql, e); + } } @Override public int executeUpdate(String sql) throws SQLException { - int update = JDBCBridge.update(connection, sql); - resultSet = null; - return update; + connection.checkNotClosed(); + discardLastResults(); + try { + return JDBCBridge.update(connection.connection, sql); + } catch (Exception e) { + connection.handleException(e); + throw new SQLException("Failed to execute SQL: " + sql, e); + } } @Override @@ -102,16 +109,13 @@ public class SQLStatement implements Statement { @Override public boolean execute(String sql) throws SQLException { - Object result = JDBCBridge.execute(connection, sql); - if (result instanceof SQLResultSet) { - resultSet = (SQLResultSet) result; - resultSet.maxRows = maxRows; - updateCount = -1; - return true; - } else { - resultSet = null; - updateCount = (Integer) result; - return false; + connection.checkNotClosed(); + discardLastResults(); + try { + return handleResult(JDBCBridge.execute(connection.connection, sql)); + } catch (Exception e) { + connection.handleException(e); + throw new SQLException("Failed to execute SQL: " + sql, e); } } @@ -186,7 +190,7 @@ public class SQLStatement implements Statement { @Override public Connection getConnection() throws SQLException { - return sqlConnection; + return connection; } @Override @@ -268,4 +272,38 @@ public class SQLStatement implements Statement { public boolean isWrapperFor(Class<?> iface) throws SQLException { throw new SQLFeatureNotSupportedException(); } + + /** + * Clears the results of the most recent execution. + */ + protected void discardLastResults() { + updateCount = -1; + if (resultSet != null) { + try { + resultSet.close(); + } catch (Exception ignored) { + // No-op. + } + resultSet = null; + } + } + + /** + * Sets the internals according to the result of last execution. + * + * @param result The result of SQL statement execution. + * @return {@code true}, if the result is a ResultSet object. + */ + protected boolean handleResult(Object result) { + if (result instanceof SQLResultSet) { + resultSet = (SQLResultSet) result; + resultSet.maxRows = maxRows; + updateCount = -1; + return true; + } else { + resultSet = null; + updateCount = (Integer) result; + return false; + } + } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java b/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java index cc6bfb9..ec16898 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java @@ -1,16 +1,24 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import org.tarantool.TarantoolConnection; +import java.lang.reflect.Field; +import java.net.Socket; import java.sql.DatabaseMetaData; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Statement; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +@SuppressWarnings("Since15") public class JdbcConnectionIT extends AbstractJdbcIT { @Test public void testCreateStatement() throws SQLException { @@ -39,4 +47,57 @@ public class JdbcConnectionIT extends AbstractJdbcIT { DatabaseMetaData meta = conn.getMetaData(); assertNotNull(meta); } + + @Test + public void testGetSetNetworkTimeout() throws Exception { + assertEquals(0, conn.getNetworkTimeout()); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + conn.setNetworkTimeout(null, -1); + } + }); + assertEquals("Network timeout cannot be negative.", e.getMessage()); + + conn.setNetworkTimeout(null, 3000); + + assertEquals(3000, conn.getNetworkTimeout()); + + // Check that timeout gets propagated to the socket. + Field sock = TarantoolConnection.class.getDeclaredField("socket"); + sock.setAccessible(true); + assertEquals(3000, ((Socket)sock.get(((SQLConnection)conn).connection)).getSoTimeout()); + } + + @Test + public void testClosedConnection() throws SQLException { + conn.close(); + + int i = 0; + for (; i < 5; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: conn.createStatement(); + break; + case 1: conn.prepareStatement("TEST"); + break; + case 2: conn.getMetaData(); + break; + case 3: conn.getNetworkTimeout(); + break; + case 4: conn.setNetworkTimeout(null, 1000); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getMessage()); + } + assertEquals(5, i); + } } \ No newline at end of file diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java index 17ab086..e8244b0 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java @@ -2,6 +2,7 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.function.Executable; import java.sql.DatabaseMetaData; import java.sql.ResultSet; @@ -12,7 +13,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT { private DatabaseMetaData meta; @@ -183,4 +186,31 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT { assertEquals(seq, rs.getInt(5)); assertEquals(pkName, rs.getString(6)); } + + @Test + public void testClosedConnection() throws SQLException { + conn.close(); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: meta.getTables(null, null, null, new String[]{"TABLE"}); + break; + case 1: meta.getColumns(null, null, "TEST", null); + break; + case 2: meta.getPrimaryKeys(null, null, "TEST"); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getMessage()); + } + assertEquals(3, i); + } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcDriverTest.java b/src/test/java/org/tarantool/jdbc/JdbcDriverTest.java new file mode 100644 index 0000000..25d26f3 --- /dev/null +++ b/src/test/java/org/tarantool/jdbc/JdbcDriverTest.java @@ -0,0 +1,276 @@ +package org.tarantool.jdbc; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import org.tarantool.CommunicationException; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.net.SocketException; +import java.net.SocketTimeoutException; +import java.net.URI; + +import java.sql.Driver; +import java.sql.DriverManager; +import java.sql.DriverPropertyInfo; +import java.sql.SQLException; +import java.util.Properties; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.tarantool.jdbc.SQLDriver.PROP_HOST; +import static org.tarantool.jdbc.SQLDriver.PROP_PASSWORD; +import static org.tarantool.jdbc.SQLDriver.PROP_PORT; +import static org.tarantool.jdbc.SQLDriver.PROP_SOCKET_PROVIDER; +import static org.tarantool.jdbc.SQLDriver.PROP_SOCKET_TIMEOUT; +import static org.tarantool.jdbc.SQLDriver.PROP_USER; + +public class JdbcDriverTest { + @Test + public void testParseQueryString() throws Exception { + SQLDriver drv = new SQLDriver(); + + Properties prop = new Properties(); + prop.setProperty(PROP_USER, "adm"); + prop.setProperty(PROP_PASSWORD, "secret"); + + URI uri = new URI(String.format( + "tarantool://server.local:3302?%s=%s&%s=%d", + PROP_SOCKET_PROVIDER, "some.class", + PROP_SOCKET_TIMEOUT, 5000)); + + Properties res = drv.parseQueryString(uri, prop); + assertNotNull(res); + + assertEquals("server.local", res.getProperty(PROP_HOST)); + assertEquals("3302", res.getProperty(PROP_PORT)); + assertEquals("adm", res.getProperty(PROP_USER)); + assertEquals("secret", res.getProperty(PROP_PASSWORD)); + assertEquals("some.class", res.getProperty(PROP_SOCKET_PROVIDER)); + assertEquals("5000", res.getProperty(PROP_SOCKET_TIMEOUT)); + } + + @Test + public void testParseQueryStringUserInfoInURI() throws Exception { + SQLDriver drv = new SQLDriver(); + Properties res = drv.parseQueryString(new URI("tarantool://adm:secret@server.local"), null); + assertNotNull(res); + assertEquals("server.local", res.getProperty(PROP_HOST)); + assertEquals("3301", res.getProperty(PROP_PORT)); + assertEquals("adm", res.getProperty(PROP_USER)); + assertEquals("secret", res.getProperty(PROP_PASSWORD)); + } + + @Test + public void testParseQueryStringValidations() { + // Check non-number port + checkParseQueryStringValidation("tarantool://0", + new Properties() {{setProperty(PROP_PORT, "nan");}}, + "Port must be a valid number."); + + // Check zero port + checkParseQueryStringValidation("tarantool://0:0", null, "Port is out of range: 0"); + + // Check high port + checkParseQueryStringValidation("tarantool://0:65536", null, "Port is out of range: 65536"); + + // Check non-number timeout + checkParseQueryStringValidation(String.format("tarantool://0:3301?%s=nan", PROP_SOCKET_TIMEOUT), null, + "Timeout must be a valid number."); + + // Check negative timeout + checkParseQueryStringValidation(String.format("tarantool://0:3301?%s=-100", PROP_SOCKET_TIMEOUT), null, + "Timeout must not be negative."); + } + + @Test + public void testGetPropertyInfo() throws SQLException { + Driver drv = new SQLDriver(); + Properties props = new Properties(); + DriverPropertyInfo[] info = drv.getPropertyInfo("tarantool://server.local:3302", props); + assertNotNull(info); + + for (DriverPropertyInfo e: info) { + assertNotNull(e.name); + assertNull(e.choices); + assertNotNull(e.description); + + if (PROP_HOST.equals(e.name)) { + assertTrue(e.required); + assertEquals("server.local", e.value); + } else if (PROP_PORT.equals(e.name)) { + assertTrue(e.required); + assertEquals("3302", e.value); + } else if (PROP_USER.equals(e.name)) { + assertFalse(e.required); + assertNull(e.value); + } else if (PROP_PASSWORD.equals(e.name)) { + assertFalse(e.required); + assertNull(e.value); + } else if (PROP_SOCKET_PROVIDER.equals(e.name)) { + assertFalse(e.required); + assertNull(e.value); + } else if (PROP_SOCKET_TIMEOUT.equals(e.name)) { + assertFalse(e.required); + assertEquals("0", e.value); + } else + fail("Unknown property '" + e.name + "'"); + } + } + + @Test + public void testDefaultSocketProviderConnectTimeoutError() throws IOException { + final int socketTimeout = 3000; + final Socket mockSocket = mock(Socket.class); + final SQLDriver drv = new TestSQLDriverThatPassesSocket(mockSocket); + + SocketTimeoutException timeoutEx = new SocketTimeoutException(); + doThrow(timeoutEx) + .when(mockSocket) + .connect(new InetSocketAddress("localhost", 3301), socketTimeout); + + final Properties prop = new Properties(); + prop.setProperty(PROP_SOCKET_TIMEOUT, String.valueOf(socketTimeout)); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + drv.connect("tarantool://localhost:3301", prop); + } + }); + + assertTrue(e.getMessage().startsWith("Couldn't connect to tarantool using"), e.getMessage()); + assertEquals(timeoutEx, e.getCause()); + } + + @Test + public void testDefaultSocketProviderSetSocketTimeoutError() throws IOException { + final int socketTimeout = 3000; + final Socket mockSocket = mock(Socket.class); + final SQLDriver drv = new TestSQLDriverThatPassesSocket(mockSocket); + + // Check error setting socket timeout + reset(mockSocket); + doNothing() + .when(mockSocket) + .connect(new InetSocketAddress("localhost", 3301), socketTimeout); + + SocketException sockEx = new SocketException("TEST"); + doThrow(sockEx) + .when(mockSocket) + .setSoTimeout(socketTimeout); + + final Properties prop = new Properties(); + prop.setProperty(PROP_SOCKET_TIMEOUT, String.valueOf(socketTimeout)); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + drv.connect("tarantool://localhost:3301", prop); + } + }); + + assertTrue(e.getMessage().startsWith("Couldn't set socket timeout."), e.getMessage()); + assertEquals(sockEx, e.getCause()); + } + + @Test + public void testCustomSocketProviderFail() throws SQLException { + checkCustomSocketProviderFail("nosuchclassexists", + "Couldn't instantiate socket provider"); + + checkCustomSocketProviderFail(Integer.class.getName(), + "Socket provider java.lang.Integer does not implement org.tarantool.jdbc.SQLSocketProvider"); + + checkCustomSocketProviderFail(TestSQLProviderThatReturnsNull.class.getName(), + "Socket provider returned null socket"); + + checkCustomSocketProviderFail(TestSQLProviderThatThrows.class.getName(), + "Socket provider has failed to connect using"); + } + + @Test + public void testNoResponseAfterInitialConnect() throws IOException { + ServerSocket socket = new ServerSocket(); + socket.bind(null, 0); + try { + final String url = "tarantool://localhost:" + socket.getLocalPort(); + final Properties prop = new Properties(); + prop.setProperty(PROP_SOCKET_TIMEOUT, "3000"); + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + DriverManager.getConnection(url, prop); + } + }); + assertTrue(e.getMessage().startsWith("Couldn't initiate connection using "), e.getMessage()); + assertTrue(e.getCause() instanceof CommunicationException); + assertTrue(e.getCause().getCause() instanceof SocketTimeoutException); + } finally { + socket.close(); + } + } + + private void checkCustomSocketProviderFail(String providerClassName, String errMsg) throws SQLException { + final Driver drv = DriverManager.getDriver("tarantool:"); + final Properties prop = new Properties(); + prop.setProperty(PROP_SOCKET_PROVIDER, providerClassName); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + drv.connect("tarantool://0:3301", prop); + } + }); + assertTrue(e.getMessage().startsWith(errMsg), e.getMessage()); + } + + private void checkParseQueryStringValidation(final String uri, final Properties prop, String errMsg) { + final SQLDriver drv = new SQLDriver(); + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + drv.parseQueryString(new URI(uri), prop); + } + }); + assertTrue(e.getMessage().startsWith(errMsg), e.getMessage()); + } + + static class TestSQLDriverThatPassesSocket extends SQLDriver { + private Socket mockSocket; + + TestSQLDriverThatPassesSocket(Socket mockSocket) { + this.mockSocket = mockSocket; + } + + @Override + protected Socket connectAndSetupDefaultSocket(Properties properties, Socket s) throws SQLException { + return super.connectAndSetupDefaultSocket(properties, mockSocket); + } + } + + static class TestSQLProviderThatReturnsNull implements SQLSocketProvider { + @Override + public Socket getConnectedSocket(URI uri, Properties params) { + return null; + } + } + + static class TestSQLProviderThatThrows implements SQLSocketProvider { + @Override + public Socket getConnectedSocket(URI uri, Properties params) { + throw new RuntimeException("ERROR"); + } + } +} diff --git a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java index 8cc7acc..0f74965 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java +++ b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java @@ -2,22 +2,33 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.api.function.ThrowingConsumer; +import org.tarantool.CommunicationException; import org.tarantool.TarantoolConnection; +import java.io.IOException; +import java.net.Socket; import java.sql.DatabaseMetaData; +import java.sql.PreparedStatement; import java.sql.SQLException; +import java.sql.Statement; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Properties; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.tarantool.jdbc.SQLDatabaseMetadata.FORMAT_IDX; import static org.tarantool.jdbc.SQLDatabaseMetadata.INDEX_FORMAT_IDX; import static org.tarantool.jdbc.SQLDatabaseMetadata.SPACE_ID_IDX; +import static org.tarantool.jdbc.SQLDatabaseMetadata.SPACES_MAX; import static org.tarantool.jdbc.SQLDatabaseMetadata._VINDEX; import static org.tarantool.jdbc.SQLDatabaseMetadata._VSPACE; @@ -57,4 +68,151 @@ public class JdbcExceptionHandlingTest { assertTrue(t.getCause().getMessage().contains("Wrong value type")); } + + @Test + public void testStatementCommunicationException() throws SQLException { + checkStatementCommunicationException(new ThrowingConsumer<Statement>() { + @Override + public void accept(Statement statement) throws Throwable { + statement.executeQuery("TEST"); + } + }); + checkStatementCommunicationException(new ThrowingConsumer<Statement>() { + @Override + public void accept(Statement statement) throws Throwable { + statement.executeUpdate("TEST"); + } + }); + checkStatementCommunicationException(new ThrowingConsumer<Statement>() { + @Override + public void accept(Statement statement) throws Throwable { + statement.execute("TEST"); + } + }); + } + + @Test + public void testPreparedStatementCommunicationException() throws SQLException { + checkPreparedStatementCommunicationException(new ThrowingConsumer<PreparedStatement>() { + @Override + public void accept(PreparedStatement prep) throws Throwable { + prep.executeQuery(); + } + }); + checkPreparedStatementCommunicationException(new ThrowingConsumer<PreparedStatement>() { + @Override + public void accept(PreparedStatement prep) throws Throwable { + prep.executeUpdate(); + } + }); + checkPreparedStatementCommunicationException(new ThrowingConsumer<PreparedStatement>() { + @Override + public void accept(PreparedStatement prep) throws Throwable { + prep.execute(); + } + }); + } + + @Test + public void testDatabaseMetaDataCommunicationException() throws SQLException { + checkDatabaseMetaDataCommunicationException(new ThrowingConsumer<DatabaseMetaData>() { + @Override + public void accept(DatabaseMetaData meta) throws Throwable { + meta.getTables(null, null, null, new String[] {"TABLE"}); + } + }, "Failed to retrieve table(s) description: tableNamePattern=\"null\"."); + + checkDatabaseMetaDataCommunicationException(new ThrowingConsumer<DatabaseMetaData>() { + @Override + public void accept(DatabaseMetaData meta) throws Throwable { + meta.getColumns(null, null, "TEST", "ID"); + } + }, "Error processing table column metadata: tableNamePattern=\"TEST\"; columnNamePattern=\"ID\"."); + + checkDatabaseMetaDataCommunicationException(new ThrowingConsumer<DatabaseMetaData>() { + @Override + public void accept(DatabaseMetaData meta) throws Throwable { + meta.getPrimaryKeys(null, null, "TEST"); + } + }, "Error processing metadata for table \"TEST\"."); + } + + private void checkStatementCommunicationException(final ThrowingConsumer<Statement> consumer) throws SQLException { + TestTarantoolConnection mockCon = mock(TestTarantoolConnection.class); + final Statement stmt = new SQLStatement(new SQLConnection(mockCon, "tarantool://0:0", new Properties())); + + Exception ex = new CommunicationException("TEST"); + + doThrow(ex).when(mockCon).sql("TEST", new Object[0]); + doThrow(ex).when(mockCon).update("TEST"); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + consumer.accept(stmt); + } + }); + assertTrue(e.getMessage().startsWith("Failed to execute"), e.getMessage()); + + assertEquals(ex, e.getCause()); + + verify(((SQLConnection)stmt.getConnection()).connection, times(1)).close(); + } + + private void checkPreparedStatementCommunicationException(final ThrowingConsumer<PreparedStatement> consumer) + throws SQLException { + TestTarantoolConnection mockCon = mock(TestTarantoolConnection.class); + + final PreparedStatement prep = new SQLPreparedStatement( + new SQLConnection(mockCon, "tarantool://0:0", new Properties()), "TEST"); + + Exception ex = new CommunicationException("TEST"); + doThrow(ex).when(mockCon).sql("TEST", new Object[0]); + doThrow(ex).when(mockCon).update("TEST"); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + consumer.accept(prep); + } + }); + assertTrue(e.getMessage().startsWith("Failed to execute"), e.getMessage()); + + assertEquals(ex, e.getCause()); + + verify(mockCon, times(1)).close(); + } + + private void checkDatabaseMetaDataCommunicationException(final ThrowingConsumer<DatabaseMetaData> consumer, + String msg) throws SQLException { + TestTarantoolConnection mockCon = mock(TestTarantoolConnection.class); + SQLConnection conn = new SQLConnection(mockCon, "tarantool://0:0", new Properties()); + final DatabaseMetaData meta = conn.getMetaData(); + + Exception ex = new CommunicationException("TEST"); + doThrow(ex).when(mockCon).select(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); + doThrow(ex).when(mockCon).select(_VSPACE, 2, Arrays.asList("TEST"), 0, 1, 0); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + consumer.accept(meta); + } + }); + assertTrue(e.getMessage().startsWith(msg), e.getMessage()); + + assertEquals(ex, e.getCause()); + + verify(mockCon, times(1)).close(); + } + + class TestTarantoolConnection extends TarantoolConnection { + TestTarantoolConnection() throws IOException { + super(null, null, mock(Socket.class)); + } + @Override + protected void sql(String sql, Object[] bind) { + super.sql(sql, bind); + } + } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java b/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java index f356f6b..3976d5b 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java @@ -2,6 +2,7 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.function.Executable; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -10,7 +11,10 @@ import java.sql.SQLException; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; public class JdbcPreparedStatementIT extends AbstractJdbcIT { private PreparedStatement prep; @@ -64,4 +68,94 @@ public class JdbcPreparedStatementIT extends AbstractJdbcIT { assertEquals("thousand", getRow("test", 1000).get(1)); } + + @Test + public void testExecuteReturnsResultSet() throws SQLException { + prep = conn.prepareStatement("SELECT val FROM test WHERE id=?"); + assertNotNull(prep); + prep.setInt(1, 1); + + assertTrue(prep.execute()); + assertEquals(-1, prep.getUpdateCount()); + + ResultSet rs = prep.getResultSet(); + assertNotNull(rs); + assertTrue(rs.next()); + assertEquals("one", rs.getString(1)); + assertFalse(rs.next()); + rs.close(); + } + + @Test + public void testExecuteReturnsUpdateCount() throws Exception { + prep = conn.prepareStatement("INSERT INTO test VALUES(?, ?), (?, ?)"); + assertNotNull(prep); + + prep.setInt(1, 10); + prep.setString(2, "ten"); + prep.setInt(3, 20); + prep.setString(4, "twenty"); + + assertFalse(prep.execute()); + assertNull(prep.getResultSet()); + assertEquals(2, prep.getUpdateCount()); + + assertEquals("ten", getRow("test", 10).get(1)); + assertEquals("twenty", getRow("test", 20).get(1)); + } + + @Test void testForbiddenMethods() throws SQLException { + prep = conn.prepareStatement("TEST"); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: prep.executeQuery("TEST"); + break; + case 1: prep.executeUpdate("TEST"); + break; + case 2: prep.execute("TEST"); + break; + default: + fail(); + } + } + }); + assertEquals("The method cannot be called on a PreparedStatement.", e.getMessage()); + } + assertEquals(3, i); + } + + @Test + public void testClosedConnection() throws SQLException { + prep = conn.prepareStatement("TEST"); + + conn.close(); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: prep.executeQuery(); + break; + case 1: prep.executeUpdate(); + break; + case 2: prep.execute(); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getMessage()); + } + assertEquals(3, i); + } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java b/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java index 925556d..735f326 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java @@ -3,6 +3,7 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; import java.sql.ResultSet; import java.sql.SQLException; @@ -10,8 +11,10 @@ import java.sql.Statement; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; public class JdbcStatementIT extends AbstractJdbcIT { private Statement stmt; @@ -63,4 +66,31 @@ public class JdbcStatementIT extends AbstractJdbcIT { assertEquals("hundred", getRow("test", 100).get(1)); assertEquals("thousand", getRow("test", 1000).get(1)); } + + @Test + public void testClosedConnection() throws Exception { + conn.close(); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: stmt.executeQuery("TEST"); + break; + case 1: stmt.executeUpdate("TEST"); + break; + case 2: stmt.execute("TEST"); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getMessage()); + } + assertEquals(3, i); + } } \ No newline at end of file -- 1.8.3.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: add connection timeout configuration and handling 2018-10-12 15:47 [tarantool-patches] [PATCH] jdbc: add connection timeout configuration and handling Sergei Kalashnikov @ 2018-10-22 4:21 ` Alexander Turenko 2018-11-15 17:22 ` Sergei Kalashnikov 0 siblings, 1 reply; 6+ messages in thread From: Alexander Turenko @ 2018-10-22 4:21 UTC (permalink / raw) To: Sergei Kalashnikov; +Cc: tarantool-patches Hi! Thanks for your work! It is really glad to see that you paying attention to cover your changes and related code with tests. I don't insist, but if it is not hard to do it would be good to have a test that we'll get SQLException when a timeout exceeds on, say, trying to get a table metadata or execute some statement. Now we test a timeout only for the case when we try to connect. Other comments are below. WBR, Alexander Turenko. > Added connection property `socketTimeout` to allow user control over > network timeout before actual connection is returned by the driver. > This is only done for default socket provider. The default timeout is > is left to be infinite. Typo: timeout is is -> timeout is. > @@ -293,15 +299,28 @@ public class SQLConnection implements Connection { > > @Override > public void setNetworkTimeout(Executor executor, int milliseconds) throws SQLException { > - throw new SQLFeatureNotSupportedException(); > + checkNotClosed(); > + > + if (milliseconds < 0) > + throw new SQLException("Network timeout cannot be negative."); > + > + try { > + connection.setSocketTimeout(milliseconds); > + } catch (SocketException e) { > + throw new SQLException("Failed to set socket timeout: timeout=" + milliseconds, e); > + } > } The executor is not used. Found [overview][1] of the problem. Checked pgjdbc, they [do][2] the same. But mysql-connector-j [does not][3]. They need to handle some complex cases like [4] (see also [5]) because of that. To be honest I don't get Douglas's Surber explanation, but I think we likely do right things here when just ignore the executor parameter. [1]: http://mail.openjdk.java.net/pipermail/jdbc-spec-discuss/2017-November/000236.html [2]: https://github.com/pgjdbc/pgjdbc/pull/849/files#diff-1ba924d72e9b18676e312b83bc90c7e7R1484 [3]: https://github.com/mysql/mysql-connector-j/tree/fe1903b1ecb4a96a917f7ed3190d80c049b1de29/src/com/mysql/jdbc/ConnectionImpl.java#L5495 [4]: https://bugs.mysql.com/bug.php?id=75615 [5]: https://github.com/mysql/mysql-connector-j/commit/e29f2e2aa579686e1e1549ac599e2f5e8488163b > @@ -311,4 +330,28 @@ public class SQLConnection implements Connection { > public boolean isWrapperFor(Class<?> iface) throws SQLException { > throw new SQLFeatureNotSupportedException(); > } > + > + /** > + * @throws SQLException If connection is closed. > + */ > + protected void checkNotClosed() throws SQLException { > + if (isClosed()) > + throw new SQLException("Connection is closed."); > + } > + > + /** > + * Inspects passed exception and closes the connection if appropriate. > + * > + * @param e Exception to process. > + */ > + protected void handleException(Exception e) { > + if (CommunicationException.class.isAssignableFrom(e.getClass()) || > + IOException.class.isAssignableFrom(e.getClass())) { > + try { > + close(); > + } catch (SQLException ignored) { > + // No-op. > + } > + } > + } Having the protected handleException method seems to break encapsulation of the SQLConnection class (and maybe checkNotClosed too). I think it is due to using the connection field (of the TarantoolConnection type) outside of the class. Maybe we should wrap calls to connection.select and so on with protected methods of the SQLConnection class like nativeSelect and so on. And perform checkNotClosed and handleException actions inside these wrappers. What do you think? > @@ -45,22 +53,42 @@ public class SQLDriver implements Driver { > if (providerClassName != null) { > socket = getSocketFromProvider(uri, urlProperties, providerClassName); > } else { > - socket = createAndConnectDefaultSocket(urlProperties); > + // Passing the socket to allow unit tests to mock it. > + socket = connectAndSetupDefaultSocket(urlProperties, new Socket()); > } > try { > - TarantoolConnection connection = new TarantoolConnection(urlProperties.getProperty(PROP_USER), urlProperties.getProperty(PROP_PASSWORD), socket) {{ > + TarantoolConnection connection = new TarantoolConnection( > + urlProperties.getProperty(PROP_USER), > + urlProperties.getProperty(PROP_PASSWORD), > + socket) {{ > msgPackLite = SQLMsgPackLite.INSTANCE; > }}; > > - return new SQLConnection(connection, url, info); > - } catch (IOException e) { > - throw new SQLException("Couldn't initiate connection. Provider class name is " + providerClassName, e); > + return new SQLConnection(connection, url, urlProperties); > + } catch (Exception e) { > + try { > + socket.close(); > + } catch (IOException ignored) { > + // No-op. > + } > + throw new SQLException("Couldn't initiate connection using " + diagProperties(urlProperties), e); > } > - > } Are we really need to work with Socket and TarantoolConnection within this class? Are we can create Socket inside TarantoolConnection and TarantoolConnection inside SQLConnection? I think it will improve encapsulation. Hope we can mock it in some less intrusive way. Are we can? Even if we'll need to pass some implementation explicitly via constructors arguments (Socket for the TarantoolConnection constructor and TarantoolConnection for the SQLConnection constructor), maybe it is better to provide a class and not an instance to encapsulate handling of possible construction exceptions inside TarantoolConnection / SQLConnection implementations? I don't very familiar with Java approaches (code patterns) to do such things, so I ask you to help me find best approach here. I don't push you to rewrite it right now (but maybe now is the good time to do so), but want to consider alternatives and, then, either plan future work or ask you to change things now (or, maybe, decide to leave things as is). > @Override > public ResultSet executeQuery() throws SQLException { > - return new SQLResultSet(JDBCBridge.query(connection, sql, getParams())); > + connection.checkNotClosed(); > + discardLastResults(); > + Object[] args = getParams(); > + try { > + return new SQLResultSet(JDBCBridge.query(connection.connection, sql, args)); > + } catch (Exception e) { > + connection.handleException(e); > + throw new SQLException(formatError(sql, args), e); > + } > } > The encapsulation concerns described above are applicable here too. > @Override > public int executeUpdate() throws SQLException { > - return JDBCBridge.update(connection, sql, getParams()); > - > + connection.checkNotClosed(); > + discardLastResults(); > + Object[] args = getParams(); > + try { > + return JDBCBridge.update(connection.connection, sql, args); > + } catch (Exception e) { > + connection.handleException(e); > + throw new SQLException(formatError(sql, args), e); > + } > } > The encapsulation concerns described above are applicable here too. > @Override > public boolean execute() throws SQLException { > - return false; > + connection.checkNotClosed(); > + discardLastResults(); > + Object[] args = getParams(); > + try { > + return handleResult(JDBCBridge.execute(connection.connection, sql, args)); > + } catch (Exception e) { > + connection.handleException(e); > + throw new SQLException(formatError(sql, args), e); > + } > } > The encapsulation concerns described above are applicable here too. I wonder also whether we can break things when call execute* methods in parallel from multiple threads? Will one execute breaks resultSet for the another? Of course it is not part of your issue, but maybe it should be handled as a separate one. > @Override > public ResultSet executeQuery(String sql) throws SQLException { > - resultSet = new SQLResultSet(JDBCBridge.query(connection, sql)); > - updateCount = -1; > - return resultSet; > + connection.checkNotClosed(); > + discardLastResults(); > + try { > + return new SQLResultSet(JDBCBridge.query(connection.connection, sql)); > + } catch (Exception e) { > + connection.handleException(e); > + throw new SQLException("Failed to execute SQL: " + sql, e); > + } > } The encapsulation concerns described above are applicable here too. > @Override > public int executeUpdate(String sql) throws SQLException { > - int update = JDBCBridge.update(connection, sql); > - resultSet = null; > - return update; > + connection.checkNotClosed(); > + discardLastResults(); > + try { > + return JDBCBridge.update(connection.connection, sql); > + } catch (Exception e) { > + connection.handleException(e); > + throw new SQLException("Failed to execute SQL: " + sql, e); > + } > } The encapsulation concerns described above are applicable here too. > @Override > public boolean execute(String sql) throws SQLException { > - Object result = JDBCBridge.execute(connection, sql); > - if (result instanceof SQLResultSet) { > - resultSet = (SQLResultSet) result; > - resultSet.maxRows = maxRows; > - updateCount = -1; > - return true; > - } else { > - resultSet = null; > - updateCount = (Integer) result; > - return false; > + connection.checkNotClosed(); > + discardLastResults(); > + try { > + return handleResult(JDBCBridge.execute(connection.connection, sql)); > + } catch (Exception e) { > + connection.handleException(e); > + throw new SQLException("Failed to execute SQL: " + sql, e); > } > } The encapsulation concerns described above are applicable here too. > + @Test > + public void testNoResponseAfterInitialConnect() throws IOException { > + ServerSocket socket = new ServerSocket(); > + socket.bind(null, 0); > + try { > + final String url = "tarantool://localhost:" + socket.getLocalPort(); > + final Properties prop = new Properties(); > + prop.setProperty(PROP_SOCKET_TIMEOUT, "3000"); Why so long? I think 100ms is enough. Tests will be run faster. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: add connection timeout configuration and handling 2018-10-22 4:21 ` [tarantool-patches] " Alexander Turenko @ 2018-11-15 17:22 ` Sergei Kalashnikov 2018-11-26 15:01 ` Alexander Turenko 0 siblings, 1 reply; 6+ messages in thread From: Sergei Kalashnikov @ 2018-11-15 17:22 UTC (permalink / raw) To: Alexander Turenko; +Cc: tarantool-patches On Mon, Oct 22, 2018 at 07:21:30AM +0300, Alexander Turenko wrote: Hi Alexander! My sincere apologies for the slow reply; Please see my answers inline below. I also included the amended patch at the very end of this email. Thank you. -- Best regards, Sergei > Hi! > > Thanks for your work! > > It is really glad to see that you paying attention to cover your changes > and related code with tests. > > I don't insist, but if it is not hard to do it would be good to have a > test that we'll get SQLException when a timeout exceeds on, say, trying > to get a table metadata or execute some statement. Now we test a timeout > only for the case when we try to connect. > > Other comments are below. > > WBR, Alexander Turenko. > > > Added connection property `socketTimeout` to allow user control over > > network timeout before actual connection is returned by the driver. > > This is only done for default socket provider. The default timeout is > > is left to be infinite. > > Typo: timeout is is -> timeout is. Fixed, thanks. > > @@ -293,15 +299,28 @@ public class SQLConnection implements Connection { > > > > @Override > > public void setNetworkTimeout(Executor executor, int milliseconds) throws SQLException { > > - throw new SQLFeatureNotSupportedException(); > > + checkNotClosed(); > > + > > + if (milliseconds < 0) > > + throw new SQLException("Network timeout cannot be negative."); > > + > > + try { > > + connection.setSocketTimeout(milliseconds); > > + } catch (SocketException e) { > > + throw new SQLException("Failed to set socket timeout: timeout=" + milliseconds, e); > > + } > > } > > The executor is not used. Found [overview][1] of the problem. Checked > pgjdbc, they [do][2] the same. But mysql-connector-j [does not][3]. They > need to handle some complex cases like [4] (see also [5]) because of > that. To be honest I don't get Douglas's Surber explanation, but I think > we likely do right things here when just ignore the executor parameter. My understanding is that executor in question was intended for providing a thread of execution that driver may use to track timeout, interrupt its own threads that perform network calls and cleanup the network resources afterwards. If the implementation can detect timeouts and cleanup resources without the use of provided executor, it may choose to do so. In my opinion, the vague wording in specification alone is not a good reason to go and implement wrapping of a mere saving of timeout value into the executor just to add ourselves a concurrency issue and then add a test for it (it is what pg did). So this is not a right thing either. > > [1]: http://mail.openjdk.java.net/pipermail/jdbc-spec-discuss/2017-November/000236.html > [2]: https://github.com/pgjdbc/pgjdbc/pull/849/files#diff-1ba924d72e9b18676e312b83bc90c7e7R1484 > [3]: https://github.com/mysql/mysql-connector-j/tree/fe1903b1ecb4a96a917f7ed3190d80c049b1de29/src/com/mysql/jdbc/ConnectionImpl.java#L5495 > [4]: https://bugs.mysql.com/bug.php?id=75615 > [5]: https://github.com/mysql/mysql-connector-j/commit/e29f2e2aa579686e1e1549ac599e2f5e8488163b > > > @@ -311,4 +330,28 @@ public class SQLConnection implements Connection { > > public boolean isWrapperFor(Class<?> iface) throws SQLException { > > throw new SQLFeatureNotSupportedException(); > > } > > + > > + /** > > + * @throws SQLException If connection is closed. > > + */ > > + protected void checkNotClosed() throws SQLException { > > + if (isClosed()) > > + throw new SQLException("Connection is closed."); > > + } > > + > > + /** > > + * Inspects passed exception and closes the connection if appropriate. > > + * > > + * @param e Exception to process. > > + */ > > + protected void handleException(Exception e) { > > + if (CommunicationException.class.isAssignableFrom(e.getClass()) || > > + IOException.class.isAssignableFrom(e.getClass())) { > > + try { > > + close(); > > + } catch (SQLException ignored) { > > + // No-op. > > + } > > + } > > + } > > Having the protected handleException method seems to break encapsulation > of the SQLConnection class (and maybe checkNotClosed too). I think it is > due to using the connection field (of the TarantoolConnection type) > outside of the class. Maybe we should wrap calls to connection.select > and so on with protected methods of the SQLConnection class like > nativeSelect and so on. And perform checkNotClosed and handleException > actions inside these wrappers. What do you think? > Yes, accesses like `connecttion.connection` must be cleaned up. I've changed the `connection` to private inside `SQLConnection` and made other refactorings including wrapping calls to TarantoolConnection methods outside of SQLConnection into new SQLConnection methods. But I tend to disagree regarding checkNotClosed(). It is useful by itself in situations when parameters passed to a function allow to return a local answer, but connection is closed. > > @@ -45,22 +53,42 @@ public class SQLDriver implements Driver { > > if (providerClassName != null) { > > socket = getSocketFromProvider(uri, urlProperties, providerClassName); > > } else { > > - socket = createAndConnectDefaultSocket(urlProperties); > > + // Passing the socket to allow unit tests to mock it. > > + socket = connectAndSetupDefaultSocket(urlProperties, new Socket()); > > } > > try { > > - TarantoolConnection connection = new TarantoolConnection(urlProperties.getProperty(PROP_USER), urlProperties.getProperty(PROP_PASSWORD), socket) {{ > > + TarantoolConnection connection = new TarantoolConnection( > > + urlProperties.getProperty(PROP_USER), > > + urlProperties.getProperty(PROP_PASSWORD), > > + socket) {{ > > msgPackLite = SQLMsgPackLite.INSTANCE; > > }}; > > > > - return new SQLConnection(connection, url, info); > > - } catch (IOException e) { > > - throw new SQLException("Couldn't initiate connection. Provider class name is " + providerClassName, e); > > + return new SQLConnection(connection, url, urlProperties); > > + } catch (Exception e) { > > + try { > > + socket.close(); > > + } catch (IOException ignored) { > > + // No-op. > > + } > > + throw new SQLException("Couldn't initiate connection using " + diagProperties(urlProperties), e); > > } > > - > > } > > Are we really need to work with Socket and TarantoolConnection within > this class? Are we can create Socket inside TarantoolConnection and > TarantoolConnection inside SQLConnection? I think it will improve > encapsulation. > > Hope we can mock it in some less intrusive way. Are we can? > > Even if we'll need to pass some implementation explicitly via > constructors arguments (Socket for the TarantoolConnection constructor > and TarantoolConnection for the SQLConnection constructor), maybe it is > better to provide a class and not an instance to encapsulate handling of > possible construction exceptions inside TarantoolConnection / > SQLConnection implementations? > > I don't very familiar with Java approaches (code patterns) to do such > things, so I ask you to help me find best approach here. > > I don't push you to rewrite it right now (but maybe now is the good time > to do so), but want to consider alternatives and, then, either plan > future work or ask you to change things now (or, maybe, decide to leave > things as is). > I made an attempt to apply your proposition. Now a socket is created within TarantoolConnection which in its turn is created inside SQLConnection. The instantiations are done with the help of provider/factory interfaces. It doesn't look particularly well, I must admit, due to the fact that we need to provide a different interface on each nesting level and adapt them. I like the idea of SQLSocketProvider accepting an URI and properties, but the TarantoolConnection is unable to pass them by itself. I guess we must think a little bit more on this. > > @Override > > public ResultSet executeQuery() throws SQLException { > > - return new SQLResultSet(JDBCBridge.query(connection, sql, getParams())); > > + connection.checkNotClosed(); > > + discardLastResults(); > > + Object[] args = getParams(); > > + try { > > + return new SQLResultSet(JDBCBridge.query(connection.connection, sql, args)); > > + } catch (Exception e) { > > + connection.handleException(e); > > + throw new SQLException(formatError(sql, args), e); > > + } > > } > > > > The encapsulation concerns described above are applicable here too. > > > @Override > > public int executeUpdate() throws SQLException { > > - return JDBCBridge.update(connection, sql, getParams()); > > - > > + connection.checkNotClosed(); > > + discardLastResults(); > > + Object[] args = getParams(); > > + try { > > + return JDBCBridge.update(connection.connection, sql, args); > > + } catch (Exception e) { > > + connection.handleException(e); > > + throw new SQLException(formatError(sql, args), e); > > + } > > } > > > > The encapsulation concerns described above are applicable here too. > > > @Override > > public boolean execute() throws SQLException { > > - return false; > > + connection.checkNotClosed(); > > + discardLastResults(); > > + Object[] args = getParams(); > > + try { > > + return handleResult(JDBCBridge.execute(connection.connection, sql, args)); > > + } catch (Exception e) { > > + connection.handleException(e); > > + throw new SQLException(formatError(sql, args), e); > > + } > > } > > > > The encapsulation concerns described above are applicable here too. > > I wonder also whether we can break things when call execute* methods in > parallel from multiple threads? Will one execute breaks resultSet for > the another? Of course it is not part of your issue, but maybe it should > be handled as a separate one. > Yes, it will break. Let us address the aspects of multi-thread usage of a connection later on. (If that will ever be requested.) > > @Override > > public ResultSet executeQuery(String sql) throws SQLException { > > - resultSet = new SQLResultSet(JDBCBridge.query(connection, sql)); > > - updateCount = -1; > > - return resultSet; > > + connection.checkNotClosed(); > > + discardLastResults(); > > + try { > > + return new SQLResultSet(JDBCBridge.query(connection.connection, sql)); > > + } catch (Exception e) { > > + connection.handleException(e); > > + throw new SQLException("Failed to execute SQL: " + sql, e); > > + } > > } > > The encapsulation concerns described above are applicable here too. > > > @Override > > public int executeUpdate(String sql) throws SQLException { > > - int update = JDBCBridge.update(connection, sql); > > - resultSet = null; > > - return update; > > + connection.checkNotClosed(); > > + discardLastResults(); > > + try { > > + return JDBCBridge.update(connection.connection, sql); > > + } catch (Exception e) { > > + connection.handleException(e); > > + throw new SQLException("Failed to execute SQL: " + sql, e); > > + } > > } > > The encapsulation concerns described above are applicable here too. > > > @Override > > public boolean execute(String sql) throws SQLException { > > - Object result = JDBCBridge.execute(connection, sql); > > - if (result instanceof SQLResultSet) { > > - resultSet = (SQLResultSet) result; > > - resultSet.maxRows = maxRows; > > - updateCount = -1; > > - return true; > > - } else { > > - resultSet = null; > > - updateCount = (Integer) result; > > - return false; > > + connection.checkNotClosed(); > > + discardLastResults(); > > + try { > > + return handleResult(JDBCBridge.execute(connection.connection, sql)); > > + } catch (Exception e) { > > + connection.handleException(e); > > + throw new SQLException("Failed to execute SQL: " + sql, e); > > } > > } > > The encapsulation concerns described above are applicable here too. > > > + @Test > > + public void testNoResponseAfterInitialConnect() throws IOException { > > + ServerSocket socket = new ServerSocket(); > > + socket.bind(null, 0); > > + try { > > + final String url = "tarantool://localhost:" + socket.getLocalPort(); > > + final Properties prop = new Properties(); > > + prop.setProperty(PROP_SOCKET_TIMEOUT, "3000"); > > Why so long? I think 100ms is enough. Tests will be run faster. Fixed that too. Please find the amended patch attached below. It is re-freshed to account for latest changes on the target branch. =============================================== Commit 8246ff160163a4ca7b61c9a8989ae1300171d7ca jdbc: add connection timeout configuration and handling Added connection property `socketTimeout` to allow user control over network timeout before actual connection is returned by the driver. This is only done for default socket provider. The default timeout is left to be infinite. Implemented `Connection.setNetworkTimeout` API to make it possible to change the maximum amount of time to wait for server replies after the connection is established. The connection that has timed out is forced to close. New subsequent operations requested on such connection must fail right away. The corresponding checks are embedded into relevant APIs. Closes #38 --- https://github.com/tarantool/tarantool-java/issues/38 https://github.com/ztarvos/tarantool-java/commits/gh-38-add-connect-timeout .../org/tarantool/TestTarantoolConnection.java | 10 +- src/main/java/org/tarantool/SocketProvider.java | 7 + .../java/org/tarantool/SocketProviderImpl.java | 24 ++ src/main/java/org/tarantool/TarantoolBase.java | 8 +- .../java/org/tarantool/TarantoolConnection.java | 47 +++- .../java/org/tarantool/jdbc/SQLConnection.java | 145 ++++++++++- .../org/tarantool/jdbc/SQLDatabaseMetadata.java | 101 ++++---- src/main/java/org/tarantool/jdbc/SQLDriver.java | 171 +++++++++---- .../org/tarantool/jdbc/SQLPreparedStatement.java | 35 ++- .../org/tarantool/jdbc/SQLSocketProviderImpl.java | 53 ++++ src/main/java/org/tarantool/jdbc/SQLStatement.java | 70 ++++-- .../java/org/tarantool/jdbc/SocketFoundry.java | 7 + .../tarantool/jdbc/TarantoolConnectionFactory.java | 8 + .../java/org/tarantool/jdbc/AbstractJdbcIT.java | 55 ++--- .../java/org/tarantool/jdbc/JdbcConnectionIT.java | 65 +++++ .../org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java | 30 +++ .../java/org/tarantool/jdbc/JdbcDriverTest.java | 271 +++++++++++++++++++++ .../tarantool/jdbc/JdbcExceptionHandlingTest.java | 176 ++++++++++++- .../tarantool/jdbc/JdbcPreparedStatementIT.java | 94 +++++++ .../java/org/tarantool/jdbc/JdbcStatementIT.java | 30 +++ 20 files changed, 1212 insertions(+), 195 deletions(-) create mode 100644 src/main/java/org/tarantool/SocketProvider.java create mode 100644 src/main/java/org/tarantool/SocketProviderImpl.java create mode 100644 src/main/java/org/tarantool/jdbc/SQLSocketProviderImpl.java create mode 100644 src/main/java/org/tarantool/jdbc/SocketFoundry.java create mode 100644 src/main/java/org/tarantool/jdbc/TarantoolConnectionFactory.java create mode 100644 src/test/java/org/tarantool/jdbc/JdbcDriverTest.java diff --git a/src/it/java/org/tarantool/TestTarantoolConnection.java b/src/it/java/org/tarantool/TestTarantoolConnection.java index 3ee2ee1..b0bab5d 100644 --- a/src/it/java/org/tarantool/TestTarantoolConnection.java +++ b/src/it/java/org/tarantool/TestTarantoolConnection.java @@ -1,16 +1,10 @@ package org.tarantool; -import java.io.IOException; -import java.net.InetSocketAddress; -import java.net.Socket; -import java.nio.channels.SocketChannel; import java.util.Collections; public class TestTarantoolConnection { - public static void main(String[] args) throws IOException { - Socket socket = new Socket(); - socket.connect(new InetSocketAddress("127.0.0.1", 3301)); - TarantoolConnection con = new TarantoolConnection("test", "test", socket); + public static void main(String[] args) { + TarantoolConnection con = new TarantoolConnection("test", "test", new SocketProviderImpl("127.0.0.1", 3301)); System.out.println(con.select(281,0, Collections.emptyList(),0,1024,0)); System.out.println(con.call("box.begin")); con.close(); diff --git a/src/main/java/org/tarantool/SocketProvider.java b/src/main/java/org/tarantool/SocketProvider.java new file mode 100644 index 0000000..205142e --- /dev/null +++ b/src/main/java/org/tarantool/SocketProvider.java @@ -0,0 +1,7 @@ +package org.tarantool; + +import java.net.Socket; + +public interface SocketProvider { + Socket getConnectedSocket(); +} diff --git a/src/main/java/org/tarantool/SocketProviderImpl.java b/src/main/java/org/tarantool/SocketProviderImpl.java new file mode 100644 index 0000000..43a2651 --- /dev/null +++ b/src/main/java/org/tarantool/SocketProviderImpl.java @@ -0,0 +1,24 @@ +package org.tarantool; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.Socket; + +public class SocketProviderImpl implements SocketProvider { + private final String host; + private final int port; + public SocketProviderImpl(String host, int port) { + this.host = host; + this.port = port; + } + @Override + public Socket getConnectedSocket() { + Socket socket = new Socket(); + try { + socket.connect(new InetSocketAddress(host, port)); + } catch (IOException e) { + throw new IllegalStateException(e); + } + return socket; + } +} diff --git a/src/main/java/org/tarantool/TarantoolBase.java b/src/main/java/org/tarantool/TarantoolBase.java index 3fb1ce4..0f4516d 100644 --- a/src/main/java/org/tarantool/TarantoolBase.java +++ b/src/main/java/org/tarantool/TarantoolBase.java @@ -37,8 +37,7 @@ public abstract class TarantoolBase<Result> extends AbstractTarantoolOps<Integer public TarantoolBase() { } - public TarantoolBase(String username, String password, Socket socket) { - super(); + public void init(String username, String password, Socket socket) { try { this.is = new DataInputStream(cis = new CountInputStreamImpl(socket.getInputStream())); byte[] bytes = new byte[64]; @@ -73,6 +72,11 @@ public abstract class TarantoolBase<Result> extends AbstractTarantoolOps<Integer } catch (IOException ignored) { } + try { + socket.close(); + } catch (IOException ignored) { + + } throw new CommunicationException("Couldn't connect to tarantool", e); } } diff --git a/src/main/java/org/tarantool/TarantoolConnection.java b/src/main/java/org/tarantool/TarantoolConnection.java index be94995..4cf5352 100644 --- a/src/main/java/org/tarantool/TarantoolConnection.java +++ b/src/main/java/org/tarantool/TarantoolConnection.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.Socket; +import java.net.SocketException; import java.nio.ByteBuffer; import java.util.List; import java.util.Map; @@ -13,12 +14,28 @@ public class TarantoolConnection extends TarantoolBase<List<?>> implements Taran protected OutputStream out; protected Socket socket; + public TarantoolConnection(String username, String password, SocketProvider socketProvider) { + if (socketProvider == null) + throw new IllegalArgumentException("socketProvider is null."); - public TarantoolConnection(String username, String password, Socket socket) throws IOException { - super(username, password, socket); - this.socket = socket; - this.out = socket.getOutputStream(); - this.in = socket.getInputStream(); + socket = socketProvider.getConnectedSocket(); + + if (socket == null) + throw new IllegalStateException("The socket provider returned null socket."); + + init(username, password, socket); + + try { + this.out = socket.getOutputStream(); + this.in = socket.getInputStream(); + } catch (IOException e) { + try { + socket.close(); + } catch (IOException ignored) { + // No-op. + } + throw new IllegalStateException(e); + } } @Override @@ -80,4 +97,24 @@ public class TarantoolConnection extends TarantoolBase<List<?>> implements Taran public boolean isClosed() { return socket.isClosed(); } + + /** + * Sets given timeout value on underlying socket. + * + * @param timeout Timeout in milliseconds. + * @throws SocketException If failed. + */ + public void setSocketTimeout(int timeout) throws SocketException { + socket.setSoTimeout(timeout); + } + + /** + * Retrieves timeout value from underlying socket. + * + * @return Timeout in milliseconds. + * @throws SocketException If failed. + */ + public int getSocketTimeout() throws SocketException { + return socket.getSoTimeout(); + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLConnection.java b/src/main/java/org/tarantool/jdbc/SQLConnection.java index de28850..a199982 100644 --- a/src/main/java/org/tarantool/jdbc/SQLConnection.java +++ b/src/main/java/org/tarantool/jdbc/SQLConnection.java @@ -1,5 +1,9 @@ package org.tarantool.jdbc; +import java.io.IOException; +import java.net.Socket; +import java.net.SocketException; +import java.net.URI; import java.sql.Array; import java.sql.Blob; import java.sql.CallableStatement; @@ -8,6 +12,7 @@ import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.NClob; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLClientInfoException; import java.sql.SQLException; import java.sql.SQLFeatureNotSupportedException; @@ -16,32 +21,60 @@ import java.sql.SQLXML; import java.sql.Savepoint; import java.sql.Statement; import java.sql.Struct; +import java.util.Arrays; +import java.util.List; import java.util.Map; import java.util.Properties; import java.util.concurrent.Executor; +import org.tarantool.CommunicationException; +import org.tarantool.JDBCBridge; +import org.tarantool.SocketProvider; import org.tarantool.TarantoolConnection; +import static org.tarantool.jdbc.SQLDriver.PROP_PASSWORD; +import static org.tarantool.jdbc.SQLDriver.PROP_USER; + @SuppressWarnings("Since15") -public class SQLConnection implements Connection { - final TarantoolConnection connection; +public class SQLConnection implements Connection, SocketProvider { + private final SQLSocketProvider sqlSocketProvider; + private final TarantoolConnection connection; final String url; final Properties properties; - public SQLConnection(TarantoolConnection connection, String url, Properties properties) { - this.connection = connection; + SQLConnection(TarantoolConnectionFactory connectionFactory, SQLSocketProvider sqlSocketProvider, String url, + Properties properties) throws SQLException { + this.sqlSocketProvider = sqlSocketProvider; this.url = url; this.properties = properties; + + try { + this.connection = connectionFactory.makeConnection( + properties.getProperty(PROP_USER), + properties.getProperty(PROP_PASSWORD), + this); + } catch (IllegalStateException e) { + throw new SQLException(e.getMessage(), e); + } catch (Exception e) { + throw new SQLException("Couldn't initiate connection using " + SQLDriver.diagProperties(properties), e); + } + } + + @Override + public Socket getConnectedSocket() { + return sqlSocketProvider.getConnectedSocket(URI.create(url), properties); } @Override public Statement createStatement() throws SQLException { - return new SQLStatement(connection, this); + checkNotClosed(); + return new SQLStatement(this); } @Override public PreparedStatement prepareStatement(String sql) throws SQLException { - return new SQLPreparedStatement(connection, this, sql); + checkNotClosed(); + return new SQLPreparedStatement(this, sql); } @Override @@ -89,6 +122,7 @@ public class SQLConnection implements Connection { @Override public DatabaseMetaData getMetaData() throws SQLException { + checkNotClosed(); return new SQLDatabaseMetadata(this); } @@ -293,15 +327,28 @@ public class SQLConnection implements Connection { @Override public void setNetworkTimeout(Executor executor, int milliseconds) throws SQLException { - throw new SQLFeatureNotSupportedException(); + checkNotClosed(); + + if (milliseconds < 0) + throw new SQLException("Network timeout cannot be negative."); + + try { + connection.setSocketTimeout(milliseconds); + } catch (SocketException e) { + throw new SQLException("Failed to set socket timeout: timeout=" + milliseconds, e); + } } @Override public int getNetworkTimeout() throws SQLException { - throw new SQLFeatureNotSupportedException(); + checkNotClosed(); + try { + return connection.getSocketTimeout(); + } catch (SocketException e) { + throw new SQLException("Failed to retrieve socket timeout", e); + } } - @Override public <T> T unwrap(Class<T> iface) throws SQLException { throw new SQLFeatureNotSupportedException(); @@ -311,4 +358,84 @@ public class SQLConnection implements Connection { public boolean isWrapperFor(Class<?> iface) throws SQLException { throw new SQLFeatureNotSupportedException(); } + + protected Object execute(String sql, Object ... args) throws SQLException { + checkNotClosed(); + try { + return JDBCBridge.execute(connection, sql, args); + } catch (Exception e) { + handleException(e); + throw new SQLException(formatError(sql, args), e); + } + } + + protected ResultSet executeQuery(String sql, Object ... args) throws SQLException { + checkNotClosed(); + try { + return new SQLResultSet(JDBCBridge.query(connection, sql, args)); + } catch (Exception e) { + handleException(e); + throw new SQLException(formatError(sql, args), e); + } + } + + protected int executeUpdate(String sql, Object ... args) throws SQLException { + checkNotClosed(); + try { + return JDBCBridge.update(connection, sql, args); + } catch (Exception e) { + handleException(e); + throw new SQLException(formatError(sql, args), e); + } + } + + protected List<?> nativeSelect(Integer space, Integer index, List<?> key, int offset, int limit, int iterator) + throws SQLException { + checkNotClosed(); + try { + return connection.select(space, index, key, offset, limit, iterator); + } catch (Exception e) { + handleException(e); + throw new SQLException(e); + } + } + + protected String getServerVersion() { + return connection.getServerVersion(); + } + + /** + * @throws SQLException If connection is closed. + */ + protected void checkNotClosed() throws SQLException { + if (isClosed()) + throw new SQLException("Connection is closed."); + } + + /** + * Inspects passed exception and closes the connection if appropriate. + * + * @param e Exception to process. + */ + private void handleException(Exception e) { + if (CommunicationException.class.isAssignableFrom(e.getClass()) || + IOException.class.isAssignableFrom(e.getClass())) { + try { + close(); + } catch (SQLException ignored) { + // No-op. + } + } + } + + /** + * Provides error message that contains parameters of failed SQL statement. + * + * @param sql SQL Text. + * @param params Parameters of the SQL statement. + * @return Formatted error message. + */ + private static String formatError(String sql, Object ... params) { + return "Failed to execute SQL: " + sql + ", params: " + Arrays.deepToString(params); + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java index c8879c9..2ddb0ef 100644 --- a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java +++ b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java @@ -105,7 +105,7 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { @Override public String getDatabaseProductVersion() throws SQLException { - return connection.connection.getServerVersion(); + return connection.getServerVersion(); } @Override @@ -672,23 +672,29 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { @Override public ResultSet getTables(String catalog, String schemaPattern, String tableNamePattern, String[] types) - throws SQLException { - if (types != null && !Arrays.asList(types).contains("TABLE")) { - return new SQLResultSet(JDBCBridge.EMPTY); - } - String[] parts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); - List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); - List<List<Object>> rows = new ArrayList<List<Object>>(); - for (List<Object> space : spaces) { - String name = (String) space.get(NAME_IDX); - Map flags = (Map) space.get(FLAGS_IDX); - if (flags != null && flags.containsKey("sql") && like(name, parts)) { - rows.add(Arrays.asList(name, "TABLE", flags.get("sql"))); + throws SQLException { + try { + if (types != null && !Arrays.asList(types).contains("TABLE")) { + connection.checkNotClosed(); + return new SQLResultSet(JDBCBridge.EMPTY); + } + String[] parts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); + List<List<Object>> spaces = (List<List<Object>>) connection.nativeSelect(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); + List<List<Object>> rows = new ArrayList<List<Object>>(); + for (List<Object> space : spaces) { + String name = (String) space.get(NAME_IDX); + Map flags = (Map) space.get(FLAGS_IDX); + if (flags != null && flags.containsKey("sql") && like(name, parts)) { + rows.add(Arrays.asList(name, "TABLE", flags.get("sql"))); + } } + return new SQLNullResultSet(JDBCBridge.mock(Arrays.asList("TABLE_NAME", "TABLE_TYPE", "REMARKS", + //nulls + "TABLE_CAT", "TABLE_SCHEM", "TABLE_TYPE", "TYPE_CAT", "TYPE_SCHEM", "TYPE_NAME", "SELF_REFERENCING_COL_NAME", "REF_GENERATION"), rows)); + } catch (Exception e) { + throw new SQLException("Failed to retrieve table(s) description: " + + "tableNamePattern=\"" + tableNamePattern + "\".", e); } - return new SQLNullResultSet(JDBCBridge.mock(Arrays.asList("TABLE_NAME", "TABLE_TYPE", "REMARKS", - //nulls - "TABLE_CAT", "TABLE_SCHEM", "TABLE_TYPE", "TYPE_CAT", "TYPE_SCHEM", "TYPE_NAME", "SELF_REFERENCING_COL_NAME", "REF_GENERATION"), rows)); } @Override @@ -713,32 +719,38 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { @Override public ResultSet getColumns(String catalog, String schemaPattern, String tableNamePattern, String columnNamePattern) throws SQLException { - String[] tableParts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); - String[] colParts = columnNamePattern == null ? new String[]{""} : columnNamePattern.split("%"); - List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); - List<List<Object>> rows = new ArrayList<List<Object>>(); - for (List<Object> space : spaces) { - String tableName = (String) space.get(NAME_IDX); - Map flags = (Map) space.get(FLAGS_IDX); - if (flags != null && flags.containsKey("sql") && like(tableName, tableParts)) { - List<Map<String, Object>> format = (List<Map<String, Object>>) space.get(FORMAT_IDX); - for (int columnIdx = 1; columnIdx <= format.size(); columnIdx++) { - Map<String, Object> f = format.get(columnIdx - 1); - String columnName = (String) f.get("name"); - String dbType = (String) f.get("type"); - if (like(columnName, colParts)) { - rows.add(Arrays.<Object>asList(tableName, columnName, columnIdx, Types.OTHER, dbType, 10, 1, "YES", Types.OTHER, "NO", "NO")); + try { + String[] tableParts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); + String[] colParts = columnNamePattern == null ? new String[]{""} : columnNamePattern.split("%"); + List<List<Object>> spaces = (List<List<Object>>) connection.nativeSelect(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); + List<List<Object>> rows = new ArrayList<List<Object>>(); + for (List<Object> space : spaces) { + String tableName = (String) space.get(NAME_IDX); + Map flags = (Map) space.get(FLAGS_IDX); + if (flags != null && flags.containsKey("sql") && like(tableName, tableParts)) { + List<Map<String, Object>> format = (List<Map<String, Object>>) space.get(FORMAT_IDX); + for (int columnIdx = 1; columnIdx <= format.size(); columnIdx++) { + Map<String, Object> f = format.get(columnIdx - 1); + String columnName = (String) f.get("name"); + String dbType = (String) f.get("type"); + if (like(columnName, colParts)) { + rows.add(Arrays.<Object>asList(tableName, columnName, columnIdx, Types.OTHER, dbType, 10, 1, "YES", Types.OTHER, "NO", "NO")); + } } } } - } - return new SQLNullResultSet((JDBCBridge.mock( - Arrays.asList("TABLE_NAME", "COLUMN_NAME", "ORDINAL_POSITION", "DATA_TYPE", "TYPE_NAME", "NUM_PREC_RADIX", "NULLABLE", "IS_NULLABLE", "SOURCE_DATA_TYPE", "IS_AUTOINCREMENT", "IS_GENERATEDCOLUMN", - //nulls - "TABLE_CAT", "TABLE_SCHEM", "COLUMN_SIZE", "BUFFER_LENGTH", "DECIMAL_DIGITS", "REMARKS", "COLUMN_DEF", "SQL_DATA_TYPE", "SQL_DATETIME_SUB", "CHAR_OCTET_LENGTH", "SCOPE_CATALOG", "SCOPE_SCHEMA", "SCOPE_TABLE" - ), - rows))); + return new SQLNullResultSet((JDBCBridge.mock( + Arrays.asList("TABLE_NAME", "COLUMN_NAME", "ORDINAL_POSITION", "DATA_TYPE", "TYPE_NAME", "NUM_PREC_RADIX", "NULLABLE", "IS_NULLABLE", "SOURCE_DATA_TYPE", "IS_AUTOINCREMENT", "IS_GENERATEDCOLUMN", + //nulls + "TABLE_CAT", "TABLE_SCHEM", "COLUMN_SIZE", "BUFFER_LENGTH", "DECIMAL_DIGITS", "REMARKS", "COLUMN_DEF", "SQL_DATA_TYPE", "SQL_DATETIME_SUB", "CHAR_OCTET_LENGTH", "SCOPE_CATALOG", "SCOPE_SCHEMA", "SCOPE_TABLE" + ), + rows))); + } catch (Exception e) { + throw new SQLException("Error processing table column metadata: " + + "tableNamePattern=\"" + tableNamePattern + "\"; " + + "columnNamePattern=\"" + columnNamePattern + "\".", e); + } } @Override @@ -769,11 +781,13 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { final List<String> colNames = Arrays.asList( "TABLE_CAT", "TABLE_SCHEM", "TABLE_NAME", "COLUMN_NAME", "KEY_SEQ", "PK_NAME"); - if (table == null || table.isEmpty()) + if (table == null || table.isEmpty()) { + connection.checkNotClosed(); return emptyResultSet(colNames); + } try { - List spaces = connection.connection.select(_VSPACE, 2, Collections.singletonList(table), 0, 1, 0); + List spaces = connection.nativeSelect(_VSPACE, 2, Collections.singletonList(table), 0, 1, 0); if (spaces == null || spaces.size() == 0) return emptyResultSet(colNames); @@ -781,7 +795,7 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { List space = ensureType(List.class, spaces.get(0)); List fields = ensureType(List.class, space.get(FORMAT_IDX)); int spaceId = ensureType(Number.class, space.get(SPACE_ID_IDX)).intValue(); - List indexes = connection.connection.select(_VINDEX, 0, Arrays.asList(spaceId, 0), 0, 1, 0); + List indexes = connection.nativeSelect(_VINDEX, 0, Arrays.asList(spaceId, 0), 0, 1, 0); List primaryKey = ensureType(List.class, indexes.get(0)); List parts = ensureType(List.class, primaryKey.get(INDEX_FORMAT_IDX)); @@ -809,9 +823,8 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { } }); return new SQLNullResultSet((JDBCBridge.mock(colNames, rows))); - } - catch (Throwable t) { - throw new SQLException("Error processing metadata for table \"" + table + "\".", t); + } catch (Exception e) { + throw new SQLException("Error processing metadata for table \"" + table + "\".", e); } } diff --git a/src/main/java/org/tarantool/jdbc/SQLDriver.java b/src/main/java/org/tarantool/jdbc/SQLDriver.java index 6867997..1c0168e 100644 --- a/src/main/java/org/tarantool/jdbc/SQLDriver.java +++ b/src/main/java/org/tarantool/jdbc/SQLDriver.java @@ -1,8 +1,8 @@ package org.tarantool.jdbc; -import java.io.IOException; -import java.net.InetSocketAddress; -import java.net.Socket; +import org.tarantool.SocketProvider; +import org.tarantool.TarantoolConnection; + import java.net.URI; import java.sql.Connection; import java.sql.Driver; @@ -14,8 +14,6 @@ import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Logger; -import org.tarantool.TarantoolConnection; - @SuppressWarnings("Since15") public class SQLDriver implements Driver { @@ -32,35 +30,61 @@ public class SQLDriver implements Driver { public static final String PROP_SOCKET_PROVIDER = "socketProvider"; public static final String PROP_USER = "user"; public static final String PROP_PASSWORD = "password"; + public static final String PROP_SOCKET_TIMEOUT = "socketTimeout"; + + // Define default values once here. + final static Properties defaults = new Properties() {{ + setProperty(PROP_HOST, "localhost"); + setProperty(PROP_PORT, "3301"); + setProperty(PROP_SOCKET_TIMEOUT, "0"); + }}; + + final static TarantoolConnectionFactory defaultConnectionFactory = new TarantoolConnectionFactory() { + @Override + public TarantoolConnection makeConnection(String user, String pass, SocketProvider provider) { + return new TarantoolConnection(user, pass, provider) {{ + msgPackLite = SQLMsgPackLite.INSTANCE; + }}; + } + }; + private final SQLSocketProvider defaultSocketProvider; + private final Map<String, SQLSocketProvider> providerCache = new ConcurrentHashMap<String, SQLSocketProvider>(); - protected Map<String, SQLSocketProvider> providerCache = new ConcurrentHashMap<String, SQLSocketProvider>(); + public SQLDriver() { + this(SQLSocketProviderImpl.INSTANCE); + } + + SQLDriver(SQLSocketProvider defaultSocketProvider) { + this.defaultSocketProvider = defaultSocketProvider; + } @Override public Connection connect(String url, Properties info) throws SQLException { URI uri = URI.create(url); Properties urlProperties = parseQueryString(uri, info); String providerClassName = urlProperties.getProperty(PROP_SOCKET_PROVIDER); - Socket socket; - if (providerClassName != null) { - socket = getSocketFromProvider(uri, urlProperties, providerClassName); - } else { - socket = createAndConnectDefaultSocket(urlProperties); - } - try { - TarantoolConnection connection = new TarantoolConnection(urlProperties.getProperty(PROP_USER), urlProperties.getProperty(PROP_PASSWORD), socket) {{ - msgPackLite = SQLMsgPackLite.INSTANCE; - }}; - return new SQLConnection(connection, url, info); - } catch (IOException e) { - throw new SQLException("Couldn't initiate connection. Provider class name is " + providerClassName, e); - } + SQLSocketProvider provider = providerClassName == null ? defaultSocketProvider : + getSocketProviderInstance(providerClassName); + return new SQLConnection(defaultConnectionFactory, provider, url, urlProperties); } - protected Properties parseQueryString(URI uri, Properties info) { - Properties urlProperties = new Properties(info); + protected Properties parseQueryString(URI uri, Properties info) throws SQLException { + Properties urlProperties = new Properties(defaults); + + String userInfo = uri.getUserInfo(); + if (userInfo != null) { + // Get user and password from the corresponding part of the URI, i.e. before @ sign. + int i = userInfo.indexOf(':'); + if (i < 0) { + urlProperties.setProperty(PROP_USER, userInfo); + } else { + urlProperties.setProperty(PROP_USER, userInfo.substring(0, i)); + urlProperties.setProperty(PROP_PASSWORD, userInfo.substring(i + 1)); + } + } if (uri.getQuery() != null) { String[] parts = uri.getQuery().split("&"); for (String part : parts) { @@ -72,44 +96,62 @@ public class SQLDriver implements Driver { } } } - urlProperties.put(PROP_HOST, uri.getHost() == null ? "localhost" : uri.getHost()); - urlProperties.put(PROP_PORT, uri.getPort() < 1 ? "3301" : uri.getPort()); - return urlProperties; - } + if (uri.getHost() != null) { + // Default values are pre-put above. + urlProperties.setProperty(PROP_HOST, uri.getHost()); + } + if (uri.getPort() >= 0) { + // We need to convert port to string otherwise getProperty() will not see it. + urlProperties.setProperty(PROP_PORT, String.valueOf(uri.getPort())); + } + if (info != null) + urlProperties.putAll(info); - protected Socket createAndConnectDefaultSocket(Properties properties) throws SQLException { - Socket socket; - socket = new Socket(); + // Validate properties. + int port; try { - socket.connect(new InetSocketAddress(properties.getProperty(PROP_HOST, "localhost"), Integer.parseInt(properties.getProperty(PROP_PORT, "3301")))); + port = Integer.parseInt(urlProperties.getProperty(PROP_PORT)); } catch (Exception e) { - throw new SQLException("Couldn't connect to tarantool using" + properties, e); + throw new SQLException("Port must be a valid number."); + } + if (port <= 0 || port > 65535) { + throw new SQLException("Port is out of range: " + port); + } + int timeout; + try { + timeout = Integer.parseInt(urlProperties.getProperty(PROP_SOCKET_TIMEOUT)); + } catch (Exception e) { + throw new SQLException("Timeout must be a valid number."); + } + if (timeout < 0) { + throw new SQLException("Timeout must not be negative."); } - return socket; + return urlProperties; } - protected Socket getSocketFromProvider(URI uri, Properties urlProperties, String providerClassName) - throws SQLException { - Socket socket; - SQLSocketProvider sqlSocketProvider = providerCache.get(providerClassName); - if (sqlSocketProvider == null) { + protected SQLSocketProvider getSocketProviderInstance(String className) throws SQLException { + SQLSocketProvider provider = providerCache.get(className); + if (provider == null) { synchronized (this) { - sqlSocketProvider = providerCache.get(providerClassName); - if (sqlSocketProvider == null) { + provider = providerCache.get(className); + if (provider == null) { try { - Class<?> cls = Class.forName(providerClassName); + Class<?> cls = Class.forName(className); if (SQLSocketProvider.class.isAssignableFrom(cls)) { - sqlSocketProvider = (SQLSocketProvider) cls.newInstance(); - providerCache.put(providerClassName, sqlSocketProvider); + provider = (SQLSocketProvider)cls.newInstance(); + providerCache.put(className, provider); } } catch (Exception e) { - throw new SQLException("Couldn't initiate socket provider " + providerClassName, e); + throw new SQLException("Couldn't instantiate socket provider: " + className, e); } } } } - socket = sqlSocketProvider.getConnectedSocket(uri, urlProperties); - return socket; + if (provider == null) { + throw new SQLException(String.format("The socket provider %s does not implement %s", + className, SQLSocketProvider.class.getCanonicalName())); + } + return provider; } @Override @@ -121,16 +163,15 @@ public class SQLDriver implements Driver { public DriverPropertyInfo[] getPropertyInfo(String url, Properties info) throws SQLException { try { URI uri = new URI(url); - Properties properties = parseQueryString(uri, new Properties(info == null ? new Properties() : info)); + Properties properties = parseQueryString(uri, info); DriverPropertyInfo host = new DriverPropertyInfo(PROP_HOST, properties.getProperty(PROP_HOST)); host.required = true; - host.description = "Tarantool sever host"; + host.description = "Tarantool server host"; DriverPropertyInfo port = new DriverPropertyInfo(PROP_PORT, properties.getProperty(PROP_PORT)); port.required = true; - port.description = "Tarantool sever port"; - + port.description = "Tarantool server port"; DriverPropertyInfo user = new DriverPropertyInfo(PROP_USER, properties.getProperty(PROP_USER)); user.required = false; @@ -140,12 +181,20 @@ public class SQLDriver implements Driver { password.required = false; password.description = "password"; - DriverPropertyInfo socketProvider = new DriverPropertyInfo(PROP_SOCKET_PROVIDER, properties.getProperty(PROP_SOCKET_PROVIDER)); + DriverPropertyInfo socketProvider = new DriverPropertyInfo( + PROP_SOCKET_PROVIDER, properties.getProperty(PROP_SOCKET_PROVIDER)); + socketProvider.required = false; socketProvider.description = "SocketProvider class implements org.tarantool.jdbc.SQLSocketProvider"; + DriverPropertyInfo socketTimeout = new DriverPropertyInfo( + PROP_SOCKET_TIMEOUT, properties.getProperty(PROP_SOCKET_TIMEOUT)); - return new DriverPropertyInfo[]{host, port, user, password, socketProvider}; + socketTimeout.required = false; + socketTimeout.description = "The number of milliseconds to wait before a timeout is occurred on a socket" + + " connect or read. The default value is 0, which means infinite timeout."; + + return new DriverPropertyInfo[]{host, port, user, password, socketProvider, socketTimeout}; } catch (Exception e) { throw new SQLException(e); } @@ -171,5 +220,23 @@ public class SQLDriver implements Driver { throw new SQLFeatureNotSupportedException(); } - + /** + * Builds a string representation of given connection properties + * along with their sanitized values. + * + * @param props Connection properties. + * @return Comma-separated pairs of property names and values. + */ + protected static String diagProperties(Properties props) { + StringBuilder sb = new StringBuilder(); + for (Map.Entry<Object, Object> e : props.entrySet()) { + if (sb.length() > 0) + sb.append(", "); + sb.append(e.getKey()); + sb.append('='); + sb.append(PROP_USER.equals(e.getKey()) || PROP_PASSWORD.equals(e.getKey()) ? + "*****" : e.getValue().toString()); + } + return sb.toString(); + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java b/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java index 23d1073..99b5137 100644 --- a/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java +++ b/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java @@ -24,23 +24,22 @@ import java.util.Calendar; import java.util.HashMap; import java.util.Map; -import org.tarantool.JDBCBridge; -import org.tarantool.TarantoolConnection; - public class SQLPreparedStatement extends SQLStatement implements PreparedStatement { + final static String INVALID_CALL_MSG = "The method cannot be called on a PreparedStatement."; final String sql; final Map<Integer, Object> params; - public SQLPreparedStatement(TarantoolConnection connection, SQLConnection sqlConnection, String sql) { - super(connection, sqlConnection); + public SQLPreparedStatement(SQLConnection connection, String sql) { + super(connection); this.sql = sql; this.params = new HashMap<Integer, Object>(); } @Override public ResultSet executeQuery() throws SQLException { - return new SQLResultSet(JDBCBridge.query(connection, sql, getParams())); + discardLastResults(); + return connection.executeQuery(sql, getParams()); } protected Object[] getParams() throws SQLException { @@ -49,7 +48,7 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem if (params.containsKey(i)) { objects[i - 1] = params.get(i); } else { - throw new SQLException("Parameter " + i + "is not"); + throw new SQLException("Parameter " + i + " is missing"); } } return objects; @@ -57,8 +56,8 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem @Override public int executeUpdate() throws SQLException { - return JDBCBridge.update(connection, sql, getParams()); - + discardLastResults(); + return connection.executeUpdate(sql, getParams()); } @Override @@ -163,7 +162,8 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem @Override public boolean execute() throws SQLException { - return false; + discardLastResults(); + return handleResult(connection.execute(sql, getParams())); } @Override @@ -336,10 +336,23 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem throw new SQLFeatureNotSupportedException(); } - @Override public void addBatch() throws SQLException { throw new SQLFeatureNotSupportedException(); } + @Override + public ResultSet executeQuery(String sql) throws SQLException { + throw new SQLException(INVALID_CALL_MSG); + } + + @Override + public int executeUpdate(String sql) throws SQLException { + throw new SQLException(INVALID_CALL_MSG); + } + + @Override + public boolean execute(String sql) throws SQLException { + throw new SQLException(INVALID_CALL_MSG); + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLSocketProviderImpl.java b/src/main/java/org/tarantool/jdbc/SQLSocketProviderImpl.java new file mode 100644 index 0000000..af146df --- /dev/null +++ b/src/main/java/org/tarantool/jdbc/SQLSocketProviderImpl.java @@ -0,0 +1,53 @@ +package org.tarantool.jdbc; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.net.SocketException; +import java.net.URI; +import java.util.Properties; + +import static org.tarantool.jdbc.SQLDriver.PROP_HOST; +import static org.tarantool.jdbc.SQLDriver.PROP_PORT; +import static org.tarantool.jdbc.SQLDriver.PROP_SOCKET_TIMEOUT; + +public class SQLSocketProviderImpl implements SQLSocketProvider { + public final static SQLSocketProvider INSTANCE = new SQLSocketProviderImpl(new SocketFoundry() { + public Socket makeSocket() { + return new Socket(); + } + }); + + private final SocketFoundry provider; + + SQLSocketProviderImpl(SocketFoundry provider) { + this.provider = provider; + } + + @Override + public Socket getConnectedSocket(URI uri, Properties properties) { + Socket socket = provider.makeSocket(); + int timeout = Integer.parseInt(properties.getProperty(PROP_SOCKET_TIMEOUT)); + String host = properties.getProperty(PROP_HOST); + int port = Integer.parseInt(properties.getProperty(PROP_PORT)); + try { + socket.connect(new InetSocketAddress(host, port), timeout); + } catch (IOException e) { + throw new IllegalStateException("Couldn't connect to " + host + ":" + port, e); + } + // Setup socket further. + if (timeout > 0) { + try { + socket.setSoTimeout(timeout); + } catch (SocketException e) { + try { + socket.close(); + } catch (IOException ignored) { + // No-op. + } + throw new IllegalStateException("Couldn't set socket timeout. timeout=" + timeout, e); + } + } + return socket; + } +} diff --git a/src/main/java/org/tarantool/jdbc/SQLStatement.java b/src/main/java/org/tarantool/jdbc/SQLStatement.java index 141ae52..3e75694 100644 --- a/src/main/java/org/tarantool/jdbc/SQLStatement.java +++ b/src/main/java/org/tarantool/jdbc/SQLStatement.java @@ -7,34 +7,27 @@ import java.sql.SQLFeatureNotSupportedException; import java.sql.SQLWarning; import java.sql.Statement; -import org.tarantool.JDBCBridge; -import org.tarantool.TarantoolConnection; - @SuppressWarnings("Since15") public class SQLStatement implements Statement { - protected final TarantoolConnection connection; - protected final SQLConnection sqlConnection; + protected final SQLConnection connection; private SQLResultSet resultSet; private int updateCount; private int maxRows; - protected SQLStatement(TarantoolConnection connection, SQLConnection sqlConnection) { - this.connection = connection; - this.sqlConnection = sqlConnection; + protected SQLStatement(SQLConnection sqlConnection) { + this.connection = sqlConnection; } @Override public ResultSet executeQuery(String sql) throws SQLException { - resultSet = new SQLResultSet(JDBCBridge.query(connection, sql)); - updateCount = -1; - return resultSet; + discardLastResults(); + return connection.executeQuery(sql); } @Override public int executeUpdate(String sql) throws SQLException { - int update = JDBCBridge.update(connection, sql); - resultSet = null; - return update; + discardLastResults(); + return connection.executeUpdate(sql); } @Override @@ -102,17 +95,8 @@ public class SQLStatement implements Statement { @Override public boolean execute(String sql) throws SQLException { - Object result = JDBCBridge.execute(connection, sql); - if (result instanceof SQLResultSet) { - resultSet = (SQLResultSet) result; - resultSet.maxRows = maxRows; - updateCount = -1; - return true; - } else { - resultSet = null; - updateCount = (Integer) result; - return false; - } + discardLastResults(); + return handleResult(connection.execute(sql)); } @Override @@ -186,7 +170,7 @@ public class SQLStatement implements Statement { @Override public Connection getConnection() throws SQLException { - return sqlConnection; + return connection; } @Override @@ -268,4 +252,38 @@ public class SQLStatement implements Statement { public boolean isWrapperFor(Class<?> iface) throws SQLException { throw new SQLFeatureNotSupportedException(); } + + /** + * Clears the results of the most recent execution. + */ + protected void discardLastResults() { + updateCount = -1; + if (resultSet != null) { + try { + resultSet.close(); + } catch (Exception ignored) { + // No-op. + } + resultSet = null; + } + } + + /** + * Sets the internals according to the result of last execution. + * + * @param result The result of SQL statement execution. + * @return {@code true}, if the result is a ResultSet object. + */ + protected boolean handleResult(Object result) { + if (result instanceof SQLResultSet) { + resultSet = (SQLResultSet) result; + resultSet.maxRows = maxRows; + updateCount = -1; + return true; + } else { + resultSet = null; + updateCount = (Integer) result; + return false; + } + } } diff --git a/src/main/java/org/tarantool/jdbc/SocketFoundry.java b/src/main/java/org/tarantool/jdbc/SocketFoundry.java new file mode 100644 index 0000000..e34bcbe --- /dev/null +++ b/src/main/java/org/tarantool/jdbc/SocketFoundry.java @@ -0,0 +1,7 @@ +package org.tarantool.jdbc; + +import java.net.Socket; + +public interface SocketFoundry { + Socket makeSocket(); +} diff --git a/src/main/java/org/tarantool/jdbc/TarantoolConnectionFactory.java b/src/main/java/org/tarantool/jdbc/TarantoolConnectionFactory.java new file mode 100644 index 0000000..8dcd2c6 --- /dev/null +++ b/src/main/java/org/tarantool/jdbc/TarantoolConnectionFactory.java @@ -0,0 +1,8 @@ +package org.tarantool.jdbc; + +import org.tarantool.SocketProvider; +import org.tarantool.TarantoolConnection; + +public interface TarantoolConnectionFactory { + TarantoolConnection makeConnection(String user, String pass, SocketProvider provider); +} diff --git a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java index 3df1aa4..5f32874 100644 --- a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java +++ b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java @@ -4,13 +4,12 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; +import org.tarantool.SocketProvider; +import org.tarantool.SocketProviderImpl; import org.tarantool.TarantoolConnection; -import java.io.IOException; import java.math.BigDecimal; import java.math.BigInteger; -import java.net.InetSocketAddress; -import java.net.Socket; import java.sql.Connection; import java.sql.Date; import java.sql.DriverManager; @@ -31,6 +30,8 @@ public abstract class AbstractJdbcIT { private static final String pass = System.getProperty("tntPass", "4pWBZmLEgkmKK5WP"); private static String URL = String.format("tarantool://%s:%d?user=%s&password=%s", host, port, user, pass); + private static final SocketProvider testSocketProvider = new SocketProviderImpl(host, port); + private static String[] initSql = new String[] { "CREATE TABLE test(id INT PRIMARY KEY, val VARCHAR(100))", "INSERT INTO test VALUES (1, 'one'), (2, 'two'), (3, 'three')", @@ -133,45 +134,25 @@ public abstract class AbstractJdbcIT { conn.close(); } - private static void sqlExec(String[] text) throws IOException { - Socket socket = new Socket(); + private static void sqlExec(String[] text) { + TarantoolConnection con = new TarantoolConnection(user, pass, testSocketProvider); try { - socket.connect(new InetSocketAddress(host, port)); - TarantoolConnection con = new TarantoolConnection(user, pass, socket); - try { - for (String cmd : text) - con.eval("box.sql.execute(\"" + cmd + "\")"); - } - finally { - con.close(); - socket = null; - } - } - finally { - if (socket != null) - socket.close(); + for (String cmd : text) + con.eval("box.sql.execute(\"" + cmd + "\")"); + } finally { + con.close(); } } - static List<?> getRow(String space, Object key) throws IOException { - Socket socket = new Socket(); + static List<?> getRow(String space, Object key) { + TarantoolConnection con = new TarantoolConnection(user, pass, testSocketProvider); try { - socket.connect(new InetSocketAddress(host, port)); - TarantoolConnection con = new TarantoolConnection(user, pass, socket); - try { - List<?> l = con.select(281, 2, Arrays.asList(space.toUpperCase()), 0, 1, 0); - Integer spaceId = (Integer) ((List) l.get(0)).get(0); - l = con.select(spaceId, 0, Arrays.asList(key), 0, 1, 0); - return (l == null || l.size() == 0) ? Collections.emptyList() : (List<?>) l.get(0); - } - finally { - con.close(); - socket = null; - } - } - finally { - if (socket != null) - socket.close(); + List<?> l = con.select(281, 2, Arrays.asList(space.toUpperCase()), 0, 1, 0); + Integer spaceId = (Integer) ((List) l.get(0)).get(0); + l = con.select(spaceId, 0, Arrays.asList(key), 0, 1, 0); + return (l == null || l.size() == 0) ? Collections.emptyList() : (List<?>) l.get(0); + } finally { + con.close(); } } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java b/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java index cc6bfb9..a6a05e8 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java @@ -1,16 +1,24 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import org.tarantool.TarantoolConnection; +import java.lang.reflect.Field; +import java.net.Socket; import java.sql.DatabaseMetaData; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Statement; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +@SuppressWarnings("Since15") public class JdbcConnectionIT extends AbstractJdbcIT { @Test public void testCreateStatement() throws SQLException { @@ -39,4 +47,61 @@ public class JdbcConnectionIT extends AbstractJdbcIT { DatabaseMetaData meta = conn.getMetaData(); assertNotNull(meta); } + + @Test + public void testGetSetNetworkTimeout() throws Exception { + assertEquals(0, conn.getNetworkTimeout()); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + conn.setNetworkTimeout(null, -1); + } + }); + assertEquals("Network timeout cannot be negative.", e.getMessage()); + + conn.setNetworkTimeout(null, 3000); + + assertEquals(3000, conn.getNetworkTimeout()); + + // Check that timeout gets propagated to the socket. + Field tntCon = SQLConnection.class.getDeclaredField("connection"); + tntCon.setAccessible(true); + + Field sock = TarantoolConnection.class.getDeclaredField("socket"); + sock.setAccessible(true); + + assertEquals(3000, ((Socket)sock.get(tntCon.get(conn))).getSoTimeout()); + } + + @Test + public void testClosedConnection() throws SQLException { + conn.close(); + + int i = 0; + for (; i < 5; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: conn.createStatement(); + break; + case 1: conn.prepareStatement("TEST"); + break; + case 2: conn.getMetaData(); + break; + case 3: conn.getNetworkTimeout(); + break; + case 4: conn.setNetworkTimeout(null, 1000); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getMessage()); + } + assertEquals(5, i); + } } \ No newline at end of file diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java index 17ab086..76caa19 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java @@ -2,6 +2,7 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.function.Executable; import java.sql.DatabaseMetaData; import java.sql.ResultSet; @@ -12,7 +13,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT { private DatabaseMetaData meta; @@ -183,4 +186,31 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT { assertEquals(seq, rs.getInt(5)); assertEquals(pkName, rs.getString(6)); } + + @Test + public void testClosedConnection() throws SQLException { + conn.close(); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: meta.getTables(null, null, null, new String[]{"TABLE"}); + break; + case 1: meta.getColumns(null, null, "TEST", null); + break; + case 2: meta.getPrimaryKeys(null, null, "TEST"); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getCause().getMessage()); + } + assertEquals(3, i); + } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcDriverTest.java b/src/test/java/org/tarantool/jdbc/JdbcDriverTest.java new file mode 100644 index 0000000..70e28cb --- /dev/null +++ b/src/test/java/org/tarantool/jdbc/JdbcDriverTest.java @@ -0,0 +1,271 @@ +package org.tarantool.jdbc; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import org.tarantool.CommunicationException; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.net.SocketException; +import java.net.SocketTimeoutException; +import java.net.URI; + +import java.sql.Driver; +import java.sql.DriverManager; +import java.sql.DriverPropertyInfo; +import java.sql.SQLException; +import java.util.Properties; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.tarantool.jdbc.SQLDriver.PROP_HOST; +import static org.tarantool.jdbc.SQLDriver.PROP_PASSWORD; +import static org.tarantool.jdbc.SQLDriver.PROP_PORT; +import static org.tarantool.jdbc.SQLDriver.PROP_SOCKET_PROVIDER; +import static org.tarantool.jdbc.SQLDriver.PROP_SOCKET_TIMEOUT; +import static org.tarantool.jdbc.SQLDriver.PROP_USER; + +public class JdbcDriverTest { + @Test + public void testParseQueryString() throws Exception { + SQLDriver drv = new SQLDriver(); + + Properties prop = new Properties(); + prop.setProperty(PROP_USER, "adm"); + prop.setProperty(PROP_PASSWORD, "secret"); + + URI uri = new URI(String.format( + "tarantool://server.local:3302?%s=%s&%s=%d", + PROP_SOCKET_PROVIDER, "some.class", + PROP_SOCKET_TIMEOUT, 5000)); + + Properties res = drv.parseQueryString(uri, prop); + assertNotNull(res); + + assertEquals("server.local", res.getProperty(PROP_HOST)); + assertEquals("3302", res.getProperty(PROP_PORT)); + assertEquals("adm", res.getProperty(PROP_USER)); + assertEquals("secret", res.getProperty(PROP_PASSWORD)); + assertEquals("some.class", res.getProperty(PROP_SOCKET_PROVIDER)); + assertEquals("5000", res.getProperty(PROP_SOCKET_TIMEOUT)); + } + + @Test + public void testParseQueryStringUserInfoInURI() throws Exception { + SQLDriver drv = new SQLDriver(); + Properties res = drv.parseQueryString(new URI("tarantool://adm:secret@server.local"), null); + assertNotNull(res); + assertEquals("server.local", res.getProperty(PROP_HOST)); + assertEquals("3301", res.getProperty(PROP_PORT)); + assertEquals("adm", res.getProperty(PROP_USER)); + assertEquals("secret", res.getProperty(PROP_PASSWORD)); + } + + @Test + public void testParseQueryStringValidations() { + // Check non-number port + checkParseQueryStringValidation("tarantool://0", + new Properties() {{setProperty(PROP_PORT, "nan");}}, + "Port must be a valid number."); + + // Check zero port + checkParseQueryStringValidation("tarantool://0:0", null, "Port is out of range: 0"); + + // Check high port + checkParseQueryStringValidation("tarantool://0:65536", null, "Port is out of range: 65536"); + + // Check non-number timeout + checkParseQueryStringValidation(String.format("tarantool://0:3301?%s=nan", PROP_SOCKET_TIMEOUT), null, + "Timeout must be a valid number."); + + // Check negative timeout + checkParseQueryStringValidation(String.format("tarantool://0:3301?%s=-100", PROP_SOCKET_TIMEOUT), null, + "Timeout must not be negative."); + } + + @Test + public void testGetPropertyInfo() throws SQLException { + Driver drv = new SQLDriver(); + Properties props = new Properties(); + DriverPropertyInfo[] info = drv.getPropertyInfo("tarantool://server.local:3302", props); + assertNotNull(info); + + for (DriverPropertyInfo e: info) { + assertNotNull(e.name); + assertNull(e.choices); + assertNotNull(e.description); + + if (PROP_HOST.equals(e.name)) { + assertTrue(e.required); + assertEquals("server.local", e.value); + } else if (PROP_PORT.equals(e.name)) { + assertTrue(e.required); + assertEquals("3302", e.value); + } else if (PROP_USER.equals(e.name)) { + assertFalse(e.required); + assertNull(e.value); + } else if (PROP_PASSWORD.equals(e.name)) { + assertFalse(e.required); + assertNull(e.value); + } else if (PROP_SOCKET_PROVIDER.equals(e.name)) { + assertFalse(e.required); + assertNull(e.value); + } else if (PROP_SOCKET_TIMEOUT.equals(e.name)) { + assertFalse(e.required); + assertEquals("0", e.value); + } else + fail("Unknown property '" + e.name + "'"); + } + } + + @Test + public void testDefaultSocketProviderConnectTimeoutError() throws IOException { + final int socketTimeout = 3000; + final Socket mockSocket = mock(Socket.class); + final SQLDriver drv = buildTestSQLDriver(mockSocket); + + SocketTimeoutException timeoutEx = new SocketTimeoutException(); + doThrow(timeoutEx) + .when(mockSocket) + .connect(new InetSocketAddress("localhost", 3301), socketTimeout); + + final Properties prop = new Properties(); + prop.setProperty(PROP_SOCKET_TIMEOUT, String.valueOf(socketTimeout)); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + drv.connect("tarantool://localhost:3301", prop); + } + }); + + assertTrue(e.getMessage().startsWith("Couldn't connect to localhost:3301"), e.getMessage()); + assertEquals(timeoutEx, e.getCause().getCause()); + } + + @Test + public void testDefaultSocketProviderSetSocketTimeoutError() throws IOException { + final int socketTimeout = 3000; + final Socket mockSocket = mock(Socket.class); + final SQLDriver drv = buildTestSQLDriver(mockSocket); + + // Check error setting socket timeout + reset(mockSocket); + doNothing() + .when(mockSocket) + .connect(new InetSocketAddress("localhost", 3301), socketTimeout); + + SocketException sockEx = new SocketException("TEST"); + doThrow(sockEx) + .when(mockSocket) + .setSoTimeout(socketTimeout); + + final Properties prop = new Properties(); + prop.setProperty(PROP_SOCKET_TIMEOUT, String.valueOf(socketTimeout)); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + drv.connect("tarantool://localhost:3301", prop); + } + }); + + assertTrue(e.getMessage().startsWith("Couldn't set socket timeout."), e.getMessage()); + assertEquals(sockEx, e.getCause().getCause()); + } + + @Test + public void testCustomSocketProviderFail() throws SQLException { + checkCustomSocketProviderFail("nosuchclassexists", + "Couldn't instantiate socket provider"); + + checkCustomSocketProviderFail(Integer.class.getName(), + "The socket provider java.lang.Integer does not implement org.tarantool.jdbc.SQLSocketProvider"); + + checkCustomSocketProviderFail(TestSQLProviderThatReturnsNull.class.getName(), + "The socket provider returned null socket"); + + checkCustomSocketProviderFail(TestSQLProviderThatThrows.class.getName(), + "Couldn't initiate connection using"); + } + + @Test + public void testNoResponseAfterInitialConnect() throws IOException { + ServerSocket socket = new ServerSocket(); + socket.bind(null, 0); + try { + final String url = "tarantool://localhost:" + socket.getLocalPort(); + final Properties prop = new Properties(); + prop.setProperty(PROP_SOCKET_TIMEOUT, "100"); + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + DriverManager.getConnection(url, prop); + } + }); + assertTrue(e.getMessage().startsWith("Couldn't initiate connection using "), e.getMessage()); + assertTrue(e.getCause() instanceof CommunicationException); + assertTrue(e.getCause().getCause() instanceof SocketTimeoutException); + } finally { + socket.close(); + } + } + + private void checkCustomSocketProviderFail(String providerClassName, String errMsg) throws SQLException { + final Driver drv = DriverManager.getDriver("tarantool:"); + final Properties prop = new Properties(); + prop.setProperty(PROP_SOCKET_PROVIDER, providerClassName); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + drv.connect("tarantool://0:3301", prop); + } + }); + assertTrue(e.getMessage().startsWith(errMsg), e.getMessage()); + } + + private void checkParseQueryStringValidation(final String uri, final Properties prop, String errMsg) { + final SQLDriver drv = new SQLDriver(); + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + drv.parseQueryString(new URI(uri), prop); + } + }); + assertTrue(e.getMessage().startsWith(errMsg), e.getMessage()); + } + + private SQLDriver buildTestSQLDriver(final Socket socket) { + return new SQLDriver(new SQLSocketProviderImpl(new SocketFoundry() { + public Socket makeSocket() { + return socket; + } + })); + } + + static class TestSQLProviderThatReturnsNull implements SQLSocketProvider { + @Override + public Socket getConnectedSocket(URI uri, Properties params) { + return null; + } + } + + static class TestSQLProviderThatThrows implements SQLSocketProvider { + @Override + public Socket getConnectedSocket(URI uri, Properties params) { + throw new RuntimeException("ERROR"); + } + } +} diff --git a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java index 8cc7acc..b1c8231 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java +++ b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java @@ -2,22 +2,33 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.api.function.ThrowingConsumer; +import org.tarantool.CommunicationException; +import org.tarantool.SocketProvider; import org.tarantool.TarantoolConnection; +import java.net.Socket; import java.sql.DatabaseMetaData; +import java.sql.PreparedStatement; import java.sql.SQLException; +import java.sql.Statement; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Properties; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.tarantool.jdbc.SQLDatabaseMetadata.FORMAT_IDX; import static org.tarantool.jdbc.SQLDatabaseMetadata.INDEX_FORMAT_IDX; import static org.tarantool.jdbc.SQLDatabaseMetadata.SPACE_ID_IDX; +import static org.tarantool.jdbc.SQLDatabaseMetadata.SPACES_MAX; import static org.tarantool.jdbc.SQLDatabaseMetadata._VINDEX; import static org.tarantool.jdbc.SQLDatabaseMetadata._VSPACE; @@ -30,7 +41,7 @@ public class JdbcExceptionHandlingTest { @Test public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws SQLException { TarantoolConnection tntCon = mock(TarantoolConnection.class); - SQLConnection conn = new SQLConnection(tntCon, "", new Properties()); + SQLConnection conn = buildTestSQLConnection(tntCon, "", new Properties()); Object[] spc = new Object[7]; spc[FORMAT_IDX] = Collections.singletonList(new HashMap<String, Object>()); @@ -57,4 +68,167 @@ public class JdbcExceptionHandlingTest { assertTrue(t.getCause().getMessage().contains("Wrong value type")); } + + @Test + public void testStatementCommunicationException() throws SQLException { + checkStatementCommunicationException(new ThrowingConsumer<Statement>() { + @Override + public void accept(Statement statement) throws Throwable { + statement.executeQuery("TEST"); + } + }); + checkStatementCommunicationException(new ThrowingConsumer<Statement>() { + @Override + public void accept(Statement statement) throws Throwable { + statement.executeUpdate("TEST"); + } + }); + checkStatementCommunicationException(new ThrowingConsumer<Statement>() { + @Override + public void accept(Statement statement) throws Throwable { + statement.execute("TEST"); + } + }); + } + + @Test + public void testPreparedStatementCommunicationException() throws SQLException { + checkPreparedStatementCommunicationException(new ThrowingConsumer<PreparedStatement>() { + @Override + public void accept(PreparedStatement prep) throws Throwable { + prep.executeQuery(); + } + }); + checkPreparedStatementCommunicationException(new ThrowingConsumer<PreparedStatement>() { + @Override + public void accept(PreparedStatement prep) throws Throwable { + prep.executeUpdate(); + } + }); + checkPreparedStatementCommunicationException(new ThrowingConsumer<PreparedStatement>() { + @Override + public void accept(PreparedStatement prep) throws Throwable { + prep.execute(); + } + }); + } + + @Test + public void testDatabaseMetaDataCommunicationException() throws SQLException { + checkDatabaseMetaDataCommunicationException(new ThrowingConsumer<DatabaseMetaData>() { + @Override + public void accept(DatabaseMetaData meta) throws Throwable { + meta.getTables(null, null, null, new String[] {"TABLE"}); + } + }, "Failed to retrieve table(s) description: tableNamePattern=\"null\"."); + + checkDatabaseMetaDataCommunicationException(new ThrowingConsumer<DatabaseMetaData>() { + @Override + public void accept(DatabaseMetaData meta) throws Throwable { + meta.getColumns(null, null, "TEST", "ID"); + } + }, "Error processing table column metadata: tableNamePattern=\"TEST\"; columnNamePattern=\"ID\"."); + + checkDatabaseMetaDataCommunicationException(new ThrowingConsumer<DatabaseMetaData>() { + @Override + public void accept(DatabaseMetaData meta) throws Throwable { + meta.getPrimaryKeys(null, null, "TEST"); + } + }, "Error processing metadata for table \"TEST\"."); + } + + private void checkStatementCommunicationException(final ThrowingConsumer<Statement> consumer) + throws SQLException { + TestTarantoolConnection mockCon = mock(TestTarantoolConnection.class); + final Statement stmt = new SQLStatement(buildTestSQLConnection(mockCon, "tarantool://0:0", new Properties())); + + Exception ex = new CommunicationException("TEST"); + + doThrow(ex).when(mockCon).sql("TEST", new Object[0]); + doThrow(ex).when(mockCon).update("TEST"); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + consumer.accept(stmt); + } + }); + assertTrue(e.getMessage().startsWith("Failed to execute"), e.getMessage()); + + assertEquals(ex, e.getCause()); + + verify(mockCon, times(1)).close(); + } + + private void checkPreparedStatementCommunicationException(final ThrowingConsumer<PreparedStatement> consumer) + throws SQLException { + TestTarantoolConnection mockCon = mock(TestTarantoolConnection.class); + + final PreparedStatement prep = new SQLPreparedStatement( + buildTestSQLConnection(mockCon, "tarantool://0:0", new Properties()), "TEST"); + + Exception ex = new CommunicationException("TEST"); + doThrow(ex).when(mockCon).sql("TEST", new Object[0]); + doThrow(ex).when(mockCon).update("TEST"); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + consumer.accept(prep); + } + }); + assertTrue(e.getMessage().startsWith("Failed to execute"), e.getMessage()); + + assertEquals(ex, e.getCause()); + + verify(mockCon, times(1)).close(); + } + + private void checkDatabaseMetaDataCommunicationException(final ThrowingConsumer<DatabaseMetaData> consumer, + String msg) throws SQLException { + TestTarantoolConnection mockCon = mock(TestTarantoolConnection.class); + SQLConnection conn = buildTestSQLConnection(mockCon, "tarantool://0:0", new Properties()); + final DatabaseMetaData meta = conn.getMetaData(); + + Exception ex = new CommunicationException("TEST"); + doThrow(ex).when(mockCon).select(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); + doThrow(ex).when(mockCon).select(_VSPACE, 2, Arrays.asList("TEST"), 0, 1, 0); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + consumer.accept(meta); + } + }); + assertTrue(e.getMessage().startsWith(msg), e.getMessage()); + + assertEquals(ex, e.getCause().getCause()); + + verify(mockCon, times(1)).close(); + } + + private SQLConnection buildTestSQLConnection(final TarantoolConnection tntCon, String url, Properties properties) + throws SQLException { + return new SQLConnection(new TarantoolConnectionFactory() { + @Override + public TarantoolConnection makeConnection(String user, String pass, SocketProvider provider) { + return tntCon; + } + }, null, url, properties); + } + + class TestTarantoolConnection extends TarantoolConnection { + TestTarantoolConnection() { + super(null, null, new SocketProvider() { + @Override + public Socket getConnectedSocket() { + return mock(Socket.class); + } + }); + } + @Override + protected void sql(String sql, Object[] bind) { + super.sql(sql, bind); + } + } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java b/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java index 68628ef..ad8fae7 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java @@ -2,6 +2,7 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.function.Executable; import java.math.BigDecimal; import java.sql.Date; @@ -14,7 +15,10 @@ import java.sql.Timestamp; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; public class JdbcPreparedStatementIT extends AbstractJdbcIT { private PreparedStatement prep; @@ -130,4 +134,94 @@ public class JdbcPreparedStatementIT extends AbstractJdbcIT { rs.close(); } + + @Test + public void testExecuteReturnsResultSet() throws SQLException { + prep = conn.prepareStatement("SELECT val FROM test WHERE id=?"); + assertNotNull(prep); + prep.setInt(1, 1); + + assertTrue(prep.execute()); + assertEquals(-1, prep.getUpdateCount()); + + ResultSet rs = prep.getResultSet(); + assertNotNull(rs); + assertTrue(rs.next()); + assertEquals("one", rs.getString(1)); + assertFalse(rs.next()); + rs.close(); + } + + @Test + public void testExecuteReturnsUpdateCount() throws Exception { + prep = conn.prepareStatement("INSERT INTO test VALUES(?, ?), (?, ?)"); + assertNotNull(prep); + + prep.setInt(1, 10); + prep.setString(2, "ten"); + prep.setInt(3, 20); + prep.setString(4, "twenty"); + + assertFalse(prep.execute()); + assertNull(prep.getResultSet()); + assertEquals(2, prep.getUpdateCount()); + + assertEquals("ten", getRow("test", 10).get(1)); + assertEquals("twenty", getRow("test", 20).get(1)); + } + + @Test void testForbiddenMethods() throws SQLException { + prep = conn.prepareStatement("TEST"); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: prep.executeQuery("TEST"); + break; + case 1: prep.executeUpdate("TEST"); + break; + case 2: prep.execute("TEST"); + break; + default: + fail(); + } + } + }); + assertEquals("The method cannot be called on a PreparedStatement.", e.getMessage()); + } + assertEquals(3, i); + } + + @Test + public void testClosedConnection() throws SQLException { + prep = conn.prepareStatement("TEST"); + + conn.close(); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: prep.executeQuery(); + break; + case 1: prep.executeUpdate(); + break; + case 2: prep.execute(); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getMessage()); + } + assertEquals(3, i); + } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java b/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java index 925556d..735f326 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java @@ -3,6 +3,7 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; import java.sql.ResultSet; import java.sql.SQLException; @@ -10,8 +11,10 @@ import java.sql.Statement; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; public class JdbcStatementIT extends AbstractJdbcIT { private Statement stmt; @@ -63,4 +66,31 @@ public class JdbcStatementIT extends AbstractJdbcIT { assertEquals("hundred", getRow("test", 100).get(1)); assertEquals("thousand", getRow("test", 1000).get(1)); } + + @Test + public void testClosedConnection() throws Exception { + conn.close(); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: stmt.executeQuery("TEST"); + break; + case 1: stmt.executeUpdate("TEST"); + break; + case 2: stmt.execute("TEST"); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getMessage()); + } + assertEquals(3, i); + } } \ No newline at end of file -- 1.8.3.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: add connection timeout configuration and handling 2018-11-15 17:22 ` Sergei Kalashnikov @ 2018-11-26 15:01 ` Alexander Turenko 2018-12-05 11:16 ` Sergei Kalashnikov 0 siblings, 1 reply; 6+ messages in thread From: Alexander Turenko @ 2018-11-26 15:01 UTC (permalink / raw) To: Sergei Kalashnikov; +Cc: tarantool-patches On Thu, Nov 15, 2018 at 08:22:09PM +0300, Sergei Kalashnikov wrote: > On Mon, Oct 22, 2018 at 07:21:30AM +0300, Alexander Turenko wrote: > > Hi Alexander! > > My sincere apologies for the slow reply; Please see my answers inline below. > I also included the amended patch at the very end of this email. > > Thank you. > -- > Best regards, > Sergei Sorry for the slow reply too. It is hard for me to immerse enough to the code to make a review useful... I'm okay with tests and mostly okay with the patch. The most important thing is that (I think) we should not break TarantoolConnection API. Other notes are minor and are matters of opinion. I think we should split features and refactoring in the future, because it is hard to understand and review many different (not coupled) changes. It is about future patches. My comments are below. WBR, Alexander Turenko. > > > @@ -293,15 +299,28 @@ public class SQLConnection implements Connection { > > > > > > @Override > > > public void setNetworkTimeout(Executor executor, int milliseconds) throws SQLException { > > > - throw new SQLFeatureNotSupportedException(); > > > + checkNotClosed(); > > > + > > > + if (milliseconds < 0) > > > + throw new SQLException("Network timeout cannot be negative."); > > > + > > > + try { > > > + connection.setSocketTimeout(milliseconds); > > > + } catch (SocketException e) { > > > + throw new SQLException("Failed to set socket timeout: timeout=" + milliseconds, e); > > > + } > > > } > > > > The executor is not used. Found [overview][1] of the problem. Checked > > pgjdbc, they [do][2] the same. But mysql-connector-j [does not][3]. They > > need to handle some complex cases like [4] (see also [5]) because of > > that. To be honest I don't get Douglas's Surber explanation, but I think > > we likely do right things here when just ignore the executor parameter. > > My understanding is that executor in question was intended for providing a thread of > execution that driver may use to track timeout, interrupt its own threads that perform > network calls and cleanup the network resources afterwards. If the implementation can detect > timeouts and cleanup resources without the use of provided executor, it may choose to do so. > > In my opinion, the vague wording in specification alone is not a good reason to go and > implement wrapping of a mere saving of timeout value into the executor just to add ourselves > a concurrency issue and then add a test for it (it is what pg did). > So this is not a right thing either. > So we are on the one side. Great. > > > @@ -311,4 +330,28 @@ public class SQLConnection implements Connection { > > > public boolean isWrapperFor(Class<?> iface) throws SQLException { > > > throw new SQLFeatureNotSupportedException(); > > > } > > > + > > > + /** > > > + * @throws SQLException If connection is closed. > > > + */ > > > + protected void checkNotClosed() throws SQLException { > > > + if (isClosed()) > > > + throw new SQLException("Connection is closed."); > > > + } > > > + > > > + /** > > > + * Inspects passed exception and closes the connection if appropriate. > > > + * > > > + * @param e Exception to process. > > > + */ > > > + protected void handleException(Exception e) { > > > + if (CommunicationException.class.isAssignableFrom(e.getClass()) || > > > + IOException.class.isAssignableFrom(e.getClass())) { > > > + try { > > > + close(); > > > + } catch (SQLException ignored) { > > > + // No-op. > > > + } > > > + } > > > + } > > > > Having the protected handleException method seems to break encapsulation > > of the SQLConnection class (and maybe checkNotClosed too). I think it is > > due to using the connection field (of the TarantoolConnection type) > > outside of the class. Maybe we should wrap calls to connection.select > > and so on with protected methods of the SQLConnection class like > > nativeSelect and so on. And perform checkNotClosed and handleException > > actions inside these wrappers. What do you think? > > > > Yes, accesses like `connecttion.connection` must be cleaned up. > I've changed the `connection` to private inside `SQLConnection` and made other > refactorings including wrapping calls to TarantoolConnection methods > outside of SQLConnection into new SQLConnection methods. But I tend to > disagree regarding checkNotClosed(). It is useful by itself in situations > when parameters passed to a function allow to return a local answer, > but connection is closed. Looks good. > > > @@ -45,22 +53,42 @@ public class SQLDriver implements Driver { > > > if (providerClassName != null) { > > > socket = getSocketFromProvider(uri, urlProperties, providerClassName); > > > } else { > > > - socket = createAndConnectDefaultSocket(urlProperties); > > > + // Passing the socket to allow unit tests to mock it. > > > + socket = connectAndSetupDefaultSocket(urlProperties, new Socket()); > > > } > > > try { > > > - TarantoolConnection connection = new TarantoolConnection(urlProperties.getProperty(PROP_USER), urlProperties.getProperty(PROP_PASSWORD), socket) {{ > > > + TarantoolConnection connection = new TarantoolConnection( > > > + urlProperties.getProperty(PROP_USER), > > > + urlProperties.getProperty(PROP_PASSWORD), > > > + socket) {{ > > > msgPackLite = SQLMsgPackLite.INSTANCE; > > > }}; > > > > > > - return new SQLConnection(connection, url, info); > > > - } catch (IOException e) { > > > - throw new SQLException("Couldn't initiate connection. Provider class name is " + providerClassName, e); > > > + return new SQLConnection(connection, url, urlProperties); > > > + } catch (Exception e) { > > > + try { > > > + socket.close(); > > > + } catch (IOException ignored) { > > > + // No-op. > > > + } > > > + throw new SQLException("Couldn't initiate connection using " + diagProperties(urlProperties), e); > > > } > > > - > > > } > > > > Are we really need to work with Socket and TarantoolConnection within > > this class? Are we can create Socket inside TarantoolConnection and > > TarantoolConnection inside SQLConnection? I think it will improve > > encapsulation. > > > > Hope we can mock it in some less intrusive way. Are we can? > > > > Even if we'll need to pass some implementation explicitly via > > constructors arguments (Socket for the TarantoolConnection constructor > > and TarantoolConnection for the SQLConnection constructor), maybe it is > > better to provide a class and not an instance to encapsulate handling of > > possible construction exceptions inside TarantoolConnection / > > SQLConnection implementations? > > > > I don't very familiar with Java approaches (code patterns) to do such > > things, so I ask you to help me find best approach here. > > > > I don't push you to rewrite it right now (but maybe now is the good time > > to do so), but want to consider alternatives and, then, either plan > > future work or ask you to change things now (or, maybe, decide to leave > > things as is). > > > > I made an attempt to apply your proposition. Now a socket is created > within TarantoolConnection which in its turn is created inside SQLConnection. > The instantiations are done with the help of provider/factory interfaces. > It doesn't look particularly well, I must admit, due to the fact that we > need to provide a different interface on each nesting level and adapt them. > I like the idea of SQLSocketProvider accepting an URI and properties, but > the TarantoolConnection is unable to pass them by itself. > > I guess we must think a little bit more on this. I'm against of changing API of basic connector in incompatible manner. TarantoolConnection constructor and even maybe TarantoolBase constructor should not be changed. Can we add separate constructor with SocketProvider? Or maybe create connected socket in SQLConnection's constructor (yep, it is encapsulation break, but only in one place to retain downside API -- should be properly commented I think). To be honest I don't think that, say, providing TarantoolConnectionFactory instead of TarantoolConnection in constructor parameters improves encapsulation, because calling code still need to create an object, which create TarantoolConnection in its method. I'm understand the approach when it is optional parameter: a user who want some custom behaviour able to provide it in that way, others don't obligated to write 'boilerplace'. Other provider / fabric parameters across the code are the same from this perspective. Hovewer SQLConnection constructor is not part of public API, so I'm not against this way if you found it convenient. What would be ideal API from my point of view? Keeping in the mind TarantoolConnection API should not be changed, the following: ``` class SQLConnection { SQLConnection(Properties properties) { String username = properties.getProperty(PROP_USER); String password = properties.getProperty(PROP_PASSWORD); try { Socket socket = getConnectedSocket(properties); this.connection = TarantoolConnection(username, password, socket); } catch(...) { // rethrow } } // Obtain connected socket to use as parameter for // TarantoolConnection. private Socket getConnectedSocket(Properties properties) { // Code from SQLSocketProviderImpl.getConnectedSocket(). } ``` No need to introduce TarantoolConnectionFactory, SocketProvider[Impl], SQLSocketProvider[Impl], SocketFoundry. Are there anything bad here that I missed? > > I wonder also whether we can break things when call execute* methods in > > parallel from multiple threads? Will one execute breaks resultSet for > > the another? Of course it is not part of your issue, but maybe it should > > be handled as a separate one. > > > > Yes, it will break. > Let us address the aspects of multi-thread usage of a connection later on. > (If that will ever be requested.) I would prefer to formalize known issues even if we'll decide later to don't fix them. Filed #95. https://github.com/tarantool/tarantool-java/issues/95 > @SuppressWarnings("Since15") > -public class SQLConnection implements Connection { > - final TarantoolConnection connection; > +public class SQLConnection implements Connection, SocketProvider { > + private final SQLSocketProvider sqlSocketProvider; > + private final TarantoolConnection connection; > final String url; > final Properties properties; > > - public SQLConnection(TarantoolConnection connection, String url, Properties properties) { > - this.connection = connection; > + SQLConnection(TarantoolConnectionFactory connectionFactory, SQLSocketProvider sqlSocketProvider, String url, > + Properties properties) throws SQLException { > + this.sqlSocketProvider = sqlSocketProvider; > this.url = url; > this.properties = properties; Should we copy it to don't change external properties in case of setClientInfo() call? > + protected int executeUpdate(String sql, Object ... args) throws SQLException { > + checkNotClosed(); > + try { > + return JDBCBridge.update(connection, sql, args); > + } catch (Exception e) { > + handleException(e); > + throw new SQLException(formatError(sql, args), e); > + } > + } > + It seems that JDBCBridge is result formatter for SQL statements, but its logic spread over multiple classes and is hard to understand, because of that and because of naming. Also I found confusing that its methods get TarantoolConnection as a parameter, but uses only results from last operation. It is not about your patch, just side note. > +public class SQLSocketProviderImpl implements SQLSocketProvider { > + public final static SQLSocketProvider INSTANCE = new SQLSocketProviderImpl(new SocketFoundry() { > + public Socket makeSocket() { > + return new Socket(); > + } > + }); > + > + private final SocketFoundry provider; > + > + SQLSocketProviderImpl(SocketFoundry provider) { > + this.provider = provider; > + } > + > + @Override > + public Socket getConnectedSocket(URI uri, Properties properties) { uri is ignored, so this provider is very limited to be used in SQLDriver. Such things should be commented. > + Socket socket = provider.makeSocket(); Cannot we just do `new Socket()` here? I don't insist at all, just curious why such high level of abstraction was introduced. > + @Test > + public void testGetPropertyInfo() throws SQLException { > + Driver drv = new SQLDriver(); > + Properties props = new Properties(); > + DriverPropertyInfo[] info = drv.getPropertyInfo("tarantool://server.local:3302", props); > + assertNotNull(info); I think it worth to add array size check here. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: add connection timeout configuration and handling 2018-11-26 15:01 ` Alexander Turenko @ 2018-12-05 11:16 ` Sergei Kalashnikov 2018-12-10 12:59 ` Alexander Turenko 0 siblings, 1 reply; 6+ messages in thread From: Sergei Kalashnikov @ 2018-12-05 11:16 UTC (permalink / raw) To: Alexander Turenko; +Cc: tarantool-patches Hi Alexander! Thank you for review. Please find my answers and the patch below. On Mon, Nov 26, 2018 at 06:01:53PM +0300, Alexander Turenko wrote: > On Thu, Nov 15, 2018 at 08:22:09PM +0300, Sergei Kalashnikov wrote: > > On Mon, Oct 22, 2018 at 07:21:30AM +0300, Alexander Turenko wrote: > > > > Hi Alexander! > > > > My sincere apologies for the slow reply; Please see my answers inline below. > > I also included the amended patch at the very end of this email. > > > > Thank you. > > -- > > Best regards, > > Sergei > > Sorry for the slow reply too. It is hard for me to immerse enough to the > code to make a review useful... > > I'm okay with tests and mostly okay with the patch. The most important > thing is that (I think) we should not break TarantoolConnection API. > Other notes are minor and are matters of opinion. > > I think we should split features and refactoring in the future, because > it is hard to understand and review many different (not coupled) > changes. It is about future patches. > > My comments are below. > > WBR, Alexander Turenko. > > > > > @@ -293,15 +299,28 @@ public class SQLConnection implements Connection { > > > > > > > > @Override > > > > public void setNetworkTimeout(Executor executor, int milliseconds) throws SQLException { > > > > - throw new SQLFeatureNotSupportedException(); > > > > + checkNotClosed(); > > > > + > > > > + if (milliseconds < 0) > > > > + throw new SQLException("Network timeout cannot be negative."); > > > > + > > > > + try { > > > > + connection.setSocketTimeout(milliseconds); > > > > + } catch (SocketException e) { > > > > + throw new SQLException("Failed to set socket timeout: timeout=" + milliseconds, e); > > > > + } > > > > } > > > > > > The executor is not used. Found [overview][1] of the problem. Checked > > > pgjdbc, they [do][2] the same. But mysql-connector-j [does not][3]. They > > > need to handle some complex cases like [4] (see also [5]) because of > > > that. To be honest I don't get Douglas's Surber explanation, but I think > > > we likely do right things here when just ignore the executor parameter. > > > > My understanding is that executor in question was intended for providing a thread of > > execution that driver may use to track timeout, interrupt its own threads that perform > > network calls and cleanup the network resources afterwards. If the implementation can detect > > timeouts and cleanup resources without the use of provided executor, it may choose to do so. > > > > In my opinion, the vague wording in specification alone is not a good reason to go and > > implement wrapping of a mere saving of timeout value into the executor just to add ourselves > > a concurrency issue and then add a test for it (it is what pg did). > > So this is not a right thing either. > > > > So we are on the one side. Great. > > > > > @@ -311,4 +330,28 @@ public class SQLConnection implements Connection { > > > > public boolean isWrapperFor(Class<?> iface) throws SQLException { > > > > throw new SQLFeatureNotSupportedException(); > > > > } > > > > + > > > > + /** > > > > + * @throws SQLException If connection is closed. > > > > + */ > > > > + protected void checkNotClosed() throws SQLException { > > > > + if (isClosed()) > > > > + throw new SQLException("Connection is closed."); > > > > + } > > > > + > > > > + /** > > > > + * Inspects passed exception and closes the connection if appropriate. > > > > + * > > > > + * @param e Exception to process. > > > > + */ > > > > + protected void handleException(Exception e) { > > > > + if (CommunicationException.class.isAssignableFrom(e.getClass()) || > > > > + IOException.class.isAssignableFrom(e.getClass())) { > > > > + try { > > > > + close(); > > > > + } catch (SQLException ignored) { > > > > + // No-op. > > > > + } > > > > + } > > > > + } > > > > > > Having the protected handleException method seems to break encapsulation > > > of the SQLConnection class (and maybe checkNotClosed too). I think it is > > > due to using the connection field (of the TarantoolConnection type) > > > outside of the class. Maybe we should wrap calls to connection.select > > > and so on with protected methods of the SQLConnection class like > > > nativeSelect and so on. And perform checkNotClosed and handleException > > > actions inside these wrappers. What do you think? > > > > > > > Yes, accesses like `connecttion.connection` must be cleaned up. > > I've changed the `connection` to private inside `SQLConnection` and made other > > refactorings including wrapping calls to TarantoolConnection methods > > outside of SQLConnection into new SQLConnection methods. But I tend to > > disagree regarding checkNotClosed(). It is useful by itself in situations > > when parameters passed to a function allow to return a local answer, > > but connection is closed. > > Looks good. > > > > > @@ -45,22 +53,42 @@ public class SQLDriver implements Driver { > > > > if (providerClassName != null) { > > > > socket = getSocketFromProvider(uri, urlProperties, providerClassName); > > > > } else { > > > > - socket = createAndConnectDefaultSocket(urlProperties); > > > > + // Passing the socket to allow unit tests to mock it. > > > > + socket = connectAndSetupDefaultSocket(urlProperties, new Socket()); > > > > } > > > > try { > > > > - TarantoolConnection connection = new TarantoolConnection(urlProperties.getProperty(PROP_USER), urlProperties.getProperty(PROP_PASSWORD), socket) {{ > > > > + TarantoolConnection connection = new TarantoolConnection( > > > > + urlProperties.getProperty(PROP_USER), > > > > + urlProperties.getProperty(PROP_PASSWORD), > > > > + socket) {{ > > > > msgPackLite = SQLMsgPackLite.INSTANCE; > > > > }}; > > > > > > > > - return new SQLConnection(connection, url, info); > > > > - } catch (IOException e) { > > > > - throw new SQLException("Couldn't initiate connection. Provider class name is " + providerClassName, e); > > > > + return new SQLConnection(connection, url, urlProperties); > > > > + } catch (Exception e) { > > > > + try { > > > > + socket.close(); > > > > + } catch (IOException ignored) { > > > > + // No-op. > > > > + } > > > > + throw new SQLException("Couldn't initiate connection using " + diagProperties(urlProperties), e); > > > > } > > > > - > > > > } > > > > > > Are we really need to work with Socket and TarantoolConnection within > > > this class? Are we can create Socket inside TarantoolConnection and > > > TarantoolConnection inside SQLConnection? I think it will improve > > > encapsulation. > > > > > > Hope we can mock it in some less intrusive way. Are we can? > > > > > > Even if we'll need to pass some implementation explicitly via > > > constructors arguments (Socket for the TarantoolConnection constructor > > > and TarantoolConnection for the SQLConnection constructor), maybe it is > > > better to provide a class and not an instance to encapsulate handling of > > > possible construction exceptions inside TarantoolConnection / > > > SQLConnection implementations? > > > > > > I don't very familiar with Java approaches (code patterns) to do such > > > things, so I ask you to help me find best approach here. > > > > > > I don't push you to rewrite it right now (but maybe now is the good time > > > to do so), but want to consider alternatives and, then, either plan > > > future work or ask you to change things now (or, maybe, decide to leave > > > things as is). > > > > > > > I made an attempt to apply your proposition. Now a socket is created > > within TarantoolConnection which in its turn is created inside SQLConnection. > > The instantiations are done with the help of provider/factory interfaces. > > It doesn't look particularly well, I must admit, due to the fact that we > > need to provide a different interface on each nesting level and adapt them. > > I like the idea of SQLSocketProvider accepting an URI and properties, but > > the TarantoolConnection is unable to pass them by itself. > > > > I guess we must think a little bit more on this. > > I'm against of changing API of basic connector in incompatible manner. > TarantoolConnection constructor and even maybe TarantoolBase constructor > should not be changed. Can we add separate constructor with > SocketProvider? Or maybe create connected socket in SQLConnection's > constructor (yep, it is encapsulation break, but only in one place to > retain downside API -- should be properly commented I think). > > To be honest I don't think that, say, providing > TarantoolConnectionFactory instead of TarantoolConnection in constructor > parameters improves encapsulation, because calling code still need to > create an object, which create TarantoolConnection in its method. I'm > understand the approach when it is optional parameter: a user who want > some custom behaviour able to provide it in that way, others don't > obligated to write 'boilerplace'. Other provider / fabric parameters > across the code are the same from this perspective. > > Hovewer SQLConnection constructor is not part of public API, so I'm not > against this way if you found it convenient. > > What would be ideal API from my point of view? Keeping in the mind > TarantoolConnection API should not be changed, the following: > > ``` > class SQLConnection { > SQLConnection(Properties properties) { > String username = properties.getProperty(PROP_USER); > String password = properties.getProperty(PROP_PASSWORD); > try { > Socket socket = getConnectedSocket(properties); > this.connection = TarantoolConnection(username, password, socket); > } catch(...) { > // rethrow > } > } > > // Obtain connected socket to use as parameter for > // TarantoolConnection. > private Socket getConnectedSocket(Properties properties) { > // Code from SQLSocketProviderImpl.getConnectedSocket(). > } > ``` > > No need to introduce TarantoolConnectionFactory, SocketProvider[Impl], > SQLSocketProvider[Impl], SocketFoundry. Are there anything bad here that > I missed? > SQLSocketProvider interface was here initially and is already a part of a public API (but not the JDBC API obviously). The user provides a name of the class implementing SQLSocketProvider via a connection property named "socketProvider". With your approach the SQLConnection knows nothing about this SQLSocketProvider and to support it, the SQLDriver needs to subclass SQLConnection, override getConnectedSocket() and inject the call to SQLSocketProvider given by the user. So, getConnectedSocket() may not be private. I added these changes into the patch. Also, the url is still required for SQLConnection constructor, since there is a SQLDatabaseMetadata.getURL() API. > > > I wonder also whether we can break things when call execute* methods in > > > parallel from multiple threads? Will one execute breaks resultSet for > > > the another? Of course it is not part of your issue, but maybe it should > > > be handled as a separate one. > > > > > > > Yes, it will break. > > Let us address the aspects of multi-thread usage of a connection later on. > > (If that will ever be requested.) > > I would prefer to formalize known issues even if we'll decide later to > don't fix them. Filed #95. > > https://github.com/tarantool/tarantool-java/issues/95 > > > @SuppressWarnings("Since15") > > -public class SQLConnection implements Connection { > > - final TarantoolConnection connection; > > +public class SQLConnection implements Connection, SocketProvider { > > + private final SQLSocketProvider sqlSocketProvider; > > + private final TarantoolConnection connection; > > final String url; > > final Properties properties; > > > > - public SQLConnection(TarantoolConnection connection, String url, Properties properties) { > > - this.connection = connection; > > + SQLConnection(TarantoolConnectionFactory connectionFactory, SQLSocketProvider sqlSocketProvider, String url, > > + Properties properties) throws SQLException { > > + this.sqlSocketProvider = sqlSocketProvider; > > this.url = url; > > this.properties = properties; > > Should we copy it to don't change external properties in case of > setClientInfo() call? > The properties map here is already a copy made in SQLDriver. Also, the properties managed by setClientInfo() have different purpose than the connection properties and it is incorrect to store them in one map anyway. > > + protected int executeUpdate(String sql, Object ... args) throws SQLException { > > + checkNotClosed(); > > + try { > > + return JDBCBridge.update(connection, sql, args); > > + } catch (Exception e) { > > + handleException(e); > > + throw new SQLException(formatError(sql, args), e); > > + } > > + } > > + > > It seems that JDBCBridge is result formatter for SQL statements, but its > logic spread over multiple classes and is hard to understand, because of > that and because of naming. Also I found confusing that its methods get > TarantoolConnection as a parameter, but uses only results from last > operation. > > It is not about your patch, just side note. > > > +public class SQLSocketProviderImpl implements SQLSocketProvider { > > + public final static SQLSocketProvider INSTANCE = new SQLSocketProviderImpl(new SocketFoundry() { > > + public Socket makeSocket() { > > + return new Socket(); > > + } > > + }); > > + > > + private final SocketFoundry provider; > > + > > + SQLSocketProviderImpl(SocketFoundry provider) { > > + this.provider = provider; > > + } > > + > > + @Override > > + public Socket getConnectedSocket(URI uri, Properties properties) { > > uri is ignored, so this provider is very limited to be used in > SQLDriver. Such things should be commented. > I have removed this class in the amended patch. Added some comments regarding the use of uri and properties in SQLConnection#getConnectedSocket(). > > + Socket socket = provider.makeSocket(); > > Cannot we just do `new Socket()` here? I don't insist at all, just > curious why such high level of abstraction was introduced. > That is solely for the purpose of unit testing. The mockito framework we adopted can't mock the constructor. So we need some parameter, method, or interface to inject test classes. To keep the unit tests, I had to introduce the following methods in the amended patch. SQLConnection#makeSocket() SQLConnection#makeConnection() They only contain the call to constructor of corresponding class. > > + @Test > > + public void testGetPropertyInfo() throws SQLException { > > + Driver drv = new SQLDriver(); > > + Properties props = new Properties(); > > + DriverPropertyInfo[] info = drv.getPropertyInfo("tarantool://server.local:3302", props); > > + assertNotNull(info); > > I think it worth to add array size check here. Yes, added it. Please find below the amended patch: =============================================== Commit 6237967f0b83035a9717a8fe95941126a18396b1 jdbc: add connection timeout configuration and handling Added connection property `socketTimeout` to allow user control over network timeout before actual connection is returned by the driver. This is only done for default socket provider. The default timeout is left to be infinite. Implemented `Connection.setNetworkTimeout` API to make it possible to change the maximum amount of time to wait for server replies after the connection is established. The connection that has timed out is forced to close. New subsequent operations requested on such connection must fail right away. The corresponding checks are embedded into relevant APIs. Closes #38 --- https://github.com/tarantool/tarantool-java/issues/38 https://github.com/ztarvos/tarantool-java/commits/gh-38-add-connect-timeout .../java/org/tarantool/TarantoolConnection.java | 21 ++ .../java/org/tarantool/jdbc/SQLConnection.java | 216 +++++++++++++++++- .../org/tarantool/jdbc/SQLDatabaseMetadata.java | 101 +++++---- src/main/java/org/tarantool/jdbc/SQLDriver.java | 163 +++++++++----- .../org/tarantool/jdbc/SQLPreparedStatement.java | 35 ++- src/main/java/org/tarantool/jdbc/SQLStatement.java | 70 +++--- .../java/org/tarantool/jdbc/AbstractJdbcIT.java | 53 ++--- .../java/org/tarantool/jdbc/JdbcConnectionIT.java | 65 ++++++ .../org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java | 30 +++ .../java/org/tarantool/jdbc/JdbcDriverTest.java | 202 +++++++++++++++++ .../tarantool/jdbc/JdbcExceptionHandlingTest.java | 241 ++++++++++++++++++++- .../tarantool/jdbc/JdbcPreparedStatementIT.java | 94 ++++++++ .../java/org/tarantool/jdbc/JdbcStatementIT.java | 30 +++ 13 files changed, 1149 insertions(+), 172 deletions(-) create mode 100644 src/test/java/org/tarantool/jdbc/JdbcDriverTest.java diff --git a/src/main/java/org/tarantool/TarantoolConnection.java b/src/main/java/org/tarantool/TarantoolConnection.java index be94995..b817988 100644 --- a/src/main/java/org/tarantool/TarantoolConnection.java +++ b/src/main/java/org/tarantool/TarantoolConnection.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.Socket; +import java.net.SocketException; import java.nio.ByteBuffer; import java.util.List; import java.util.Map; @@ -80,4 +81,24 @@ public class TarantoolConnection extends TarantoolBase<List<?>> implements Taran public boolean isClosed() { return socket.isClosed(); } + + /** + * Sets given timeout value on underlying socket. + * + * @param timeout Timeout in milliseconds. + * @throws SocketException If failed. + */ + public void setSocketTimeout(int timeout) throws SocketException { + socket.setSoTimeout(timeout); + } + + /** + * Retrieves timeout value from underlying socket. + * + * @return Timeout in milliseconds. + * @throws SocketException If failed. + */ + public int getSocketTimeout() throws SocketException { + return socket.getSoTimeout(); + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLConnection.java b/src/main/java/org/tarantool/jdbc/SQLConnection.java index de28850..89b0f81 100644 --- a/src/main/java/org/tarantool/jdbc/SQLConnection.java +++ b/src/main/java/org/tarantool/jdbc/SQLConnection.java @@ -1,5 +1,9 @@ package org.tarantool.jdbc; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.net.SocketException; import java.sql.Array; import java.sql.Blob; import java.sql.CallableStatement; @@ -8,6 +12,7 @@ import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.NClob; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLClientInfoException; import java.sql.SQLException; import java.sql.SQLFeatureNotSupportedException; @@ -16,32 +21,133 @@ import java.sql.SQLXML; import java.sql.Savepoint; import java.sql.Statement; import java.sql.Struct; +import java.util.Arrays; +import java.util.List; import java.util.Map; import java.util.Properties; import java.util.concurrent.Executor; +import org.tarantool.CommunicationException; +import org.tarantool.JDBCBridge; import org.tarantool.TarantoolConnection; +import static org.tarantool.jdbc.SQLDriver.PROP_HOST; +import static org.tarantool.jdbc.SQLDriver.PROP_PASSWORD; +import static org.tarantool.jdbc.SQLDriver.PROP_PORT; +import static org.tarantool.jdbc.SQLDriver.PROP_SOCKET_TIMEOUT; +import static org.tarantool.jdbc.SQLDriver.PROP_USER; + @SuppressWarnings("Since15") public class SQLConnection implements Connection { - final TarantoolConnection connection; + private final TarantoolConnection connection; final String url; final Properties properties; - public SQLConnection(TarantoolConnection connection, String url, Properties properties) { - this.connection = connection; + SQLConnection(String url, Properties properties) throws SQLException { this.url = url; this.properties = properties; + + String user = properties.getProperty(PROP_USER); + String pass = properties.getProperty(PROP_PASSWORD); + Socket socket = null; + try { + socket = getConnectedSocket(); + this.connection = makeConnection(user, pass, socket); + } catch (Exception e) { + if (socket != null) { + try { + socket.close(); + } catch (IOException ignored) { + // No-op. + } + } + if (e instanceof SQLException) + throw (SQLException)e; + throw new SQLException("Couldn't initiate connection using " + SQLDriver.diagProperties(properties), e); + } + } + + /** + * Provides a connected socket to be used to initialize a native tarantool + * connection. + * + * The implementation assumes that {@link #properties} contains all the + * necessary info extracted from both the URI and connection properties + * provided by the user. However, the overrides are free to also use the + * {@link #url} if required. + * + * A connect is guarded with user provided timeout. Socket is configured + * to honor this timeout for the following read/write operations as well. + * + * @return Connected socket. + * @throws SQLException if failed. + */ + protected Socket getConnectedSocket() throws SQLException { + Socket socket = makeSocket(); + int timeout = Integer.parseInt(properties.getProperty(PROP_SOCKET_TIMEOUT)); + String host = properties.getProperty(PROP_HOST); + int port = Integer.parseInt(properties.getProperty(PROP_PORT)); + try { + socket.connect(new InetSocketAddress(host, port), timeout); + } catch (IOException e) { + throw new SQLException("Couldn't connect to " + host + ":" + port, e); + } + // Setup socket further. + if (timeout > 0) { + try { + socket.setSoTimeout(timeout); + } catch (SocketException e) { + try { + socket.close(); + } catch (IOException ignored) { + // No-op. + } + throw new SQLException("Couldn't set socket timeout. timeout=" + timeout, e); + } + } + return socket; + } + + /** + * Provides a newly connected socket instance. The method is intended to be + * overridden to enable unit testing of the class. + * + * Not supposed to contain any logic other than a call to constructor. + * + * @return socket. + */ + protected Socket makeSocket() { + return new Socket(); + } + + /** + * Provides a native tarantool connection instance. The method is intended + * to be overridden to enable unit testing of the class. + * + * Not supposed to contain any logic other than a call to constructor. + * + * @param user User name. + * @param pass Password. + * @param socket Connected socket. + * @return Native tarantool connection. + * @throws IOException if failed. + */ + protected TarantoolConnection makeConnection(String user, String pass, Socket socket) throws IOException { + return new TarantoolConnection(user, pass, socket) {{ + msgPackLite = SQLMsgPackLite.INSTANCE; + }}; } @Override public Statement createStatement() throws SQLException { - return new SQLStatement(connection, this); + checkNotClosed(); + return new SQLStatement(this); } @Override public PreparedStatement prepareStatement(String sql) throws SQLException { - return new SQLPreparedStatement(connection, this, sql); + checkNotClosed(); + return new SQLPreparedStatement(this, sql); } @Override @@ -89,6 +195,7 @@ public class SQLConnection implements Connection { @Override public DatabaseMetaData getMetaData() throws SQLException { + checkNotClosed(); return new SQLDatabaseMetadata(this); } @@ -293,15 +400,28 @@ public class SQLConnection implements Connection { @Override public void setNetworkTimeout(Executor executor, int milliseconds) throws SQLException { - throw new SQLFeatureNotSupportedException(); + checkNotClosed(); + + if (milliseconds < 0) + throw new SQLException("Network timeout cannot be negative."); + + try { + connection.setSocketTimeout(milliseconds); + } catch (SocketException e) { + throw new SQLException("Failed to set socket timeout: timeout=" + milliseconds, e); + } } @Override public int getNetworkTimeout() throws SQLException { - throw new SQLFeatureNotSupportedException(); + checkNotClosed(); + try { + return connection.getSocketTimeout(); + } catch (SocketException e) { + throw new SQLException("Failed to retrieve socket timeout", e); + } } - @Override public <T> T unwrap(Class<T> iface) throws SQLException { throw new SQLFeatureNotSupportedException(); @@ -311,4 +431,84 @@ public class SQLConnection implements Connection { public boolean isWrapperFor(Class<?> iface) throws SQLException { throw new SQLFeatureNotSupportedException(); } + + protected Object execute(String sql, Object ... args) throws SQLException { + checkNotClosed(); + try { + return JDBCBridge.execute(connection, sql, args); + } catch (Exception e) { + handleException(e); + throw new SQLException(formatError(sql, args), e); + } + } + + protected ResultSet executeQuery(String sql, Object ... args) throws SQLException { + checkNotClosed(); + try { + return new SQLResultSet(JDBCBridge.query(connection, sql, args)); + } catch (Exception e) { + handleException(e); + throw new SQLException(formatError(sql, args), e); + } + } + + protected int executeUpdate(String sql, Object ... args) throws SQLException { + checkNotClosed(); + try { + return JDBCBridge.update(connection, sql, args); + } catch (Exception e) { + handleException(e); + throw new SQLException(formatError(sql, args), e); + } + } + + protected List<?> nativeSelect(Integer space, Integer index, List<?> key, int offset, int limit, int iterator) + throws SQLException { + checkNotClosed(); + try { + return connection.select(space, index, key, offset, limit, iterator); + } catch (Exception e) { + handleException(e); + throw new SQLException(e); + } + } + + protected String getServerVersion() { + return connection.getServerVersion(); + } + + /** + * @throws SQLException If connection is closed. + */ + protected void checkNotClosed() throws SQLException { + if (isClosed()) + throw new SQLException("Connection is closed."); + } + + /** + * Inspects passed exception and closes the connection if appropriate. + * + * @param e Exception to process. + */ + private void handleException(Exception e) { + if (CommunicationException.class.isAssignableFrom(e.getClass()) || + IOException.class.isAssignableFrom(e.getClass())) { + try { + close(); + } catch (SQLException ignored) { + // No-op. + } + } + } + + /** + * Provides error message that contains parameters of failed SQL statement. + * + * @param sql SQL Text. + * @param params Parameters of the SQL statement. + * @return Formatted error message. + */ + private static String formatError(String sql, Object ... params) { + return "Failed to execute SQL: " + sql + ", params: " + Arrays.deepToString(params); + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java index c8879c9..2ddb0ef 100644 --- a/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java +++ b/src/main/java/org/tarantool/jdbc/SQLDatabaseMetadata.java @@ -105,7 +105,7 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { @Override public String getDatabaseProductVersion() throws SQLException { - return connection.connection.getServerVersion(); + return connection.getServerVersion(); } @Override @@ -672,23 +672,29 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { @Override public ResultSet getTables(String catalog, String schemaPattern, String tableNamePattern, String[] types) - throws SQLException { - if (types != null && !Arrays.asList(types).contains("TABLE")) { - return new SQLResultSet(JDBCBridge.EMPTY); - } - String[] parts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); - List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); - List<List<Object>> rows = new ArrayList<List<Object>>(); - for (List<Object> space : spaces) { - String name = (String) space.get(NAME_IDX); - Map flags = (Map) space.get(FLAGS_IDX); - if (flags != null && flags.containsKey("sql") && like(name, parts)) { - rows.add(Arrays.asList(name, "TABLE", flags.get("sql"))); + throws SQLException { + try { + if (types != null && !Arrays.asList(types).contains("TABLE")) { + connection.checkNotClosed(); + return new SQLResultSet(JDBCBridge.EMPTY); + } + String[] parts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); + List<List<Object>> spaces = (List<List<Object>>) connection.nativeSelect(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); + List<List<Object>> rows = new ArrayList<List<Object>>(); + for (List<Object> space : spaces) { + String name = (String) space.get(NAME_IDX); + Map flags = (Map) space.get(FLAGS_IDX); + if (flags != null && flags.containsKey("sql") && like(name, parts)) { + rows.add(Arrays.asList(name, "TABLE", flags.get("sql"))); + } } + return new SQLNullResultSet(JDBCBridge.mock(Arrays.asList("TABLE_NAME", "TABLE_TYPE", "REMARKS", + //nulls + "TABLE_CAT", "TABLE_SCHEM", "TABLE_TYPE", "TYPE_CAT", "TYPE_SCHEM", "TYPE_NAME", "SELF_REFERENCING_COL_NAME", "REF_GENERATION"), rows)); + } catch (Exception e) { + throw new SQLException("Failed to retrieve table(s) description: " + + "tableNamePattern=\"" + tableNamePattern + "\".", e); } - return new SQLNullResultSet(JDBCBridge.mock(Arrays.asList("TABLE_NAME", "TABLE_TYPE", "REMARKS", - //nulls - "TABLE_CAT", "TABLE_SCHEM", "TABLE_TYPE", "TYPE_CAT", "TYPE_SCHEM", "TYPE_NAME", "SELF_REFERENCING_COL_NAME", "REF_GENERATION"), rows)); } @Override @@ -713,32 +719,38 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { @Override public ResultSet getColumns(String catalog, String schemaPattern, String tableNamePattern, String columnNamePattern) throws SQLException { - String[] tableParts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); - String[] colParts = columnNamePattern == null ? new String[]{""} : columnNamePattern.split("%"); - List<List<Object>> spaces = (List<List<Object>>) connection.connection.select(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); - List<List<Object>> rows = new ArrayList<List<Object>>(); - for (List<Object> space : spaces) { - String tableName = (String) space.get(NAME_IDX); - Map flags = (Map) space.get(FLAGS_IDX); - if (flags != null && flags.containsKey("sql") && like(tableName, tableParts)) { - List<Map<String, Object>> format = (List<Map<String, Object>>) space.get(FORMAT_IDX); - for (int columnIdx = 1; columnIdx <= format.size(); columnIdx++) { - Map<String, Object> f = format.get(columnIdx - 1); - String columnName = (String) f.get("name"); - String dbType = (String) f.get("type"); - if (like(columnName, colParts)) { - rows.add(Arrays.<Object>asList(tableName, columnName, columnIdx, Types.OTHER, dbType, 10, 1, "YES", Types.OTHER, "NO", "NO")); + try { + String[] tableParts = tableNamePattern == null ? new String[]{""} : tableNamePattern.split("%"); + String[] colParts = columnNamePattern == null ? new String[]{""} : columnNamePattern.split("%"); + List<List<Object>> spaces = (List<List<Object>>) connection.nativeSelect(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); + List<List<Object>> rows = new ArrayList<List<Object>>(); + for (List<Object> space : spaces) { + String tableName = (String) space.get(NAME_IDX); + Map flags = (Map) space.get(FLAGS_IDX); + if (flags != null && flags.containsKey("sql") && like(tableName, tableParts)) { + List<Map<String, Object>> format = (List<Map<String, Object>>) space.get(FORMAT_IDX); + for (int columnIdx = 1; columnIdx <= format.size(); columnIdx++) { + Map<String, Object> f = format.get(columnIdx - 1); + String columnName = (String) f.get("name"); + String dbType = (String) f.get("type"); + if (like(columnName, colParts)) { + rows.add(Arrays.<Object>asList(tableName, columnName, columnIdx, Types.OTHER, dbType, 10, 1, "YES", Types.OTHER, "NO", "NO")); + } } } } - } - return new SQLNullResultSet((JDBCBridge.mock( - Arrays.asList("TABLE_NAME", "COLUMN_NAME", "ORDINAL_POSITION", "DATA_TYPE", "TYPE_NAME", "NUM_PREC_RADIX", "NULLABLE", "IS_NULLABLE", "SOURCE_DATA_TYPE", "IS_AUTOINCREMENT", "IS_GENERATEDCOLUMN", - //nulls - "TABLE_CAT", "TABLE_SCHEM", "COLUMN_SIZE", "BUFFER_LENGTH", "DECIMAL_DIGITS", "REMARKS", "COLUMN_DEF", "SQL_DATA_TYPE", "SQL_DATETIME_SUB", "CHAR_OCTET_LENGTH", "SCOPE_CATALOG", "SCOPE_SCHEMA", "SCOPE_TABLE" - ), - rows))); + return new SQLNullResultSet((JDBCBridge.mock( + Arrays.asList("TABLE_NAME", "COLUMN_NAME", "ORDINAL_POSITION", "DATA_TYPE", "TYPE_NAME", "NUM_PREC_RADIX", "NULLABLE", "IS_NULLABLE", "SOURCE_DATA_TYPE", "IS_AUTOINCREMENT", "IS_GENERATEDCOLUMN", + //nulls + "TABLE_CAT", "TABLE_SCHEM", "COLUMN_SIZE", "BUFFER_LENGTH", "DECIMAL_DIGITS", "REMARKS", "COLUMN_DEF", "SQL_DATA_TYPE", "SQL_DATETIME_SUB", "CHAR_OCTET_LENGTH", "SCOPE_CATALOG", "SCOPE_SCHEMA", "SCOPE_TABLE" + ), + rows))); + } catch (Exception e) { + throw new SQLException("Error processing table column metadata: " + + "tableNamePattern=\"" + tableNamePattern + "\"; " + + "columnNamePattern=\"" + columnNamePattern + "\".", e); + } } @Override @@ -769,11 +781,13 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { final List<String> colNames = Arrays.asList( "TABLE_CAT", "TABLE_SCHEM", "TABLE_NAME", "COLUMN_NAME", "KEY_SEQ", "PK_NAME"); - if (table == null || table.isEmpty()) + if (table == null || table.isEmpty()) { + connection.checkNotClosed(); return emptyResultSet(colNames); + } try { - List spaces = connection.connection.select(_VSPACE, 2, Collections.singletonList(table), 0, 1, 0); + List spaces = connection.nativeSelect(_VSPACE, 2, Collections.singletonList(table), 0, 1, 0); if (spaces == null || spaces.size() == 0) return emptyResultSet(colNames); @@ -781,7 +795,7 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { List space = ensureType(List.class, spaces.get(0)); List fields = ensureType(List.class, space.get(FORMAT_IDX)); int spaceId = ensureType(Number.class, space.get(SPACE_ID_IDX)).intValue(); - List indexes = connection.connection.select(_VINDEX, 0, Arrays.asList(spaceId, 0), 0, 1, 0); + List indexes = connection.nativeSelect(_VINDEX, 0, Arrays.asList(spaceId, 0), 0, 1, 0); List primaryKey = ensureType(List.class, indexes.get(0)); List parts = ensureType(List.class, primaryKey.get(INDEX_FORMAT_IDX)); @@ -809,9 +823,8 @@ public class SQLDatabaseMetadata implements DatabaseMetaData { } }); return new SQLNullResultSet((JDBCBridge.mock(colNames, rows))); - } - catch (Throwable t) { - throw new SQLException("Error processing metadata for table \"" + table + "\".", t); + } catch (Exception e) { + throw new SQLException("Error processing metadata for table \"" + table + "\".", e); } } diff --git a/src/main/java/org/tarantool/jdbc/SQLDriver.java b/src/main/java/org/tarantool/jdbc/SQLDriver.java index 6867997..6ca9786 100644 --- a/src/main/java/org/tarantool/jdbc/SQLDriver.java +++ b/src/main/java/org/tarantool/jdbc/SQLDriver.java @@ -1,7 +1,5 @@ package org.tarantool.jdbc; -import java.io.IOException; -import java.net.InetSocketAddress; import java.net.Socket; import java.net.URI; import java.sql.Connection; @@ -14,8 +12,6 @@ import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Logger; -import org.tarantool.TarantoolConnection; - @SuppressWarnings("Since15") public class SQLDriver implements Driver { @@ -32,35 +28,53 @@ public class SQLDriver implements Driver { public static final String PROP_SOCKET_PROVIDER = "socketProvider"; public static final String PROP_USER = "user"; public static final String PROP_PASSWORD = "password"; + public static final String PROP_SOCKET_TIMEOUT = "socketTimeout"; + // Define default values once here. + final static Properties defaults = new Properties() {{ + setProperty(PROP_HOST, "localhost"); + setProperty(PROP_PORT, "3301"); + setProperty(PROP_SOCKET_TIMEOUT, "0"); + }}; - protected Map<String, SQLSocketProvider> providerCache = new ConcurrentHashMap<String, SQLSocketProvider>(); + private final Map<String, SQLSocketProvider> providerCache = new ConcurrentHashMap<String, SQLSocketProvider>(); @Override public Connection connect(String url, Properties info) throws SQLException { - URI uri = URI.create(url); - Properties urlProperties = parseQueryString(uri, info); + final URI uri = URI.create(url); + final Properties urlProperties = parseQueryString(uri, info); String providerClassName = urlProperties.getProperty(PROP_SOCKET_PROVIDER); - Socket socket; - if (providerClassName != null) { - socket = getSocketFromProvider(uri, urlProperties, providerClassName); - } else { - socket = createAndConnectDefaultSocket(urlProperties); - } - try { - TarantoolConnection connection = new TarantoolConnection(urlProperties.getProperty(PROP_USER), urlProperties.getProperty(PROP_PASSWORD), socket) {{ - msgPackLite = SQLMsgPackLite.INSTANCE; - }}; - return new SQLConnection(connection, url, info); - } catch (IOException e) { - throw new SQLException("Couldn't initiate connection. Provider class name is " + providerClassName, e); - } + if (providerClassName == null) + return new SQLConnection(url, urlProperties); + final SQLSocketProvider provider = getSocketProviderInstance(providerClassName); + + return new SQLConnection(url, urlProperties) { + @Override + protected Socket getConnectedSocket() throws SQLException { + Socket socket = provider.getConnectedSocket(uri, urlProperties); + if (socket == null) + throw new SQLException("The socket provider returned null socket"); + return socket; + } + }; } - protected Properties parseQueryString(URI uri, Properties info) { - Properties urlProperties = new Properties(info); + protected Properties parseQueryString(URI uri, Properties info) throws SQLException { + Properties urlProperties = new Properties(defaults); + + String userInfo = uri.getUserInfo(); + if (userInfo != null) { + // Get user and password from the corresponding part of the URI, i.e. before @ sign. + int i = userInfo.indexOf(':'); + if (i < 0) { + urlProperties.setProperty(PROP_USER, userInfo); + } else { + urlProperties.setProperty(PROP_USER, userInfo.substring(0, i)); + urlProperties.setProperty(PROP_PASSWORD, userInfo.substring(i + 1)); + } + } if (uri.getQuery() != null) { String[] parts = uri.getQuery().split("&"); for (String part : parts) { @@ -72,44 +86,62 @@ public class SQLDriver implements Driver { } } } - urlProperties.put(PROP_HOST, uri.getHost() == null ? "localhost" : uri.getHost()); - urlProperties.put(PROP_PORT, uri.getPort() < 1 ? "3301" : uri.getPort()); - return urlProperties; - } + if (uri.getHost() != null) { + // Default values are pre-put above. + urlProperties.setProperty(PROP_HOST, uri.getHost()); + } + if (uri.getPort() >= 0) { + // We need to convert port to string otherwise getProperty() will not see it. + urlProperties.setProperty(PROP_PORT, String.valueOf(uri.getPort())); + } + if (info != null) + urlProperties.putAll(info); - protected Socket createAndConnectDefaultSocket(Properties properties) throws SQLException { - Socket socket; - socket = new Socket(); + // Validate properties. + int port; try { - socket.connect(new InetSocketAddress(properties.getProperty(PROP_HOST, "localhost"), Integer.parseInt(properties.getProperty(PROP_PORT, "3301")))); + port = Integer.parseInt(urlProperties.getProperty(PROP_PORT)); } catch (Exception e) { - throw new SQLException("Couldn't connect to tarantool using" + properties, e); + throw new SQLException("Port must be a valid number."); + } + if (port <= 0 || port > 65535) { + throw new SQLException("Port is out of range: " + port); } - return socket; + int timeout; + try { + timeout = Integer.parseInt(urlProperties.getProperty(PROP_SOCKET_TIMEOUT)); + } catch (Exception e) { + throw new SQLException("Timeout must be a valid number."); + } + if (timeout < 0) { + throw new SQLException("Timeout must not be negative."); + } + return urlProperties; } - protected Socket getSocketFromProvider(URI uri, Properties urlProperties, String providerClassName) - throws SQLException { - Socket socket; - SQLSocketProvider sqlSocketProvider = providerCache.get(providerClassName); - if (sqlSocketProvider == null) { + protected SQLSocketProvider getSocketProviderInstance(String className) throws SQLException { + SQLSocketProvider provider = providerCache.get(className); + if (provider == null) { synchronized (this) { - sqlSocketProvider = providerCache.get(providerClassName); - if (sqlSocketProvider == null) { + provider = providerCache.get(className); + if (provider == null) { try { - Class<?> cls = Class.forName(providerClassName); + Class<?> cls = Class.forName(className); if (SQLSocketProvider.class.isAssignableFrom(cls)) { - sqlSocketProvider = (SQLSocketProvider) cls.newInstance(); - providerCache.put(providerClassName, sqlSocketProvider); + provider = (SQLSocketProvider)cls.newInstance(); + providerCache.put(className, provider); } } catch (Exception e) { - throw new SQLException("Couldn't initiate socket provider " + providerClassName, e); + throw new SQLException("Couldn't instantiate socket provider: " + className, e); } } } } - socket = sqlSocketProvider.getConnectedSocket(uri, urlProperties); - return socket; + if (provider == null) { + throw new SQLException(String.format("The socket provider %s does not implement %s", + className, SQLSocketProvider.class.getCanonicalName())); + } + return provider; } @Override @@ -121,16 +153,15 @@ public class SQLDriver implements Driver { public DriverPropertyInfo[] getPropertyInfo(String url, Properties info) throws SQLException { try { URI uri = new URI(url); - Properties properties = parseQueryString(uri, new Properties(info == null ? new Properties() : info)); + Properties properties = parseQueryString(uri, info); DriverPropertyInfo host = new DriverPropertyInfo(PROP_HOST, properties.getProperty(PROP_HOST)); host.required = true; - host.description = "Tarantool sever host"; + host.description = "Tarantool server host"; DriverPropertyInfo port = new DriverPropertyInfo(PROP_PORT, properties.getProperty(PROP_PORT)); port.required = true; - port.description = "Tarantool sever port"; - + port.description = "Tarantool server port"; DriverPropertyInfo user = new DriverPropertyInfo(PROP_USER, properties.getProperty(PROP_USER)); user.required = false; @@ -140,12 +171,20 @@ public class SQLDriver implements Driver { password.required = false; password.description = "password"; - DriverPropertyInfo socketProvider = new DriverPropertyInfo(PROP_SOCKET_PROVIDER, properties.getProperty(PROP_SOCKET_PROVIDER)); + DriverPropertyInfo socketProvider = new DriverPropertyInfo( + PROP_SOCKET_PROVIDER, properties.getProperty(PROP_SOCKET_PROVIDER)); + socketProvider.required = false; socketProvider.description = "SocketProvider class implements org.tarantool.jdbc.SQLSocketProvider"; + DriverPropertyInfo socketTimeout = new DriverPropertyInfo( + PROP_SOCKET_TIMEOUT, properties.getProperty(PROP_SOCKET_TIMEOUT)); + + socketTimeout.required = false; + socketTimeout.description = "The number of milliseconds to wait before a timeout is occurred on a socket" + + " connect or read. The default value is 0, which means infinite timeout."; - return new DriverPropertyInfo[]{host, port, user, password, socketProvider}; + return new DriverPropertyInfo[]{host, port, user, password, socketProvider, socketTimeout}; } catch (Exception e) { throw new SQLException(e); } @@ -171,5 +210,23 @@ public class SQLDriver implements Driver { throw new SQLFeatureNotSupportedException(); } - + /** + * Builds a string representation of given connection properties + * along with their sanitized values. + * + * @param props Connection properties. + * @return Comma-separated pairs of property names and values. + */ + protected static String diagProperties(Properties props) { + StringBuilder sb = new StringBuilder(); + for (Map.Entry<Object, Object> e : props.entrySet()) { + if (sb.length() > 0) + sb.append(", "); + sb.append(e.getKey()); + sb.append('='); + sb.append(PROP_USER.equals(e.getKey()) || PROP_PASSWORD.equals(e.getKey()) ? + "*****" : e.getValue().toString()); + } + return sb.toString(); + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java b/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java index 23d1073..99b5137 100644 --- a/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java +++ b/src/main/java/org/tarantool/jdbc/SQLPreparedStatement.java @@ -24,23 +24,22 @@ import java.util.Calendar; import java.util.HashMap; import java.util.Map; -import org.tarantool.JDBCBridge; -import org.tarantool.TarantoolConnection; - public class SQLPreparedStatement extends SQLStatement implements PreparedStatement { + final static String INVALID_CALL_MSG = "The method cannot be called on a PreparedStatement."; final String sql; final Map<Integer, Object> params; - public SQLPreparedStatement(TarantoolConnection connection, SQLConnection sqlConnection, String sql) { - super(connection, sqlConnection); + public SQLPreparedStatement(SQLConnection connection, String sql) { + super(connection); this.sql = sql; this.params = new HashMap<Integer, Object>(); } @Override public ResultSet executeQuery() throws SQLException { - return new SQLResultSet(JDBCBridge.query(connection, sql, getParams())); + discardLastResults(); + return connection.executeQuery(sql, getParams()); } protected Object[] getParams() throws SQLException { @@ -49,7 +48,7 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem if (params.containsKey(i)) { objects[i - 1] = params.get(i); } else { - throw new SQLException("Parameter " + i + "is not"); + throw new SQLException("Parameter " + i + " is missing"); } } return objects; @@ -57,8 +56,8 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem @Override public int executeUpdate() throws SQLException { - return JDBCBridge.update(connection, sql, getParams()); - + discardLastResults(); + return connection.executeUpdate(sql, getParams()); } @Override @@ -163,7 +162,8 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem @Override public boolean execute() throws SQLException { - return false; + discardLastResults(); + return handleResult(connection.execute(sql, getParams())); } @Override @@ -336,10 +336,23 @@ public class SQLPreparedStatement extends SQLStatement implements PreparedStatem throw new SQLFeatureNotSupportedException(); } - @Override public void addBatch() throws SQLException { throw new SQLFeatureNotSupportedException(); } + @Override + public ResultSet executeQuery(String sql) throws SQLException { + throw new SQLException(INVALID_CALL_MSG); + } + + @Override + public int executeUpdate(String sql) throws SQLException { + throw new SQLException(INVALID_CALL_MSG); + } + + @Override + public boolean execute(String sql) throws SQLException { + throw new SQLException(INVALID_CALL_MSG); + } } diff --git a/src/main/java/org/tarantool/jdbc/SQLStatement.java b/src/main/java/org/tarantool/jdbc/SQLStatement.java index 141ae52..3e75694 100644 --- a/src/main/java/org/tarantool/jdbc/SQLStatement.java +++ b/src/main/java/org/tarantool/jdbc/SQLStatement.java @@ -7,34 +7,27 @@ import java.sql.SQLFeatureNotSupportedException; import java.sql.SQLWarning; import java.sql.Statement; -import org.tarantool.JDBCBridge; -import org.tarantool.TarantoolConnection; - @SuppressWarnings("Since15") public class SQLStatement implements Statement { - protected final TarantoolConnection connection; - protected final SQLConnection sqlConnection; + protected final SQLConnection connection; private SQLResultSet resultSet; private int updateCount; private int maxRows; - protected SQLStatement(TarantoolConnection connection, SQLConnection sqlConnection) { - this.connection = connection; - this.sqlConnection = sqlConnection; + protected SQLStatement(SQLConnection sqlConnection) { + this.connection = sqlConnection; } @Override public ResultSet executeQuery(String sql) throws SQLException { - resultSet = new SQLResultSet(JDBCBridge.query(connection, sql)); - updateCount = -1; - return resultSet; + discardLastResults(); + return connection.executeQuery(sql); } @Override public int executeUpdate(String sql) throws SQLException { - int update = JDBCBridge.update(connection, sql); - resultSet = null; - return update; + discardLastResults(); + return connection.executeUpdate(sql); } @Override @@ -102,17 +95,8 @@ public class SQLStatement implements Statement { @Override public boolean execute(String sql) throws SQLException { - Object result = JDBCBridge.execute(connection, sql); - if (result instanceof SQLResultSet) { - resultSet = (SQLResultSet) result; - resultSet.maxRows = maxRows; - updateCount = -1; - return true; - } else { - resultSet = null; - updateCount = (Integer) result; - return false; - } + discardLastResults(); + return handleResult(connection.execute(sql)); } @Override @@ -186,7 +170,7 @@ public class SQLStatement implements Statement { @Override public Connection getConnection() throws SQLException { - return sqlConnection; + return connection; } @Override @@ -268,4 +252,38 @@ public class SQLStatement implements Statement { public boolean isWrapperFor(Class<?> iface) throws SQLException { throw new SQLFeatureNotSupportedException(); } + + /** + * Clears the results of the most recent execution. + */ + protected void discardLastResults() { + updateCount = -1; + if (resultSet != null) { + try { + resultSet.close(); + } catch (Exception ignored) { + // No-op. + } + resultSet = null; + } + } + + /** + * Sets the internals according to the result of last execution. + * + * @param result The result of SQL statement execution. + * @return {@code true}, if the result is a ResultSet object. + */ + protected boolean handleResult(Object result) { + if (result instanceof SQLResultSet) { + resultSet = (SQLResultSet) result; + resultSet.maxRows = maxRows; + updateCount = -1; + return true; + } else { + resultSet = null; + updateCount = (Integer) result; + return false; + } + } } diff --git a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java index 3df1aa4..b270139 100644 --- a/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java +++ b/src/test/java/org/tarantool/jdbc/AbstractJdbcIT.java @@ -133,45 +133,40 @@ public abstract class AbstractJdbcIT { conn.close(); } - private static void sqlExec(String[] text) throws IOException { - Socket socket = new Socket(); + private static void sqlExec(String[] text) { + TarantoolConnection con = makeConnection(); try { - socket.connect(new InetSocketAddress(host, port)); - TarantoolConnection con = new TarantoolConnection(user, pass, socket); - try { - for (String cmd : text) - con.eval("box.sql.execute(\"" + cmd + "\")"); - } - finally { - con.close(); - socket = null; - } + for (String cmd : text) + con.eval("box.sql.execute(\"" + cmd + "\")"); + } finally { + con.close(); } - finally { - if (socket != null) - socket.close(); + } + + static List<?> getRow(String space, Object key) { + TarantoolConnection con = makeConnection(); + try { + List<?> l = con.select(281, 2, Arrays.asList(space.toUpperCase()), 0, 1, 0); + Integer spaceId = (Integer) ((List) l.get(0)).get(0); + l = con.select(spaceId, 0, Arrays.asList(key), 0, 1, 0); + return (l == null || l.size() == 0) ? Collections.emptyList() : (List<?>) l.get(0); + } finally { + con.close(); } } - static List<?> getRow(String space, Object key) throws IOException { + static TarantoolConnection makeConnection() { Socket socket = new Socket(); try { socket.connect(new InetSocketAddress(host, port)); - TarantoolConnection con = new TarantoolConnection(user, pass, socket); + return new TarantoolConnection(user, pass, socket); + } catch (IOException e) { try { - List<?> l = con.select(281, 2, Arrays.asList(space.toUpperCase()), 0, 1, 0); - Integer spaceId = (Integer) ((List) l.get(0)).get(0); - l = con.select(spaceId, 0, Arrays.asList(key), 0, 1, 0); - return (l == null || l.size() == 0) ? Collections.emptyList() : (List<?>) l.get(0); - } - finally { - con.close(); - socket = null; - } - } - finally { - if (socket != null) socket.close(); + } catch (IOException ignored) { + // No-op. + } + throw new RuntimeException(e); } } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java b/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java index cc6bfb9..a6a05e8 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcConnectionIT.java @@ -1,16 +1,24 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import org.tarantool.TarantoolConnection; +import java.lang.reflect.Field; +import java.net.Socket; import java.sql.DatabaseMetaData; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Statement; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +@SuppressWarnings("Since15") public class JdbcConnectionIT extends AbstractJdbcIT { @Test public void testCreateStatement() throws SQLException { @@ -39,4 +47,61 @@ public class JdbcConnectionIT extends AbstractJdbcIT { DatabaseMetaData meta = conn.getMetaData(); assertNotNull(meta); } + + @Test + public void testGetSetNetworkTimeout() throws Exception { + assertEquals(0, conn.getNetworkTimeout()); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + conn.setNetworkTimeout(null, -1); + } + }); + assertEquals("Network timeout cannot be negative.", e.getMessage()); + + conn.setNetworkTimeout(null, 3000); + + assertEquals(3000, conn.getNetworkTimeout()); + + // Check that timeout gets propagated to the socket. + Field tntCon = SQLConnection.class.getDeclaredField("connection"); + tntCon.setAccessible(true); + + Field sock = TarantoolConnection.class.getDeclaredField("socket"); + sock.setAccessible(true); + + assertEquals(3000, ((Socket)sock.get(tntCon.get(conn))).getSoTimeout()); + } + + @Test + public void testClosedConnection() throws SQLException { + conn.close(); + + int i = 0; + for (; i < 5; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: conn.createStatement(); + break; + case 1: conn.prepareStatement("TEST"); + break; + case 2: conn.getMetaData(); + break; + case 3: conn.getNetworkTimeout(); + break; + case 4: conn.setNetworkTimeout(null, 1000); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getMessage()); + } + assertEquals(5, i); + } } \ No newline at end of file diff --git a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java index 17ab086..76caa19 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcDatabaseMetaDataIT.java @@ -2,6 +2,7 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.function.Executable; import java.sql.DatabaseMetaData; import java.sql.ResultSet; @@ -12,7 +13,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT { private DatabaseMetaData meta; @@ -183,4 +186,31 @@ public class JdbcDatabaseMetaDataIT extends AbstractJdbcIT { assertEquals(seq, rs.getInt(5)); assertEquals(pkName, rs.getString(6)); } + + @Test + public void testClosedConnection() throws SQLException { + conn.close(); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: meta.getTables(null, null, null, new String[]{"TABLE"}); + break; + case 1: meta.getColumns(null, null, "TEST", null); + break; + case 2: meta.getPrimaryKeys(null, null, "TEST"); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getCause().getMessage()); + } + assertEquals(3, i); + } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcDriverTest.java b/src/test/java/org/tarantool/jdbc/JdbcDriverTest.java new file mode 100644 index 0000000..38ca2d4 --- /dev/null +++ b/src/test/java/org/tarantool/jdbc/JdbcDriverTest.java @@ -0,0 +1,202 @@ +package org.tarantool.jdbc; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import org.tarantool.CommunicationException; + +import java.io.IOException; +import java.net.ServerSocket; +import java.net.Socket; +import java.net.SocketTimeoutException; +import java.net.URI; + +import java.sql.Driver; +import java.sql.DriverManager; +import java.sql.DriverPropertyInfo; +import java.sql.SQLException; +import java.util.Properties; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import static org.tarantool.jdbc.SQLDriver.PROP_HOST; +import static org.tarantool.jdbc.SQLDriver.PROP_PASSWORD; +import static org.tarantool.jdbc.SQLDriver.PROP_PORT; +import static org.tarantool.jdbc.SQLDriver.PROP_SOCKET_PROVIDER; +import static org.tarantool.jdbc.SQLDriver.PROP_SOCKET_TIMEOUT; +import static org.tarantool.jdbc.SQLDriver.PROP_USER; + +public class JdbcDriverTest { + @Test + public void testParseQueryString() throws Exception { + SQLDriver drv = new SQLDriver(); + + Properties prop = new Properties(); + prop.setProperty(PROP_USER, "adm"); + prop.setProperty(PROP_PASSWORD, "secret"); + + URI uri = new URI(String.format( + "tarantool://server.local:3302?%s=%s&%s=%d", + PROP_SOCKET_PROVIDER, "some.class", + PROP_SOCKET_TIMEOUT, 5000)); + + Properties res = drv.parseQueryString(uri, prop); + assertNotNull(res); + + assertEquals("server.local", res.getProperty(PROP_HOST)); + assertEquals("3302", res.getProperty(PROP_PORT)); + assertEquals("adm", res.getProperty(PROP_USER)); + assertEquals("secret", res.getProperty(PROP_PASSWORD)); + assertEquals("some.class", res.getProperty(PROP_SOCKET_PROVIDER)); + assertEquals("5000", res.getProperty(PROP_SOCKET_TIMEOUT)); + } + + @Test + public void testParseQueryStringUserInfoInURI() throws Exception { + SQLDriver drv = new SQLDriver(); + Properties res = drv.parseQueryString(new URI("tarantool://adm:secret@server.local"), null); + assertNotNull(res); + assertEquals("server.local", res.getProperty(PROP_HOST)); + assertEquals("3301", res.getProperty(PROP_PORT)); + assertEquals("adm", res.getProperty(PROP_USER)); + assertEquals("secret", res.getProperty(PROP_PASSWORD)); + } + + @Test + public void testParseQueryStringValidations() { + // Check non-number port + checkParseQueryStringValidation("tarantool://0", + new Properties() {{setProperty(PROP_PORT, "nan");}}, + "Port must be a valid number."); + + // Check zero port + checkParseQueryStringValidation("tarantool://0:0", null, "Port is out of range: 0"); + + // Check high port + checkParseQueryStringValidation("tarantool://0:65536", null, "Port is out of range: 65536"); + + // Check non-number timeout + checkParseQueryStringValidation(String.format("tarantool://0:3301?%s=nan", PROP_SOCKET_TIMEOUT), null, + "Timeout must be a valid number."); + + // Check negative timeout + checkParseQueryStringValidation(String.format("tarantool://0:3301?%s=-100", PROP_SOCKET_TIMEOUT), null, + "Timeout must not be negative."); + } + + @Test + public void testGetPropertyInfo() throws SQLException { + Driver drv = new SQLDriver(); + Properties props = new Properties(); + DriverPropertyInfo[] info = drv.getPropertyInfo("tarantool://server.local:3302", props); + assertNotNull(info); + assertEquals(6, info.length); + + for (DriverPropertyInfo e: info) { + assertNotNull(e.name); + assertNull(e.choices); + assertNotNull(e.description); + + if (PROP_HOST.equals(e.name)) { + assertTrue(e.required); + assertEquals("server.local", e.value); + } else if (PROP_PORT.equals(e.name)) { + assertTrue(e.required); + assertEquals("3302", e.value); + } else if (PROP_USER.equals(e.name)) { + assertFalse(e.required); + assertNull(e.value); + } else if (PROP_PASSWORD.equals(e.name)) { + assertFalse(e.required); + assertNull(e.value); + } else if (PROP_SOCKET_PROVIDER.equals(e.name)) { + assertFalse(e.required); + assertNull(e.value); + } else if (PROP_SOCKET_TIMEOUT.equals(e.name)) { + assertFalse(e.required); + assertEquals("0", e.value); + } else + fail("Unknown property '" + e.name + "'"); + } + } + + @Test + public void testCustomSocketProviderFail() throws SQLException { + checkCustomSocketProviderFail("nosuchclassexists", + "Couldn't instantiate socket provider"); + + checkCustomSocketProviderFail(Integer.class.getName(), + "The socket provider java.lang.Integer does not implement org.tarantool.jdbc.SQLSocketProvider"); + + checkCustomSocketProviderFail(TestSQLProviderThatReturnsNull.class.getName(), + "The socket provider returned null socket"); + + checkCustomSocketProviderFail(TestSQLProviderThatThrows.class.getName(), + "Couldn't initiate connection using"); + } + + @Test + public void testNoResponseAfterInitialConnect() throws IOException { + ServerSocket socket = new ServerSocket(); + socket.bind(null, 0); + try { + final String url = "tarantool://localhost:" + socket.getLocalPort(); + final Properties prop = new Properties(); + prop.setProperty(PROP_SOCKET_TIMEOUT, "100"); + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + DriverManager.getConnection(url, prop); + } + }); + assertTrue(e.getMessage().startsWith("Couldn't initiate connection using "), e.getMessage()); + assertTrue(e.getCause() instanceof CommunicationException); + assertTrue(e.getCause().getCause() instanceof SocketTimeoutException); + } finally { + socket.close(); + } + } + + private void checkCustomSocketProviderFail(String providerClassName, String errMsg) throws SQLException { + final Driver drv = DriverManager.getDriver("tarantool:"); + final Properties prop = new Properties(); + prop.setProperty(PROP_SOCKET_PROVIDER, providerClassName); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + drv.connect("tarantool://0:3301", prop); + } + }); + assertTrue(e.getMessage().startsWith(errMsg), e.getMessage()); + } + + private void checkParseQueryStringValidation(final String uri, final Properties prop, String errMsg) { + final SQLDriver drv = new SQLDriver(); + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + drv.parseQueryString(new URI(uri), prop); + } + }); + assertTrue(e.getMessage().startsWith(errMsg), e.getMessage()); + } + + static class TestSQLProviderThatReturnsNull implements SQLSocketProvider { + @Override + public Socket getConnectedSocket(URI uri, Properties params) { + return null; + } + } + + static class TestSQLProviderThatThrows implements SQLSocketProvider { + @Override + public Socket getConnectedSocket(URI uri, Properties params) { + throw new RuntimeException("ERROR"); + } + } +} diff --git a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java index 8cc7acc..363ae32 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java +++ b/src/test/java/org/tarantool/jdbc/JdbcExceptionHandlingTest.java @@ -2,24 +2,41 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.api.function.ThrowingConsumer; +import org.tarantool.CommunicationException; import org.tarantool.TarantoolConnection; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.net.SocketException; +import java.net.SocketTimeoutException; import java.sql.DatabaseMetaData; +import java.sql.PreparedStatement; import java.sql.SQLException; +import java.sql.Statement; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Properties; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.tarantool.jdbc.SQLDatabaseMetadata.FORMAT_IDX; import static org.tarantool.jdbc.SQLDatabaseMetadata.INDEX_FORMAT_IDX; import static org.tarantool.jdbc.SQLDatabaseMetadata.SPACE_ID_IDX; +import static org.tarantool.jdbc.SQLDatabaseMetadata.SPACES_MAX; import static org.tarantool.jdbc.SQLDatabaseMetadata._VINDEX; import static org.tarantool.jdbc.SQLDatabaseMetadata._VSPACE; +import static org.tarantool.jdbc.SQLDriver.PROP_SOCKET_TIMEOUT; public class JdbcExceptionHandlingTest { /** @@ -30,7 +47,7 @@ public class JdbcExceptionHandlingTest { @Test public void testDatabaseMetaDataGetPrimaryKeysFormatError() throws SQLException { TarantoolConnection tntCon = mock(TarantoolConnection.class); - SQLConnection conn = new SQLConnection(tntCon, "", new Properties()); + SQLConnection conn = buildTestSQLConnection(tntCon, "", SQLDriver.defaults); Object[] spc = new Object[7]; spc[FORMAT_IDX] = Collections.singletonList(new HashMap<String, Object>()); @@ -57,4 +74,226 @@ public class JdbcExceptionHandlingTest { assertTrue(t.getCause().getMessage().contains("Wrong value type")); } + + @Test + public void testStatementCommunicationException() throws SQLException { + checkStatementCommunicationException(new ThrowingConsumer<Statement>() { + @Override + public void accept(Statement statement) throws Throwable { + statement.executeQuery("TEST"); + } + }); + checkStatementCommunicationException(new ThrowingConsumer<Statement>() { + @Override + public void accept(Statement statement) throws Throwable { + statement.executeUpdate("TEST"); + } + }); + checkStatementCommunicationException(new ThrowingConsumer<Statement>() { + @Override + public void accept(Statement statement) throws Throwable { + statement.execute("TEST"); + } + }); + } + + @Test + public void testPreparedStatementCommunicationException() throws SQLException { + checkPreparedStatementCommunicationException(new ThrowingConsumer<PreparedStatement>() { + @Override + public void accept(PreparedStatement prep) throws Throwable { + prep.executeQuery(); + } + }); + checkPreparedStatementCommunicationException(new ThrowingConsumer<PreparedStatement>() { + @Override + public void accept(PreparedStatement prep) throws Throwable { + prep.executeUpdate(); + } + }); + checkPreparedStatementCommunicationException(new ThrowingConsumer<PreparedStatement>() { + @Override + public void accept(PreparedStatement prep) throws Throwable { + prep.execute(); + } + }); + } + + @Test + public void testDatabaseMetaDataCommunicationException() throws SQLException { + checkDatabaseMetaDataCommunicationException(new ThrowingConsumer<DatabaseMetaData>() { + @Override + public void accept(DatabaseMetaData meta) throws Throwable { + meta.getTables(null, null, null, new String[] {"TABLE"}); + } + }, "Failed to retrieve table(s) description: tableNamePattern=\"null\"."); + + checkDatabaseMetaDataCommunicationException(new ThrowingConsumer<DatabaseMetaData>() { + @Override + public void accept(DatabaseMetaData meta) throws Throwable { + meta.getColumns(null, null, "TEST", "ID"); + } + }, "Error processing table column metadata: tableNamePattern=\"TEST\"; columnNamePattern=\"ID\"."); + + checkDatabaseMetaDataCommunicationException(new ThrowingConsumer<DatabaseMetaData>() { + @Override + public void accept(DatabaseMetaData meta) throws Throwable { + meta.getPrimaryKeys(null, null, "TEST"); + } + }, "Error processing metadata for table \"TEST\"."); + } + + @Test + public void testDefaultSocketProviderConnectTimeoutError() throws IOException { + final int socketTimeout = 3000; + final Socket mockSocket = mock(Socket.class); + + SocketTimeoutException timeoutEx = new SocketTimeoutException(); + doThrow(timeoutEx) + .when(mockSocket) + .connect(new InetSocketAddress("localhost", 3301), socketTimeout); + + final Properties prop = new Properties(SQLDriver.defaults); + prop.setProperty(PROP_SOCKET_TIMEOUT, String.valueOf(socketTimeout)); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + new SQLConnection("tarantool://localhost:3301", prop) { + @Override + protected Socket makeSocket() { + return mockSocket; + } + }; + } + }); + + assertTrue(e.getMessage().startsWith("Couldn't connect to localhost:3301"), e.getMessage()); + assertEquals(timeoutEx, e.getCause()); + } + + @Test + public void testDefaultSocketProviderSetSocketTimeoutError() throws IOException { + final int socketTimeout = 3000; + final Socket mockSocket = mock(Socket.class); + + // Check error setting socket timeout + reset(mockSocket); + doNothing() + .when(mockSocket) + .connect(new InetSocketAddress("localhost", 3301), socketTimeout); + + SocketException sockEx = new SocketException("TEST"); + doThrow(sockEx) + .when(mockSocket) + .setSoTimeout(socketTimeout); + + final Properties prop = new Properties(SQLDriver.defaults); + prop.setProperty(PROP_SOCKET_TIMEOUT, String.valueOf(socketTimeout)); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + new SQLConnection("tarantool://localhost:3301", prop) { + @Override + protected Socket makeSocket() { + return mockSocket; + } + }; + } + }); + + assertTrue(e.getMessage().startsWith("Couldn't set socket timeout."), e.getMessage()); + assertEquals(sockEx, e.getCause()); + } + + private void checkStatementCommunicationException(final ThrowingConsumer<Statement> consumer) + throws SQLException { + TestTarantoolConnection mockCon = mock(TestTarantoolConnection.class); + final Statement stmt = new SQLStatement(buildTestSQLConnection(mockCon, "tarantool://0:0", SQLDriver.defaults)); + + Exception ex = new CommunicationException("TEST"); + + doThrow(ex).when(mockCon).sql("TEST", new Object[0]); + doThrow(ex).when(mockCon).update("TEST"); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + consumer.accept(stmt); + } + }); + assertTrue(e.getMessage().startsWith("Failed to execute"), e.getMessage()); + + assertEquals(ex, e.getCause()); + + verify(mockCon, times(1)).close(); + } + + private void checkPreparedStatementCommunicationException(final ThrowingConsumer<PreparedStatement> consumer) + throws SQLException { + TestTarantoolConnection mockCon = mock(TestTarantoolConnection.class); + + final PreparedStatement prep = new SQLPreparedStatement( + buildTestSQLConnection(mockCon, "tarantool://0:0", SQLDriver.defaults), "TEST"); + + Exception ex = new CommunicationException("TEST"); + doThrow(ex).when(mockCon).sql("TEST", new Object[0]); + doThrow(ex).when(mockCon).update("TEST"); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + consumer.accept(prep); + } + }); + assertTrue(e.getMessage().startsWith("Failed to execute"), e.getMessage()); + + assertEquals(ex, e.getCause()); + + verify(mockCon, times(1)).close(); + } + + private void checkDatabaseMetaDataCommunicationException(final ThrowingConsumer<DatabaseMetaData> consumer, + String msg) throws SQLException { + TestTarantoolConnection mockCon = mock(TestTarantoolConnection.class); + SQLConnection conn = buildTestSQLConnection(mockCon, "tarantool://0:0", new Properties(SQLDriver.defaults)); + final DatabaseMetaData meta = conn.getMetaData(); + + Exception ex = new CommunicationException("TEST"); + doThrow(ex).when(mockCon).select(_VSPACE, 0, Arrays.asList(), 0, SPACES_MAX, 0); + doThrow(ex).when(mockCon).select(_VSPACE, 2, Arrays.asList("TEST"), 0, 1, 0); + + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + consumer.accept(meta); + } + }); + assertTrue(e.getMessage().startsWith(msg), e.getMessage()); + + assertEquals(ex, e.getCause().getCause()); + + verify(mockCon, times(1)).close(); + } + + private SQLConnection buildTestSQLConnection(final TarantoolConnection tntCon, String url, Properties properties) + throws SQLException { + return new SQLConnection(url, properties) { + @Override + protected TarantoolConnection makeConnection (String user, String pass, Socket socket) { + return tntCon; + } + }; + } + + class TestTarantoolConnection extends TarantoolConnection { + TestTarantoolConnection() throws IOException { + super(null, null, mock(Socket.class)); + } + @Override + protected void sql(String sql, Object[] bind) { + super.sql(sql, bind); + } + } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java b/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java index 68628ef..ad8fae7 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcPreparedStatementIT.java @@ -2,6 +2,7 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.function.Executable; import java.math.BigDecimal; import java.sql.Date; @@ -14,7 +15,10 @@ import java.sql.Timestamp; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; public class JdbcPreparedStatementIT extends AbstractJdbcIT { private PreparedStatement prep; @@ -130,4 +134,94 @@ public class JdbcPreparedStatementIT extends AbstractJdbcIT { rs.close(); } + + @Test + public void testExecuteReturnsResultSet() throws SQLException { + prep = conn.prepareStatement("SELECT val FROM test WHERE id=?"); + assertNotNull(prep); + prep.setInt(1, 1); + + assertTrue(prep.execute()); + assertEquals(-1, prep.getUpdateCount()); + + ResultSet rs = prep.getResultSet(); + assertNotNull(rs); + assertTrue(rs.next()); + assertEquals("one", rs.getString(1)); + assertFalse(rs.next()); + rs.close(); + } + + @Test + public void testExecuteReturnsUpdateCount() throws Exception { + prep = conn.prepareStatement("INSERT INTO test VALUES(?, ?), (?, ?)"); + assertNotNull(prep); + + prep.setInt(1, 10); + prep.setString(2, "ten"); + prep.setInt(3, 20); + prep.setString(4, "twenty"); + + assertFalse(prep.execute()); + assertNull(prep.getResultSet()); + assertEquals(2, prep.getUpdateCount()); + + assertEquals("ten", getRow("test", 10).get(1)); + assertEquals("twenty", getRow("test", 20).get(1)); + } + + @Test void testForbiddenMethods() throws SQLException { + prep = conn.prepareStatement("TEST"); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: prep.executeQuery("TEST"); + break; + case 1: prep.executeUpdate("TEST"); + break; + case 2: prep.execute("TEST"); + break; + default: + fail(); + } + } + }); + assertEquals("The method cannot be called on a PreparedStatement.", e.getMessage()); + } + assertEquals(3, i); + } + + @Test + public void testClosedConnection() throws SQLException { + prep = conn.prepareStatement("TEST"); + + conn.close(); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: prep.executeQuery(); + break; + case 1: prep.executeUpdate(); + break; + case 2: prep.execute(); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getMessage()); + } + assertEquals(3, i); + } } diff --git a/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java b/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java index 925556d..735f326 100644 --- a/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java +++ b/src/test/java/org/tarantool/jdbc/JdbcStatementIT.java @@ -3,6 +3,7 @@ package org.tarantool.jdbc; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; import java.sql.ResultSet; import java.sql.SQLException; @@ -10,8 +11,10 @@ import java.sql.Statement; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; public class JdbcStatementIT extends AbstractJdbcIT { private Statement stmt; @@ -63,4 +66,31 @@ public class JdbcStatementIT extends AbstractJdbcIT { assertEquals("hundred", getRow("test", 100).get(1)); assertEquals("thousand", getRow("test", 1000).get(1)); } + + @Test + public void testClosedConnection() throws Exception { + conn.close(); + + int i = 0; + for (; i < 3; i++) { + final int step = i; + SQLException e = assertThrows(SQLException.class, new Executable() { + @Override + public void execute() throws Throwable { + switch (step) { + case 0: stmt.executeQuery("TEST"); + break; + case 1: stmt.executeUpdate("TEST"); + break; + case 2: stmt.execute("TEST"); + break; + default: + fail(); + } + } + }); + assertEquals("Connection is closed.", e.getMessage()); + } + assertEquals(3, i); + } } \ No newline at end of file -- 1.8.3.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH] jdbc: add connection timeout configuration and handling 2018-12-05 11:16 ` Sergei Kalashnikov @ 2018-12-10 12:59 ` Alexander Turenko 0 siblings, 0 replies; 6+ messages in thread From: Alexander Turenko @ 2018-12-10 12:59 UTC (permalink / raw) To: Sergei Kalashnikov; +Cc: tarantool-patches Thank you for all that work! Pushed to connector-1.8.jdbc. WBR, Alexander Turenko. On Wed, Dec 05, 2018 at 02:16:23PM +0300, Sergei Kalashnikov wrote: > Hi Alexander! Thank you for review. > Please find my answers and the patch below. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-10 12:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-12 15:47 [tarantool-patches] [PATCH] jdbc: add connection timeout configuration and handling Sergei Kalashnikov 2018-10-22 4:21 ` [tarantool-patches] " Alexander Turenko 2018-11-15 17:22 ` Sergei Kalashnikov 2018-11-26 15:01 ` Alexander Turenko 2018-12-05 11:16 ` Sergei Kalashnikov 2018-12-10 12:59 ` Alexander Turenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox