SQLDatabaseManager refactor pass 2

This commit is contained in:
nossr50
2025-11-27 12:42:22 -08:00
parent 43502b255c
commit 99bb38ea06
4 changed files with 392 additions and 97 deletions

View File

@@ -574,105 +574,125 @@ public final class SQLDatabaseManager implements DatabaseManager {
}
public Map<PrimarySkillType, Integer> readRank(String playerName) {
Map<PrimarySkillType, Integer> skills = new HashMap<>();
// NOTE: We keep HashMap so we can still use `null` as the "total" key,
// just like the original code.
Map<PrimarySkillType, Integer> ranks = new HashMap<>();
Connection connection = null;
PreparedStatement statement = null;
ResultSet resultSet = null;
// Preload this player's skill levels & total in a single query
try (Connection connection = getConnection(PoolIdentifier.MISC)) {
try {
connection = getConnection(PoolIdentifier.MISC);
// 1) Load all relevant skill levels for this player in one shot
Map<PrimarySkillType, Integer> levels = new EnumMap<>(PrimarySkillType.class);
int totalLevel = 0;
// Per-skill rank
for (PrimarySkillType primarySkillType : SkillTools.NON_CHILD_SKILLS) {
String skillName = primarySkillType.name().toLowerCase(Locale.ENGLISH);
String loadSql =
"SELECT s.*, u.`user` " +
"FROM " + tablePrefix + "users u " +
"JOIN " + tablePrefix + "skills s ON s.user_id = u.id " +
"WHERE u.`user` = ?";
String sql = "SELECT COUNT(*) AS 'rank' FROM " + tablePrefix + "users JOIN "
+ tablePrefix + "skills ON user_id = id WHERE " + skillName + " > 0 " +
"AND " + skillName + " > (SELECT " + skillName + " FROM " + tablePrefix
+ "users JOIN " + tablePrefix + "skills ON user_id = id " +
"WHERE `user` = ?)";
try (PreparedStatement stmt = connection.prepareStatement(loadSql)) {
stmt.setString(1, playerName);
statement = connection.prepareStatement(sql);
statement.setString(1, playerName);
resultSet = statement.executeQuery();
resultSet.next();
int rank = resultSet.getInt("rank");
resultSet.close();
statement.close();
sql = "SELECT user, " + skillName + " FROM " + tablePrefix + "users JOIN "
+ tablePrefix + "skills ON user_id = id WHERE " + skillName + " > 0 " +
"AND " + skillName + " = (SELECT " + skillName + " FROM " + tablePrefix
+ "users JOIN " + tablePrefix + "skills ON user_id = id " +
"WHERE `user` = '" + playerName + "') ORDER BY user";
statement = connection.prepareStatement(sql);
resultSet = statement.executeQuery();
while (resultSet.next()) {
if (resultSet.getString("user").equalsIgnoreCase(playerName)) {
skills.put(primarySkillType, rank + resultSet.getRow());
break;
try (ResultSet rs = stmt.executeQuery()) {
if (!rs.next()) {
// Player not found in DB, no ranks to report
return ranks;
}
}
resultSet.close();
statement.close();
}
for (PrimarySkillType primarySkillType : SkillTools.NON_CHILD_SKILLS) {
String column = primarySkillType.name().toLowerCase(Locale.ENGLISH);
levels.put(primarySkillType, rs.getInt(column));
}
// Total rank
String sql =
"SELECT COUNT(*) AS 'rank' FROM " + tablePrefix + "users JOIN " + tablePrefix
+ "skills ON user_id = id " +
"WHERE " + ALL_QUERY_VERSION + " > 0 " +
"AND " + ALL_QUERY_VERSION + " > " +
"(SELECT " + ALL_QUERY_VERSION + " " +
"FROM " + tablePrefix + "users JOIN " + tablePrefix
+ "skills ON user_id = id WHERE `user` = ?)";
statement = connection.prepareStatement(sql);
statement.setString(1, playerName);
resultSet = statement.executeQuery();
resultSet.next();
int rank = resultSet.getInt("rank");
resultSet.close();
statement.close();
sql = "SELECT user, " + ALL_QUERY_VERSION + " " +
"FROM " + tablePrefix + "users JOIN " + tablePrefix + "skills ON user_id = id "
+
"WHERE " + ALL_QUERY_VERSION + " > 0 " +
"AND " + ALL_QUERY_VERSION + " = " +
"(SELECT " + ALL_QUERY_VERSION + " " +
"FROM " + tablePrefix + "users JOIN " + tablePrefix
+ "skills ON user_id = id WHERE `user` = ?) ORDER BY user";
statement = connection.prepareStatement(sql);
statement.setString(1, playerName);
resultSet = statement.executeQuery();
while (resultSet.next()) {
if (resultSet.getString("user").equalsIgnoreCase(playerName)) {
skills.put(null, rank + resultSet.getRow());
break;
totalLevel = rs.getInt(ALL_QUERY_VERSION); // "total" column
}
}
// Helper method to compute a rank (base + tie offset + 1)
// for any numeric column on the skills table.
class RankCalculator {
int computeRank(String columnName, int value) throws SQLException {
if (value <= 0) {
// Original logic effectively did not assign a rank when the value <= 0
return -1;
}
// Base: number of players with strictly higher value
String higherSql =
"SELECT COUNT(*) AS cnt " +
"FROM " + tablePrefix + "users u " +
"JOIN " + tablePrefix + "skills s ON s.user_id = u.id " +
"WHERE s." + columnName + " > ?";
int higherCount = 0;
try (PreparedStatement stmt = connection.prepareStatement(higherSql)) {
stmt.setInt(1, value);
try (ResultSet rs = stmt.executeQuery()) {
if (rs.next()) {
higherCount = rs.getInt("cnt");
}
}
}
// Tie offset: number of players with the same value whose username
// sorts alphabetically before this player's name.
String tieSql =
"SELECT COUNT(*) AS cnt " +
"FROM " + tablePrefix + "users u " +
"JOIN " + tablePrefix + "skills s ON s.user_id = u.id " +
"WHERE s." + columnName + " = ? " +
"AND s." + columnName + " > 0 " +
"AND u.`user` < ?";
int tieCount = 0;
try (PreparedStatement stmt = connection.prepareStatement(tieSql)) {
stmt.setInt(1, value);
stmt.setString(2, playerName);
try (ResultSet rs = stmt.executeQuery()) {
if (rs.next()) {
tieCount = rs.getInt("cnt");
}
}
}
// 1-based rank: higher values first, then alphabetical by username
return higherCount + tieCount + 1;
}
}
RankCalculator rankCalculator = new RankCalculator();
// 2) Per-skill rank
for (PrimarySkillType primarySkillType : SkillTools.NON_CHILD_SKILLS) {
int level = levels.getOrDefault(primarySkillType, 0);
int rank = rankCalculator.computeRank(
primarySkillType.name().toLowerCase(Locale.ENGLISH),
level
);
if (rank > 0) {
ranks.put(primarySkillType, rank);
}
}
// 3) Total rank (null key matches original behavior)
if (totalLevel > 0) {
int totalRank = rankCalculator.computeRank(ALL_QUERY_VERSION, totalLevel);
if (totalRank > 0) {
ranks.put(null, totalRank);
}
}
} catch (SQLException ex) {
logSQLException(ex);
} finally {
tryClose(resultSet);
tryClose(statement);
tryClose(connection);
}
return skills;
return ranks;
}
// ---------------------------------------------------------------------
// New user / load profile
// ---------------------------------------------------------------------
@@ -815,7 +835,7 @@ public final class SQLDatabaseManager implements DatabaseManager {
int id = getUserID(connection, playerName, uuid);
if (id == -1) {
return createEmptyProfile(playerName);
return createEmptyProfile(playerName, uuid);
}
writeMissingRows(connection, id);
@@ -895,7 +915,7 @@ public final class SQLDatabaseManager implements DatabaseManager {
try (ResultSet resultSet = statement.executeQuery()) {
if (!resultSet.next()) {
return createEmptyProfile(playerName);
return createEmptyProfile(playerName, uuid);
}
String nameInDb = resultSet.getString("username");
@@ -914,12 +934,12 @@ public final class SQLDatabaseManager implements DatabaseManager {
}
} catch (SQLException ex) {
logSQLException(ex);
return createEmptyProfile(playerName);
return createEmptyProfile(playerName, uuid);
}
}
private PlayerProfile createEmptyProfile(@Nullable String playerName) {
return new PlayerProfile(playerName, mcMMO.p.getAdvancedConfig().getStartingLevel());
private PlayerProfile createEmptyProfile(@Nullable String playerName, @Nullable UUID uuid) {
return new PlayerProfile(playerName, uuid, mcMMO.p.getAdvancedConfig().getStartingLevel());
}
private boolean shouldUpdateUsername(@Nullable String playerName,
@@ -1354,8 +1374,6 @@ public final class SQLDatabaseManager implements DatabaseManager {
}
private void deleteOrphans(Connection connection) throws SQLException {
LogUtils.debug(logger, "Killing orphans");
try (Statement stmt = connection.createStatement()) {
stmt.executeUpdate(
"DELETE FROM `" + tablePrefix + "experience` " +
@@ -1455,7 +1473,6 @@ public final class SQLDatabaseManager implements DatabaseManager {
*/
private void checkDatabaseStructure(Connection connection, UpgradeType upgrade) {
if (!mcMMO.getUpgradeManager().shouldUpgrade(upgrade)) {
LogUtils.debug(logger, "Skipping " + upgrade.name() + " upgrade (unneeded)");
return;
}
@@ -1463,10 +1480,6 @@ public final class SQLDatabaseManager implements DatabaseManager {
switch (upgrade) {
case ADD_FISHING -> checkUpgradeAddFishing(statement);
case ADD_BLAST_MINING_COOLDOWN -> checkUpgradeAddBlastMiningCooldown(statement);
case ADD_SQL_INDEXES -> {
// historical: currently disabled
// checkUpgradeAddSQLIndexes(statement);
}
case ADD_MOB_HEALTHBARS -> checkUpgradeAddMobHealthbars(statement);
case DROP_SQL_PARTY_NAMES -> checkUpgradeDropPartyNames(statement);
case DROP_SPOUT -> checkUpgradeDropSpout(statement);
@@ -1721,13 +1734,22 @@ public final class SQLDatabaseManager implements DatabaseManager {
mcMMO.getUpgradeManager().setUpgradeCompleted(UpgradeType.ADD_SKILL_TOTAL);
} catch (SQLException ex) {
logSQLException(ex);
try {
connection.rollback();
} catch (SQLException ignored) {
// best effort
}
} finally {
connection.setAutoCommit(true);
try {
connection.setAutoCommit(true);
} catch (SQLException ignored) {
}
tryClose(resultSet);
tryClose(statement);
}
}
private void checkUpgradeDropSpout(final Statement statement) {
ResultSet resultSet = null;

View File

@@ -3,7 +3,6 @@ package com.gmail.nossr50.datatypes.database;
public enum UpgradeType {
ADD_FISHING,
ADD_BLAST_MINING_COOLDOWN,
ADD_SQL_INDEXES,
ADD_MOB_HEALTHBARS,
DROP_SQL_PARTY_NAMES,
DROP_SPOUT,

View File

@@ -8,6 +8,7 @@ public class LogUtils {
public static final String DEBUG_STR = "[D] ";
public static void debug(@NotNull Logger logger, @NotNull String message) {
// Messages here will get filtered based on config settings via LogFilter
logger.info(DEBUG_STR + message);
}
}

View File

@@ -1184,6 +1184,246 @@ class SQLDatabaseManagerTest {
databaseManager.onDisable();
}
@ParameterizedTest(name = "{0} - readRank for unknown user returns empty map")
@MethodSource("dbFlavors")
void whenReadingRankForUnknownUserShouldReturnEmptyMap(DbFlavor flavor) {
truncateAllCoreTables(flavor);
SQLDatabaseManager databaseManager = createManagerFor(flavor);
try {
Map<PrimarySkillType, Integer> ranks = databaseManager.readRank("ghost_" + flavor.name().toLowerCase());
assertThat(ranks)
.as("Unknown user should yield an empty rank map")
.isEmpty();
} finally {
databaseManager.onDisable();
}
}
@ParameterizedTest(name = "{0} - readRank for zero-skill user returns empty map")
@MethodSource("dbFlavors")
void whenReadingRankForZeroSkillUserShouldReturnEmptyMap(DbFlavor flavor) {
truncateAllCoreTables(flavor);
SQLDatabaseManager databaseManager = createManagerFor(flavor);
String zeroName = "zeros_" + flavor.name().toLowerCase();
UUID zeroUuid = UUID.randomUUID();
try {
// newUser -> all skills 0, total 0
databaseManager.newUser(zeroName, zeroUuid);
PlayerProfile zeroProfile = databaseManager.loadPlayerProfile(zeroUuid);
assertThat(databaseManager.saveUser(zeroProfile)).isTrue();
// Also create a powered user for sanity; zero user still should not be ranked
String poweredName = "nonzero_" + flavor.name().toLowerCase();
UUID poweredUuid = UUID.randomUUID();
createUserWithUniformNonChildSkills(databaseManager, poweredName, poweredUuid, 100);
Map<PrimarySkillType, Integer> ranks = databaseManager.readRank(zeroName);
assertThat(ranks)
.as("User with all skills at 0 should not have any rank entries")
.isEmpty();
} finally {
databaseManager.onDisable();
}
}
@ParameterizedTest(name = "{0} - readRank with a single user → rank 1 for all non-child skills")
@MethodSource("dbFlavors")
void whenSingleUserShouldBeRankOneForAllNonChildSkills(DbFlavor flavor) {
truncateAllCoreTables(flavor);
SQLDatabaseManager databaseManager = createManagerFor(flavor);
String soloName = "solo_" + flavor.name().toLowerCase();
UUID soloUuid = UUID.randomUUID();
try {
// All non-child skills = 50
createUserWithUniformNonChildSkills(databaseManager, soloName, soloUuid, 50);
Map<PrimarySkillType, Integer> ranks = databaseManager.readRank(soloName);
for (PrimarySkillType type : PrimarySkillType.values()) {
if (SkillTools.isChildSkill(type)) {
assertThat(ranks.get(type))
.as("Child skill %s should have no rank", type)
.isNull();
} else {
assertThat(ranks.get(type))
.as("Solo player should be rank 1 for skill %s", type)
.isEqualTo(1);
}
}
// Total rank (null key) should also be 1
assertThat(ranks.get(null))
.as("Solo player total rank should be 1")
.isEqualTo(1);
} finally {
databaseManager.onDisable();
}
}
@ParameterizedTest(name = "{0} - readRank alphabetical tiebreaker with only equal-skill users")
@MethodSource("dbFlavors")
void whenEqualSkillUsersOnlyShouldUseAlphabeticalTiebreaker(DbFlavor flavor) {
truncateAllCoreTables(flavor);
SQLDatabaseManager databaseManager = createManagerFor(flavor);
String nameA = "aaa_" + flavor.name().toLowerCase();
String nameB = "bbb_" + flavor.name().toLowerCase();
String nameC = "ccc_" + flavor.name().toLowerCase();
UUID uuidA = UUID.randomUUID();
UUID uuidB = UUID.randomUUID();
UUID uuidC = UUID.randomUUID();
try {
// For simplicity, set only MINING and let total = mining
Map<PrimarySkillType, Integer> skillMap = Map.of(PrimarySkillType.MINING, 100);
createUserWithSkills(databaseManager, nameA, uuidA, skillMap);
createUserWithSkills(databaseManager, nameB, uuidB, skillMap);
createUserWithSkills(databaseManager, nameC, uuidC, skillMap);
Map<PrimarySkillType, Integer> ranksA = databaseManager.readRank(nameA);
Map<PrimarySkillType, Integer> ranksB = databaseManager.readRank(nameB);
Map<PrimarySkillType, Integer> ranksC = databaseManager.readRank(nameC);
// Mining ranks: alphabetical order
assertThat(ranksA.get(PrimarySkillType.MINING)).isEqualTo(1);
assertThat(ranksB.get(PrimarySkillType.MINING)).isEqualTo(2);
assertThat(ranksC.get(PrimarySkillType.MINING)).isEqualTo(3);
// Total ranks behave the same in this setup (total == mining)
assertThat(ranksA.get(null)).isEqualTo(1);
assertThat(ranksB.get(null)).isEqualTo(2);
assertThat(ranksC.get(null)).isEqualTo(3);
} finally {
databaseManager.onDisable();
}
}
@ParameterizedTest(name = "{0} - readRank tie group is offset by number of higher players")
@MethodSource("dbFlavors")
void whenEqualSkillUsersHaveHigherPlayerShouldOffsetByHigherCount(DbFlavor flavor) {
truncateAllCoreTables(flavor);
SQLDatabaseManager databaseManager = createManagerFor(flavor);
String higherName = "zoe_" + flavor.name().toLowerCase();
String nameA = "aaa2_" + flavor.name().toLowerCase();
String nameB = "bbb2_" + flavor.name().toLowerCase();
String nameC = "ccc2_" + flavor.name().toLowerCase();
UUID uuidHigher = UUID.randomUUID();
UUID uuidA = UUID.randomUUID();
UUID uuidB = UUID.randomUUID();
UUID uuidC = UUID.randomUUID();
try {
// Higher player
createUserWithSkills(
databaseManager,
higherName,
uuidHigher,
Map.of(PrimarySkillType.MINING, 200)
);
// Tie group
Map<PrimarySkillType, Integer> tieSkills = Map.of(PrimarySkillType.MINING, 100);
createUserWithSkills(databaseManager, nameA, uuidA, tieSkills);
createUserWithSkills(databaseManager, nameB, uuidB, tieSkills);
createUserWithSkills(databaseManager, nameC, uuidC, tieSkills);
Map<PrimarySkillType, Integer> higherRanks = databaseManager.readRank(higherName);
Map<PrimarySkillType, Integer> ranksA = databaseManager.readRank(nameA);
Map<PrimarySkillType, Integer> ranksB = databaseManager.readRank(nameB);
Map<PrimarySkillType, Integer> ranksC = databaseManager.readRank(nameC);
// Higher player is rank 1
assertThat(higherRanks.get(PrimarySkillType.MINING)).isEqualTo(1);
// Others follow in alphabetical order, offset by 1
assertThat(ranksA.get(PrimarySkillType.MINING)).isEqualTo(2);
assertThat(ranksB.get(PrimarySkillType.MINING)).isEqualTo(3);
assertThat(ranksC.get(PrimarySkillType.MINING)).isEqualTo(4);
} finally {
databaseManager.onDisable();
}
}
@ParameterizedTest(name = "{0} - readRank per-skill vs total ranking can differ")
@MethodSource("dbFlavors")
void whenDifferentSkillDistributionsShouldComputePerSkillAndTotalRanksSeparately(DbFlavor flavor) {
truncateAllCoreTables(flavor);
SQLDatabaseManager databaseManager = createManagerFor(flavor);
String alphaName = "alpha_" + flavor.name().toLowerCase();
String bravoName = "bravo_" + flavor.name().toLowerCase();
String charlieName = "charlie_" + flavor.name().toLowerCase();
UUID alphaUuid = UUID.randomUUID();
UUID bravoUuid = UUID.randomUUID();
UUID charlieUuid = UUID.randomUUID();
try {
// alpha: mining 100, fishing 0
createUserWithSkills(
databaseManager,
alphaName,
alphaUuid,
Map.of(PrimarySkillType.MINING, 100)
);
// bravo: mining 50, fishing 200
createUserWithSkills(
databaseManager,
bravoName,
bravoUuid,
Map.of(
PrimarySkillType.MINING, 50,
PrimarySkillType.FISHING, 200
)
);
// charlie: mining 75, fishing 50
createUserWithSkills(
databaseManager,
charlieName,
charlieUuid,
Map.of(
PrimarySkillType.MINING, 75,
PrimarySkillType.FISHING, 50
)
);
Map<PrimarySkillType, Integer> alphaRanks = databaseManager.readRank(alphaName);
Map<PrimarySkillType, Integer> bravoRanks = databaseManager.readRank(bravoName);
Map<PrimarySkillType, Integer> charlieRanks = databaseManager.readRank(charlieName);
// --- Mining (100 > 75 > 50) ---
assertThat(alphaRanks.get(PrimarySkillType.MINING)).isEqualTo(1);
assertThat(charlieRanks.get(PrimarySkillType.MINING)).isEqualTo(2);
assertThat(bravoRanks.get(PrimarySkillType.MINING)).isEqualTo(3);
// --- Fishing (200 > 50 > 0) ---
// alpha has 0 -> no rank entry for fishing
assertThat(alphaRanks.get(PrimarySkillType.FISHING)).isNull();
assertThat(bravoRanks.get(PrimarySkillType.FISHING)).isEqualTo(1);
assertThat(charlieRanks.get(PrimarySkillType.FISHING)).isEqualTo(2);
// --- Total: alpha 100, bravo 250, charlie 125 ---
assertThat(bravoRanks.get(null)).isEqualTo(1); // 250 highest
assertThat(charlieRanks.get(null)).isEqualTo(2); // 125
assertThat(alphaRanks.get(null)).isEqualTo(3); // 100
} finally {
databaseManager.onDisable();
}
}
// ------------------------------------------------------------------------
// getDatabaseType
@@ -1385,4 +1625,37 @@ class SQLDatabaseManagerTest {
return resultSet.next();
}
}
// ------------------------------------------------------------------------
// Helpers for readRank tests
// ------------------------------------------------------------------------
private void createUserWithUniformNonChildSkills(SQLDatabaseManager manager,
String name,
UUID uuid,
int level) {
manager.newUser(name, uuid);
PlayerProfile profile = manager.loadPlayerProfile(uuid);
for (PrimarySkillType type : PrimarySkillType.values()) {
if (SkillTools.isChildSkill(type)) {
continue;
}
profile.modifySkill(type, level);
}
assertThat(manager.saveUser(profile)).isTrue();
}
private void createUserWithSkills(SQLDatabaseManager manager,
String name,
UUID uuid,
Map<PrimarySkillType, Integer> levels) {
manager.newUser(name, uuid);
PlayerProfile profile = manager.loadPlayerProfile(uuid);
for (Map.Entry<PrimarySkillType, Integer> e : levels.entrySet()) {
// modifySkill adds; starting level is 0 in tests
profile.modifySkill(e.getKey(), e.getValue());
}
assertThat(manager.saveUser(profile)).isTrue();
}
}