From 8557621c02c13c9b172302f547ad0180d9fe64ac Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 12 Mar 2017 18:43:37 +0100 Subject: [PATCH] #1125 Create infrastructure for Limbo persistence + restore 5.2 JSON storage - Introduce configurable storage mechanism - LimboPersistence wraps a LimboPersistenceHandler, of which there are multiple implementations - Outside of the limbo.persistence package, classes only talk to LimboPersistence - Restore the way of persisting to JSON from 5.2 (SeparateFilePersistenceHandler) - Add handling for stored limbo players - Merge any existing LimboPlayers together with the goal of only keeping one version of a LimboPlayer: there is no way for a player to be online without triggering the creation of a LimboPlayer first, so we can guarantee that the in-memory LimboPlayer is the most up-to-date, i.e. when restoring limbo data we don't have to check against the disk. - Create and delete LimboPlayers at the same time when LimboPlayers are added or removed from the in-memory map - Catch all exceptions in LimboPersistence so a handler throwing an unexpected exception does not stop the limbo process (#1070) - Extend debug command /authme debug limbo to show LimboPlayer information on disk, too --- .checkstyle.xml | 1 + .../executable/authme/debug/DebugCommand.java | 4 +- .../authme/debug/LimboPlayerViewer.java | 42 +++-- .../xephi/authme/data/limbo/LimboService.java | 21 ++- .../authme/data/limbo/LimboServiceHelper.java | 10 +- .../limbo/persistence/LimboPersistence.java | 82 ++++++++ .../persistence/LimboPersistenceHandler.java | 39 ++++ .../persistence/LimboPersistenceType.java | 29 +++ .../persistence/NoOpPersistenceHandler.java | 30 +++ .../SeparateFilePersistenceHandler.java | 178 ++++++++++++++++++ .../settings/properties/LimboSettings.java | 12 +- .../data/limbo/LimboServiceHelperTest.java | 5 +- .../authme/data/limbo/LimboServiceTest.java | 4 + .../persistence/LimboPersistenceTest.java | 175 +++++++++++++++++ .../SeparateFilePersistenceHandlerTest.java | 127 +++++++++++++ .../AuthMeSettingsRetrieverTest.java | 2 +- 16 files changed, 733 insertions(+), 28 deletions(-) create mode 100644 src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistence.java create mode 100644 src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceHandler.java create mode 100644 src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceType.java create mode 100644 src/main/java/fr/xephi/authme/data/limbo/persistence/NoOpPersistenceHandler.java create mode 100644 src/main/java/fr/xephi/authme/data/limbo/persistence/SeparateFilePersistenceHandler.java create mode 100644 src/test/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceTest.java create mode 100644 src/test/java/fr/xephi/authme/data/limbo/persistence/SeparateFilePersistenceHandlerTest.java diff --git a/.checkstyle.xml b/.checkstyle.xml index 4ff5d8dfb..ca570f34f 100644 --- a/.checkstyle.xml +++ b/.checkstyle.xml @@ -159,6 +159,7 @@ + diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/debug/DebugCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/debug/DebugCommand.java index ae3dc22f1..5166df0da 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/debug/DebugCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/debug/DebugCommand.java @@ -6,10 +6,10 @@ import fr.xephi.authme.initialization.factory.Factory; import org.bukkit.command.CommandSender; import javax.inject.Inject; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeMap; /** * Debug command main. @@ -47,7 +47,7 @@ public class DebugCommand implements ExecutableCommand { // Lazy getter private Map getSections() { if (sections == null) { - Map sections = new HashMap<>(); + Map sections = new TreeMap<>(); for (Class sectionClass : sectionClasses) { DebugSection section = debugSectionFactory.newInstance(sectionClass); sections.put(section.getName(), section); diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/debug/LimboPlayerViewer.java b/src/main/java/fr/xephi/authme/command/executable/authme/debug/LimboPlayerViewer.java index 5ac00fb18..c93ad2391 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/debug/LimboPlayerViewer.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/debug/LimboPlayerViewer.java @@ -3,6 +3,7 @@ package fr.xephi.authme.command.executable.authme.debug; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.data.limbo.LimboPlayer; import fr.xephi.authme.data.limbo.LimboService; +import fr.xephi.authme.data.limbo.persistence.LimboPersistence; import fr.xephi.authme.service.BukkitService; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; @@ -27,6 +28,9 @@ class LimboPlayerViewer implements DebugSection { @Inject private LimboService limboService; + @Inject + private LimboPersistence limboPersistence; + @Inject private BukkitService bukkitService; @@ -50,22 +54,23 @@ class LimboPlayerViewer implements DebugSection { return; } - LimboPlayer limbo = limboService.getLimboPlayer(arguments.get(0)); + LimboPlayer memoryLimbo = limboService.getLimboPlayer(arguments.get(0)); Player player = bukkitService.getPlayerExact(arguments.get(0)); - if (limbo == null && player == null) { + LimboPlayer diskLimbo = player != null ? limboPersistence.getLimboPlayer(player) : null; + if (memoryLimbo == null && player == null) { sender.sendMessage("No limbo info and no player online with name '" + arguments.get(0) + "'"); return; } - sender.sendMessage(ChatColor.GOLD + "Showing limbo / player info for '" + arguments.get(0) + "'"); - new InfoDisplayer(sender, limbo, player) + sender.sendMessage(ChatColor.GOLD + "Showing disk limbo / limbo / player info for '" + arguments.get(0) + "'"); + new InfoDisplayer(sender, diskLimbo, memoryLimbo, player) .sendEntry("Is op", LimboPlayer::isOperator, Player::isOp) .sendEntry("Walk speed", LimboPlayer::getWalkSpeed, Player::getWalkSpeed) .sendEntry("Can fly", LimboPlayer::isCanFly, Player::getAllowFlight) .sendEntry("Fly speed", LimboPlayer::getFlySpeed, Player::getFlySpeed) .sendEntry("Location", l -> formatLocation(l.getLocation()), p -> formatLocation(p.getLocation())) .sendEntry("Group", LimboPlayer::getGroup, p -> ""); - sender.sendMessage("Note: group is only shown for LimboPlayer"); + sender.sendMessage("Note: group is not shown for Player. Use /authme debug groups"); } /** @@ -102,25 +107,30 @@ class LimboPlayerViewer implements DebugSection { */ private static final class InfoDisplayer { private final CommandSender sender; - private final Optional limbo; + private final Optional diskLimbo; + private final Optional memoryLimbo; private final Optional player; /** * Constructor. * * @param sender command sender to send the information to - * @param limbo the limbo player to get data from + * @param memoryLimbo the limbo player to get data from * @param player the player to get data from */ - InfoDisplayer(CommandSender sender, LimboPlayer limbo, Player player) { + InfoDisplayer(CommandSender sender, LimboPlayer diskLimbo, LimboPlayer memoryLimbo, Player player) { this.sender = sender; - this.limbo = Optional.ofNullable(limbo); + this.diskLimbo = Optional.ofNullable(diskLimbo); + this.memoryLimbo = Optional.ofNullable(memoryLimbo); this.player = Optional.ofNullable(player); - if (limbo == null) { + if (memoryLimbo == null) { sender.sendMessage("Note: no Limbo information available"); - } else if (player == null) { + } + if (player == null) { sender.sendMessage("Note: player is not online"); + } else if (diskLimbo == null) { + sender.sendMessage("Note: no Limbo on disk available"); } } @@ -138,10 +148,16 @@ class LimboPlayerViewer implements DebugSection { Function playerGetter) { sender.sendMessage( title + ": " - + limbo.map(limboGetter).map(String::valueOf).orElse("--") + + getData(diskLimbo, limboGetter) + " / " - + player.map(playerGetter).map(String::valueOf).orElse("--")); + + getData(memoryLimbo, limboGetter) + + " / " + + getData(player, playerGetter)); return this; } + + static String getData(Optional entity, Function getter) { + return entity.map(getter).map(String::valueOf).orElse(" -- "); + } } } diff --git a/src/main/java/fr/xephi/authme/data/limbo/LimboService.java b/src/main/java/fr/xephi/authme/data/limbo/LimboService.java index 290b79474..e78ca3139 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/LimboService.java +++ b/src/main/java/fr/xephi/authme/data/limbo/LimboService.java @@ -1,6 +1,7 @@ package fr.xephi.authme.data.limbo; import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.data.limbo.persistence.LimboPersistence; import fr.xephi.authme.settings.Settings; import org.bukkit.entity.Player; @@ -28,7 +29,10 @@ public class LimboService { private LimboPlayerTaskManager taskManager; @Inject - private LimboServiceHelper limboServiceHelper; + private LimboServiceHelper helper; + + @Inject + private LimboPersistence persistence; LimboService() { } @@ -42,19 +46,25 @@ public class LimboService { public void createLimboPlayer(Player player, boolean isRegistered) { final String name = player.getName().toLowerCase(); + LimboPlayer limboFromDisk = persistence.getLimboPlayer(player); + if (limboFromDisk != null) { + ConsoleLogger.debug("LimboPlayer for `{0}` already exists on disk", name); + } + LimboPlayer existingLimbo = entries.remove(name); if (existingLimbo != null) { existingLimbo.clearTasks(); - ConsoleLogger.debug("LimboPlayer for `{0}` was already present", name); + ConsoleLogger.debug("LimboPlayer for `{0}` already present in memory", name); } - LimboPlayer limboPlayer = limboServiceHelper.merge( - limboServiceHelper.createLimboPlayer(player, isRegistered), existingLimbo); + LimboPlayer limboPlayer = helper.merge(existingLimbo, limboFromDisk); + limboPlayer = helper.merge(helper.createLimboPlayer(player, isRegistered), limboPlayer); taskManager.registerMessageTask(player, limboPlayer, isRegistered); taskManager.registerTimeoutTask(player, limboPlayer); - limboServiceHelper.revokeLimboStates(player); + helper.revokeLimboStates(player); entries.put(name, limboPlayer); + persistence.saveLimboPlayer(player, limboPlayer); } /** @@ -98,6 +108,7 @@ public class LimboService { settings.getProperty(RESTORE_WALK_SPEED).restoreWalkSpeed(player, limbo); limbo.clearTasks(); ConsoleLogger.debug("Restored LimboPlayer stats for `{0}`", lowerName); + persistence.removeLimboPlayer(player); } } diff --git a/src/main/java/fr/xephi/authme/data/limbo/LimboServiceHelper.java b/src/main/java/fr/xephi/authme/data/limbo/LimboServiceHelper.java index fc6933461..7373212f7 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/LimboServiceHelper.java +++ b/src/main/java/fr/xephi/authme/data/limbo/LimboServiceHelper.java @@ -69,8 +69,7 @@ class LimboServiceHelper { *
    *
  • isOperator, allowFlight: true if either limbo has true
    • *
  • flySpeed, walkSpeed: maximum value of either limbo player
    • - *
  • group: from old limbo if not empty, otherwise from new limbo
    • - *
  • location: from old limbo
    • + *
  • group, location: from old limbo if not empty/null, otherwise from new limbo
    • *
* * @param newLimbo the new limbo player @@ -89,8 +88,9 @@ class LimboServiceHelper { float flySpeed = Math.max(newLimbo.getFlySpeed(), oldLimbo.getFlySpeed()); float walkSpeed = Math.max(newLimbo.getWalkSpeed(), oldLimbo.getWalkSpeed()); String group = firstNotEmpty(newLimbo.getGroup(), oldLimbo.getGroup()); + Location location = firstNotNull(oldLimbo.getLocation(), newLimbo.getLocation()); - return new LimboPlayer(oldLimbo.getLocation(), isOperator, group, canFly, walkSpeed, flySpeed); + return new LimboPlayer(location, isOperator, group, canFly, walkSpeed, flySpeed); } private static String firstNotEmpty(String newGroup, String oldGroup) { @@ -100,4 +100,8 @@ class LimboServiceHelper { } return oldGroup; } + + private static Location firstNotNull(Location first, Location second) { + return first == null ? second : first; + } } diff --git a/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistence.java b/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistence.java new file mode 100644 index 000000000..35cefb486 --- /dev/null +++ b/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistence.java @@ -0,0 +1,82 @@ +package fr.xephi.authme.data.limbo.persistence; + +import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.data.limbo.LimboPlayer; +import fr.xephi.authme.initialization.SettingsDependent; +import fr.xephi.authme.initialization.factory.Factory; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.LimboSettings; +import org.bukkit.entity.Player; + +import javax.inject.Inject; + +/** + * Handles the persistence of LimboPlayers. + */ +public class LimboPersistence implements SettingsDependent { + + private final Factory handlerFactory; + + private LimboPersistenceHandler handler; + + @Inject + LimboPersistence(Settings settings, Factory handlerFactory) { + this.handlerFactory = handlerFactory; + reload(settings); + } + + /** + * Retrieves the LimboPlayer for the given player if available. + * + * @param player the player to retrieve the LimboPlayer for + * @return the player's limbo player, or null if not available + */ + public LimboPlayer getLimboPlayer(Player player) { + try { + return handler.getLimboPlayer(player); + } catch (Exception e) { + ConsoleLogger.logException("Could not get LimboPlayer for '" + player.getName() + "'", e); + } + return null; + } + + /** + * Saves the given LimboPlayer for the provided player. + * + * @param player the player to save the LimboPlayer for + * @param limbo the limbo player to save + */ + public void saveLimboPlayer(Player player, LimboPlayer limbo) { + try { + handler.saveLimboPlayer(player, limbo); + } catch (Exception e) { + ConsoleLogger.logException("Could not save LimboPlayer for '" + player.getName() + "'", e); + } + } + + /** + * Removes the LimboPlayer for the given player. + * + * @param player the player whose LimboPlayer should be removed + */ + public void removeLimboPlayer(Player player) { + try { + handler.removeLimboPlayer(player); + } catch (Exception e) { + ConsoleLogger.logException("Could not remove LimboPlayer for '" + player.getName() + "'", e); + } + } + + @Override + public void reload(Settings settings) { + LimboPersistenceType persistenceType = settings.getProperty(LimboSettings.LIMBO_PERSISTENCE_TYPE); + if (handler == null || handler.getType() != persistenceType) { + // If we're changing from an existing handler, output a quick hint that nothing is converted. + if (handler != null) { + ConsoleLogger.info("Limbo persistence type has changed! Note that the data is not converted."); + } + + handler = handlerFactory.newInstance(persistenceType.getImplementationClass()); + } + } +} diff --git a/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceHandler.java b/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceHandler.java new file mode 100644 index 000000000..95e88aada --- /dev/null +++ b/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceHandler.java @@ -0,0 +1,39 @@ +package fr.xephi.authme.data.limbo.persistence; + +import fr.xephi.authme.data.limbo.LimboPlayer; +import org.bukkit.entity.Player; + +/** + * Handles I/O for storing LimboPlayer objects. + */ +interface LimboPersistenceHandler { + + /** + * Returns the limbo player for the given player if it exists. + * + * @param player the player + * @return the stored limbo player, or null if not available + */ + LimboPlayer getLimboPlayer(Player player); + + /** + * Saves the given limbo player for the given player to the disk. + * + * @param player the player to save the limbo player for + * @param limbo the limbo player to save + */ + void saveLimboPlayer(Player player, LimboPlayer limbo); + + /** + * Removes the limbo player from the disk. + * + * @param player the player whose limbo player should be removed + */ + void removeLimboPlayer(Player player); + + /** + * @return the type of the limbo persistence implementation + */ + LimboPersistenceType getType(); + +} diff --git a/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceType.java b/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceType.java new file mode 100644 index 000000000..9e6d8fe91 --- /dev/null +++ b/src/main/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceType.java @@ -0,0 +1,29 @@ +package fr.xephi.authme.data.limbo.persistence; + +/** + * Types of persistence for LimboPlayer objects. + */ +public enum LimboPersistenceType { + + INDIVIDUAL_FILES(SeparateFilePersistenceHandler.class), + + DISABLED(NoOpPersistenceHandler.class); + + private final Class implementationClass; + + /** + * Constructor. + * + * @param implementationClass the implementation class + */ + LimboPersistenceType(Class implementationClass) { + this.implementationClass= implementationClass; + } + + /** + * @return class implementing the persistence type + */ + public Class getImplementationClass() { + return implementationClass; + } +} diff --git a/src/main/java/fr/xephi/authme/data/limbo/persistence/NoOpPersistenceHandler.java b/src/main/java/fr/xephi/authme/data/limbo/persistence/NoOpPersistenceHandler.java new file mode 100644 index 000000000..ac6ff9b36 --- /dev/null +++ b/src/main/java/fr/xephi/authme/data/limbo/persistence/NoOpPersistenceHandler.java @@ -0,0 +1,30 @@ +package fr.xephi.authme.data.limbo.persistence; + +import fr.xephi.authme.data.limbo.LimboPlayer; +import org.bukkit.entity.Player; + +/** + * Limbo player persistence implementation that does nothing. + */ +class NoOpPersistenceHandler implements LimboPersistenceHandler { + + @Override + public LimboPlayer getLimboPlayer(Player player) { + return null; + } + + @Override + public void saveLimboPlayer(Player player, LimboPlayer limbo) { + // noop + } + + @Override + public void removeLimboPlayer(Player player) { + // noop + } + + @Override + public LimboPersistenceType getType() { + return LimboPersistenceType.DISABLED; + } +} diff --git a/src/main/java/fr/xephi/authme/data/limbo/persistence/SeparateFilePersistenceHandler.java b/src/main/java/fr/xephi/authme/data/limbo/persistence/SeparateFilePersistenceHandler.java new file mode 100644 index 000000000..60927cf09 --- /dev/null +++ b/src/main/java/fr/xephi/authme/data/limbo/persistence/SeparateFilePersistenceHandler.java @@ -0,0 +1,178 @@ +package fr.xephi.authme.data.limbo.persistence; + +import com.google.common.io.Files; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.JsonDeserializationContext; +import com.google.gson.JsonDeserializer; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonSerializationContext; +import com.google.gson.JsonSerializer; +import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.data.limbo.LimboPlayer; +import fr.xephi.authme.initialization.DataFolder; +import fr.xephi.authme.service.BukkitService; +import fr.xephi.authme.util.FileUtils; +import fr.xephi.authme.util.PlayerUtils; +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.entity.Player; + +import javax.inject.Inject; +import java.io.File; +import java.io.IOException; +import java.lang.reflect.Type; +import java.nio.charset.StandardCharsets; + +/** + * Saves LimboPlayer objects as JSON into individual files. + */ +class SeparateFilePersistenceHandler implements LimboPersistenceHandler { + + private final Gson gson; + private final File cacheDir; + private final BukkitService bukkitService; + + @Inject + SeparateFilePersistenceHandler(@DataFolder File dataFolder, BukkitService bukkitService) { + this.bukkitService = bukkitService; + + cacheDir = new File(dataFolder, "playerdata"); + if (!cacheDir.exists() && !cacheDir.isDirectory() && !cacheDir.mkdir()) { + ConsoleLogger.warning("Failed to create userdata directory."); + } + gson = new GsonBuilder() + .registerTypeAdapter(LimboPlayer.class, new LimboPlayerSerializer()) + .registerTypeAdapter(LimboPlayer.class, new LimboPlayerDeserializer()) + .setPrettyPrinting() + .create(); + } + + @Override + public LimboPlayer getLimboPlayer(Player player) { + String id = PlayerUtils.getUUIDorName(player); + File file = new File(cacheDir, id + File.separator + "data.json"); + if (!file.exists()) { + return null; + } + + try { + String str = Files.toString(file, StandardCharsets.UTF_8); + return gson.fromJson(str, LimboPlayer.class); + } catch (IOException e) { + ConsoleLogger.logException("Could not read player data on disk for '" + player.getName() + "'", e); + return null; + } + } + + @Override + public void saveLimboPlayer(Player player, LimboPlayer limboPlayer) { + String id = PlayerUtils.getUUIDorName(player); + try { + File file = new File(cacheDir, id + File.separator + "data.json"); + Files.createParentDirs(file); + Files.touch(file); + Files.write(gson.toJson(limboPlayer), file, StandardCharsets.UTF_8); + } catch (IOException e) { + ConsoleLogger.logException("Failed to write " + player.getName() + " data:", e); + } + } + + /** + * Removes the LimboPlayer. This will delete the + * "playerdata/<uuid or name>/" folder from disk. + * + * @param player player to remove + */ + @Override + public void removeLimboPlayer(Player player) { + String id = PlayerUtils.getUUIDorName(player); + File file = new File(cacheDir, id); + if (file.exists()) { + FileUtils.purgeDirectory(file); + FileUtils.delete(file); + } + } + + @Override + public LimboPersistenceType getType() { + return LimboPersistenceType.INDIVIDUAL_FILES; + } + + private final class LimboPlayerDeserializer implements JsonDeserializer { + + @Override + public LimboPlayer deserialize(JsonElement jsonElement, Type type, + JsonDeserializationContext context) { + JsonObject jsonObject = jsonElement.getAsJsonObject(); + if (jsonObject == null) { + return null; + } + + Location loc = null; + String group = ""; + boolean operator = false; + boolean canFly = false; + float walkSpeed = LimboPlayer.DEFAULT_WALK_SPEED; + float flySpeed = LimboPlayer.DEFAULT_FLY_SPEED; + + JsonElement e; + if ((e = jsonObject.getAsJsonObject("location")) != null) { + JsonObject obj = e.getAsJsonObject(); + World world = bukkitService.getWorld(obj.get("world").getAsString()); + if (world != null) { + double x = obj.get("x").getAsDouble(); + double y = obj.get("y").getAsDouble(); + double z = obj.get("z").getAsDouble(); + float yaw = obj.get("yaw").getAsFloat(); + float pitch = obj.get("pitch").getAsFloat(); + loc = new Location(world, x, y, z, yaw, pitch); + } + } + if ((e = jsonObject.get("group")) != null) { + group = e.getAsString(); + } + if ((e = jsonObject.get("operator")) != null) { + operator = e.getAsBoolean(); + } + if ((e = jsonObject.get("can-fly")) != null) { + canFly = e.getAsBoolean(); + } + if ((e = jsonObject.get("walk-speed")) != null) { + walkSpeed = e.getAsFloat(); + } + if ((e = jsonObject.get("fly-speed")) != null) { + flySpeed = e.getAsFloat(); + } + + return new LimboPlayer(loc, operator, group, canFly, walkSpeed, flySpeed); + } + } + + private static final class LimboPlayerSerializer implements JsonSerializer { + + @Override + public JsonElement serialize(LimboPlayer limboPlayer, Type type, + JsonSerializationContext context) { + JsonObject obj = new JsonObject(); + obj.addProperty("group", limboPlayer.getGroup()); + + Location loc = limboPlayer.getLocation(); + JsonObject obj2 = new JsonObject(); + obj2.addProperty("world", loc.getWorld().getName()); + obj2.addProperty("x", loc.getX()); + obj2.addProperty("y", loc.getY()); + obj2.addProperty("z", loc.getZ()); + obj2.addProperty("yaw", loc.getYaw()); + obj2.addProperty("pitch", loc.getPitch()); + obj.add("location", obj2); + + obj.addProperty("operator", limboPlayer.isOperator()); + obj.addProperty("can-fly", limboPlayer.isCanFly()); + obj.addProperty("walk-speed", limboPlayer.getWalkSpeed()); + obj.addProperty("fly-speed", limboPlayer.getFlySpeed()); + return obj; + } + } +} diff --git a/src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java b/src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java index f861c4990..1e9d80d5c 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/LimboSettings.java @@ -7,6 +7,7 @@ import ch.jalu.configme.properties.Property; import com.google.common.collect.ImmutableMap; import fr.xephi.authme.data.limbo.AllowFlightRestoreType; import fr.xephi.authme.data.limbo.WalkFlySpeedRestoreType; +import fr.xephi.authme.data.limbo.persistence.LimboPersistenceType; import java.util.Map; @@ -17,6 +18,15 @@ import static ch.jalu.configme.properties.PropertyInitializer.newProperty; */ public final class LimboSettings implements SettingsHolder { + @Comment({ + "Besides storing the data in memory, you can define if/how the data should be persisted", + "on disk. This is useful in case of a server crash, so next time the server starts we can", + "properly restore things like OP status, ability to fly, and walk/fly speed.", + "DISABLED: no disk storage, INDIVIDUAL_FILES: each player data in its own file" + }) + public static final Property LIMBO_PERSISTENCE_TYPE = + newProperty(LimboPersistenceType.class, "limbo.persistence", LimboPersistenceType.INDIVIDUAL_FILES); + @Comment({ "Whether the player is allowed to fly: RESTORE, ENABLE, DISABLE.", "RESTORE sets back the old property from the player." @@ -50,7 +60,7 @@ public final class LimboSettings implements SettingsHolder { "Before a user logs in, various properties are temporarily removed from the player,", "such as OP status, ability to fly, and walk/fly speed.", "Once the user is logged in, we add back the properties we previously saved.", - "In this section, you may define how the properties should be restored." + "In this section, you may define how these properties should be handled." }; return ImmutableMap.of("limbo", limboExplanation); } diff --git a/src/test/java/fr/xephi/authme/data/limbo/LimboServiceHelperTest.java b/src/test/java/fr/xephi/authme/data/limbo/LimboServiceHelperTest.java index 77a21d692..43f110331 100644 --- a/src/test/java/fr/xephi/authme/data/limbo/LimboServiceHelperTest.java +++ b/src/test/java/fr/xephi/authme/data/limbo/LimboServiceHelperTest.java @@ -48,14 +48,13 @@ public class LimboServiceHelperTest { // given Location newLocation = mock(Location.class); LimboPlayer newLimbo = new LimboPlayer(newLocation, false, "grp-new", true, 0.3f, 0.0f); - Location oldLocation = mock(Location.class); - LimboPlayer oldLimbo = new LimboPlayer(oldLocation, false, "", false, 0.1f, 0.1f); + LimboPlayer oldLimbo = new LimboPlayer(null, false, "", false, 0.1f, 0.1f); // when LimboPlayer result = limboServiceHelper.merge(newLimbo, oldLimbo); // then - assertThat(result.getLocation(), equalTo(oldLocation)); + assertThat(result.getLocation(), equalTo(newLocation)); assertThat(result.isOperator(), equalTo(false)); assertThat(result.getGroup(), equalTo("grp-new")); assertThat(result.isCanFly(), equalTo(true)); diff --git a/src/test/java/fr/xephi/authme/data/limbo/LimboServiceTest.java b/src/test/java/fr/xephi/authme/data/limbo/LimboServiceTest.java index a2e791fab..807024cd6 100644 --- a/src/test/java/fr/xephi/authme/data/limbo/LimboServiceTest.java +++ b/src/test/java/fr/xephi/authme/data/limbo/LimboServiceTest.java @@ -4,6 +4,7 @@ import ch.jalu.injector.testing.DelayedInjectionRunner; import ch.jalu.injector.testing.InjectDelayed; import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.TestHelper; +import fr.xephi.authme.data.limbo.persistence.LimboPersistence; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.SpawnLoader; @@ -57,6 +58,9 @@ public class LimboServiceTest { @Mock private LimboPlayerTaskManager taskManager; + @Mock + private LimboPersistence limboPersistence; + @BeforeClass public static void initLogger() { TestHelper.setupLogger(); diff --git a/src/test/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceTest.java b/src/test/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceTest.java new file mode 100644 index 000000000..3511771dd --- /dev/null +++ b/src/test/java/fr/xephi/authme/data/limbo/persistence/LimboPersistenceTest.java @@ -0,0 +1,175 @@ +package fr.xephi.authme.data.limbo.persistence; + +import ch.jalu.injector.testing.BeforeInjecting; +import ch.jalu.injector.testing.DelayedInjectionRunner; +import ch.jalu.injector.testing.InjectDelayed; +import fr.xephi.authme.ReflectionTestUtils; +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.data.limbo.LimboPlayer; +import fr.xephi.authme.initialization.factory.Factory; +import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.LimboSettings; +import org.bukkit.entity.Player; +import org.hamcrest.Matcher; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; + +import java.util.logging.Logger; + +import static org.hamcrest.Matchers.both; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.Assert.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.hamcrest.MockitoHamcrest.argThat; + +/** + * Test for {@link LimboPersistence}. + */ +@RunWith(DelayedInjectionRunner.class) +public class LimboPersistenceTest { + + @InjectDelayed + private LimboPersistence limboPersistence; + + @Mock + private Factory handlerFactory; + + @Mock + private Settings settings; + + @BeforeClass + public static void setUpLogger() { + TestHelper.setupLogger(); + } + + @BeforeInjecting + @SuppressWarnings("unchecked") + public void setUpMocks() { + given(settings.getProperty(LimboSettings.LIMBO_PERSISTENCE_TYPE)).willReturn(LimboPersistenceType.DISABLED); + given(handlerFactory.newInstance(any(Class.class))) + .willAnswer(invocation -> mock(invocation.getArgument(0))); + } + + @Test + public void shouldInitializeProperly() { + // given / when / then + assertThat(getHandler(), instanceOf(NoOpPersistenceHandler.class)); + } + + @Test + public void shouldDelegateToHandler() { + // given + Player player = mock(Player.class); + LimboPersistenceHandler handler = getHandler(); + LimboPlayer limbo = mock(LimboPlayer.class); + given(handler.getLimboPlayer(player)).willReturn(limbo); + + // when + LimboPlayer result = limboPersistence.getLimboPlayer(player); + limboPersistence.saveLimboPlayer(player, mock(LimboPlayer.class)); + limboPersistence.removeLimboPlayer(mock(Player.class)); + + // then + assertThat(result, equalTo(limbo)); + verify(handler).getLimboPlayer(player); + verify(handler).saveLimboPlayer(eq(player), argThat(notNullAndDifferentFrom(limbo))); + verify(handler).removeLimboPlayer(argThat(notNullAndDifferentFrom(player))); + } + + @Test + public void shouldReloadProperly() { + // given + given(settings.getProperty(LimboSettings.LIMBO_PERSISTENCE_TYPE)) + .willReturn(LimboPersistenceType.INDIVIDUAL_FILES); + + // when + limboPersistence.reload(settings); + + // then + assertThat(getHandler(), instanceOf(LimboPersistenceType.INDIVIDUAL_FILES.getImplementationClass())); + } + + @Test + public void shouldNotReinitializeHandlerForSameType() { + // given + LimboPersistenceHandler currentHandler = getHandler(); + Mockito.reset(handlerFactory); + given(currentHandler.getType()).willCallRealMethod(); + + // when + limboPersistence.reload(settings); + + // then + verifyZeroInteractions(handlerFactory); + assertThat(currentHandler, sameInstance(getHandler())); + } + + @Test + public void shouldHandleExceptionWhenGettingLimbo() { + // given + Player player = mock(Player.class); + Logger logger = TestHelper.setupLogger(); + LimboPersistenceHandler handler = getHandler(); + doThrow(IllegalAccessException.class).when(handler).getLimboPlayer(player); + + // when + LimboPlayer result = limboPersistence.getLimboPlayer(player); + + // then + assertThat(result, nullValue()); + verify(logger).warning(argThat(containsString("[IllegalAccessException]"))); + } + + @Test + public void shouldHandleExceptionWhenSavingLimbo() { + // given + Player player = mock(Player.class); + LimboPlayer limbo = mock(LimboPlayer.class); + Logger logger = TestHelper.setupLogger(); + LimboPersistenceHandler handler = getHandler(); + doThrow(IllegalStateException.class).when(handler).saveLimboPlayer(player, limbo); + + // when + limboPersistence.saveLimboPlayer(player, limbo); + + // then + verify(logger).warning(argThat(containsString("[IllegalStateException]"))); + } + + @Test + public void shouldHandleExceptionWhenRemovingLimbo() { + // given + Player player = mock(Player.class); + Logger logger = TestHelper.setupLogger(); + LimboPersistenceHandler handler = getHandler(); + doThrow(UnsupportedOperationException.class).when(handler).removeLimboPlayer(player); + + // when + limboPersistence.removeLimboPlayer(player); + + // then + verify(logger).warning(argThat(containsString("[UnsupportedOperationException]"))); + } + + private LimboPersistenceHandler getHandler() { + return ReflectionTestUtils.getFieldValue(LimboPersistence.class, limboPersistence, "handler"); + } + + private static Matcher notNullAndDifferentFrom(T o) { + return both(not(sameInstance(o))).and(not(nullValue())); + } +} diff --git a/src/test/java/fr/xephi/authme/data/limbo/persistence/SeparateFilePersistenceHandlerTest.java b/src/test/java/fr/xephi/authme/data/limbo/persistence/SeparateFilePersistenceHandlerTest.java new file mode 100644 index 000000000..a4e7dc93f --- /dev/null +++ b/src/test/java/fr/xephi/authme/data/limbo/persistence/SeparateFilePersistenceHandlerTest.java @@ -0,0 +1,127 @@ +package fr.xephi.authme.data.limbo.persistence; + +import ch.jalu.injector.testing.BeforeInjecting; +import ch.jalu.injector.testing.DelayedInjectionRunner; +import ch.jalu.injector.testing.InjectDelayed; +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.data.limbo.LimboPlayer; +import fr.xephi.authme.initialization.DataFolder; +import fr.xephi.authme.service.BukkitService; +import fr.xephi.authme.util.FileUtils; +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.entity.Player; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.mockito.Mock; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.UUID; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Test for {@link SeparateFilePersistenceHandler}. + */ +@RunWith(DelayedInjectionRunner.class) +public class SeparateFilePersistenceHandlerTest { + + private static final UUID SAMPLE_UUID = UUID.nameUUIDFromBytes("PersistenceTest".getBytes()); + private static final String SOURCE_FOLDER = TestHelper.PROJECT_ROOT + "data/backup/"; + + @InjectDelayed + private SeparateFilePersistenceHandler handler; + + @Mock + private BukkitService bukkitService; + + @DataFolder + private File dataFolder; + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @BeforeInjecting + public void copyTestFiles() throws IOException { + dataFolder = temporaryFolder.newFolder(); + File playerFolder = new File(dataFolder, FileUtils.makePath("playerdata", SAMPLE_UUID.toString())); + if (!playerFolder.mkdirs()) { + throw new IllegalStateException("Cannot create '" + playerFolder.getAbsolutePath() + "'"); + } + Files.copy(TestHelper.getJarPath(FileUtils.makePath(SOURCE_FOLDER, "sample-folder", "data.json")), + new File(playerFolder, "data.json").toPath()); + } + + @Test + public void shouldReadDataFromFile() { + // given + Player player = mock(Player.class); + given(player.getUniqueId()).willReturn(SAMPLE_UUID); + World world = mock(World.class); + given(bukkitService.getWorld("nether")).willReturn(world); + + // when + LimboPlayer data = handler.getLimboPlayer(player); + + // then + assertThat(data, not(nullValue())); + assertThat(data.isOperator(), equalTo(true)); + assertThat(data.isCanFly(), equalTo(true)); + assertThat(data.getWalkSpeed(), equalTo(0.2f)); + assertThat(data.getFlySpeed(), equalTo(0.1f)); + assertThat(data.getGroup(), equalTo("players")); + Location location = data.getLocation(); + assertThat(location.getX(), equalTo(-113.219)); + assertThat(location.getY(), equalTo(72.0)); + assertThat(location.getZ(), equalTo(130.637)); + assertThat(location.getWorld(), equalTo(world)); + assertThat(location.getPitch(), equalTo(24.15f)); + assertThat(location.getYaw(), equalTo(-292.484f)); + } + + @Test + public void shouldReturnNullForUnavailablePlayer() { + // given + Player player = mock(Player.class); + given(player.getUniqueId()).willReturn(UUID.nameUUIDFromBytes("other-player".getBytes())); + + // when + LimboPlayer data = handler.getLimboPlayer(player); + + // then + assertThat(data, nullValue()); + } + + @Test + public void shouldSavePlayerData() { + // given + Player player = mock(Player.class); + UUID uuid = UUID.nameUUIDFromBytes("New player".getBytes()); + given(player.getUniqueId()).willReturn(uuid); + + + World world = mock(World.class); + given(world.getName()).willReturn("player-world"); + Location location = new Location(world, 0.2, 102.25, -89.28, 3.02f, 90.13f); + String group = "primary-grp"; + LimboPlayer limbo = new LimboPlayer(location, true, group, true, 1.2f, 0.8f); + + // when + handler.saveLimboPlayer(player, limbo); + + // then + File playerFile = new File(dataFolder, FileUtils.makePath("playerdata", uuid.toString(), "data.json")); + assertThat(playerFile.exists(), equalTo(true)); + // TODO ljacqu 20160711: Check contents of file + } + +} diff --git a/src/test/java/fr/xephi/authme/settings/properties/AuthMeSettingsRetrieverTest.java b/src/test/java/fr/xephi/authme/settings/properties/AuthMeSettingsRetrieverTest.java index 2a1c588e6..fcb1228c9 100644 --- a/src/test/java/fr/xephi/authme/settings/properties/AuthMeSettingsRetrieverTest.java +++ b/src/test/java/fr/xephi/authme/settings/properties/AuthMeSettingsRetrieverTest.java @@ -22,7 +22,7 @@ public class AuthMeSettingsRetrieverTest { // an error margin of 10: this prevents us from having to adjust the test every time the config is changed. // If this test fails, replace the first argument in closeTo() with the new number of properties assertThat((double) configurationData.getProperties().size(), - closeTo(150, 10)); + closeTo(160, 10)); } @Test