Rework plugin config system #2

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

@@ -119,8 +119,8 @@ public void saveDefaultConfig(File directory, File configFile, Logger logger) {
 			}
 			try {
 				ConfigurationAdapter<YamlConfiguration> bukkitConfigurationAdapter = new BukkitConfigurationAdapter(new YamlConfiguration());
-				configGenerator.populateDefaultConfig(bukkitConfigurationAdapter);
-				bukkitConfigurationAdapter.config().save(configFile);
+				ConfigurationAdapter<YamlConfiguration> generatedConfig = configGenerator.generate(bukkitConfigurationAdapter);
+				generatedConfig.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());
@@ -135,7 +135,7 @@ public void saveDefaultConfig(File directory, File configFile, Logger logger) {
 		try {
 			YamlConfiguration existingYamlConfig = YamlConfiguration.loadConfiguration(configFile);
 			ConfigurationAdapter<YamlConfiguration> existingConfig = new BukkitConfigurationAdapter(existingYamlConfig);
-			ConfigurationAdapter<YamlConfiguration> updatedConfig = configGenerator.generateWithNewValues(existingConfig);
+			ConfigurationAdapter<YamlConfiguration> updatedConfig = configGenerator.generate(existingConfig);
 			if (updatedConfig == null) {
 				return;
 			}

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,208 @@
+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;
+
+public 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());
+		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
+	public void testDefaultConfigOptionGeneration() {
+		validateConfigOptions(Set.of(
+			"silent-logs", "verbose-outputs", "messages.missing-executor-implementation"
+		), adapter);
+	}
+
+	@Test
+	public 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
+	public 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
+	public void testConfigOptionDeletion() {
+		bukkitConfig.getAllOptions().remove("silent-logs");
+		generator = ConfigGenerator.createNew(bukkitConfig);
+		BukkitConfigurationAdapter updatedAdapter = (BukkitConfigurationAdapter) generator.generate(adapter);
+
+		validateConfigOptionsAbsent(Set.of("silent-logs"), updatedAdapter);
+	}
+
+	@Test
+	public 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
+	public 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 methods
+	public void validateConfigOptions(Set<String> options, BukkitConfigurationAdapter adapter) {
+		boolean containsAll;
+		for (String option : options) {
+			containsAll = adapter.contains(option);
+			if (!containsAll) {
+				throw new IllegalStateException("Config option '" + option + "' does not exist!");
+			}
+		}
+	}
+
+	public void validateConfigOptionComments(Map<String, String[]> comments, BukkitConfigurationAdapter adapter) {
+		boolean correctComment;
+		for (String option : comments.keySet()) {
+			String[] generatedComment = adapter.getComment(option);
+			correctComment = Arrays.equals(comments.get(option), generatedComment);
+			if (!correctComment) {
+				throw new IllegalStateException("Config option comment for option '" + option + "' does not exist or was incorrect!");
+			}
+		}
+	}
+
+	public void validateConfigOptionsAbsent(Set<String> options, BukkitConfigurationAdapter adapter) {
+		boolean isAbsent;
+		for (String option : options) {
+			isAbsent = !adapter.contains(option);
+			if (!isAbsent) {
+				throw new IllegalStateException("Config option '" + option + "' does still exist!");
+			}
+		}
+	}
+
+	public void validateSections(List<String> sections, String expectedOption, YamlConfiguration config) {
+		ConfigurationSection root = config.getConfigurationSection(sections.get(0));
+		if (root == null) {
+			throw new IllegalStateException("Section '" + sections.get(0) + "' does not exist!");
+		}
+		for (int i = 1; i < sections.size(); i++) {
+			root = root.getConfigurationSection(sections.get(i));
+			if (root == null) {
+				throw new IllegalStateException("Section '" + sections.get(i) + "' does not exist!");
+			}
+		}
+		Object expectedValue = root.get(expectedOption);
+		if (expectedValue == null) {
+			throw new IllegalStateException("Expected option '" + expectedOption + "' was not found in section '" + root.getName() + "'!");
+		}
+	}
+
+}

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.createDummyInstance(loader).saveDefaultConfig(configFile.getParent().toFile(), configFile.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);

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

@@ -3,22 +3,18 @@
 import org.spongepowered.configurate.CommentedConfigurationNode;
 import org.spongepowered.configurate.ConfigurationNode;
 import org.spongepowered.configurate.serialize.SerializationException;
-import org.spongepowered.configurate.yaml.NodeStyle;
 import org.spongepowered.configurate.yaml.YamlConfigurationLoader;
 
 import java.io.File;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 import java.util.logging.Logger;
 
 public record VelocityConfigurationAdapter(YamlConfigurationLoader loader, CommentedConfigurationNode config, DefaultVelocityConfig defaultVelocityConfig) implements ConfigurationAdapter<ConfigurationNode> {
 
-	public static VelocityConfigurationAdapter createDummyInstance() {
-		return new VelocityConfigurationAdapter(null, null, null);
+	public static VelocityConfigurationAdapter createDummyInstance(YamlConfigurationLoader loader) {
+		return new VelocityConfigurationAdapter(loader, null, null);
 	}
 
 	@Override
@@ -32,14 +28,7 @@ public void setValue(String key, Object value) {
 
 	@Override
 	public void setComment(String key, String[] comment) {
-		StringBuilder commentBuilder = new StringBuilder();
-		for (int i = 0; i < comment.length; i++) {
-			if (i > 0) {
-				commentBuilder.append("\n");
-			}
-			commentBuilder.append(comment[i]);
-		}
-		node(key).comment(commentBuilder.toString());
+		node(key).comment(String.join("\n", comment));
 	}
 
 	@Override
@@ -90,10 +79,6 @@ public ConfigurationAdapter<ConfigurationNode> createNew() {
 
 	@Override
 	public void saveDefaultConfig(File directory, File configFile, Logger logger) {
-		YamlConfigurationLoader configLoader = YamlConfigurationLoader.builder()
-			.nodeStyle(NodeStyle.BLOCK)
-			.file(configFile)
-			.build();
 		DefaultVelocityConfig defaultConfig = DefaultVelocityConfig.createDefault();
 		ConfigGenerator configGenerator = ConfigGenerator.createNew(defaultConfig);
 		if (!directory.exists()) {
@@ -102,9 +87,9 @@ public void saveDefaultConfig(File directory, File configFile, Logger logger) {
 				logger.severe("Failed to create directory for the CommandAPI's config.yml file!");
 			}
 			try {
-				ConfigurationAdapter<ConfigurationNode> velocityConfigurationAdapter = new VelocityConfigurationAdapter(configLoader, configLoader.createNode(), defaultConfig);
-				configGenerator.populateDefaultConfig(velocityConfigurationAdapter);
-				configLoader.save(velocityConfigurationAdapter.config());
+				ConfigurationAdapter<ConfigurationNode> velocityConfigurationAdapter = new VelocityConfigurationAdapter(loader, loader.createNode(), defaultConfig);
+				configGenerator.generate(velocityConfigurationAdapter);
+				loader.save(velocityConfigurationAdapter.config());
 			} catch (IOException e) {
 				logger.severe("Could not create default config file! This is (probably) a bug.");
 				logger.severe("Error message: " + e.getMessage());
@@ -116,11 +101,11 @@ public void saveDefaultConfig(File directory, File configFile, Logger logger) {
 		} else {
 			try {
 				// If the config does exist, update it if necessary
-				CommentedConfigurationNode existingYamlConfig = configLoader.load();
-				ConfigurationAdapter<ConfigurationNode> existingConfig = new VelocityConfigurationAdapter(configLoader, existingYamlConfig, defaultConfig);
-				ConfigurationAdapter<ConfigurationNode> updatedConfig = configGenerator.generateWithNewValues(existingConfig);
+				CommentedConfigurationNode existingYamlConfig = loader.load();
+				ConfigurationAdapter<ConfigurationNode> existingConfig = new VelocityConfigurationAdapter(loader, existingYamlConfig, defaultConfig);
+				ConfigurationAdapter<ConfigurationNode> updatedConfig = configGenerator.generate(existingConfig);
 				if (updatedConfig != null) {
-					configLoader.save(updatedConfig.config());
+					loader.save(updatedConfig.config());
 				}
 			} catch (IOException e) {
 				logger.severe("Could not update config! This is (probably) a bug.");

commandapi-plugin/src/main/java/dev/jorel/commandapi/config/ConfigGenerator.java

@@ -16,16 +16,7 @@ public static ConfigGenerator createNew(DefaultConfig defaultConfig) {
 		return new ConfigGenerator(defaultConfig);
 	}
 
-	public <T> void populateDefaultConfig(ConfigurationAdapter<T> adapter) {
-		for (Map.Entry<String, CommentedConfigOption<?>> commentedConfigOption : defaultConfig.getAllOptions().entrySet()) {
-			adapter.tryCreateSection(commentedConfigOption.getKey());
-			adapter.setValue(commentedConfigOption.getKey(), commentedConfigOption.getValue().option());
-			adapter.setComment(commentedConfigOption.getKey(), commentedConfigOption.getValue().comment());
-		}
-		adapter.complete();
-	}
-
-	public <T> ConfigurationAdapter<T> generateWithNewValues(ConfigurationAdapter<T> existingConfig) {
+	public <T> ConfigurationAdapter<T> generate(ConfigurationAdapter<T> existingConfig) {
 		ConfigurationAdapter<T> updatedConfig = existingConfig.createNew();
 
 		boolean shouldRemoveValues = shouldRemoveOptions(existingConfig);