Make PluginContainer injectable (#272)

proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java

@@ -4,11 +4,18 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.base.Joiner;
+import com.google.inject.AbstractModule;
+import com.google.inject.Module;
+import com.google.inject.name.Names;
+import com.velocitypowered.api.command.CommandManager;
+import com.velocitypowered.api.event.EventManager;
 import com.velocitypowered.api.plugin.PluginContainer;
 import com.velocitypowered.api.plugin.PluginDescription;
 import com.velocitypowered.api.plugin.PluginManager;
 import com.velocitypowered.api.plugin.meta.PluginDependency;
+import com.velocitypowered.api.proxy.ProxyServer;
 import com.velocitypowered.proxy.VelocityServer;
+import com.velocitypowered.proxy.plugin.loader.VelocityPluginContainer;
 import com.velocitypowered.proxy.plugin.loader.java.JavaPluginLoader;
 import com.velocitypowered.proxy.plugin.util.PluginDependencyUtils;
 import java.io.IOException;
@@ -80,6 +87,7 @@ public class VelocityPluginManager implements PluginManager {
     found.sort(Comparator.comparing(PluginDescription::getId));
     List<PluginDescription> sortedPlugins = PluginDependencyUtils.sortCandidates(found);
 
+    Map<PluginContainer, Module> pluginContainers = new HashMap<>();
     // Now load the plugins
     pluginLoad:
     for (PluginDescription plugin : sortedPlugins) {
@@ -92,19 +100,44 @@ public class VelocityPluginManager implements PluginManager {
         }
       }
 
-      // Actually create the plugin
+      try {
-      PluginContainer pluginObject;
+        VelocityPluginContainer container = new VelocityPluginContainer(plugin);
+        pluginContainers.put(container, loader.createModule(container));
+      } catch (Exception e) {
+        logger.error("Can't create module for plugin {}", plugin.getId(), e);
+      }
+    }
+
+    // Make a global Guice module that with common bindings for every plugin
+    AbstractModule commonModule = new AbstractModule() {
+      @Override
+      protected void configure() {
+        bind(ProxyServer.class).toInstance(server);
+        bind(PluginManager.class).toInstance(server.getPluginManager());
+        bind(EventManager.class).toInstance(server.getEventManager());
+        bind(CommandManager.class).toInstance(server.getCommandManager());
+        for (PluginContainer container : pluginContainers.keySet()) {
+          bind(PluginContainer.class)
+            .annotatedWith(Names.named(container.getDescription().getId()))
+            .toInstance(container);
+        }
+      }
+    };
+
+    for (Map.Entry<PluginContainer, Module> plugin : pluginContainers.entrySet()) {
+      PluginContainer container = plugin.getKey();
+      PluginDescription description = container.getDescription();
 
       try {
-        pluginObject = loader.createPlugin(plugin);
+        loader.createPlugin(container, plugin.getValue(), commonModule);
       } catch (Exception e) {
-        logger.error("Can't create plugin {}", plugin.getId(), e);
+        logger.error("Can't create plugin {}", description.getId(), e);
         continue;
       }
 
-      logger.info("Loaded plugin {} {} by {}", plugin.getId(), plugin.getVersion()
+      logger.info("Loaded plugin {} {} by {}", description.getId(), description.getVersion()
-          .orElse("<UNKNOWN>"), Joiner.on(", ").join(plugin.getAuthors()));
+          .orElse("<UNKNOWN>"), Joiner.on(", ").join(description.getAuthors()));
-      registerPlugin(pluginObject);
+      registerPlugin(container);
     }
   }
 

proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/PluginLoader.java

@@ -1,7 +1,9 @@
 package com.velocitypowered.proxy.plugin.loader;
 
+import com.google.inject.Module;
 import com.velocitypowered.api.plugin.PluginContainer;
 import com.velocitypowered.api.plugin.PluginDescription;
+
 import java.nio.file.Path;
 
 /**
@@ -11,5 +13,32 @@ public interface PluginLoader {
 
   PluginDescription loadPlugin(Path source) throws Exception;
 
-  PluginContainer createPlugin(PluginDescription plugin) throws Exception;
+  /**
+   * Creates a {@link Module} for the provided {@link PluginContainer}
+   * and verifies that the container's {@link PluginDescription} is correct.
+   *
+   * <p>Does not create an instance of the plugin.</p>
+   *
+   * @param container the plugin container
+   * @return the module containing bindings specific to this plugin
+   * @throws IllegalArgumentException If the {@link PluginDescription}
+   *                                  is missing the path
+   */
+  Module createModule(PluginContainer container) throws Exception;
+
+  /**
+   * Creates an instance of the plugin as specified by the
+   * plugin's main class found in the {@link PluginDescription}.
+   *
+   * <p>The provided {@link Module modules} are used to create an
+   * injector which is then used to create the plugin instance.</p>
+   *
+   * <p>The plugin instance is set in the provided {@link PluginContainer}.</p>
+   *
+   * @param container the plugin container
+   * @param modules   the modules to be used when creating this plugin's injector
+   * @throws IllegalStateException If the plugin instance could not be
+   *                               created from the provided modules
+   */
+  void createPlugin(PluginContainer container, Module... modules) throws Exception;
 }

proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/VelocityPluginContainer.java

@@ -7,11 +7,10 @@ import java.util.Optional;
 public class VelocityPluginContainer implements PluginContainer {
 
   private final PluginDescription description;
-  private final Object instance;
+  private Object instance;
 
-  public VelocityPluginContainer(PluginDescription description, Object instance) {
+  public VelocityPluginContainer(PluginDescription description) {
     this.description = description;
-    this.instance = instance;
   }
 
   @Override
@@ -23,4 +22,8 @@ public class VelocityPluginContainer implements PluginContainer {
   public Optional<?> getInstance() {
     return Optional.ofNullable(instance);
   }
+
+  public void setInstance(Object instance) {
+    this.instance = instance;
+  }
 }

proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/JavaPluginLoader.java

@@ -2,6 +2,7 @@ package com.velocitypowered.proxy.plugin.loader.java;
 
 import com.google.inject.Guice;
 import com.google.inject.Injector;
+import com.google.inject.Module;
 import com.velocitypowered.api.plugin.InvalidPluginException;
 import com.velocitypowered.api.plugin.PluginContainer;
 import com.velocitypowered.api.plugin.PluginDescription;
@@ -13,8 +14,6 @@ import com.velocitypowered.proxy.plugin.PluginClassLoader;
 import com.velocitypowered.proxy.plugin.loader.PluginLoader;
 import com.velocitypowered.proxy.plugin.loader.VelocityPluginContainer;
 import com.velocitypowered.proxy.plugin.loader.VelocityPluginDescription;
-import com.velocitypowered.proxy.plugin.loader.java.JavaVelocityPluginDescription;
-import com.velocitypowered.proxy.plugin.loader.java.VelocityPluginModule;
 import java.io.BufferedInputStream;
 import java.io.InputStreamReader;
 import java.io.Reader;
@@ -61,7 +60,8 @@ public class JavaPluginLoader implements PluginLoader {
   }
 
   @Override
-  public PluginContainer createPlugin(PluginDescription description) throws Exception {
+  public Module createModule(PluginContainer container) throws Exception {
+    PluginDescription description = container.getDescription();
     if (!(description instanceof JavaVelocityPluginDescription)) {
       throw new IllegalArgumentException("Description provided isn't of the Java plugin loader");
     }
@@ -73,16 +73,29 @@ public class JavaPluginLoader implements PluginLoader {
       throw new IllegalArgumentException("No path in plugin description");
     }
 
-    Injector injector = Guice
+    return new VelocityPluginModule(server, javaDescription, container, baseDirectory);
-        .createInjector(new VelocityPluginModule(server, javaDescription, baseDirectory));
+  }
-    Object instance = injector.getInstance(javaDescription.getMainClass());
+
+  @Override
+  public void createPlugin(PluginContainer container, Module... modules) {
+    if (!(container instanceof VelocityPluginContainer)) {
+      throw new IllegalArgumentException("Container provided isn't of the Java plugin loader");
+    }
+    PluginDescription description = container.getDescription();
+    if (!(description instanceof JavaVelocityPluginDescription)) {
+      throw new IllegalArgumentException("Description provided isn't of the Java plugin loader");
+    }
+
+    Injector injector = Guice.createInjector(modules);
+    Object instance = injector
+        .getInstance(((JavaVelocityPluginDescription) description).getMainClass());
 
     if (instance == null) {
       throw new IllegalStateException(
-          "Got nothing from injector for plugin " + javaDescription.getId());
+        "Got nothing from injector for plugin " + description.getId());
     }
 
-    return new VelocityPluginContainer(description, instance);
+    ((VelocityPluginContainer) container).setInstance(instance);
   }
 
   private Optional<SerializedPluginDescription> getSerializedPluginInfo(Path source)

proxy/src/main/java/com/velocitypowered/proxy/plugin/loader/java/VelocityPluginModule.java

@@ -3,10 +3,8 @@ package com.velocitypowered.proxy.plugin.loader.java;
 import com.google.inject.Binder;
 import com.google.inject.Module;
 import com.google.inject.Scopes;
-import com.velocitypowered.api.command.CommandManager;
+import com.velocitypowered.api.plugin.PluginContainer;
-import com.velocitypowered.api.event.EventManager;
 import com.velocitypowered.api.plugin.PluginDescription;
-import com.velocitypowered.api.plugin.PluginManager;
 import com.velocitypowered.api.plugin.annotation.DataDirectory;
 import com.velocitypowered.api.proxy.ProxyServer;
 import java.nio.file.Path;
@@ -17,12 +15,14 @@ class VelocityPluginModule implements Module {
 
   private final ProxyServer server;
   private final JavaVelocityPluginDescription description;
+  private final PluginContainer pluginContainer;
   private final Path basePluginPath;
 
   VelocityPluginModule(ProxyServer server, JavaVelocityPluginDescription description,
-      Path basePluginPath) {
+                       PluginContainer pluginContainer, Path basePluginPath) {
     this.server = server;
     this.description = description;
+    this.pluginContainer = pluginContainer;
     this.basePluginPath = basePluginPath;
   }
 
@@ -31,12 +31,9 @@ class VelocityPluginModule implements Module {
     binder.bind(description.getMainClass()).in(Scopes.SINGLETON);
 
     binder.bind(Logger.class).toInstance(LoggerFactory.getLogger(description.getId()));
-    binder.bind(ProxyServer.class).toInstance(server);
     binder.bind(Path.class).annotatedWith(DataDirectory.class)
         .toInstance(basePluginPath.resolve(description.getId()));
     binder.bind(PluginDescription.class).toInstance(description);
-    binder.bind(PluginManager.class).toInstance(server.getPluginManager());
+    binder.bind(PluginContainer.class).toInstance(pluginContainer);
-    binder.bind(EventManager.class).toInstance(server.getEventManager());
-    binder.bind(CommandManager.class).toInstance(server.getCommandManager());
   }
 }