Improve error handling on data sync

* Maintain legacy snapshot IDs when updating

* Also maintain timestamps during conversion

* Actually implement timestamp fix in LegacyConverter

bukkit/build.gradle

@@ -10,7 +10,7 @@ dependencies {
     implementation 'net.kyori:adventure-platform-bukkit:4.3.0'
     implementation 'dev.triumphteam:triumph-gui:3.1.5'
     implementation 'space.arim.morepaperlib:morepaperlib:0.4.3'
-    implementation 'de.tr7zw:item-nbt-api:2.12.0-SNAPSHOT'
+    implementation 'de.tr7zw:item-nbt-api:2.12.0'
 
     compileOnly 'org.spigotmc:spigot-api:1.16.5-R0.1-SNAPSHOT'
     compileOnly 'commons-io:commons-io:2.13.0'

bukkit/src/main/java/net/william278/husksync/data/BukkitData.java

@@ -53,14 +53,8 @@ import java.util.stream.Collectors;
 public abstract class BukkitData implements Data {
 
     @Override
-    public final void apply(@NotNull UserDataHolder dataHolder, @NotNull HuskSync plugin) {
+    public final void apply(@NotNull UserDataHolder dataHolder, @NotNull HuskSync plugin) throws IllegalStateException {
-        final BukkitUser user = (BukkitUser) dataHolder;
+        this.apply((BukkitUser) dataHolder, (BukkitHuskSync) plugin);
-        try {
-            this.apply(user, (BukkitHuskSync) plugin);
-        } catch (Throwable e) {
-            plugin.log(Level.WARNING, String.format("[%s] Failed to apply %s data object; skipping",
-                    user.getUsername(), this.getClass().getSimpleName()), e);
-        }
     }
 
     public abstract void apply(@NotNull BukkitUser user, @NotNull BukkitHuskSync plugin) throws IllegalStateException;
@@ -808,11 +802,11 @@ public abstract class BukkitData implements Data {
 
         @NotNull
         public static BukkitData.Health adapt(@NotNull Player player) {
+            final double maxHealth = getMaxHealth(player);
             return from(
-                    player.getHealth(),
+                    Math.min(player.getHealth(), maxHealth),
-                    Objects.requireNonNull(player.getAttribute(Attribute.GENERIC_MAX_HEALTH),
+                    maxHealth,
-                            "Missing max health attribute").getValue(),
+                    player.isHealthScaled() ? player.getHealthScale() : 0d
-                    player.getHealthScale()
             );
         }
 
@@ -822,9 +816,10 @@ public abstract class BukkitData implements Data {
 
             // Set base max health
             final AttributeInstance maxHealthAttribute = Objects.requireNonNull(
-                    player.getAttribute(Attribute.GENERIC_MAX_HEALTH), "Missing max health attribute");
+                    player.getAttribute(Attribute.GENERIC_MAX_HEALTH), "Max health attribute was null"
+            );
             double currentMaxHealth = maxHealthAttribute.getBaseValue();
-            if (maxHealth != 0d) {
+            if (plugin.getSettings().doSynchronizeMaxHealth() && maxHealth != 0d) {
                 maxHealthAttribute.setBaseValue(maxHealth);
                 currentMaxHealth = maxHealth;
             }
@@ -853,6 +848,30 @@ public abstract class BukkitData implements Data {
             }
         }
 
+        /**
+         * Returns a {@link Player}'s maximum health, minus any health boost effects
+         *
+         * @param player The {@link Player} to get the maximum health of
+         * @return The {@link Player}'s max health
+         */
+        private static double getMaxHealth(@NotNull Player player) {
+            double maxHealth = Objects.requireNonNull(
+                    player.getAttribute(Attribute.GENERIC_MAX_HEALTH), "Max health attribute was null"
+            ).getBaseValue();
+
+            // If the player has additional health bonuses from synchronized potion effects,
+            // subtract these from this number as they are synchronized separately
+            if (player.hasPotionEffect(PotionEffectType.HEALTH_BOOST) && maxHealth > 20d) {
+                final PotionEffect healthBoost = Objects.requireNonNull(
+                        player.getPotionEffect(PotionEffectType.HEALTH_BOOST), "Health boost effect was null"
+                );
+                final double boostEffect = 4 * (healthBoost.getAmplifier() + 1);
+                maxHealth -= boostEffect;
+            }
+
+            return maxHealth;
+        }
+
         @Override
         public double getHealth() {
             return health;

bukkit/src/main/java/net/william278/husksync/util/BukkitLegacyConverter.java

@@ -39,7 +39,9 @@ import org.yaml.snakeyaml.external.biz.base64Coder.Base64Coder;
 import java.io.ByteArrayInputStream;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
+import java.time.OffsetDateTime;
 import java.util.*;
+import java.util.logging.Level;
 
 public class BukkitLegacyConverter extends LegacyConverter {
 
@@ -49,18 +51,17 @@ public class BukkitLegacyConverter extends LegacyConverter {
 
     @NotNull
     @Override
-    public DataSnapshot.Packed convert(@NotNull byte[] data) throws DataAdapter.AdaptionException {
+    public DataSnapshot.Packed convert(@NotNull byte[] data, @NotNull UUID id,
+                                       @NotNull OffsetDateTime timestamp) throws DataAdapter.AdaptionException {
         final JSONObject object = new JSONObject(plugin.getDataAdapter().bytesToString(data));
         final int version = object.getInt("format_version");
         if (version != 3) {
-            throw new DataAdapter.AdaptionException(String.format(
+            plugin.log(Level.WARNING, String.format("Converting data from older v2 data format (%s).", version));
-                    "Unsupported legacy data format version: %s. Please downgrade to an earlier version of HuskSync, " +
-                            "perform a manual legacy migration, then attempt to upgrade again.", version
-            ));
         }
 
         // Read legacy data from the JSON object
         final DataSnapshot.Builder builder = DataSnapshot.builder(plugin)
+                .id(id).timestamp(timestamp)
                 .saveCause(DataSnapshot.SaveCause.CONVERTED_FROM_V2)
                 .data(readStatusData(object));
         readInventory(object).ifPresent(builder::inventory);

common/src/main/java/net/william278/husksync/command/UserDataCommand.java

@@ -129,7 +129,7 @@ public class UserDataCommand extends Command implements TabProvider {
             case "restore" -> {
                 if (optionalUuid.isEmpty()) {
                     plugin.getLocales().getLocale("error_invalid_syntax",
-                                    "/userdata delete <username> <version_uuid>")
+                                    "/userdata restore <username> <version_uuid>")
                             .ifPresent(executor::sendMessage);
                     return;
                 }

common/src/main/java/net/william278/husksync/config/Settings.java

@@ -177,7 +177,11 @@ public class Settings {
 
     @YamlComment("(Experimental) Persist Cartography Table locked maps to let them be viewed on any server")
     @YamlKey("synchronization.persist_locked_maps")
-    private boolean persistLockedMaps = false;
+    private boolean persistLockedMaps = true;
+
+    @YamlComment("Whether to synchronize player max health (requires health syncing to be enabled)")
+    @YamlKey("synchronization.synchronize_max_health")
+    private boolean synchronizeMaxHealth = true;
 
     @YamlComment("Whether dead players who log out and log in to a different server should have their items saved. "
             + "You may need to modify this if you're using the keepInventory gamerule.")
@@ -352,6 +356,10 @@ public class Settings {
         return synchronizeDeadPlayersChangingServer;
     }
 
+    public boolean doSynchronizeMaxHealth() {
+        return synchronizeMaxHealth;
+    }
+
     public int getNetworkLatencyMilliseconds() {
         return networkLatencyMilliseconds;
     }

common/src/main/java/net/william278/husksync/data/DataSnapshot.java

@@ -45,7 +45,7 @@ public class DataSnapshot {
 
     /*
      * Current version of the snapshot data format.
-     * HuskSync v3.0 uses v4; HuskSync v2.0 uses v3. HuskSync v1.0 uses v1 or v2
+     * HuskSync v3.0 uses v4; HuskSync v2.0 uses v1-v3
      */
     protected static final int CURRENT_FORMAT_VERSION = 4;
 
@@ -96,9 +96,11 @@ public class DataSnapshot {
         return new Builder(plugin);
     }
 
+    // Deserialize a DataSnapshot downloaded from the database (with an ID & Timestamp from the database)
     @NotNull
     @ApiStatus.Internal
-    public static DataSnapshot.Packed deserialize(@NotNull HuskSync plugin, byte[] data) throws IllegalStateException {
+    public static DataSnapshot.Packed deserialize(@NotNull HuskSync plugin, byte[] data, @Nullable UUID id,
+                                                  @Nullable OffsetDateTime timestamp) throws IllegalStateException {
         final DataSnapshot.Packed snapshot = plugin.getDataAdapter().fromBytes(data, DataSnapshot.Packed.class);
         if (snapshot.getMinecraftVersion().compareTo(plugin.getMinecraftVersion()) > 0) {
             throw new IllegalStateException(String.format("Cannot set data for user because the Minecraft version of " +
@@ -114,7 +116,11 @@ public class DataSnapshot {
         }
         if (snapshot.getFormatVersion() < CURRENT_FORMAT_VERSION) {
             if (plugin.getLegacyConverter().isPresent()) {
-                return plugin.getLegacyConverter().get().convert(data);
+                return plugin.getLegacyConverter().get().convert(
+                        data,
+                        Objects.requireNonNull(id, "Attempted legacy conversion with null UUID!"),
+                        Objects.requireNonNull(timestamp, "Attempted legacy conversion with null timestamp!")
+                );
             }
             throw new IllegalStateException(String.format(
                     "No legacy converter to convert format version: %s", snapshot.getFormatVersion()
@@ -129,6 +135,13 @@ public class DataSnapshot {
         return snapshot;
     }
 
+    // Deserialize a DataSnapshot from a network message payload (without an ID)
+    @NotNull
+    @ApiStatus.Internal
+    public static DataSnapshot.Packed deserialize(@NotNull HuskSync plugin, byte[] data) throws IllegalStateException {
+        return deserialize(plugin, data, null, null);
+    }
+
     /**
      * Return the ID of the snapshot
      *
@@ -227,12 +240,6 @@ public class DataSnapshot {
 
     /**
      * Get the format version of the snapshot (indicating the version of HuskSync that created it)
-     * <ul>
-     *     <li>1: HuskSync v1.0+</li>
-     *     <li>2: HuskSync v1.5+</li>
-     *     <li>3: HuskSync v2.0+</li>
-     *     <li>4: HuskSync v3.0+</li>
-     * </ul>
      *
      * @return The format version of the snapshot
      * @since 3.0
@@ -393,6 +400,7 @@ public class DataSnapshot {
     public static class Builder {
 
         private final HuskSync plugin;
+        private UUID id;
         private SaveCause saveCause;
         private boolean pinned;
         private OffsetDateTime timestamp;
@@ -403,6 +411,19 @@ public class DataSnapshot {
             this.pinned = false;
             this.data = new HashMap<>();
             this.timestamp = OffsetDateTime.now();
+            this.id = UUID.randomUUID();
+        }
+
+        /**
+         * Set the {@link UUID unique ID} of the snapshot
+         *
+         * @param id The {@link UUID} of the snapshot
+         * @return The builder
+         */
+        @NotNull
+        public Builder id(@NotNull UUID id) {
+            this.id = id;
+            return this;
         }
 
         /**
@@ -661,7 +682,7 @@ public class DataSnapshot {
                 throw new IllegalStateException("Cannot build DataSnapshot without a save cause");
             }
             return new Unpacked(
-                    UUID.randomUUID(),
+                    id,
                     pinned || plugin.getSettings().doAutoPin(saveCause),
                     timestamp,
                     saveCause,

common/src/main/java/net/william278/husksync/data/UserDataHolder.java

@@ -26,6 +26,7 @@ import org.jetbrains.annotations.NotNull;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.logging.Level;
 
 /**
  * A holder of data in the form of {@link Data}s, which can be synced
@@ -83,22 +84,40 @@ public interface UserDataHolder extends DataHolder {
      * The {@code runAfter} callback function will be run after the snapshot has been applied.
      *
      * @param snapshot the snapshot to apply
-     * @param runAfter the function to run asynchronously after the snapshot has been applied
+     * @param runAfter a consumer accepting a boolean value, indicating if the data was successfully applied,
+     *                 which will be run after the snapshot has been applied
      * @since 3.0
      */
-    default void applySnapshot(@NotNull DataSnapshot.Packed snapshot, @NotNull ThrowingConsumer<UserDataHolder> runAfter) {
+    default void applySnapshot(@NotNull DataSnapshot.Packed snapshot, @NotNull ThrowingConsumer<Boolean> runAfter) {
         final HuskSync plugin = getPlugin();
-        final DataSnapshot.Unpacked unpacked = snapshot.unpack(plugin);
+
+        // Unpack the snapshot
+        final DataSnapshot.Unpacked unpacked;
+        try {
+            unpacked = snapshot.unpack(plugin);
+        } catch (Throwable e) {
+            plugin.log(Level.SEVERE, String.format("Failed to unpack data snapshot for %s", getUsername()), e);
+            return;
+        }
+
+        // Synchronously attempt to apply the snapshot
         plugin.runSync(() -> {
-            unpacked.getData().forEach((type, data) -> {
+            try {
-                if (plugin.getSettings().isSyncFeatureEnabled(type)) {
+                for (Map.Entry<Identifier, Data> entry : unpacked.getData().entrySet()) {
-                    if (type.isCustom()) {
+                    final Identifier identifier = entry.getKey();
-                        getCustomDataStore().put(type, data);
+                    if (plugin.getSettings().isSyncFeatureEnabled(identifier)) {
+                        if (identifier.isCustom()) {
+                            getCustomDataStore().put(identifier, entry.getValue());
+                        }
+                        entry.getValue().apply(this, plugin);
                     }
-                    data.apply(this, plugin);
                 }
-            });
+            } catch (Throwable e) {
-            plugin.runAsync(() -> runAfter.accept(this));
+                plugin.log(Level.SEVERE, String.format("Failed to apply data snapshot to %s", getUsername()), e);
+                plugin.runAsync(() -> runAfter.accept(false));
+                return;
+            }
+            plugin.runAsync(() -> runAfter.accept(true));
         });
     }
 
@@ -157,6 +176,9 @@ public interface UserDataHolder extends DataHolder {
         this.setData(Identifier.PERSISTENT_DATA, persistentData);
     }
 
+    @NotNull
+    String getUsername();
+
     @NotNull
     Map<Identifier, Data> getCustomDataStore();
 

common/src/main/java/net/william278/husksync/database/MySqlDatabase.java

@@ -217,7 +217,7 @@ public class MySqlDatabase extends Database {
     public Optional<DataSnapshot.Packed> getLatestSnapshot(@NotNull User user) {
         try (Connection connection = getConnection()) {
             try (PreparedStatement statement = connection.prepareStatement(formatStatementTables("""
-                    SELECT `version_uuid`, `timestamp`, `save_cause`, `pinned`, `data`
+                    SELECT `version_uuid`, `timestamp`, `data`
                     FROM `%user_data_table%`
                     WHERE `player_uuid`=?
                     ORDER BY `timestamp` DESC
@@ -225,10 +225,14 @@ public class MySqlDatabase extends Database {
                 statement.setString(1, user.getUuid().toString());
                 final ResultSet resultSet = statement.executeQuery();
                 if (resultSet.next()) {
+                    final UUID versionUuid = UUID.fromString(resultSet.getString("version_uuid"));
+                    final OffsetDateTime timestamp = OffsetDateTime.ofInstant(
+                            resultSet.getTimestamp("timestamp").toInstant(), TimeZone.getDefault().toZoneId()
+                    );
                     final Blob blob = resultSet.getBlob("data");
                     final byte[] dataByteArray = blob.getBytes(1, (int) blob.length());
                     blob.free();
-                    return Optional.of(DataSnapshot.deserialize(plugin, dataByteArray));
+                    return Optional.of(DataSnapshot.deserialize(plugin, dataByteArray, versionUuid, timestamp));
                 }
             }
         } catch (SQLException | DataAdapter.AdaptionException e) {
@@ -244,17 +248,21 @@ public class MySqlDatabase extends Database {
         final List<DataSnapshot.Packed> retrievedData = new ArrayList<>();
         try (Connection connection = getConnection()) {
             try (PreparedStatement statement = connection.prepareStatement(formatStatementTables("""
-                    SELECT `version_uuid`, `timestamp`, `save_cause`, `pinned`, `data`
+                    SELECT `version_uuid`, `timestamp`,  `data`
                     FROM `%user_data_table%`
                     WHERE `player_uuid`=?
                     ORDER BY `timestamp` DESC;"""))) {
                 statement.setString(1, user.getUuid().toString());
                 final ResultSet resultSet = statement.executeQuery();
                 while (resultSet.next()) {
+                    final UUID versionUuid = UUID.fromString(resultSet.getString("version_uuid"));
+                    final OffsetDateTime timestamp = OffsetDateTime.ofInstant(
+                            resultSet.getTimestamp("timestamp").toInstant(), TimeZone.getDefault().toZoneId()
+                    );
                     final Blob blob = resultSet.getBlob("data");
                     final byte[] dataByteArray = blob.getBytes(1, (int) blob.length());
                     blob.free();
-                    retrievedData.add(DataSnapshot.deserialize(plugin, dataByteArray));
+                    retrievedData.add(DataSnapshot.deserialize(plugin, dataByteArray, versionUuid, timestamp));
                 }
                 return retrievedData;
             }
@@ -269,7 +277,7 @@ public class MySqlDatabase extends Database {
     public Optional<DataSnapshot.Packed> getSnapshot(@NotNull User user, @NotNull UUID versionUuid) {
         try (Connection connection = getConnection()) {
             try (PreparedStatement statement = connection.prepareStatement(formatStatementTables("""
-                    SELECT `version_uuid`, `timestamp`, `save_cause`, `pinned`, `data`
+                    SELECT `version_uuid`, `timestamp`,  `data`
                     FROM `%user_data_table%`
                     WHERE `player_uuid`=? AND `version_uuid`=?
                     ORDER BY `timestamp` DESC
@@ -279,9 +287,12 @@ public class MySqlDatabase extends Database {
                 final ResultSet resultSet = statement.executeQuery();
                 if (resultSet.next()) {
                     final Blob blob = resultSet.getBlob("data");
+                    final OffsetDateTime timestamp = OffsetDateTime.ofInstant(
+                            resultSet.getTimestamp("timestamp").toInstant(), TimeZone.getDefault().toZoneId()
+                    );
                     final byte[] dataByteArray = blob.getBytes(1, (int) blob.length());
                     blob.free();
-                    return Optional.of(DataSnapshot.deserialize(plugin, dataByteArray));
+                    return Optional.of(DataSnapshot.deserialize(plugin, dataByteArray, versionUuid, timestamp));
                 }
             }
         } catch (SQLException | DataAdapter.AdaptionException e) {

common/src/main/java/net/william278/husksync/user/OnlineUser.java

@@ -125,12 +125,14 @@ public abstract class OnlineUser extends User implements CommandUser, UserDataHo
      * Set a player's status from a {@link DataSnapshot}
      *
      * @param snapshot The {@link DataSnapshot} to set the player's status from
+     * @param cause    The {@link DataSnapshot.UpdateCause} of the snapshot
+     * @since 3.0
      */
     public void applySnapshot(@NotNull DataSnapshot.Packed snapshot, @NotNull DataSnapshot.UpdateCause cause) {
         getPlugin().fireEvent(getPlugin().getPreSyncEvent(this, snapshot), (event) -> {
             if (!isOffline()) {
                 UserDataHolder.super.applySnapshot(
-                        event.getData(), (owner) -> completeSync(true, cause, getPlugin())
+                        event.getData(), (succeeded) -> completeSync(succeeded, cause, getPlugin())
                 );
             }
         });

common/src/main/java/net/william278/husksync/util/LegacyConverter.java

@@ -24,6 +24,9 @@ import net.william278.husksync.adapter.DataAdapter;
 import net.william278.husksync.data.DataSnapshot;
 import org.jetbrains.annotations.NotNull;
 
+import java.time.OffsetDateTime;
+import java.util.UUID;
+
 public abstract class LegacyConverter {
 
     protected final HuskSync plugin;
@@ -33,6 +36,7 @@ public abstract class LegacyConverter {
     }
 
     @NotNull
-    public abstract DataSnapshot.Packed convert(@NotNull byte[] data) throws DataAdapter.AdaptionException;
+    public abstract DataSnapshot.Packed convert(@NotNull byte[] data, @NotNull UUID id,
+                                                @NotNull OffsetDateTime timestamp) throws DataAdapter.AdaptionException;
 
 }

docs/Config-File.md

@@ -77,7 +77,9 @@ synchronization:
   # Where to display sync notifications (ACTION_BAR, CHAT, TOAST or NONE)
   notification_display_slot: ACTION_BAR
   # (Experimental) Persist Cartography Table locked maps to let them be viewed on any server
-  persist_locked_maps: false
+  persist_locked_maps: true
+  # Whether to synchronize player max health (requires health syncing to be enabled)
+  synchronize_max_health: true
   # Whether dead players who log out and log in to a different server should have their items saved. You may need to modify this if you're using the keepInventory gamerule.
   synchronize_dead_players_changing_server: true
   # How long, in milliseconds, this server should wait for a response from the redis server before pulling data from the database instead (i.e., if the user did not change servers).

gradle.properties

@@ -3,7 +3,7 @@ org.gradle.jvmargs='-Dfile.encoding=UTF-8'
 org.gradle.daemon=true
 javaVersion=16
 
-plugin_version=3.0
+plugin_version=3.0.1
 plugin_archive=husksync
 plugin_description=A modern, cross-server player data synchronization system