[BREAKING] Shift plugin initialization; Addresses BUKKIT-1788

This reverts commit ae4f1c05d825e232d7fc0483639ba65ad54d2db4, restoring
commit 27cb5e7c9c6b2cfc5419262df75d89bc6bfe7879 (mostly).

Shared class loading was removed as an explicit feature in the plugin.yml,
as all plugins implicitly share class loaders already.

Some deprecated, internal functionality is now (package) private, namely
some sections pointed out in 203de4180b40f069d2c175d763476bd4ce338c76.

By: Wesley Wolfe <weswolf@aol.com>

paper-api/src/main/java/org/bukkit/plugin/PluginDescriptionFile.java

@@ -773,6 +773,10 @@ public final class PluginDescriptionFile {
         return name + " v" + version;
     }
 
+    /**
+     * @deprecated unused
+     */
+    @Deprecated
     public String getClassLoaderOf() {
         return classLoaderOf;
     }

paper-api/src/main/java/org/bukkit/plugin/java/JavaPlugin.java

@@ -13,12 +13,14 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import org.bukkit.Server;
+import org.bukkit.Warning.WarningState;
 import org.bukkit.command.Command;
 import org.bukkit.command.CommandSender;
 import org.bukkit.command.PluginCommand;
 import org.bukkit.configuration.file.FileConfiguration;
 import org.bukkit.configuration.file.YamlConfiguration;
 import org.bukkit.generator.ChunkGenerator;
+import org.bukkit.plugin.AuthorNagException;
 import org.bukkit.plugin.PluginBase;
 import org.bukkit.plugin.PluginDescriptionFile;
 import org.bukkit.plugin.PluginLoader;
@@ -36,7 +38,6 @@ import com.avaje.ebeaninternal.server.ddl.DdlGenerator;
  */
 public abstract class JavaPlugin extends PluginBase {
     private boolean isEnabled = false;
-    private boolean initialized = false;
     private PluginLoader loader = null;
     private Server server = null;
     private File file = null;
@@ -49,7 +50,37 @@ public abstract class JavaPlugin extends PluginBase {
     private File configFile = null;
     private PluginLogger logger = null;
 
-    public JavaPlugin() {}
+    public JavaPlugin() {
+        final ClassLoader classLoader = this.getClass().getClassLoader();
+        if (!(classLoader instanceof PluginClassLoader)) {
+            throw new IllegalStateException("JavaPlugin requires " + PluginClassLoader.class.getName());
+        }
+        ((PluginClassLoader) classLoader).initialize(this);
+    }
+
+    /**
+     * @deprecated This method is intended for unit testing purposes when the
+     *     other {@linkplain #JavaPlugin(JavaPluginLoader,
+     *     PluginDescriptionFile, File, File) constructor} cannot be used.
+     *     <p>
+     *     Its existence may be temporary.
+     */
+    @Deprecated
+    protected JavaPlugin(final PluginLoader loader, final Server server, final PluginDescriptionFile description, final File dataFolder, final File file) {
+        final ClassLoader classLoader = this.getClass().getClassLoader();
+        if (classLoader instanceof PluginClassLoader) {
+            throw new IllegalStateException("Cannot use initialization constructor at runtime");
+        }
+        init(loader, server, description, dataFolder, file, classLoader);
+    }
+
+    protected JavaPlugin(final JavaPluginLoader loader, final PluginDescriptionFile description, final File dataFolder, final File file) {
+        final ClassLoader classLoader = this.getClass().getClassLoader();
+        if (classLoader instanceof PluginClassLoader) {
+            throw new IllegalStateException("Cannot use initialization constructor at runtime");
+        }
+        init(loader, loader.server, description, dataFolder, file, classLoader);
+    }
 
     /**
      * Returns the folder that the plugin data's files are located in. The
@@ -223,21 +254,18 @@ public abstract class JavaPlugin extends PluginBase {
     }
 
     /**
-     * Initializes this plugin with the given variables.
+     * @deprecated This method is legacy and will be removed - it must be
-     * <p>
+     *     replaced by the specially provided constructor(s).
-     * This method should never be called manually.
-     *
-     * @param loader PluginLoader that is responsible for this plugin
-     * @param server Server instance that is running this plugin
-     * @param description PluginDescriptionFile containing metadata on this
-     *     plugin
-     * @param dataFolder Folder containing the plugin's data
-     * @param file File containing this plugin
-     * @param classLoader ClassLoader which holds this plugin
      */
+    @Deprecated
     protected final void initialize(PluginLoader loader, Server server, PluginDescriptionFile description, File dataFolder, File file, ClassLoader classLoader) {
-        if (!initialized) {
+        if (server.getWarningState() == WarningState.OFF) {
-            this.initialized = true;
+            return;
+        }
+        getLogger().log(Level.WARNING, getClass().getName() + " is already initialized", server.getWarningState() == WarningState.DEFAULT ? null : new AuthorNagException("Explicit initialization"));
+    }
+
+    final void init(PluginLoader loader, Server server, PluginDescriptionFile description, File dataFolder, File file, ClassLoader classLoader) {
         this.loader = loader;
         this.server = server;
         this.file = file;
@@ -268,7 +296,6 @@ public abstract class JavaPlugin extends PluginBase {
             Thread.currentThread().setContextClassLoader(previous);
         }
     }
-    }
 
     /**
      * Provides a list of all classes that should be persisted in the database
@@ -289,9 +316,12 @@ public abstract class JavaPlugin extends PluginBase {
      * Gets the initialization status of this plugin
      *
      * @return true if this plugin is initialized, otherwise false
+     * @deprecated This method cannot return false, as {@link
+     *     JavaPlugin} is now initialized in the constructor.
      */
+    @Deprecated
     public final boolean isInitialized() {
-        return initialized;
+        return true;
     }
 
     /**

paper-api/src/main/java/org/bukkit/plugin/java/JavaPluginLoader.java

@@ -4,10 +4,8 @@ import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
-import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.net.URL;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -49,44 +47,19 @@ import com.google.common.collect.ImmutableList;
 /**
  * Represents a Java plugin loader, allowing plugins in the form of .jar
  */
-public class JavaPluginLoader implements PluginLoader {
+public final class JavaPluginLoader implements PluginLoader {
     final Server server;
-    final boolean extended = this.getClass() != JavaPluginLoader.class;
+    private final Pattern[] fileFilters = new Pattern[] { Pattern.compile("\\.jar$"), };
-    boolean warn;
+    private final Map<String, Class<?>> classes = new HashMap<String, Class<?>>();
-
+    private final Map<String, PluginClassLoader> loaders = new LinkedHashMap<String, PluginClassLoader>();
-    private final Pattern[] fileFilters0 = new Pattern[] { Pattern.compile("\\.jar$"), };
-    /**
-     * @deprecated Internal field that wasn't intended to be exposed
-     */
-    @Deprecated
-    protected final Pattern[] fileFilters = fileFilters0;
-
-    private final Map<String, Class<?>> classes0 = new HashMap<String, Class<?>>();
-    /**
-     * @deprecated Internal field that wasn't intended to be exposed
-     */
-    @Deprecated
-    protected final Map<String, Class<?>> classes = classes0;
-
-    private final Map<String, PluginClassLoader> loaders0 = new LinkedHashMap<String, PluginClassLoader>();
-    /**
-     * @deprecated Internal field that wasn't intended to be exposed
-     */
-    @Deprecated
-    protected final Map<String, PluginClassLoader> loaders = loaders0;
 
     /**
-     * This class was not meant to be extended
+     * This class was not meant to be constructed explicitly
      */
     @Deprecated
     public JavaPluginLoader(Server instance) {
         Validate.notNull(instance, "Server cannot be null");
         server = instance;
-        warn = instance.getWarningState() != WarningState.OFF;
-        if (extended && warn) {
-            warn = false;
-            instance.getLogger().log(Level.WARNING, "JavaPluginLoader not intended to be extended by " + getClass() + ", and may be final in a future version of Bukkit");
-        }
     }
 
     public Plugin loadPlugin(File file) throws InvalidPluginException {
@@ -104,7 +77,7 @@ public class JavaPluginLoader implements PluginLoader {
         }
 
         File dataFolder = new File(file.getParentFile(), description.getName());
-        File oldDataFolder = extended ? getDataFolder(file) : getDataFolder0(file); // Don't warn on deprecation, but maintain overridability
+        File oldDataFolder = getDataFolder(file);
 
         // Found old data folder
         if (dataFolder.equals(oldDataFolder)) {
@@ -145,75 +118,31 @@ public class JavaPluginLoader implements PluginLoader {
         }
 
         for (String pluginName : depend) {
-            if (loaders0 == null) {
+            if (loaders == null) {
                 throw new UnknownDependencyException(pluginName);
             }
-            PluginClassLoader current = loaders0.get(pluginName);
+            PluginClassLoader current = loaders.get(pluginName);
 
             if (current == null) {
                 throw new UnknownDependencyException(pluginName);
             }
         }
 
-        PluginClassLoader loader = null;
+        PluginClassLoader loader;
-        JavaPlugin result = null;
-
         try {
-            URL[] urls = new URL[1];
+            loader = new PluginClassLoader(this, getClass().getClassLoader(), description, dataFolder, file);
-
+        } catch (InvalidPluginException ex) {
-            urls[0] = file.toURI().toURL();
+            throw ex;
-
-            if (description.getClassLoaderOf() != null) {
-                loader = loaders0.get(description.getClassLoaderOf());
-                loader.addURL(urls[0]);
-            } else {
-                loader = new PluginClassLoader(this, urls, getClass().getClassLoader(), null);
-            }
-
-            Class<?> jarClass = Class.forName(description.getMain(), true, loader);
-            Class<? extends JavaPlugin> plugin = jarClass.asSubclass(JavaPlugin.class);
-
-            Constructor<? extends JavaPlugin> constructor = plugin.getConstructor();
-
-            result = constructor.newInstance();
-
-            result.initialize(this, server, description, dataFolder, file, loader);
-        } catch (InvocationTargetException ex) {
-            throw new InvalidPluginException(ex.getCause());
         } catch (Throwable ex) {
             throw new InvalidPluginException(ex);
         }
 
-        loaders0.put(description.getName(), loader);
+        loaders.put(description.getName(), loader);
 
-        return result;
+        return loader.plugin;
     }
 
-    /**
+    private File getDataFolder(File file) {
-     * @deprecated Relic method from PluginLoader that didn't get purged
-     */
-    @Deprecated
-    public Plugin loadPlugin(File file, boolean ignoreSoftDependencies) throws InvalidPluginException {
-        if (warn) {
-            server.getLogger().log(Level.WARNING, "Method \"public Plugin loadPlugin(File, boolean)\" is Deprecated, and may be removed in a future version of Bukkit", new AuthorNagException(""));
-            warn = false;
-        }
-        return loadPlugin(file);
-    }
-
-    /**
-     * @deprecated Internal method that wasn't intended to be exposed
-     */
-    @Deprecated
-    protected File getDataFolder(File file) {
-        if (warn) {
-            server.getLogger().log(Level.WARNING, "Method \"protected File getDataFolder(File)\" is Deprecated, and may be removed in a future version of Bukkit", new AuthorNagException(""));
-            warn = false;
-        }
-        return getDataFolder0(file);
-    }
-
-    private File getDataFolder0(File file) {
         File dataFolder = null;
 
         String filename = file.getName();
@@ -272,32 +201,20 @@ public class JavaPluginLoader implements PluginLoader {
     }
 
     public Pattern[] getPluginFileFilters() {
-        return fileFilters0.clone();
+        return fileFilters.clone();
     }
 
-    /**
+    Class<?> getClassByName(final String name) {
-     * @deprecated Internal method that wasn't intended to be exposed
+        Class<?> cachedClass = classes.get(name);
-     */
-    @Deprecated
-    public Class<?> getClassByName(final String name) {
-        if (warn) {
-            server.getLogger().log(Level.WARNING, "Method \"public Class<?> getClassByName(String)\" is Deprecated, and may be removed in a future version of Bukkit", new AuthorNagException(""));
-            warn = false;
-        }
-        return getClassByName0(name);
-    }
-
-    Class<?> getClassByName0(final String name) {
-        Class<?> cachedClass = classes0.get(name);
 
         if (cachedClass != null) {
             return cachedClass;
         } else {
-            for (String current : loaders0.keySet()) {
+            for (String current : loaders.keySet()) {
-                PluginClassLoader loader = loaders0.get(current);
+                PluginClassLoader loader = loaders.get(current);
 
                 try {
-                    cachedClass = loader.extended ? loader.findClass(name, false) : loader.findClass0(name, false); // Don't warn on deprecation, but maintain overridability
+                    cachedClass = loader.findClass(name, false);
                 } catch (ClassNotFoundException cnfe) {}
                 if (cachedClass != null) {
                     return cachedClass;
@@ -307,21 +224,9 @@ public class JavaPluginLoader implements PluginLoader {
         return null;
     }
 
-    /**
+    void setClass(final String name, final Class<?> clazz) {
-     * @deprecated Internal method that wasn't intended to be exposed
+        if (!classes.containsKey(name)) {
-     */
+            classes.put(name, clazz);
-    @Deprecated
-    public void setClass(final String name, final Class<?> clazz) {
-        if (warn) {
-            server.getLogger().log(Level.WARNING, "Method \"public void setClass(String, Class<?>)\" is Deprecated, and may be removed in a future version of Bukkit", new AuthorNagException(""));
-            warn = false;
-        }
-        setClass0(name, clazz);
-    }
-
-    void setClass0(final String name, final Class<?> clazz) {
-        if (!classes0.containsKey(name)) {
-            classes0.put(name, clazz);
 
             if (ConfigurationSerializable.class.isAssignableFrom(clazz)) {
                 Class<? extends ConfigurationSerializable> serializable = clazz.asSubclass(ConfigurationSerializable.class);
@@ -330,20 +235,8 @@ public class JavaPluginLoader implements PluginLoader {
         }
     }
 
-    /**
+    private void removeClass(String name) {
-     * @deprecated Internal method that wasn't intended to be exposed
+        Class<?> clazz = classes.remove(name);
-     */
-    @Deprecated
-    public void removeClass(String name) {
-        if (warn) {
-            server.getLogger().log(Level.WARNING, "Method \"public void removeClass(String)\" is Deprecated, and may be removed in a future version of Bukkit", new AuthorNagException(""));
-            warn = false;
-        }
-        removeClass0(name);
-    }
-
-    private void removeClass0(String name) {
-        Class<?> clazz = classes0.remove(name);
 
         try {
             if ((clazz != null) && (ConfigurationSerializable.class.isAssignableFrom(clazz))) {
@@ -449,8 +342,8 @@ public class JavaPluginLoader implements PluginLoader {
 
             String pluginName = jPlugin.getDescription().getName();
 
-            if (!loaders0.containsKey(pluginName)) {
+            if (!loaders.containsKey(pluginName)) {
-                loaders0.put(pluginName, (PluginClassLoader) jPlugin.getClassLoader());
+                loaders.put(pluginName, (PluginClassLoader) jPlugin.getClassLoader());
             }
 
             try {
@@ -483,18 +376,14 @@ public class JavaPluginLoader implements PluginLoader {
                 server.getLogger().log(Level.SEVERE, "Error occurred while disabling " + plugin.getDescription().getFullName() + " (Is it up to date?)", ex);
             }
 
-            loaders0.remove(jPlugin.getDescription().getName());
+            loaders.remove(jPlugin.getDescription().getName());
 
             if (cloader instanceof PluginClassLoader) {
                 PluginClassLoader loader = (PluginClassLoader) cloader;
-                Set<String> names = loader.extended ? loader.getClasses() : loader.getClasses0(); // Don't warn on deprecation, but maintain overridability
+                Set<String> names = loader.getClasses();
 
                 for (String name : names) {
-                    if (extended) {
                     removeClass(name);
-                    } else {
-                        removeClass0(name);
-                    }
                 }
             }
         }

paper-api/src/main/java/org/bukkit/plugin/java/PluginClassLoader.java

@@ -1,71 +1,68 @@
 package org.bukkit.plugin.java;
 
+import java.io.File;
+import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
-import java.util.logging.Level;
 
 import org.apache.commons.lang.Validate;
-import org.bukkit.plugin.AuthorNagException;
+import org.bukkit.plugin.InvalidPluginException;
+import org.bukkit.plugin.PluginDescriptionFile;
 
 /**
  * A ClassLoader for plugins, to allow shared classes across multiple plugins
  */
-public class PluginClassLoader extends URLClassLoader {
+final class PluginClassLoader extends URLClassLoader {
     private final JavaPluginLoader loader;
     private final Map<String, Class<?>> classes = new HashMap<String, Class<?>>();
-    final boolean extended = this.getClass() != PluginClassLoader.class;
+    private final PluginDescriptionFile description;
+    private final File dataFolder;
+    private final File file;
+    final JavaPlugin plugin;
+    private JavaPlugin pluginInit;
+    private IllegalStateException pluginState;
 
-    /**
+    PluginClassLoader(final JavaPluginLoader loader, final ClassLoader parent, final PluginDescriptionFile description, final File dataFolder, final File file) throws InvalidPluginException, MalformedURLException {
-     * Internal class not intended to be exposed
+        super(new URL[] {file.toURI().toURL()}, parent);
-     */
-    @Deprecated
-    public PluginClassLoader(final JavaPluginLoader loader, final URL[] urls, final ClassLoader parent) {
-        this(loader, urls, parent, null);
-
-        if (loader.warn) {
-            if (extended) {
-                loader.server.getLogger().log(Level.WARNING, "PluginClassLoader not intended to be extended by " + getClass() + ", and may be final in a future version of Bukkit");
-            } else {
-                loader.server.getLogger().log(Level.WARNING, "Constructor \"public PluginClassLoader(JavaPluginLoader, URL[], ClassLoader)\" is Deprecated, and may be removed in a future version of Bukkit", new AuthorNagException(""));
-            }
-            loader.warn = false;
-        }
-    }
-
-
-    PluginClassLoader(final JavaPluginLoader loader, final URL[] urls, final ClassLoader parent, final Object methodSignature) {
-        super(urls, parent);
         Validate.notNull(loader, "Loader cannot be null");
 
         this.loader = loader;
+        this.description = description;
+        this.dataFolder = dataFolder;
+        this.file = file;
+
+        try {
+            Class<?> jarClass;
+            try {
+                jarClass = Class.forName(description.getMain(), true, this);
+            } catch (ClassNotFoundException ex) {
+                throw new InvalidPluginException("Cannot find main class `" + description.getMain() + "'", ex);
             }
 
-    @Override
+            Class<? extends JavaPlugin> pluginClass;
-    public void addURL(URL url) { // Override for access level!
+            try {
-        super.addURL(url);
+                pluginClass = jarClass.asSubclass(JavaPlugin.class);
+            } catch (ClassCastException ex) {
+                throw new InvalidPluginException("main class `" + description.getMain() + "' does not extend JavaPlugin", ex);
+            }
+
+            plugin = pluginClass.newInstance();
+        } catch (IllegalAccessException ex) {
+            throw new InvalidPluginException("No public constructor", ex);
+        } catch (InstantiationException ex) {
+            throw new InvalidPluginException("Abnormal plugin type", ex);
+        }
     }
 
     @Override
     protected Class<?> findClass(String name) throws ClassNotFoundException {
-        return extended ? findClass(name, true) : findClass0(name, true); // Don't warn on deprecation, but maintain overridability
+        return findClass(name, true);
     }
 
-    /**
+    Class<?> findClass(String name, boolean checkGlobal) throws ClassNotFoundException {
-     * @deprecated Internal method that wasn't intended to be exposed
-     */
-    @Deprecated
-    protected Class<?> findClass(String name, boolean checkGlobal) throws ClassNotFoundException {
-        if (loader.warn) {
-            loader.server.getLogger().log(Level.WARNING, "Method \"protected Class<?> findClass(String, boolean)\" is Deprecated, and may be removed in a future version of Bukkit", new AuthorNagException(""));
-            loader.warn = false;
-        }
-        return findClass0(name, checkGlobal);
-    }
-
-    Class<?> findClass0(String name, boolean checkGlobal) throws ClassNotFoundException {
         if (name.startsWith("org.bukkit.") || name.startsWith("net.minecraft.")) {
             throw new ClassNotFoundException(name);
         }
@@ -73,18 +70,14 @@ public class PluginClassLoader extends URLClassLoader {
 
         if (result == null) {
             if (checkGlobal) {
-                result = loader.extended ? loader.getClassByName(name) : loader.getClassByName0(name); // Don't warn on deprecation, but maintain overridability
+                result = loader.getClassByName(name);
             }
 
             if (result == null) {
                 result = super.findClass(name);
 
                 if (result != null) {
-                    if (loader.extended) { // Don't warn on deprecation, but maintain overridability
                     loader.setClass(name, result);
-                    } else {
-                        loader.setClass0(name, result);
-                    }
                 }
             }
 
@@ -94,19 +87,20 @@ public class PluginClassLoader extends URLClassLoader {
         return result;
     }
 
-    /**
+    Set<String> getClasses() {
-     * @deprecated Internal method that wasn't intended to be exposed
-     */
-    @Deprecated
-    public Set<String> getClasses() {
-        if (loader.warn) {
-            loader.server.getLogger().log(Level.WARNING, "Method \"public Set<String> getClasses()\" is Deprecated, and may be removed in a future version of Bukkit", new AuthorNagException(""));
-            loader.warn = false;
-        }
-        return getClasses0();
-    }
-
-    Set<String> getClasses0() {
         return classes.keySet();
     }
+
+    synchronized void initialize(JavaPlugin javaPlugin) {
+        Validate.notNull(javaPlugin, "Initializing plugin cannot be null");
+        Validate.isTrue(javaPlugin.getClass().getClassLoader() == this, "Cannot initialize plugin outside of this class loader");
+        if (this.plugin != null || this.pluginInit != null) {
+            throw new IllegalArgumentException("Plugin already intialized!", pluginState);
+        }
+
+        pluginState = new IllegalStateException("Initial initialization");
+        this.pluginInit = javaPlugin;
+
+        javaPlugin.init(loader, loader.server, description, dataFolder, file, this);
+    }
 }