Upstream/Minestom
1
0
Fork 0
mirror of https://github.com/Minestom/Minestom.git synced 2025-01-23 16:41:35 +01:00
Code Issues Projects Releases Wiki Activity

Move to DiscoveredExtension vs ExtensionDescription

Browse Source 
For those who are wondering why I replaced some streams:

https://stackoverflow.com/questions/16635398/java-8-iterable-foreach-vs-foreach-loop
This commit is contained in:
LeoDog896 2021-03-23 11:35:52 -04:00
parent 4c3215bf1b
commit 701b1cb2e5
7 changed files with 86 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/main/java/net/minestom/server/extensions/DiscoveredExtension.java
View File

@ -10,7 +10,7 @@ import java.net.URL;
import java.util.LinkedList;
import java.util.List;
final class DiscoveredExtension {
public final class DiscoveredExtension {
public final static Logger LOGGER = LoggerFactory.getLogger(DiscoveredExtension.class);
@ -23,7 +23,7 @@ final class DiscoveredExtension {
private String[] codeModifiers;
private String[] dependencies;
private ExternalDependencies externalDependencies;
private List<String> missingCodeModifiers = new LinkedList<>();
private final List<String> missingCodeModifiers = new LinkedList<>();
private boolean failedToLoadMixin = false;
transient List<URL> files = new LinkedList<>();
transient LoadStatus loadStatus = LoadStatus.LOAD_SUCCESS;
@ -77,7 +77,7 @@ final class DiscoveredExtension {
}
@Nullable
File getOriginalJar() {
public File getOriginalJar() {
return originalJar;
}

85
src/main/java/net/minestom/server/extensions/Extension.java
View File

@ -13,7 +13,7 @@ import java.util.function.Consumer;
public abstract class Extension {
// Set by reflection
@SuppressWarnings("unused")
private ExtensionDescription description;
private DiscoveredExtension origin;
// Set by reflection
@SuppressWarnings("unused")
private Logger logger;
@ -24,8 +24,13 @@ public abstract class Extension {
* this extension holds a reference to it. A WeakReference makes sure this extension does not prevent the memory
* from being cleaned up.
*/
private Set<WeakReference<IExtensionObserver>> observers = Collections.newSetFromMap(new ConcurrentHashMap<>());
private ReferenceQueue<IExtensionObserver> observerReferenceQueue = new ReferenceQueue<>();
protected final Set<WeakReference<IExtensionObserver>> observers = Collections.newSetFromMap(new ConcurrentHashMap<>());
protected final ReferenceQueue<IExtensionObserver> observerReferenceQueue = new ReferenceQueue<>();
/**
* List of extensions that depend on this extension.
*/
protected final Set<String> dependents = new HashSet<>();
protected Extension() {
@ -59,10 +64,14 @@ public abstract class Extension {
}
@NotNull
public ExtensionDescription getDescription() {
return description;
public DiscoveredExtension getOrigin() {
return origin;
}
/**
* Gets the logger for the extension
* @return The logger for the extension
*/
@NotNull
protected Logger getLogger() {
return logger;
@ -71,7 +80,8 @@ public abstract class Extension {
/**
* Adds a new observer to this extension.
* Will be kept as a WeakReference.
* @param observer
*
* @param observer The observer to add
*/
public void observe(IExtensionObserver observer) {
observers.add(new WeakReference<>(observer, observerReferenceQueue));
@ -82,9 +92,9 @@ public abstract class Extension {
* @param action code to execute on each observer
*/
public void triggerChange(Consumer<IExtensionObserver> action) {
for(WeakReference<IExtensionObserver> weakObserver : observers) {
for (WeakReference<IExtensionObserver> weakObserver : observers) {
IExtensionObserver observer = weakObserver.get();
if(observer != null) {
if (observer != null) {
action.accept(observer);
}
}
@ -94,7 +104,7 @@ public abstract class Extension {
* If this extension registers code modifiers and/or mixins, are they loaded correctly?
*/
public boolean areCodeModifiersAllLoadedCorrectly() {
return !getDescription().failedToLoadMixin && getDescription().getMissingCodeModifiers().isEmpty();
return !getOrigin().hasFailedToLoadMixin() && getOrigin().getMissingCodeModifiers().isEmpty();
}
/**
@ -109,57 +119,10 @@ public abstract class Extension {
}
}
public static class ExtensionDescription {
private final String name;
private final String version;
private final List<String> authors;
private final List<String> dependents = new ArrayList<>();
private final List<String> missingCodeModifiers = new LinkedList<>();
private final boolean failedToLoadMixin;
private final DiscoveredExtension origin;
ExtensionDescription(@NotNull String name, @NotNull String version, @NotNull List<String> authors, @NotNull DiscoveredExtension origin) {
this.name = name;
this.version = version;
this.authors = authors;
this.origin = origin;
failedToLoadMixin = origin.hasFailedToLoadMixin();
missingCodeModifiers.addAll(origin.getMissingCodeModifiers());
}
@NotNull
public String getName() {
return name;
}
@NotNull
public String getVersion() {
return version;
}
@NotNull
public List<String> getAuthors() {
return authors;
}
@NotNull
public List<String> getDependents() {
return dependents;
}
@NotNull
DiscoveredExtension getOrigin() {
return origin;
}
@NotNull
public List<String> getMissingCodeModifiers() {
return missingCodeModifiers;
}
@NotNull
public boolean hasFailedToLoadMixin() {
return failedToLoadMixin;
}
/**
* @return A modifiable list of dependents.
*/
public Set<String> getDependents() {
return dependents;
}
}

10
src/main/java/net/minestom/server/extensions/ExtensionDependencyResolver.java
View File

@ -37,11 +37,13 @@ public class ExtensionDependencyResolver implements DependencyResolver {
// 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<ResolvedDependency> deps = new LinkedList<>();
for (URL u : ext.files) {
deps.add(new ResolvedDependency(u.toExternalForm(), u.toExternalForm(), "", u, new LinkedList<>()));
List<ResolvedDependency> 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), deps);
return new ResolvedDependency(ext.getName(), ext.getName(), ext.getVersion(), ext.files.get(0), dependencies);
}
throw new UnresolvedDependencyException("No extension named " + extensionName);
}

84
src/main/java/net/minestom/server/extensions/ExtensionManager.java
View File

@ -24,7 +24,6 @@ import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.net.URL;
import java.net.URLClassLoader;
import java.util.*;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.stream.Collectors;
@ -148,12 +147,6 @@ public class ExtensionManager {
// Create ExtensionDescription (authors, version etc.)
final String extensionName = discoveredExtension.getName();
String mainClass = discoveredExtension.getEntrypoint();
Extension.ExtensionDescription extensionDescription = new Extension.ExtensionDescription(
extensionName,
discoveredExtension.getVersion(),
Arrays.asList(discoveredExtension.getAuthors()),
discoveredExtension
);
MinestomExtensionClassLoader loader = extensionLoaders.get(extensionName.toLowerCase());
@ -207,9 +200,9 @@ public class ExtensionManager {
// Set extension description
try {
Field descriptionField = Extension.class.getDeclaredField("description");
descriptionField.setAccessible(true);
descriptionField.set(extension, extensionDescription);
Field originField = Extension.class.getDeclaredField("origin");
originField.setAccessible(true);
originField.set(extension, discoveredExtension);
} catch (IllegalAccessException e) {
// We made it accessible, should not occur
} catch (NoSuchFieldException e) {
@ -236,7 +229,7 @@ public class ExtensionManager {
if (dep == null) {
LOGGER.warn("Dependency {} of {} is null? This means the extension has been loaded without its dependency, which could cause issues later.", dependency, discoveredExtension.getName());
} else {
dep.getDescription().getDependents().add(discoveredExtension.getName());
dep.getDependents().add(discoveredExtension.getName());
}
}
@ -306,14 +299,16 @@ public class ExtensionManager {
}
}
@Nullable
@NotNull
private List<DiscoveredExtension> generateLoadOrder(@NotNull List<DiscoveredExtension> discoveredExtensions) {
// Do some mapping so we can map strings to extensions.
Map<String, DiscoveredExtension> extensionMap = new HashMap<>();
Map<DiscoveredExtension, List<DiscoveredExtension>> dependencyMap = new HashMap<>();
for (DiscoveredExtension discoveredExtension : discoveredExtensions) {
extensionMap.put(discoveredExtension.getName().toLowerCase(), discoveredExtension);
}
for (DiscoveredExtension discoveredExtension : discoveredExtensions) {
List<DiscoveredExtension> dependencies = Arrays.stream(discoveredExtension.getDependencies())
@ -323,7 +318,7 @@ public class ExtensionManager {
if (dependencyExtension == null) {
// attempt to see if it is not already loaded (happens with dynamic (re)loading)
if (extensions.containsKey(dependencyName.toLowerCase())) {
return extensions.get(dependencyName.toLowerCase()).getDescription().getOrigin();
return extensions.get(dependencyName.toLowerCase()).getOrigin();
} else {
LOGGER.error("Extension {} requires an extension called {}.", discoveredExtension.getName(), dependencyName);
LOGGER.error("However the extension {} could not be found.", dependencyName);
@ -356,12 +351,17 @@ public class ExtensionManager {
) {
// Get all "loadable" (not actually being loaded!) extensions and put them in the sorted list.
for (Map.Entry<DiscoveredExtension, List<DiscoveredExtension>> entry : loadableExtensions) {
// Add to sorted list.
sortedList.add(entry.getKey());
// Remove to make the next iterations a little bit quicker (hopefully) and to find cyclic dependencies.
dependencyMap.remove(entry.getKey());
// Remove this dependency from all the lists (if they include it) to make way for next level of extensions.
dependencyMap.forEach((key, dependencyList) -> dependencyList.remove(entry.getKey()));
for (var dependencies : dependencyMap.values()) {
dependencies.remove(entry.getKey());
}
}
}
@ -387,46 +387,56 @@ public class ExtensionManager {
private void loadDependencies(List<DiscoveredExtension> extensions) {
List<DiscoveredExtension> allLoadedExtensions = new LinkedList<>(extensions);
extensionList.stream().map(ext -> ext.getDescription().getOrigin()).forEach(allLoadedExtensions::add);
for (Extension extension : extensionList)
allLoadedExtensions.add(extension.getOrigin());
ExtensionDependencyResolver extensionDependencyResolver = new ExtensionDependencyResolver(allLoadedExtensions);
for (DiscoveredExtension ext : extensions) {
for (DiscoveredExtension discoveredExtension : extensions) {
try {
DependencyGetter getter = new DependencyGetter();
DiscoveredExtension.ExternalDependencies externalDependencies = ext.getExternalDependencies();
DiscoveredExtension.ExternalDependencies externalDependencies = discoveredExtension.getExternalDependencies();
List<MavenRepository> repoList = new LinkedList<>();
for (var repository : externalDependencies.repositories) {
if (repository.name == null) {
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) {
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 (var artifact : externalDependencies.artifacts) {
for (String artifact : externalDependencies.artifacts) {
var resolved = getter.get(artifact, dependenciesFolder);
addDependencyFile(resolved, ext);
LOGGER.trace("Dependency of extension {}: {}", ext.getName(), resolved);
addDependencyFile(resolved, discoveredExtension);
LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved);
}
for (var dependencyName : ext.getDependencies()) {
for (String dependencyName : discoveredExtension.getDependencies()) {
var resolved = getter.get(dependencyName, dependenciesFolder);
addDependencyFile(resolved, ext);
LOGGER.trace("Dependency of extension {}: {}", ext.getName(), resolved);
addDependencyFile(resolved, discoveredExtension);
LOGGER.trace("Dependency of extension {}: {}", discoveredExtension.getName(), resolved);
}
} catch (Exception e) {
ext.loadStatus = DiscoveredExtension.LoadStatus.MISSING_DEPENDENCIES;
LOGGER.error("Failed to load dependencies for extension {}", ext.getName());
LOGGER.error("Extension '{}' will not be loaded", ext.getName());
discoveredExtension.loadStatus = DiscoveredExtension.LoadStatus.MISSING_DEPENDENCIES;
LOGGER.error("Failed to load dependencies for extension {}", discoveredExtension.getName());
LOGGER.error("Extension '{}' will not be loaded", discoveredExtension.getName());
LOGGER.error("This is the exception", e);
}
}
@ -560,7 +570,7 @@ public class ExtensionManager {
ext.preTerminate();
ext.terminate();
// remove callbacks for this extension
String extensionName = ext.getDescription().getName();
String extensionName = ext.getOrigin().getName();
ext.triggerChange(observer -> observer.onExtensionUnload(extensionName));
// TODO: more callback types
@ -570,10 +580,10 @@ public class ExtensionManager {
// remove as dependent of other extensions
// this avoids issues where a dependent extension fails to reload, and prevents the base extension to reload too
for (Extension e : extensionList) {
e.getDescription().getDependents().remove(ext.getDescription().getName());
e.getDependents().remove(ext.getOrigin().getName());
}
String id = ext.getDescription().getName().toLowerCase();
String id = ext.getOrigin().getName().toLowerCase();
// remove from loaded extensions
extensions.remove(id);
extensionList.remove(ext);
@ -595,19 +605,19 @@ public class ExtensionManager {
throw new IllegalArgumentException("Extension " + extensionName + " is not currently loaded.");
}
File originalJar = ext.getDescription().getOrigin().getOriginalJar();
File originalJar = ext.getOrigin().getOriginalJar();
if (originalJar == null) {
LOGGER.error("Cannot reload extension {} that is not from a .jar file!", extensionName);
return;
}
LOGGER.info("Reload extension {} from jar file {}", extensionName, originalJar.getAbsolutePath());
List<String> dependents = new LinkedList<>(ext.getDescription().getDependents()); // copy dependents list
List<String> dependents = new LinkedList<>(ext.getDependents()); // copy dependents list
List<File> originalJarsOfDependents = new LinkedList<>();
for (String dependentID : dependents) {
Extension dependentExt = extensions.get(dependentID.toLowerCase());
File dependentOriginalJar = dependentExt.getDescription().getOrigin().getOriginalJar();
File dependentOriginalJar = dependentExt.getOrigin().getOriginalJar();
originalJarsOfDependents.add(dependentOriginalJar);
if (dependentOriginalJar == null) {
LOGGER.error("Cannot reload extension {} that is not from a .jar file!", dependentID);
@ -696,7 +706,7 @@ public class ExtensionManager {
if (ext == null) {
throw new IllegalArgumentException("Extension " + extensionName + " is not currently loaded.");
}
List<String> dependents = new LinkedList<>(ext.getDescription().getDependents()); // copy dependents list
List<String> dependents = new LinkedList<>(ext.getDependents()); // copy dependents list
for (String dependentID : dependents) {
Extension dependentExt = extensions.get(dependentID.toLowerCase());
@ -715,7 +725,9 @@ public class ExtensionManager {
* Shutdowns all the extensions by unloading them.
*/
public void shutdown() {
this.extensionList.forEach(this::unload);
for (Extension extension : getExtensions()) {
extension.unload();
}
}
/**
@ -733,7 +745,7 @@ public class ExtensionManager {
List<DiscoveredExtension> discovered = manager.discoverExtensions();
// setup extension class loaders, so that Mixin can load the json configuration file correctly
for(DiscoveredExtension e : discovered) {
for (DiscoveredExtension e : discovered) {
manager.setupClassLoader(e);
}
@ -741,7 +753,7 @@ public class ExtensionManager {
manager.setupCodeModifiers(discovered, MinestomRootClassLoader.getInstance());
// setup is done, remove all extension classloaders
for(MinestomExtensionClassLoader extensionLoader : manager.getExtensionLoaders().values()) {
for (MinestomExtensionClassLoader extensionLoader : manager.getExtensionLoaders().values()) {
MinestomRootClassLoader.getInstance().removeChildInHierarchy(extensionLoader);
}
LOGGER.info("Early load of code modifiers from extensions done!");

2
src/test/java/demo/commands/ReloadExtensionCommand.java
View File

@ -25,7 +25,7 @@ public class ReloadExtensionCommand extends Command {
static {
ReloadExtensionCommand.extensionsName = MinecraftServer.getExtensionManager().getExtensions()
.stream()
.map(extension -> extension.getDescription().getName())
.map(extension -> extension.getOrigin().getName())
.toArray(String[]::new);
}

6
src/test/java/improveextensions/missingmodifiers/MissingCodeModifiersExtension.java
View File

@ -12,9 +12,9 @@ public class MissingCodeModifiersExtension extends Extension {
// force load of InstanceContainer class
try {
Assertions.assertFalse(areCodeModifiersAllLoadedCorrectly(), "Mixin configuration could not be loaded and code modifiers are unavailable, the failure should be reported");
Assertions.assertTrue(getDescription().hasFailedToLoadMixin(), "Mixin configuration does not exist and should not be loaded");
Assertions.assertEquals(1, getDescription().getMissingCodeModifiers().size(), "Code modifier does not exist, it should be reported as missing");
Assertions.assertEquals("InvalidCodeModifierClass", getDescription().getMissingCodeModifiers().get(0));
Assertions.assertTrue(getOrigin().hasFailedToLoadMixin(), "Mixin configuration does not exist and should not be loaded");
Assertions.assertEquals(1, getOrigin().getMissingCodeModifiers().size(), "Code modifier does not exist, it should be reported as missing");
Assertions.assertEquals("InvalidCodeModifierClass", getOrigin().getMissingCodeModifiers().get(0));
System.out.println("All tests passed.");
} catch (AssertionFailedError e) {
e.printStackTrace();

2
src/test/java/improveextensions/unloadcallbacks/UnloadCallbacksExtension.java
View File

@ -72,7 +72,7 @@ public class UnloadCallbacksExtension extends Extension {
MinecraftServer.getSchedulerManager().buildTask(() -> {
// unload self
MinecraftServer.getExtensionManager().unloadExtension(getDescription().getName());
MinecraftServer.getExtensionManager().unloadExtension(getOrigin().getName());
}).delay(1L, TimeUnit.SECOND).schedule();
}