From 8847483ff8c2c64e4be657d3edf1d2c9f653d2ae Mon Sep 17 00:00:00 2001 From: William Date: Thu, 21 Jul 2022 14:23:27 +0100 Subject: [PATCH] Correct Persistent Data serialization --- bukkit/build.gradle | 2 + .../data/BukkitPersistentDataTagType.java | 60 -------- .../husksync/player/BukkitPlayer.java | 141 +++++++++++++----- .../data/BukkitPersistentDataTagType.java | 35 +++++ .../data/PersistentDataContainerData.java | 25 +++- .../husksync/data/PersistentDataTag.java | 16 +- .../husksync/data/DataAdaptionTests.java | 28 ++++ 7 files changed, 204 insertions(+), 103 deletions(-) delete mode 100644 bukkit/src/main/java/net/william278/husksync/data/BukkitPersistentDataTagType.java create mode 100644 common/src/main/java/net/william278/husksync/data/BukkitPersistentDataTagType.java diff --git a/bukkit/build.gradle b/bukkit/build.gradle index 8ee4277c..8c0f8c28 100644 --- a/bukkit/build.gradle +++ b/bukkit/build.gradle @@ -10,6 +10,8 @@ dependencies { compileOnly 'dev.dejvokep:boosted-yaml:1.2' compileOnly 'org.spigotmc:spigot-api:1.16.5-R0.1-SNAPSHOT' compileOnly 'com.zaxxer:HikariCP:5.0.1' + + testImplementation 'org.spigotmc:spigot-api:1.16.5-R0.1-SNAPSHOT' } shadowJar { diff --git a/bukkit/src/main/java/net/william278/husksync/data/BukkitPersistentDataTagType.java b/bukkit/src/main/java/net/william278/husksync/data/BukkitPersistentDataTagType.java deleted file mode 100644 index 9f4d3b51..00000000 --- a/bukkit/src/main/java/net/william278/husksync/data/BukkitPersistentDataTagType.java +++ /dev/null @@ -1,60 +0,0 @@ -package net.william278.husksync.data; - -import org.bukkit.NamespacedKey; -import org.bukkit.persistence.PersistentDataContainer; -import org.bukkit.persistence.PersistentDataType; -import org.jetbrains.annotations.NotNull; - -import java.util.Optional; - -/** - * Represents the type of persistent data tag, implemented by a Bukkit PersistentDataType. - */ -public enum BukkitPersistentDataTagType { - - BYTE(PersistentDataType.BYTE), - SHORT(PersistentDataType.SHORT), - INTEGER(PersistentDataType.INTEGER), - LONG(PersistentDataType.LONG), - FLOAT(PersistentDataType.FLOAT), - DOUBLE(PersistentDataType.DOUBLE), - STRING(PersistentDataType.STRING), - BYTE_ARRAY(PersistentDataType.BYTE_ARRAY), - INTEGER_ARRAY(PersistentDataType.INTEGER_ARRAY), - LONG_ARRAY(PersistentDataType.LONG_ARRAY), - TAG_CONTAINER_ARRAY(PersistentDataType.TAG_CONTAINER_ARRAY), - TAG_CONTAINER(PersistentDataType.TAG_CONTAINER); - - public final PersistentDataType dataType; - - BukkitPersistentDataTagType(PersistentDataType persistentDataType) { - this.dataType = persistentDataType; - } - - public static Optional getDataType(@NotNull String typeName) { - for (BukkitPersistentDataTagType type : values()) { - if (type.name().equalsIgnoreCase(typeName)) { - return Optional.of(type); - } - } - return Optional.empty(); - } - - /** - * Determine the {@link BukkitPersistentDataTagType} of a tag in a {@link PersistentDataContainer}. - * - * @param container The {@link PersistentDataContainer} to check. - * @param key The {@link NamespacedKey} of the tag to check. - * @return The {@link BukkitPersistentDataTagType} of the key, or {@link Optional#empty()} if the key does not exist. - */ - public static Optional getKeyDataType(@NotNull PersistentDataContainer container, - @NotNull NamespacedKey key) { - for (BukkitPersistentDataTagType dataType : values()) { - if (container.has(key, dataType.dataType)) { - return Optional.of(dataType); - } - } - return Optional.empty(); - } - -} diff --git a/bukkit/src/main/java/net/william278/husksync/player/BukkitPlayer.java b/bukkit/src/main/java/net/william278/husksync/player/BukkitPlayer.java index 7d56559d..05582fdf 100644 --- a/bukkit/src/main/java/net/william278/husksync/player/BukkitPlayer.java +++ b/bukkit/src/main/java/net/william278/husksync/player/BukkitPlayer.java @@ -32,6 +32,20 @@ import java.util.logging.Level; */ public class BukkitPlayer extends OnlineUser { + private static final PersistentDataType[] PRIMITIVE_PERSISTENT_DATA_TYPES = new PersistentDataType[]{ + PersistentDataType.BYTE, + PersistentDataType.SHORT, + PersistentDataType.INTEGER, + PersistentDataType.LONG, + PersistentDataType.FLOAT, + PersistentDataType.DOUBLE, + PersistentDataType.STRING, + PersistentDataType.BYTE_ARRAY, + PersistentDataType.INTEGER_ARRAY, + PersistentDataType.LONG_ARRAY, + PersistentDataType.TAG_CONTAINER_ARRAY, + PersistentDataType.TAG_CONTAINER}; + private final Player player; private BukkitPlayer(@NotNull Player player) { @@ -401,15 +415,56 @@ public class BukkitPlayer extends OnlineUser { if (container.isEmpty()) { return new PersistentDataContainerData(new HashMap<>()); } - final HashMap persistentDataMap = new HashMap<>(); - // Set persistent data keys; ignore keys that we cannot synchronise as byte arrays + final HashMap> persistentDataMap = new HashMap<>(); for (final NamespacedKey key : container.getKeys()) { - BukkitPersistentDataTagType.getKeyDataType(container, key).ifPresent(dataType -> { - final Object value = container.get(key, dataType.dataType); - if (value != null) { - persistentDataMap.put(key.toString(), new PersistentDataTag(dataType.name(), value)); + PersistentDataType type = null; + for (PersistentDataType dataType : PRIMITIVE_PERSISTENT_DATA_TYPES) { + if (container.has(key, dataType)) { + type = dataType; + break; } - }); + } + if (type != null) { + // This is absolutely disgusting code and needs to be swiftly put out of its misery with a refactor + final Class primitiveType = type.getPrimitiveType(); + if (String.class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.STRING, + Objects.requireNonNull(container.get(key, PersistentDataType.STRING)))); + } else if (int.class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.INTEGER, + Objects.requireNonNull(container.get(key, PersistentDataType.INTEGER)))); + } else if (double.class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.DOUBLE, + Objects.requireNonNull(container.get(key, PersistentDataType.DOUBLE)))); + } else if (float.class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.FLOAT, + Objects.requireNonNull(container.get(key, PersistentDataType.FLOAT)))); + } else if (long.class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.LONG, + Objects.requireNonNull(container.get(key, PersistentDataType.LONG)))); + } else if (short.class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.SHORT, + Objects.requireNonNull(container.get(key, PersistentDataType.SHORT)))); + } else if (byte.class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.BYTE, + Objects.requireNonNull(container.get(key, PersistentDataType.BYTE)))); + } else if (byte[].class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.BYTE_ARRAY, + Objects.requireNonNull(container.get(key, PersistentDataType.BYTE_ARRAY)))); + } else if (int[].class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.INTEGER_ARRAY, + Objects.requireNonNull(container.get(key, PersistentDataType.INTEGER_ARRAY)))); + } else if (long[].class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.LONG_ARRAY, + Objects.requireNonNull(container.get(key, PersistentDataType.LONG_ARRAY)))); + } else if (PersistentDataContainer.class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.TAG_CONTAINER, + Objects.requireNonNull(container.get(key, PersistentDataType.TAG_CONTAINER)))); + } else if (PersistentDataContainer[].class.equals(primitiveType)) { + persistentDataMap.put(key.toString(), new PersistentDataTag<>(BukkitPersistentDataTagType.TAG_CONTAINER_ARRAY, + Objects.requireNonNull(container.get(key, PersistentDataType.TAG_CONTAINER_ARRAY)))); + } + } } return new PersistentDataContainerData(persistentDataMap); }).exceptionally(throwable -> { @@ -425,41 +480,57 @@ public class BukkitPlayer extends OnlineUser { return CompletableFuture.runAsync(() -> { player.getPersistentDataContainer().getKeys().forEach(namespacedKey -> player.getPersistentDataContainer().remove(namespacedKey)); - final Map dataMap = persistentDataContainerData.persistentDataMap; - dataMap.keySet().forEach(keyString -> { + persistentDataContainerData.getTags().forEach(keyString -> { final NamespacedKey key = NamespacedKey.fromString(keyString); if (key != null) { // Set a tag with the given key and value. This is crying out for a refactor. - BukkitPersistentDataTagType.getDataType(dataMap.get(keyString).type).ifPresentOrElse(dataType -> { + persistentDataContainerData.getTagType(keyString).ifPresentOrElse(dataType -> { switch (dataType) { - case BYTE -> player.getPersistentDataContainer().set(key, - PersistentDataType.BYTE, (byte) dataMap.get(keyString).value); - case SHORT -> player.getPersistentDataContainer().set(key, - PersistentDataType.SHORT, (short) dataMap.get(keyString).value); - case INTEGER -> player.getPersistentDataContainer().set(key, - PersistentDataType.INTEGER, (int) dataMap.get(keyString).value); - case LONG -> player.getPersistentDataContainer().set(key, - PersistentDataType.LONG, (long) dataMap.get(keyString).value); - case FLOAT -> player.getPersistentDataContainer().set(key, - PersistentDataType.FLOAT, (float) dataMap.get(keyString).value); - case DOUBLE -> player.getPersistentDataContainer().set(key, - PersistentDataType.DOUBLE, (double) dataMap.get(keyString).value); - case STRING -> player.getPersistentDataContainer().set(key, - PersistentDataType.STRING, (String) dataMap.get(keyString).value); - case BYTE_ARRAY -> player.getPersistentDataContainer().set(key, - PersistentDataType.BYTE_ARRAY, (byte[]) dataMap.get(keyString).value); - case INTEGER_ARRAY -> player.getPersistentDataContainer().set(key, - PersistentDataType.INTEGER_ARRAY, (int[]) dataMap.get(keyString).value); - case LONG_ARRAY -> player.getPersistentDataContainer().set(key, - PersistentDataType.LONG_ARRAY, (long[]) dataMap.get(keyString).value); - case TAG_CONTAINER -> player.getPersistentDataContainer().set(key, - PersistentDataType.TAG_CONTAINER, (PersistentDataContainer) dataMap.get(keyString).value); - case TAG_CONTAINER_ARRAY -> player.getPersistentDataContainer().set(key, - PersistentDataType.TAG_CONTAINER_ARRAY, (PersistentDataContainer[]) dataMap.get(keyString).value); + case BYTE -> persistentDataContainerData.getTagValue(keyString, byte.class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.BYTE, value)); + case SHORT -> persistentDataContainerData.getTagValue(keyString, short.class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.SHORT, value)); + case INTEGER -> persistentDataContainerData.getTagValue(keyString, int.class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.INTEGER, value)); + case LONG -> persistentDataContainerData.getTagValue(keyString, long.class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.LONG, value)); + case FLOAT -> persistentDataContainerData.getTagValue(keyString, float.class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.FLOAT, value)); + case DOUBLE -> persistentDataContainerData.getTagValue(keyString, double.class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.DOUBLE, value)); + case STRING -> persistentDataContainerData.getTagValue(keyString, String.class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.STRING, value)); + case BYTE_ARRAY -> + persistentDataContainerData.getTagValue(keyString, byte[].class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.BYTE_ARRAY, value)); + case INTEGER_ARRAY -> + persistentDataContainerData.getTagValue(keyString, int[].class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.INTEGER_ARRAY, value)); + case LONG_ARRAY -> + persistentDataContainerData.getTagValue(keyString, long[].class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.LONG_ARRAY, value)); + case TAG_CONTAINER -> + persistentDataContainerData.getTagValue(keyString, PersistentDataContainer.class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.TAG_CONTAINER, value)); + case TAG_CONTAINER_ARRAY -> + persistentDataContainerData.getTagValue(keyString, PersistentDataContainer[].class).ifPresent( + value -> player.getPersistentDataContainer().set(key, + PersistentDataType.TAG_CONTAINER_ARRAY, value)); } }, () -> BukkitHuskSync.getInstance().getLoggingAdapter().log(Level.WARNING, "Could not set " + player.getName() + "'s persistent data key " + keyString + - " with the invalid type, " + dataMap.get(keyString).type + ". Skipping!")); + " as it has an invalid type. Skipping!")); } }); }).exceptionally(throwable -> { diff --git a/common/src/main/java/net/william278/husksync/data/BukkitPersistentDataTagType.java b/common/src/main/java/net/william278/husksync/data/BukkitPersistentDataTagType.java new file mode 100644 index 00000000..b6c30848 --- /dev/null +++ b/common/src/main/java/net/william278/husksync/data/BukkitPersistentDataTagType.java @@ -0,0 +1,35 @@ +package net.william278.husksync.data; + +import org.jetbrains.annotations.NotNull; + +import java.util.Optional; + +/** + * Represents the type of a {@link PersistentDataTag} + */ +public enum BukkitPersistentDataTagType { + + BYTE, + SHORT, + INTEGER, + LONG, + FLOAT, + DOUBLE, + STRING, + BYTE_ARRAY, + INTEGER_ARRAY, + LONG_ARRAY, + TAG_CONTAINER_ARRAY, + TAG_CONTAINER; + + + public static Optional getDataType(@NotNull String typeName) { + for (BukkitPersistentDataTagType type : values()) { + if (type.name().equalsIgnoreCase(typeName)) { + return Optional.of(type); + } + } + return Optional.empty(); + } + +} diff --git a/common/src/main/java/net/william278/husksync/data/PersistentDataContainerData.java b/common/src/main/java/net/william278/husksync/data/PersistentDataContainerData.java index 64760781..1458ee0d 100644 --- a/common/src/main/java/net/william278/husksync/data/PersistentDataContainerData.java +++ b/common/src/main/java/net/william278/husksync/data/PersistentDataContainerData.java @@ -4,6 +4,8 @@ import com.google.gson.annotations.SerializedName; import org.jetbrains.annotations.NotNull; import java.util.Map; +import java.util.Optional; +import java.util.Set; /** * Store's a user's persistent data container, holding a map of plugin-set persistent values @@ -14,9 +16,9 @@ public class PersistentDataContainerData { * Map of namespaced key strings to a byte array representing the persistent data */ @SerializedName("persistent_data_map") - public Map persistentDataMap; + protected Map> persistentDataMap; - public PersistentDataContainerData(@NotNull final Map persistentDataMap) { + public PersistentDataContainerData(@NotNull final Map> persistentDataMap) { this.persistentDataMap = persistentDataMap; } @@ -24,4 +26,23 @@ public class PersistentDataContainerData { protected PersistentDataContainerData() { } + + public Optional getTagValue(@NotNull final String tagName, @NotNull Class tagClass) { + if (persistentDataMap.containsKey(tagName)) { + return Optional.of(tagClass.cast(persistentDataMap.get(tagName).value)); + } + return Optional.empty(); + } + + public Optional getTagType(@NotNull final String tagType) { + if (persistentDataMap.containsKey(tagType)) { + return BukkitPersistentDataTagType.getDataType(persistentDataMap.get(tagType).type); + } + return Optional.empty(); + } + + public Set getTags() { + return persistentDataMap.keySet(); + } + } diff --git a/common/src/main/java/net/william278/husksync/data/PersistentDataTag.java b/common/src/main/java/net/william278/husksync/data/PersistentDataTag.java index dd7d2b8b..a4222637 100644 --- a/common/src/main/java/net/william278/husksync/data/PersistentDataTag.java +++ b/common/src/main/java/net/william278/husksync/data/PersistentDataTag.java @@ -2,29 +2,33 @@ package net.william278.husksync.data; import org.jetbrains.annotations.NotNull; -import java.util.Objects; +import java.util.Optional; /** * Represents a persistent data tag set by a plugin. */ -public class PersistentDataTag { +public class PersistentDataTag { /** * The enumerated primitive data type name value of the tag */ - public String type; + protected String type; /** * The value of the tag */ - public Object value; + public T value; - public PersistentDataTag(@NotNull String type, @NotNull Object value) { - this.type = type; + public PersistentDataTag(@NotNull BukkitPersistentDataTagType type, @NotNull T value) { + this.type = type.name(); this.value = value; } private PersistentDataTag() { } + public Optional getType() { + return BukkitPersistentDataTagType.getDataType(type); + } + } diff --git a/common/src/test/java/net/william278/husksync/data/DataAdaptionTests.java b/common/src/test/java/net/william278/husksync/data/DataAdaptionTests.java index e80e0b45..df29df66 100644 --- a/common/src/test/java/net/william278/husksync/data/DataAdaptionTests.java +++ b/common/src/test/java/net/william278/husksync/data/DataAdaptionTests.java @@ -3,13 +3,19 @@ package net.william278.husksync.data; import net.william278.husksync.logger.DummyLogger; import net.william278.husksync.player.DummyPlayer; import net.william278.husksync.player.OnlineUser; +import net.william278.husksync.player.User; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import static java.util.Map.*; + /** * Tests for the data system {@link DataAdapter} */ @@ -82,4 +88,26 @@ public class DataAdaptionTests { Assertions.assertTrue(isEquals.get()); } + private String getTestSerializedPersistentDataContainer() { + final HashMap> persistentDataTest = new HashMap<>(); + persistentDataTest.put("husksync:byte_test", new PersistentDataTag<>(BukkitPersistentDataTagType.BYTE, 0x01)); + persistentDataTest.put("husksync:double_test", new PersistentDataTag<>(BukkitPersistentDataTagType.DOUBLE, 2d)); + persistentDataTest.put("husksync:string_test", new PersistentDataTag<>(BukkitPersistentDataTagType.STRING, "test")); + persistentDataTest.put("husksync:int_test", new PersistentDataTag<>(BukkitPersistentDataTagType.INTEGER, 3)); + persistentDataTest.put("husksync:long_test", new PersistentDataTag<>(BukkitPersistentDataTagType.LONG, 4L)); + persistentDataTest.put("husksync:float_test", new PersistentDataTag<>(BukkitPersistentDataTagType.FLOAT, 5f)); + persistentDataTest.put("husksync:short_test", new PersistentDataTag<>(BukkitPersistentDataTagType.SHORT, 6)); + final PersistentDataContainerData persistentDataContainerData = new PersistentDataContainerData(persistentDataTest); + + final DataAdapter dataAdapter = new JsonDataAdapter(); + UserData userData = new UserData(); + userData.persistentDataContainerData = persistentDataContainerData; + return dataAdapter.toJson(userData, false); + } + + @Test + public void testPersistentDataContainerSerialization() { + Assertions.assertEquals(getTestSerializedPersistentDataContainer(), "{\"persistent_data_container\":{\"persistent_data_map\":{\"husksync:int_test\":{\"type\":\"INTEGER\",\"value\":3},\"husksync:string_test\":{\"type\":\"STRING\",\"value\":\"test\"},\"husksync:long_test\":{\"type\":\"LONG\",\"value\":4},\"husksync:byte_test\":{\"type\":\"BYTE\",\"value\":1},\"husksync:short_test\":{\"type\":\"SHORT\",\"value\":6},\"husksync:double_test\":{\"type\":\"DOUBLE\",\"value\":2.0},\"husksync:float_test\":{\"type\":\"FLOAT\",\"value\":5.0}}},\"format_version\":0}"); + } + }