From 9b8c8f6bc870bffc14313434045b294197a13c0d Mon Sep 17 00:00:00 2001 From: tastybento Date: Fri, 7 Jun 2019 17:37:16 -0700 Subject: [PATCH] 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. --- .../bentobox/api/addons/AddonClassLoader.java | 8 ++ .../commands/BentoBoxReloadCommand.java | 24 ++--- .../bentobox/managers/AddonsManager.java | 87 +++++++++---------- .../bentobox/managers/CommandsManager.java | 30 ++++++- src/main/resources/locales/en-US.yml | 4 +- src/main/resources/locales/it-IT.yml | 3 +- src/main/resources/locales/lv-LV.yml | 1 - .../bentobox/managers/AddonsManagerTest.java | 18 ---- 8 files changed, 82 insertions(+), 93 deletions(-) diff --git a/src/main/java/world/bentobox/bentobox/api/addons/AddonClassLoader.java b/src/main/java/world/bentobox/bentobox/api/addons/AddonClassLoader.java index 9f3b72380..66116bd2f 100644 --- 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 getClasses() { + return classes.keySet(); + } + } diff --git a/src/main/java/world/bentobox/bentobox/commands/BentoBoxReloadCommand.java b/src/main/java/world/bentobox/bentobox/commands/BentoBoxReloadCommand.java index d06f043f3..00c1fcc46 100644 --- 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 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 = 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); } diff --git a/src/main/java/world/bentobox/bentobox/managers/AddonsManager.java b/src/main/java/world/bentobox/bentobox/managers/AddonsManager.java index 44dd114bc..9485757b2 100644 --- 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 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 diff --git a/src/main/java/world/bentobox/bentobox/managers/CommandsManager.java b/src/main/java/world/bentobox/bentobox/managers/CommandsManager.java index 97ba770b0..e3d3c1107 100644 --- 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 knownCommands = (Map) 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 diff --git a/src/main/resources/locales/en-US.yml b/src/main/resources/locales/en-US.yml index 80fdac352..6177f039b 100644 --- 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]" diff --git a/src/main/resources/locales/it-IT.yml b/src/main/resources/locales/it-IT.yml index efc3d747d..4447cf080 100644 --- 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." diff --git a/src/main/resources/locales/lv-LV.yml b/src/main/resources/locales/lv-LV.yml index 2985e4b6b..1c1ddca3a 100644 --- 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!' diff --git a/src/test/java/world/bentobox/bentobox/managers/AddonsManagerTest.java b/src/test/java/world/bentobox/bentobox/managers/AddonsManagerTest.java index b97e989df..0e49e8ebb 100644 --- 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)}. */