fixes issue with bbox reload command

https://github.com/BentoBoxWorld/BentoBox/issues/731 Issue was that classes were not being fully removed from class loaders and commands needed to be unregistered from Bukkit. For the latter, reflection was required to obtain the knownCommand map and change it because there is no Bukkit API to unregister commands.
2024-12-11 20:06:42 +01:00 · 2019-06-07 17:37:16 -07:00 · 2019-06-07 17:37:16 -07:00 · 9b8c8f6bc8
commit 9b8c8f6bc8
parent 9da79f3bc2
8 changed files with 82 additions and 93 deletions
--- a/src/main/java/world/bentobox/bentobox/api/addons/AddonClassLoader.java
+++ b/src/main/java/world/bentobox/bentobox/api/addons/AddonClassLoader.java
@ -21,6 +21,7 @@ import java.net.URLClassLoader;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Set;

 /**
 * Loads addons and sets up permissions
@ -155,4 +156,11 @@ public class AddonClassLoader extends URLClassLoader {
        return addon;
    }

+    /**
+     * @return class list
+     */
+    public Set<String> getClasses() {
+        return classes.keySet();
+    }
+
 }
--- a/src/main/java/world/bentobox/bentobox/commands/BentoBoxReloadCommand.java
+++ b/src/main/java/world/bentobox/bentobox/commands/BentoBoxReloadCommand.java
@ -1,15 +1,13 @@
 package world.bentobox.bentobox.commands;

-import world.bentobox.bentobox.api.addons.Addon;
-import world.bentobox.bentobox.api.commands.CompositeCommand;
-import world.bentobox.bentobox.api.commands.ConfirmableCommand;
-import world.bentobox.bentobox.api.localization.TextVariables;
-import world.bentobox.bentobox.api.user.User;
-
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;

+import world.bentobox.bentobox.api.commands.CompositeCommand;
+import world.bentobox.bentobox.api.commands.ConfirmableCommand;
+import world.bentobox.bentobox.api.user.User;
+
 /**
 * Reloads settings, addons and localization.
 *
@ -36,7 +34,7 @@ public class BentoBoxReloadCommand extends ConfirmableCommand {
    @Override
    public boolean execute(User user, String label, List<String> args) {
        if (args.isEmpty()) {
-            this.askConfirmation(user, () -> {
+            this.askConfirmation(user, user.getTranslation("commands.bentobox.reload.warning"), () -> {
                // Reload settings
                getPlugin().loadSettings();
                user.sendMessage("commands.bentobox.reload.settings-reloaded");
@ -49,18 +47,6 @@ public class BentoBoxReloadCommand extends ConfirmableCommand {
                getPlugin().getLocalesManager().reloadLanguages();
                user.sendMessage("commands.bentobox.reload.locales-reloaded");
            });
-        } else if (args.size() == 1) {
-            Optional<Addon> addon = getPlugin().getAddonsManager().getAddonByName(args.get(0));
-            if (!addon.isPresent()) {
-                user.sendMessage("commands.bentobox.reload.unknown-addon");
-                return false;
-            }
-
-            this.askConfirmation(user, () -> {
-                user.sendMessage("commands.bentobox.reload.addon", TextVariables.NAME, addon.get().getDescription().getName());
-                addon.ifPresent(getPlugin().getAddonsManager()::reloadAddon);
-                user.sendMessage("commands.bentobox.reload.addon-reloaded", TextVariables.NAME, addon.get().getDescription().getName());
-            });
        } else {
            showHelp(this, user);
        }
--- a/src/main/java/world/bentobox/bentobox/managers/AddonsManager.java
+++ b/src/main/java/world/bentobox/bentobox/managers/AddonsManager.java
@ -1,27 +1,9 @@
 package world.bentobox.bentobox.managers;

-import org.bukkit.Bukkit;
-import org.bukkit.configuration.InvalidConfigurationException;
-import org.bukkit.configuration.file.YamlConfiguration;
-import org.bukkit.event.HandlerList;
-import org.bukkit.event.Listener;
-import org.bukkit.generator.ChunkGenerator;
-import org.eclipse.jdt.annotation.NonNull;
-import org.eclipse.jdt.annotation.Nullable;
-import world.bentobox.bentobox.BentoBox;
-import world.bentobox.bentobox.api.addons.Addon;
-import world.bentobox.bentobox.api.addons.AddonClassLoader;
-import world.bentobox.bentobox.api.addons.GameModeAddon;
-import world.bentobox.bentobox.api.addons.exceptions.InvalidAddonFormatException;
-import world.bentobox.bentobox.api.configuration.ConfigObject;
-import world.bentobox.bentobox.api.events.addon.AddonEvent;
-import world.bentobox.bentobox.database.objects.DataObject;
-
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStreamReader;
-import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@ -35,6 +17,26 @@ import java.util.jar.JarEntry;
 import java.util.jar.JarFile;
 import java.util.stream.Collectors;

+import org.bukkit.Bukkit;
+import org.bukkit.configuration.InvalidConfigurationException;
+import org.bukkit.configuration.file.YamlConfiguration;
+import org.bukkit.event.HandlerList;
+import org.bukkit.event.Listener;
+import org.bukkit.generator.ChunkGenerator;
+import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.Nullable;
+
+import world.bentobox.bentobox.BentoBox;
+import world.bentobox.bentobox.api.addons.Addon;
+import world.bentobox.bentobox.api.addons.Addon.State;
+import world.bentobox.bentobox.api.addons.AddonClassLoader;
+import world.bentobox.bentobox.api.addons.GameModeAddon;
+import world.bentobox.bentobox.api.addons.exceptions.InvalidAddonFormatException;
+import world.bentobox.bentobox.api.configuration.ConfigObject;
+import world.bentobox.bentobox.api.events.addon.AddonEvent;
+import world.bentobox.bentobox.commands.BentoBoxCommand;
+import world.bentobox.bentobox.database.objects.DataObject;
+
 /**
 * @author tastybento, ComminQ
 */
@ -213,19 +215,29 @@ public class AddonsManager {
     */
    public void reloadAddons() {
        disableAddons();
+        // Reload BentoBox commands
+        new BentoBoxCommand();
        loadAddons();
        enableAddons();
    }

    /**
-     * Reloads one addon
-     * @param addon - addon
+     * Disable all the enabled addons
     */
-    public void reloadAddon(Addon addon) {
-        Path p = addon.getFile().toPath();
-        disable(addon);
-        loadAddon(p.toFile());
-        enableAddon(addon);
+    public void disableAddons() {
+        if (!getEnabledAddons().isEmpty()) {
+            plugin.log("Disabling addons...");
+            // Disable addons
+            getEnabledAddons().forEach(this::disable);
+            plugin.log("Addons successfully disabled.");
+        }
+        // Unregister all commands
+        plugin.getCommandsManager().unregisterCommands();
+        // Clear all maps
+        listeners.clear();
+        addons.clear();
+        loaders.clear();
+        classes.clear();
    }

    /**
@ -253,23 +265,6 @@ public class AddonsManager {
        return data;
    }

-    /**
-     * Disable all the enabled addons
-     */
-    public void disableAddons() {
-        if (!getEnabledAddons().isEmpty()) {
-            plugin.log("Disabling addons...");
-            // Disable addons
-            getEnabledAddons().forEach(this::disable);
-            plugin.log("Addons successfully disabled.");
-        }
-        // Clear all maps
-        listeners.clear();
-        addons.clear();
-        loaders.clear();
-        classes.clear();
-    }
-
    @NonNull
    public List<Addon> getAddons() {
        return addons;
@ -422,11 +417,9 @@ public class AddonsManager {
        }
        // Clear loaders
        if (loaders.containsKey(addon)) {
-            try {
-                loaders.get(addon).close();
-            } catch (IOException ignore) {
-                // Nothing
-            }
+            loaders.get(addon).getClasses().forEach(classes::remove);
+            addon.setState(State.DISABLED);
+            loaders.remove(addon);
        }

        // Remove it from the addons list
--- a/src/main/java/world/bentobox/bentobox/managers/CommandsManager.java
+++ b/src/main/java/world/bentobox/bentobox/managers/CommandsManager.java
@ -7,16 +7,19 @@ import java.util.Map;
 import java.util.Set;

 import org.bukkit.Bukkit;
-import org.bukkit.command.CommandMap;
+import org.bukkit.command.Command;
+import org.bukkit.command.SimpleCommandMap;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;

+import world.bentobox.bentobox.BentoBox;
 import world.bentobox.bentobox.api.commands.CompositeCommand;

 public class CommandsManager {

    @NonNull
    private Map<@NonNull String, @NonNull CompositeCommand> commands = new HashMap<>();
+    private SimpleCommandMap commandMap;

    public void registerCommand(@NonNull CompositeCommand command) {
        commands.put(command.getLabel(), command);
@ -24,20 +27,39 @@ public class CommandsManager {
        try{
            Field commandMapField = Bukkit.getServer().getClass().getDeclaredField("commandMap");
            commandMapField.setAccessible(true);
-            CommandMap commandMap = (CommandMap) commandMapField.get(Bukkit.getServer());
+            commandMap = (SimpleCommandMap) commandMapField.get(Bukkit.getServer());

            String commandPrefix = "bentobox";
            if (command.getAddon() != null) {
                commandPrefix = command.getAddon().getDescription().getName().toLowerCase(Locale.ENGLISH);
            }
-
-            commandMap.register(commandPrefix, command);
+            if (!commandMap.register(commandPrefix, command)) {
+                BentoBox.getInstance().logError("Failed to register command " + commandPrefix + " " + command.getLabel());
+            }
        }
        catch(Exception exception){
            Bukkit.getLogger().severe("Bukkit server commandMap method is not there! This means no commands can be registered!");
        }
    }

+    /**
+     * Unregisters all BentoBox registered commands with Bukkit
+     */
+    public void unregisterCommands() {
+        // Use reflection to obtain the knownCommands in the commandMap
+        try {
+            @SuppressWarnings("unchecked")
+            Map<String, Command> knownCommands = (Map<String, Command>) commandMap.getClass().getMethod("getKnownCommands").invoke(commandMap);
+            knownCommands.values().removeIf(commands.values()::contains);
+            // Not sure if this is needed, but it clears out all references
+            commands.values().forEach(c -> c.unregister(commandMap));
+            // Zap everything
+            commands.clear();
+        } catch(Exception e){
+            Bukkit.getLogger().severe("Known commands reflection was not possible, BentoBox is now unstable, so restart server!");
+        }
+    }
+
    /**
     * Try to get a registered command.
     * @param command - command string
--- a/src/main/resources/locales/en-US.yml
+++ b/src/main/resources/locales/en-US.yml
@ -305,13 +305,13 @@ commands:
    about:
      description: "display copyright and license info"
    reload:
-      parameters: "[addon]"
-      description: "reloads all addons, settings and locales or reloads an individual addon"
+      description: "reloads BentoBox and all addons, settings and locales"
      locales-reloaded: "&2Languages reloaded."
      addons-reloaded: "&2Addons reloaded."
      settings-reloaded: "&2Settings reloaded."
      addon: "&6Reloading &b[name]&2."
      addon-reloaded: "&b[name] &2reloaded."
+      warning: "&cWarning: Reloading may cause instability, so if you see errors afterwards, restart the server"
      unknown-addon: "&2Unknown addon!"
    version:
      plugin-version: "&2BentoBox version: &3[version]"
--- a/src/main/resources/locales/it-IT.yml
+++ b/src/main/resources/locales/it-IT.yml
@ -300,8 +300,7 @@ commands:
    about:
      description: "mostra le info riguardo copyright e licenza"
    reload:
-      parameters: "[addon]"
-      description: "ricarica gli addons, le impostazioni, i locali o ricarica un singolo addon"
+      description: "ricarica gli addons, le impostazioni, i locali"
      locales-reloaded: "&2Linguaggi ricaricati."
      addons-reloaded: "&2Addons ricaricati."
      settings-reloaded: "&2Impostazioni ricaricate."
--- a/src/main/resources/locales/lv-LV.yml
+++ b/src/main/resources/locales/lv-LV.yml
@ -313,7 +313,6 @@ commands:
      locales-reloaded: "&2Valodas faili pārlādēti."
      addons-reloaded: "&2Papildinājumu pārlādēti."
      settings-reloaded: "&2Iestatījumi pārlādēti."
-      parameters: '[addon]'
      addon: '&6Pārlādē &b[name]&2.'
      addon-reloaded: '&b[name] &2pārlādēts.'
      unknown-addon: '&2Nezināms papildinājums!'
--- a/src/test/java/world/bentobox/bentobox/managers/AddonsManagerTest.java
+++ b/src/test/java/world/bentobox/bentobox/managers/AddonsManagerTest.java
@ -29,7 +29,6 @@ import org.powermock.reflect.Whitebox;

 import world.bentobox.bentobox.BentoBox;
 import world.bentobox.bentobox.api.addons.Addon;
-import world.bentobox.bentobox.api.addons.AddonDescription;
 import world.bentobox.bentobox.database.objects.DataObject;

@RunWith(PowerMockRunner.class)
@ -105,23 +104,6 @@ public class AddonsManagerTest {
        verify(plugin, never()).log("Disabling addons...");
    }

-    /**
-     * Test method for {@link world.bentobox.bentobox.managers.AddonsManager#reloadAddon(world.bentobox.bentobox.api.addons.Addon)}.
-     */
-    @Test
-    public void testReloadAddon() {
-        Addon addon = mock(Addon.class);
-        when(addon.isEnabled()).thenReturn(true);
-        File f = new File(plugin.getDataFolder(), "addons");
-        File file = new File(f, "addon.jar");
-        when(addon.getFile()).thenReturn(file);
-        AddonDescription desc = new AddonDescription.Builder("main", "addon-name", "1.0").build();
-        when(addon.getDescription()).thenReturn(desc);
-        am.reloadAddon(addon);
-        verify(plugin).log("Disabling addon-name...");
-        verify(addon).onDisable();
-    }
-
    /**
     * Test method for {@link world.bentobox.bentobox.managers.AddonsManager#getAddonByName(java.lang.String)}.
     */