Refactor DB code a bit and fix console spam when using the Plan plugin

Fixes #4450

Changelog.txt

@@ -1,3 +1,10 @@
+Version 2.1.181
+    Removed the "name change detected" message as some plugins (such as Plan) invoke API calls which spams the console with this message
+    Refactored code related to loading player data from the database
+    (API) Added DatabaseManager::loadPlayerProfile(String)
+    (API) Removed DatabaseManager::loadPlayerProfile(String, UUID, boolean)
+    (API) Removed DatabaseManager::loadPlayerProfile(String, boolean)
+
 Version 2.1.180
     mcMMO will now automatically remove corrupted data from mcmmo.users instead of catastrophic failure
     When using FlatFile database (the default) mcMMO will try its best to inform you which players had corrupted data when it does repairs

pom.xml

@@ -2,7 +2,7 @@
     <modelVersion>4.0.0</modelVersion>
     <groupId>com.gmail.nossr50.mcMMO</groupId>
     <artifactId>mcMMO</artifactId>
-    <version>2.1.180</version>
+    <version>2.1.181-SNAPSHOT</version>
     <name>mcMMO</name>
     <url>https://github.com/mcMMO-Dev/mcMMO</url>
     <scm>

src/main/java/com/gmail/nossr50/api/DatabaseAPI.java

@@ -22,7 +22,7 @@ public class DatabaseAPI {
      * @return true if the player exists in the DB, false if they do not
      */
     public boolean doesPlayerExistInDB(UUID uuid) {
-        PlayerProfile playerProfile = mcMMO.getDatabaseManager().loadPlayerProfile(uuid);
+        PlayerProfile playerProfile = mcMMO.getDatabaseManager().loadPlayerProfile(uuid, null);
 
         return playerProfile.isLoaded();
     }

src/main/java/com/gmail/nossr50/api/ExperienceAPI.java

@@ -13,6 +13,8 @@ import com.gmail.nossr50.mcMMO;
 import com.gmail.nossr50.skills.child.FamilyTree;
 import com.gmail.nossr50.util.player.UserManager;
 import com.gmail.nossr50.util.skills.CombatUtils;
+import org.bukkit.Bukkit;
+import org.bukkit.OfflinePlayer;
 import org.bukkit.block.BlockState;
 import org.bukkit.entity.LivingEntity;
 import org.bukkit.entity.Player;
@@ -714,7 +716,6 @@ public final class ExperienceAPI {
      * @throws InvalidSkillException if the given skill is not valid
      * @throws InvalidPlayerException if the given player does not exist in the database
      */
-    @Deprecated
     public static int getLevelOffline(String playerName, String skillType) {
         return getOfflineProfile(playerName).getSkillLevel(getSkillType(skillType));
     }
@@ -1126,8 +1127,6 @@ public final class ExperienceAPI {
         }
     }
 
-
-
     // Utility methods follow.
     private static void addOfflineXP(UUID playerUniqueId, PrimarySkillType skill, int XP) {
         PlayerProfile profile = getOfflineProfile(playerUniqueId);
@@ -1144,8 +1143,10 @@ public final class ExperienceAPI {
         profile.scheduleAsyncSave();
     }
 
-    private static PlayerProfile getOfflineProfile(UUID uuid) {
-        PlayerProfile profile = mcMMO.getDatabaseManager().loadPlayerProfile(uuid);
+    private static PlayerProfile getOfflineProfile(UUID uuid) throws InvalidPlayerException {
+        OfflinePlayer offlinePlayer = Bukkit.getServer().getOfflinePlayer(uuid);
+        String playerName = offlinePlayer.getName();
+        PlayerProfile profile = mcMMO.getDatabaseManager().loadPlayerProfile(uuid, playerName);
 
         if (!profile.isLoaded()) {
             throw new InvalidPlayerException();
@@ -1155,9 +1156,10 @@ public final class ExperienceAPI {
     }
 
     @Deprecated
-    private static PlayerProfile getOfflineProfile(String playerName) {
-        UUID uuid = mcMMO.p.getServer().getOfflinePlayer(playerName).getUniqueId();
-        PlayerProfile profile = mcMMO.getDatabaseManager().loadPlayerProfile(uuid);
+    private static PlayerProfile getOfflineProfile(String playerName) throws InvalidPlayerException {
+        OfflinePlayer offlinePlayer = Bukkit.getServer().getOfflinePlayer(playerName);
+        UUID uuid = offlinePlayer.getUniqueId();
+        PlayerProfile profile = mcMMO.getDatabaseManager().loadPlayerProfile(uuid, playerName);
 
         if (!profile.isLoaded()) {
             throw new InvalidPlayerException();

src/main/java/com/gmail/nossr50/commands/database/ConvertDatabaseCommand.java

@@ -54,7 +54,7 @@ public class ConvertDatabaseCommand implements CommandExecutor {
             UserManager.clearAll();
 
             for (Player player : mcMMO.p.getServer().getOnlinePlayers()) {
-                PlayerProfile profile = oldDatabase.loadPlayerProfile(player.getUniqueId());
+                PlayerProfile profile = oldDatabase.loadPlayerProfile(player.getUniqueId(), null);
 
                 if (profile.isLoaded()) {
                     mcMMO.getDatabaseManager().saveUser(profile);

src/main/java/com/gmail/nossr50/commands/database/McremoveCommand.java

@@ -22,7 +22,7 @@ public class McremoveCommand implements TabExecutor {
         if (args.length == 1) {
             String playerName = CommandUtils.getMatchedPlayerName(args[0]);
 
-            if (UserManager.getOfflinePlayer(playerName) == null && CommandUtils.unloadedProfile(sender, mcMMO.getDatabaseManager().loadPlayerProfile(playerName, false))) {
+            if (UserManager.getOfflinePlayer(playerName) == null && CommandUtils.unloadedProfile(sender, mcMMO.getDatabaseManager().loadPlayerProfile(playerName))) {
                 return true;
             }
 

src/main/java/com/gmail/nossr50/commands/experience/ExperienceCommand.java

@@ -97,14 +97,20 @@ public abstract class ExperienceCommand implements TabExecutor {
                 // If the mcMMOPlayer doesn't exist, create a temporary profile and check if it's present in the database. If it's not, abort the process.
                 if (mcMMOPlayer == null) {
                     UUID uuid = null;
-                    OfflinePlayer player = mcMMO.p.getServer().getOfflinePlayer(playerName);
-                    if (player != null) {
-                        uuid = player.getUniqueId();
-                    }
-                    PlayerProfile profile = mcMMO.getDatabaseManager().loadPlayerProfile(playerName, uuid, false);
+                    OfflinePlayer offlinePlayer = mcMMO.p.getServer().getOfflinePlayer(playerName);
+                    PlayerProfile profile;
+
+                    uuid = offlinePlayer.getUniqueId();
+                    profile = mcMMO.getDatabaseManager().loadPlayerProfile(uuid, null);
 
+                    //Check loading by UUID
                     if (CommandUtils.unloadedProfile(sender, profile)) {
-                        return true;
+                        //Check loading by name
+                        profile = mcMMO.getDatabaseManager().loadPlayerProfile(playerName);
+
+                        if(CommandUtils.unloadedProfile(sender, profile)) {
+                            return true;
+                        }
                     }
 
                     editValues(null, profile, skill, value, isSilent(args));

src/main/java/com/gmail/nossr50/commands/experience/SkillresetCommand.java

@@ -80,13 +80,19 @@ public class SkillresetCommand implements TabExecutor {
                 if (mcMMOPlayer == null) {
                     UUID uuid = null;
                     OfflinePlayer player = mcMMO.p.getServer().getOfflinePlayer(playerName);
-                    if (player != null) {
-                        uuid = player.getUniqueId();
-                    }
-                    PlayerProfile profile = mcMMO.getDatabaseManager().loadPlayerProfile(playerName, uuid, false);
+                    uuid = player.getUniqueId();
+
+                    PlayerProfile profile = mcMMO.getDatabaseManager().loadPlayerProfile(uuid, playerName);
 
+                    //Check loading by UUID
                     if (CommandUtils.unloadedProfile(sender, profile)) {
-                        return true;
+                        //Didn't find it by UUID so try to find it by name
+                        profile = mcMMO.getDatabaseManager().loadPlayerProfile(playerName);
+
+                        //Check if it was present in DB
+                        if(CommandUtils.unloadedProfile(sender, profile)) {
+                            return true;
+                        }
                     }
 
                     editValues(null, profile, skill);

src/main/java/com/gmail/nossr50/commands/player/InspectCommand.java

@@ -30,7 +30,7 @@ public class InspectCommand implements TabExecutor {
 
             // If the mcMMOPlayer doesn't exist, create a temporary profile and check if it's present in the database. If it's not, abort the process.
             if (mcMMOPlayer == null) {
-                PlayerProfile profile = mcMMO.getDatabaseManager().loadPlayerProfile(playerName, false); // Temporary Profile
+                PlayerProfile profile = mcMMO.getDatabaseManager().loadPlayerProfile(playerName); // Temporary Profile
 
                 if (!CommandUtils.isLoaded(sender, profile)) {
                     return true;

src/main/java/com/gmail/nossr50/database/DatabaseManager.java

@@ -6,6 +6,7 @@ import com.gmail.nossr50.datatypes.database.DatabaseType;
 import com.gmail.nossr50.datatypes.database.PlayerStat;
 import com.gmail.nossr50.datatypes.player.PlayerProfile;
 import com.gmail.nossr50.datatypes.skills.PrimarySkillType;
+import org.bukkit.entity.Player;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -83,19 +84,16 @@ public interface DatabaseManager {
      */
     void newUser(String playerName, UUID uuid);
 
+    @NotNull PlayerProfile newUser(@NotNull Player player);
+
     /**
      * Load a player from the database.
      *
-     * @deprecated replaced by {@link #loadPlayerProfile(String playerName, UUID uuid, boolean createNew)}
-     *
      * @param playerName The name of the player to load from the database
-     * @param createNew Whether to create a new record if the player is not
-     *          found
      * @return The player's data, or an unloaded PlayerProfile if not found
      *          and createNew is false
      */
-    @Deprecated
-    PlayerProfile loadPlayerProfile(String playerName, boolean createNew);
+    @NotNull PlayerProfile loadPlayerProfile(@NotNull String playerName);
 
     /**
      * Load a player from the database.
@@ -103,19 +101,7 @@ public interface DatabaseManager {
      * @param uuid The uuid of the player to load from the database
      * @return The player's data, or an unloaded PlayerProfile if not found
      */
-    PlayerProfile loadPlayerProfile(UUID uuid);
-
-    /**
-     * Load a player from the database. Attempt to use uuid, fall back on playername
-     *
-     * @param playerName The name of the player to load from the database
-     * @param uuid The uuid of the player to load from the database
-     * @param createNew Whether to create a new record if the player is not
-     *          found
-     * @return The player's data, or an unloaded PlayerProfile if not found
-     *          and createNew is false
-     */
-    PlayerProfile loadPlayerProfile(String playerName, UUID uuid, boolean createNew);
+    @NotNull PlayerProfile loadPlayerProfile(@NotNull UUID uuid, @Nullable String playerName);
 
     /**
      * Get all users currently stored in the database.

src/main/java/com/gmail/nossr50/database/FlatFileDatabaseManager.java

@@ -14,8 +14,8 @@ import com.gmail.nossr50.datatypes.skills.SuperAbilityType;
 import com.gmail.nossr50.mcMMO;
 import com.gmail.nossr50.runnables.database.UUIDUpdateAsyncTask;
 import com.gmail.nossr50.util.Misc;
-import com.gmail.nossr50.util.text.StringUtils;
 import org.bukkit.OfflinePlayer;
+import org.bukkit.entity.Player;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -461,6 +461,11 @@ public final class FlatFileDatabaseManager implements DatabaseManager {
         return skills;
     }
 
+    public @NotNull PlayerProfile newUser(@NotNull Player player) {
+        newUser(player.getName(), player.getUniqueId());
+        return new PlayerProfile(player.getName(), player.getUniqueId(), true);
+    }
+
     public void newUser(String playerName, UUID uuid) {
         BufferedWriter out = null;
         synchronized (fileWritingLock) {
@@ -534,17 +539,15 @@ public final class FlatFileDatabaseManager implements DatabaseManager {
         }
     }
 
-    @Deprecated
-    public PlayerProfile loadPlayerProfile(String playerName, boolean create) {
-        return loadPlayerProfile(playerName, null, false);
+    public @NotNull PlayerProfile loadPlayerProfile(@NotNull String playerName) {
+        return loadPlayerByName(playerName);
     }
 
-    public PlayerProfile loadPlayerProfile(UUID uuid) {
-        return loadPlayerProfile("", uuid, false);
+    public @NotNull PlayerProfile loadPlayerProfile(@NotNull UUID uuid, @Nullable String playerName) {
+        return loadPlayerByUUID(uuid, playerName);
     }
 
-    public PlayerProfile loadPlayerProfile(String playerName, UUID uuid, boolean create) {
-//        boolean updateRequired = false;
+    private @NotNull PlayerProfile loadPlayerByUUID(@NotNull UUID uuid, @Nullable String playerName) {
         BufferedReader in = null;
         String usersFilePath = mcMMO.getUsersFilePath();
 
@@ -558,68 +561,105 @@ public final class FlatFileDatabaseManager implements DatabaseManager {
                     // Find if the line contains the player we want.
                     String[] rawSplitData = line.split(":");
 
-                    if(rawSplitData.length < (USERNAME_INDEX + 1)) {
-                        //Users without a name aren't worth it
-                        mcMMO.p.getLogger().severe("Corrupted data was found in mcmmo.users, removing it from the database");
+                    /* Don't read corrupt data */
+                    if(rawSplitData.length < (UUID_INDEX + 1)) {
+                        continue;
                     }
 
-                    // Compare names because we don't have a valid uuid for that player even
-                    // if input uuid is not null
+                    /* Does this entry have a UUID? */
                     if (rawSplitData[UUID_INDEX].equalsIgnoreCase("NULL")
                             || rawSplitData[UUID_INDEX].isEmpty()
                             || rawSplitData[UUID_INDEX].equalsIgnoreCase("")) {
-                        if (!rawSplitData[USERNAME_INDEX].equalsIgnoreCase(playerName)) {
-                            continue;
-                        }
+                        continue; //No UUID entry found for this data in the DB, go to next entry
                     }
 
-                    // If input uuid is not null then we should compare uuids
-                    else if ((uuid != null && !rawSplitData[UUID_INDEX].equalsIgnoreCase(uuid.toString())) || (uuid == null && !rawSplitData[USERNAME_INDEX].equalsIgnoreCase(playerName))) {
-                        continue;
+                    // Compare provided UUID to DB
+                    if (!rawSplitData[UUID_INDEX].equalsIgnoreCase(uuid.toString())) {
+                        continue; //Doesn't match, go to the next entry
                     }
 
-                    // Update playerName in database after name change
+                    /*
+                     * UUID Matched!
+                     * Making it this far means the current data line is considered a match
+                     */
+
+
+                    /* Check for nickname changes and update since we are here anyways */
                     if (!rawSplitData[USERNAME_INDEX].equalsIgnoreCase(playerName)) {
-                        //TODO: A proper fix for changed names
-                        mcMMO.p.getLogger().info("Name change detected: " + rawSplitData[USERNAME_INDEX] + " => " + playerName);
+                        //mcMMO.p.getLogger().info("Name updated for player: " + rawSplitData[USERNAME_INDEX] + " => " + playerName);
                         rawSplitData[USERNAME_INDEX] = playerName;
-//                        updateRequired = true; //Flag profile to update
                     }
 
                     return loadFromLine(rawSplitData);
                 }
+            } catch (Exception e) {
+                e.printStackTrace();
+            } finally {
+                // I have no idea why it's necessary to inline tryClose() here, but it removes
+                // a resource leak warning, and I'm trusting the compiler on this one.
+                if (in != null) {
+                    try {
+                        in.close();
+                    } catch (IOException e) {
+                        // Ignore
+                    }
+                }
+            }
+        }
+
+        /*
+         * No match was found in the file
+         */
+
+        return grabUnloadedProfile(uuid, playerName); //Create an empty new profile and return
+    }
 
-                // Didn't find the player, create a new one
-                if (create) {
-                    if (uuid == null) {
-                        newUser(playerName, uuid);
-                        return new PlayerProfile(playerName, true);
+    private @NotNull PlayerProfile loadPlayerByName(@NotNull String playerName) {
+        BufferedReader in = null;
+        String usersFilePath = mcMMO.getUsersFilePath();
+
+        synchronized (fileWritingLock) {
+            try {
+                // Open the user file
+                in = new BufferedReader(new FileReader(usersFilePath));
+                String line;
+
+                while ((line = in.readLine()) != null) {
+                    // Find if the line contains the player we want.
+                    String[] rawSplitData = line.split(":");
+
+                    /* Don't read corrupt data */
+                    if(rawSplitData.length < (USERNAME_INDEX + 1)) {
+                        continue;
                     }
 
-                    newUser(playerName, uuid);
-                    return new PlayerProfile(playerName, uuid, true);
+                    //If we couldn't find anyone
+                    if(playerName.equalsIgnoreCase(rawSplitData[USERNAME_INDEX])) {
+                        return loadFromLine(rawSplitData);
+                    }
                 }
-            }
-            catch (Exception e) {
+            } catch (Exception e) {
                 e.printStackTrace();
-            }
-            finally {
+            } finally {
                 // I have no idea why it's necessary to inline tryClose() here, but it removes
                 // a resource leak warning, and I'm trusting the compiler on this one.
                 if (in != null) {
                     try {
                         in.close();
-                    }
-                    catch (IOException e) {
+                    } catch (IOException e) {
                         // Ignore
                     }
                 }
             }
         }
 
-        // Return unloaded profile
-        if (uuid == null) {
-            return new PlayerProfile(playerName);
+        //Return a new blank profile
+        return new PlayerProfile(playerName, null);
+    }
+
+    private @NotNull PlayerProfile grabUnloadedProfile(@NotNull UUID uuid, @Nullable String playerName) {
+        if(playerName == null) {
+            playerName = ""; //No name for you boy!
         }
 
         return new PlayerProfile(playerName, uuid);
@@ -1205,8 +1245,6 @@ public final class FlatFileDatabaseManager implements DatabaseManager {
         return skills;
     }
 
-
-
     public DatabaseType getDatabaseType() {
         return DatabaseType.FLATFILE;
     }

src/main/java/com/gmail/nossr50/database/SQLDatabaseManager.java

@@ -16,6 +16,7 @@ import com.gmail.nossr50.runnables.database.UUIDUpdateAsyncTask;
 import com.gmail.nossr50.util.Misc;
 import org.apache.tomcat.jdbc.pool.DataSource;
 import org.apache.tomcat.jdbc.pool.PoolProperties;
+import org.bukkit.entity.Player;
 import org.bukkit.scheduler.BukkitRunnable;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -506,6 +507,24 @@ public final class SQLDatabaseManager implements DatabaseManager {
         }
     }
 
+    @Override
+    public @NotNull PlayerProfile newUser(@NotNull Player player) {
+        try {
+            Connection connection = getConnection(PoolIdentifier.SAVE);
+            int id = newUser(connection, player.getName(), player.getUniqueId());
+
+            if (id == -1) {
+                return new PlayerProfile(player.getName(), player.getUniqueId(), false);
+            } else {
+                return loadPlayerProfile(player.getUniqueId(), player.getName());
+            }
+        } catch (SQLException e) {
+            e.printStackTrace();
+        }
+
+        return new PlayerProfile(player.getName(), player.getUniqueId(), false);
+    }
+
     private int newUser(Connection connection, String playerName, UUID uuid) {
         ResultSet resultSet = null;
         PreparedStatement statement = null;
@@ -544,20 +563,24 @@ public final class SQLDatabaseManager implements DatabaseManager {
         return -1;
     }
 
-    @Deprecated
-    public PlayerProfile loadPlayerProfile(String playerName, boolean create) {
-        return loadPlayerProfile(playerName, null, false, true);
+    public @NotNull PlayerProfile loadPlayerProfile(@NotNull String playerName) {
+        try {
+            return loadPlayerFromDB(null, playerName);
+        } catch (RuntimeException e) {
+            e.printStackTrace();
+            return new PlayerProfile(playerName, false);
+        }
     }
 
-    public PlayerProfile loadPlayerProfile(UUID uuid) {
-        return loadPlayerProfile("", uuid, false, true);
+    public @NotNull PlayerProfile loadPlayerProfile(@NotNull UUID uuid, @Nullable String playerName) {
+        return loadPlayerFromDB(uuid, playerName);
     }
 
-    public PlayerProfile loadPlayerProfile(String playerName, UUID uuid, boolean create) {
-        return loadPlayerProfile(playerName, uuid, create, true);
-    }
+    private PlayerProfile loadPlayerFromDB(@Nullable UUID uuid, @Nullable String playerName) throws RuntimeException {
+        if(uuid == null && playerName == null) {
+            throw new RuntimeException("Error looking up player, both UUID and playerName are null and one must not be.");
+        }
 
-    private PlayerProfile loadPlayerProfile(String playerName, UUID uuid, boolean create, boolean retry) {
         PreparedStatement statement = null;
         Connection connection = null;
         ResultSet resultSet = null;
@@ -567,16 +590,8 @@ public final class SQLDatabaseManager implements DatabaseManager {
             int id = getUserID(connection, playerName, uuid);
 
             if (id == -1) {
-                // There is no such user
-                if (create) {
-                    id = newUser(connection, playerName, uuid);
-                    create = false;
-                    if (id == -1) {
-                        return new PlayerProfile(playerName, false);
-                    }
-                } else {
-                    return new PlayerProfile(playerName, false);
-                }
+            // There is no such user
+                return new PlayerProfile(playerName, false);
             }
             // There is such a user
             writeMissingRows(connection, id);
@@ -604,7 +619,10 @@ public final class SQLDatabaseManager implements DatabaseManager {
                     resultSet.close();
                     statement.close();
 
-                    if (!playerName.isEmpty() && !playerName.equalsIgnoreCase(name) && uuid != null) {
+                    if (playerName != null
+                            && !playerName.isEmpty()
+                            && !playerName.equalsIgnoreCase(name)
+                            && uuid != null) {
                         statement = connection.prepareStatement(
                                 "UPDATE `" + tablePrefix + "users` "
                                         + "SET user = ? "
@@ -641,15 +659,8 @@ public final class SQLDatabaseManager implements DatabaseManager {
             tryClose(connection);
         }
 
-        // Problem, nothing was returned
-
-        // return unloaded profile
-        if (!retry) {
-            return new PlayerProfile(playerName, false);
-        }
-
-        // Retry, and abort on re-failure
-        return loadPlayerProfile(playerName, uuid, create, false);
+        //Return empty profile
+        return new PlayerProfile(playerName, false);
     }
 
     public void convertUsers(DatabaseManager destination) {

src/main/java/com/gmail/nossr50/runnables/database/FormulaConversionTask.java

@@ -32,7 +32,7 @@ public class FormulaConversionTask extends BukkitRunnable {
 
             // If the mcMMOPlayer doesn't exist, create a temporary profile and check if it's present in the database. If it's not, abort the process.
             if (mcMMOPlayer == null) {
-                profile = mcMMO.getDatabaseManager().loadPlayerProfile(playerName, false);
+                profile = mcMMO.getDatabaseManager().loadPlayerProfile(playerName);
 
                 if (!profile.isLoaded()) {
                     mcMMO.p.debug("Profile not loaded.");

src/main/java/com/gmail/nossr50/runnables/player/PlayerProfileLoadingTask.java

@@ -42,7 +42,13 @@ public class PlayerProfileLoadingTask extends BukkitRunnable {
             return;
         }
 
-        PlayerProfile profile = mcMMO.getDatabaseManager().loadPlayerProfile(player.getName(), player.getUniqueId(), true);
+        PlayerProfile profile = mcMMO.getDatabaseManager().loadPlayerProfile(player.getUniqueId(), player.getName());
+
+        if(!profile.isLoaded()) {
+            mcMMO.p.getLogger().info("Creating new data for player: "+player.getName());
+            //Profile isn't loaded so add as new user
+            profile = mcMMO.getDatabaseManager().newUser(player);
+        }
 
         // If successful, schedule the apply
         if (profile.isLoaded()) {