[FIXED JENKINS-19976] Merged #1054: fix class loading from dynamically...

[FIXED JENKINS-19976] Merged #1054: fix class loading from dynamically installed optional dependencies.

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

@@ -377,6 +377,45 @@ public class ClassicPluginStrategy implements PluginStrategy {
         plugin.getPlugin().start();
     }
 
+    /**
+     * Called when a plugin is deployed, and there is a plugin optionally depending on that plugin.
+     * The class loader of the existing depending plugin should be updated
+     * to load classes from the newly deployed plugin.
+     * 
+     * @param depender the plugin to update its class loader
+     * @param dependee
+     * @see hudson.PluginStrategy#updateDependency(hudson.PluginWrapper, hudson.PluginWrapper)
+     */
+    @Override
+    public void updateDependency(PluginWrapper depender, PluginWrapper dependee) {
+        DependencyClassLoader classLoader = findAncestorDependencyClassLoader(depender.classLoader);
+        if (classLoader != null) {
+            classLoader.updateTransientDependencies();
+            LOGGER.log(Level.INFO, "Updated dependency of {0}", depender.getShortName());
+        }
+    }
+
+    private DependencyClassLoader findAncestorDependencyClassLoader(ClassLoader classLoader)
+    {
+        for (; classLoader != null; classLoader = classLoader.getParent()) {
+            if (classLoader instanceof DependencyClassLoader) {
+                return (DependencyClassLoader)classLoader;
+            }
+            
+            if (classLoader instanceof AntClassLoader) {
+                // AntClassLoaders hold parents not only as AntClassLoader#getParent()
+                // but also as AntClassLoader#getConfiguredParent()
+                DependencyClassLoader ret = findAncestorDependencyClassLoader(
+                        ((AntClassLoader)classLoader).getConfiguredParent()
+                );
+                if (ret != null) {
+                    return ret;
+                }
+            }
+        }
+        return null;
+    }
+
     private static File resolve(File base, String relative) {
         File rel = new File(relative);
         if(rel.isAbsolute())
@@ -523,6 +562,11 @@ public class ClassicPluginStrategy implements PluginStrategy {
             this.dependencies = dependencies;
         }
 
+        private void updateTransientDependencies() {
+            // This will be recalculated at the next time.
+            transientDependencies = null;
+        }
+
         private List<PluginWrapper> getTransitiveDependencies() {
             if (transientDependencies==null) {
                 CyclicGraphDetector<PluginWrapper> cgd = new CyclicGraphDetector<PluginWrapper>() {

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

@@ -453,6 +453,23 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
         } catch (ReactorException e) {
             throw new IOException("Failed to initialize "+ sn +" plugin",e);
         }
+        
+        // recalculate dependencies of plugins optionally depending the newly deployed one.
+        for (PluginWrapper depender: plugins) {
+            if (depender.equals(p)) {
+                // skip itself.
+                continue;
+            }
+            for (Dependency d: depender.getOptionalDependencies()) {
+                if (d.shortName.equals(p.getShortName())) {
+                    // this plugin depends on the newly loaded one!
+                    // recalculate dependencies!
+                    getPluginStrategy().updateDependency(depender, p);
+                    break;
+                }
+            }
+        }
+        
         LOGGER.info("Plugin " + sn + " dynamically installed");
     }
 

core/src/main/java/hudson/PluginStrategy.java

@@ -77,4 +77,12 @@ public interface PluginStrategy extends ExtensionPoint {
 	 * @since 1.400
 	 */
 	<T> List<ExtensionComponent<T>> findComponents(Class<T> type, Hudson hudson);
+    
+    /**
+     * Called when a plugin that is depended by another plugin is newly deployed.
+     * 
+     * @param depender plugin depending on dependee.
+     * @param dependee newly loaded plugin.
+     */
+    void updateDependency(PluginWrapper depender, PluginWrapper dependee);
 }

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

@@ -41,6 +41,7 @@ import org.jvnet.hudson.test.recipes.WithPlugin;
 import org.jvnet.hudson.test.recipes.WithPluginManager;
 
 import java.io.File;
+import java.lang.reflect.Method;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.util.Collections;
@@ -222,4 +223,129 @@ public class PluginManagerTest extends HudsonTestCase {
         // TODO required plugin installed but inactive
     }
 
+    // plugin "depender" optionally depends on plugin "dependee".
+    // they are written like this:
+    // org.jenkinsci.plugins.dependencytest.dependee:
+    //   public class Dependee {
+    //     public static String getValue() {
+    //       return "dependee";
+    //     }
+    //   }
+    //   
+    //   public abstract class DependeeExtensionPoint implements ExtensionPoint {
+    //   }
+    //   
+    // org.jenkinsci.plugins.dependencytest.depender:
+    //   public class Depender {
+    //     public static String getValue() {
+    //       if (Jenkins.getInstance().getPlugin("dependee") != null) {
+    //         return Dependee.getValue();
+    //       }
+    //       return "depender";
+    //     }
+    //   }
+    //   
+    //   @Extension(optional=true)
+    //   public class DependerExtension extends DependeeExtensionPoint {
+    //   }
+    
+    
+    /**
+     * call org.jenkinsci.plugins.dependencytest.depender.Depender.getValue().
+     * 
+     * @return
+     * @throws Exception
+     */
+    private String callDependerValue() throws Exception {
+        Class<?> c = jenkins.getPluginManager().uberClassLoader.loadClass("org.jenkinsci.plugins.dependencytest.depender.Depender");
+        Method m = c.getMethod("getValue");
+        return (String)m.invoke(null);
+    }
+    
+    /**
+     * Load "dependee" and then load "depender".
+     * Asserts that "depender" can access to "dependee".
+     * 
+     * @throws Exception
+     */
+    public void testInstallDependingPluginWithoutRestart() throws Exception {
+        // Load dependee.
+        {
+            String target = "dependee.hpi";
+            URL src = getClass().getClassLoader().getResource(String.format("plugins/%s", target));
+            File dest = new File(jenkins.getRootDir(), String.format("plugins/%s", target));
+            FileUtils.copyURLToFile(src, dest);
+            jenkins.pluginManager.dynamicLoad(dest);
+        }
+        
+        // before load depender, of course failed to call Depender.getValue()
+        try {
+            callDependerValue();
+            fail();
+        } catch (ClassNotFoundException _) {
+        }
+        
+        // No extensions exist.
+        assertTrue(jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.dependee.DependeeExtensionPoint").isEmpty());
+        
+        // Load depender.
+        {
+            String target = "depender.hpi";
+            URL src = getClass().getClassLoader().getResource(String.format("plugins/%s", target));
+            File dest = new File(jenkins.getRootDir(), String.format("plugins/%s", target));
+            FileUtils.copyURLToFile(src, dest);
+            jenkins.pluginManager.dynamicLoad(dest);
+        }
+        
+        // depender successfully accesses to dependee.
+        assertEquals("dependee", callDependerValue());
+        
+        // Extension in depender is loaded.
+        assertFalse(jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.dependee.DependeeExtensionPoint").isEmpty());
+    }
+    
+    /**
+     * Load "depender" and then load "dependee".
+     * Asserts that "depender" can access to "dependee".
+     * 
+     * @throws Exception
+     */
+    public void testInstallDependedPluginWithoutRestart() throws Exception {
+        // Load depender.
+        {
+            String target = "depender.hpi";
+            URL src = getClass().getClassLoader().getResource(String.format("plugins/%s", target));
+            File dest = new File(jenkins.getRootDir(), String.format("plugins/%s", target));
+            FileUtils.copyURLToFile(src, dest);
+            jenkins.pluginManager.dynamicLoad(dest);
+        }
+        
+        // before load dependee, depender does not access to dependee.
+        assertEquals("depender", callDependerValue());
+        
+        // before load dependee, of course failed to list extensions for dependee.
+        try {
+            jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.dependee.DependeeExtensionPoint");
+            fail();
+        } catch( ClassNotFoundException _ ){
+        }
+        
+        // Load dependee.
+        {
+            String target = "dependee.hpi";
+            URL src = getClass().getClassLoader().getResource(String.format("plugins/%s", target));
+            File dest = new File(jenkins.getRootDir(), String.format("plugins/%s", target));
+            FileUtils.copyURLToFile(src, dest);
+            jenkins.pluginManager.dynamicLoad(dest);
+        }
+        
+        // (MUST) Not throws an exception
+        // (SHOULD) depender successfully accesses to dependee.
+        assertEquals("dependee", callDependerValue());
+        
+        // No extensions exist.
+        // extensions in depender is not loaded.
+        assertTrue(jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.dependee.DependeeExtensionPoint").isEmpty());
+    }
+
 }