Create resource closing test

- Generic test for MySQL and SQLite checking that all generated (Prepared)Statement and ResultSet instances are closed afterwards
- Fix offending code for test to pass
This commit is contained in:
ljacqu 2016-02-27 22:45:39 +01:00
parent 5fce849ce7
commit 63b31b0814
5 changed files with 440 additions and 64 deletions

View File

@ -42,6 +42,10 @@ public class MySQL implements DataSource {
private final HashAlgorithm hashAlgorithm;
private HikariDataSource ds;
private final String phpBbPrefix;
private final int phpBbGroup;
private final String wordpressPrefix;
public MySQL(NewSetting settings) throws ClassNotFoundException, SQLException, PoolInitializationException {
this.host = settings.getProperty(DatabaseSettings.MYSQL_HOST);
this.port = settings.getProperty(DatabaseSettings.MYSQL_PORT);
@ -52,6 +56,9 @@ public class MySQL implements DataSource {
this.columnOthers = settings.getProperty(HooksSettings.MYSQL_OTHER_USERNAME_COLS);
this.col = new Columns(settings);
this.hashAlgorithm = settings.getProperty(SecuritySettings.PASSWORD_HASH);
this.phpBbPrefix = settings.getProperty(HooksSettings.PHPBB_TABLE_PREFIX);
this.phpBbGroup = settings.getProperty(HooksSettings.PHPBB_ACTIVATED_GROUP_ID);
this.wordpressPrefix = settings.getProperty(HooksSettings.WORDPRESS_TABLE_PREFIX);
// Set the connection arguments (and check if connection is ok)
try {
@ -93,6 +100,9 @@ public class MySQL implements DataSource {
this.columnOthers = settings.getProperty(HooksSettings.MYSQL_OTHER_USERNAME_COLS);
this.col = new Columns(settings);
this.hashAlgorithm = settings.getProperty(SecuritySettings.PASSWORD_HASH);
this.phpBbPrefix = settings.getProperty(HooksSettings.PHPBB_TABLE_PREFIX);
this.phpBbGroup = settings.getProperty(HooksSettings.PHPBB_ACTIVATED_GROUP_ID);
this.wordpressPrefix = settings.getProperty(HooksSettings.WORDPRESS_TABLE_PREFIX);
ds = hikariDataSource;
}
@ -361,10 +371,10 @@ public class MySQL implements DataSource {
if (rs.next()) {
int id = rs.getInt(col.ID);
// Insert player in phpbb_user_group
sql = "INSERT INTO " + Settings.getPhpbbPrefix
sql = "INSERT INTO " + phpBbPrefix
+ "user_group (group_id, user_id, group_leader, user_pending) VALUES (?,?,?,?);";
pst2 = con.prepareStatement(sql);
pst2.setInt(1, Settings.getPhpbbGroup);
pst2.setInt(1, phpBbGroup);
pst2.setInt(2, id);
pst2.setInt(3, 0);
pst2.setInt(4, 0);
@ -382,7 +392,7 @@ public class MySQL implements DataSource {
sql = "UPDATE " + tableName + " SET " + tableName
+ ".group_id=? WHERE " + col.NAME + "=?;";
pst2 = con.prepareStatement(sql);
pst2.setInt(1, Settings.getPhpbbGroup);
pst2.setInt(1, phpBbGroup);
pst2.setString(2, auth.getNickname());
pst2.executeUpdate();
pst2.close();
@ -405,7 +415,7 @@ public class MySQL implements DataSource {
pst2.executeUpdate();
pst2.close();
// Increment num_users
sql = "UPDATE " + Settings.getPhpbbPrefix
sql = "UPDATE " + phpBbPrefix
+ "config SET config_value = config_value + 1 WHERE config_name = 'num_users';";
pst2 = con.prepareStatement(sql);
pst2.executeUpdate();
@ -419,7 +429,7 @@ public class MySQL implements DataSource {
rs = pst.executeQuery();
if (rs.next()) {
int id = rs.getInt(col.ID);
sql = "INSERT INTO " + Settings.getWordPressPrefix + "usermeta (user_id, meta_key, meta_value) VALUES (?,?,?);";
sql = "INSERT INTO " + wordpressPrefix + "usermeta (user_id, meta_key, meta_value) VALUES (?,?,?);";
pst2 = con.prepareStatement(sql);
// First Name
pst2.setInt(1, id);
@ -622,31 +632,32 @@ public class MySQL implements DataSource {
@Override
public synchronized boolean removeAuth(String user) {
user = user.toLowerCase();
try (Connection con = getConnection()) {
String sql;
PreparedStatement pst;
String sql = "DELETE FROM " + tableName + " WHERE " + col.NAME + "=?;";
PreparedStatement xfSelect = null;
PreparedStatement xfDelete = null;
try (Connection con = getConnection(); PreparedStatement pst = con.prepareStatement(sql)) {
if (hashAlgorithm == HashAlgorithm.XFBCRYPT) {
sql = "SELECT " + col.ID + " FROM " + tableName + " WHERE " + col.NAME + "=?;";
pst = con.prepareStatement(sql);
pst.setString(1, user);
ResultSet rs = pst.executeQuery();
xfSelect = con.prepareStatement(sql);
xfSelect.setString(1, user);
try (ResultSet rs = xfSelect.executeQuery()) {
if (rs.next()) {
int id = rs.getInt(col.ID);
sql = "DELETE FROM xf_user_authenticate WHERE " + col.ID + "=?;";
PreparedStatement st = con.prepareStatement(sql);
st.setInt(1, id);
st.executeUpdate();
st.close();
xfDelete = con.prepareStatement(sql);
xfDelete.setInt(1, id);
xfDelete.executeUpdate();
}
}
rs.close();
pst.close();
}
pst = con.prepareStatement("DELETE FROM " + tableName + " WHERE " + col.NAME + "=?;");
pst.setString(1, user);
pst.executeUpdate();
return true;
} catch (SQLException ex) {
logSqlException(ex);
} finally {
close(xfSelect);
close(xfDelete);
}
return false;
}
@ -881,13 +892,14 @@ public class MySQL implements DataSource {
@Override
public List<PlayerAuth> getLoggedPlayers() {
List<PlayerAuth> auths = new ArrayList<>();
try (Connection con = getConnection()) {
String sql = "SELECT * FROM " + tableName + " WHERE " + col.IS_LOGGED + "=1;";
try (Connection con = getConnection();
Statement st = con.createStatement();
ResultSet rs = st.executeQuery("SELECT * FROM " + tableName + " WHERE " + col.IS_LOGGED + "=1;");
PreparedStatement pst = con.prepareStatement("SELECT data FROM xf_user_authenticate WHERE " + col.ID + "=?;");
ResultSet rs = st.executeQuery(sql)) {
while (rs.next()) {
PlayerAuth pAuth = buildAuthFromResultSet(rs);
if (hashAlgorithm == HashAlgorithm.XFBCRYPT) {
try (PreparedStatement pst = con.prepareStatement("SELECT data FROM xf_user_authenticate WHERE " + col.ID + "=?;")) {
int id = rs.getInt(col.ID);
pst.setInt(1, id);
ResultSet rs2 = pst.executeQuery();
@ -898,6 +910,7 @@ public class MySQL implements DataSource {
}
rs2.close();
}
}
auths.add(pAuth);
}
} catch (SQLException ex) {
@ -980,13 +993,23 @@ public class MySQL implements DataSource {
}
private static void close(ResultSet rs) {
if (rs != null) {
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);
}
}
}

View File

@ -423,17 +423,14 @@ public class SQLite implements DataSource {
@Override
public void purgeBanned(List<String> banned) {
PreparedStatement pst = null;
try {
String sql = "DELETE FROM " + tableName + " WHERE " + col.NAME + "=?;";
try (PreparedStatement pst = con.prepareStatement(sql)) {
for (String name : banned) {
pst = con.prepareStatement("DELETE FROM " + tableName + " WHERE " + col.NAME + "=?;");
pst.setString(1, name);
pst.executeUpdate();
}
} catch (SQLException ex) {
logSqlException(ex);
} finally {
close(pst);
}
}
@ -509,18 +506,13 @@ public class SQLite implements DataSource {
@Override
public int getAccountsRegistered() {
PreparedStatement pst = null;
ResultSet rs;
try {
pst = con.prepareStatement("SELECT COUNT(*) FROM " + tableName + ";");
rs = pst.executeQuery();
if (rs != null && rs.next()) {
String sql = "SELECT COUNT(*) FROM " + tableName + ";";
try (PreparedStatement pst = con.prepareStatement(sql); ResultSet rs = pst.executeQuery()) {
if (rs.next()) {
return rs.getInt(1);
}
} catch (SQLException ex) {
logSqlException(ex);
} finally {
close(pst);
}
return 0;
}
@ -556,19 +548,14 @@ public class SQLite implements DataSource {
@Override
public List<PlayerAuth> getAllAuths() {
List<PlayerAuth> auths = new ArrayList<>();
PreparedStatement pst = null;
ResultSet rs;
try {
pst = con.prepareStatement("SELECT * FROM " + tableName + ";");
rs = pst.executeQuery();
String sql = "SELECT * FROM " + tableName + ";";
try (PreparedStatement pst = con.prepareStatement(sql); ResultSet rs = pst.executeQuery()) {
while (rs.next()) {
PlayerAuth auth = buildAuthFromResultSet(rs);
auths.add(auth);
}
} catch (SQLException ex) {
logSqlException(ex);
} finally {
close(pst);
}
return auths;
}
@ -576,19 +563,14 @@ public class SQLite implements DataSource {
@Override
public List<PlayerAuth> getLoggedPlayers() {
List<PlayerAuth> auths = new ArrayList<>();
PreparedStatement pst = null;
ResultSet rs;
try {
pst = con.prepareStatement("SELECT * FROM " + tableName + " WHERE " + col.IS_LOGGED + "=1;");
rs = pst.executeQuery();
String sql = "SELECT * FROM " + tableName + " WHERE " + col.IS_LOGGED + "=1;";
try (PreparedStatement pst = con.prepareStatement(sql); ResultSet rs = pst.executeQuery()) {
while (rs.next()) {
PlayerAuth auth = buildAuthFromResultSet(rs);
auths.add(auth);
}
} catch (SQLException ex) {
logSqlException(ex);
} finally {
close(pst);
}
return auths;
}

View File

@ -0,0 +1,319 @@
package fr.xephi.authme.datasource;
import com.google.common.base.Objects;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import fr.xephi.authme.ConsoleLoggerTestInitializer;
import fr.xephi.authme.cache.auth.PlayerAuth;
import fr.xephi.authme.security.HashAlgorithm;
import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.settings.domain.Property;
import fr.xephi.authme.settings.properties.SecuritySettings;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.sql.Blob;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Set;
import static org.mockito.BDDMockito.given;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyLong;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
/**
* Test class which runs through a datasource implementation and verifies that all
* instances of {@link AutoCloseable} that are created in the calls are closed again.
* <p>
* Instead of an actual connection to a datasource, we pass a mock Connection object
* which is set to create additional mocks on demand for Statement and ResultSet objects.
* This test ensures that all such objects that are created will be closed again by
* keeping a list of mocks ({@link #closeables}) and then verifying that all have been
* closed {@link #verifyHaveMocksBeenClosed()}.
*/
@RunWith(Parameterized.class)
public abstract class AbstractResourceClosingTest {
/** List of DataSource method names not to test. */
private static final Set<String> IGNORED_METHODS = ImmutableSet.of("reload", "close", "getType");
/** Collection of values to use to call methods with the parameters they expect. */
private static final Map<Class<?>, Object> PARAM_VALUES = getDefaultParameters();
/**
* Custom list of hash algorithms to use to test a method. By default we define {@link HashAlgorithm#XFBCRYPT} as
* algorithms we use as a lot of methods execute additional statements in {@link MySQL}. If other algorithms
* have custom behaviors, they can be supplied in this map so it will be tested as well.
*/
private static final Map<String, HashAlgorithm[]> CUSTOM_ALGORITHMS = getCustomAlgorithmList();
/** Mock of a settings instance. */
private static NewSetting settings;
/** The datasource to test. */
private DataSource dataSource;
/** The DataSource method to test. */
private Method method;
/** Keeps track of the closeables which are created during the tested call. */
private List<AutoCloseable> closeables = new ArrayList<>();
/**
* Constructor for the test instance verifying the given method with the given hash algorithm.
*
* @param method The DataSource method to test
* @param name The name of the method
* @param algorithm The hash algorithm to use
*/
public AbstractResourceClosingTest(Method method, String name, HashAlgorithm algorithm) {
// Note ljacqu 20160227: The name parameter is necessary as we pass it from the @Parameters method;
// we use the method name in the annotation to name the test sensibly
this.method = method;
given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(algorithm);
}
/** Initialize the settings mock and makes it return the default of any given property by default. */
@BeforeClass
public static void initializeSettings() throws IOException, ClassNotFoundException {
settings = mock(NewSetting.class);
given(settings.getProperty(any(Property.class))).willAnswer(new Answer() {
@Override
public Object answer(InvocationOnMock invocation) {
return ((Property) invocation.getArguments()[0]).getDefaultValue();
}
});
ConsoleLoggerTestInitializer.setupLogger();
}
/** Initialize the dataSource implementation to test based on a mock connection. */
@Before
public void setUpMockConnection() throws Exception {
Connection connection = initConnection();
dataSource = createDataSource(settings, connection);
}
/**
* The actual test -- executes the method given through the constructor and then verifies that all
* AutoCloseable mocks it constructed have been closed.
*/
@Test
public void shouldCloseResources() throws IllegalAccessException, InvocationTargetException {
method.invoke(dataSource, buildParamListForMethod(method));
verifyHaveMocksBeenClosed();
}
/**
* Initialization method -- provides the parameters to run the test with by scanning all DataSource
* methods. By default, we run one test per method with the default hash algorithm, XFBCRYPT.
* If the map of custom algorithms has an entry for the method name, we add an entry for each algorithm
* supplied by the map.
*
* @return Test parameters
*/
@Parameterized.Parameters(name = "{1}({2})")
public static Collection<Object[]> data() {
List<Method> methods = getDataSourceMethods();
List<Object[]> data = new ArrayList<>();
// Use XFBCRYPT if nothing else specified as there is a lot of specific behavior to this hash algorithm in MySQL
final HashAlgorithm[] defaultAlgorithm = new HashAlgorithm[]{HashAlgorithm.XFBCRYPT};
for (Method method : methods) {
HashAlgorithm[] algorithms = Objects.firstNonNull(CUSTOM_ALGORITHMS.get(method.getName()), defaultAlgorithm);
for (HashAlgorithm algorithm : algorithms) {
data.add(new Object[]{method, method.getName(), algorithm});
}
}
return data;
}
/* Create a DataSource instance with the given mock settings and mock connection. */
protected abstract DataSource createDataSource(NewSetting settings, Connection connection) throws Exception;
/* Get all methods of the DataSource interface, minus the ones in the ignored list. */
private static List<Method> getDataSourceMethods() {
List<Method> publicMethods = new ArrayList<>();
for (Method method : DataSource.class.getDeclaredMethods()) {
if (!IGNORED_METHODS.contains(method.getName())) {
publicMethods.add(method);
}
}
return publicMethods;
}
/**
* Verify that all AutoCloseables that have been created during the method execution have been closed.
*/
private void verifyHaveMocksBeenClosed() {
System.out.println("Found " + closeables.size() + " resources");
try {
for (AutoCloseable autoCloseable : closeables) {
verify(autoCloseable).close();
}
} catch (Exception e) {
throw new IllegalStateException("Error verifying if autoCloseable was closed", e);
}
}
/**
* Helper method for building a list of test values to satisfy a method's signature.
*
* @param method The method to create a valid parameter list for
* @return Parameter list to invoke the given method with
*/
private static Object[] buildParamListForMethod(Method method) {
List<Object> params = new ArrayList<>();
int index = 0;
for (Class<?> paramType : method.getParameterTypes()) {
// Checking List.class == paramType instead of Class#isAssignableFrom means we really only accept List,
// but that is a sensible assumption and makes our life much easier later on when juggling with Type
Object param = (List.class == paramType)
? getTypedList(method.getGenericParameterTypes()[index])
: PARAM_VALUES.get(paramType);
Preconditions.checkNotNull(param, "No param type for " + paramType);
params.add(param);
++index;
}
return params.toArray();
}
/**
* Return a list with some test elements that correspond to the given list type's generic type.
*
* @param type The list type to process and build a test list for
* @return Test list with sample elements of the correct type
*/
private static List<?> getTypedList(Type type) {
if (type instanceof ParameterizedType) {
ParameterizedType parameterizedType = (ParameterizedType) type;
Preconditions.checkArgument(List.class == parameterizedType.getRawType(), type + " should be a List");
Type genericType = parameterizedType.getActualTypeArguments()[0];
Object element = PARAM_VALUES.get(genericType);
Preconditions.checkNotNull(element, "No sample element for list of generic type " + genericType);
return Arrays.asList(element, element, element);
}
throw new IllegalStateException("Cannot build list for unexpected Type: " + type);
}
/* Initialize the map of test values to pass to methods to satisfy their signature. */
private static Map<Class<?>, Object> getDefaultParameters() {
HashedPassword hash = new HashedPassword("test", "test");
return ImmutableMap.<Class<?>, Object>builder()
.put(String.class, "test")
.put(int.class, 3)
.put(long.class, 102L)
.put(PlayerAuth.class, PlayerAuth.builder().name("test").realName("test").password(hash).build())
.put(HashedPassword.class, hash)
.build();
}
/**
* Return the custom list of hash algorithms to test a method with to execute code specific to
* one hash algorithm. By default, XFBCRYPT is used. Only MySQL has code specific to algorithms
* but for technical reasons the custom list will be used for all tested classes.
*
* @return List of custom algorithms by method
*/
private static Map<String, HashAlgorithm[]> getCustomAlgorithmList() {
// We use XFBCRYPT as default encryption method so we don't have to list many of the special cases for it
return ImmutableMap.<String, HashAlgorithm[]>builder()
.put("saveAuth", new HashAlgorithm[]{HashAlgorithm.PHPBB, HashAlgorithm.WORDPRESS})
.build();
}
// ---------------------
// Mock initialization
// ---------------------
/**
* Initialize the connection mock which produces additional AutoCloseable mocks and records them.
*
* @return Connection mock
*/
private Connection initConnection() {
Connection connection = mock(Connection.class);
try {
given(connection.prepareStatement(anyString())).willAnswer(preparedStatementAnswer());
given(connection.createStatement()).willAnswer(preparedStatementAnswer());
given(connection.createBlob()).willReturn(mock(Blob.class));
return connection;
} catch (SQLException e) {
throw new IllegalStateException("Could not initialize connection mock", e);
}
}
/* Create Answer that returns a PreparedStatement mock. */
private Answer<PreparedStatement> preparedStatementAnswer() {
return new Answer<PreparedStatement>() {
@Override
public PreparedStatement answer(InvocationOnMock invocation) throws SQLException {
PreparedStatement pst = mock(PreparedStatement.class);
closeables.add(pst);
given(pst.executeQuery()).willAnswer(resultSetAnswer());
given(pst.executeQuery(anyString())).willAnswer(resultSetAnswer());
return pst;
}
};
}
/* Create Answer that returns a ResultSet mock. */
private Answer<ResultSet> resultSetAnswer() throws SQLException {
return new Answer<ResultSet>() {
@Override
public ResultSet answer(InvocationOnMock invocation) throws Throwable {
ResultSet rs = initResultSet();
closeables.add(rs);
return rs;
}
};
}
/* Create a ResultSet mock. */
private ResultSet initResultSet() throws SQLException {
ResultSet rs = mock(ResultSet.class);
// Return true for ResultSet#next the first time to make sure we execute all code
given(rs.next()).willAnswer(new Answer<Boolean>() {
boolean isInitial = true;
@Override
public Boolean answer(InvocationOnMock invocation) {
if (isInitial) {
isInitial = false;
return true;
}
return false;
}
});
given(rs.getString(anyInt())).willReturn("test");
given(rs.getString(anyString())).willReturn("test");
Blob blob = mock(Blob.class);
given(blob.getBytes(anyLong(), anyInt())).willReturn(new byte[]{});
given(blob.length()).willReturn(0L);
given(rs.getBlob(anyInt())).willReturn(blob);
given(rs.getBlob(anyString())).willReturn(blob);
return rs;
}
}

View File

@ -0,0 +1,29 @@
package fr.xephi.authme.datasource;
import com.zaxxer.hikari.HikariDataSource;
import fr.xephi.authme.security.HashAlgorithm;
import fr.xephi.authme.settings.NewSetting;
import java.lang.reflect.Method;
import java.sql.Connection;
import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock;
/**
* Resource closing test for {@link MySQL}.
*/
public class MySqlResourceClosingTest extends AbstractResourceClosingTest {
public MySqlResourceClosingTest(Method method, String name, HashAlgorithm algorithm) {
super(method, name, algorithm);
}
@Override
protected DataSource createDataSource(NewSetting settings, Connection connection) throws Exception {
HikariDataSource hikariDataSource = mock(HikariDataSource.class);
given(hikariDataSource.getConnection()).willReturn(connection);
return new MySQL(settings, hikariDataSource);
}
}

View File

@ -0,0 +1,23 @@
package fr.xephi.authme.datasource;
import fr.xephi.authme.security.HashAlgorithm;
import fr.xephi.authme.settings.NewSetting;
import java.lang.reflect.Method;
import java.sql.Connection;
/**
* Resource closing test for {@link SQLite}.
*/
public class SQLiteResourceClosingTest extends AbstractResourceClosingTest {
public SQLiteResourceClosingTest(Method method, String name, HashAlgorithm algorithm) {
super(method, name, algorithm);
}
@Override
protected DataSource createDataSource(NewSetting settings, Connection connection) throws Exception {
return new SQLite(settings, connection);
}
}