diff --git a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java index a6bf6c09..6b603c14 100644 --- a/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/CacheDataSource.java @@ -152,7 +152,7 @@ public class CacheDataSource implements DataSource { } @Override - public void close() { + public void closeConnection() { executorService.shutdown(); try { executorService.awaitTermination(5, TimeUnit.SECONDS); @@ -160,7 +160,7 @@ public class CacheDataSource implements DataSource { ConsoleLogger.logException("Could not close executor service:", e); } cachedAuths.invalidateAll(); - source.close(); + source.closeConnection(); } @Override diff --git a/src/main/java/fr/xephi/authme/datasource/DataSource.java b/src/main/java/fr/xephi/authme/datasource/DataSource.java index 9d30f933..14faaf39 100644 --- a/src/main/java/fr/xephi/authme/datasource/DataSource.java +++ b/src/main/java/fr/xephi/authme/datasource/DataSource.java @@ -130,7 +130,7 @@ public interface DataSource extends Reloadable { /** * Close the underlying connections to the data source. */ - void close(); + void closeConnection(); /** * Return the data source type. diff --git a/src/main/java/fr/xephi/authme/datasource/FlatFile.java b/src/main/java/fr/xephi/authme/datasource/FlatFile.java index a59617c4..0958e126 100644 --- a/src/main/java/fr/xephi/authme/datasource/FlatFile.java +++ b/src/main/java/fr/xephi/authme/datasource/FlatFile.java @@ -270,7 +270,7 @@ public class FlatFile implements DataSource { } @Override - public synchronized void close() { + public void closeConnection() { } @Override diff --git a/src/main/java/fr/xephi/authme/datasource/MySQL.java b/src/main/java/fr/xephi/authme/datasource/MySQL.java index cffb8263..27448f07 100644 --- a/src/main/java/fr/xephi/authme/datasource/MySQL.java +++ b/src/main/java/fr/xephi/authme/datasource/MySQL.java @@ -29,6 +29,9 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import static fr.xephi.authme.datasource.SqlDataSourceUtils.close; +import static fr.xephi.authme.datasource.SqlDataSourceUtils.logSqlException; + public class MySQL implements DataSource { private boolean useSsl; @@ -74,7 +77,7 @@ public class MySQL implements DataSource { try { checkTablesAndColumns(); } catch (SQLException e) { - close(); + closeConnection(); ConsoleLogger.logException("Can't initialize the MySQL database:", e); ConsoleLogger.warning("Please check your database settings in the config.yml file!"); throw e; @@ -742,7 +745,7 @@ public class MySQL implements DataSource { } @Override - public void close() { + public void closeConnection() { if (ds != null && !ds.isClosed()) { ds.close(); } @@ -1039,29 +1042,4 @@ public class MySQL implements DataSource { ConsoleLogger.warning("You may have entries with invalid timestamps. Please check your data " + "before purging. " + changedRows + " rows were migrated from seconds to milliseconds."); } - - private static void logSqlException(SQLException e) { - ConsoleLogger.logException("Error during SQL operation:", e); - } - - private static void close(ResultSet rs) { - try { - if (rs != null && !rs.isClosed()) { - rs.close(); - } - } catch (SQLException e) { - ConsoleLogger.logException("Could not close ResultSet", e); - } - } - - private static void close(PreparedStatement pst) { - try { - if (pst != null && !pst.isClosed()) { - pst.close(); - } - } catch (SQLException e) { - ConsoleLogger.logException("Could not close PreparedStatement", e); - } - } - } diff --git a/src/main/java/fr/xephi/authme/datasource/SQLite.java b/src/main/java/fr/xephi/authme/datasource/SQLite.java index d564224a..4b52922d 100644 --- a/src/main/java/fr/xephi/authme/datasource/SQLite.java +++ b/src/main/java/fr/xephi/authme/datasource/SQLite.java @@ -21,6 +21,9 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import static fr.xephi.authme.datasource.SqlDataSourceUtils.close; +import static fr.xephi.authme.datasource.SqlDataSourceUtils.logSqlException; + /** */ public class SQLite implements DataSource { @@ -60,10 +63,6 @@ public class SQLite implements DataSource { this.con = connection; } - private static void logSqlException(SQLException e) { - ConsoleLogger.logException("Error while executing SQL statement:", e); - } - private void connect() throws ClassNotFoundException, SQLException { Class.forName("org.sqlite.JDBC"); ConsoleLogger.info("SQLite driver loaded"); @@ -384,7 +383,7 @@ public class SQLite implements DataSource { } @Override - public void close() { + public void closeConnection() { try { if (con != null && !con.isClosed()) { con.close(); @@ -394,36 +393,6 @@ public class SQLite implements DataSource { } } - private static void close(Statement st) { - if (st != null) { - try { - st.close(); - } catch (SQLException ex) { - logSqlException(ex); - } - } - } - - private static void close(Connection con) { - if (con != null) { - try { - con.close(); - } catch (SQLException ex) { - logSqlException(ex); - } - } - } - - private static void close(ResultSet rs) { - if (rs != null) { - try { - rs.close(); - } catch (SQLException ex) { - logSqlException(ex); - } - } - } - @Override public List getAllAuthsByIp(String ip) { PreparedStatement pst = null; diff --git a/src/main/java/fr/xephi/authme/datasource/SqlDataSourceUtils.java b/src/main/java/fr/xephi/authme/datasource/SqlDataSourceUtils.java new file mode 100644 index 00000000..7a25b75c --- /dev/null +++ b/src/main/java/fr/xephi/authme/datasource/SqlDataSourceUtils.java @@ -0,0 +1,75 @@ +package fr.xephi.authme.datasource; + +import fr.xephi.authme.ConsoleLogger; + +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; + +/** + * Utilities for SQL data sources. + */ +final class SqlDataSourceUtils { + + private SqlDataSourceUtils() { + } + + /** + * Logs a SQL exception. + * + * @param e the exception to log + */ + static void logSqlException(SQLException e) { + ConsoleLogger.logException("Error during SQL operation:", e); + } + + + // We use overloaded close() methods instead of one close(AutoCloseable) method in order to limit the + // checked exceptions to SQLException, which is the only checked exception these classes throw. + + /** + * Closes a {@link ResultSet} safely. + * + * @param rs the result set to close + */ + static void close(ResultSet rs) { + try { + if (rs != null && !rs.isClosed()) { + rs.close(); + } + } catch (SQLException e) { + ConsoleLogger.logException("Could not close ResultSet", e); + } + } + + /** + * Closes a {@link Statement} safely. + * + * @param st the statement set to close + */ + static void close(Statement st) { + try { + if (st != null && !st.isClosed()) { + st.close(); + } + } catch (SQLException e) { + ConsoleLogger.logException("Could not close Statement", e); + } + } + + /** + * Closes a {@link Connection} safely. + * + * @param con the connection set to close + */ + static void close(Connection con) { + try { + if (con != null && !con.isClosed()) { + con.close(); + } + } catch (SQLException e) { + ConsoleLogger.logException("Could not close Connection", e); + } + } +} diff --git a/src/main/java/fr/xephi/authme/initialization/TaskCloser.java b/src/main/java/fr/xephi/authme/initialization/TaskCloser.java index c1126975..7cf1b0a7 100644 --- a/src/main/java/fr/xephi/authme/initialization/TaskCloser.java +++ b/src/main/java/fr/xephi/authme/initialization/TaskCloser.java @@ -70,7 +70,7 @@ public class TaskCloser implements Runnable { } if (dataSource != null) { - dataSource.close(); + dataSource.closeConnection(); } } diff --git a/src/test/java/fr/xephi/authme/datasource/SqlDataSourceUtilsTest.java b/src/test/java/fr/xephi/authme/datasource/SqlDataSourceUtilsTest.java new file mode 100644 index 00000000..94650a56 --- /dev/null +++ b/src/test/java/fr/xephi/authme/datasource/SqlDataSourceUtilsTest.java @@ -0,0 +1,135 @@ +package fr.xephi.authme.datasource; + +import fr.xephi.authme.TestHelper; +import org.junit.Before; +import org.junit.Test; + +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.logging.Logger; + +import static org.hamcrest.Matchers.containsString; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.hamcrest.MockitoHamcrest.argThat; +import static org.mockito.Mockito.verify; + +/** + * Test for {@link SqlDataSourceUtils}. + */ +public class SqlDataSourceUtilsTest { + + private Logger logger; + + @Before + public void initLogger() { + logger = TestHelper.setupLogger(); + } + + @Test + public void shouldHaveHiddenConstructor() { + TestHelper.validateHasOnlyPrivateEmptyConstructor(SqlDataSourceUtils.class); + } + + @Test + public void shouldLogException() { + // given + String msg = "Hocus pocus did not work"; + SQLException ex = new SQLException(msg); + + // when + SqlDataSourceUtils.logSqlException(ex); + + // then + verify(logger).warning(argThat(containsString(msg))); + } + + @Test + public void shouldCloseStatement() throws SQLException { + // given + Statement st = mock(Statement.class); + + // when + SqlDataSourceUtils.close(st); + + // then + verify(st).close(); + } + + @Test + public void shouldHandleExceptionFromStatement() throws SQLException { + // given + Statement st = mock(Statement.class); + doThrow(SQLException.class).when(st).close(); + + // when + SqlDataSourceUtils.close(st); + + // then + verify(logger).warning(anyString()); + } + + @Test + public void shouldCloseResultSet() throws SQLException { + // given + ResultSet rs = mock(ResultSet.class); + + // when + SqlDataSourceUtils.close(rs); + + // then + verify(rs).close(); + } + + @Test + public void shouldHandleExceptionFromResultSet() throws SQLException { + // given + ResultSet rs = mock(ResultSet.class); + doThrow(SQLException.class).when(rs).close(); + + // when + SqlDataSourceUtils.close(rs); + + // then + verify(logger).warning(anyString()); + } + + @Test + public void shouldCloseConnection() throws SQLException { + // given + Connection con = mock(Connection.class); + + // when + SqlDataSourceUtils.close(con); + + // then + verify(con).close(); + } + + @Test + public void shouldHandleExceptionFromConnection() throws SQLException { + // given + Connection con = mock(Connection.class); + doThrow(SQLException.class).when(con).close(); + + // when + SqlDataSourceUtils.close(con); + + // then + verify(logger).warning(anyString()); + } + + @Test + public void shouldHandleNullArgument() { + // given / when + SqlDataSourceUtils.close((Statement) null); + SqlDataSourceUtils.close((ResultSet) null); + SqlDataSourceUtils.close((Connection) null); + + // then - nothing happens + } + +} diff --git a/src/test/java/fr/xephi/authme/initialization/TaskCloserTest.java b/src/test/java/fr/xephi/authme/initialization/TaskCloserTest.java index cb1e6cb4..83efd0eb 100644 --- a/src/test/java/fr/xephi/authme/initialization/TaskCloserTest.java +++ b/src/test/java/fr/xephi/authme/initialization/TaskCloserTest.java @@ -78,7 +78,7 @@ public class TaskCloserTest { verify(bukkitScheduler, times(3)).isCurrentlyRunning(taskIds.capture()); assertThat(taskIds.getAllValues(), contains(ACTIVE_WORKERS_ID[0], ACTIVE_WORKERS_ID[1], ACTIVE_WORKERS_ID[1])); verify(taskCloser, times(2)).sleep(); - verify(dataSource).close(); + verify(dataSource).closeConnection(); } @Test @@ -97,7 +97,7 @@ public class TaskCloserTest { verify(bukkitScheduler, times(3)).isQueued(anyInt()); verify(bukkitScheduler, times(61)).isCurrentlyRunning(anyInt()); verify(taskCloser, times(60)).sleep(); - verify(dataSource).close(); + verify(dataSource).closeConnection(); } @Test