The plugin will now shutdown in case a severe issue prevents loading the config.

This includes the case that the config version is invalid. Previously it would treat invalid and missing config versions the same and apply config migrations nevertheless.

Also separated config and language file loading, and changed/added a few information/warning messages related to config and language file loading.

CHANGELOG.md

@@ -16,6 +16,8 @@ Date format: (YYYY-MM-DD)
 * Fixed: The internal default for message 'msg-list-shops-entry' (that gets used if the message is missing in the config) was not matching the message in the default config.
 
 * Changed: Only printing the 'Config already loaded' message during startup if the debug mode is enabled.
+* Changed: The plugin will now shutdown in case a severe issue prevents loading the config. This includes the case that the config version is invalid. Previously it would treat invalid and missing config versions the same and apply config migrations nevertheless.
+* Changed: Changed/Added a few information/warning messages related to config and language file loading.
 * API: Added ShopkeepersStartupEvent which can be used by plugins to make registrations during Shopkeepers' startup process (eg. to register custom shop types, object types, etc.). This event is marked as deprecated because custom shop types, object types, etc. are not yet officially supported as part of the API. Also, the event is called so early that the plugin (and thereby the API) are not yet fully setup and ready to be used, so this event is only of use for plugins which know what they are doing.
 
 * Various (mostly internal) changes to commands and argument parsing:  
@@ -57,6 +59,7 @@ Other internal changes:
 * Internal: bstats gets shaded into the package '[..].libs.bstats' now.
 * Internal: Added Settings#isDebugging and Settings#isDebugging(option) to conveniently (and thread-safe) check for debugging options.
 * Internal: Default shop, object and ui types are getting registered early during onLoad now.
+* Internal: Separated config and language file loading.
 
 Changed messages (you will have to manually update those!):  
 * msg-list-shops-entry: 'object type' changed to 'object', and the arguments '{shopSessionId}' and '{shopId}' changed to '{shopId}' and '{shopUUID}' respectively. Argument '{shopSessionId}' still works but will likely get removed in the future.

src/main/java/com/nisovin/shopkeepers/SKShopkeepersPlugin.java

@@ -36,6 +36,7 @@ import com.nisovin.shopkeepers.api.shopkeeper.player.PlayerShopkeeper;
 import com.nisovin.shopkeepers.chestprotection.ProtectedChests;
 import com.nisovin.shopkeepers.commands.Commands;
 import com.nisovin.shopkeepers.compat.NMSManager;
+import com.nisovin.shopkeepers.config.ConfigLoadException;
 import com.nisovin.shopkeepers.metrics.CitizensChart;
 import com.nisovin.shopkeepers.metrics.FeaturesChart;
 import com.nisovin.shopkeepers.metrics.GringottsChart;
@@ -116,6 +117,7 @@ public class SKShopkeepersPlugin extends JavaPlugin implements ShopkeepersPlugin
 
 	private boolean outdatedServer = false;
 	private boolean incompatibleServer = false;
+	private ConfigLoadException configLoadError = null; // null on success
 
 	private void loadRequiredClasses() {
 		// making sure that certain classes, that are needed during shutdown, are loaded:
@@ -152,23 +154,35 @@ public class SKShopkeepersPlugin extends JavaPlugin implements ShopkeepersPlugin
 		return (NMSManager.getProvider() != null);
 	}
 
-	private void loadConfig() {
+	// returns null on success, otherwise a severe issue prevented loading the config
+	private ConfigLoadException loadConfig() {
 		Log.info("Loading config.");
+
 		// save default config in case the config file doesn't exist
 		this.saveDefaultConfig();
 
 		// load config:
 		this.reloadConfig();
+		Configuration config = this.getConfig();
 
 		// load settings from config:
-		Configuration config = this.getConfig();
-		boolean configChanged = Settings.loadConfiguration(config);
+		boolean configChanged;
+		try {
+			configChanged = Settings.loadConfiguration(config);
+		} catch (ConfigLoadException e) {
+			// config loading failed with a severe issue:
+			return e;
+		}
+
 		if (configChanged) {
 			// if the config was modified (migrations, adding missing settings, ..), save it:
 			// TODO persist comments somehow
 			this.saveConfig();
 		}
+		return null; // config loaded successfully
+	}
 
+	private void loadLanguageFile() {
 		// load language config:
 		String lang = Settings.language;
 		String langFileName = "language-" + lang + ".yml";
@@ -176,13 +190,19 @@ public class SKShopkeepersPlugin extends JavaPlugin implements ShopkeepersPlugin
 		if (!langFile.exists() && this.getResource(langFileName) != null) {
 			this.saveResource(langFileName, false);
 		}
-		if (langFile.exists()) {
+
+		if (!langFile.exists()) {
+			if (lang.equals("en")) { // if not default // TODO don't hardcode
+				Log.warning("Could not find language file '" + langFile.getPath() + "'!");
+			} // else: ignore
+		} else {
+			Log.info("Loading language file: " + langFileName);
 			try {
 				YamlConfiguration langConfig = new YamlConfiguration();
 				langConfig.load(langFile);
 				Settings.loadLanguageConfiguration(langConfig);
 			} catch (Exception e) {
-				e.printStackTrace();
+				Log.warning("Could not load language file '" + langFile.getPath() + "'!", e);
 			}
 		}
 	}
@@ -223,7 +243,13 @@ public class SKShopkeepersPlugin extends JavaPlugin implements ShopkeepersPlugin
 		}
 
 		// load config:
-		this.loadConfig();
+		this.configLoadError = this.loadConfig();
+		if (this.configLoadError != null) {
+			return;
+		}
+
+		// load language file:
+		this.loadLanguageFile();
 
 		// WorldGuard only allows registering flags before it gets enabled.
 		// Note: Changing the config setting has no effect until the next server restart or server reload.
@@ -262,10 +288,22 @@ public class SKShopkeepersPlugin extends JavaPlugin implements ShopkeepersPlugin
 
 		// load config (if not already loaded during onLoad):
 		if (!alreadySetup) {
-			this.loadConfig();
+			this.configLoadError = this.loadConfig();
 		} else {
 			Log.debug("Config already loaded.");
 		}
+		if (this.configLoadError != null) {
+			Log.severe("Could not load the config!", configLoadError);
+			this.setEnabled(false); // also calls onDisable
+			return;
+		}
+
+		// load language file (if not already loaded during onLoad):
+		if (!alreadySetup) {
+			this.loadLanguageFile();
+		} else {
+			Log.debug("Language file already loaded.");
+		}
 
 		// process additional permissions
 		String[] perms = Settings.maxShopsPermOptions.replace(" ", "").split(",");

src/main/java/com/nisovin/shopkeepers/Settings.java

@@ -15,6 +15,7 @@ import org.bukkit.entity.EntityType;
 import org.bukkit.entity.Player;
 import org.bukkit.inventory.ItemStack;
 
+import com.nisovin.shopkeepers.config.ConfigLoadException;
 import com.nisovin.shopkeepers.config.migration.ConfigMigrations;
 import com.nisovin.shopkeepers.util.ConfigUtils;
 import com.nisovin.shopkeepers.util.ConversionUtils;
@@ -499,7 +500,7 @@ public class Settings {
 	}
 
 	// returns true, if the config misses values which need to be saved
-	public static boolean loadConfiguration(Configuration config) {
+	public static boolean loadConfiguration(Configuration config) throws ConfigLoadException {
 		boolean configChanged = false;
 
 		// perform config migrations:
@@ -552,7 +553,7 @@ public class Settings {
 				field.set(null, value);
 			}
 		} catch (Exception e) {
-			e.printStackTrace();
+			throw new ConfigLoadException("Error while loading config values!", e);
 		}
 
 		// validation:
@@ -684,7 +685,7 @@ public class Settings {
 		}
 	}
 
-	public static void loadLanguageConfiguration(Configuration config) {
+	public static void loadLanguageConfiguration(Configuration config) throws ConfigLoadException {
 		try {
 			Field[] fields = Settings.class.getDeclaredFields();
 			for (Field field : fields) {
@@ -706,7 +707,7 @@ public class Settings {
 				field.set(null, value);
 			}
 		} catch (Exception e) {
-			e.printStackTrace();
+			throw new ConfigLoadException("Error while loading messages from language file!", e);
 		}
 
 		onSettingsChanged();

src/main/java/com/nisovin/shopkeepers/config/ConfigLoadException.java

@@ -0,0 +1,17 @@
+package com.nisovin.shopkeepers.config;
+
+/**
+ * This exception gets used if a severe issue prevents loading a configuration.
+ */
+public class ConfigLoadException extends Exception {
+
+	private static final long serialVersionUID = 3283134205506144514L;
+
+	public ConfigLoadException(String message) {
+		super(message);
+	}
+
+	public ConfigLoadException(String message, Throwable cause) {
+		super(message, cause);
+	}
+}

src/main/java/com/nisovin/shopkeepers/config/migration/ConfigMigration.java

@@ -4,5 +4,14 @@ import org.bukkit.configuration.Configuration;
 
 public interface ConfigMigration {
 
+	/**
+	 * Applies the config migration.
+	 * <p>
+	 * If an issue prevents the migration of individual config entries, it is usually preferred to log a warning and
+	 * continue the migration, instead of aborting the migration or throwing an exception.
+	 * 
+	 * @param config
+	 *            the current config
+	 */
 	public void apply(Configuration config);
 }

src/main/java/com/nisovin/shopkeepers/config/migration/ConfigMigration1.java

@@ -10,7 +10,7 @@ import com.nisovin.shopkeepers.util.Log;
 import com.nisovin.shopkeepers.util.StringUtils;
 
 /**
- * Migrate the config from version <= 0 to version 1.
+ * Migrates the config from version 0 (pre versioning) to version 1.
  */
 public class ConfigMigration1 implements ConfigMigration {
 

src/main/java/com/nisovin/shopkeepers/config/migration/ConfigMigration2.java

@@ -12,7 +12,7 @@ import com.nisovin.shopkeepers.util.StringUtils;
 import com.nisovin.shopkeepers.util.TextUtils;
 
 /**
- * Migrate the config from version 1 to version 2.
+ * Migrates the config from version 1 to version 2.
  */
 public class ConfigMigration2 implements ConfigMigration {
 

src/main/java/com/nisovin/shopkeepers/config/migration/ConfigMigrations.java

@@ -5,35 +5,67 @@ import java.util.List;
 
 import org.bukkit.configuration.Configuration;
 
+import com.nisovin.shopkeepers.config.ConfigLoadException;
+import com.nisovin.shopkeepers.util.ConversionUtils;
 import com.nisovin.shopkeepers.util.Log;
 
 public class ConfigMigrations {
 
 	private static final String CONFIG_VERSION_KEY = "config-version";
+	private static final int FIRST_VERSION = 0; // also applies if the config version is missing
+
 	// each index corresponds to a source config version and its migration to the next version
 	private static final List<ConfigMigration> migrations = Arrays.asList(new ConfigMigration1(), new ConfigMigration2());
 
+	public static int getLatestVersion() {
+		return migrations.size();
+	}
+
 	// returns true if any migrations got applied (if the config has potentially been modified)
-	public static boolean applyMigrations(Configuration config) {
+	public static boolean applyMigrations(Configuration config) throws ConfigLoadException {
 		// no migrations are required if the config is empty (missing entries will get generated from defaults):
 		if (config.getKeys(false).isEmpty()) return false;
 
-		boolean migrated = false;
-		int configVersion = config.getInt(CONFIG_VERSION_KEY, 0); // default value is important here
-		if (configVersion < 0) {
-			configVersion = 0; // first version is 0
+		// parse config version:
+		int configVersion;
+		Object rawConfigVersion = config.get(CONFIG_VERSION_KEY, null); // explicit default is important here
+		if (rawConfigVersion == null) {
+			Log.info("Missing config version. Assuming version '" + FIRST_VERSION + "'.");
+			configVersion = FIRST_VERSION;
+		} else {
+			Integer configVersionI = ConversionUtils.toInteger(rawConfigVersion);
+			if (configVersionI == null) { // parsing failed
+				throw new ConfigLoadException("Could not parse config version: " + rawConfigVersion);
+			} else {
+				configVersion = configVersionI.intValue();
+			}
 		}
+
+		// check bounds:
+		if (configVersion < FIRST_VERSION) {
+			throw new ConfigLoadException("Invalid config version: " + rawConfigVersion);
+		}
+		if (configVersion > getLatestVersion()) {
+			throw new ConfigLoadException("Invalid config version: " + configVersion + " (the latest version is " + getLatestVersion() + ")");
+		}
+
+		// apply required migrations:
+		boolean migrated = false;
 		for (int version = configVersion; version < migrations.size(); ++version) {
 			int nextVersion = (version + 1);
 			ConfigMigration migration = migrations.get(version);
 			Log.info("Migrating config from version " + version + " to version " + nextVersion + " ..");
 			if (migration != null) {
-				migration.apply(config);
+				try {
+					migration.apply(config);
+				} catch (Exception e) {
+					throw new ConfigLoadException("Config migration failed with an error!", e);
+				}
 			}
 			// update config version:
 			config.set(CONFIG_VERSION_KEY, nextVersion);
 			migrated = true;
-			Log.info("Config migrated to version " + nextVersion + "." + (migration == null ? " (skipped)" : ""));
+			Log.info("Config migrated to version " + nextVersion + "." + (migration == null ? " (no changes)" : ""));
 		}
 		return migrated;
 	}