Various touchups & fixes

Removed unused imports
Added an alias to flatfile in /mmoupdate
Removed vestiges of /mmoupdate confirm system
Commented PURGE_TIME
Add getSkillMapFromLine for powerless purge & flatfile leaderboards
Add tryClose() for non-repetition glory
Synchronize access to the flatfile database, to avoid data loss
Add @SuppressWarnings to PlayerUpdateInventoryTask
Use getGeneratedKeys() to avoid an extra query on player creation
Change default of readId to -1
SQL: saveUser calls newUser() on id read failure
SQL: fix loadPlayerProfile behavior on not-found player names and orphaned data rows

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

@@ -10,7 +10,6 @@ import org.bukkit.command.TabExecutor;
 import org.bukkit.util.StringUtil;
 
 import com.gmail.nossr50.mcMMO;
-import com.gmail.nossr50.datatypes.player.PlayerProfile;
 import com.gmail.nossr50.locale.LocaleLoader;
 import com.gmail.nossr50.util.commands.CommandUtils;
 import com.gmail.nossr50.util.player.UserManager;

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

@@ -1,9 +1,9 @@
 package com.gmail.nossr50.commands.database;
 
 import java.util.List;
+
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandSender;
-import org.bukkit.command.ConsoleCommandSender;
 import org.bukkit.command.TabExecutor;
 import org.bukkit.entity.Player;
 
@@ -81,11 +81,6 @@ public class MmoupdateCommand implements TabExecutor {
             default:
                 break;
         }
-        // If you don't do your research, you get lied to
-        if (!(sender instanceof ConsoleCommandSender)) {
-            sender.sendMessage(LocaleLoader.getString("Commands.mmoupdate.OpOnly"));
-            return true;
-        }
         return false;
     }
 
@@ -99,7 +94,7 @@ public class MmoupdateCommand implements TabExecutor {
             return "sql";
         }
 
-        if (type.equalsIgnoreCase("flatfile")) {
+        if (type.equalsIgnoreCase("flatfile") || type.equalsIgnoreCase("file")) {
             return "flatfile";
         }
 
@@ -134,15 +129,10 @@ public class MmoupdateCommand implements TabExecutor {
 
     @Override
     public List<String> onTabComplete(CommandSender sender, Command command, String alias, String[] args) {
-        if (sender instanceof Player) {
-            // What, you're trying to run this on a live server? No autocomplete for you! Do your research first!
-            return ImmutableList.of();
-        }
-
         Class<?> clazz = DatabaseManagerFactory.getCustomDatabaseManagerClass();
         if (clazz != null) {
-            return ImmutableList.of("confirm", "flatfile", "sql", clazz.getName());
+            return ImmutableList.of("flatfile", "sql", clazz.getName());
         }
-        return ImmutableList.of("confirm", "flatfile", "sql");
+        return ImmutableList.of("flatfile", "sql");
     }
 }

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

@@ -5,7 +5,6 @@ import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
 
 import com.gmail.nossr50.mcMMO;
-import com.gmail.nossr50.datatypes.player.PlayerProfile;
 import com.gmail.nossr50.datatypes.skills.SkillType;
 import com.gmail.nossr50.events.experience.McMMOPlayerLevelUpEvent;
 import com.gmail.nossr50.locale.LocaleLoader;

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

@@ -13,7 +13,6 @@ import org.bukkit.util.StringUtil;
 import com.gmail.nossr50.mcMMO;
 import com.gmail.nossr50.config.Config;
 import com.gmail.nossr50.datatypes.player.McMMOPlayer;
-import com.gmail.nossr50.datatypes.player.PlayerProfile;
 import com.gmail.nossr50.runnables.commands.McrankCommandAsyncTask;
 import com.gmail.nossr50.util.Permissions;
 import com.gmail.nossr50.util.commands.CommandUtils;

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

@@ -8,6 +8,7 @@ import com.gmail.nossr50.datatypes.database.PlayerStat;
 import com.gmail.nossr50.datatypes.player.PlayerProfile;
 
 public interface DatabaseManager {
+    // One month in milliseconds
     public final long PURGE_TIME = 2630000000L * Config.getInstance().getOldUsersCutoff();
 
     /**

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

@@ -2,6 +2,7 @@ package com.gmail.nossr50.database;
 
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
+import java.io.Closeable;
 import java.io.File;
 import java.io.FileReader;
 import java.io.FileWriter;
@@ -59,17 +60,11 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
 
                 while ((line = in.readLine()) != null) {
                     String[] character = line.split(":");
-                    PlayerProfile profile = null;
+                    Map<SkillType, Integer> skills = getSkillMapFromLine(character);
-                    try {
-                        profile = loadFromLine(character);
-                    } catch (Exception e) {
-                        e.printStackTrace();
-                    }
-                    if (profile == null) continue; // skip malformed lines
 
                     boolean powerless = true;
-                    for (SkillType skill : SkillType.nonChildSkills()) {
+                    for (int skill : skills.values()) {
-                        if (profile.getSkillLevel(skill) != 0) {
+                        if (skill != 0) {
                             powerless = false;
                             break;
                         }
@@ -93,23 +88,8 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
                 mcMMO.p.getLogger().severe("Exception while reading " + usersFilePath + " (Are you sure you formatted it correctly?)" + e.toString());
             }
             finally {
-                if (in != null) {
+                tryClose(in);
-                    try {
+                tryClose(out);
-                        in.close();
-                    }
-                    catch (IOException ex) {
-                        ex.printStackTrace();
-                    }
-                }
-
-                if (out != null) {
-                    try {
-                        out.close();
-                    }
-                    catch (IOException ex) {
-                        ex.printStackTrace();
-                    }
-                }
             }
         }
 
@@ -144,7 +124,7 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
                     }
 
                     if (!old) {
-                        writer.append(line);
+                        writer.append(line).append("\r\n");
                     }
                     else {
                         removedPlayers++;
@@ -160,23 +140,8 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
                 mcMMO.p.getLogger().severe("Exception while reading " + usersFilePath + " (Are you sure you formatted it correctly?)" + e.toString());
             }
             finally {
-                if (in != null) {
+                tryClose(in);
-                    try {
+                tryClose(out);
-                        in.close();
-                    }
-                    catch (IOException ex) {
-                        ex.printStackTrace();
-                    }
-                }
-
-                if (out != null) {
-                    try {
-                        out.close();
-                    }
-                    catch (IOException ex) {
-                        ex.printStackTrace();
-                    }
-                }
             }
         }
 
@@ -214,23 +179,8 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
                 mcMMO.p.getLogger().severe("Exception while reading " + usersFilePath + " (Are you sure you formatted it correctly?)" + e.toString());
             }
             finally {
-                if (in != null) {
+                tryClose(in);
-                    try {
+                tryClose(out);
-                        in.close();
-                    }
-                    catch (IOException ex) {
-                        ex.printStackTrace();
-                    }
-                }
-
-                if (out != null) {
-                    try {
-                        out.close();
-                    }
-                    catch (IOException ex) {
-                        ex.printStackTrace();
-                    }
-                }
             }
         }
 
@@ -314,23 +264,8 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
                 e.printStackTrace();
             }
             finally {
-                if (in != null) {
+                tryClose(in);
-                    try {
+                tryClose(out);
-                        in.close();
-                    }
-                    catch (IOException ex) {
-                        ex.printStackTrace();
-                    }
-                }
-
-                if (out != null) {
-                    try {
-                        out.close();
-                    }
-                    catch (IOException ex) {
-                        ex.printStackTrace();
-                    }
-                }
             }
         }
     }
@@ -358,10 +293,11 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
     }
 
     public void newUser(String playerName) {
+        BufferedWriter out = null;
         synchronized (fileWritingLock) {
             try {
                 // Open the file to write the player
-                BufferedWriter out = new BufferedWriter(new FileWriter(mcMMO.getUsersFilePath(), true));
+                out = new BufferedWriter(new FileWriter(mcMMO.getUsersFilePath(), true));
 
                 // Add the player to the end
                 out.append(playerName).append(":");
@@ -407,22 +343,24 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
                 // Add more in the same format as the line above
 
                 out.newLine();
-                out.close();
             }
             catch (Exception e) {
                 e.printStackTrace();
             }
+            finally {
+                tryClose(out);
+            }
         }
     }
 
     public PlayerProfile loadPlayerProfile(String playerName, boolean create) {
-        FileReader file = null;
         BufferedReader in = null;
+        String usersFilePath = mcMMO.getUsersFilePath();
+
         synchronized (fileWritingLock) {
             try {
                 // Open the user file
-                file = new FileReader(mcMMO.getUsersFilePath());
+                in = new BufferedReader(new FileReader(usersFilePath));
-                in = new BufferedReader(file);
                 String line;
 
                 while ((line = in.readLine()) != null) {
@@ -433,23 +371,16 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
                         continue;
                     }
 
-                    return loadFromLine(character);
+                    PlayerProfile p = loadFromLine(character);
+                    in.close();
+                    return p;
                 }
             }
             catch (Exception e) {
                 e.printStackTrace();
             }
             finally {
-                try {
+                tryClose(in);
-                    if (in != null) {
-                        in.close();
-                    }
-                    if (file != null) {
-                        file.close();
-                    }
-                } catch (IOException e) {
-                    e.printStackTrace();
-                }
             }
         }
 
@@ -461,14 +392,13 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
     }
 
     public void convertUsers(DatabaseManager destination) {
-        FileReader file = null;
         BufferedReader in = null;
+        String usersFilePath = mcMMO.getUsersFilePath();
 
         synchronized (fileWritingLock) {
             try {
                 // Open the user file
-                file = new FileReader(mcMMO.getUsersFilePath());
+                in = new BufferedReader(new FileReader(usersFilePath));
-                in = new BufferedReader(file);
                 String line;
 
                 while ((line = in.readLine()) != null) {
@@ -486,16 +416,7 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
                 e.printStackTrace();
             }
             finally {
-                try {
+                tryClose(in);
-                    if (in != null) {
-                        in.close();
-                    }
-                    if (file != null) {
-                        file.close();
-                    }
-                } catch (IOException e) {
-                    e.printStackTrace();
-                }
             }
         }
     }
@@ -508,12 +429,12 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
     public List<String> getStoredUsers() {
         ArrayList<String> users = new ArrayList<String>();
         BufferedReader in = null;
+        String usersFilePath = mcMMO.getUsersFilePath();
 
         synchronized (fileWritingLock) {
             try {
                 // Open the user file
-                FileReader file = new FileReader(mcMMO.getUsersFilePath());
+                in = new BufferedReader(new FileReader(usersFilePath));
-                in = new BufferedReader(file);
                 String line;
 
                 while ((line = in.readLine()) != null) {
@@ -525,11 +446,7 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
                 e.printStackTrace();
             }
             finally {
-                try {
+                tryClose(in);
-                    in.close();
-                } catch (IOException e) {
-                    e.printStackTrace();
-                }
             }
         }
         return users;
@@ -562,43 +479,50 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
         List<PlayerStat> taming = new ArrayList<PlayerStat>();
         List<PlayerStat> fishing = new ArrayList<PlayerStat>();
 
+        BufferedReader in = null;
         // Read from the FlatFile database and fill our arrays with information
-        try {
+        synchronized (fileWritingLock) {
-            BufferedReader in = new BufferedReader(new FileReader(usersFilePath));
+            try {
-            String line = "";
+                in = new BufferedReader(new FileReader(usersFilePath));
-            ArrayList<String> players = new ArrayList<String>();
+                String line = "";
+                ArrayList<String> players = new ArrayList<String>();
 
-            while ((line = in.readLine()) != null) {
+                while ((line = in.readLine()) != null) {
-                String[] data = line.split(":");
+                    String[] data = line.split(":");
-                String playerName = data[0];
+                    String playerName = data[0];
-                int powerLevel = 0;
+                    int powerLevel = 0;
 
-                // Prevent the same player from being added multiple times (I'd like to note that this shouldn't happen...)
+                    // Prevent the same player from being added multiple times (I'd like to note that this shouldn't happen...)
-                if (players.contains(playerName)) {
+                    if (players.contains(playerName)) {
-                    continue;
+                        continue;
+                    }
+
+                    players.add(playerName);
+
+                    Map<SkillType, Integer> skills = getSkillMapFromLine(data);
+
+                    powerLevel += putStat(acrobatics, playerName, skills.get(SkillType.ACROBATICS));
+                    powerLevel += putStat(archery, playerName, skills.get(SkillType.ARCHERY));
+                    powerLevel += putStat(axes, playerName, skills.get(SkillType.AXES));
+                    powerLevel += putStat(excavation, playerName, skills.get(SkillType.EXCAVATION));
+                    powerLevel += putStat(fishing, playerName, skills.get(SkillType.FISHING));
+                    powerLevel += putStat(herbalism, playerName, skills.get(SkillType.HERBALISM));
+                    powerLevel += putStat(mining, playerName, skills.get(SkillType.MINING));
+                    powerLevel += putStat(repair, playerName, skills.get(SkillType.REPAIR));
+                    powerLevel += putStat(swords, playerName, skills.get(SkillType.SWORDS));
+                    powerLevel += putStat(taming, playerName, skills.get(SkillType.TAMING));
+                    powerLevel += putStat(unarmed, playerName, skills.get(SkillType.UNARMED));
+                    powerLevel += putStat(woodcutting, playerName, skills.get(SkillType.WOODCUTTING));
+
+                    putStat(powerLevels, playerName, powerLevel);
                 }
-
-                players.add(playerName);
-
-                powerLevel += loadStat(mining, playerName, data, 1);
-                powerLevel += loadStat(woodcutting, playerName, data, 5);
-                powerLevel += loadStat(repair, playerName, data, 7);
-                powerLevel += loadStat(unarmed, playerName, data, 8);
-                powerLevel += loadStat(herbalism, playerName, data, 9);
-                powerLevel += loadStat(excavation, playerName, data, 10);
-                powerLevel += loadStat(archery, playerName, data, 11);
-                powerLevel += loadStat(swords, playerName, data, 12);
-                powerLevel += loadStat(axes, playerName, data, 13);
-                powerLevel += loadStat(acrobatics, playerName, data, 14);
-                powerLevel += loadStat(taming, playerName, data, 24);
-                powerLevel += loadStat(fishing, playerName, data, 34);
-
-                powerLevels.add(new PlayerStat(playerName, powerLevel));
             }
-            in.close();
+            catch (Exception e) {
-        }
+                mcMMO.p.getLogger().severe("Exception while reading " + usersFilePath + " (Are you sure you formatted it correctly?)" + e.toString());
-        catch (Exception e) {
+            }
-            mcMMO.p.getLogger().severe("Exception while reading " + usersFilePath + " (Are you sure you formatted it correctly?)" + e.toString());
+            finally {
+                tryClose(in);
+            }
         }
 
         SkillComparator c = new SkillComparator();
@@ -647,6 +571,15 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
         }
     }
 
+    private void tryClose(Closeable c) {
+        if (c == null) return;
+        try {
+            c.close();
+        } catch (IOException e) {
+            e.printStackTrace();
+        }
+    }
+
     private Integer getPlayerRank(String playerName, List<PlayerStat> statsList) {
         if (statsList == null) {
             return null;
@@ -665,14 +598,8 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
         return null;
     }
 
-    private int loadStat(List<PlayerStat> statList, String playerName, String[] data, int dataIndex) {
+    private int putStat(List<PlayerStat> statList, String playerName, int statValue) {
-        if (data.length <= dataIndex) {
-            return 0;
-        }
-
-        int statValue = Integer.parseInt(data[dataIndex]);
         statList.add(new PlayerStat(playerName, statValue));
-
         return statValue;
     }
 
@@ -684,24 +611,13 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
     }
 
     private PlayerProfile loadFromLine(String[] character) throws Exception {
-        Map<SkillType, Integer>   skills     = new HashMap<SkillType, Integer>();   // Skill & Level
+        Map<SkillType, Integer>   skills     = getSkillMapFromLine(character);      // Skill levels
         Map<SkillType, Float>     skillsXp   = new HashMap<SkillType, Float>();     // Skill & XP
         Map<AbilityType, Integer> skillsDATS = new HashMap<AbilityType, Integer>(); // Ability & Cooldown
         HudType hudType;
         MobHealthbarType mobHealthbarType;
 
-        skills.put(SkillType.TAMING, Integer.valueOf(character[24]));
+        // TODO on updates, put new values in a try{} ?
-        skills.put(SkillType.MINING, Integer.valueOf(character[1]));
-        skills.put(SkillType.REPAIR, Integer.valueOf(character[7]));
-        skills.put(SkillType.WOODCUTTING, Integer.valueOf(character[5]));
-        skills.put(SkillType.UNARMED, Integer.valueOf(character[8]));
-        skills.put(SkillType.HERBALISM, Integer.valueOf(character[9]));
-        skills.put(SkillType.EXCAVATION, Integer.valueOf(character[10]));
-        skills.put(SkillType.ARCHERY, Integer.valueOf(character[11]));
-        skills.put(SkillType.SWORDS, Integer.valueOf(character[12]));
-        skills.put(SkillType.AXES, Integer.valueOf(character[13]));
-        skills.put(SkillType.ACROBATICS, Integer.valueOf(character[14]));
-        skills.put(SkillType.FISHING, Integer.valueOf(character[34]));
 
         skillsXp.put(SkillType.TAMING, (float) Integer.valueOf(character[25]));
         skillsXp.put(SkillType.MINING, (float) Integer.valueOf(character[4]));
@@ -745,4 +661,23 @@ public final class FlatfileDatabaseManager implements DatabaseManager {
 
         return new PlayerProfile(character[0], skills, skillsXp, skillsDATS, hudType, mobHealthbarType);
     }
+
+    private Map<SkillType, Integer> getSkillMapFromLine(String[] character) {
+        Map<SkillType, Integer> skills = new HashMap<SkillType, Integer>();   // Skill & Level
+
+        skills.put(SkillType.TAMING, Integer.valueOf(character[24]));
+        skills.put(SkillType.MINING, Integer.valueOf(character[1]));
+        skills.put(SkillType.REPAIR, Integer.valueOf(character[7]));
+        skills.put(SkillType.WOODCUTTING, Integer.valueOf(character[5]));
+        skills.put(SkillType.UNARMED, Integer.valueOf(character[8]));
+        skills.put(SkillType.HERBALISM, Integer.valueOf(character[9]));
+        skills.put(SkillType.EXCAVATION, Integer.valueOf(character[10]));
+        skills.put(SkillType.ARCHERY, Integer.valueOf(character[11]));
+        skills.put(SkillType.SWORDS, Integer.valueOf(character[12]));
+        skills.put(SkillType.AXES, Integer.valueOf(character[13]));
+        skills.put(SkillType.ACROBATICS, Integer.valueOf(character[14]));
+        skills.put(SkillType.FISHING, Integer.valueOf(character[34]));
+
+        return skills;
+    }
 }

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

@@ -12,6 +12,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.logging.Level;
 
 import com.gmail.nossr50.mcMMO;
 import com.gmail.nossr50.config.Config;
@@ -102,7 +103,16 @@ public final class SQLDatabaseManager implements DatabaseManager {
     }
 
     public void saveUser(PlayerProfile profile) {
+        checkConnected();
         int userId = readId(profile.getPlayerName());
+        if (userId == -1) {
+            newUser(profile.getPlayerName());
+            userId = readId(profile.getPlayerName());
+            if (userId == -1) {
+                mcMMO.p.getLogger().log(Level.WARNING, "Failed to save user " + profile.getPlayerName());
+                return;
+            }
+        }
         MobHealthbarType mobHealthbarType = profile.getMobHealthbarType();
         HudType hudType = profile.getHudType();
 
@@ -297,7 +307,8 @@ public final class SQLDatabaseManager implements DatabaseManager {
             statement.setLong(2, System.currentTimeMillis() / Misc.TIME_CONVERSION_FACTOR);
             statement.execute();
 
-            writeMissingRows(readId(playerName));
+            int id = statement.getGeneratedKeys().getInt("id");
+            writeMissingRows(id);
         }
         catch (SQLException ex) {
             printErrors(ex);
@@ -335,36 +346,34 @@ public final class SQLDatabaseManager implements DatabaseManager {
             ResultSet result = statement.executeQuery();
 
             if (result.next()) {
-                PlayerProfile ret = loadFromResult(playerName, result);
+                try {
-                result.close();
+                    PlayerProfile ret = loadFromResult(playerName, result);
-                return ret;
+                    result.close();
-            }
+                    return ret;
-            else {
-                // Problem, no rows returned
-                int userId = readId(playerName);
-
-                if (userId == 0) {
-                    if (!create) {
-                        // Give up
-                        return new PlayerProfile(playerName, false);
-                    }
-                    else {
-                        newUser(playerName);
-                        userId = readId(playerName);
-                    }
                 }
+                catch (SQLException e) {}
+            }
+            // Problem, no rows returned
+            result.close();
 
-                writeMissingRows(userId);
+            // First, read User Id - this is to check for orphans
 
-                if (!create) {
+            int id = readId(playerName);
-                    // Give up
+
-                    return new PlayerProfile(playerName, false);
+            if (id == -1) {
+                // There is no such user
+                if (create) {
+                    newUser(playerName);
+                    return new PlayerProfile(playerName, true);
                 }
                 else {
-                    // Re-read data
+                    return new PlayerProfile(playerName, false);
-                    return loadPlayerProfile(playerName, false);
                 }
             }
+            // There is such a user
+            writeMissingRows(id);
+            // Retry, and abort on re-failure
+            return loadPlayerProfile(playerName, false);
         }
         catch (SQLException ex) {
             printErrors(ex);
@@ -379,7 +388,14 @@ public final class SQLDatabaseManager implements DatabaseManager {
                 }
             }
         }
-        return new PlayerProfile(playerName, false);
+
+        if (!create) {
+            return new PlayerProfile(playerName, false);
+        }
+
+        newUser(playerName);
+
+        return new PlayerProfile(playerName, true);
     }
 
     public void convertUsers(DatabaseManager destination) {
@@ -1034,7 +1050,7 @@ public final class SQLDatabaseManager implements DatabaseManager {
     }
 
     private int readId(String playerName) {
-        int id = 0;
+        int id = -1;
 
         try {
             PreparedStatement statement = connection.prepareStatement("SELECT id FROM " + tablePrefix + "users WHERE user = ?");

src/main/java/com/gmail/nossr50/datatypes/player/PlayerProfile.java

@@ -1,7 +1,6 @@
 package com.gmail.nossr50.datatypes.player;
 
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 

src/main/java/com/gmail/nossr50/runnables/PlayerUpdateInventoryTask.java

@@ -3,6 +3,7 @@ package com.gmail.nossr50.runnables;
 import org.bukkit.entity.Player;
 import org.bukkit.scheduler.BukkitRunnable;
 
+@SuppressWarnings("deprecation")
 public class PlayerUpdateInventoryTask extends BukkitRunnable {
     private Player player;