From ac9f060e795d23808a3d851eb9b2b22ee6701120 Mon Sep 17 00:00:00 2001 From: nossr50 Date: Thu, 27 Nov 2025 11:45:05 -0800 Subject: [PATCH] more code coverage --- .../nossr50/database/SQLDatabaseManager.java | 36 +----- .../database/SQLDatabaseManagerTest.java | 110 +++++++++++++++++- 2 files changed, 111 insertions(+), 35 deletions(-) diff --git a/src/main/java/com/gmail/nossr50/database/SQLDatabaseManager.java b/src/main/java/com/gmail/nossr50/database/SQLDatabaseManager.java index d302e32ad..a29cf0f68 100644 --- a/src/main/java/com/gmail/nossr50/database/SQLDatabaseManager.java +++ b/src/main/java/com/gmail/nossr50/database/SQLDatabaseManager.java @@ -76,8 +76,8 @@ public final class SQLDatabaseManager implements DatabaseManager { driverPath = LEGACY_DRIVER_PATH; //fall on deprecated path if new path isn't found Class.forName(driverPath); } catch (ClassNotFoundException ex) { - e.printStackTrace(); - ex.printStackTrace(); + logger.log(Level.SEVERE, "Initial driver path load failed", e); + logger.log(Level.SEVERE, "Legacy driver path load failed", ex); logger.severe("Neither driver found"); return; } @@ -1691,38 +1691,6 @@ public final class SQLDatabaseManager implements DatabaseManager { } } - private void checkUpgradeAddSQLIndexes(final Statement statement) { - ResultSet resultSet = null; - - try { - resultSet = statement.executeQuery( - "SHOW INDEX FROM `" + tablePrefix + "skills` WHERE `Key_name` LIKE 'idx\\_%'"); - resultSet.last(); - - if (resultSet.getRow() != SkillTools.NON_CHILD_SKILLS.size()) { - logger.info("Indexing tables, this may take a while on larger databases"); - - for (PrimarySkillType skill : SkillTools.NON_CHILD_SKILLS) { - String skill_name = skill.name().toLowerCase(Locale.ENGLISH); - - try { - statement.executeUpdate( - "ALTER TABLE `" + tablePrefix + "skills` ADD INDEX `idx_" - + skill_name + "` (`" + skill_name + "`) USING BTREE"); - } catch (SQLException ex) { - // Ignore - } - } - } - - mcMMO.getUpgradeManager().setUpgradeCompleted(UpgradeType.ADD_SQL_INDEXES); - } catch (SQLException ex) { - logSQLException(ex); - } finally { - tryClose(resultSet); - } - } - private void checkUpgradeAddUUIDs(final Statement statement) { ResultSet resultSet = null; diff --git a/src/test/java/com/gmail/nossr50/database/SQLDatabaseManagerTest.java b/src/test/java/com/gmail/nossr50/database/SQLDatabaseManagerTest.java index ad2d486d6..ca4a216a0 100644 --- a/src/test/java/com/gmail/nossr50/database/SQLDatabaseManagerTest.java +++ b/src/test/java/com/gmail/nossr50/database/SQLDatabaseManagerTest.java @@ -6,6 +6,7 @@ import com.gmail.nossr50.config.GeneralConfig; import com.gmail.nossr50.datatypes.MobHealthbarType; import com.gmail.nossr50.datatypes.database.DatabaseType; import com.gmail.nossr50.datatypes.database.PlayerStat; +import com.gmail.nossr50.datatypes.database.UpgradeType; import com.gmail.nossr50.datatypes.player.PlayerProfile; import com.gmail.nossr50.datatypes.skills.PrimarySkillType; import com.gmail.nossr50.mcMMO; @@ -23,6 +24,7 @@ import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.TestInstance.Lifecycle; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.ArgumentCaptor; import org.mockito.MockedStatic; import org.mockito.Mockito; import org.testcontainers.containers.JdbcDatabaseContainer; @@ -47,7 +49,11 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @TestInstance(Lifecycle.PER_CLASS) @@ -382,7 +388,7 @@ class SQLDatabaseManagerTest { } // ------------------------------------------------------------------------ - // Schema upgrades (legacy spears) + // Schema upgrades // ------------------------------------------------------------------------ @ParameterizedTest(name = "{0} - upgrades legacy schema to add spears columns") @@ -421,6 +427,55 @@ class SQLDatabaseManagerTest { } } + @ParameterizedTest(name = "{0} - SQL_CHARSET_UTF8MB4 upgrade runs without error") + @MethodSource("dbFlavors") + void whenCharsetUpgradeIsRequiredShouldUpdateCharacterSet(DbFlavor flavor) { + // GIVEN + truncateAllCoreTables(flavor); + + // First reset and restub the upgrade manager so only the charset upgrade runs + reset(upgradeManager); + when(mcMMO.getUpgradeManager()).thenReturn(upgradeManager); + when(upgradeManager.shouldUpgrade(any(UpgradeType.class))).thenReturn(false); + when(upgradeManager.shouldUpgrade(UpgradeType.SQL_CHARSET_UTF8MB4)).thenReturn(true); + + // WHEN – constructor will call checkStructure(), which will in turn call updateCharacterSet(...) + SQLDatabaseManager manager = createManagerFor(flavor); + + // THEN – we at least expect the upgrade to be marked completed + verify(upgradeManager, atLeastOnce()).setUpgradeCompleted(UpgradeType.SQL_CHARSET_UTF8MB4); + + manager.onDisable(); + + // Restore default behavior for other tests: no upgrades + when(upgradeManager.shouldUpgrade(any(UpgradeType.class))).thenReturn(false); + } + + @ParameterizedTest(name = "{0} - when all upgrades are required, all upgrade helpers execute") + @MethodSource("dbFlavors") + void whenAllUpgradesRequiredShouldExecuteAllUpgradeHelpers(DbFlavor flavor) { + // GIVEN – clean schema + truncateAllCoreTables(flavor); + + // GIVEN – every UpgradeType should be considered "needed" + reset(upgradeManager); + when(mcMMO.getUpgradeManager()).thenReturn(upgradeManager); + when(upgradeManager.shouldUpgrade(any(UpgradeType.class))).thenReturn(true); + + // WHEN – constructor will call checkStructure() which loops all UpgradeType values + SQLDatabaseManager databaseManager = createManagerFor(flavor); + + // THEN – at least one call to mark some upgrades complete (and in practice, many) + verify(upgradeManager, atLeastOnce()).setUpgradeCompleted(any(UpgradeType.class)); + + databaseManager.onDisable(); + + // Restore default for other tests + when(upgradeManager.shouldUpgrade(any(UpgradeType.class))).thenReturn(false); + } + + + // ------------------------------------------------------------------------ // New user -> rows in all core tables // ------------------------------------------------------------------------ @@ -1153,6 +1208,59 @@ class SQLDatabaseManagerTest { } } + // -------------------------------------------------------------------------- + // Convert Users Tests + // -------------------------------------------------------------------------- + + @ParameterizedTest(name = "{0} - convertUsers migrates all stored users") + @MethodSource("dbFlavors") + void whenConvertingUsersShouldSaveEachStoredUserToDestination(DbFlavor flavor) { + // GIVEN + truncateAllCoreTables(flavor); + SQLDatabaseManager sourceManager = createManagerFor(flavor); + + String userA = "convert_user_a_" + flavor.name().toLowerCase(); + String userB = "convert_user_b_" + flavor.name().toLowerCase(); + sourceManager.newUser(userA, new UUID(1L, 2L)); + sourceManager.newUser(userB, new UUID(3L, 4L)); + + DatabaseManager destination = mock(DatabaseManager.class); + when(destination.saveUser(any(PlayerProfile.class))).thenReturn(true); + + // WHEN + sourceManager.convertUsers(destination); + + // THEN – destination.saveUser(...) called once per stored user + ArgumentCaptor profileCaptor = ArgumentCaptor.forClass(PlayerProfile.class); + verify(destination, times(2)).saveUser(profileCaptor.capture()); + + assertThat(profileCaptor.getAllValues()) + .extracting(PlayerProfile::getPlayerName) + .containsExactlyInAnyOrder(userA, userB); + + sourceManager.onDisable(); + } + + @ParameterizedTest(name = "{0} - convertUsers on empty database does nothing") + @MethodSource("dbFlavors") + void whenConvertingUsersWithNoStoredUsersShouldNotCallDestination(DbFlavor flavor) { + // GIVEN + truncateAllCoreTables(flavor); + SQLDatabaseManager sourceManager = createManagerFor(flavor); + + DatabaseManager destination = mock(DatabaseManager.class); + when(destination.saveUser(any(PlayerProfile.class))).thenReturn(true); + + // WHEN + sourceManager.convertUsers(destination); + + // THEN + verify(destination, times(0)).saveUser(any(PlayerProfile.class)); + + sourceManager.onDisable(); + } + + // ------------------------------------------------------------------------ // Helpers for legacy schema tests // ------------------------------------------------------------------------