Improve world config management with parsing of new and removed worlds on reload

src/main/java/com/onarandombox/MultiverseCore/MultiverseCore.java

@@ -30,6 +30,7 @@ import io.vavr.control.Try;
 import jakarta.inject.Inject;
 import jakarta.inject.Provider;
 import me.main__.util.SerializationConfig.SerializationConfig;
+import org.bukkit.Bukkit;
 import org.bukkit.configuration.serialization.ConfigurationSerialization;
 import org.bukkit.plugin.PluginDescriptionFile;
 import org.bukkit.plugin.java.JavaPlugin;
@@ -115,7 +116,10 @@ public class MultiverseCore extends JavaPlugin implements MVCore {
         Logging.setShowingConfig(shouldShowConfig());
 
         // Initialize the worlds
-        worldManagerProvider.get().initAllWorlds();
+        worldManagerProvider.get().initAllWorlds().onFailure(e -> {
+            Logging.severe("Failed to initialize worlds");
+            e.printStackTrace();
+        });
 
         // Setup economy here so vault is loaded
         loadEconomist();

src/main/java/com/onarandombox/MultiverseCore/commands/ReloadCommand.java

@@ -52,7 +52,7 @@ public class ReloadCommand extends MultiverseCommand {
         try {
             // TODO: Make this all Try<Void>
             this.config.load().getOrElseThrow(e -> new RuntimeException("Failed to load config", e));
-            this.worldManager.initAllWorlds();
+            this.worldManager.initAllWorlds().getOrElseThrow(e -> new RuntimeException("Failed to init worlds", e));
             this.anchorManager.loadAnchors();
         } catch (Exception e) {
             e.printStackTrace();

src/main/java/com/onarandombox/MultiverseCore/worldnew/WorldManager.java

@@ -93,37 +93,40 @@ public class WorldManager {
 
     /**
      * Loads all worlds from the worlds config.
+     *
+     * @return The result of the load.
      */
-    public void initAllWorlds() {
-        if (!populateWorldFromConfig()) {
-            return;
-        }
-        loadDefaultWorlds();
-        autoLoadWorlds();
-        saveWorldsConfig();
+    public Try<Void> initAllWorlds() {
+        return populateWorldFromConfig().andThenTry(() -> {
+            loadDefaultWorlds();
+            autoLoadWorlds();
+            saveWorldsConfig();
+        });
     }
 
     /**
-     * Generate worlds from the worlds config.
+     * Populate world map from the worlds.yml config.
+     *
+     * @return The result of the world map population.
      */
-    private boolean populateWorldFromConfig() {
-        Try<Void> load = worldsConfigManager.load();
-        if (load.isFailure()) {
-            Logging.severe("Failed to load worlds config: " + load.getCause().getMessage());
-            load.getCause().printStackTrace();
-            return false;
-        }
-        worldsConfigManager.getAllWorldConfigs().forEach(worldConfig -> {
-            getLoadedWorld(worldConfig.getWorldName())
-                    .peek(loadedWorld -> loadedWorld.setWorldConfig(worldConfig));
-            getWorld(worldConfig.getWorldName())
+    private Try<Void> populateWorldFromConfig() {
+        return worldsConfigManager.load().mapTry(result -> {
+            var newWorldConfigs = result._1();
+            var removedWorlds = result._2();
+
+            newWorldConfigs.forEach(worldConfig -> getWorld(worldConfig.getWorldName())
                     .peek(unloadedWorld -> unloadedWorld.setWorldConfig(worldConfig))
                     .onEmpty(() -> {
                         MultiverseWorld mvWorld = new MultiverseWorld(worldConfig.getWorldName(), worldConfig);
                         worldsMap.put(mvWorld.getName(), mvWorld);
-                    });
+                    }));
+
+            removedWorlds.forEach(worldName -> removeWorld(worldName)
+                    .onFailure(failure -> Logging.severe("Failed to unload world %s: %s", worldName, failure))
+                    .onSuccess(success -> Logging.fine("Unloaded world %s as it was removed from config", worldName)));
+
+            return null;
         });
-        return true;
     }
 
     /**
@@ -361,7 +364,7 @@ public class WorldManager {
                                     replace("{world}").with(world.getName()));
                         },
                         mvWorld -> {
-                            Logging.fine("Removed MVWorld from map: " + world.getName());
+                            Logging.fine("Removed MultiverseWorld from map: " + world.getName());
                             mvWorld.getWorldConfig().deferenceMVWorld();
                             return Result.success(UnloadWorldResult.Success.UNLOADED,
                                     replace("{world}").with(world.getName()));

src/main/java/com/onarandombox/MultiverseCore/worldnew/config/WorldConfig.java

@@ -38,6 +38,10 @@ public final class WorldConfig {
         return configHandle.load();
     }
 
+    public Try<Void> load(ConfigurationSection section) {
+        return configHandle.load(section);
+    }
+
     public String getWorldName() {
         return worldName;
     }

src/main/java/com/onarandombox/MultiverseCore/worldnew/config/WorldsConfigManager.java

@@ -1,22 +1,30 @@
 package com.onarandombox.MultiverseCore.worldnew.config;
 
 import com.onarandombox.MultiverseCore.MultiverseCore;
+import io.vavr.Tuple2;
 import io.vavr.control.Try;
 import jakarta.inject.Inject;
 import org.bukkit.configuration.ConfigurationSection;
+import org.bukkit.configuration.InvalidConfigurationException;
 import org.bukkit.configuration.file.YamlConfiguration;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.jvnet.hk2.annotations.Service;
 
 import java.io.File;
-import java.util.Collection;
+import java.io.IOException;
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+/**
+ * Manages the worlds.yml file.
+ */
 @Service
 public final class WorldsConfigManager {
-    private static final String CONFIG_FILENAME = "worlds2.yml";
+    private static final String CONFIG_FILENAME = "worlds2.yml"; // TODO: Rename to worlds.yml
 
     private final Map<String, WorldConfig> worldConfigMap;
     private final File worldConfigFile;
@@ -28,46 +36,88 @@ public final class WorldsConfigManager {
         worldConfigFile = core.getDataFolder().toPath().resolve(CONFIG_FILENAME).toFile();
     }
 
-    public Try<Void> load() {
-        worldConfigMap.clear();
-
-        return Try.run(() -> {
-            if (!worldConfigFile.exists()) {
-                worldConfigFile.createNewFile();
-            }
-            worldsConfig = new YamlConfiguration();
-            worldsConfig.load(worldConfigFile);
-        }).andThenTry(() -> {
-            for (String worldName : getAllWorldsInConfig()) {
-                worldConfigMap.put(worldName, new WorldConfig(worldName, getWorldConfigSection(worldName)));
-            }
-        }).onFailure(e -> {
-            worldsConfig = null;
-            worldConfigMap.clear();
+    /**
+     * Loads the worlds.yml file and creates a WorldConfig for each world in the file if it doesn't already exist.
+     *
+     * @return A tuple containing a list of the new WorldConfigs added and a list of the worlds removed from the config.
+     */
+    public Try<Tuple2<List<WorldConfig>, List<String>>> load() {
+        return Try.of(() -> {
+            loadWorldYmlFile();
+            return parseNewAndRemovedWorlds();
         });
     }
 
+    private void loadWorldYmlFile() throws IOException, InvalidConfigurationException {
+        if (!worldConfigFile.exists() && !worldConfigFile.createNewFile()) {
+            throw new IllegalStateException("Could not create worlds.yml config file");
+        }
+
+        worldsConfig = new YamlConfiguration();
+        worldsConfig.load(worldConfigFile);
+    }
+
+    private Tuple2<List<WorldConfig>, List<String>> parseNewAndRemovedWorlds() {
+        Set<String> allWorldsInConfig = worldsConfig.getKeys(false);
+        List<WorldConfig> newWorldsAdded = new ArrayList<>();
+
+        for (String worldName : allWorldsInConfig) {
+            WorldConfig worldConfig = getWorldConfig(worldName);
+            if (worldConfig == null) {
+                WorldConfig newWorldConfig = new WorldConfig(worldName, getWorldConfigSection(worldName));
+                worldConfigMap.put(worldName, newWorldConfig);
+                newWorldsAdded.add(newWorldConfig);
+            } else {
+                worldConfig.load(getWorldConfigSection(worldName));
+            }
+        }
+
+        List<String> worldsRemoved = worldConfigMap.keySet().stream()
+                .filter(worldName -> !allWorldsInConfig.contains(worldName))
+                .toList();
+
+        for (String s : worldsRemoved) {
+            worldConfigMap.remove(s);
+        }
+
+        return new Tuple2<>(newWorldsAdded, worldsRemoved);
+    }
+
+    /**
+     * Whether the worlds.yml file has been loaded.
+     *
+     * @return Whether the worlds.yml file has been loaded.
+     */
     public boolean isLoaded() {
         return worldsConfig != null;
     }
 
+    /**
+     * Saves the worlds.yml file.
+     *
+     * @return Whether the save was successful or the error that occurred.
+     */
     public Try<Void> save() {
         return Try.run(() -> worldsConfig.save(worldConfigFile));
     }
 
-    public Set<String> getAllWorldsInConfig() {
-        return worldsConfig.getKeys(false);
-    }
-
-    public Collection<WorldConfig> getAllWorldConfigs() {
-        return worldConfigMap.values();
-    }
-
-    public WorldConfig getWorldConfig(String worldName) {
+    /**
+     * Gets the {@link WorldConfig} instance of all worlds in the worlds.yml file.
+     *
+     * @param worldName The name of the world to check.
+     * @return Whether the worlds.yml file contains the given world.
+     */
+    public @Nullable WorldConfig getWorldConfig(@NotNull String worldName) {
         return worldConfigMap.get(worldName);
     }
 
-    public WorldConfig addWorldConfig(String worldName) {
+    /**
+     * Add a new world to the worlds.yml file. If a world with the given name already exists, an exception is thrown.
+     *
+     * @param worldName The name of the world to add.
+     * @return The newly created {@link WorldConfig} instance.
+     */
+    public @NotNull WorldConfig addWorldConfig(@NotNull String worldName) {
         if (worldConfigMap.containsKey(worldName)) {
             throw new IllegalArgumentException("WorldConfig for world " + worldName + " already exists.");
         }
@@ -76,7 +126,12 @@ public final class WorldsConfigManager {
         return worldConfig;
     }
 
-    public void deleteWorldConfig(String worldName) {
+    /**
+     * Deletes the world config for the given world.
+     *
+     * @param worldName The name of the world to delete.
+     */
+    public void deleteWorldConfig(@NotNull String worldName) {
         worldConfigMap.remove(worldName);
         worldsConfig.set(worldName, null);
     }

src/test/java/org/mvplugins/multiverse/core/world/WorldConfigMangerTest.kt

@@ -48,6 +48,8 @@ class WorldConfigMangerTest : TestWithMockBukkit() {
     @Test
     fun `Updating existing world properties`() {
         val worldConfig = worldConfigManager.getWorldConfig("world")
+        assertNotNull(worldConfig)
+
         worldConfig.setProperty("adjust-spawn", true)
         worldConfig.setProperty("alias", "newalias")
         worldConfig.setProperty("spawn-location", SpawnLocation(-64.0, 64.0, 48.0))

src/test/java/org/mvplugins/multiverse/core/world/WorldConfigTest.kt

@@ -29,7 +29,8 @@ class WorldConfigTest : TestWithMockBukkit() {
             throw IllegalStateException("WorldsConfigManager is not available as a service") }
 
         assertTrue(worldConfigManager.load().isSuccess)
-        worldConfig = worldConfigManager.getWorldConfig("world")
+        worldConfig = worldConfigManager.getWorldConfig("world").takeIf { it != null } ?: run {
+            throw IllegalStateException("WorldConfig for world is not available") }
         assertNotNull(worldConfig);
     }