Rework plugin config system #2

commandapi-codecov/pom.xml

@@ -40,6 +40,16 @@
 			<artifactId>commandapi-bukkit-core</artifactId>
 			<version>${project.version}</version>
 		</dependency>
+		<dependency>
+			<groupId>dev.jorel</groupId>
+			<artifactId>commandapi-plugin</artifactId>
+			<version>${project.version}</version>
+		</dependency>
+		<dependency>
+			<groupId>dev.jorel</groupId>
+			<artifactId>commandapi-bukkit-plugin-common</artifactId>
+			<version>${project.version}</version>
+		</dependency>
 
 		<!-- Code coverage the tests -->
 		<dependency>

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-plugin-common/src/main/java/dev/jorel/commandapi/CommandAPIMain.java

@@ -145,7 +145,7 @@ public void onEnable() {
 	@Override
 	public void saveDefaultConfig() {
 		File configFile = new File(getDataFolder(), "config.yml");
-		BukkitConfigurationAdapter.createDummyInstance().saveDefaultConfig(getDataFolder(), configFile, getLogger());
+		BukkitConfigurationAdapter.createMinimalInstance(configFile).saveDefaultConfig(getDataFolder(), getLogger());
 	}
 
 }

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-plugin-common/src/main/java/dev/jorel/commandapi/config/BukkitConfigurationAdapter.java

@@ -12,12 +12,11 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
-import java.util.logging.Logger;
 
-public record BukkitConfigurationAdapter(YamlConfiguration config) implements ConfigurationAdapter<YamlConfiguration> {
+public record BukkitConfigurationAdapter(YamlConfiguration config, File configFile) implements ConfigurationAdapter<YamlConfiguration> {
 
-	public static BukkitConfigurationAdapter createDummyInstance() {
-		return new BukkitConfigurationAdapter(null);
+	public static BukkitConfigurationAdapter createMinimalInstance(File configFile) {
+		return new BukkitConfigurationAdapter(null, configFile);
 	}
 
 	@Override
@@ -104,50 +103,26 @@ public ConfigurationAdapter<YamlConfiguration> complete() {
 		return this;
 	}
 
+
 	@Override
 	public ConfigurationAdapter<YamlConfiguration> createNew() {
-		return new BukkitConfigurationAdapter(new YamlConfiguration());
+		return new BukkitConfigurationAdapter(new YamlConfiguration(), configFile);
 	}
 
 	@Override
-	public void saveDefaultConfig(File directory, File configFile, Logger logger) {
-		ConfigGenerator configGenerator = ConfigGenerator.createNew(DefaultBukkitConfig.createDefault());
-		if (!directory.exists()) {
-			boolean createdDirectory = directory.mkdirs();
-			if (!createdDirectory) {
-				logger.severe("Failed to create directory for the CommandAPI's config.yml file!");
-			}
-			try {
-				ConfigurationAdapter<YamlConfiguration> bukkitConfigurationAdapter = new BukkitConfigurationAdapter(new YamlConfiguration());
-				configGenerator.populateDefaultConfig(bukkitConfigurationAdapter);
-				bukkitConfigurationAdapter.config().save(configFile);
-			} catch (IOException e) {
-				logger.severe("Could not create default config file! This is (probably) a bug.");
-				logger.severe("Error message: " + e.getMessage());
-				logger.severe("Stacktrace:");
-				for (StackTraceElement element : e.getStackTrace()) {
-					logger.severe(element.toString());
-				}
-			}
-			return;
-		}
-		// Update the config if necessary
-		try {
-			YamlConfiguration existingYamlConfig = YamlConfiguration.loadConfiguration(configFile);
-			ConfigurationAdapter<YamlConfiguration> existingConfig = new BukkitConfigurationAdapter(existingYamlConfig);
-			ConfigurationAdapter<YamlConfiguration> updatedConfig = configGenerator.generateWithNewValues(existingConfig);
-			if (updatedConfig == null) {
-				return;
-			}
-			updatedConfig.config().save(configFile);
-		} catch (IOException e) {
-			logger.severe("Could not update config! This is (probably) a bug.");
-			logger.severe("Error message: " + e.getMessage());
-			logger.severe("Stacktrace:");
-			for (StackTraceElement element : e.getStackTrace()) {
-				logger.severe(element.toString());
-			}
-		}
+	public DefaultBukkitConfig createDefaultConfig() {
+		return DefaultBukkitConfig.createDefault();
+	}
+
+	@Override
+	public ConfigurationAdapter<YamlConfiguration> loadFromFile() {
+		YamlConfiguration config = YamlConfiguration.loadConfiguration(configFile);
+		return new BukkitConfigurationAdapter(config, configFile);
+	}
+
+	@Override
+	public void saveToFile() throws IOException {
+		config.save(configFile);
 	}
 
 }

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-test/commandapi-bukkit-test-tests/pom.xml

@@ -86,6 +86,11 @@
 			<artifactId>commandapi-bukkit-test-impl</artifactId>
 			<version>${project.version}</version>
 		</dependency>
+		<dependency>
+			<groupId>dev.jorel</groupId>
+			<artifactId>commandapi-bukkit-plugin-common</artifactId>
+			<version>${project.version}</version>
+		</dependency>
 
 		<!-- NBT API implementations (in no particular order) -->
 		<dependency>

commandapi-platforms/commandapi-bukkit/commandapi-bukkit-test/commandapi-bukkit-test-tests/src/test/java/dev/jorel/commandapi/test/ConfigGenerationTests.java

@@ -0,0 +1,202 @@
+package dev.jorel.commandapi.test;
+
+import dev.jorel.commandapi.config.BukkitConfigurationAdapter;
+import dev.jorel.commandapi.config.CommentedConfigOption;
+import dev.jorel.commandapi.config.CommentedSection;
+import dev.jorel.commandapi.config.ConfigGenerator;
+import dev.jorel.commandapi.config.ConfigurationAdapter;
+import dev.jorel.commandapi.config.DefaultBukkitConfig;
+import org.bukkit.configuration.ConfigurationSection;
+import org.bukkit.configuration.file.YamlConfiguration;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.Arrays;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class ConfigGenerationTests {
+
+	private CommentedConfigOption<Boolean> silentLogs;
+	private CommentedConfigOption<Boolean> verboseOutputs;
+	private CommentedConfigOption<String> missingExecutorImplementation;
+
+	private CommentedSection messages;
+
+	private ConfigGenerator generator;
+	private DefaultBukkitConfig bukkitConfig;
+	private BukkitConfigurationAdapter adapter;
+
+	@BeforeEach
+	public void setup() {
+		silentLogs = new CommentedConfigOption<>(new String[]{
+			"Silent logs (default: false)",
+			"If \"true\", turns off all logging from the CommandAPI, except for errors."
+		}, false);
+		verboseOutputs = new CommentedConfigOption<>(new String[]{
+			"Verbose outputs (default: false)",
+			"If \"true\", outputs command registration and unregistration logs in the console"
+		}, false);
+		missingExecutorImplementation = new CommentedConfigOption<>(new String[]{
+			"Missing executor implementation (default: \"This command has no implementations for %s\")",
+			"The message to display to senders when a command has no executor. Available",
+			"parameters are:",
+			"  %s - the executor class (lowercase)",
+			"  %S - the executor class (normal case)"
+		}, "This command has no implementations for %s");
+
+		messages = new CommentedSection(new String[]{
+			"Messages",
+			"Controls messages that the CommandAPI displays to players"
+		});
+
+		Map<String, CommentedConfigOption<?>> options = new LinkedHashMap<>();
+		options.put("silent-logs", silentLogs);
+		options.put("verbose-outputs", verboseOutputs);
+		options.put("messages.missing-executor-implementation", missingExecutorImplementation);
+
+		Map<String, CommentedSection> sections = new LinkedHashMap<>();
+		sections.put("messages", messages);
+
+		ConfigurationAdapter<YamlConfiguration> adapter = new BukkitConfigurationAdapter(new YamlConfiguration(), null);
+		bukkitConfig = DefaultBukkitConfig.create(options, sections);
+		generator = ConfigGenerator.createNew(bukkitConfig);
+		this.adapter = (BukkitConfigurationAdapter) generator.generate(adapter);
+	}
+
+	@AfterEach
+	public void reset() {
+		this.silentLogs = null;
+		this.verboseOutputs = null;
+		this.missingExecutorImplementation = null;
+		this.messages = null;
+		this.generator = null;
+		this.bukkitConfig = null;
+		this.adapter = null;
+	}
+
+	// Test methods
+	private void validateConfigOptions(Set<String> options, BukkitConfigurationAdapter adapter) {
+		for (String option : options) {
+			assertTrue(adapter.contains(option), "Config option '" + option + "' does not exist!");
+		}
+	}
+
+	private void validateConfigOptionComments(Map<String, String[]> comments, BukkitConfigurationAdapter adapter) {
+		for (String option : comments.keySet()) {
+			String[] expectedComment = comments.get(option);
+			String[] generatedComment = adapter.getComment(option);
+			assertArrayEquals(expectedComment, generatedComment, "Config option comment for option '" + option + "' does not exist or was incorrect!");
+		}
+	}
+
+	private void validateConfigOptionsAbsent(Set<String> options, BukkitConfigurationAdapter adapter) {
+		for (String option : options) {
+			assertFalse(adapter.contains(option), "Config option '" + option + "' does still exist!");
+		}
+	}
+
+	private void validateSections(List<String> sections, String expectedOption, YamlConfiguration config) {
+		ConfigurationSection root = config;
+
+		for (String section : sections) {
+			root = root.getConfigurationSection(section);
+			assertNotNull(root, "Section '" + section + "' does not exist!");
+		}
+		Object expectedValue = root.get(expectedOption);
+		assertNotNull(expectedValue, "Expected option '" + expectedOption + "' was not found in section '" + root.getName() + "'!");
+	}
+
+	@Test
+	void testDefaultConfigOptionGeneration() {
+		validateConfigOptions(Set.of(
+			"silent-logs", "verbose-outputs", "messages.missing-executor-implementation"
+		), adapter);
+	}
+
+	@Test
+	void testDefaultConfigOptionCommentGeneration() {
+		validateConfigOptionComments(Map.ofEntries(
+			Map.entry("silent-logs", silentLogs.comment()),
+			Map.entry("verbose-outputs", verboseOutputs.comment()),
+			Map.entry("messages.missing-executor-implementation", missingExecutorImplementation.comment()),
+			Map.entry("messages", messages.comment())
+		), adapter);
+	}
+
+	@Test
+	void testConfigOptionAddition() {
+		CommentedConfigOption<Boolean> createDispatcherJson = new CommentedConfigOption<>(new String[] {
+			"Create dispatcher JSON (default: false)",
+			"If \"true\", the CommandAPI creates a command_registration.json file showing the",
+			"mapping of registered commands. This is designed to be used by developers -",
+			"setting this to \"false\" will improve command registration performance."
+		}, false);
+
+		bukkitConfig.getAllOptions().put("create-dispatcher-json", createDispatcherJson);
+		generator = ConfigGenerator.createNew(bukkitConfig);
+		BukkitConfigurationAdapter updatedAdapter = (BukkitConfigurationAdapter) generator.generate(adapter);
+
+		validateConfigOptions(Set.of(
+			"silent-logs", "verbose-outputs", "messages.missing-executor-implementation", "create-dispatcher-json"
+		), updatedAdapter);
+
+		validateConfigOptionComments(Map.ofEntries(
+			Map.entry("silent-logs", silentLogs.comment()),
+			Map.entry("verbose-outputs", verboseOutputs.comment()),
+			Map.entry("messages.missing-executor-implementation", missingExecutorImplementation.comment()),
+			Map.entry("create-dispatcher-json", createDispatcherJson.comment()),
+			Map.entry("messages", messages.comment())
+		), updatedAdapter);
+	}
+
+	@Test
+	void testConfigOptionDeletion() {
+		bukkitConfig.getAllOptions().remove("silent-logs");
+		generator = ConfigGenerator.createNew(bukkitConfig);
+		BukkitConfigurationAdapter updatedAdapter = (BukkitConfigurationAdapter) generator.generate(adapter);
+
+		validateConfigOptionsAbsent(Set.of("silent-logs"), updatedAdapter);
+	}
+
+	@Test
+	void testConfigOptionCommentUpdate() {
+		silentLogs = new CommentedConfigOption<>(new String[] {
+			"Defines if silent logs should happen"
+		}, false);
+
+		bukkitConfig.getAllOptions().put("silent-logs", silentLogs);
+		generator = ConfigGenerator.createNew(bukkitConfig);
+		BukkitConfigurationAdapter updatedAdapter = (BukkitConfigurationAdapter) generator.generate(adapter);
+
+		validateConfigOptionComments(Map.ofEntries(
+			Map.entry("silent-logs", silentLogs.comment())
+		), updatedAdapter);
+	}
+
+	@Test
+	void testNestedSections() {
+		CommentedConfigOption<Boolean> subSubOption = new CommentedConfigOption<>(new String[0], false);
+
+		bukkitConfig.getAllOptions().put("root.nested.option", subSubOption);
+		generator = ConfigGenerator.createNew(bukkitConfig);
+		BukkitConfigurationAdapter updatedAdapter = (BukkitConfigurationAdapter) generator.generate(adapter);
+
+		validateSections(List.of("root", "nested"), "option", updatedAdapter.config());
+	}
+
+	@Test
+	void testConfigUpdateNotNeeded() {
+		assertNull(generator.generate(adapter));
+	}
+
+}

commandapi-platforms/commandapi-velocity/commandapi-velocity-plugin/src/main/java/dev/jorel/commandapi/CommandAPIMain.java

@@ -35,16 +35,17 @@ public CommandAPIMain(ProxyServer server, Logger logger, @DataDirectory Path dat
 		// Try to find the config file
 		Path configFile = dataDirectory.resolve("config.yml");
 
+		YamlConfigurationLoader loader = YamlConfigurationLoader.builder()
+			.nodeStyle(NodeStyle.BLOCK)
+			.path(configFile)
+			.build();
+
 		// Create or update config
-		VelocityConfigurationAdapter.createDummyInstance().saveDefaultConfig(configFile.getParent().toFile(), configFile.toFile(), logger);
+		VelocityConfigurationAdapter.createMinimalInstance(loader).saveDefaultConfig(configFile.getParent().toFile(), logger);
 
 		// Load the file as a yaml node
 		ConfigurationNode configYAML;
 		try {
-			YamlConfigurationLoader loader = YamlConfigurationLoader.builder()
-				.nodeStyle(NodeStyle.BLOCK)
-				.path(configFile)
-				.build();
 			configYAML = loader.load();
 		} catch (IOException e) {
 			throw new RuntimeException(e);