From 9b856f456c00b320e40b7cfd0d9847da8fe9957e Mon Sep 17 00:00:00 2001 From: nossr50 Date: Fri, 19 Mar 2021 13:31:18 -0700 Subject: [PATCH] Move save operation handling to its own class --- .../commands/experience/AddlevelsCommand.java | 3 +- .../commands/experience/AddxpCommand.java | 2 +- .../commands/experience/MmoeditCommand.java | 2 +- .../experience/SkillresetCommand.java | 2 +- .../nossr50/database/DatabaseManager.java | 8 ++ .../database/FlatFileDatabaseManager.java | 11 ++- .../datatypes/player/MMODataSnapshot.java | 16 ++++ .../nossr50/runnables/SaveTimerTask.java | 6 +- .../database/FormulaConversionTask.java | 5 +- .../util/player/PlayerSaveHandler.java | 81 +++++++++++++++++++ .../nossr50/util/player/UserManager.java | 27 ++++--- 11 files changed, 137 insertions(+), 26 deletions(-) create mode 100644 src/main/java/com/gmail/nossr50/util/player/PlayerSaveHandler.java diff --git a/src/main/java/com/gmail/nossr50/commands/experience/AddlevelsCommand.java b/src/main/java/com/gmail/nossr50/commands/experience/AddlevelsCommand.java index af3b3366c..b0410a3dd 100644 --- a/src/main/java/com/gmail/nossr50/commands/experience/AddlevelsCommand.java +++ b/src/main/java/com/gmail/nossr50/commands/experience/AddlevelsCommand.java @@ -1,6 +1,5 @@ package com.gmail.nossr50.commands.experience; -import com.gmail.nossr50.datatypes.experience.XPGainReason; import com.gmail.nossr50.datatypes.player.McMMOPlayer; import com.gmail.nossr50.datatypes.player.PlayerProfile; import com.gmail.nossr50.datatypes.skills.PrimarySkillType; @@ -28,7 +27,7 @@ public class AddlevelsCommand extends ExperienceCommand { profile.addLevels(skill, value); if (player == null) { - profile.scheduleAsyncSave(); + UserManager.getPlayerSaveHandler().scheduleAsyncSave(profile.getPlayerData()); return; } diff --git a/src/main/java/com/gmail/nossr50/commands/experience/AddxpCommand.java b/src/main/java/com/gmail/nossr50/commands/experience/AddxpCommand.java index 03ad449bb..5110accd9 100644 --- a/src/main/java/com/gmail/nossr50/commands/experience/AddxpCommand.java +++ b/src/main/java/com/gmail/nossr50/commands/experience/AddxpCommand.java @@ -32,7 +32,7 @@ public class AddxpCommand extends ExperienceCommand { } else { profile.addXp(skill, value); - profile.scheduleAsyncSave(); + UserManager.getPlayerSaveHandler().scheduleAsyncSave(profile.getPlayerData()); } } diff --git a/src/main/java/com/gmail/nossr50/commands/experience/MmoeditCommand.java b/src/main/java/com/gmail/nossr50/commands/experience/MmoeditCommand.java index ac8405525..801bc3b03 100644 --- a/src/main/java/com/gmail/nossr50/commands/experience/MmoeditCommand.java +++ b/src/main/java/com/gmail/nossr50/commands/experience/MmoeditCommand.java @@ -30,7 +30,7 @@ public class MmoeditCommand extends ExperienceCommand { profile.modifySkill(skill, value); if (player == null) { - profile.scheduleAsyncSave(); + UserManager.getPlayerSaveHandler().scheduleAsyncSave(profile.getPlayerData()); return; } diff --git a/src/main/java/com/gmail/nossr50/commands/experience/SkillresetCommand.java b/src/main/java/com/gmail/nossr50/commands/experience/SkillresetCommand.java index 9b97fa77a..e73d96183 100644 --- a/src/main/java/com/gmail/nossr50/commands/experience/SkillresetCommand.java +++ b/src/main/java/com/gmail/nossr50/commands/experience/SkillresetCommand.java @@ -129,7 +129,7 @@ public class SkillresetCommand implements TabExecutor { profile.modifySkill(skill, 0); if (player == null) { - profile.scheduleAsyncSave(); + UserManager.getPlayerSaveHandler().scheduleAsyncSave(profile.getPlayerData()); return; } diff --git a/src/main/java/com/gmail/nossr50/database/DatabaseManager.java b/src/main/java/com/gmail/nossr50/database/DatabaseManager.java index 6fc76bd9a..72253d658 100644 --- a/src/main/java/com/gmail/nossr50/database/DatabaseManager.java +++ b/src/main/java/com/gmail/nossr50/database/DatabaseManager.java @@ -4,6 +4,7 @@ import com.gmail.nossr50.api.exceptions.InvalidSkillException; import com.gmail.nossr50.config.Config; import com.gmail.nossr50.datatypes.database.DatabaseType; import com.gmail.nossr50.datatypes.database.PlayerStat; +import com.gmail.nossr50.datatypes.player.MMODataSnapshot; import com.gmail.nossr50.datatypes.player.PlayerData; import com.gmail.nossr50.datatypes.player.PlayerProfile; import com.gmail.nossr50.datatypes.skills.PrimarySkillType; @@ -55,6 +56,13 @@ public interface DatabaseManager { */ boolean saveUser(@NotNull PlayerData playerData); + /** + * + * @param dataSnapshot target data snapshot + * @return true if successful, false on failure + */ + boolean saveUser(@NotNull MMODataSnapshot dataSnapshot); + /** * Retrieve leaderboard info. * Will never be null but it may be empty diff --git a/src/main/java/com/gmail/nossr50/database/FlatFileDatabaseManager.java b/src/main/java/com/gmail/nossr50/database/FlatFileDatabaseManager.java index 136972312..9b2dc42f3 100644 --- a/src/main/java/com/gmail/nossr50/database/FlatFileDatabaseManager.java +++ b/src/main/java/com/gmail/nossr50/database/FlatFileDatabaseManager.java @@ -258,9 +258,12 @@ public final class FlatFileDatabaseManager implements DatabaseManager { //Not used in FlatFile } - public boolean saveUser(@NotNull PlayerData playerData) { - MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData); //Clone data into Immutable data + MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData); + return saveUser(mmoDataSnapshot); //Clone data into Immutable data + } + + public boolean saveUser(@NotNull MMODataSnapshot mmoDataSnapshot) { String playerName = mmoDataSnapshot.getPlayerName(); UUID uuid = mmoDataSnapshot.getPlayerUUID(); @@ -305,8 +308,8 @@ public final class FlatFileDatabaseManager implements DatabaseManager { } if (!(uuid != null - && splitData[FlatFileMappings.UUID_INDEX].equalsIgnoreCase(uuid.toString())) - && !splitData[FlatFileMappings.USERNAME].equalsIgnoreCase(playerName)) { + && splitData[FlatFileMappings.UUID_INDEX].equalsIgnoreCase(uuid.toString())) + && !splitData[FlatFileMappings.USERNAME].equalsIgnoreCase(playerName)) { writer.append(line).append("\r\n"); //Not the user so write it to file and move on } else { //User found diff --git a/src/main/java/com/gmail/nossr50/datatypes/player/MMODataSnapshot.java b/src/main/java/com/gmail/nossr50/datatypes/player/MMODataSnapshot.java index 820a3079e..c7f2c85de 100644 --- a/src/main/java/com/gmail/nossr50/datatypes/player/MMODataSnapshot.java +++ b/src/main/java/com/gmail/nossr50/datatypes/player/MMODataSnapshot.java @@ -10,6 +10,9 @@ import org.jetbrains.annotations.NotNull; import java.util.UUID; public class MMODataSnapshot { + /* Save Attempts */ + private int saveAttempts = 0; + /* Player Stuff */ private final @NotNull String playerName; private final @NotNull UUID playerUUID; @@ -109,4 +112,17 @@ public class MMODataSnapshot { public boolean isLeaderBoardExcluded() { return leaderBoardExclusion; } + + public int getSaveAttempts() { + return saveAttempts; + } + + public void setSaveAttempts(int saveAttempts) { + this.saveAttempts = saveAttempts; + } + + public void incrementSaveAttempts() { + this.saveAttempts += 1; + } + } diff --git a/src/main/java/com/gmail/nossr50/runnables/SaveTimerTask.java b/src/main/java/com/gmail/nossr50/runnables/SaveTimerTask.java index 33a2c9d46..2ebe81bec 100644 --- a/src/main/java/com/gmail/nossr50/runnables/SaveTimerTask.java +++ b/src/main/java/com/gmail/nossr50/runnables/SaveTimerTask.java @@ -1,6 +1,8 @@ package com.gmail.nossr50.runnables; +import com.gmail.nossr50.datatypes.player.McMMOPlayer; import com.gmail.nossr50.mcMMO; +import com.gmail.nossr50.util.player.UserManager; import com.neetgames.mcmmo.player.OnlineMMOPlayer; import org.bukkit.scheduler.BukkitRunnable; @@ -12,8 +14,8 @@ public class SaveTimerTask extends BukkitRunnable { int count = 1; //TODO: write a more efficient bulk save - for (OnlineMMOPlayer mmoPlayer : UserManager.getPlayers()) { - UserManager.saveUserWithDelay(mmoPlayer.getPersistentPlayerData(), false, count); + for (McMMOPlayer mmoPlayer : UserManager.getPlayers()) { + UserManager.getPlayerSaveHandler().scheduleAsyncSaveDelay(mmoPlayer.getPlayerData()); count++; } diff --git a/src/main/java/com/gmail/nossr50/runnables/database/FormulaConversionTask.java b/src/main/java/com/gmail/nossr50/runnables/database/FormulaConversionTask.java index 5447feb64..337999308 100644 --- a/src/main/java/com/gmail/nossr50/runnables/database/FormulaConversionTask.java +++ b/src/main/java/com/gmail/nossr50/runnables/database/FormulaConversionTask.java @@ -41,9 +41,8 @@ public class FormulaConversionTask extends BukkitRunnable { editValues(profile); // Since this is a temporary profile, we save it here. - profile.scheduleAsyncSave(); - } - else { + UserManager.getPlayerSaveHandler().scheduleAsyncSave(profile.getPlayerData()); + } else { profile = mcMMOPlayer.getProfile(); editValues(profile); } diff --git a/src/main/java/com/gmail/nossr50/util/player/PlayerSaveHandler.java b/src/main/java/com/gmail/nossr50/util/player/PlayerSaveHandler.java new file mode 100644 index 000000000..ed307bec0 --- /dev/null +++ b/src/main/java/com/gmail/nossr50/util/player/PlayerSaveHandler.java @@ -0,0 +1,81 @@ +package com.gmail.nossr50.util.player; + +import com.gmail.nossr50.datatypes.player.MMODataSnapshot; +import com.gmail.nossr50.datatypes.player.PlayerData; +import com.gmail.nossr50.mcMMO; +import org.bukkit.Bukkit; +import org.jetbrains.annotations.NotNull; + +//TODO: Low priority - Track pending Async saves to avoid data loss during server shutdown +//TODO: T&C Javadocs +public class PlayerSaveHandler { + + private void save(@NotNull MMODataSnapshot mmoDataSnapshot, boolean useSync) { + boolean saveSuccessful = mcMMO.getDatabaseManager().saveUser(mmoDataSnapshot); + + //Check for failure to save + if (!saveSuccessful) { + String playerName = mmoDataSnapshot.getPlayerName(); + String uuidStr = mmoDataSnapshot.getPlayerUUID().toString(); + mcMMO.p.getLogger().severe("PlayerProfile saving failed for player name: " + playerName + " UUID: " + uuidStr); + + if(mmoDataSnapshot.getSaveAttempts() > 0) { + mcMMO.p.getLogger().severe("Attempted to save profile for player "+playerName + + " resulted in failure. " + mmoDataSnapshot.getSaveAttempts() + " have been made so far."); + } + + if(mmoDataSnapshot.getSaveAttempts() < 10) { + mmoDataSnapshot.incrementSaveAttempts(); + + //Back out of async saving if we detect a server shutdown, this is not always going to be caught + if(mcMMO.isServerShutdownExecuted() || useSync) + scheduleSyncSave(mmoDataSnapshot); //Execute sync saves immediately + else + scheduleAsyncSave(mmoDataSnapshot); + + } else { + mcMMO.p.getLogger().severe("mcMMO has failed to save the profile for " + + playerName + " numerous times." + + " mcMMO will now stop attempting to save this profile." + + " Check your console for errors and inspect your DB for issues."); + } + } + } + + public void save(@NotNull PlayerData playerData, boolean useSync) { + //TODO: We no longer check if a profile is loaded or not as it should never be unloaded if a save operation is being called, need to double check this to be true + if(!playerData.isDirtyProfile()) { + return; //Don't save data that hasn't changed + } + + MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData); + save(mmoDataSnapshot, useSync); + } + + public void scheduleAsyncSave(@NotNull PlayerData playerData) { + MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData); + scheduleAsyncSave(mmoDataSnapshot); + } + + public void scheduleAsyncSaveDelay(@NotNull PlayerData playerData) { + MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData); + scheduleAsyncSaveDelay(mmoDataSnapshot); + } + + public void scheduleSyncSave(@NotNull PlayerData playerData) { + MMODataSnapshot mmoDataSnapshot = new MMODataSnapshot(playerData); + scheduleSyncSave(mmoDataSnapshot); + } + + private void scheduleAsyncSave(@NotNull MMODataSnapshot mmoDataSnapshot) { + Bukkit.getScheduler().runTaskAsynchronously(mcMMO.p, () -> save(mmoDataSnapshot, false)); + } + + private void scheduleAsyncSaveDelay(@NotNull MMODataSnapshot mmoDataSnapshot) { + Bukkit.getScheduler().runTaskLaterAsynchronously(mcMMO.p, () -> save(mmoDataSnapshot, false), 20L); + } + + private void scheduleSyncSave(@NotNull MMODataSnapshot mmoDataSnapshot) { + Bukkit.getScheduler().runTask(mcMMO.p, () -> save(mmoDataSnapshot, true)); + } +} diff --git a/src/main/java/com/gmail/nossr50/util/player/UserManager.java b/src/main/java/com/gmail/nossr50/util/player/UserManager.java index 8de5fcddf..6082f541e 100644 --- a/src/main/java/com/gmail/nossr50/util/player/UserManager.java +++ b/src/main/java/com/gmail/nossr50/util/player/UserManager.java @@ -19,6 +19,7 @@ import java.util.HashSet; public final class UserManager { private static HashSet playerDataSet; //Used to track players for sync saves on shutdown + private @NotNull static final PlayerSaveHandler playerSaveHandler = new PlayerSaveHandler(); private UserManager() {} @@ -69,7 +70,7 @@ public final class UserManager { } /** - * Save all users ON THIS THREAD. + * Save all users on main thread */ public static void saveAll() { if(playerDataSet == null) @@ -79,15 +80,12 @@ public final class UserManager { mcMMO.p.getLogger().info("Saving mcMMOPlayers... (" + trackedSyncData.size() + ")"); - for (McMMOPlayer playerData : trackedSyncData) { - try - { - mcMMO.p.getLogger().info("Saving data for player: "+playerData.getPlayerName()); - playerData.getProfile().save(true); - } - catch (Exception e) - { - mcMMO.p.getLogger().warning("Could not save mcMMO player data for player: " + playerData.getPlayerName()); + for (McMMOPlayer mmoPlayer : trackedSyncData) { + try { + mcMMO.p.getLogger().info("Saving data for player: "+mmoPlayer.getPlayerName()); + getPlayerSaveHandler().save(mmoPlayer.getPlayerData(), true); + } catch (Exception e) { + mcMMO.p.getLogger().severe("Could not save mcMMO player data for player: " + mmoPlayer.getPlayerName()); } } @@ -120,9 +118,9 @@ public final class UserManager { mmoPlayer.getTamingManager().cleanupAllSummons(); if (syncSave) { - getProfile().save(true); //TODO: T&C Wire this up, see master branch com.gmail.nossr50.datatypes.player.PlayerProfile#save + getPlayerSaveHandler().save(mmoPlayer.getPlayerData(), true); //TODO: T&C Wire this up, see master branch com.gmail.nossr50.datatypes.player.PlayerProfile#save } else { - getProfile().scheduleAsyncSave(); //TODO: T&C Wire this up, see master branch com.gmail.nossr50.datatypes.player.PlayerProfile#scheduleAsyncSave + getPlayerSaveHandler().scheduleAsyncSave(mmoPlayer.getPlayerData()); //TODO: T&C Wire this up, see master branch com.gmail.nossr50.datatypes.player.PlayerProfile#scheduleAsyncSave } UserManager.remove(targetPlayer); @@ -190,4 +188,9 @@ public final class UserManager { public static boolean hasPlayerDataKey(Entity entity) { return entity != null && entity.hasMetadata(mcMMO.playerDataKey); } + + public static @NotNull PlayerSaveHandler getPlayerSaveHandler() { + return playerSaveHandler; + } + }