Merged 2dbe144b

Conflicts:
	core/src/main/java/hudson/LocalPluginManager.java

core/src/main/java/hudson/ClassicPluginStrategy.java

@@ -289,8 +289,8 @@ public class ClassicPluginStrategy implements PluginStrategy {
     }
 
     public void load(PluginWrapper wrapper) throws IOException {
-        // override the context classloader so that XStream activity in plugin.start()
-        // will be able to resolve classes in this plugin
+        // override the context classloader. This no longer makes sense,
+        // but it is left for the backward compatibility
         ClassLoader old = Thread.currentThread().getContextClassLoader();
         Thread.currentThread().setContextClassLoader(wrapper.classLoader);
         try {

core/src/main/java/hudson/LocalPluginManager.java

@@ -26,6 +26,7 @@ package hudson;
 
 import jenkins.model.Jenkins;
 
+import javax.servlet.ServletContext;
 import java.io.File;
 import java.io.IOException;
 import java.net.URL;
@@ -42,10 +43,12 @@ import java.util.logging.Logger;
  * @author Kohsuke Kawaguchi
  */
 public class LocalPluginManager extends PluginManager {
-    private final Jenkins jenkins;
     public LocalPluginManager(Jenkins jenkins) {
         super(jenkins.servletContext, new File(jenkins.getRootDir(),"plugins"));
-        this.jenkins = jenkins;
+    }
+
+    public LocalPluginManager(File rootDir) {
+        super(null, new File(rootDir,"plugins"));
     }
 
     /**
@@ -63,7 +66,9 @@ public class LocalPluginManager extends PluginManager {
 
         Set<String> names = new HashSet<String>();
 
-        for( String path : Util.fixNull((Set<String>) jenkins.servletContext.getResourcePaths("/WEB-INF/plugins"))) {
+        ServletContext context = Jenkins.getInstance().servletContext;
+
+        for( String path : Util.fixNull((Set<String>)context.getResourcePaths("/WEB-INF/plugins"))) {
             String fileName = path.substring(path.lastIndexOf('/')+1);
             if(fileName.length()==0) {
                 // see http://www.nabble.com/404-Not-Found-error-when-clicking-on-help-td24508544.html
@@ -73,7 +78,7 @@ public class LocalPluginManager extends PluginManager {
             try {
                 names.add(fileName);
 
-                URL url = jenkins.servletContext.getResource(path);
+                URL url = context.getResource(path);
                 copyBundledPlugin(url, fileName);
             } catch (IOException e) {
                 LOGGER.log(Level.SEVERE, "Failed to extract the bundled plugin "+fileName,e);

core/src/main/java/hudson/PluginManager.java

@@ -358,7 +358,7 @@ public abstract class PluginManager extends AbstractModelObject {
     /**
      * Creates a hudson.PluginStrategy, looking at the corresponding system property. 
      */
-    private PluginStrategy createPluginStrategy() {
+    protected PluginStrategy createPluginStrategy() {
 		String strategyName = System.getProperty(PluginStrategy.class.getName());
 		if (strategyName != null) {
 			try {
@@ -635,16 +635,6 @@ public abstract class PluginManager extends AbstractModelObject {
                 else            generatedClasses.remove(name,wc);
             }
 
-            // first, use the context classloader so that plugins that are loading
-            // can use its own classloader first.
-            ClassLoader cl = Thread.currentThread().getContextClassLoader();
-            if(cl!=null && cl!=this)
-                try {
-                    return cl.loadClass(name);
-                } catch(ClassNotFoundException e) {
-                    // not found. try next
-                }
-
             for (PluginWrapper p : activePlugins) {
                 try {
                     return p.classLoader.loadClass(name);

test/src/main/java/org/jvnet/hudson/test/HudsonTestCase.java

@@ -244,11 +244,20 @@ public abstract class HudsonTestCase extends TestCase implements RootAction {
      * This will cause a fresh {@link PluginManager} to be created for this test.
      * Leaving this to false enables the test harness to use a pre-loaded plugin manager,
      * which runs faster.
+     *
+     * @deprecated
+     *      Use {@link #pluginManager}
      */
     public boolean useLocalPluginManager;
 
-    public ComputerConnectorTester computerConnectorTester = new ComputerConnectorTester(this);
+    /**
+     * Set the plugin manager to be passed to {@link Jenkins} constructor.
+     *
+     * For historical reasons, {@link #useLocalPluginManager}==true will take the precedence.
+     */
+    private PluginManager pluginManager = TestPluginManager.INSTANCE;
 
+    public ComputerConnectorTester computerConnectorTester = new ComputerConnectorTester(this);
 
     protected HudsonTestCase(String name) {
         super(name);
@@ -313,6 +322,8 @@ public abstract class HudsonTestCase extends TestCase implements RootAction {
             d.load();
     }
 
+
+
     /**
      * Configures the update center setting for the test.
      * By default, we load updates from local proxy to avoid network traffic as much as possible.
@@ -411,7 +422,20 @@ public abstract class HudsonTestCase extends TestCase implements RootAction {
         File home = homeLoader.allocate();
         for (Runner r : recipes)
             r.decorateHome(this,home);
-        return new Hudson(home, createWebServer(), useLocalPluginManager ? null : TestPluginManager.INSTANCE);
+        return new Hudson(home, createWebServer(), useLocalPluginManager ? null : pluginManager);
+    }
+
+    /**
+     * Sets the {@link PluginManager} to be used when creating a new {@link Jenkins} instance.
+     *
+     * @param pluginManager
+     *      null to let Jenkins create a new instance of default plugin manager, like it normally does when running as a webapp outside the test.
+     */
+    public void setPluginManager(PluginManager pluginManager) {
+        this.useLocalPluginManager = false;
+        this.pluginManager = pluginManager;
+        if (hudson!=null)
+            throw new IllegalStateException("Too late to override the plugin manager");
     }
 
     /**

test/src/main/java/org/jvnet/hudson/test/recipes/WithPluginManager.java

+/*
+ * The MIT License
+ *
+ * Copyright (c) 2011, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.jvnet.hudson.test.recipes;
+
+import hudson.PluginManager;
+import org.jvnet.hudson.test.HudsonTestCase;
+
+import java.io.File;
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+import java.lang.reflect.Constructor;
+
+import static java.lang.annotation.ElementType.*;
+import static java.lang.annotation.RetentionPolicy.*;
+
+/**
+ * Runs the test case with a custom plugin manager.
+ *
+ * @author Kohsuke Kawaguchi
+ */
+@Documented
+@Recipe(WithPluginManager.RunnerImpl.class)
+@Target(METHOD)
+@Retention(RUNTIME)
+public @interface WithPluginManager {
+    Class<? extends PluginManager> value();
+
+    public class RunnerImpl extends Recipe.Runner<WithPluginManager> {
+        private WithPluginManager recipe;
+        @Override
+        public void setup(HudsonTestCase testCase, WithPluginManager recipe) throws Exception {
+            this.recipe = recipe;
+        }
+
+        @Override
+        public void decorateHome(HudsonTestCase testCase, File home) throws Exception {
+            Class<? extends PluginManager> c = recipe.value();
+            Constructor ctr = c.getConstructors()[0];
+
+            // figure out parameters
+            Class[] pt = ctr.getParameterTypes();
+            Object[] args = new Object[pt.length];
+            for (int i=0; i<args.length; i++) {
+                Class t = pt[i];
+                if (t==File.class) {
+                    args[i] = home;
+                    continue;
+                }
+                if (t.isAssignableFrom(testCase.getClass())) {
+                    args[i] = testCase;
+                    continue;
+                }
+            }
+
+            testCase.setPluginManager((PluginManager)ctr.newInstance(args));
+        }
+    }
+
+}

test/src/test/java/hudson/PluginManagerTest.java

@@ -25,11 +25,18 @@ package hudson;
 
 import com.gargoylesoftware.htmlunit.html.HtmlForm;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import hudson.PluginManager.UberClassLoader;
+import hudson.model.Hudson;
+import hudson.scm.SubversionSCM;
 import org.apache.commons.io.FileUtils;
 import org.jvnet.hudson.test.HudsonTestCase;
+import org.jvnet.hudson.test.Url;
 import org.jvnet.hudson.test.recipes.WithPlugin;
+import org.jvnet.hudson.test.recipes.WithPluginManager;
 
 import java.io.File;
+import java.net.URL;
+import java.net.URLClassLoader;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -37,7 +44,7 @@ import java.io.File;
 public class PluginManagerTest extends HudsonTestCase {
     @Override
     protected void setUp() throws Exception {
-        useLocalPluginManager = true;
+        setPluginManager(null); // use a fresh instance
         super.setUp();
     }
 
@@ -84,4 +91,69 @@ public class PluginManagerTest extends HudsonTestCase {
         // TODO: write a separate test that tests the optional dependency loading
         tasks.classLoader.loadClass(hudson.maven.agent.AbortException.class.getName());
     }
+
+    /**
+     * Verifies that by the time {@link Plugin#start()} is called, uber classloader is fully functioning.
+     * This is necessary as plugin start method can engage in XStream loading activities, and they should
+     * resolve all the classes in the system (for example, a plugin X can define an extension point
+     * other plugins implement, so when X loads its config it better sees all the implementations defined elsewhere)
+     */
+    @WithPlugin("tasks.hpi")
+    @WithPluginManager(PluginManagerImpl_for_testUberClassLoaderIsAvailableDuringStart.class)
+    public void testUberClassLoaderIsAvailableDuringStart() {
+        assertTrue(((PluginManagerImpl_for_testUberClassLoaderIsAvailableDuringStart)hudson.pluginManager).tested);
+    }
+
+    public class PluginManagerImpl_for_testUberClassLoaderIsAvailableDuringStart extends LocalPluginManager {
+        boolean tested;
+
+        public PluginManagerImpl_for_testUberClassLoaderIsAvailableDuringStart(File rootDir) {
+            super(rootDir);
+        }
+
+        @Override
+        protected PluginStrategy createPluginStrategy() {
+            return new ClassicPluginStrategy(this) {
+                @Override
+                public void startPlugin(PluginWrapper plugin) throws Exception {
+                    tested = true;
+
+                    // plugins should be already visible in the UberClassLoader
+                    assertTrue(!activePlugins.isEmpty());
+
+                    uberClassLoader.loadClass(SubversionSCM.class.getName());
+                    uberClassLoader.loadClass("hudson.plugins.tasks.Messages");
+
+                    super.startPlugin(plugin);
+                }
+            };
+        }
+    }
+
+
+    /**
+     * Makes sure that thread context classloader isn't used by {@link UberClassLoader}, or else
+     * infinite cycle ensues.
+     */
+    @Url("http://jenkins.361315.n4.nabble.com/channel-example-and-plugin-classes-gives-ClassNotFoundException-td3756092.html")
+    public void testUberClassLoaderDoesntUseContextClassLoader() throws Exception {
+        Thread t = Thread.currentThread();
+
+        URLClassLoader ucl = new URLClassLoader(new URL[0],hudson.pluginManager.uberClassLoader);
+
+        ClassLoader old = t.getContextClassLoader();
+        t.setContextClassLoader(ucl);
+        try {
+            try {
+                ucl.loadClass("No such class");
+                fail();
+            } catch (ClassNotFoundException e) {
+                // as expected
+            }
+
+            ucl.loadClass(Hudson.class.getName());
+        } finally {
+            t.setContextClassLoader(old);
+        }
+    }
 }