Storage rewrite - Phase 1

.adr-dir

@@ -0,0 +1 @@
+docs/adr

.gitignore

@@ -5,6 +5,7 @@
 /.settings/
 /.idea/
 /.vscode/
+/data-store/
 
 dependency-reduced-pom.xml
 

docs/adr/0001-storage-layer.md

@@ -0,0 +1,120 @@
+# 1. Storage layer
+
+Date: 2023-11-15
+
+**DO NOT rely on any APIs introduced until we finish the work completely!**
+
+## Status
+
+Proposed
+
+## 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 can to 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`]() 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 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.
+
+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 and 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".
+  * **MAYBE** 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.
+  * **MAYBE** 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

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.

src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/GPSNetwork.java

@@ -331,7 +331,7 @@ public void addWaypoint(@Nonnull Player p, @Nonnull String name, @Nonnull Locati
                         }
                     }
 
-                    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);

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,16 +32,16 @@
  */
 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 ownerId
+     *            The owning {@link Player}'s {@link UUID}
      * @param id
      *            The unique id for this {@link Waypoint}
      * @param loc
@@ -48,26 +50,40 @@ public class Waypoint {
      *            The name of this {@link Waypoint}
      */
     @ParametersAreNonnullByDefault
-    public Waypoint(PlayerProfile profile, String id, Location loc, String name) {
-        Validate.notNull(profile, "Profile must never be null!");
+    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 {@link UUID} of the {@link Waypoint}.
      * 
+     * @return The corresponding owner {@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 #getUuid()} 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 +142,7 @@ public ItemStack getIcon() {
      */
     @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 +155,9 @@ public boolean equals(Object obj) {
         }
 
         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());
     }
-
 }

src/main/java/io/github/thebusybiscuit/slimefun4/api/player/PlayerBackpack.java

@@ -2,10 +2,13 @@
 
 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
      * 
@@ -49,6 +61,7 @@ public class PlayerBackpack {
      * @param id
      *            The id of this Backpack
      */
+    @Deprecated
     public PlayerBackpack(@Nonnull PlayerProfile profile, int id) {
         this(profile, id, profile.getConfig().getInt(CONFIG_PREFIX + id + ".size"));
 
@@ -67,11 +80,13 @@ public PlayerBackpack(@Nonnull PlayerProfile profile, int id) {
      * @param size
      *            The size of this Backpack
      */
+    @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();
@@ -97,9 +112,14 @@ public int getId() {
      * 
      * @return The owning {@link PlayerProfile}
      */
+    @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 +192,6 @@ public void setSize(int size) {
         }
 
         this.size = size;
-        cfg.setValue(CONFIG_PREFIX + id + ".size", size);
 
         Inventory inv = Bukkit.createInventory(null, size, "Backpack [" + size + " Slots]");
 
@@ -188,6 +207,7 @@ public void setSize(int size) {
     /**
      * This method will save the contents of this backpack to a {@link File}.
      */
+    @Deprecated
     public void save() {
         for (int i = 0; i < size; i++) {
             cfg.setValue(CONFIG_PREFIX + id + ".contents." + i, inventory.getItem(i));
@@ -198,8 +218,43 @@ public void save() {
      * This method marks the backpack dirty, it will then be queued for an autosave
      * using {@link PlayerBackpack#save()}
      */
+    @Deprecated
     public void markDirty() {
-        profile.markDirty();
+        if (profile != null) {
+            profile.markDirty();
+        }
     }
 
+    private void setContents(int size, HashMap<Integer, ItemStack> 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));
+        }
+    }
+
+    // NTS: The data loading is handled in the storage layer but we handle this specific class here
+    @ParametersAreNonnullByDefault
+    public static PlayerBackpack load(UUID ownerId, int id, int size, HashMap<Integer, ItemStack> 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;
+    }
 }