From a425eacf2d8812549594d59fcbc54cb7882cc14f Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 20 Oct 2017 17:49:30 +0200 Subject: [PATCH] #792 Add missing tests / fix CodeClimate issues --- .../authme/debug/MySqlDefaultChanger.java | 77 +++++++++++++++---- .../xephi/authme/service/SessionService.java | 6 +- .../debug/MySqlDefaultChangerColumnsTest.java | 12 +-- .../authme/debug/MySqlDefaultChangerTest.java | 60 ++++++++++++++- .../datasource/MySqlIntegrationTest.java | 9 +-- .../authme/datasource/MySqlTestUtil.java | 30 ++++++++ .../authme/service/SessionServiceTest.java | 4 +- 7 files changed, 165 insertions(+), 33 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/datasource/MySqlTestUtil.java diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChanger.java b/src/main/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChanger.java index d951570c..7b2f7636 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChanger.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChanger.java @@ -98,24 +98,33 @@ class MySqlDefaultChanger implements DebugSection { } } + /** + * Adds a default value to the column definition and adds a {@code NOT NULL} constraint for + * the specified column. + * + * @param sender the command sender initiation the action + * @param column the column to modify + * @param con connection to the database + * @throws SQLException . + */ private void changeColumnToNotNullWithDefault(CommandSender sender, Columns column, Connection con) throws SQLException { final String tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); - final String columnName = settings.getProperty(column.columnName); + final String columnName = settings.getProperty(column.getColumnNameProperty()); // Replace NULLs with future default value String sql = format("UPDATE %s SET %s = ? WHERE %s IS NULL;", tableName, columnName, columnName); int updatedRows; try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setObject(1, column.defaultValue); + pst.setObject(1, column.getDefaultValue()); updatedRows = pst.executeUpdate(); } - sender.sendMessage("Replaced NULLs with default value ('" + column.defaultValue + sender.sendMessage("Replaced NULLs with default value ('" + column.getDefaultValue() + "'), modifying " + updatedRows + " entries"); // Change column definition to NOT NULL version try (Statement st = con.createStatement()) { - st.execute(format("ALTER TABLE %s MODIFY %s %s", tableName, columnName, column.notNullDefinition)); + st.execute(format("ALTER TABLE %s MODIFY %s %s", tableName, columnName, column.getNotNullDefinition())); sender.sendMessage("Changed column '" + columnName + "' to have NOT NULL constraint"); } @@ -124,13 +133,22 @@ class MySqlDefaultChanger implements DebugSection { + sender.getName() + "'"); } + /** + * Removes the {@code NOT NULL} constraint of a column definition and replaces rows with the + * default value to {@code NULL}. + * + * @param sender the command sender initiation the action + * @param column the column to modify + * @param con connection to the database + * @throws SQLException . + */ private void removeNotNullAndDefault(CommandSender sender, Columns column, Connection con) throws SQLException { final String tableName = settings.getProperty(DatabaseSettings.MYSQL_TABLE); - final String columnName = settings.getProperty(column.columnName); + final String columnName = settings.getProperty(column.getColumnNameProperty()); // Change column definition to nullable version try (Statement st = con.createStatement()) { - st.execute(format("ALTER TABLE %s MODIFY %s %s", tableName, columnName, column.nullableDefinition)); + st.execute(format("ALTER TABLE %s MODIFY %s %s", tableName, columnName, column.getNullableDefinition())); sender.sendMessage("Changed column '" + columnName + "' to allow nulls"); } @@ -138,10 +156,10 @@ class MySqlDefaultChanger implements DebugSection { String sql = format("UPDATE %s SET %s = NULL WHERE %s = ?;", tableName, columnName, columnName); int updatedRows; try (PreparedStatement pst = con.prepareStatement(sql)) { - pst.setObject(1, column.defaultValue); + pst.setObject(1, column.getDefaultValue()); updatedRows = pst.executeUpdate(); } - sender.sendMessage("Replaced default value ('" + column.defaultValue + sender.sendMessage("Replaced default value ('" + column.getDefaultValue() + "') to be NULL, modifying " + updatedRows + " entries"); // Log success message @@ -183,7 +201,8 @@ class MySqlDefaultChanger implements DebugSection { List formattedColumns = new ArrayList<>(Columns.values().length); for (Columns col : Columns.values()) { - boolean isNotNull = isNotNullColumn(metaData, tableName, settings.getProperty(col.columnName)); + String columnName = settings.getProperty(col.getColumnNameProperty()); + boolean isNotNull = isNotNullColumn(metaData, tableName, columnName); String formattedColumn = (isNotNull ? ChatColor.DARK_AQUA : ChatColor.GOLD) + col.name().toLowerCase(); formattedColumns.add(formattedColumn); } @@ -212,6 +231,12 @@ class MySqlDefaultChanger implements DebugSection { return false; } + /** + * Gets the Connection object from the MySQL data source. + * + * @param mySql the MySQL data source to get the connection from + * @return the connection + */ @VisibleForTesting Connection getConnection(MySQL mySql) { try { @@ -259,6 +284,7 @@ class MySqlDefaultChanger implements DebugSection { ADD, REMOVE } + /** MySQL columns which can be toggled between being NOT NULL and allowing NULL values. */ enum Columns { LASTLOGIN(DatabaseSettings.MYSQL_COL_LASTLOGIN, @@ -267,16 +293,37 @@ class MySqlDefaultChanger implements DebugSection { EMAIL(DatabaseSettings.MYSQL_COL_EMAIL, "VARCHAR(255)", "VARCHAR(255) NOT NULL DEFAULT 'your@email.com'", DB_EMAIL_DEFAULT); - final Property columnName; - final String nullableDefinition; - final String notNullDefinition; - final Object defaultValue; + private final Property columnNameProperty; + private final String nullableDefinition; + private final String notNullDefinition; + private final Object defaultValue; - Columns(Property columnName, String nullableDefinition, String notNullDefinition, Object defaultValue) { - this.columnName = columnName; + Columns(Property columnNameProperty, String nullableDefinition, + String notNullDefinition, Object defaultValue) { + this.columnNameProperty = columnNameProperty; this.nullableDefinition = nullableDefinition; this.notNullDefinition = notNullDefinition; this.defaultValue = defaultValue; } + + /** @return property defining the column name in the database */ + Property getColumnNameProperty() { + return columnNameProperty; + } + + /** @return SQL definition of the column allowing NULL values */ + String getNullableDefinition() { + return nullableDefinition; + } + + /** @return SQL definition of the column with a NOT NULL constraint */ + String getNotNullDefinition() { + return notNullDefinition; + } + + /** @return the default value used in {@link #notNullDefinition} */ + Object getDefaultValue() { + return defaultValue; + } } } diff --git a/src/main/java/fr/xephi/authme/service/SessionService.java b/src/main/java/fr/xephi/authme/service/SessionService.java index ef51533b..a4000e91 100644 --- a/src/main/java/fr/xephi/authme/service/SessionService.java +++ b/src/main/java/fr/xephi/authme/service/SessionService.java @@ -12,6 +12,8 @@ import org.bukkit.entity.Player; import javax.inject.Inject; +import static fr.xephi.authme.util.Utils.MILLIS_PER_MINUTE; + /** * Handles the user sessions. */ @@ -66,11 +68,13 @@ public class SessionService implements Reloadable { if (auth == null) { ConsoleLogger.warning("No PlayerAuth in database for '" + player.getName() + "' during session check"); return false; + } else if (auth.getLastLogin() == null) { + return false; } long timeSinceLastLogin = System.currentTimeMillis() - auth.getLastLogin(); return PlayerUtils.getPlayerIp(player).equals(auth.getLastIp()) && timeSinceLastLogin > 0 - && timeSinceLastLogin < service.getProperty(PluginSettings.SESSIONS_TIMEOUT) * 60 * 1000; + && timeSinceLastLogin < service.getProperty(PluginSettings.SESSIONS_TIMEOUT) * MILLIS_PER_MINUTE; } public void grantSession(String name) { diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChangerColumnsTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChangerColumnsTest.java index 8a7e03d4..df891fe6 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChangerColumnsTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChangerColumnsTest.java @@ -21,9 +21,9 @@ public class MySqlDefaultChangerColumnsTest { // when / then for (MySqlDefaultChanger.Columns col : MySqlDefaultChanger.Columns.values()) { - if (!properties.add(col.columnName.getPath())) { + if (!properties.add(col.getColumnNameProperty().getPath())) { fail("Column '" + col + "' has a column name property path that was already encountered: " - + col.columnName.getPath()); + + col.getColumnNameProperty().getPath()); } } } @@ -44,8 +44,8 @@ public class MySqlDefaultChangerColumnsTest { private void verifyHasCorrespondingColumnDefinitions(MySqlDefaultChanger.Columns column) { // given / when - String nullable = column.nullableDefinition; - String notNull = column.notNullDefinition; + String nullable = column.getNullableDefinition(); + String notNull = column.getNotNullDefinition(); // then String expectedNotNull = nullable + " NOT NULL DEFAULT "; @@ -56,8 +56,8 @@ public class MySqlDefaultChangerColumnsTest { private void verifyHasSameDefaultValueInNotNullDefinition(MySqlDefaultChanger.Columns column) { // given / when - String notNull = column.notNullDefinition; - Object defaultValue = column.defaultValue; + String notNull = column.getNotNullDefinition(); + Object defaultValue = column.getDefaultValue(); // then String defaultValueAsString = String.valueOf(defaultValue); diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChangerTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChangerTest.java index 3d639c60..c40f5a99 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChangerTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/debug/MySqlDefaultChangerTest.java @@ -1,18 +1,29 @@ package fr.xephi.authme.command.executable.authme.debug; +import com.zaxxer.hikari.HikariDataSource; import fr.xephi.authme.ReflectionTestUtils; +import fr.xephi.authme.TestHelper; import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.datasource.CacheDataSource; import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.datasource.MySQL; +import fr.xephi.authme.datasource.MySqlTestUtil; import fr.xephi.authme.settings.Settings; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import java.sql.Connection; +import java.sql.SQLException; + import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; /** * Test for {@link MySqlDefaultChanger}. @@ -49,7 +60,54 @@ public class MySqlDefaultChangerTest { assertThat(result, equalTo(source)); } - // TODO #792: Add more tests + @Test + public void shouldReturnMySqlConnection() throws SQLException { + // given + Settings settings = mock(Settings.class); + TestHelper.returnDefaultsForAllProperties(settings); + HikariDataSource dataSource = mock(HikariDataSource.class); + Connection connection = mock(Connection.class); + given(dataSource.getConnection()).willReturn(connection); + MySQL mySQL = MySqlTestUtil.createMySql(settings, dataSource); + MySqlDefaultChanger defaultChanger = createDefaultChanger(mySQL); + + // when + Connection result = defaultChanger.getConnection(mySQL); + + // then + assertThat(result, equalTo(connection)); + verify(dataSource).getConnection(); + } + + @Test + public void shouldSetMySqlFieldOnInitialization() { + // given + MySQL mySql = mock(MySQL.class); + MySqlDefaultChanger defaultChanger = createDefaultChanger(mySql); + + // when + defaultChanger.setMySqlField(); + + // then + assertThat(ReflectionTestUtils.getFieldValue(MySqlDefaultChanger.class, defaultChanger, "mySql"), + sameInstance(mySql)); + } + + @Test + public void shouldLeaveMySqlFieldToNullOnInitialization() { + // given + DataSource dataSource = mock(DataSource.class); + PlayerCache playerCache = mock(PlayerCache.class); + CacheDataSource cacheDataSource = new CacheDataSource(dataSource, playerCache); + MySqlDefaultChanger defaultChanger = createDefaultChanger(cacheDataSource); + + // when + defaultChanger.setMySqlField(); + + // then + assertThat(ReflectionTestUtils.getFieldValue(MySqlDefaultChanger.class, defaultChanger, "mySql"), + nullValue()); + } private MySqlDefaultChanger createDefaultChanger(DataSource dataSource) { MySqlDefaultChanger defaultChanger = new MySqlDefaultChanger(); diff --git a/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java b/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java index 9da39a3a..75be337d 100644 --- a/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/datasource/MySqlIntegrationTest.java @@ -4,8 +4,6 @@ import ch.jalu.configme.properties.Property; import com.zaxxer.hikari.HikariConfig; import com.zaxxer.hikari.HikariDataSource; import fr.xephi.authme.TestHelper; -import fr.xephi.authme.datasource.mysqlextensions.MySqlExtension; -import fr.xephi.authme.datasource.mysqlextensions.MySqlExtensionsFactory; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.DatabaseSettings; import org.junit.After; @@ -19,7 +17,6 @@ import java.sql.Connection; import java.sql.SQLException; import java.sql.Statement; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -30,8 +27,6 @@ public class MySqlIntegrationTest extends AbstractDataSourceIntegrationTest { /** Mock of a settings instance. */ private static Settings settings; - /** Mock of extensions factory. */ - private static MySqlExtensionsFactory extensionsFactory; /** SQL statement to execute before running a test. */ private static String sqlInitialize; /** Connection to the H2 test database. */ @@ -47,8 +42,6 @@ public class MySqlIntegrationTest extends AbstractDataSourceIntegrationTest { settings = mock(Settings.class); TestHelper.returnDefaultsForAllProperties(settings); - extensionsFactory = mock(MySqlExtensionsFactory.class); - when(extensionsFactory.buildExtension(any(Columns.class))).thenReturn(mock(MySqlExtension.class)); set(DatabaseSettings.MYSQL_DATABASE, "h2_test"); set(DatabaseSettings.MYSQL_TABLE, "authme"); TestHelper.setRealLogger(); @@ -83,7 +76,7 @@ public class MySqlIntegrationTest extends AbstractDataSourceIntegrationTest { @Override protected DataSource getDataSource(String saltColumn) { when(settings.getProperty(DatabaseSettings.MYSQL_COL_SALT)).thenReturn(saltColumn); - return new MySQL(settings, hikariSource, extensionsFactory); + return MySqlTestUtil.createMySql(settings, hikariSource); } private static void set(Property property, T value) { diff --git a/src/test/java/fr/xephi/authme/datasource/MySqlTestUtil.java b/src/test/java/fr/xephi/authme/datasource/MySqlTestUtil.java new file mode 100644 index 00000000..597a7eec --- /dev/null +++ b/src/test/java/fr/xephi/authme/datasource/MySqlTestUtil.java @@ -0,0 +1,30 @@ +package fr.xephi.authme.datasource; + +import com.zaxxer.hikari.HikariDataSource; +import fr.xephi.authme.datasource.mysqlextensions.MySqlExtension; +import fr.xephi.authme.datasource.mysqlextensions.MySqlExtensionsFactory; +import fr.xephi.authme.settings.Settings; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Test util for the MySQL data source. + */ +public final class MySqlTestUtil { + + private MySqlTestUtil() { + } + + public static MySQL createMySql(Settings settings, HikariDataSource hikariDataSource) { + MySqlExtensionsFactory extensionsFactory = mock(MySqlExtensionsFactory.class); + given(extensionsFactory.buildExtension(any())).willReturn(mock(MySqlExtension.class)); + return createMySql(settings, hikariDataSource, extensionsFactory); + } + + public static MySQL createMySql(Settings settings, HikariDataSource hikariDataSource, + MySqlExtensionsFactory extensionsFactory) { + return new MySQL(settings, hikariDataSource, extensionsFactory); + } +} diff --git a/src/test/java/fr/xephi/authme/service/SessionServiceTest.java b/src/test/java/fr/xephi/authme/service/SessionServiceTest.java index a560e764..f67326a6 100644 --- a/src/test/java/fr/xephi/authme/service/SessionServiceTest.java +++ b/src/test/java/fr/xephi/authme/service/SessionServiceTest.java @@ -113,7 +113,7 @@ public class SessionServiceTest { } @Test - public void shouldRefuseSessionForAuthWithZeroLastLoginTimestamp() { + public void shouldRefuseSessionForAuthWithNullLastLoginTimestamp() { // given String name = "Bobby"; String ip = "127.3.12.15"; @@ -122,7 +122,7 @@ public class SessionServiceTest { given(dataSource.hasSession(name)).willReturn(true); PlayerAuth auth = PlayerAuth.builder() .name(name) - .lastLogin(0L) + .lastLogin(null) .lastIp(ip).build(); given(dataSource.getAuth(name)).willReturn(auth);