From: Sergei Kalashnikov <ztarvos@gmail.com> To: Alexander Turenko <alexander.turenko@tarantool.org> Cc: tarantool-patches <tarantool-patches@freelists.org> Subject: [tarantool-patches] Re: [PATCH] jdbc: add connection timeout configuration and handling Date: Thu, 15 Nov 2018 20:22:09 +0300 [thread overview] Message-ID: <20181115172207.GA7831@daedra.localdomain> (raw) In-Reply-To: <20181022042130.4qf2lzj3pezfxi63@tkn_work_nb> 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
next prev parent reply other threads:[~2018-11-15 17:23 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-12 15:47 [tarantool-patches] " Sergei Kalashnikov 2018-10-22 4:21 ` [tarantool-patches] " Alexander Turenko 2018-11-15 17:22 ` Sergei Kalashnikov [this message] 2018-11-26 15:01 ` Alexander Turenko 2018-12-05 11:16 ` Sergei Kalashnikov 2018-12-10 12:59 ` Alexander Turenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20181115172207.GA7831@daedra.localdomain \ --to=ztarvos@gmail.com \ --cc=alexander.turenko@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH] jdbc: add connection timeout configuration and handling' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox