diff --git a/src/main/java/net/minestom/server/extensions/DiscoveredExtension.java b/src/main/java/net/minestom/server/extensions/DiscoveredExtension.java index 637e20da1..8e1f13ae2 100644 --- a/src/main/java/net/minestom/server/extensions/DiscoveredExtension.java +++ b/src/main/java/net/minestom/server/extensions/DiscoveredExtension.java @@ -206,7 +206,7 @@ public final class DiscoveredExtension { return meta; } - public MinestomExtensionClassLoader makeClassLoader(List discoveredExtensions) { + public MinestomExtensionClassLoader makeClassLoader() { final URL[] urls = this.files.toArray(new URL[0]); MinestomExtensionClassLoader loader = new MinestomExtensionClassLoader(this.getName(), urls); @@ -214,21 +214,21 @@ public final class DiscoveredExtension { System.out.println("CREATED " + loader + " WITH " + Arrays.toString(urls)); // add children to the dependencies - for (String dependencyName : this.getDependencies()) { - DiscoveredExtension dependency = discoveredExtensions.stream() - .filter(ext -> ext.getName().equalsIgnoreCase(dependencyName)) - .findFirst().orElse(null); - - if (dependency != null) { - MinestomExtensionClassLoader dependencyLoader = dependency.getMinestomExtensionClassLoader(); - assert dependencyLoader != null; //TODO: Better error handling - - loader.addChild(dependencyLoader); - } else { - //TODO: Better error handling - throw new RuntimeException("Missing dependency " + dependencyName); - } - } +// for (String dependencyName : this.getDependencies()) { +// DiscoveredExtension dependency = discoveredExtensions.stream() +// .filter(ext -> ext.getName().equalsIgnoreCase(dependencyName)) +// .findFirst().orElse(null); +// +// if (dependency != null) { +// MinestomExtensionClassLoader dependencyLoader = dependency.getMinestomExtensionClassLoader(); +// assert dependencyLoader != null; //TODO: Better error handling +// +// loader.addChild(dependencyLoader); +// } else { +// //TODO: Better error handling +// throw new RuntimeException("Missing dependency " + dependencyName); +// } +// } // if (this.getDependencies().length == 0 || MinecraftServer.getExtensionManager() == null) { // it also may invoked in early class loader // // orphaned extension, we can insert it directly diff --git a/src/main/java/net/minestom/server/extensions/ExtensionDependencyResolver.java b/src/main/java/net/minestom/server/extensions/ExtensionDependencyResolver.java deleted file mode 100644 index 300cdc4c1..000000000 --- a/src/main/java/net/minestom/server/extensions/ExtensionDependencyResolver.java +++ /dev/null @@ -1,59 +0,0 @@ -package net.minestom.server.extensions; - -import net.minestom.dependencies.DependencyResolver; -import net.minestom.dependencies.ResolvedDependency; -import net.minestom.dependencies.UnresolvedDependencyException; -import org.jetbrains.annotations.NotNull; - -import java.io.File; -import java.net.URL; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; - -/** - * Does NOT relocate extensions - */ -public class ExtensionDependencyResolver implements DependencyResolver { - - private final Map extensionMap = new HashMap<>(); - - public ExtensionDependencyResolver(@NotNull List extensions) { - for (DiscoveredExtension ext : extensions) { - extensionMap.put(ext.getName(), ext); - } - } - - @NotNull - @Override - public ResolvedDependency resolve(@NotNull String extensionName, @NotNull File file) throws UnresolvedDependencyException { - if (extensionMap.containsKey(extensionName)) { - DiscoveredExtension ext = extensionMap.get(extensionName); - // convert extension URLs to subdependencies - // FIXME: this is not a deep conversion, this might create an issue in this scenario with different classloaders: - // A depends on an external lib (Ext<-A) - // B depends on A (A<-B) - // When loading B, with no deep conversion, Ext will not be added to the list of dependencies (because it is not a direct dependency) - // But when trying to call/access code from extension A, the parts dependent on Ext won't be inside B's dependencies, triggering a ClassNotFoundException - List dependencies = new LinkedList<>(); - - for (URL url : ext.files) { - dependencies.add(new ResolvedDependency(url.toExternalForm(), url.toExternalForm(), "", url, new LinkedList<>())); - } - - return new ResolvedDependency(ext.getName(), ext.getName(), ext.getVersion(), ext.files.get(0), dependencies); - } - throw new UnresolvedDependencyException("No extension named " + extensionName); - } - - @Override - public String toString() { - String list = extensionMap.values() - .stream() - .map(DiscoveredExtension::getName) - .collect(Collectors.joining(", ")); - return "ExtensionDependencyResolver[" + list + "]"; - } -} diff --git a/src/main/java/net/minestom/server/extensions/ExtensionManager.java b/src/main/java/net/minestom/server/extensions/ExtensionManager.java index 32024fc68..794190365 100644 --- a/src/main/java/net/minestom/server/extensions/ExtensionManager.java +++ b/src/main/java/net/minestom/server/extensions/ExtensionManager.java @@ -168,6 +168,7 @@ public class ExtensionManager { } } + //TODO(mattw) What is the purpose of this? // Periodically cleanup observers MinecraftServer.getSchedulerManager().buildTask(() -> { for (Extension ext : extensions.values()) { @@ -183,25 +184,26 @@ public class ExtensionManager { // Don't waste resources on doing extra actions if there is nothing to do. if (discoveredExtensions.isEmpty()) return; - discoveredExtensions = generateLoadOrder(discoveredExtensions); - loadDependencies(discoveredExtensions); - - // remove invalid extensions - discoveredExtensions.removeIf(ext -> ext.loadStatus != DiscoveredExtension.LoadStatus.LOAD_SUCCESS); - - // set class loaders for all extensions. - for (DiscoveredExtension discoveredExtension : discoveredExtensions) { + // Create classloaders for each extension (so that they can be used during dependency resolution) + Iterator extensionIterator = discoveredExtensions.iterator(); + while (extensionIterator.hasNext()) { + DiscoveredExtension discoveredExtension = extensionIterator.next(); try { - discoveredExtension.setMinestomExtensionClassLoader(discoveredExtension.makeClassLoader(discoveredExtensions)); //TODO: This is a hack to pass the dependent DiscoveredExtensions to the classloader + MinestomExtensionClassLoader classLoader = discoveredExtension.makeClassLoader(); + discoveredExtension.setMinestomExtensionClassLoader(classLoader); System.out.println("SET " + discoveredExtension.getName() + " TO " + discoveredExtension.getMinestomExtensionClassLoader()); } catch (Exception e) { discoveredExtension.loadStatus = DiscoveredExtension.LoadStatus.FAILED_TO_SETUP_CLASSLOADER; MinecraftServer.getExceptionManager().handleException(e); LOGGER.error("Failed to load extension {}", discoveredExtension.getName()); LOGGER.error("Failed to load extension", e); + extensionIterator.remove(); } } + discoveredExtensions = generateLoadOrder(discoveredExtensions); + loadDependencies(discoveredExtensions); + // remove invalid extensions discoveredExtensions.removeIf(ext -> ext.loadStatus != DiscoveredExtension.LoadStatus.LOAD_SUCCESS); @@ -382,6 +384,9 @@ public class ExtensionManager { } } + //TODO(mattw): Extract this into its own method to load an extension given classes and resources directory. + //TODO(mattw): Should show a warning if one is set and not the other. It is most likely a mistake. + // this allows developers to have their extension discovered while working on it, without having to build a jar and put in the extension folder if (System.getProperty(INDEV_CLASSES_FOLDER) != null && System.getProperty(INDEV_RESOURCES_FOLDER) != null) { LOGGER.info("Found indev folders for extension. Adding to list of discovered extensions."); @@ -558,8 +563,6 @@ public class ExtensionManager { for (Extension extension : immutableExtensions.values()) allLoadedExtensions.add(extension.getOrigin()); - ExtensionDependencyResolver extensionDependencyResolver = new ExtensionDependencyResolver(allLoadedExtensions); - for (DiscoveredExtension discoveredExtension : extensions) { try { DependencyGetter getter = new DependencyGetter(); @@ -567,27 +570,18 @@ public class ExtensionManager { List repoList = new LinkedList<>(); for (var repository : externalDependencies.repositories) { - if (repository.name == null) { + if (repository.name == null || repository.name.isEmpty()) { throw new IllegalStateException("Missing 'name' element in repository object."); } - if (repository.name.isEmpty()) { - throw new IllegalStateException("Invalid 'name' element in repository object."); - } - - if (repository.url == null) { + if (repository.url == null || repository.url.isEmpty()) { throw new IllegalStateException("Missing 'url' element in repository object."); } - if (repository.url.isEmpty()) { - throw new IllegalStateException("Invalid 'url' element in repository object."); - } - repoList.add(new MavenRepository(repository.name, repository.url)); } getter.addMavenResolver(repoList); -// getter.addResolver(extensionDependencyResolver); for (String artifact : externalDependencies.artifacts) { var resolved = getter.get(artifact, dependenciesFolder); @@ -595,11 +589,20 @@ public class ExtensionManager { LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved); } -// for (String dependencyName : discoveredExtension.getDependencies()) { -// var resolved = getter.get(dependencyName, dependenciesFolder); -// addDependencyFile(resolved, discoveredExtension); -// LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved); -// } + MinestomExtensionClassLoader extensionClassLoader = discoveredExtension.getMinestomExtensionClassLoader(); + for (String dependencyName : discoveredExtension.getDependencies()) { + var resolved = extensions.stream() + .filter(ext -> ext.getName().equalsIgnoreCase(dependencyName)) + .findFirst().orElse(null); + + MinestomExtensionClassLoader dependencyClassLoader = resolved.getMinestomExtensionClassLoader(); + if (dependencyClassLoader == null) { + throw new IllegalStateException("Dependency '" + dependencyName + "' of '" + discoveredExtension.getName() + "' has not been loaded."); + } + + extensionClassLoader.addChild(dependencyClassLoader); + LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved); + } } catch (Exception e) { discoveredExtension.loadStatus = DiscoveredExtension.LoadStatus.MISSING_DEPENDENCIES; LOGGER.error("Failed to load dependencies for extension {}", discoveredExtension.getName()); @@ -613,6 +616,7 @@ public class ExtensionManager { System.out.println("ADDING DEPENDENCY " + dependency.getName() + " TO " + extension.getName()); URL location = dependency.getContentsLocation(); extension.files.add(location); + extension.getMinestomExtensionClassLoader().addURL(location); LOGGER.trace("Added dependency {} to extension {} classpath", location.toExternalForm(), extension.getName()); // recurse to add full dependency tree diff --git a/src/main/java/net/minestom/server/extensions/isolation/HierarchyClassLoader.java b/src/main/java/net/minestom/server/extensions/isolation/HierarchyClassLoader.java index 2542c448b..5477c9bf6 100644 --- a/src/main/java/net/minestom/server/extensions/isolation/HierarchyClassLoader.java +++ b/src/main/java/net/minestom/server/extensions/isolation/HierarchyClassLoader.java @@ -18,6 +18,11 @@ public abstract class HierarchyClassLoader extends URLClassLoader { super(name, urls, parent); } + @Override + public void addURL(@NotNull URL url) { + super.addURL(url); + } + public void addChild(@NotNull MinestomExtensionClassLoader loader) { children.add(loader); }