Refactor config handling to only serialise and deserialise on load an…

…d save
Multiverse · Feb 26, 2025 · 54ebdcb · 54ebdcb
1 parent 1a3baf2
commit 54ebdcb
Show file tree

Hide file tree

Showing 10 changed files with 112 additions and 112 deletions.
diff --git a/...main/java/org/mvplugins/multiverse/core/configuration/handle/BaseConfigurationHandle.java b/...main/java/org/mvplugins/multiverse/core/configuration/handle/BaseConfigurationHandle.java
@@ -1,6 +1,8 @@
 package org.mvplugins.multiverse.core.configuration.handle;
 
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.logging.Logger;
 
 import com.dumptruckman.minecraft.util.Logging;
@@ -13,6 +15,7 @@
 
 import org.mvplugins.multiverse.core.configuration.migration.ConfigMigrator;
 import org.mvplugins.multiverse.core.configuration.node.ListValueNode;
+import org.mvplugins.multiverse.core.configuration.node.Node;
 import org.mvplugins.multiverse.core.configuration.node.NodeGroup;
 import org.mvplugins.multiverse.core.configuration.node.ValueNode;
 
@@ -26,6 +29,7 @@ public abstract class BaseConfigurationHandle<C extends ConfigurationSection> {
     protected final @Nullable Logger logger;
     protected final @NotNull NodeGroup nodes;
     protected final @Nullable ConfigMigrator migrator;
+    protected final @NotNull Map<Node, Object> nodeValueMap;
 
     protected C config;
 
@@ -36,6 +40,7 @@ protected BaseConfigurationHandle(
         this.logger = logger;
         this.nodes = nodes;
         this.migrator = migrator;
+        this.nodeValueMap = new HashMap<>(nodes.size());
     }
 
     /**
@@ -63,31 +68,72 @@ protected void migrateConfig() {
      * Sets up the nodes.
      */
     protected void setUpNodes() {
+        nodeValueMap.clear();
         if (nodes == null || nodes.isEmpty()) {
             return;
         }
 
         nodes.forEach(node -> {
             if (node instanceof ValueNode valueNode) {
-                set(valueNode, get(valueNode));
+                var value = deserializeNodeFromConfig(valueNode);
+                nodeValueMap.put(valueNode, value);
+                valueNode.onSetValue(value, value);
             }
         });
     }
 
+    protected <T> T deserializeNodeFromConfig(ValueNode<T> node) {
+        if (node.getSerializer() == null) {
+            return Option.of(config.getObject(node.getPath(), node.getType())).getOrElse(node::getDefaultValue);
+        }
+        return Try.of(() -> {
+                    var value = config.get(node.getPath());
+                    if (value == null) {
+                        return node.getDefaultValue();
+                    }
+                    return node.getSerializer().deserialize(value, node.getType());
+                }).flatMap(value -> node.validate(value).map(ignore -> value))
+                .onFailure(e -> Logging.warning("Failed to deserialize node %s: %s", node.getPath(), e.getMessage()))
+                .getOrElse(node::getDefaultValue);
+    }
+
+    /**
+     * Saves the configuration.
+     */
+    public Try<Void> save() {
+        return Try.run(() -> nodes.forEach(node -> {
+            if (!(node instanceof ValueNode valueNode)) {
+                return;
+            }
+            serializeNodeToConfig(valueNode);
+        }));
+    }
+
+    protected void serializeNodeToConfig(ValueNode node) {
+        var value = nodeValueMap.get(node);
+        if (value == null) {
+            value = node.getDefaultValue();
+        }
+        if (node.getSerializer() != null) {
+            var serialized = node.getSerializer().serialize(value, node.getType());
+            config.set(node.getPath(), serialized);
+        } else {
+            config.set(node.getPath(), value);
+        }
+    }
+
+    public boolean isLoaded() {
+        return !nodeValueMap.isEmpty();
+    }
+
     /**
      * Gets the value of a node, if the node has a default value, it will be returned if the node is not found.
      *
      * @param node The node to get the value of.
      * @return The value of the node.
      */
     public <T> T get(@NotNull ValueNode<T> node) {
-        if (node.getSerializer() == null) {
-            return Option.of(config.getObject(node.getPath(), node.getType())).getOrElse(node::getDefaultValue);
-        }
-        return Try.of(() -> node.getSerializer()
-                        .deserialize(config.get(node.getPath(), node.getDefaultValue()), node.getType()))
-                .onFailure(e -> Logging.warning("Failed to deserialize node %s: %s", node.getPath(), e.getMessage()))
-                .getOrElse(node::getDefaultValue);
+        return (T) nodeValueMap.get(node);
     }
 
     /**
@@ -101,12 +147,7 @@ public <T> T get(@NotNull ValueNode<T> node) {
     public <T> Try<Void> set(@NotNull ValueNode<T> node, T value) {
         return node.validate(value).map(ignore -> {
             T oldValue = get(node);
-            if (node.getSerializer() != null) {
-                var serialized = node.getSerializer().serialize(value, node.getType());
-                config.set(node.getPath(), serialized);
-            } else {
-                config.set(node.getPath(), value);
-            }
+            nodeValueMap.put(node, value);
             node.onSetValue(oldValue, get(node));
             return null;
         });
@@ -122,15 +163,8 @@ public <T> Try<Void> set(@NotNull ValueNode<T> node, T value) {
      */
     public <I> Try<Void> add(@NotNull ListValueNode<I> node, I itemValue) {
         return node.validateItem(itemValue).map(ignore -> {
-            var serialized = node.getItemSerializer() != null
-                    ? node.getItemSerializer().serialize(itemValue, node.getItemType())
-                    : itemValue;
-            List valueList = config.getList(node.getPath());
-            if (valueList == null) {
-                throw new IllegalArgumentException("Cannot add item to non-list node");
-            }
-            valueList.add(serialized);
-            config.set(node.getPath(), valueList);
+            List<I> list = get(node);
+            list.add(itemValue);
             node.onSetItemValue(null, itemValue);
             return null;
         });
@@ -146,17 +180,10 @@ public <I> Try<Void> add(@NotNull ListValueNode<I> node, I itemValue) {
      */
     public <I> Try<Void> remove(@NotNull ListValueNode<I> node, I itemValue) {
         return node.validateItem(itemValue).map(ignore -> {
-            var serialized = node.getItemSerializer() != null
-                    ? node.getItemSerializer().serialize(itemValue, node.getItemType())
-                    : itemValue;
-            List valueList = config.getList(node.getPath());
-            if (valueList == null) {
-                throw new IllegalArgumentException("Cannot remove item from non-list node");
-            }
-            if (!valueList.remove(serialized)) {
-                throw new IllegalArgumentException("Cannot remove item from list node");
+            List<I> list = get(node);
+            if (!list.remove(itemValue)) {
+                throw new IllegalArgumentException("Cannot remove item as it is already not in the list!");
             }
-            config.set(node.getPath(), valueList);
             node.onSetItemValue(itemValue, null);
             return null;
         });
@@ -170,7 +197,7 @@ public <I> Try<Void> remove(@NotNull ListValueNode<I> node, I itemValue) {
      * @return Empty try if the value was set, try containing an error otherwise.
      */
     public <T> Try<Void> reset(@NotNull ValueNode<T> node) {
-        return Try.run(() -> set(node, node.getDefaultValue()));
+        return set(node, node.getDefaultValue());
     }
 
     /**

diff --git a/...java/org/mvplugins/multiverse/core/configuration/handle/CommentedConfigurationHandle.java b/...java/org/mvplugins/multiverse/core/configuration/handle/CommentedConfigurationHandle.java
@@ -5,7 +5,6 @@
 import java.util.logging.Logger;
 
 import io.github.townyadvanced.commentedconfiguration.CommentedConfiguration;
-import io.vavr.control.Option;
 import io.vavr.control.Try;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -55,41 +54,23 @@ protected void loadConfigObject() throws IOException {
      * {@inheritDoc}
      */
     @Override
-    protected void setUpNodes() {
-        if (nodes == null || nodes.isEmpty()) {
-            return;
-        }
-
-        CommentedConfiguration oldConfig = config;
-        this.config = new CommentedConfiguration(configPath, logger);
-
-        nodes.forEach(node -> {
-            if (node instanceof CommentedNode typedNode) {
-                if (typedNode.getComments().length > 0) {
-                    config.addComment(typedNode.getPath(), typedNode.getComments());
+    public Try<Void> save() {
+        return Try.run(() -> {
+            config = new CommentedConfiguration(configPath, logger);
+            nodes.forEach(node -> {
+                if (node instanceof CommentedNode typedNode) {
+                    if (typedNode.getComments().length > 0) {
+                        config.addComment(typedNode.getPath(), typedNode.getComments());
+                    }
+                }
+                if (node instanceof ValueNode valueNode) {
+                    serializeNodeToConfig(valueNode);
                 }
-            }
-            if (node instanceof ValueNode valueNode) {
-                //noinspection unchecked
-                Option.of(oldConfig.get(valueNode.getPath()))
-                        .peek(oldValue -> {
-                            this.config.set(valueNode.getPath(), oldValue);
-                            set(valueNode, get(valueNode));
-                        })
-                        .onEmpty(() -> reset(valueNode));
-            }
+            });
+            config.save();
         });
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    @Override
-    public Try<Void> save() {
-        // TODO: There is no way to check if the save was successful.
-        return Try.run(() -> config.save());
-    }
-
     /**
      * Builder for {@link CommentedConfigurationHandle}.
      */

diff --git a/...main/java/org/mvplugins/multiverse/core/configuration/handle/FileConfigurationHandle.java b/...main/java/org/mvplugins/multiverse/core/configuration/handle/FileConfigurationHandle.java
@@ -90,20 +90,6 @@ protected Try<Void> createConfigFile() {
      */
     protected abstract void loadConfigObject() throws IOException, InvalidConfigurationException;
 
-    /**
-     * Saves the configuration.
-     */
-    public abstract Try<Void> save();
-
-    /**
-     * Checks if the configuration is loaded.
-     *
-     * @return True if the configuration is loaded, false otherwise.
-     */
-    public boolean isLoaded() {
-        return config != null;
-    }
-
     /**
      * Abstract builder for {@link FileConfigurationHandle}.
      *

diff --git a/...in/java/org/mvplugins/multiverse/core/configuration/handle/MemoryConfigurationHandle.java b/...in/java/org/mvplugins/multiverse/core/configuration/handle/MemoryConfigurationHandle.java
@@ -1,8 +1,10 @@
 package org.mvplugins.multiverse.core.configuration.handle;
 
 import io.vavr.control.Option;
+import io.vavr.control.Try;
 import org.bukkit.configuration.ConfigurationSection;
 import org.bukkit.configuration.MemoryConfiguration;
+import org.bukkit.configuration.MemorySection;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.mvplugins.multiverse.core.configuration.migration.ConfigMigrator;
@@ -34,26 +36,9 @@ protected MemoryConfigurationHandle(
     }
 
     @Override
-    protected void setUpNodes() {
-        if (nodes == null || nodes.isEmpty()) {
-            return;
-        }
-
-        ConfigurationSection oldConfig = config;
-        config = new MemoryConfiguration();
-
-        nodes.forEach(node -> {
-            if (!(node instanceof ValueNode valueNode)) {
-                return;
-            }
-            //todo: this is a copied from CommentedConfigurationHandle
-            Option.of(oldConfig.get(valueNode.getPath()))
-                    .peek(oldValue -> {
-                        this.config.set(valueNode.getPath(), oldValue);
-                        set(valueNode, get(valueNode));
-                    })
-                    .onEmpty(() -> reset(valueNode));
-        });
+    public Try<Void> save() {
+        return Try.run(() -> config = new MemoryConfiguration())
+                .flatMap(ignore -> super.save());
     }
 
     /**

diff --git a/...main/java/org/mvplugins/multiverse/core/configuration/handle/YamlConfigurationHandle.java b/...main/java/org/mvplugins/multiverse/core/configuration/handle/YamlConfigurationHandle.java
@@ -51,7 +51,9 @@ protected void loadConfigObject() throws IOException, InvalidConfigurationExcept
      */
     @Override
     public Try<Void> save() {
-        return Try.run(() -> config.save(configFile));
+        return Try.run(() -> config = new YamlConfiguration())
+                .flatMap(ignore -> super.save())
+                .andThenTry(ignore -> config.save(configFile));
     }
 
     /**

diff --git a/src/main/java/org/mvplugins/multiverse/core/configuration/node/ConfigHeaderNode.java b/src/main/java/org/mvplugins/multiverse/core/configuration/node/ConfigHeaderNode.java
@@ -45,6 +45,13 @@ protected ConfigHeaderNode(@NotNull String path, @NotNull String[] comments) {
         return comments;
     }
 
+    @Override
+    public String toString() {
+        return "Node{" +
+                "path='" + path + '\'' +
+                '}';
+    }
+
     public static class Builder<B extends Builder<B>> {
 
         protected final @NotNull String path;

diff --git a/src/main/java/org/mvplugins/multiverse/core/configuration/node/ListConfigNode.java b/src/main/java/org/mvplugins/multiverse/core/configuration/node/ListConfigNode.java
@@ -9,6 +9,7 @@
 import java.util.function.BiConsumer;
 import java.util.function.Function;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 
 import io.vavr.Value;
 import io.vavr.control.Try;
@@ -141,9 +142,9 @@ private void setDefaultSerialiser() {
             public List<I> deserialize(Object object, Class<List<I>> type) {
                 if (object instanceof List list) {
                     //noinspection unchecked
-                    return list.stream()
+                    return (List<I>) list.stream()
                             .map(item -> itemSerializer != null ? itemSerializer.deserialize(item, itemType) : item)
-                            .toList();
+                            .collect(Collectors.toList());
                 }
                 //todo: Maybe assume object is the first element of the list
                 return new ArrayList<>();
@@ -156,7 +157,7 @@ public Object serialize(List<I> object, Class<List<I>> type) {
                 }
                 return object.stream()
                         .map(item -> itemSerializer != null ? itemSerializer.serialize(item, itemType) : item)
-                        .toList();
+                        .collect(Collectors.toList());
             }
         };
     }

diff --git a/src/main/java/org/mvplugins/multiverse/core/world/WorldConfig.java b/src/main/java/org/mvplugins/multiverse/core/world/WorldConfig.java
@@ -123,6 +123,14 @@ Try<Void> load(ConfigurationSection section) {
         return configHandle.load(section);
     }
 
+    Try<Void> save() {
+        return configHandle.save();
+    }
+
+    ConfigurationSection getConfigurationSection() {
+        return configHandle.getConfig();
+    }
+
     StringPropertyHandle getStringPropertyHandle() {
         return stringPropertyHandle;
     }
@@ -405,10 +413,6 @@ void deferenceMVWorld() {
         configNodes.setWorld(null);
     }
 
-    ConfigurationSection getConfigurationSection() {
-        return configHandle.getConfig();
-    }
-
     /**
      * Migrates the entry fee settings. Assumes entry fee is disabled if currency is not set.
      */

diff --git a/src/main/java/org/mvplugins/multiverse/core/world/WorldManager.java b/src/main/java/org/mvplugins/multiverse/core/world/WorldManager.java
@@ -302,6 +302,7 @@ private LoadedMultiverseWorld newLoadedMultiverseWorld(
         worldConfig.setAdjustSpawn(adjustSpawn);
         worldConfig.setGenerator(generator == null ? "" : generator);
         worldConfig.setBiome(biome == null ? "" : biome);
+        worldConfig.save();
 
         MultiverseWorld mvWorld = newMultiverseWorld(world.getName(), worldConfig);
         LoadedMultiverseWorld loadedWorld = new LoadedMultiverseWorld(