diff --git a/AGENTS.md b/AGENTS.md index 406270e..9938de3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,6 +59,13 @@ outcome that cannot occur. class loader, so a module must clean up its own partial registrations before throwing (documented on `PluginModule#initialize`). `SimplePluginModule` tracks listeners but unregisters nothing by itself. Changing this is a contract change for every module, not a bug fix. +- **Module commands are declared, never unregistered.** Paper only accepts commands during its + lifecycle commands event, before modules initialize, and its API cannot remove them. A module + overrides `PluginModule#declareCommands` (a default no-op), which + `ModuleLifecycleManager#declareCommands` calls between `load()` and `initializeLoaded()`. + `CommandGuard` rebuilds each tree so executors and suggestions run only while the module is + active, and the host's `ModuleUnavailableHandler` replies otherwise. Redirects are rejected + because they would bypass the guard. A module never cleans up its commands. - **`ModuleLifecycleManager` is not thread-safe.** Its state must be touched from one thread (the server main thread on Paper). - **`SchemaMigrator` is abstract and dialect-agnostic.** It provides no SQL of its own — subclasses diff --git a/README.md b/README.md index 798bc40..b316a99 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,19 @@ by a `ModuleManifest` and loaded at runtime by its own `URLClassLoader`. (configurate-serializable). - `ModuleLoader` — reads module jars from a directory and instantiates their entry classes. - `ModuleLifecycleManager` — owns the runtime lifecycle: `start()`, `stop()`, - `reloadAsync(moduleName)`, `getActiveModules()`. + `reloadAsync(moduleName)`, `getActiveModules()`. `start()` is `load()` then `initializeLoaded()`; + call the two separately when the host must do something with the loaded modules first, such as + letting them declare commands while the server still accepts registrations. +- `PluginModule#declareCommands(ModuleCommandRoot, ModuleCommandRegistrar)` — a default no-op that a + module overrides to add Brigadier commands: subcommands under the host's root, or top-level + commands. Both parameters guard every tree. + +> **Commands:** Paper only accepts commands during `LifecycleEvents.COMMANDS`, before modules +> initialize, and cannot unregister them. Call `declareCommands(commands, root, unavailable)` from that +> event handler, every time it fires, between `load()` and `initializeLoaded()`. A module command runs +> module code only while its module is active; when the module failed to initialize or has stopped, +> the host's `ModuleUnavailableHandler` replies instead. Commands therefore need no cleanup on a +> failed start. Module commands must not use redirects. > **Failed start:** if `initialize` throws, the manager closes the module's class loader but does > **not** call `shutdown`. A module must do its fallible work before registering anything, or undo @@ -69,7 +81,14 @@ Path moduleDir = getDataFolder().toPath().resolve("modules"); ModuleLoader loader = new ModuleLoader(moduleDir); var lifecycle = new ModuleLifecycleManager<>(this, loader, MyPlugin.class.getName(), getLogger()); -lifecycle.start(); // load + initialize all modules (call from the main thread) +lifecycle.load(); // load module jars, no initialize yet +getLifecycleManager().registerEventHandler(LifecycleEvents.COMMANDS, event -> { + LiteralArgumentBuilder root = Commands.literal("myplugin"); + lifecycle.declareCommands(event.registrar(), root, + (source, module) -> source.getSender().sendMessage("That feature is unavailable.")); + event.registrar().register(root.build()); +}); +lifecycle.initializeLoaded(); // initialize (call from the main thread) // ... lifecycle.reloadAsync("economy"); // refresh one reloadable module's config // ... diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandGuard.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandGuard.java new file mode 100644 index 0000000..9c0632f --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/CommandGuard.java @@ -0,0 +1,73 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; + +import com.mojang.brigadier.Command; +import com.mojang.brigadier.builder.ArgumentBuilder; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.mojang.brigadier.suggestion.SuggestionProvider; +import com.mojang.brigadier.tree.ArgumentCommandNode; +import com.mojang.brigadier.tree.CommandNode; +import com.mojang.brigadier.tree.LiteralCommandNode; +import io.papermc.paper.command.brigadier.CommandSourceStack; +import org.jetbrains.annotations.NotNull; + +import java.util.function.BooleanSupplier; + +/** + * Rebuilds a module's command tree so that every executor and suggestion provider first checks + * whether the module is active. Brigadier nodes are immutable once built, so the guard is applied + * by rebuilding each node from {@link CommandNode#createBuilder()}. + */ +final class CommandGuard { + + private CommandGuard() { + } + + // Rebuilding a literal yields a literal, and an argument node's builder is a RequiredArgumentBuilder. + @SuppressWarnings("unchecked") + static @NotNull LiteralCommandNode guard( + @NotNull LiteralArgumentBuilder command, + @NotNull String moduleName, + @NotNull BooleanSupplier active, + @NotNull ModuleUnavailableHandler unavailable + ) { + return (LiteralCommandNode) rebuild(command.build(), moduleName, active, unavailable); + } + + @SuppressWarnings("unchecked") + private static @NotNull CommandNode rebuild( + @NotNull CommandNode node, + @NotNull String moduleName, + @NotNull BooleanSupplier active, + @NotNull ModuleUnavailableHandler unavailable + ) { + // A redirect points at a node outside this rebuild, which would bypass the guard. + if (node.getRedirect() != null) { + throw new IllegalArgumentException("Module '" + moduleName + "' declared a redirect at '" + + node.getName() + "'; module commands must not use redirects"); + } + ArgumentBuilder builder = node.createBuilder(); + Command command = node.getCommand(); + if (command != null) { + builder.executes(ctx -> { + if (active.getAsBoolean()) { + return command.run(ctx); + } + unavailable.handle(ctx.getSource(), moduleName); + return 0; + }); + } + if (node instanceof ArgumentCommandNode argumentNode + && argumentNode.getCustomSuggestions() != null) { + SuggestionProvider suggestions = argumentNode.getCustomSuggestions(); + RequiredArgumentBuilder argument = (RequiredArgumentBuilder) builder; + argument.suggests((ctx, suggestionsBuilder) -> active.getAsBoolean() + ? suggestions.getSuggestions(ctx, suggestionsBuilder) + : suggestionsBuilder.buildFuture()); + } + for (CommandNode child : node.getChildren()) { + builder.then(rebuild(child, moduleName, active, unavailable)); + } + return builder.build(); + } +} diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java new file mode 100644 index 0000000..1e85111 --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRegistrar.java @@ -0,0 +1,32 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; + +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import io.papermc.paper.command.brigadier.CommandSourceStack; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; +import java.util.List; + +/** + * Registers top-level commands for {@link PluginModule#declareCommands}. Each tree is guarded by the + * module's availability before it reaches Paper's registrar, so a module cannot register an + * unguarded command. + */ +public interface ModuleCommandRegistrar { + + /** + * Registers a top-level command. The tree must not contain redirects. + * + * @param command the command tree + * @param description the help description, or {@code null} + * @param aliases extra labels + */ + void register(@NotNull LiteralArgumentBuilder command, + @Nullable String description, @NotNull Collection aliases); + + default void register(@NotNull LiteralArgumentBuilder command, + @Nullable String description) { + register(command, description, List.of()); + } +} diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java new file mode 100644 index 0000000..b746f21 --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandRoot.java @@ -0,0 +1,15 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; + +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import io.papermc.paper.command.brigadier.CommandSourceStack; +import org.jetbrains.annotations.NotNull; + +/** The host's root command, as offered to {@link PluginModule#declareCommands}. */ +public interface ModuleCommandRoot { + + /** + * Adds a subcommand under the host's root command. The tree is guarded by the module's + * availability; it must not contain redirects. + */ + void then(@NotNull LiteralArgumentBuilder subcommand); +} diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java index c30d325..37ba204 100644 --- a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManager.java @@ -1,15 +1,26 @@ package com.minecraftcitiesnetwork.pluginInfrastructure.modules; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.tree.LiteralCommandNode; +import io.papermc.paper.command.brigadier.CommandSourceStack; +import io.papermc.paper.command.brigadier.Commands; import org.bukkit.plugin.Plugin; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.io.IOException; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.BooleanSupplier; import java.util.logging.Level; import java.util.logging.Logger; @@ -38,6 +49,28 @@ public class ModuleLifecycleManager { */ private final Map> activeModules = new LinkedHashMap<>(); + /** + * Modules returned by {@link #load()} and not yet passed to {@link #initializeLoaded()}, or + * {@code null} when nothing is waiting. + */ + private List> loadedModules; + + /** + * Names of the active modules, read by command guards. Commands can run off the main thread, + * so this is concurrent while the rest of the manager is not. + */ + private final Set activeNames = ConcurrentHashMap.newKeySet(); + + /** + * Guarded command trees keyed by module name. Kept after a module stops so that a later + * commands event (Paper re-fires it on {@code /minecraft:reload}) registers the same commands, + * which then reply as unavailable. + */ + private final Map declaredCommands = new LinkedHashMap<>(); + + /** Modules from the current {@link #load()} that have already declared their commands. */ + private final Set declaredThisLoad = new HashSet<>(); + /** * Creates a manager that expects modules targeting the host plugin's own class name and logs * through the plugin's logger. @@ -59,27 +92,123 @@ public ModuleLifecycleManager( } /** - * Loads every module from disk and initializes it against the host plugin. + * Loads every module from disk and initializes it against the host plugin. Equivalent to + * {@link #load()} followed by {@link #initializeLoaded()}. * - * @throws IllegalStateException if modules are already active; call {@link #stop()} first + * @throws IllegalStateException if modules are already loaded or active; call {@link #stop()} first * @throws IOException if the module directory cannot be scanned */ public void start() throws IOException { - if (!this.activeModules.isEmpty()) { - throw new IllegalStateException("Modules already started; call stop() or reload() first"); + load(); + initializeLoaded(); + } + + /** + * Loads every module from disk without initializing any of them. The host can then inspect the + * returned modules, for example to let them declare commands while the server still accepts + * registrations, before calling {@link #initializeLoaded()}. + * + *

A loaded module's classes are live but {@link PluginModule#initialize} has not run, so the + * host must not expect any module state yet. + * + * @return the loaded modules, in load order + * @throws IllegalStateException if modules are already loaded or active; call {@link #stop()} first + * @throws IOException if the module directory cannot be scanned + */ + public @NotNull List> load() throws IOException { + if (this.loadedModules != null || !this.activeModules.isEmpty()) { + throw new IllegalStateException("Modules already loaded; call stop() first"); } - List> loaded = this.moduleLoader.loadModulesFromDisk(this.expectedPluginClass, - this.logger); + List> loaded = new ArrayList<>( + this.moduleLoader.loadModulesFromDisk(this.expectedPluginClass, this.logger)); + this.loadedModules = loaded; + this.declaredThisLoad.clear(); + return List.copyOf(loaded); + } + + /** + * Initializes the modules returned by the last {@link #load()}. A module whose + * {@code initialize} throws is unloaded and is not active afterwards; see + * {@link PluginModule#initialize} for its cleanup contract. + * + * @throws IllegalStateException if nothing is loaded + */ + public void initializeLoaded() { + List> loaded = this.loadedModules; + if (loaded == null) { + throw new IllegalStateException("No modules loaded; call load() first"); + } + this.loadedModules = null; for (LoadedModule module : loaded) { initialize(module); } } + /** + * Registers module commands. Call it from Paper's {@code LifecycleEvents.COMMANDS} handler, + * after {@link #load()} and before {@link #initializeLoaded()}, every time the event fires. + * + *

Each loaded module that has not declared yet is asked to, via {@link PluginModule#declareCommands}. + * A module whose {@code declareCommands} throws is logged, unloaded and not initialized; nothing + * it declared is registered. Then every module's guarded commands are added: subcommands to + * {@code root}, top-level commands to {@code commands}. A guarded command runs the module's code + * only while the module is active; otherwise {@code unavailable} replies. + * + * @param commands Paper's registrar from the commands event + * @param root the host's root command builder; the host registers it after this call + * @param unavailable replies when a command's module is not active + */ + public void declareCommands( + @NotNull Commands commands, + @NotNull LiteralArgumentBuilder root, + @NotNull ModuleUnavailableHandler unavailable + ) { + List> pending = this.loadedModules; + if (pending != null) { + Iterator> modules = pending.iterator(); + while (modules.hasNext()) { + LoadedModule loaded = modules.next(); + String name = loaded.manifest().moduleName(); + // A second copy with the same name is dropped by initializeLoaded and declares nothing. + if (!this.declaredThisLoad.add(name)) { + continue; + } + BooleanSupplier active = () -> this.activeNames.contains(name); + DeclaredCommands declared = new DeclaredCommands(new ArrayList<>(), new ArrayList<>()); + try { + loaded.module().declareCommands( + sub -> declared.subcommands().add(CommandGuard.guard(sub, name, active, unavailable)), + (command, description, aliases) -> declared.topLevel().add(new TopLevelCommand( + CommandGuard.guard(command, name, active, unavailable), description, + List.copyOf(aliases)))); + } catch (RuntimeException e) { + this.logger.log(Level.SEVERE, "Failed to declare commands for module " + name + "; unloading", e); + modules.remove(); + closeQuietly(loaded); + continue; + } + this.declaredCommands.put(name, declared); + } + } + for (DeclaredCommands declared : this.declaredCommands.values()) { + declared.subcommands().forEach(root::then); + for (TopLevelCommand command : declared.topLevel()) { + commands.register(command.node(), command.description(), command.aliases()); + } + } + } + /** * Shuts down every active module in reverse initialization order and releases their class - * loaders. Safe to call when nothing is active. + * loaders. Modules that were loaded but never initialized are closed without + * {@link PluginModule#shutdown}. Safe to call when nothing is loaded or active. */ public void stop() { + List> pending = this.loadedModules; + if (pending != null) { + this.loadedModules = null; + pending.forEach(this::closeQuietly); + } List> ordered = new ArrayList<>(this.activeModules.values()); for (int i = ordered.size() - 1; i >= 0; i--) { shutdown(ordered.get(i)); @@ -132,6 +261,7 @@ private void initialize(@NotNull LoadedModule loaded) { try { loaded.module().initialize(this.plugin, loaded.dataFolder()); this.activeModules.put(name, loaded); + this.activeNames.add(name); this.logger.info("Initialized module " + name); } catch (ModuleInitializationException | RuntimeException e) { this.logger.log(Level.SEVERE, "Failed to initialize module " + name + "; unloading", e); @@ -141,6 +271,7 @@ private void initialize(@NotNull LoadedModule loaded) { private void shutdown(@NotNull LoadedModule loaded) { String name = loaded.manifest().moduleName(); + this.activeNames.remove(name); try { loaded.module().shutdown(this.plugin); this.logger.info("Shut down module " + name); @@ -159,4 +290,17 @@ private void closeQuietly(@NotNull LoadedModule loaded) { "Failed to close class loader for module " + loaded.manifest().moduleName(), e); } } + + private record DeclaredCommands( + @NotNull List> subcommands, + @NotNull List topLevel + ) { + } + + private record TopLevelCommand( + @NotNull LiteralCommandNode node, + @Nullable String description, + @NotNull Collection aliases + ) { + } } diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleUnavailableHandler.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleUnavailableHandler.java new file mode 100644 index 0000000..a34cbc9 --- /dev/null +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleUnavailableHandler.java @@ -0,0 +1,15 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; + +import io.papermc.paper.command.brigadier.CommandSourceStack; +import org.jetbrains.annotations.NotNull; + +/** Replies to a sender who ran a module command while that module is not active. */ +@FunctionalInterface +public interface ModuleUnavailableHandler { + + /** + * @param source who ran the command + * @param moduleName the {@link ModuleManifest#moduleName()} of the inactive module + */ + void handle(@NotNull CommandSourceStack source, @NotNull String moduleName); +} diff --git a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java index 9510487..6bba5ce 100644 --- a/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java +++ b/src/main/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/PluginModule.java @@ -9,8 +9,8 @@ public interface PluginModule { /** - * Starts this module: reads its configuration and registers its listeners, tasks and commands - * with the host. + * Starts this module: reads its configuration and registers its listeners and tasks with the + * host. Commands are not registered here; see {@link #declareCommands}. * *

Failure is the module's to clean up. If this method throws, the * {@link ModuleLifecycleManager} logs the failure and closes the module's class loader, but it @@ -18,6 +18,8 @@ public interface PluginModule { * a listener keeps firing with classes from a closed class loader, a task keeps running. An * implementation must therefore either do all fallible work (reading config, resolving * services) before registering anything, or undo what it registered before rethrowing. + * Commands declared through {@link #declareCommands} are the exception: they stay + * registered and reply as unavailable, so they need no cleanup. * * @param plugin the host plugin * @param dataFolder this module's own data folder @@ -26,6 +28,27 @@ public interface PluginModule { */ void initialize(@NotNull T plugin, @NotNull Path dataFolder) throws ModuleInitializationException; + /** + * Declares this module's Brigadier commands. Override it to add commands; the default declares + * none. + * + *

Paper only accepts commands during its lifecycle commands event, which fires before modules + * initialize, so the {@link ModuleLifecycleManager} calls this from + * {@link ModuleLifecycleManager#declareCommands}, between {@code load()} and + * {@code initializeLoaded()}, at most once per load. It must only build command trees: no IO, + * listeners or tasks, and no state that {@link #initialize} sets up. Executors and suggestion + * providers may use that state, because they only run while the module is active; otherwise + * the host's unavailable handler replies. Requirements ({@code requires}) run regardless and must + * not depend on it. Commands are never unregistered, so they need no cleanup. + * + *

If this method throws, nothing it declared is registered and the module is unloaded. + * + * @param root subcommands under the host's root command + * @param registrar top-level commands + */ + default void declareCommands(@NotNull ModuleCommandRoot root, @NotNull ModuleCommandRegistrar registrar) { + } + /** * Stops this module and releases everything {@link #initialize} registered. * diff --git a/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandDeclarationTest.java b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandDeclarationTest.java new file mode 100644 index 0000000..d3b0025 --- /dev/null +++ b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleCommandDeclarationTest.java @@ -0,0 +1,287 @@ +package com.minecraftcitiesnetwork.pluginInfrastructure.modules; + +import com.mojang.brigadier.CommandDispatcher; +import com.mojang.brigadier.arguments.StringArgumentType; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.mojang.brigadier.suggestion.Suggestions; +import com.mojang.brigadier.tree.LiteralCommandNode; +import io.papermc.paper.command.brigadier.CommandSourceStack; +import io.papermc.paper.command.brigadier.Commands; +import org.bukkit.plugin.Plugin; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.function.BiConsumer; +import java.util.logging.Logger; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +class ModuleCommandDeclarationTest { + + private static final String HOST = "HostPlugin"; + + private Plugin plugin; + private ModuleLoader loader; + private Commands commands; + private CommandSourceStack source; + private final List unavailableFor = new ArrayList<>(); + private final ModuleUnavailableHandler unavailable = (src, name) -> this.unavailableFor.add(name); + + /** A module that declares commands through a callback and can fail to initialize. */ + private static final class CommandModule implements PluginModule { + private final BiConsumer declare; + private RuntimeException initFailure; + int declareCalls; + boolean initialized; + + CommandModule(BiConsumer declare) { + this.declare = declare; + } + + @Override + public void declareCommands(@NotNull ModuleCommandRoot root, @NotNull ModuleCommandRegistrar registrar) { + this.declareCalls++; + this.declare.accept(root, registrar); + } + + @Override + public void initialize(@NotNull Plugin plugin, @NotNull Path dataFolder) { + if (this.initFailure != null) { + throw this.initFailure; + } + this.initialized = true; + } + + @Override + public void shutdown(@NotNull Plugin plugin) { + } + } + + @BeforeEach + void setUp() { + this.plugin = mock(Plugin.class); + this.loader = mock(ModuleLoader.class); + this.commands = mock(Commands.class); + this.source = mock(CommandSourceStack.class); + } + + private ModuleLifecycleManager manager(CommandModule... modules) throws Exception { + List> loaded = new ArrayList<>(); + for (int i = 0; i < modules.length; i++) { + String name = "m" + i; + ModuleManifest manifest = new ModuleManifest(name, "Entry", "author", HOST, false); + loaded.add(new LoadedModule<>(manifest, modules[i], Path.of(name), new URLClassLoader(new URL[0]))); + } + doReturn(loaded).when(this.loader).loadModulesFromDisk(anyString(), any(Logger.class)); + return new ModuleLifecycleManager<>(this.plugin, this.loader, HOST, Logger.getLogger("mcd-test")); + } + + private static LiteralArgumentBuilder ping(List ran) { + return LiteralArgumentBuilder.literal("ping").executes(ctx -> { + ran.add("ping"); + return 1; + }); + } + + /** Runs one commands event and returns a dispatcher holding the host root as "host". */ + private CommandDispatcher declare(ModuleLifecycleManager manager) { + LiteralArgumentBuilder root = LiteralArgumentBuilder.literal("host"); + manager.declareCommands(this.commands, root, this.unavailable); + CommandDispatcher dispatcher = new CommandDispatcher<>(); + dispatcher.getRoot().addChild(root.build()); + return dispatcher; + } + + @Test + @DisplayName("a subcommand runs module code once the module is active") + void subcommandRunsWhenActive() throws Exception { + List ran = new ArrayList<>(); + ModuleLifecycleManager manager = manager(new CommandModule((root, reg) -> root.then(ping(ran)))); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + manager.initializeLoaded(); + + assertEquals(1, dispatcher.execute("host ping", this.source)); + assertEquals(List.of("ping"), ran); + assertTrue(this.unavailableFor.isEmpty()); + } + + @Test + @DisplayName("commands are declared before initialize and reply unavailable until then") + void unavailableBeforeInitialize() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> root.then(ping(ran))); + ModuleLifecycleManager manager = manager(module); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + + assertFalse(module.initialized); + assertEquals(0, dispatcher.execute("host ping", this.source)); + assertTrue(ran.isEmpty()); + assertEquals(List.of("m0"), this.unavailableFor); + } + + @Test + @DisplayName("a module that fails to initialize keeps its commands, which reply unavailable") + void failedInitializeIsUnavailable() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> root.then(ping(ran))); + module.initFailure = new RuntimeException("boom"); + ModuleLifecycleManager manager = manager(module); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + manager.initializeLoaded(); + + assertEquals(0, dispatcher.execute("host ping", this.source)); + assertTrue(ran.isEmpty()); + assertEquals(List.of("m0"), this.unavailableFor); + } + + @Test + @DisplayName("commands reply unavailable after stop()") + void unavailableAfterStop() throws Exception { + List ran = new ArrayList<>(); + ModuleLifecycleManager manager = manager(new CommandModule((root, reg) -> root.then(ping(ran)))); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + manager.initializeLoaded(); + manager.stop(); + + assertEquals(0, dispatcher.execute("host ping", this.source)); + assertTrue(ran.isEmpty()); + } + + @Test + @DisplayName("nested executors are guarded too") + void nestedExecutorsGuarded() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> root.then( + LiteralArgumentBuilder.literal("home").then( + RequiredArgumentBuilder.argument("name", StringArgumentType.word()) + .executes(ctx -> { + ran.add(StringArgumentType.getString(ctx, "name")); + return 1; + })))); + module.initFailure = new RuntimeException("boom"); + ModuleLifecycleManager manager = manager(module); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + manager.initializeLoaded(); + + assertEquals(0, dispatcher.execute("host home base", this.source)); + assertTrue(ran.isEmpty()); + } + + @Test + @DisplayName("suggestions come from the module only while it is active") + void suggestionsGuarded() throws Exception { + CommandModule module = new CommandModule((root, reg) -> root.then( + LiteralArgumentBuilder.literal("home").then( + RequiredArgumentBuilder.argument("name", StringArgumentType.word()) + .suggests((ctx, builder) -> builder.suggest("base").buildFuture()) + .executes(ctx -> 1)))); + ModuleLifecycleManager manager = manager(module); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + + Suggestions before = dispatcher.getCompletionSuggestions(dispatcher.parse("host home ", this.source)).get(); + manager.initializeLoaded(); + Suggestions after = dispatcher.getCompletionSuggestions(dispatcher.parse("host home ", this.source)).get(); + + assertTrue(before.isEmpty()); + assertEquals("base", after.getList().getFirst().getText()); + } + + @Test + @DisplayName("top-level commands go to Paper's registrar guarded") + @SuppressWarnings("unchecked") + void topLevelRegisteredGuarded() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> reg.register(ping(ran), "Pings", List.of("p"))); + module.initFailure = new RuntimeException("boom"); + ModuleLifecycleManager manager = manager(module); + manager.load(); + declare(manager); + manager.initializeLoaded(); + + ArgumentCaptor> node = ArgumentCaptor.forClass(LiteralCommandNode.class); + verify(this.commands).register(node.capture(), eq("Pings"), eq(List.of("p"))); + CommandDispatcher dispatcher = new CommandDispatcher<>(); + dispatcher.getRoot().addChild(node.getValue()); + + assertEquals(0, dispatcher.execute("ping", this.source)); + assertTrue(ran.isEmpty()); + } + + @Test + @DisplayName("a module whose declareCommands throws registers nothing and is not initialized") + void declareFailureUnloads() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> { + root.then(ping(ran)); + throw new IllegalStateException("bad declaration"); + }); + ModuleLifecycleManager manager = manager(module); + manager.load(); + CommandDispatcher dispatcher = declare(manager); + manager.initializeLoaded(); + + assertFalse(module.initialized); + assertTrue(manager.getActiveModules().isEmpty()); + assertTrue(dispatcher.getRoot().getChild("host").getChildren().isEmpty()); + } + + @Test + @DisplayName("a redirect in a module command is rejected as a failed declaration") + void redirectRejected() throws Exception { + CommandModule module = new CommandModule((root, reg) -> { + LiteralCommandNode target = LiteralArgumentBuilder.literal("t").build(); + root.then(LiteralArgumentBuilder.literal("alias").redirect(target)); + }); + ModuleLifecycleManager manager = manager(module); + manager.load(); + declare(manager); + manager.initializeLoaded(); + + assertFalse(module.initialized); + } + + @Test + @DisplayName("a later commands event re-registers the same commands without asking the module again") + @SuppressWarnings("unchecked") + void laterEventReplays() throws Exception { + List ran = new ArrayList<>(); + CommandModule module = new CommandModule((root, reg) -> { + root.then(ping(ran)); + reg.register(ping(ran), null); + }); + ModuleLifecycleManager manager = manager(module); + manager.load(); + declare(manager); + manager.initializeLoaded(); + CommandDispatcher second = declare(manager); + + assertEquals(1, module.declareCalls); + assertEquals(1, second.execute("host ping", this.source)); + verify(this.commands, times(2)).register(any(LiteralCommandNode.class), eq(null), any(Collection.class)); + } +} diff --git a/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManagerTest.java b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManagerTest.java index 4ea58ec..7f58dbc 100644 --- a/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManagerTest.java +++ b/src/test/java/com/minecraftcitiesnetwork/pluginInfrastructure/modules/ModuleLifecycleManagerTest.java @@ -267,6 +267,75 @@ void reloadNullFutureBecomesCompleted() throws Exception { assertFalse(future.isCompletedExceptionally()); } + @Test + @DisplayName("load() loads modules without initializing them") + void loadDoesNotInitialize() throws Exception { + FakeModule a = new FakeModule("a", new ArrayList<>()); + stubLoader(List.of(loaded("a", false, a))); + + ModuleLifecycleManager manager = manager(); + List> loaded = manager.load(); + + assertFalse(a.initialized); + assertEquals(1, loaded.size()); + assertTrue(manager.getActiveModules().isEmpty()); + } + + @Test + @DisplayName("initializeLoaded() initializes what load() loaded and activates it") + void initializeLoadedActivates() throws Exception { + FakeModule a = new FakeModule("a", new ArrayList<>()); + stubLoader(List.of(loaded("a", false, a))); + + ModuleLifecycleManager manager = manager(); + manager.load(); + manager.initializeLoaded(); + + assertTrue(a.initialized); + assertEquals(java.util.Set.of("a"), manager.getActiveModules().keySet()); + } + + @Test + @DisplayName("initializeLoaded() without load() is rejected") + void initializeLoadedWithoutLoadThrows() { + assertThrows(IllegalStateException.class, manager()::initializeLoaded); + } + + @Test + @DisplayName("load() twice without stop() is rejected") + void loadTwiceThrows() throws Exception { + stubLoader(List.of(loaded("a", false, new FakeModule("a", new ArrayList<>())))); + ModuleLifecycleManager manager = manager(); + manager.load(); + + assertThrows(IllegalStateException.class, manager::load); + } + + @Test + @DisplayName("stop() after load() closes loaded modules without shutting them down") + void stopAfterLoadClosesWithoutShutdown() throws Exception { + List shutdowns = new ArrayList<>(); + boolean[] closed = new boolean[1]; + ModuleManifest manifest = new ModuleManifest("a", "Entry", "author", HOST, false); + URLClassLoader classLoader = new URLClassLoader(new URL[0]) { + @Override + public void close() throws IOException { + closed[0] = true; + super.close(); + } + }; + stubLoader(List.of(new LoadedModule<>(manifest, new FakeModule("a", shutdowns), Path.of("a"), classLoader))); + + ModuleLifecycleManager manager = manager(); + manager.load(); + manager.stop(); + + assertTrue(closed[0]); + assertTrue(shutdowns.isEmpty()); + // Releasing the loaded modules allows a fresh load. + manager.load(); + } + @Test @DisplayName("getActiveModules() returns an immutable snapshot") void activeModulesIsImmutableSnapshot() throws Exception {