Merge pull request #208 from kutzi/JENKINS-10530

[FIXED JENKINS-10530] regression: plugins were no longer identified as upstream snapshot dependencies

changelog.html

@@ -61,6 +61,9 @@ Upcoming changes</a>
   <li class=bug>
     isPartial() check for matrix builds now only reference active configurations.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-10197">issue 10197</a>)
+  <li class=bug>
+    Maven jobs building plugins were no longer identified as upstream snapshot dependencies.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-10530">issue 10530</a>)
 </ul>
 </div><!--=TRUNK-END=-->
 

core/src/main/java/hudson/model/DependencyGraph.java

@@ -66,7 +66,7 @@ import java.util.Stack;
  * @see Jenkins#getDependencyGraph()
  * @author Kohsuke Kawaguchi
  */
-public final class DependencyGraph implements Comparator<AbstractProject> {
+public class DependencyGraph implements Comparator<AbstractProject> {
 
     private Map<AbstractProject, List<DependencyGroup>> forward = new HashMap<AbstractProject, List<DependencyGroup>>();
     private Map<AbstractProject, List<DependencyGroup>> backward = new HashMap<AbstractProject, List<DependencyGroup>>();
@@ -79,6 +79,9 @@ public final class DependencyGraph implements Comparator<AbstractProject> {
      * Builds the dependency graph.
      */
     public DependencyGraph() {
+    }
+    
+    public void build() {
         // Set full privileges while computing to avoid missing any projects the current user cannot see.
         // Use setContext (NOT getContext().setAuthentication()) so we don't affect concurrent threads for same HttpSession.
         SecurityContext saveCtx = SecurityContextHolder.getContext();
@@ -87,7 +90,7 @@ public final class DependencyGraph implements Comparator<AbstractProject> {
             NotSerilizableSecurityContext system = new NotSerilizableSecurityContext();
             system.setAuthentication(ACL.SYSTEM);
             SecurityContextHolder.setContext(system);
-            for( AbstractProject p : Jenkins.getInstance().getAllItems(AbstractProject.class) )
+            for( AbstractProject p : getAllProjects() )
                 p.buildDependencyGraph(this);
 
             forward = finalize(forward);
@@ -99,6 +102,10 @@ public final class DependencyGraph implements Comparator<AbstractProject> {
             SecurityContextHolder.setContext(saveCtx);
         }
     }
+    
+    Collection<AbstractProject> getAllProjects() {
+        return Jenkins.getInstance().getAllItems(AbstractProject.class);
+    }
 
     /**
      * Special constructor for creating an empty graph

core/src/main/java/jenkins/model/Jenkins.java

@@ -3297,7 +3297,11 @@ public class Jenkins extends AbstractCIBase implements ModifiableItemGroup<TopLe
      * Rebuilds the dependency map.
      */
     public void rebuildDependencyGraph() {
-        dependencyGraph = new DependencyGraph();
+        DependencyGraph graph = new DependencyGraph();
+        graph.build();
+        // volatile acts a as a memory barrier here and therefore guarantees 
+        // that graph is fully build, before it's visible to other threads
+        dependencyGraph = graph;
     }
 
     public DependencyGraph getDependencyGraph() {

maven-plugin/pom.xml

@@ -332,6 +332,24 @@ THE SOFTWARE.
         <version>4.8.2</version>
         <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <version>1.8.5</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-module-junit4</artifactId>
+      <version>1.4.9</version>
+      <scope>test</scope>
+   </dependency>
+   <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-api-mockito</artifactId>
+      <version>1.4.9</version>
+      <scope>test</scope>
+   </dependency>
 
   </dependencies>
 

maven-plugin/src/main/java/hudson/maven/MavenModule.java

@@ -24,42 +24,55 @@
 package hudson.maven;
 
 import hudson.CopyOnWrite;
-import hudson.Util;
 import hudson.Functions;
+import hudson.Util;
 import hudson.maven.reporters.MavenMailer;
-import hudson.model.*;
+import hudson.model.AbstractProject;
+import hudson.model.Action;
+import hudson.model.DependencyGraph;
+import hudson.model.Descriptor;
 import hudson.model.Descriptor.FormException;
-import jenkins.model.Jenkins;
+import hudson.model.Item;
+import hudson.model.ItemGroup;
+import hudson.model.JDK;
+import hudson.model.Job;
+import hudson.model.Label;
+import hudson.model.Node;
+import hudson.model.Resource;
+import hudson.model.Saveable;
 import hudson.tasks.LogRotator;
-import hudson.tasks.Publisher;
 import hudson.tasks.Maven.MavenInstallation;
+import hudson.tasks.Publisher;
 import hudson.util.AlternativeUiTextProvider;
 import hudson.util.DescribableList;
-import org.apache.maven.project.MavenProject;
-import org.kohsuke.stapler.StaplerRequest;
-import org.kohsuke.stapler.StaplerResponse;
-import org.kohsuke.stapler.export.Exported;
-
-import com.google.common.util.concurrent.MoreExecutors;
 
-import javax.servlet.ServletException;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.HashSet;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import javax.servlet.ServletException;
+
+import jenkins.model.Jenkins;
+
+import org.apache.maven.project.MavenProject;
+import org.kohsuke.stapler.StaplerRequest;
+import org.kohsuke.stapler.StaplerResponse;
+import org.kohsuke.stapler.export.Exported;
+
 /**
  * {@link Job} that builds projects based on Maven2.
  * 
  * @author Kohsuke Kawaguchi
  */
-public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBuild> implements Saveable {
+public class MavenModule extends AbstractMavenProject<MavenModule,MavenBuild> implements Saveable {
     private DescribableList<MavenReporter,Descriptor<MavenReporter>> reporters =
         new DescribableList<MavenReporter,Descriptor<MavenReporter>>(this);
 
@@ -79,6 +92,13 @@ public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBui
      * @since 1.199
      */
     private String version;
+    
+    /**
+     * Packaging type of the module.
+     * 
+     * pom, jar, maven-plugin, ejb, war, ear, rar, par or other custom types.
+     */
+    private String packaging;
 
     private transient ModuleName moduleName;
 
@@ -175,6 +195,7 @@ public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBui
     /*package*/ void reconfigure(PomInfo pom) {
         this.displayName = pom.displayName;
         this.version = pom.version;
+        this.packaging = pom.packaging;
         this.relativePath = pom.relativePath;
         this.dependencies = pom.dependencies;
         this.children = pom.children;
@@ -224,7 +245,7 @@ public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBui
                 if (d instanceof ModuleDependency) {
                     deps.add((ModuleDependency) d);
                 } else {
-                    deps.add(new ModuleDependency((ModuleName)d, ModuleDependency.UNKNOWN));
+                    deps.add(new ModuleDependency((ModuleName)d, ModuleDependency.UNKNOWN, false));
                 }
             }
             dependencies = deps;
@@ -304,7 +325,8 @@ public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBui
      * Gets groupId+artifactId+version as {@link ModuleDependency}.
      */
     public ModuleDependency asDependency() {
-        return new ModuleDependency(moduleName,Functions.defaulted(version,ModuleDependency.UNKNOWN));
+        return new ModuleDependency(moduleName,Functions.defaulted(version,ModuleDependency.UNKNOWN),
+                PomInfo.PACKAGING_TYPE_PLUGIN.equals(this.packaging));
     }
 
     @Override
@@ -403,7 +425,7 @@ public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBui
         if (data == null) {
             Map<ModuleDependency,MavenModule> modules = new HashMap<ModuleDependency,MavenModule>();
     
-            for (MavenModule m : Jenkins.getInstance().getAllItems(MavenModule.class)) {
+            for (MavenModule m : getAllMavenModules()) {
                 if(!m.isBuildable())  continue;
                 ModuleDependency moduleDependency = m.asDependency();
                 MavenModule old = modules.get(moduleDependency);
@@ -419,7 +441,7 @@ public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBui
         } else {
             if (hasDependenciesWithUnknownVersion && !data.withUnknownVersions) {
                 // found 'old' MavenModule: add dependencies with unknown versions now
-                for (MavenModule m : Jenkins.getInstance().getAllItems(MavenModule.class)) {
+                for (MavenModule m : getAllMavenModules()) {
                     if(m.isDisabled())  continue;
                     ModuleDependency moduleDependency = m.asDependency().withUnknownVersion();
                     data.allModules.put(moduleDependency,m);
@@ -469,6 +491,13 @@ public final class MavenModule extends AbstractMavenProject<MavenModule,MavenBui
         }
     }
     
+    /**
+     * Returns all Maven modules in this Jenkins instance.
+     */
+    protected Collection<MavenModule> getAllMavenModules() {
+        return Jenkins.getInstance().getAllItems(MavenModule.class);
+    }
+    
     /**
      * Check if this module has dependencies recorded without a concrete version -
      * which shouldn't happen for any module which was at least build once with Jenkins >= 1.207. 

maven-plugin/src/main/java/hudson/maven/MavenModuleSet.java

@@ -103,7 +103,7 @@ import org.kohsuke.stapler.export.Exported;
  *
  * @author Kohsuke Kawaguchi
  */
-public final class MavenModuleSet extends AbstractMavenProject<MavenModuleSet,MavenModuleSetBuild> implements TopLevelItem, ItemGroup<MavenModule>, SCMedItem, Saveable, BuildableItemWithBuildWrappers {
+public class MavenModuleSet extends AbstractMavenProject<MavenModuleSet,MavenModuleSetBuild> implements TopLevelItem, ItemGroup<MavenModule>, SCMedItem, Saveable, BuildableItemWithBuildWrappers {
     /**
      * All {@link MavenModule}s, keyed by their {@link MavenModule#getModuleName()} module name}s.
      */

maven-plugin/src/main/java/hudson/maven/ModuleDependency.java

@@ -46,19 +46,28 @@ public final class ModuleDependency implements Serializable {
     /**
      * @since 1.395
      */
-    public boolean plugin = false;
+    public final boolean plugin;
 
     public ModuleDependency(String groupId, String artifactId, String version) {
+        this(groupId, artifactId, version, false);
+    }
+    
+    public ModuleDependency(String groupId, String artifactId, String version, boolean plugin) {
         this.groupId = groupId.intern();
         this.artifactId = artifactId.intern();
         if(version==null)
             this.version = UNKNOWN;
         else
             this.version = version.intern();
+        this.plugin = plugin;
     }
 
     public ModuleDependency(ModuleName name, String version) {
-        this(name.groupId,name.artifactId,version);
+        this(name.groupId,name.artifactId,version,false);
+    }
+    
+    public ModuleDependency(ModuleName name, String version, boolean plugin) {
+        this(name.groupId,name.artifactId,version,plugin);
     }
 
     public ModuleDependency(org.apache.maven.model.Dependency dep) {
@@ -70,26 +79,25 @@ public final class ModuleDependency implements Serializable {
     }
 
     public ModuleDependency(Plugin p) {
-        this(p.getGroupId(),p.getArtifactId(), Functions.defaulted(p.getVersion(),NONE));
-        this.plugin = true;
+        this(p.getGroupId(),p.getArtifactId(), Functions.defaulted(p.getVersion(),NONE),true);
     }
 
     public ModuleDependency(ReportPlugin p) {
-        this(p.getGroupId(),p.getArtifactId(),p.getVersion());
-        this.plugin = true;
+        this(p.getGroupId(),p.getArtifactId(),p.getVersion(),true);
     }
 
     public ModuleDependency(Extension ext) {
         this(ext.getGroupId(),ext.getArtifactId(),ext.getVersion());
     }
 
-    private ModuleDependency(String groupId, String artifactId) {
+    private ModuleDependency(String groupId, String artifactId, boolean plugin) {
         // to be used only by the withUnknownVersion() method
         // where we know that groupId and artifactId are already interned
         // and where we want an UNKNOWN version
         this.groupId = groupId;
         this.artifactId = artifactId;
         this.version = UNKNOWN;
+        this.plugin = plugin;
     }
     
     public ModuleName getName() {
@@ -103,7 +111,7 @@ public final class ModuleDependency implements Serializable {
         if (UNKNOWN.equals(version))
             return this;
         else
-            return new ModuleDependency(groupId,artifactId);
+            return new ModuleDependency(groupId,artifactId,plugin);
     }
 
     public boolean equals(Object o) {
@@ -131,7 +139,7 @@ public final class ModuleDependency implements Serializable {
      * Upon reading from the disk, intern strings.
      */
     public ModuleDependency readResolve() {
-        return new ModuleDependency(groupId,artifactId,version);
+        return new ModuleDependency(groupId,artifactId,version,plugin);
     }
 
     /**

maven-plugin/src/main/java/hudson/maven/PomInfo.java

@@ -48,6 +48,8 @@ import java.util.ArrayList;
  * @author Kohsuke Kawaguchi
  */
 final class PomInfo implements Serializable {
+    
+    public static final String PACKAGING_TYPE_PLUGIN = "maven-plugin";
 
     public final ModuleName name;
 
@@ -107,6 +109,8 @@ final class PomInfo implements Serializable {
     private final String artifactId;
 
     public final Notifier mailNotifier;
+    
+    public final String packaging;
 
     public PomInfo(MavenProject project, PomInfo parent, String relPath) {
         this.name = new ModuleName(project);
@@ -154,13 +158,14 @@ final class PomInfo implements Serializable {
         
         this.groupId = project.getGroupId();
         this.artifactId = project.getArtifactId();
+        this.packaging = project.getPackaging();
     }
     
     /**
      * Creates {@link ModuleDependency} that represents this {@link PomInfo}.
      */
     private ModuleDependency asDependency() {
-        return new ModuleDependency(name,version);
+        return new ModuleDependency(name,version,PACKAGING_TYPE_PLUGIN.equals(this.packaging));
     }
 
     private void addPluginsAsDependencies(List<Plugin> plugins, Set<ModuleDependency> dependencies) {

maven-plugin/src/test/java/hudson/maven/MavenModuleTest.java

+package hudson.maven;
+
+import static org.mockito.Mockito.doCallRealMethod;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.powermock.api.support.membermodification.MemberMatcher.constructor;
+import static org.powermock.api.support.membermodification.MemberModifier.suppress;
+import hudson.maven.MavenModuleSet.DescriptorImpl;
+import hudson.model.AbstractProject;
+import hudson.model.DependencyGraph;
+import hudson.model.MockHelper;
+
+import java.util.Collections;
+import java.util.List;
+
+import junit.framework.Assert;
+
+import org.apache.maven.model.Build;
+import org.apache.maven.model.Plugin;
+import org.apache.maven.project.MavenProject;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.jvnet.hudson.test.Bug;
+import org.mockito.Matchers;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import com.google.common.collect.Lists;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest( { MavenModuleSet.class, DescriptorImpl.class, AbstractProject.class})
+public class MavenModuleTest {
+    
+    private MavenModuleSet parent;
+    
+    private MavenModule module;
+
+    private MavenProject project;
+    
+    @Before
+    public void before() {
+        suppress(constructor(AbstractProject.class));
+        suppress(constructor(DescriptorImpl.class));
+        
+        this.module = mock(MavenModule.class);
+        basicMocking(this.module);
+        
+        this.project = new MavenProject();
+        project.setGroupId("test");
+        project.setArtifactId("testmodule");
+        project.setVersion("2.0-SNAPSHOT");
+        project.setPackaging("jar");
+        
+        this.module.reconfigure(new PomInfo(project, null, "relPath"));
+        this.module.doSetName("test$testmodule");
+    }
+    
+    /**
+     * Tests that a {@link MavenModule} which builds a plugin is recognized as a snapshot
+     * dependency in another module using that plugin.
+     */
+    @Test
+    @Bug(10530)
+    public void testMavenModuleAsPluginDependency() {
+        MavenModule pluginModule = createPluginProject();
+        
+        addModuleAsPluginDependency(this.module, pluginModule);
+        
+        when(this.module.getAllMavenModules()).thenReturn(Lists.newArrayList(this.module, pluginModule));
+        
+        DependencyGraph graph = MockHelper.mockDependencyGraph(
+                Lists.<AbstractProject<?,?>>newArrayList(this.module, pluginModule));
+        graph.build();
+        
+        List<AbstractProject> downstream = graph.getDownstream(pluginModule);
+        Assert.assertEquals(1, downstream.size());
+        Assert.assertSame(this.module, downstream.get(0));
+    }
+
+    private static void addModuleAsPluginDependency(MavenModule module, MavenModule pluginModule) {
+        Build build = new Build();
+        Plugin plugin = new Plugin();
+        plugin.setGroupId(pluginModule.getModuleName().groupId);
+        plugin.setArtifactId(pluginModule.getModuleName().artifactId);
+        plugin.setVersion(pluginModule.getVersion());
+        build.setPlugins(Collections.singletonList(plugin));
+        
+        MavenProject project = new MavenProject();
+        project.setGroupId(module.getModuleName().groupId);
+        project.setArtifactId(module.getModuleName().artifactId);
+        project.setVersion(module.getVersion());
+        project.setPackaging("jar");
+        project.setBuild(build);
+        
+        module.reconfigure(new PomInfo(project, null, "relPath"));
+    }
+
+    private static MavenModule createPluginProject() {
+        MavenModule pluginModule = mock(MavenModule.class);
+        basicMocking(pluginModule);
+        
+        MavenProject proj = new MavenProject();
+        proj.setGroupId("test");
+        proj.setArtifactId("pluginmodule");
+        proj.setVersion("1.0-SNAPSHOT");
+        proj.setPackaging("maven-plugin");
+        PomInfo info = new PomInfo(proj, null, "relPath");
+        pluginModule.reconfigure(info);
+        pluginModule.doSetName("test$pluginmodule");
+        
+        return pluginModule;
+    }
+    
+    private static void basicMocking(MavenModule mock) {
+        when(mock.isBuildable()).thenReturn(Boolean.TRUE);
+        doCallRealMethod().when(mock).reconfigure(Matchers.any(PomInfo.class));
+        doCallRealMethod().when(mock).buildDependencyGraph(Matchers.any(DependencyGraph.class));
+        when(mock.asDependency()).thenCallRealMethod();
+        doCallRealMethod().when(mock).doSetName(Matchers.anyString());
+        when(mock.getModuleName()).thenCallRealMethod();
+        when(mock.getVersion()).thenCallRealMethod();
+        
+        MavenModuleSet parent = mock(MavenModuleSet.class);
+        when(parent.isAggregatorStyleBuild()).thenReturn(Boolean.FALSE);
+        when(mock.getParent()).thenReturn(parent);
+        
+        when(parent.getModules()).thenReturn(Collections.singleton(mock));
+    }
+}

maven-plugin/src/test/java/hudson/model/MockHelper.java

+package hudson.model;
+
+import java.util.Collection;
+
+import org.mockito.Mockito;
+
+public class MockHelper {
+    public static DependencyGraph mockDependencyGraph(Collection<AbstractProject<?,?>> allProjects) {
+        DependencyGraph graph = new DependencyGraph();
+        graph = Mockito.spy(graph);
+        Mockito.doReturn(allProjects).when(graph).getAllProjects();
+        return graph;
+    }
+}