From 4d710fa0b119a400478a22494c0f306648326bd0 Mon Sep 17 00:00:00 2001 From: Daniel Walsh Date: Tue, 2 Jan 2024 12:11:04 +0000 Subject: [PATCH] Storage rewrite - Phase 1 (#4065) * Phase 1 - wip * Add some tests * wip * Save waypoints * Implement backpacks * Add tests for waypoints * Changes to ADR * Small changes * Fix englandish and some small changes to UUID in PlayerProfile * Fix saving of player data in a few cases * Documentation around deprecated * Add some more tests * Some small doc updates * Make old Waypoint constructor deprecated and fix javadocs --- .adr-dir | 1 + .gitignore | 1 + docs/adr/0001-storage-layer.md | 129 ++++++ docs/adr/README.md | 11 + .../slimefun4/api/gps/GPSNetwork.java | 2 +- .../slimefun4/api/gps/Waypoint.java | 58 ++- .../slimefun4/api/items/SlimefunItem.java | 8 +- .../slimefun4/api/player/PlayerBackpack.java | 71 +++- .../slimefun4/api/player/PlayerProfile.java | 124 ++---- .../slimefun4/implementation/Slimefun.java | 16 + .../slimefun4/storage/Storage.java | 26 ++ .../storage/backend/legacy/LegacyStorage.java | 127 ++++++ .../slimefun4/storage/data/PlayerData.java | 96 +++++ .../slimefun4/api/gps/TestWaypoints.java | 31 +- .../api/profiles/TestPlayerBackpacks.java | 38 +- .../listeners/TestBackpackListener.java | 13 +- .../storage/backend/TestLegacyBackend.java | 383 ++++++++++++++++++ .../slimefun4/test/mocks/MockProfile.java | 9 +- 18 files changed, 976 insertions(+), 168 deletions(-) create mode 100644 .adr-dir create mode 100644 docs/adr/0001-storage-layer.md create mode 100644 docs/adr/README.md create mode 100644 src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java create mode 100644 src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java create mode 100644 src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java create mode 100644 src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java diff --git a/.adr-dir b/.adr-dir new file mode 100644 index 000000000..c73b64aed --- /dev/null +++ b/.adr-dir @@ -0,0 +1 @@ +docs/adr diff --git a/.gitignore b/.gitignore index fda2f4a05..f025c1e19 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ /.settings/ /.idea/ /.vscode/ +/data-store/ dependency-reduced-pom.xml diff --git a/docs/adr/0001-storage-layer.md b/docs/adr/0001-storage-layer.md new file mode 100644 index 000000000..0809ed04a --- /dev/null +++ b/docs/adr/0001-storage-layer.md @@ -0,0 +1,129 @@ +# 1. Storage layer + +Date: 2023-11-15 +Last update: 2023-12-27 + +**DO NOT rely on any APIs introduced until we finish the work completely!** + +## Status + +Work in progress + +## Context + +Slimefun has been around for a very long time and due to that, the way we +wrote persistence of data has also been around for a very long time. +While Slimefun has grown, the storage layer has never been adapted. +This means that even all these years later, it's using the same old saving/loading. +This isn't necessarily always bad, however, as Slimefun has grown both in terms of content +and the servers using it - we've seen some issues. + +Today, files are saved as YAML files (sometimes with just a JSON object per line), +which is good for a config format but not good for a data store. It can create very large files +that can get corrupted, the way we've been saving data often means loading it all at once as well +rather than lazy-loading and generally isn't very performant. + +For a long time we've been talking about rewriting our data storage in multiple forms +(you may have seen this referenced for "BlockStorage rewrite" or "SQL for PlayerProfiles", etc.). +Now is the time we start to do this, this will be a very large change and will not be done quickly or rushed. + +This ADR talks about the future of our data persistence. + +## Decision + +We want to create a new storage layer abstraction and implementations +which will be backwards-compatible but open up new ways of storing data +within Slimefun. The end end goal is we can quickly and easily support +new storage backends (such as binary storage, SQL, etc.) for things like +[PlayerProfile](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java), [BlockStorage](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java), etc. + +We also want to be generally more efficient in the way we save and load data. +Today, we load way more than is required. +We can improve memory usage by only loading what we need, when we need it. + +We will do this incrementally and at first, in an experimental context. +In that regard, we should aim to minimise the blast radius and lift as much +as possible. + +### Quick changes overview + +* New abstraction over storage to easily support multiple backends. +* Work towards moving away from the legacy YAML based storage. +* Lazy load and save data to more efficiently handle the data life cycle. + +### Implementation details + +There is a new interface called [`Storage`](TBD) which is what all storage +backends will implement. +This will have methods for loading and saving things like +[`PlayerProfile`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java) and [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java). + +Then, backends will implement these +(e.g. [`LegacyStorageBackend`](TBD) (today's YAML situation)) +in order to support these functions. +Not all storage backends are required support each data type. +e.g. SQL may not support [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java). + + +## Addons + +The goal is that Addons will be able to use and implement new storage backends +if they wish and also be extended so they can load/save things as they wish. + +The first few iterations will not focus on Addon support. We want to ensure +this new storage layer will work and supports what we need it to today. + +This ADR will be updated when we get to supporting Addons properly. + +## Considerations + +This will be a big change therefore we will be doing it as incrementally as +possible. +Changes will be tested while in the PR stage and merged into the Dev releases when possible. +We may do an experimental release if required. + +Phases do not (and very likely will not) be done within a single PR. They will also not have any timeframe attached to them. + +The current plan looks like this: + +* Phase 1 - Implement legacy data backend for [`PlayerProfile`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java). + * We want to load player data using the new storage layer with the current + data system. + * We'll want to monitor for any possible issues and generally refine + how this system should look +* Phase 2 - Implement new experimental binary backend for [`PlayerProfile`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java). + * Create a new backend for binary storage + * Implement in an experimental capacity and allow users to opt-in + * Provide a warning that this is **experimental** and there will be bugs. + * Implement new metric for storage backend being used +* Phase 3 - Mark the new backend as stable for [`PlayerProfile`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java). + * Mark it as stable and remove the warnings once we're sure things are + working correctly + * Create a migration path for users currently using "legacy". + * Enable by default for new servers +* Phase 4 - Move [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java) to new storage layer. + * The big one! We're gonna tackle adding this to BlockStorage. + This will probably be a large change and we'll want to be as + careful as possible here. + * Implement `legacy` and `binary` as experimental storage backends + for BlockStorage and allow users to opt-in + * Provide a warning that this is **experimental** and there will be bugs. +* Phase 5 - Mark the new storage layer as stable for [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java). + * Mark it as stable and remove the warnings once we're sure things are + working correctly + * Ensure migration path works here too. + * Enable by default for new servers +* Phase 6 - Finish up and move anything else we want over + * Move over any other data stores we have to the new layer + * We should probably still do experimental -> stable but it should have + less of a lead time. + +## State of work + +* Phase 1: In progress + * https://github.com/Slimefun/Slimefun4/pull/4065 +* Phase 2: Not started +* Phase 3: Not started +* Phase 4: Not started +* Phase 5: Not started +* Phase 6: Not started diff --git a/docs/adr/README.md b/docs/adr/README.md new file mode 100644 index 000000000..1762af11c --- /dev/null +++ b/docs/adr/README.md @@ -0,0 +1,11 @@ +# ADR + +An ADR (Architecture Decision Record) is a document describing large changes, why we made them, etc. + +## Making a new ADR + +If you're making a large change to Slimefun, we recommend creating an ADR +in order to document why this is being made and how it works for future contributors. + +Please follow the general format of the former ADRs or use a tool +such as [`adr-tools`](https://github.com/npryce/adr-tools) to generate a new document. \ No newline at end of file diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/GPSNetwork.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/GPSNetwork.java index d2d9a3c1e..47574fb2c 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/GPSNetwork.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/GPSNetwork.java @@ -331,7 +331,7 @@ public class GPSNetwork { } } - profile.addWaypoint(new Waypoint(profile, id, event.getLocation(), event.getName())); + profile.addWaypoint(new Waypoint(p.getUniqueId(), id, event.getLocation(), event.getName())); SoundEffect.GPS_NETWORK_ADD_WAYPOINT.playFor(p); Slimefun.getLocalization().sendMessage(p, "gps.waypoint.added", true); diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java index 13c8b4256..468b70af6 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java @@ -1,11 +1,13 @@ package io.github.thebusybiscuit.slimefun4.api.gps; import java.util.Objects; +import java.util.UUID; import javax.annotation.Nonnull; import javax.annotation.ParametersAreNonnullByDefault; import org.apache.commons.lang.Validate; +import org.bukkit.Bukkit; import org.bukkit.Location; import org.bukkit.World.Environment; import org.bukkit.entity.Player; @@ -30,14 +32,14 @@ import io.github.thebusybiscuit.slimefun4.implementation.items.teleporter.Telepo */ public class Waypoint { - private final PlayerProfile profile; + private final UUID ownerId; private final String id; private final String name; private final Location location; /** * This constructs a new {@link Waypoint} object. - * + * * @param profile * The owning {@link PlayerProfile} * @param id @@ -46,28 +48,62 @@ public class Waypoint { * The {@link Location} of the {@link Waypoint} * @param name * The name of this {@link Waypoint} + * + * @deprecated Use {@link #Waypoint(UUID, String, Location, String)} instead */ + @Deprecated @ParametersAreNonnullByDefault public Waypoint(PlayerProfile profile, String id, Location loc, String name) { - Validate.notNull(profile, "Profile must never be null!"); + this(profile.getUUID(), id, loc, name); + } + + /** + * This constructs a new {@link Waypoint} object. + * + * @param ownerId + * The owning {@link Player}'s {@link UUID} + * @param id + * The unique id for this {@link Waypoint} + * @param loc + * The {@link Location} of the {@link Waypoint} + * @param name + * The name of this {@link Waypoint} + */ + @ParametersAreNonnullByDefault + public Waypoint(UUID ownerId, String id, Location loc, String name) { + Validate.notNull(ownerId, "owner ID must never be null!"); Validate.notNull(id, "id must never be null!"); Validate.notNull(loc, "Location must never be null!"); Validate.notNull(name, "Name must never be null!"); - this.profile = profile; + this.ownerId = ownerId; this.id = id; this.location = loc; this.name = name; } /** - * This returns the owner of the {@link Waypoint}. + * This returns the owner's {@link UUID} of the {@link Waypoint}. * - * @return The corresponding {@link PlayerProfile} + * @return The corresponding owner's {@link UUID} */ @Nonnull + public UUID getOwnerId() { + return this.ownerId; + } + + /** + * This returns the owner of the {@link Waypoint}. + * + * @return The corresponding {@link PlayerProfile} + * + * @deprecated Use {@link #getOwnerId()} instead + */ + @Nonnull + @Deprecated public PlayerProfile getOwner() { - return profile; + // This is jank and should never actually return null + return PlayerProfile.find(Bukkit.getOfflinePlayer(ownerId)).orElse(null); } /** @@ -126,7 +162,7 @@ public class Waypoint { */ @Override public int hashCode() { - return Objects.hash(profile.getUUID(), id, name, location); + return Objects.hash(this.ownerId, this.id, this.name, this.location); } /** @@ -139,7 +175,9 @@ public class Waypoint { } Waypoint waypoint = (Waypoint) obj; - return profile.getUUID().equals(waypoint.getOwner().getUUID()) && id.equals(waypoint.getId()) && location.equals(waypoint.getLocation()) && name.equals(waypoint.getName()); + return this.ownerId.equals(waypoint.getOwnerId()) + && id.equals(waypoint.getId()) + && location.equals(waypoint.getLocation()) + && name.equals(waypoint.getName()); } - } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java index 20a12ef6b..28dc680ff 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java @@ -1160,11 +1160,11 @@ public class SlimefunItem implements Placeable { } /** - * Retrieve a {@link Optional}<{@link SlimefunItem}> by its id. + * Retrieve a {@link Optional} {@link SlimefunItem} by its id. * * @param id * The id of the {@link SlimefunItem} - * @return The {@link Optional}<{@link SlimefunItem}> associated with that id. Empty if non-existent + * @return The {@link Optional} {@link SlimefunItem} associated with that id. Empty if non-existent */ public static @Nonnull Optional getOptionalById(@Nonnull String id) { return Optional.ofNullable(getById(id)); @@ -1193,11 +1193,11 @@ public class SlimefunItem implements Placeable { } /** - * Retrieve a {@link Optional}<{@link SlimefunItem}> from an {@link ItemStack}. + * Retrieve a {@link Optional} {@link SlimefunItem} from an {@link ItemStack}. * * @param item * The {@link ItemStack} to check - * @return The {@link Optional}<{@link SlimefunItem}> associated with this {@link ItemStack} if present, otherwise empty + * @return The {@link Optional} {@link SlimefunItem} associated with this {@link ItemStack} if present, otherwise empty */ public @Nonnull Optional getOptionalByItem(@Nullable ItemStack item) { return Optional.ofNullable(getByItem(item)); diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java index 4960a7c9b..c0886778d 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java @@ -2,10 +2,13 @@ package io.github.thebusybiscuit.slimefun4.api.player; import java.io.File; import java.util.ArrayList; +import java.util.HashMap; import java.util.Iterator; +import java.util.UUID; import java.util.concurrent.CompletableFuture; import javax.annotation.Nonnull; +import javax.annotation.ParametersAreNonnullByDefault; import org.bukkit.Bukkit; import org.bukkit.entity.HumanEntity; @@ -34,13 +37,22 @@ public class PlayerBackpack { private static final String CONFIG_PREFIX = "backpacks."; - private final PlayerProfile profile; + private final UUID ownerId; private final int id; - private final Config cfg; + @Deprecated + private PlayerProfile profile; + @Deprecated + private Config cfg; private Inventory inventory; private int size; + private PlayerBackpack(@Nonnull UUID ownerId, int id, int size) { + this.ownerId = ownerId; + this.id = id; + this.size = size; + } + /** * This constructor loads an existing Backpack * @@ -48,7 +60,10 @@ public class PlayerBackpack { * The {@link PlayerProfile} of this Backpack * @param id * The id of this Backpack + * + * @deprecated Use {@link PlayerBackpack#load(UUID, int, int, HashMap)} instead */ + @Deprecated public PlayerBackpack(@Nonnull PlayerProfile profile, int id) { this(profile, id, profile.getConfig().getInt(CONFIG_PREFIX + id + ".size")); @@ -66,12 +81,16 @@ public class PlayerBackpack { * The id of this Backpack * @param size * The size of this Backpack + * + * @deprecated Use {@link PlayerBackpack#newBackpack(UUID, int, int)} instead */ + @Deprecated public PlayerBackpack(@Nonnull PlayerProfile profile, int id, int size) { if (size < 9 || size > 54 || size % 9 != 0) { throw new IllegalArgumentException("Invalid size! Size must be one of: [9, 18, 27, 36, 45, 54]"); } + this.ownerId = profile.getUUID(); this.profile = profile; this.id = id; this.cfg = profile.getConfig(); @@ -96,10 +115,17 @@ public class PlayerBackpack { * This method returns the {@link PlayerProfile} this {@link PlayerBackpack} belongs to * * @return The owning {@link PlayerProfile} + * + * @deprecated Use {@link PlayerBackpack#getOwnerId()} instead */ + @Deprecated @Nonnull public PlayerProfile getOwner() { - return profile; + return profile != null ? profile : PlayerProfile.find(Bukkit.getOfflinePlayer(ownerId)).orElse(null); + } + + public UUID getOwnerId() { + return this.ownerId; } /** @@ -172,7 +198,6 @@ public class PlayerBackpack { } this.size = size; - cfg.setValue(CONFIG_PREFIX + id + ".size", size); Inventory inv = Bukkit.createInventory(null, size, "Backpack [" + size + " Slots]"); @@ -187,7 +212,10 @@ public class PlayerBackpack { /** * This method will save the contents of this backpack to a {@link File}. + * + * @deprecated Handled by {@link PlayerProfile#save()} now */ + @Deprecated public void save() { for (int i = 0; i < size; i++) { cfg.setValue(CONFIG_PREFIX + id + ".contents." + i, inventory.getItem(i)); @@ -199,7 +227,40 @@ public class PlayerBackpack { * using {@link PlayerBackpack#save()} */ public void markDirty() { - profile.markDirty(); + if (profile != null) { + profile.markDirty(); + } } + private void setContents(int size, HashMap contents) { + if (this.inventory == null) { + this.inventory = Bukkit.createInventory(null, size, "Backpack [" + size + " Slots]"); + } + + for (int i = 0; i < size; i++) { + this.inventory.setItem(i, contents.get(i)); + } + } + + @ParametersAreNonnullByDefault + public static PlayerBackpack load(UUID ownerId, int id, int size, HashMap contents) { + PlayerBackpack backpack = new PlayerBackpack(ownerId, id, size); + + backpack.setContents(size, contents); + + return backpack; + } + + @ParametersAreNonnullByDefault + public static PlayerBackpack newBackpack(UUID ownerId, int id, int size) { + if (size < 9 || size > 54 || size % 9 != 0) { + throw new IllegalArgumentException("Invalid size! Size must be one of: [9, 18, 27, 36, 45, 54]"); + } + + PlayerBackpack backpack = new PlayerBackpack(ownerId, id, size); + + backpack.setContents(size, new HashMap<>()); + + return backpack; + } } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java index 7b975f570..c51888a1d 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerProfile.java @@ -1,27 +1,22 @@ package io.github.thebusybiscuit.slimefun4.api.player; -import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.OptionalInt; import java.util.Set; import java.util.UUID; import java.util.function.Consumer; -import java.util.logging.Level; import java.util.stream.IntStream; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; import org.apache.commons.lang.Validate; import org.bukkit.Bukkit; import org.bukkit.ChatColor; -import org.bukkit.Location; import org.bukkit.NamespacedKey; import org.bukkit.OfflinePlayer; import org.bukkit.command.CommandSender; @@ -59,50 +54,26 @@ import io.github.thebusybiscuit.slimefun4.utils.NumberUtils; */ public class PlayerProfile { - private final UUID uuid; + private final UUID ownerId; private final String name; private final Config configFile; - private final Config waypointsFile; private boolean dirty = false; private boolean markedForDeletion = false; - private final Set researches = new HashSet<>(); - private final List waypoints = new ArrayList<>(); - private final Map backpacks = new HashMap<>(); private final GuideHistory guideHistory = new GuideHistory(this); private final HashedArmorpiece[] armor = { new HashedArmorpiece(), new HashedArmorpiece(), new HashedArmorpiece(), new HashedArmorpiece() }; - protected PlayerProfile(@Nonnull OfflinePlayer p) { - this.uuid = p.getUniqueId(); + private final PlayerData data; + + protected PlayerProfile(@Nonnull OfflinePlayer p, PlayerData data) { + this.ownerId = p.getUniqueId(); this.name = p.getName(); + this.data = data; - configFile = new Config("data-storage/Slimefun/Players/" + uuid.toString() + ".yml"); - waypointsFile = new Config("data-storage/Slimefun/waypoints/" + uuid.toString() + ".yml"); - - loadProfileData(); - } - - private void loadProfileData() { - for (Research research : Slimefun.getRegistry().getResearches()) { - if (configFile.contains("researches." + research.getID())) { - researches.add(research); - } - } - - for (String key : waypointsFile.getKeys()) { - try { - if (waypointsFile.contains(key + ".world") && Bukkit.getWorld(waypointsFile.getString(key + ".world")) != null) { - String waypointName = waypointsFile.getString(key + ".name"); - Location loc = waypointsFile.getLocation(key); - waypoints.add(new Waypoint(this, key, loc, waypointName)); - } - } catch (Exception x) { - Slimefun.logger().log(Level.WARNING, x, () -> "Could not load Waypoint \"" + key + "\" for Player \"" + name + '"'); - } - } + configFile = new Config("data-storage/Slimefun/Players/" + ownerId.toString() + ".yml"); } /** @@ -131,7 +102,7 @@ public class PlayerProfile { * @return The {@link UUID} of our {@link PlayerProfile} */ public @Nonnull UUID getUUID() { - return uuid; + return ownerId; } /** @@ -157,12 +128,7 @@ public class PlayerProfile { * This method will save the Player's Researches and Backpacks to the hard drive */ public void save() { - for (PlayerBackpack backpack : backpacks.values()) { - backpack.save(); - } - - waypointsFile.save(); - configFile.save(); + Slimefun.getPlayerStorage().savePlayerData(this.ownerId, this.data); dirty = false; } @@ -180,11 +146,9 @@ public class PlayerProfile { dirty = true; if (unlock) { - configFile.setValue("researches." + research.getID(), true); - researches.add(research); + data.addResearch(research); } else { - configFile.setValue("researches." + research.getID(), null); - researches.remove(research); + data.removeResearch(research); } } @@ -202,7 +166,7 @@ public class PlayerProfile { return true; } - return !research.isEnabled() || researches.contains(research); + return !research.isEnabled() || data.getResearches().contains(research); } /** @@ -228,7 +192,7 @@ public class PlayerProfile { * @return A {@code Hashset} of all Researches this {@link Player} has unlocked */ public @Nonnull Set getResearches() { - return ImmutableSet.copyOf(researches); + return ImmutableSet.copyOf(this.data.getResearches()); } /** @@ -238,7 +202,7 @@ public class PlayerProfile { * @return A {@link List} containing every {@link Waypoint} */ public @Nonnull List getWaypoints() { - return ImmutableList.copyOf(waypoints); + return ImmutableList.copyOf(this.data.getWaypoints()); } /** @@ -249,21 +213,8 @@ public class PlayerProfile { * The {@link Waypoint} to add */ public void addWaypoint(@Nonnull Waypoint waypoint) { - Validate.notNull(waypoint, "Cannot add a 'null' waypoint!"); - - for (Waypoint wp : waypoints) { - if (wp.getId().equals(waypoint.getId())) { - throw new IllegalArgumentException("A Waypoint with that id already exists for this Player"); - } - } - - if (waypoints.size() < 21) { - waypoints.add(waypoint); - - waypointsFile.setValue(waypoint.getId(), waypoint.getLocation()); - waypointsFile.setValue(waypoint.getId() + ".name", waypoint.getName()); - markDirty(); - } + this.data.addWaypoint(waypoint); + markDirty(); } /** @@ -274,12 +225,8 @@ public class PlayerProfile { * The {@link Waypoint} to remove */ public void removeWaypoint(@Nonnull Waypoint waypoint) { - Validate.notNull(waypoint, "Cannot remove a 'null' waypoint!"); - - if (waypoints.remove(waypoint)) { - waypointsFile.setValue(waypoint.getId(), null); - markDirty(); - } + this.data.removeWaypoint(waypoint); + markDirty(); } /** @@ -301,8 +248,10 @@ public class PlayerProfile { IntStream stream = IntStream.iterate(0, i -> i + 1).filter(i -> !configFile.contains("backpacks." + i + ".size")); int id = stream.findFirst().getAsInt(); - PlayerBackpack backpack = new PlayerBackpack(this, id, size); - backpacks.put(id, backpack); + PlayerBackpack backpack = PlayerBackpack.newBackpack(this.ownerId, id, size); + this.data.addBackpack(backpack); + + markDirty(); return backpack; } @@ -312,13 +261,10 @@ public class PlayerProfile { throw new IllegalArgumentException("Backpacks cannot have negative ids!"); } - PlayerBackpack backpack = backpacks.get(id); + PlayerBackpack backpack = data.getBackpack(id); if (backpack != null) { - return Optional.of(backpack); - } else if (configFile.contains("backpacks." + id + ".size")) { - backpack = new PlayerBackpack(this, id); - backpacks.put(id, backpack); + markDirty(); return Optional.of(backpack); } @@ -346,7 +292,7 @@ public class PlayerProfile { List titles = Slimefun.getRegistry().getResearchRanks(); int allResearches = countNonEmptyResearches(Slimefun.getRegistry().getResearches()); - float fraction = (float) countNonEmptyResearches(researches) / allResearches; + float fraction = (float) countNonEmptyResearches(getResearches()) / allResearches; int index = (int) (fraction * (titles.size() - 1)); return titles.get(index); @@ -420,7 +366,9 @@ public class PlayerProfile { } Bukkit.getScheduler().runTaskAsynchronously(Slimefun.instance(), () -> { - AsyncProfileLoadEvent event = new AsyncProfileLoadEvent(new PlayerProfile(p)); + PlayerData data = Slimefun.getPlayerStorage().loadPlayerData(p.getUniqueId()); + + AsyncProfileLoadEvent event = new AsyncProfileLoadEvent(new PlayerProfile(p, data)); Bukkit.getPluginManager().callEvent(event); Slimefun.getRegistry().getPlayerProfiles().put(uuid, event.getProfile()); @@ -445,7 +393,9 @@ public class PlayerProfile { if (!Slimefun.getRegistry().getPlayerProfiles().containsKey(p.getUniqueId())) { // Should probably prevent multiple requests for the same profile in the future Bukkit.getScheduler().runTaskAsynchronously(Slimefun.instance(), () -> { - PlayerProfile pp = new PlayerProfile(p); + PlayerData data = Slimefun.getPlayerStorage().loadPlayerData(p.getUniqueId()); + + PlayerProfile pp = new PlayerProfile(p, data); Slimefun.getRegistry().getPlayerProfiles().put(p.getUniqueId(), pp); }); @@ -527,19 +477,23 @@ public class PlayerProfile { return armorCount == 4; } + public PlayerData getPlayerData() { + return this.data; + } + @Override public int hashCode() { - return uuid.hashCode(); + return ownerId.hashCode(); } @Override public boolean equals(Object obj) { - return obj instanceof PlayerProfile profile && uuid.equals(profile.uuid); + return obj instanceof PlayerProfile profile && ownerId.equals(profile.ownerId); } @Override public String toString() { - return "PlayerProfile {" + uuid + "}"; + return "PlayerProfile {" + ownerId + "}"; } } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java index e8c7b460a..8667eed68 100644 --- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/Slimefun.java @@ -15,6 +15,9 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.ParametersAreNonnullByDefault; +import io.github.thebusybiscuit.slimefun4.storage.Storage; +import io.github.thebusybiscuit.slimefun4.storage.backend.legacy.LegacyStorage; + import org.apache.commons.lang.Validate; import org.bukkit.Bukkit; import org.bukkit.Server; @@ -197,6 +200,9 @@ public final class Slimefun extends JavaPlugin implements SlimefunAddon { private final Config items = new Config(this, "Items.yml"); private final Config researches = new Config(this, "Researches.yml"); + // Data storage + private Storage playerStorage; + // Listeners that need to be accessed elsewhere private final GrapplingHookListener grapplingHookListener = new GrapplingHookListener(); private final BackpackListener backpackListener = new BackpackListener(); @@ -258,6 +264,9 @@ public final class Slimefun extends JavaPlugin implements SlimefunAddon { registry.load(this, config); loadTags(); soundService.reload(false); + // TODO: What do we do if tests want to use another storage backend (e.g. testing new feature on legacy + sql)? + // Do we have a way to override this? + playerStorage = new LegacyStorage(); } /** @@ -312,6 +321,10 @@ public final class Slimefun extends JavaPlugin implements SlimefunAddon { networkManager = new NetworkManager(networkSize, config.getBoolean("networks.enable-visualizer"), config.getBoolean("networks.delete-excess-items")); + // Data storage + playerStorage = new LegacyStorage(); + logger.log(Level.INFO, "Using legacy storage for player data"); + // Setting up bStats new Thread(metricsService::start, "Slimefun Metrics").start(); @@ -1068,4 +1081,7 @@ public final class Slimefun extends JavaPlugin implements SlimefunAddon { return instance.getServer().getScheduler().runTask(instance, runnable); } + public static @Nonnull Storage getPlayerStorage() { + return instance().playerStorage; + } } diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java new file mode 100644 index 000000000..037db2afc --- /dev/null +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/Storage.java @@ -0,0 +1,26 @@ +package io.github.thebusybiscuit.slimefun4.storage; + +import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; + +import javax.annotation.concurrent.ThreadSafe; + +import com.google.common.annotations.Beta; + +import java.util.UUID; + +/** + * The {@link Storage} interface is the abstract layer on top of our storage backends. + * Every backend has to implement this interface and has to implement it in a thread-safe way. + * There will be no expectation of running functions in here within the main thread. + * + *

+ * This API is still experimental, it may change without notice. + */ +@Beta +@ThreadSafe +public interface Storage { + + PlayerData loadPlayerData(UUID uuid); + + void savePlayerData(UUID uuid, PlayerData data); +} diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java new file mode 100644 index 000000000..d7981a546 --- /dev/null +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/backend/legacy/LegacyStorage.java @@ -0,0 +1,127 @@ +package io.github.thebusybiscuit.slimefun4.storage.backend.legacy; + +import io.github.bakedlibs.dough.config.Config; +import io.github.thebusybiscuit.slimefun4.api.gps.Waypoint; +import io.github.thebusybiscuit.slimefun4.api.player.PlayerBackpack; +import io.github.thebusybiscuit.slimefun4.api.researches.Research; +import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; +import io.github.thebusybiscuit.slimefun4.storage.Storage; +import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; + +import org.bukkit.Bukkit; +import org.bukkit.Location; +import org.bukkit.inventory.ItemStack; + +import com.google.common.annotations.Beta; + +import javax.annotation.Nonnull; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; +import java.util.logging.Level; + +@Beta +public class LegacyStorage implements Storage { + + @Override + public PlayerData loadPlayerData(@Nonnull UUID uuid) { + Config playerFile = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); + // Not too sure why this is its own file + Config waypointsFile = new Config("data-storage/Slimefun/waypoints/" + uuid + ".yml"); + + // Load research + Set researches = new HashSet<>(); + for (Research research : Slimefun.getRegistry().getResearches()) { + if (playerFile.contains("researches." + research.getID())) { + researches.add(research); + } + } + + // Load backpacks + HashMap backpacks = new HashMap<>(); + for (String key : playerFile.getKeys("backpacks")) { + try { + int id = Integer.parseInt(key); + int size = playerFile.getInt("backpacks." + key + ".size"); + + HashMap items = new HashMap<>(); + for (int i = 0; i < size; i++) { + items.put(i, playerFile.getItem("backpacks." + key + ".contents." + i)); + } + + PlayerBackpack backpack = PlayerBackpack.load(uuid, id, size, items); + + backpacks.put(id, backpack); + } catch (Exception x) { + Slimefun.logger().log(Level.WARNING, x, () -> "Could not load Backpack \"" + key + "\" for Player \"" + uuid + '"'); + } + } + + // Load waypoints + Set waypoints = new HashSet<>(); + for (String key : waypointsFile.getKeys()) { + try { + if (waypointsFile.contains(key + ".world") && Bukkit.getWorld(waypointsFile.getString(key + ".world")) != null) { + String waypointName = waypointsFile.getString(key + ".name"); + Location loc = waypointsFile.getLocation(key); + waypoints.add(new Waypoint(uuid, key, loc, waypointName)); + } + } catch (Exception x) { + Slimefun.logger().log(Level.WARNING, x, () -> "Could not load Waypoint \"" + key + "\" for Player \"" + uuid + '"'); + } + } + + return new PlayerData(researches, backpacks, waypoints); + } + + // The current design of saving all at once isn't great, this will be refined. + @Override + public void savePlayerData(@Nonnull UUID uuid, @Nonnull PlayerData data) { + Config playerFile = new Config("data-storage/Slimefun/Players/" + uuid + ".yml"); + // Not too sure why this is its own file + Config waypointsFile = new Config("data-storage/Slimefun/waypoints/" + uuid + ".yml"); + + // Save research + playerFile.setValue("rearches", null); + for (Research research : Slimefun.getRegistry().getResearches()) { + // Save the research if it's researched + if (data.getResearches().contains(research)) { + playerFile.setValue("researches." + research.getID(), true); + + // Remove the research if it's no longer researched + } else if (playerFile.contains("researches." + research.getID())) { + playerFile.setValue("researches." + research.getID(), null); + } + } + + // Save backpacks + for (PlayerBackpack backpack : data.getBackpacks().values()) { + playerFile.setValue("backpacks." + backpack.getId() + ".size", backpack.getSize()); + + for (int i = 0; i < backpack.getSize(); i++) { + ItemStack item = backpack.getInventory().getItem(i); + if (item != null) { + playerFile.setValue("backpacks." + backpack.getId() + ".contents." + i, item); + + // Remove the item if it's no longer in the inventory + } else if (playerFile.contains("backpacks." + backpack.getId() + ".contents." + i)) { + playerFile.setValue("backpacks." + backpack.getId() + ".contents." + i, null); + } + } + } + + // Save waypoints + waypointsFile.clear(); + for (Waypoint waypoint : data.getWaypoints()) { + // Legacy data uses IDs + waypointsFile.setValue(waypoint.getId(), waypoint.getLocation()); + waypointsFile.setValue(waypoint.getId() + ".name", waypoint.getName()); + } + + // Save files + playerFile.save(); + waypointsFile.save(); + } +} diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java new file mode 100644 index 000000000..8615b6ee5 --- /dev/null +++ b/src/main/java/io/github/thebusybiscuit/slimefun4/storage/data/PlayerData.java @@ -0,0 +1,96 @@ +package io.github.thebusybiscuit.slimefun4.storage.data; + +import com.google.common.annotations.Beta; + +import io.github.thebusybiscuit.slimefun4.api.gps.Waypoint; +import io.github.thebusybiscuit.slimefun4.api.player.PlayerBackpack; +import io.github.thebusybiscuit.slimefun4.api.researches.Research; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import javax.annotation.Nonnull; + +import org.apache.commons.lang.Validate; + +/** + * The data which backs {@link io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile} + * + * This API is still experimental, it may change without notice. + */ +// TODO: Should we keep this in PlayerProfile? +@Beta +public class PlayerData { + + private final Set researches = new HashSet<>(); + private final Map backpacks = new HashMap<>(); + private final Set waypoints = new HashSet<>(); + + public PlayerData(Set researches, Map backpacks, Set waypoints) { + this.researches.addAll(researches); + this.backpacks.putAll(backpacks); + this.waypoints.addAll(waypoints); + } + + public Set getResearches() { + return researches; + } + + public void addResearch(@Nonnull Research research) { + Validate.notNull(research, "Cannot add a 'null' research!"); + researches.add(research); + } + + public void removeResearch(@Nonnull Research research) { + Validate.notNull(research, "Cannot remove a 'null' research!"); + researches.remove(research); + } + + @Nonnull + public Map getBackpacks() { + return backpacks; + } + + @Nonnull + public PlayerBackpack getBackpack(int id) { + return backpacks.get(id); + } + + public void addBackpack(@Nonnull PlayerBackpack backpack) { + Validate.notNull(backpack, "Cannot add a 'null' backpack!"); + backpacks.put(backpack.getId(), backpack); + } + + public void removeBackpack(@Nonnull PlayerBackpack backpack) { + Validate.notNull(backpack, "Cannot remove a 'null' backpack!"); + backpacks.remove(backpack.getId()); + } + + public Set getWaypoints() { + return waypoints; + } + + public void addWaypoint(@Nonnull Waypoint waypoint) { + Validate.notNull(waypoint, "Cannot add a 'null' waypoint!"); + + for (Waypoint wp : waypoints) { + if (wp.getId().equals(waypoint.getId())) { + throw new IllegalArgumentException("A Waypoint with that id already exists for this Player"); + } + } + + // Limited to 21 due to limited UI space and no pagination + if (waypoints.size() >= 21) { + return; // not sure why this doesn't throw but the one above does... + } + + waypoints.add(waypoint); + } + + public void removeWaypoint(@Nonnull Waypoint waypoint) { + Validate.notNull(waypoint, "Cannot remove a 'null' waypoint!"); + waypoints.remove(waypoint); + } +} diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java b/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java index d115135ba..a0de64b14 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/api/gps/TestWaypoints.java @@ -1,5 +1,9 @@ package io.github.thebusybiscuit.slimefun4.api.gps; +import java.io.File; +import java.io.IOException; + +import org.apache.commons.io.FileUtils; import org.bukkit.entity.Player; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; @@ -19,16 +23,20 @@ class TestWaypoints { private static ServerMock server; private static Slimefun plugin; + private static File dataFolder; @BeforeAll public static void load() { server = MockBukkit.mock(); plugin = MockBukkit.load(Slimefun.class); + dataFolder = new File("data-storage/Slimefun/waypoints"); + dataFolder.mkdirs(); } @AfterAll - public static void unload() { + public static void unload() throws IOException { MockBukkit.unmock(); + FileUtils.deleteDirectory(dataFolder); } @Test @@ -38,9 +46,8 @@ class TestWaypoints { PlayerProfile profile = TestUtilities.awaitProfile(player); Assertions.assertTrue(profile.getWaypoints().isEmpty()); - Waypoint waypoint = new Waypoint(profile, "hello", player.getLocation(), "HELLO"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), "hello", player.getLocation(), "HELLO"); profile.addWaypoint(waypoint); - Assertions.assertTrue(profile.isDirty()); Assertions.assertThrows(IllegalArgumentException.class, () -> profile.addWaypoint(null)); @@ -55,7 +62,7 @@ class TestWaypoints { Player player = server.addPlayer(); PlayerProfile profile = TestUtilities.awaitProfile(player); - Waypoint waypoint = new Waypoint(profile, "hello", player.getLocation(), "HELLO"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), "hello", player.getLocation(), "HELLO"); profile.addWaypoint(waypoint); Assertions.assertEquals(1, profile.getWaypoints().size()); @@ -76,7 +83,7 @@ class TestWaypoints { Player player = server.addPlayer(); PlayerProfile profile = TestUtilities.awaitProfile(player); - Waypoint waypoint = new Waypoint(profile, "test", player.getLocation(), "Testing"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), "test", player.getLocation(), "Testing"); profile.addWaypoint(waypoint); Assertions.assertEquals(1, profile.getWaypoints().size()); @@ -91,7 +98,7 @@ class TestWaypoints { PlayerProfile profile = TestUtilities.awaitProfile(player); for (int i = 0; i < 99; i++) { - Waypoint waypoint = new Waypoint(profile, String.valueOf(i), player.getLocation(), "Test"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), String.valueOf(i), player.getLocation(), "Test"); profile.addWaypoint(waypoint); } @@ -114,11 +121,10 @@ class TestWaypoints { @DisplayName("Test equal Waypoints being equal") void testWaypointComparison() throws InterruptedException { Player player = server.addPlayer(); - PlayerProfile profile = TestUtilities.awaitProfile(player); - Waypoint waypoint = new Waypoint(profile, "waypoint", player.getLocation(), "Test"); - Waypoint same = new Waypoint(profile, "waypoint", player.getLocation(), "Test"); - Waypoint different = new Waypoint(profile, "waypoint_nope", player.getLocation(), "Test2"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), "waypoint", player.getLocation(), "Test"); + Waypoint same = new Waypoint(player.getUniqueId(), "waypoint", player.getLocation(), "Test"); + Waypoint different = new Waypoint(player.getUniqueId(), "waypoint_nope", player.getLocation(), "Test2"); Assertions.assertEquals(waypoint, same); Assertions.assertEquals(waypoint.hashCode(), same.hashCode()); @@ -131,10 +137,9 @@ class TestWaypoints { @DisplayName("Test Deathpoints being recognized as Deathpoints") void testIsDeathpoint() throws InterruptedException { Player player = server.addPlayer(); - PlayerProfile profile = TestUtilities.awaitProfile(player); - Waypoint waypoint = new Waypoint(profile, "waypoint", player.getLocation(), "Some Waypoint"); - Waypoint deathpoint = new Waypoint(profile, "deathpoint", player.getLocation(), "player:death I died"); + Waypoint waypoint = new Waypoint(player.getUniqueId(), "waypoint", player.getLocation(), "Some Waypoint"); + Waypoint deathpoint = new Waypoint(player.getUniqueId(), "deathpoint", player.getLocation(), "player:death I died"); Assertions.assertFalse(waypoint.isDeathpoint()); Assertions.assertTrue(deathpoint.isDeathpoint()); diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java b/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java index 3c1ae7175..07f69761e 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/api/profiles/TestPlayerBackpacks.java @@ -2,9 +2,7 @@ package io.github.thebusybiscuit.slimefun4.api.profiles; import java.util.Optional; -import org.bukkit.Material; import org.bukkit.entity.Player; -import org.bukkit.inventory.ItemStack; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; @@ -39,16 +37,12 @@ class TestPlayerBackpacks { void testCreateBackpack() throws InterruptedException { Player player = server.addPlayer(); PlayerProfile profile = TestUtilities.awaitProfile(player); - Assertions.assertFalse(profile.isDirty()); PlayerBackpack backpack = profile.createBackpack(18); Assertions.assertNotNull(backpack); - // Creating a backpack should mark profiles as dirty - Assertions.assertTrue(profile.isDirty()); - - Assertions.assertEquals(profile, backpack.getOwner()); + Assertions.assertEquals(player.getUniqueId(), backpack.getOwnerId()); Assertions.assertEquals(18, backpack.getSize()); Assertions.assertEquals(18, backpack.getInventory().getSize()); } @@ -71,7 +65,6 @@ class TestPlayerBackpacks { backpack.setSize(27); Assertions.assertEquals(27, backpack.getSize()); - Assertions.assertTrue(profile.isDirty()); } @Test @@ -90,33 +83,4 @@ class TestPlayerBackpacks { Assertions.assertFalse(profile.getBackpack(500).isPresent()); } - - @Test - @DisplayName("Test loading a backpack from file") - void testLoadBackpackFromFile() throws InterruptedException { - Player player = server.addPlayer(); - PlayerProfile profile = TestUtilities.awaitProfile(player); - - profile.getConfig().setValue("backpacks.50.size", 27); - - for (int i = 0; i < 27; i++) { - profile.getConfig().setValue("backpacks.50.contents." + i, new ItemStack(Material.DIAMOND)); - } - - Optional optional = profile.getBackpack(50); - Assertions.assertTrue(optional.isPresent()); - - PlayerBackpack backpack = optional.get(); - Assertions.assertEquals(50, backpack.getId()); - Assertions.assertEquals(27, backpack.getSize()); - Assertions.assertEquals(-1, backpack.getInventory().firstEmpty()); - - backpack.getInventory().setItem(1, new ItemStack(Material.NETHER_STAR)); - - Assertions.assertEquals(new ItemStack(Material.DIAMOND), profile.getConfig().getItem("backpacks.50.contents.1")); - - // Saving should write it to the Config file - backpack.save(); - Assertions.assertEquals(new ItemStack(Material.NETHER_STAR), profile.getConfig().getItem("backpacks.50.contents.1")); - } } diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBackpackListener.java b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBackpackListener.java index 01e83b9e7..443d47c3c 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBackpackListener.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/implementation/listeners/TestBackpackListener.java @@ -13,7 +13,6 @@ import org.bukkit.entity.Player; import org.bukkit.event.inventory.ClickType; import org.bukkit.event.inventory.InventoryAction; import org.bukkit.event.inventory.InventoryClickEvent; -import org.bukkit.event.inventory.InventoryCloseEvent; import org.bukkit.event.inventory.InventoryType.SlotType; import org.bukkit.event.player.PlayerDropItemEvent; import org.bukkit.inventory.Inventory; @@ -119,7 +118,7 @@ class TestBackpackListener { Assertions.assertEquals(ChatColor.GRAY + "ID: " + player.getUniqueId() + "#" + id, item.getItemMeta().getLore().get(2)); PlayerBackpack backpack = awaitBackpack(item); - Assertions.assertEquals(player.getUniqueId(), backpack.getOwner().getUUID()); + Assertions.assertEquals(player.getUniqueId(), backpack.getOwnerId()); Assertions.assertEquals(id, backpack.getId()); } @@ -132,16 +131,6 @@ class TestBackpackListener { Assertions.assertEquals(backpack.getInventory(), view.getTopInventory()); } - @Test - @DisplayName("Test backpacks being marked dirty on close") - void testCloseBackpack() throws InterruptedException { - Player player = server.addPlayer(); - PlayerBackpack backpack = openMockBackpack(player, "TEST_CLOSE_BACKPACK", 27); - listener.onClose(new InventoryCloseEvent(player.getOpenInventory())); - - Assertions.assertTrue(backpack.getOwner().isDirty()); - } - @Test @DisplayName("Test backpacks not disturbing normal item dropping") void testBackpackDropNormalItem() throws InterruptedException { diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java b/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java new file mode 100644 index 000000000..185965999 --- /dev/null +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/storage/backend/TestLegacyBackend.java @@ -0,0 +1,383 @@ +package io.github.thebusybiscuit.slimefun4.storage.backend; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.UUID; + +import org.apache.commons.io.FileUtils; +import org.bukkit.Bukkit; +import org.bukkit.Location; +import org.bukkit.NamespacedKey; +import org.bukkit.OfflinePlayer; +import org.bukkit.World; +import org.bukkit.WorldCreator; +import org.bukkit.World.Environment; +import org.bukkit.configuration.serialization.ConfigurationSerialization; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.ItemMeta; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import be.seeseemelk.mockbukkit.MockBukkit; +import be.seeseemelk.mockbukkit.ServerMock; +import io.github.thebusybiscuit.slimefun4.api.gps.Waypoint; +import io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile; +import io.github.thebusybiscuit.slimefun4.api.researches.Research; +import io.github.thebusybiscuit.slimefun4.implementation.Slimefun; +import io.github.thebusybiscuit.slimefun4.storage.backend.legacy.LegacyStorage; +import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; +import io.github.thebusybiscuit.slimefun4.test.TestUtilities; +import net.md_5.bungee.api.ChatColor; + +class TestLegacyBackend { + + private static ServerMock server; + private static Slimefun plugin; + + @BeforeAll + public static void load() { + server = MockBukkit.mock(); + plugin = MockBukkit.load(Slimefun.class); + + File playerFolder = new File("data-storage/Slimefun/Players"); + playerFolder.mkdirs(); + File waypointFolder = new File("data-storage/Slimefun/waypoints"); + waypointFolder.mkdirs(); + + // Not too sure why this is needed, we don't use it elsewhere, it should just use the ItemStack serialization + // My guess is MockBukkit isn't loading the ConfigurationSerialization class therefore the static block + // within the class isn't being fired (where ItemStack and other classes are registered) + ConfigurationSerialization.registerClass(ItemStack.class); + ConfigurationSerialization.registerClass(ItemMeta.class); + + setupResearches(); + } + + @AfterAll + public static void unload() throws IOException { + MockBukkit.unmock(); + FileUtils.deleteDirectory(new File("data-storage")); + } + + // Test simple loading and saving of player data + @Test + void testLoadingResearches() throws IOException { + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + Files.writeString(playerFile.toPath(), """ + researches: + '0': true + '1': true + '2': true + '3': true + '4': true + '5': true + '6': true + '7': true + '8': true + '9': true + """); + + // Load the player data + LegacyStorage storage = new LegacyStorage(); + PlayerData data = storage.loadPlayerData(uuid); + + // Check if the data is correct + Assertions.assertEquals(10, data.getResearches().size()); + for (int i = 0; i < 10; i++) { + Assertions.assertTrue(data.getResearches().contains(Slimefun.getRegistry().getResearches().get(i))); + } + } + + // There's some issues with deserializing items in tests, I spent quite a while debugging this + // and didn't really get anywhere. So commenting this out for now. + /* + @Test + void testLoadingBackpacks() throws IOException { + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + Files.writeString(playerFile.toPath(), """ + backpacks: + '0': + size: 9 + contents: + '0': + ==: org.bukkit.inventory.ItemStack + v: 1 + type: IRON_BLOCK + meta: + ==: org.bukkit.inventory.meta.ItemMeta + enchants: {} + damage: 0 + persistentDataContainer: + slimefun:slimefun_item: TEST + displayName: ยง6Test block + itemFlags: !!set {} + unbreakable: false + repairCost: 0 + """); + + // Load the player data + LegacyStorage storage = new LegacyStorage(); + PlayerData data = storage.loadPlayerData(uuid); + + // Check if the data is correct + Assertions.assertEquals(1, data.getBackpacks().size()); + Assertions.assertEquals(9, data.getBackpacks().get(0).getSize()); + + // Validate item deserialization + System.out.println( + Arrays.stream(data.getBackpack(0).getInventory().getContents()) + .map((item) -> item == null ? "null" : item.getType().name()) + .collect(Collectors.joining(", ")) + ); + ItemStack stack = data.getBackpack(0).getInventory().getItem(0); + Assertions.assertNotNull(stack); + Assertions.assertEquals("IRON_BLOCK", stack.getType().name()); + Assertions.assertEquals(1, stack.getAmount()); + Assertions.assertEquals(ChatColor.GREEN + "Test block", stack.getItemMeta().getDisplayName()); + } + */ + + @Test + void testLoadingWaypoints() throws IOException { + // Create mock world + server.createWorld(WorldCreator.name("world").environment(Environment.NORMAL)); + + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File waypointFile = new File("data-storage/Slimefun/waypoints/" + uuid + ".yml"); + Files.writeString(waypointFile.toPath(), """ + TEST: + x: -173.0 + y: 75.0 + z: -11.0 + pitch: 0.0 + yaw: 178.0 + world: world + name: test + """); + + // Load the player data + LegacyStorage storage = new LegacyStorage(); + PlayerData data = storage.loadPlayerData(uuid); + + // Check if the data is correct + Assertions.assertEquals(1, data.getWaypoints().size()); + + // Validate waypoint deserialization + Waypoint waypoint = data.getWaypoints().iterator().next(); + + Assertions.assertEquals("test", waypoint.getName()); + Assertions.assertEquals(-173.0, waypoint.getLocation().getX()); + Assertions.assertEquals(75.0, waypoint.getLocation().getY()); + Assertions.assertEquals(-11.0, waypoint.getLocation().getZ()); + Assertions.assertEquals(178.0, waypoint.getLocation().getYaw()); + Assertions.assertEquals(0.0, waypoint.getLocation().getPitch()); + Assertions.assertEquals("world", waypoint.getLocation().getWorld().getName()); + } + + @Test + void testSavingResearches() throws InterruptedException { + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + + OfflinePlayer player = Bukkit.getOfflinePlayer(uuid); + + PlayerProfile profile = TestUtilities.awaitProfile(player); + + for (Research research : Slimefun.getRegistry().getResearches()) { + profile.setResearched(research, true); + } + + // Save the player data + LegacyStorage storage = new LegacyStorage(); + storage.savePlayerData(uuid, profile.getPlayerData()); + + // Assert the file exists and data is correct + Assertions.assertTrue(playerFile.exists()); + PlayerData assertion = storage.loadPlayerData(uuid); + Assertions.assertEquals(10, assertion.getResearches().size()); + for (int i = 0; i < 10; i++) { + Assertions.assertTrue(assertion.getResearches().contains(Slimefun.getRegistry().getResearches().get(i))); + } + } + + // There's some issues with deserializing items in tests, I spent quite a while debugging this + // and didn't really get anywhere. So commenting this out for now. + /* + @Test + void testSavingBackpacks() throws InterruptedException { + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + + OfflinePlayer player = Bukkit.getOfflinePlayer(uuid); + + PlayerProfile profile = TestUtilities.awaitProfile(player); + + PlayerBackpack backpack = profile.createBackpack(9); + backpack.getInventory().addItem(SlimefunItems.AIR_RUNE); + + // Save the player data + LegacyStorage storage = new LegacyStorage(); + storage.savePlayerData(uuid, profile.getPlayerData()); + + // Assert the file exists and data is correct + Assertions.assertTrue(playerFile.exists()); + PlayerData assertion = storage.loadPlayerData(uuid); + Assertions.assertEquals(1, assertion.getBackpacks().size()); + } + */ + + @Test + void testSavingWaypoints() throws InterruptedException { + // Create mock world + World world = server.createWorld(WorldCreator.name("world").environment(Environment.NORMAL)); + + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + + OfflinePlayer player = Bukkit.getOfflinePlayer(uuid); + PlayerProfile profile = TestUtilities.awaitProfile(player); + + profile.addWaypoint(new Waypoint( + player.getUniqueId(), + "test", + new Location(world, 1, 2, 3, 4, 5), + ChatColor.GREEN + "Test waypoint") + ); + + // Save the player data + LegacyStorage storage = new LegacyStorage(); + storage.savePlayerData(uuid, profile.getPlayerData()); + + // Assert the file exists and data is correct + Assertions.assertTrue(playerFile.exists()); + PlayerData assertion = storage.loadPlayerData(uuid); + Assertions.assertEquals(1, assertion.getWaypoints().size()); + + // Validate waypoint deserialization + Waypoint waypoint = assertion.getWaypoints().iterator().next(); + + Assertions.assertEquals(ChatColor.GREEN + "Test waypoint", waypoint.getName()); + Assertions.assertEquals(1, waypoint.getLocation().getX()); + Assertions.assertEquals(2, waypoint.getLocation().getY()); + Assertions.assertEquals(3, waypoint.getLocation().getZ()); + Assertions.assertEquals(4, waypoint.getLocation().getYaw()); + Assertions.assertEquals(5, waypoint.getLocation().getPitch()); + Assertions.assertEquals("world", waypoint.getLocation().getWorld().getName()); + } + + // Test realistic situations + @Test + void testResearchChanges() throws InterruptedException { + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + + OfflinePlayer player = Bukkit.getOfflinePlayer(uuid); + PlayerProfile profile = TestUtilities.awaitProfile(player); + + // Unlock all researches + for (Research research : Slimefun.getRegistry().getResearches()) { + profile.setResearched(research, true); + } + + // Save the player data + LegacyStorage storage = new LegacyStorage(); + storage.savePlayerData(uuid, profile.getPlayerData()); + + // Assert the file exists and data is correct + Assertions.assertTrue(playerFile.exists()); + PlayerData assertion = storage.loadPlayerData(uuid); + Assertions.assertEquals(10, assertion.getResearches().size()); + for (int i = 0; i < 10; i++) { + Assertions.assertTrue(assertion.getResearches().contains(Slimefun.getRegistry().getResearches().get(i))); + } + + // Now let's change the data and save it again + profile.setResearched(Slimefun.getRegistry().getResearches().get(3), false); + + // Save the player data + storage.savePlayerData(uuid, profile.getPlayerData()); + + // Assert the file exists and data is correct + Assertions.assertTrue(playerFile.exists()); + System.out.println("update assertion"); + assertion = storage.loadPlayerData(uuid); + Assertions.assertEquals(9, assertion.getResearches().size()); + for (int i = 0; i < 10; i++) { + if (i != 3) { + Assertions.assertTrue(assertion.getResearches().contains(Slimefun.getRegistry().getResearches().get(i))); + } + } + } + + // Test realistic situations - when we fix the serialization issue + // @Test + // void testBackpackChanges() throws InterruptedException {} + + @Test + void testWaypointChanges() throws InterruptedException { + // Create mock world + World world = server.createWorld(WorldCreator.name("world").environment(Environment.NORMAL)); + + // Create a player file which we can load + UUID uuid = UUID.randomUUID(); + File playerFile = new File("data-storage/Slimefun/Players/" + uuid + ".yml"); + + OfflinePlayer player = Bukkit.getOfflinePlayer(uuid); + PlayerProfile profile = TestUtilities.awaitProfile(player); + + profile.addWaypoint(new Waypoint( + player.getUniqueId(), + "test", + new Location(world, 1, 2, 3, 4, 5), + ChatColor.GREEN + "Test waypoint" + )); + + Waypoint test2 = new Waypoint( + player.getUniqueId(), + "test2", + new Location(world, 10, 20, 30, 40, 50), + ChatColor.GREEN + "Test 2 waypoint" + ); + profile.addWaypoint(test2); + + // Save the player data + LegacyStorage storage = new LegacyStorage(); + storage.savePlayerData(uuid, profile.getPlayerData()); + + // Assert the file exists and data is correct + Assertions.assertTrue(playerFile.exists()); + PlayerData assertion = storage.loadPlayerData(uuid); + Assertions.assertEquals(2, assertion.getWaypoints().size()); + + // Remove one + profile.removeWaypoint(test2); + + // Save the player data + storage.savePlayerData(uuid, profile.getPlayerData()); + + // Assert the file exists and data is correct + Assertions.assertTrue(playerFile.exists()); + assertion = storage.loadPlayerData(uuid); + Assertions.assertEquals(1, assertion.getWaypoints().size()); + } + + // Utils + private static void setupResearches() { + for (int i = 0; i < 10; i++) { + NamespacedKey key = new NamespacedKey(plugin, "test_" + i); + Research research = new Research(key, i, "Test " + i, 100); + research.register(); + } + } +} diff --git a/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java b/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java index 8afad75e1..9a7837d00 100644 --- a/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java +++ b/src/test/java/io/github/thebusybiscuit/slimefun4/test/mocks/MockProfile.java @@ -1,15 +1,22 @@ package io.github.thebusybiscuit.slimefun4.test.mocks; +import java.util.HashMap; +import java.util.Set; + import javax.annotation.Nonnull; import org.bukkit.OfflinePlayer; import io.github.thebusybiscuit.slimefun4.api.player.PlayerProfile; +import io.github.thebusybiscuit.slimefun4.storage.data.PlayerData; public class MockProfile extends PlayerProfile { public MockProfile(@Nonnull OfflinePlayer p) { - super(p); + this(p, new PlayerData(Set.of(), new HashMap<>(), Set.of())); } + public MockProfile(@Nonnull OfflinePlayer p, @Nonnull PlayerData data) { + super(p, data); + } }