Skip to content

Commit 4d710fa

Browse files
authored
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
1 parent dad6020 commit 4d710fa

File tree

18 files changed

+975
-167
lines changed

18 files changed

+975
-167
lines changed

Diff for: .adr-dir

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
docs/adr

Diff for: .gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
/.settings/
66
/.idea/
77
/.vscode/
8+
/data-store/
89

910
dependency-reduced-pom.xml
1011

Diff for: docs/adr/0001-storage-layer.md

+129
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
# 1. Storage layer
2+
3+
Date: 2023-11-15
4+
Last update: 2023-12-27
5+
6+
**DO NOT rely on any APIs introduced until we finish the work completely!**
7+
8+
## Status
9+
10+
Work in progress
11+
12+
## Context
13+
14+
Slimefun has been around for a very long time and due to that, the way we
15+
wrote persistence of data has also been around for a very long time.
16+
While Slimefun has grown, the storage layer has never been adapted.
17+
This means that even all these years later, it's using the same old saving/loading.
18+
This isn't necessarily always bad, however, as Slimefun has grown both in terms of content
19+
and the servers using it - we've seen some issues.
20+
21+
Today, files are saved as YAML files (sometimes with just a JSON object per line),
22+
which is good for a config format but not good for a data store. It can create very large files
23+
that can get corrupted, the way we've been saving data often means loading it all at once as well
24+
rather than lazy-loading and generally isn't very performant.
25+
26+
For a long time we've been talking about rewriting our data storage in multiple forms
27+
(you may have seen this referenced for "BlockStorage rewrite" or "SQL for PlayerProfiles", etc.).
28+
Now is the time we start to do this, this will be a very large change and will not be done quickly or rushed.
29+
30+
This ADR talks about the future of our data persistence.
31+
32+
## Decision
33+
34+
We want to create a new storage layer abstraction and implementations
35+
which will be backwards-compatible but open up new ways of storing data
36+
within Slimefun. The end end goal is we can quickly and easily support
37+
new storage backends (such as binary storage, SQL, etc.) for things like
38+
[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.
39+
40+
We also want to be generally more efficient in the way we save and load data.
41+
Today, we load way more than is required.
42+
We can improve memory usage by only loading what we need, when we need it.
43+
44+
We will do this incrementally and at first, in an experimental context.
45+
In that regard, we should aim to minimise the blast radius and lift as much
46+
as possible.
47+
48+
### Quick changes overview
49+
50+
* New abstraction over storage to easily support multiple backends.
51+
* Work towards moving away from the legacy YAML based storage.
52+
* Lazy load and save data to more efficiently handle the data life cycle.
53+
54+
### Implementation details
55+
56+
There is a new interface called [`Storage`](TBD) which is what all storage
57+
backends will implement.
58+
This will have methods for loading and saving things like
59+
[`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).
60+
61+
Then, backends will implement these
62+
(e.g. [`LegacyStorageBackend`](TBD) (today's YAML situation))
63+
in order to support these functions.
64+
Not all storage backends are required support each data type.
65+
e.g. SQL may not support [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java).
66+
67+
68+
## Addons
69+
70+
The goal is that Addons will be able to use and implement new storage backends
71+
if they wish and also be extended so they can load/save things as they wish.
72+
73+
The first few iterations will not focus on Addon support. We want to ensure
74+
this new storage layer will work and supports what we need it to today.
75+
76+
This ADR will be updated when we get to supporting Addons properly.
77+
78+
## Considerations
79+
80+
This will be a big change therefore we will be doing it as incrementally as
81+
possible.
82+
Changes will be tested while in the PR stage and merged into the Dev releases when possible.
83+
We may do an experimental release if required.
84+
85+
Phases do not (and very likely will not) be done within a single PR. They will also not have any timeframe attached to them.
86+
87+
The current plan looks like this:
88+
89+
* 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).
90+
* We want to load player data using the new storage layer with the current
91+
data system.
92+
* We'll want to monitor for any possible issues and generally refine
93+
how this system should look
94+
* 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).
95+
* Create a new backend for binary storage
96+
* Implement in an experimental capacity and allow users to opt-in
97+
* Provide a warning that this is **experimental** and there will be bugs.
98+
* Implement new metric for storage backend being used
99+
* 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).
100+
* Mark it as stable and remove the warnings once we're sure things are
101+
working correctly
102+
* Create a migration path for users currently using "legacy".
103+
* Enable by default for new servers
104+
* Phase 4 - Move [`BlockStorage`](https://github.com/Slimefun/Slimefun4/blob/bbfb9734b9f549d7e82291eff041f9b666a61b63/src/main/java/me/mrCookieSlime/Slimefun/api/BlockStorage.java) to new storage layer.
105+
* The big one! We're gonna tackle adding this to BlockStorage.
106+
This will probably be a large change and we'll want to be as
107+
careful as possible here.
108+
* Implement `legacy` and `binary` as experimental storage backends
109+
for BlockStorage and allow users to opt-in
110+
* Provide a warning that this is **experimental** and there will be bugs.
111+
* 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).
112+
* Mark it as stable and remove the warnings once we're sure things are
113+
working correctly
114+
* Ensure migration path works here too.
115+
* Enable by default for new servers
116+
* Phase 6 - Finish up and move anything else we want over
117+
* Move over any other data stores we have to the new layer
118+
* We should probably still do experimental -> stable but it should have
119+
less of a lead time.
120+
121+
## State of work
122+
123+
* Phase 1: In progress
124+
* https://github.com/Slimefun/Slimefun4/pull/4065
125+
* Phase 2: Not started
126+
* Phase 3: Not started
127+
* Phase 4: Not started
128+
* Phase 5: Not started
129+
* Phase 6: Not started

Diff for: docs/adr/README.md

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# ADR
2+
3+
An ADR (Architecture Decision Record) is a document describing large changes, why we made them, etc.
4+
5+
## Making a new ADR
6+
7+
If you're making a large change to Slimefun, we recommend creating an ADR
8+
in order to document why this is being made and how it works for future contributors.
9+
10+
Please follow the general format of the former ADRs or use a tool
11+
such as [`adr-tools`](https://github.com/npryce/adr-tools) to generate a new document.

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ public void addWaypoint(@Nonnull Player p, @Nonnull String name, @Nonnull Locati
331331
}
332332
}
333333

334-
profile.addWaypoint(new Waypoint(profile, id, event.getLocation(), event.getName()));
334+
profile.addWaypoint(new Waypoint(p.getUniqueId(), id, event.getLocation(), event.getName()));
335335

336336
SoundEffect.GPS_NETWORK_ADD_WAYPOINT.playFor(p);
337337
Slimefun.getLocalization().sendMessage(p, "gps.waypoint.added", true);

Diff for: src/main/java/io/github/thebusybiscuit/slimefun4/api/gps/Waypoint.java

+47-9
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package io.github.thebusybiscuit.slimefun4.api.gps;
22

33
import java.util.Objects;
4+
import java.util.UUID;
45

56
import javax.annotation.Nonnull;
67
import javax.annotation.ParametersAreNonnullByDefault;
78

89
import org.apache.commons.lang.Validate;
10+
import org.bukkit.Bukkit;
911
import org.bukkit.Location;
1012
import org.bukkit.World.Environment;
1113
import org.bukkit.entity.Player;
@@ -30,14 +32,14 @@
3032
*/
3133
public class Waypoint {
3234

33-
private final PlayerProfile profile;
35+
private final UUID ownerId;
3436
private final String id;
3537
private final String name;
3638
private final Location location;
3739

3840
/**
3941
* This constructs a new {@link Waypoint} object.
40-
*
42+
*
4143
* @param profile
4244
* The owning {@link PlayerProfile}
4345
* @param id
@@ -46,28 +48,62 @@ public class Waypoint {
4648
* The {@link Location} of the {@link Waypoint}
4749
* @param name
4850
* The name of this {@link Waypoint}
51+
*
52+
* @deprecated Use {@link #Waypoint(UUID, String, Location, String)} instead
4953
*/
54+
@Deprecated
5055
@ParametersAreNonnullByDefault
5156
public Waypoint(PlayerProfile profile, String id, Location loc, String name) {
52-
Validate.notNull(profile, "Profile must never be null!");
57+
this(profile.getUUID(), id, loc, name);
58+
}
59+
60+
/**
61+
* This constructs a new {@link Waypoint} object.
62+
*
63+
* @param ownerId
64+
* The owning {@link Player}'s {@link UUID}
65+
* @param id
66+
* The unique id for this {@link Waypoint}
67+
* @param loc
68+
* The {@link Location} of the {@link Waypoint}
69+
* @param name
70+
* The name of this {@link Waypoint}
71+
*/
72+
@ParametersAreNonnullByDefault
73+
public Waypoint(UUID ownerId, String id, Location loc, String name) {
74+
Validate.notNull(ownerId, "owner ID must never be null!");
5375
Validate.notNull(id, "id must never be null!");
5476
Validate.notNull(loc, "Location must never be null!");
5577
Validate.notNull(name, "Name must never be null!");
5678

57-
this.profile = profile;
79+
this.ownerId = ownerId;
5880
this.id = id;
5981
this.location = loc;
6082
this.name = name;
6183
}
6284

6385
/**
64-
* This returns the owner of the {@link Waypoint}.
86+
* This returns the owner's {@link UUID} of the {@link Waypoint}.
6587
*
88+
* @return The corresponding owner's {@link UUID}
89+
*/
90+
@Nonnull
91+
public UUID getOwnerId() {
92+
return this.ownerId;
93+
}
94+
95+
/**
96+
* This returns the owner of the {@link Waypoint}.
97+
*
6698
* @return The corresponding {@link PlayerProfile}
99+
*
100+
* @deprecated Use {@link #getOwnerId()} instead
67101
*/
68102
@Nonnull
103+
@Deprecated
69104
public PlayerProfile getOwner() {
70-
return profile;
105+
// This is jank and should never actually return null
106+
return PlayerProfile.find(Bukkit.getOfflinePlayer(ownerId)).orElse(null);
71107
}
72108

73109
/**
@@ -126,7 +162,7 @@ public ItemStack getIcon() {
126162
*/
127163
@Override
128164
public int hashCode() {
129-
return Objects.hash(profile.getUUID(), id, name, location);
165+
return Objects.hash(this.ownerId, this.id, this.name, this.location);
130166
}
131167

132168
/**
@@ -139,7 +175,9 @@ public boolean equals(Object obj) {
139175
}
140176

141177
Waypoint waypoint = (Waypoint) obj;
142-
return profile.getUUID().equals(waypoint.getOwner().getUUID()) && id.equals(waypoint.getId()) && location.equals(waypoint.getLocation()) && name.equals(waypoint.getName());
178+
return this.ownerId.equals(waypoint.getOwnerId())
179+
&& id.equals(waypoint.getId())
180+
&& location.equals(waypoint.getLocation())
181+
&& name.equals(waypoint.getName());
143182
}
144-
145183
}

Diff for: src/main/java/io/github/thebusybiscuit/slimefun4/api/items/SlimefunItem.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -1160,11 +1160,11 @@ public final int hashCode() {
11601160
}
11611161

11621162
/**
1163-
* Retrieve a {@link Optional}<{@link SlimefunItem}> by its id.
1163+
* Retrieve a {@link Optional} {@link SlimefunItem} by its id.
11641164
*
11651165
* @param id
11661166
* The id of the {@link SlimefunItem}
1167-
* @return The {@link Optional}<{@link SlimefunItem}> associated with that id. Empty if non-existent
1167+
* @return The {@link Optional} {@link SlimefunItem} associated with that id. Empty if non-existent
11681168
*/
11691169
public static @Nonnull Optional<SlimefunItem> getOptionalById(@Nonnull String id) {
11701170
return Optional.ofNullable(getById(id));
@@ -1193,11 +1193,11 @@ public final int hashCode() {
11931193
}
11941194

11951195
/**
1196-
* Retrieve a {@link Optional}<{@link SlimefunItem}> from an {@link ItemStack}.
1196+
* Retrieve a {@link Optional} {@link SlimefunItem} from an {@link ItemStack}.
11971197
*
11981198
* @param item
11991199
* The {@link ItemStack} to check
1200-
* @return The {@link Optional}<{@link SlimefunItem}> associated with this {@link ItemStack} if present, otherwise empty
1200+
* @return The {@link Optional} {@link SlimefunItem} associated with this {@link ItemStack} if present, otherwise empty
12011201
*/
12021202
public @Nonnull Optional<SlimefunItem> getOptionalByItem(@Nullable ItemStack item) {
12031203
return Optional.ofNullable(getByItem(item));

0 commit comments

Comments
 (0)