#1255 Create resource-closing tests for the MySQL extensions

- Remove test runs with different hash algorithms in the abstract super class
- Create resource-closing tests for the new extension classes

src/test/java/fr/xephi/authme/datasource/AbstractResourceClosingTest.java

@@ -1,14 +1,10 @@
 package fr.xephi.authme.datasource;
 
-import ch.jalu.configme.properties.Property;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import fr.xephi.authme.TestHelper;
 import fr.xephi.authme.data.auth.PlayerAuth;
 import fr.xephi.authme.security.crypts.HashedPassword;
-import fr.xephi.authme.settings.Settings;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -33,7 +29,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
-import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.anyString;
@@ -42,36 +37,29 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 
 /**
- * Test class which runs through a datasource implementation and verifies that all
+ * Test class which runs through objects interacting with a database 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()}.
+ * 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();
 
-    /** Mock of a settings instance. */
-    private static Settings 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<>();
 
+    private boolean hasCreatedConnection = false;
+
     /**
      * Constructor for the test instance verifying the given method.
      *
@@ -84,65 +72,22 @@ public abstract class AbstractResourceClosingTest {
         this.method = method;
     }
 
-    /** Initialize the settings mock and makes it return the default of any given property by default. */
-    @SuppressWarnings("unchecked")
     @BeforeClass
-    public static void initializeSettings() {
-        settings = mock(Settings.class);
-        given(settings.getProperty(any(Property.class))).willAnswer(new Answer() {
-            @Override
-            public Object answer(InvocationOnMock invocation) {
-                return ((Property<?>) invocation.getArguments()[0]).getDefaultValue();
-            }
-        });
+    public static void initializeLogger() {
         TestHelper.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));
+        method.invoke(getObjectUnderTest(), buildParamListForMethod(method));
         verifyHaveMocksBeenClosed();
     }
 
-    /**
-     * Initialization method -- provides the parameters to run the test with by scanning all DataSource methods.
-     *
-     * @return Test parameters
-     */
-    @Parameterized.Parameters(name = "{1}")
-    public static Collection<Object[]> data() {
-        List<Method> methods = getDataSourceMethods();
-        List<Object[]> data = new ArrayList<>();
-        for (Method method : methods) {
-            data.add(new Object[]{method, method.getName()});
-        }
-        return data;
-    }
-
-    /* Create a DataSource instance with the given mock settings and mock connection. */
-    protected abstract DataSource createDataSource(Settings 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;
-    }
+    protected abstract Object getObjectUnderTest();
 
     /**
      * Verify that all AutoCloseables that have been created during the method execution have been closed.
@@ -166,7 +111,7 @@ public abstract class AbstractResourceClosingTest {
      * @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) {
+    private Object[] buildParamListForMethod(Method method) {
         List<Object> params = new ArrayList<>();
         int index = 0;
         for (Class<?> paramType : method.getParameterTypes()) {
@@ -174,7 +119,7 @@ public abstract class AbstractResourceClosingTest {
             // but that is a sensible assumption and makes our life much easier later on when juggling with Type
             Object param = Collection.class.isAssignableFrom(paramType)
                 ? getTypedCollection(method.getGenericParameterTypes()[index])
-                : PARAM_VALUES.get(paramType);
+                : getMethodParameter(paramType);
             Preconditions.checkNotNull(param, "No param type for " + paramType);
             params.add(param);
             ++index;
@@ -182,6 +127,15 @@ public abstract class AbstractResourceClosingTest {
         return params.toArray();
     }
 
+    private Object getMethodParameter(Class<?> paramType) {
+        if (paramType.equals(Connection.class)) {
+            Preconditions.checkArgument(!hasCreatedConnection, "A Connection object was already created in this test run");
+            hasCreatedConnection = true;
+            return initConnection();
+        }
+        return PARAM_VALUES.get(paramType);
+    }
+
     /**
      * Return a collection of the required type with some test elements that correspond to the
      * collection's generic type.
@@ -230,11 +184,11 @@ public abstract class AbstractResourceClosingTest {
     // Mock initialization
     // ---------------------
     /**
-     * Initialize the connection mock which produces additional AutoCloseable mocks and records them.
+     * Initializes the connection mock which produces additional AutoCloseable mocks and records them.
      *
      * @return Connection mock
      */
-    private Connection initConnection() {
+    protected Connection initConnection() {
         Connection connection = mock(Connection.class);
         try {
             given(connection.prepareStatement(anyString())).willAnswer(preparedStatementAnswer());

src/test/java/fr/xephi/authme/datasource/AbstractSqlDataSourceResourceClosingTest.java

@@ -0,0 +1,85 @@
+package fr.xephi.authme.datasource;
+
+import ch.jalu.configme.properties.Property;
+import com.google.common.collect.ImmutableSet;
+import fr.xephi.authme.TestHelper;
+import fr.xephi.authme.settings.Settings;
+import org.junit.BeforeClass;
+import org.junit.runners.Parameterized;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+import java.lang.reflect.Method;
+import java.sql.Connection;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+
+/**
+ * Resource-closing test for SQL data sources.
+ */
+public abstract class AbstractSqlDataSourceResourceClosingTest extends AbstractResourceClosingTest {
+
+    /** List of DataSource method names not to test. */
+    private static final Set<String> IGNORED_METHODS = ImmutableSet.of("reload", "getType");
+
+    private static Settings settings;
+
+    AbstractSqlDataSourceResourceClosingTest(Method method, String name) {
+        super(method, name);
+    }
+
+    @BeforeClass
+    public static void initializeSettings() {
+        settings = mock(Settings.class);
+        given(settings.getProperty(any(Property.class))).willAnswer(new Answer() {
+            @Override
+            public Object answer(InvocationOnMock invocation) {
+                return ((Property<?>) invocation.getArguments()[0]).getDefaultValue();
+            }
+        });
+        TestHelper.setupLogger();
+    }
+
+    protected DataSource getObjectUnderTest() {
+        try {
+            return createDataSource(settings, initConnection());
+        } catch (Exception e) {
+            throw new IllegalStateException(e);
+        }
+    }
+
+    /* Create a DataSource instance with the given mock settings and mock connection. */
+    protected abstract DataSource createDataSource(Settings settings, Connection connection) throws Exception;
+
+    /**
+     * Initialization method -- provides the parameters to run the test with by scanning all DataSource methods.
+     *
+     * @return Test parameters
+     */
+    @Parameterized.Parameters(name = "{1}")
+    public static Collection<Object[]> data() {
+        List<Method> methods = getDataSourceMethods();
+        List<Object[]> data = new ArrayList<>();
+        for (Method method : methods) {
+            data.add(new Object[]{method, method.getName()});
+        }
+        return data;
+    }
+
+    /* 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;
+    }
+}

src/test/java/fr/xephi/authme/datasource/MySqlResourceClosingTest.java

@@ -15,7 +15,7 @@ import static org.mockito.Mockito.mock;
 /**
  * Resource closing test for {@link MySQL}.
  */
-public class MySqlResourceClosingTest extends AbstractResourceClosingTest {
+public class MySqlResourceClosingTest extends AbstractSqlDataSourceResourceClosingTest {
 
     public MySqlResourceClosingTest(Method method, String name) {
         super(method, name);

src/test/java/fr/xephi/authme/datasource/SQLiteResourceClosingTest.java

@@ -8,7 +8,7 @@ import java.sql.Connection;
 /**
  * Resource closing test for {@link SQLite}.
  */
-public class SQLiteResourceClosingTest extends AbstractResourceClosingTest {
+public class SQLiteResourceClosingTest extends AbstractSqlDataSourceResourceClosingTest {
 
     public SQLiteResourceClosingTest(Method method, String name) {
         super(method, name);

src/test/java/fr/xephi/authme/datasource/mysqlextensions/AbstractMySqlExtensionResourceClosingTest.java

@@ -0,0 +1,60 @@
+package fr.xephi.authme.datasource.mysqlextensions;
+
+import ch.jalu.configme.properties.Property;
+import fr.xephi.authme.datasource.AbstractResourceClosingTest;
+import fr.xephi.authme.datasource.Columns;
+import fr.xephi.authme.settings.Settings;
+import org.junit.BeforeClass;
+import org.junit.runners.Parameterized;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+
+/**
+ * Checks that SQL resources are closed properly in {@link MySqlExtension} implementations.
+ */
+public abstract class AbstractMySqlExtensionResourceClosingTest extends AbstractResourceClosingTest {
+
+    private static Settings settings;
+    private static Columns columns;
+
+    public AbstractMySqlExtensionResourceClosingTest(Method method, String name) {
+        super(method, name);
+    }
+
+    @BeforeClass
+    public static void initSettings() {
+        settings = mock(Settings.class);
+        given(settings.getProperty(any(Property.class))).willAnswer(new Answer() {
+            @Override
+            public Object answer(InvocationOnMock invocation) {
+                return ((Property<?>) invocation.getArguments()[0]).getDefaultValue();
+            }
+        });
+        columns = new Columns(settings);
+    }
+
+    @Override
+    protected MySqlExtension getObjectUnderTest() {
+        return createExtension(settings, columns);
+    }
+
+    protected abstract MySqlExtension createExtension(Settings settings, Columns columns);
+
+    @Parameterized.Parameters(name = "{1}")
+    public static List<Object[]> createParameters() {
+        return Arrays.stream(MySqlExtension.class.getDeclaredMethods())
+            .filter(m -> Modifier.isPublic(m.getModifiers()))
+            .map(m -> new Object[]{m, m.getName()})
+            .collect(Collectors.toList());
+    }
+}

src/test/java/fr/xephi/authme/datasource/mysqlextensions/Ipb4ExtensionResourceClosingTest.java

@@ -0,0 +1,21 @@
+package fr.xephi.authme.datasource.mysqlextensions;
+
+import fr.xephi.authme.datasource.Columns;
+import fr.xephi.authme.settings.Settings;
+
+import java.lang.reflect.Method;
+
+/**
+ * Resource closing test for {@link Ipb4Extension}.
+ */
+public class Ipb4ExtensionResourceClosingTest extends AbstractMySqlExtensionResourceClosingTest {
+
+    public Ipb4ExtensionResourceClosingTest(Method method, String name) {
+        super(method, name);
+    }
+
+    @Override
+    protected MySqlExtension createExtension(Settings settings, Columns columns) {
+        return new Ipb4Extension(settings, columns);
+    }
+}

src/test/java/fr/xephi/authme/datasource/mysqlextensions/NoOpExtensionTest.java

@@ -0,0 +1,58 @@
+package fr.xephi.authme.datasource.mysqlextensions;
+
+import ch.jalu.configme.properties.Property;
+import fr.xephi.authme.data.auth.PlayerAuth;
+import fr.xephi.authme.datasource.Columns;
+import fr.xephi.authme.security.crypts.HashedPassword;
+import fr.xephi.authme.settings.Settings;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+/**
+ * Test for {@link NoOpExtension}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class NoOpExtensionTest {
+
+    private NoOpExtension extension;
+
+    @Before
+    @SuppressWarnings("unchecked")
+    public void createExtension() {
+        Settings settings = mock(Settings.class);
+        given(settings.getProperty(any(Property.class))).willAnswer(
+            invocation -> ((Property<?>) invocation.getArgument(0)).getDefaultValue());
+        Columns columns = new Columns(settings);
+        extension = new NoOpExtension(settings, columns);
+    }
+
+    @Test
+    public void shouldNotHaveAnyInteractionsWithConnection() throws SQLException {
+        // given
+        Connection connection = mock(Connection.class);
+        PlayerAuth auth = mock(PlayerAuth.class);
+        int id = 3;
+        String name = "Bobby";
+        HashedPassword password = new HashedPassword("test", "toast");
+
+
+        // when
+        extension.extendAuth(auth, id, connection);
+        extension.changePassword(name, password, connection);
+        extension.removeAuth(name, connection);
+        extension.saveAuth(auth, connection);
+
+        // then
+        verifyZeroInteractions(connection, auth);
+    }
+}

src/test/java/fr/xephi/authme/datasource/mysqlextensions/PhpBbExtensionResourceClosingTest.java

@@ -0,0 +1,21 @@
+package fr.xephi.authme.datasource.mysqlextensions;
+
+import fr.xephi.authme.datasource.Columns;
+import fr.xephi.authme.settings.Settings;
+
+import java.lang.reflect.Method;
+
+/**
+ * Resource closing test for {@link PhpBbExtension}.
+ */
+public class PhpBbExtensionResourceClosingTest extends AbstractMySqlExtensionResourceClosingTest {
+
+    public PhpBbExtensionResourceClosingTest(Method method, String name) {
+        super(method, name);
+    }
+
+    @Override
+    protected MySqlExtension createExtension(Settings settings, Columns columns) {
+        return new PhpBbExtension(settings, columns);
+    }
+}

src/test/java/fr/xephi/authme/datasource/mysqlextensions/WordpressExtensionResourceClosingTest.java

@@ -0,0 +1,21 @@
+package fr.xephi.authme.datasource.mysqlextensions;
+
+import fr.xephi.authme.datasource.Columns;
+import fr.xephi.authme.settings.Settings;
+
+import java.lang.reflect.Method;
+
+/**
+ * Resource closing test for {@link WordpressExtension}.
+ */
+public class WordpressExtensionResourceClosingTest extends AbstractMySqlExtensionResourceClosingTest {
+
+    public WordpressExtensionResourceClosingTest(Method method, String name) {
+        super(method, name);
+    }
+
+    @Override
+    protected MySqlExtension createExtension(Settings settings, Columns columns) {
+        return new WordpressExtension(settings, columns);
+    }
+}

src/test/java/fr/xephi/authme/datasource/mysqlextensions/XfBcryptExtensionResourceClosingTest.java

@@ -0,0 +1,21 @@
+package fr.xephi.authme.datasource.mysqlextensions;
+
+import fr.xephi.authme.datasource.Columns;
+import fr.xephi.authme.settings.Settings;
+
+import java.lang.reflect.Method;
+
+/**
+ * Resource closing test for {@link XfBcryptExtension}.
+ */
+public class XfBcryptExtensionResourceClosingTest extends AbstractMySqlExtensionResourceClosingTest {
+
+    public XfBcryptExtensionResourceClosingTest(Method method, String name) {
+        super(method, name);
+    }
+
+    @Override
+    protected MySqlExtension createExtension(Settings settings, Columns columns) {
+        return new XfBcryptExtension(settings, columns);
+    }
+}