From 34ff8c121740a57665765bbcee6e643ebfd89a1d Mon Sep 17 00:00:00 2001 From: chris Date: Sun, 1 Oct 2023 07:17:53 +0200 Subject: [PATCH] Allow extensions to load other extension's classes, and store extensions by IDs instead of name (#3946) - the extensionmanagers `extension` method now takes in a extension id instead of name - extension folders are now created using extension id's - Extensions can load classes from other extensions now - Added warning about external class loading - Wherever applicable: store extensions internally by id instead of name --- .../api/extension/ExtensionManager.java | 8 ++--- .../platform/spigot/GeyserSpigotPlugin.java | 2 +- .../extension/GeyserExtensionClassLoader.java | 36 +++++++++++++------ .../extension/GeyserExtensionLoader.java | 29 ++++++++++----- .../extension/GeyserExtensionManager.java | 10 +++--- 5 files changed, 56 insertions(+), 29 deletions(-) diff --git a/api/src/main/java/org/geysermc/geyser/api/extension/ExtensionManager.java b/api/src/main/java/org/geysermc/geyser/api/extension/ExtensionManager.java index a9d0d7376..5226221df 100644 --- a/api/src/main/java/org/geysermc/geyser/api/extension/ExtensionManager.java +++ b/api/src/main/java/org/geysermc/geyser/api/extension/ExtensionManager.java @@ -36,13 +36,13 @@ import java.util.Collection; public abstract class ExtensionManager { /** - * Gets an extension with the given name. + * Gets an extension by the given ID. * - * @param name the name of the extension - * @return an extension with the given name + * @param id the ID of the extension + * @return an extension with the given ID */ @Nullable - public abstract Extension extension(@NonNull String name); + public abstract Extension extension(@NonNull String id); /** * Enables the given {@link Extension}. diff --git a/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotPlugin.java b/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotPlugin.java index b932962a0..cfb6ff5ae 100644 --- a/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotPlugin.java +++ b/bootstrap/spigot/src/main/java/org/geysermc/geyser/platform/spigot/GeyserSpigotPlugin.java @@ -194,7 +194,7 @@ public class GeyserSpigotPlugin extends JavaPlugin implements GeyserBootstrap { commandMap.register(extension.description().id(), "geyserext", pluginCommand); } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException ex) { - this.geyserLogger.error("Failed to construct PluginCommand for extension " + extension.description().name(), ex); + this.geyserLogger.error("Failed to construct PluginCommand for extension " + extension.name(), ex); } } } diff --git a/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionClassLoader.java b/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionClassLoader.java index 30d6ac856..dca11dfcd 100644 --- a/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionClassLoader.java +++ b/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionClassLoader.java @@ -27,6 +27,7 @@ package org.geysermc.geyser.extension; import it.unimi.dsi.fastutil.objects.Object2ObjectMap; import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap; +import org.geysermc.geyser.GeyserImpl; import org.geysermc.geyser.api.extension.Extension; import org.geysermc.geyser.api.extension.ExtensionDescription; import org.geysermc.geyser.api.extension.exception.InvalidExtensionException; @@ -39,14 +40,17 @@ import java.nio.file.Path; public class GeyserExtensionClassLoader extends URLClassLoader { private final GeyserExtensionLoader loader; + private final ExtensionDescription description; private final Object2ObjectMap> classes = new Object2ObjectOpenHashMap<>(); + private boolean warnedForExternalClassAccess; - public GeyserExtensionClassLoader(GeyserExtensionLoader loader, ClassLoader parent, Path path) throws MalformedURLException { + public GeyserExtensionClassLoader(GeyserExtensionLoader loader, ClassLoader parent, Path path, ExtensionDescription description) throws MalformedURLException { super(new URL[] { path.toUri().toURL() }, parent); this.loader = loader; + this.description = description; } - public Extension load(ExtensionDescription description) throws InvalidExtensionException { + public Extension load() throws InvalidExtensionException { try { Class jarClass; try { @@ -76,22 +80,32 @@ public class GeyserExtensionClassLoader extends URLClassLoader { } protected Class findClass(String name, boolean checkGlobal) throws ClassNotFoundException { - if (name.startsWith("org.geysermc.geyser.") || name.startsWith("net.minecraft.")) { - throw new ClassNotFoundException(name); - } - Class result = this.classes.get(name); if (result == null) { - result = super.findClass(name); - if (result == null && checkGlobal) { - result = this.loader.classByName(name); + // Try to find class in current extension + try { + result = super.findClass(name); + } catch (ClassNotFoundException ignored) { + // If class is not found in current extension, check in the global class loader + // This is used for classes that are not in the extension, but are in other extensions + if (checkGlobal) { + if (!warnedForExternalClassAccess) { + GeyserImpl.getInstance().getLogger().warning("Extension " + this.description.name() + " loads class " + name + " from an external source. " + + "This can change at any time and break the extension, additionally to potentially causing unexpected behaviour!"); + warnedForExternalClassAccess = true; + } + result = this.loader.classByName(name); + } } if (result != null) { + // If class is found, cache it this.loader.setClass(name, result); + this.classes.put(name, result); + } else { + // If class is not found, throw exception + throw new ClassNotFoundException(name); } - - this.classes.put(name, result); } return result; } diff --git a/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionLoader.java b/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionLoader.java index 7e998e413..dac5ee802 100644 --- a/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionLoader.java +++ b/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionLoader.java @@ -66,26 +66,38 @@ public class GeyserExtensionLoader extends ExtensionLoader { } Path parentFile = path.getParent(); - Path dataFolder = parentFile.resolve(description.name()); + + // Extension folders used to be created by name; this changes them to the ID + Path oldDataFolder = parentFile.resolve(description.name()); + Path dataFolder = parentFile.resolve(description.id()); + + if (Files.exists(oldDataFolder) && Files.isDirectory(oldDataFolder) && !oldDataFolder.equals(dataFolder)) { + try { + Files.move(oldDataFolder, dataFolder, StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + throw new InvalidExtensionException("Failed to move data folder for extension " + description.name(), e); + } + } + if (Files.exists(dataFolder) && !Files.isDirectory(dataFolder)) { throw new InvalidExtensionException("The folder " + dataFolder + " is not a directory and is the data folder for the extension " + description.name() + "!"); } final GeyserExtensionClassLoader loader; try { - loader = new GeyserExtensionClassLoader(this, getClass().getClassLoader(), path); + loader = new GeyserExtensionClassLoader(this, getClass().getClassLoader(), path, description); } catch (Throwable e) { throw new InvalidExtensionException(e); } - this.classLoaders.put(description.name(), loader); + this.classLoaders.put(description.id(), loader); - final Extension extension = loader.load(description); + final Extension extension = loader.load(); return this.setup(extension, description, dataFolder, new GeyserExtensionEventBus(GeyserImpl.getInstance().eventBus(), extension)); } private GeyserExtensionContainer setup(Extension extension, GeyserExtensionDescription description, Path dataFolder, ExtensionEventBus eventBus) { - GeyserExtensionLogger logger = new GeyserExtensionLogger(GeyserImpl.getInstance().getLogger(), description.name()); + GeyserExtensionLogger logger = new GeyserExtensionLogger(GeyserImpl.getInstance().getLogger(), description.id()); return new GeyserExtensionContainer(extension, dataFolder, description, this, logger, eventBus); } @@ -152,7 +164,8 @@ public class GeyserExtensionLoader extends ExtensionLoader { GeyserExtensionDescription description = this.extensionDescription(path); String name = description.name(); - if (extensions.containsKey(name) || extensionManager.extension(name) != null) { + String id = description.id(); + if (extensions.containsKey(id) || extensionManager.extension(id) != null) { GeyserImpl.getInstance().getLogger().warning(GeyserLocale.getLocaleStringLog("geyser.extensions.load.duplicate", name, path.toString())); return; } @@ -169,8 +182,8 @@ public class GeyserExtensionLoader extends ExtensionLoader { return; } - extensions.put(name, path); - loadedExtensions.put(name, this.loadExtension(path, description)); + extensions.put(id, path); + loadedExtensions.put(id, this.loadExtension(path, description)); } catch (Exception e) { GeyserImpl.getInstance().getLogger().error(GeyserLocale.getLocaleStringLog("geyser.extensions.load.failed_with_name", path.getFileName(), path.toAbsolutePath()), e); } diff --git a/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionManager.java b/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionManager.java index 5dd924301..3c41c4329 100644 --- a/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionManager.java +++ b/core/src/main/java/org/geysermc/geyser/extension/GeyserExtensionManager.java @@ -52,8 +52,8 @@ public class GeyserExtensionManager extends ExtensionManager { } @Override - public Extension extension(@NonNull String name) { - return this.extensions.get(name); + public Extension extension(@NonNull String id) { + return this.extensions.get(id); } @Override @@ -83,7 +83,7 @@ public class GeyserExtensionManager extends ExtensionManager { if (!extension.isEnabled()) { extension.setEnabled(true); GeyserImpl.getInstance().eventBus().register(extension, extension); - GeyserImpl.getInstance().getLogger().info(GeyserLocale.getLocaleStringLog("geyser.extensions.enable.success", extension.description().name())); + GeyserImpl.getInstance().getLogger().info(GeyserLocale.getLocaleStringLog("geyser.extensions.enable.success", extension.name())); } } @@ -98,7 +98,7 @@ public class GeyserExtensionManager extends ExtensionManager { GeyserImpl.getInstance().eventBus().unregisterAll(extension); extension.setEnabled(false); - GeyserImpl.getInstance().getLogger().info(GeyserLocale.getLocaleStringLog("geyser.extensions.disable.success", extension.description().name())); + GeyserImpl.getInstance().getLogger().info(GeyserLocale.getLocaleStringLog("geyser.extensions.disable.success", extension.name())); } } @@ -121,6 +121,6 @@ public class GeyserExtensionManager extends ExtensionManager { @Override public void register(@NonNull Extension extension) { - this.extensions.put(extension.name(), extension); + this.extensions.put(extension.description().id(), extension); } }