fix: /bmreload now reverts to previous config on failure (#1034)

Author: confuser

common/src/main/java/me/confuser/banmanager/common/BanManagerPlugin.java

@@ -253,28 +253,42 @@ public final void disable() {
   }
 
   public void setupConfigs() {
-    new MessagesConfig(dataFolder, logger).load();
-
-    config = new DefaultConfig(dataFolder, logger);
-    config.load();
-
-    consoleConfig = new ConsoleConfig(dataFolder, logger);
-    consoleConfig.load();
-
-    schedulesConfig = new SchedulesConfig(dataFolder, logger);
-    schedulesConfig.load();
+    MessagesConfig newMessagesConfig = new MessagesConfig(dataFolder, logger);
+    if (!newMessagesConfig.load()) {
+      logger.warning("Failed to reload messages.yml, keeping previous messages");
+    }
 
-    exemptionsConfig = new ExemptionsConfig(dataFolder, logger);
-    exemptionsConfig.load();
+    config = reloadConfig(new DefaultConfig(dataFolder, logger), config, "config.yml");
+    consoleConfig = reloadConfig(new ConsoleConfig(dataFolder, logger), consoleConfig, "console.yml");
+    schedulesConfig = reloadConfig(new SchedulesConfig(dataFolder, logger), schedulesConfig, "schedules.yml");
+    exemptionsConfig = reloadConfig(new ExemptionsConfig(dataFolder, logger), exemptionsConfig, "exemptions.yml");
+    reasonsConfig = reloadConfig(new ReasonsConfig(dataFolder, logger), reasonsConfig, "reasons.yml");
+    geoIpConfig = reloadConfig(new GeoIpConfig(dataFolder, logger), geoIpConfig, "geoip.yml");
+    webhookConfig = reloadConfig(new WebhookConfig(dataFolder, logger), webhookConfig, "webhooks.yml");
+  }
 
-    reasonsConfig = new ReasonsConfig(dataFolder, logger);
-    reasonsConfig.load();
+  /**
+   * Attempts to load a new config. If loading succeeds, returns the new config.
+   * If loading fails and a previous config exists, logs a warning and returns the previous config.
+   * If loading fails and no previous config exists (first load), returns the new (failed) config.
+   *
+   * @param newConfig the newly created config to load
+   * @param currentConfig the current config (may be null on first load)
+   * @param fileName the config file name for logging
+   * @param <T> the config type
+   * @return the config to use
+   */
+  private <T extends Config> T reloadConfig(T newConfig, T currentConfig, String fileName) {
+    if (newConfig.load()) {
+      return newConfig;
+    }
 
-    geoIpConfig = new GeoIpConfig(dataFolder, logger);
-    geoIpConfig.load();
+    if (currentConfig != null) {
+      logger.warning("Failed to reload " + fileName + ", keeping previous settings");
+      return currentConfig;
+    }
 
-    webhookConfig = new WebhookConfig(dataFolder, logger);
-    webhookConfig.load();
+    return newConfig;
   }
 
   private void disableDatabaseLogging() {

common/src/main/java/me/confuser/banmanager/common/configs/Config.java

@@ -15,6 +15,8 @@ public abstract class Config {
   protected File dataFolder;
   @Getter
   protected CommonLogger logger;
+  @Getter
+  private boolean safeToSave = false;
 
   public Config(File dataFolder, File file, CommonLogger logger) {
     this.dataFolder = dataFolder;
@@ -46,40 +48,62 @@ public Config setFile(File input) {
 
   /**
    * Lazy load
+   *
+   * @return true if the config was loaded successfully, false otherwise
    */
-  public void load() {
-    if (file != null) {
-      try {
-        onLoad(file);
-
-        afterLoad();
-      } catch (Exception e) {
-        e.printStackTrace();
-      }
-    } else {
+  public boolean load() {
+    if (file == null) {
       new InvalidConfigurationException("File cannot be null!").printStackTrace();
+      return false;
+    }
+
+    try {
+      if (!onLoad(file)) {
+        safeToSave = false;
+        return false;
+      }
+
+      afterLoad();
+      safeToSave = true;
+      return true;
+    } catch (Exception e) {
+      e.printStackTrace();
+      safeToSave = false;
+      return false;
     }
   }
 
   public abstract void afterLoad();
 
   public abstract void onSave();
 
-  public void onLoad(File file) throws Exception {
+  /**
+   * Load configuration from file
+   *
+   * @param file the file to load
+   * @return true if the config was loaded successfully, false otherwise
+   * @throws Exception if the file does not exist
+   */
+  public boolean onLoad(File file) throws Exception {
     if (!file.exists()) {
       throw new Exception("File " + file.getName() + " does not exist");
     }
 
     try {
       conf.load(file);
+      return true;
     } catch (InvalidConfigurationException e) {
       logger.severe("Invalid yaml file " + file.getName());
-
-      return;
+      return false;
     }
   }
 
   public void save() {
+    if (!safeToSave) {
+      logger.warning("Skipping save for " + file.getName() + " - config was not loaded successfully");
+      return;
+    }
+
     onSave();
 
     try {

common/src/main/java/me/confuser/banmanager/common/util/Message.java

@@ -54,10 +54,25 @@ public static String getString(String key) {
 
   public static void load(YamlConfiguration config, CommonLogger commonLogger) {
     logger = commonLogger;
-    messages.clear();
 
+    if (config.getConfigurationSection("messages") == null) {
+      commonLogger.warning("Messages section not found in messages.yml, keeping previous messages");
+      return;
+    }
+
+    HashMap<String, String> newMessages = new HashMap<>(10);
     for (String key : config.getConfigurationSection("messages").getKeys(true)) {
-      messages.put(key, config.getString("messages." + key).replace("\\n", "\n"));
+      String value = config.getString("messages." + key);
+      if (value != null) {
+        newMessages.put(key, value.replace("\\n", "\n"));
+      }
+    }
+
+    if (!newMessages.isEmpty()) {
+      messages.clear();
+      messages.putAll(newMessages);
+    } else {
+      commonLogger.warning("No messages loaded from messages.yml, keeping previous messages");
     }
   }