[JENKINS-24141] Switch to using RunWithSCM for getCulprits logic

pom.xml

@@ -4,7 +4,7 @@
     <parent>
         <groupId>org.jenkins-ci.plugins</groupId>
         <artifactId>plugin</artifactId>
-        <version>2.23</version>
+        <version>2.27</version>
         <relativePath />
     </parent>
 
@@ -30,8 +30,15 @@
     <properties>
         <powermock.version>1.6.4</powermock.version>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
-        <workflow.version>1.10</workflow.version>
+        <workflow-step-api.version>2.9</workflow-step-api.version>
+        <workflow-aggregator.version>2.5</workflow-aggregator.version>
+        <workflow-job.version>2.11</workflow-job.version>
         <jenkins.version>2.0</jenkins.version>
+        <workflow-api.version>2.11</workflow-api.version>
+        <workflow-cps.version>2.29</workflow-cps.version>
+        <workflow-durable-task-step.version>2.11</workflow-durable-task-step.version>
+        <workflow-basic-steps.version>2.4</workflow-basic-steps.version>
+        <scm-api.version>2.0.7</scm-api.version>
         <java.level>7</java.level>
         <concurrency>2</concurrency>
         <argLine />
@@ -125,7 +132,7 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins</groupId>
             <artifactId>mailer</artifactId>
-            <version>1.5</version>
+            <version>1.13</version>
         </dependency>
         <dependency>
             <groupId>org.jvnet.hudson.plugins</groupId>
@@ -150,31 +157,48 @@
             <artifactId>token-macro</artifactId>
             <version>2.0</version>
         </dependency>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins</groupId>
+            <artifactId>structs</artifactId>
+            <version>1.6</version>
+        </dependency>
 
         <!-- workflow stuff -->
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
-            <version>${workflow.version}</version>
+            <version>${workflow-step-api.version}</version>
             <artifactId>workflow-step-api</artifactId>
             <optional>true</optional>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-job</artifactId>
-            <version>${workflow.version}</version>
+            <version>${workflow-job.version}</version>
             <optional>true</optional>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
-            <artifactId>workflow-aggregator</artifactId>
-            <version>${workflow.version}</version>
+            <artifactId>workflow-api</artifactId>
+            <version>${workflow-api.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins</groupId>
+            <artifactId>scm-api</artifactId>
+            <version>${scm-api.version}</version>
             <scope>test</scope>
         </dependency>
         <dependency> <!-- StepConfigTester -->
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-step-api</artifactId>
             <classifier>tests</classifier>
-            <version>${workflow.version}</version>
+            <version>${workflow-step-api.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins.workflow</groupId>
+            <artifactId>workflow-cps</artifactId>
+            <version>${workflow-cps.version}</version>
             <scope>test</scope>
         </dependency>
         <dependency>
@@ -218,6 +242,12 @@
             <artifactId>powermock-module-junit4</artifactId>
             <version>${powermock.version}</version>
             <scope>test</scope>
+            <exclusions>
+                <exclusion>
+                    <groupId>org.javassist</groupId>
+                    <artifactId>javassist</artifactId>
+                </exclusion>
+            </exclusions>
         </dependency>
         <dependency>
             <groupId>org.powermock</groupId>
@@ -228,20 +258,32 @@
         <dependency>
             <groupId>org.javassist</groupId>
             <artifactId>javassist</artifactId>
-            <version>3.18.1-GA</version>
+            <version>3.20.0-GA</version>
             <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.main</groupId>
             <artifactId>jenkins-war</artifactId>
             <type>war</type>
-            <version>${jenkins.version}</version>
+            <version>${jenkins-war.version}</version>
             <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins</groupId>
             <artifactId>credentials</artifactId>
-            <version>2.1.4</version>
+            <version>2.1.5</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins.workflow</groupId>
+            <artifactId>workflow-durable-task-step</artifactId>
+            <version>${workflow-durable-task-step.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins.workflow</groupId>
+            <artifactId>workflow-basic-steps</artifactId>
+            <version>${workflow-basic-steps.version}</version>
             <scope>test</scope>
         </dependency>
 

src/main/java/hudson/plugins/emailext/ExtendedEmailPublisherDescriptor.java

@@ -543,12 +543,15 @@ public List<String> getDefaultTriggerIds() {
                 for(EmailTriggerDescriptor t : this.defaultTriggers) {
                     // we have to do the below because a bunch of stuff is not serialized for the Descriptor
                     EmailTriggerDescriptor d = Jenkins.getActiveInstance().getDescriptorByType(t.getClass());
-                    if(!defaultTriggerIds.contains(d.getId())) {
+                    if(d != null && !defaultTriggerIds.contains(d.getId())) {
                         defaultTriggerIds.add(d.getId());
                     }
                 }
             } else {
-                defaultTriggerIds.add(Jenkins.getActiveInstance().getDescriptor(FailureTrigger.class).getId());
+                FailureTrigger.DescriptorImpl f = Jenkins.getActiveInstance().getDescriptorByType(FailureTrigger.DescriptorImpl.class);
+                if (f != null) {
+                    defaultTriggerIds.add(f.getId());
+                }
             }
             save();
         }

src/main/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProvider.java

@@ -6,9 +6,10 @@
 
 package hudson.plugins.emailext.plugins.recipients;
 
+import com.google.common.base.Predicates;
+import com.google.common.collect.Iterables;
 import hudson.EnvVars;
 import hudson.Extension;
-import hudson.model.AbstractBuild;
 import hudson.model.Result;
 import hudson.model.Run;
 import hudson.model.User;
@@ -21,6 +22,8 @@
 
 import javax.mail.internet.InternetAddress;
 import java.io.PrintStream;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
@@ -51,10 +54,18 @@ public void send(final String format, final Object... args) {
         }
         final Debug debug = new Debug();
         Run<?,?> run = context.getRun();
-        if (run instanceof AbstractBuild) {
-            Set<User> users = ((AbstractBuild<?,?>)run).getCulprits();
-            RecipientProviderUtilities.addUsers(users, context, env, to, cc, bcc, debug);
-        } else {
+        // TODO: core 2.60+, workflow-job 2.12+: Switch to checking if run is RunWithSCM and make catch an else block
+        try {
+            Method getCulprits = run.getClass().getMethod("getCulprits");
+            if (Set.class.isAssignableFrom(getCulprits.getReturnType())) {
+                @SuppressWarnings("unchecked")
+                Set<User> users = (Set<User>) getCulprits.invoke(run);
+                if (Iterables.all(users, Predicates.instanceOf(User.class))) {
+                    RecipientProviderUtilities.addUsers(users, context, env, to, cc, bcc, debug);
+                }
+            }
+        } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) {
+            debug.send("Exception getting culprits for %s: %s", run, e);
             List<Run<?, ?>> builds = new ArrayList<>();
             Run<?, ?> build = run;
             builds.add(build);

src/main/java/hudson/plugins/emailext/plugins/recipients/RecipientProviderUtilities.java

@@ -37,6 +37,7 @@
 import hudson.scm.ChangeLogSet;
 import hudson.tasks.MailSender;
 
+import javax.mail.MethodNotSupportedException;
 import javax.mail.internet.InternetAddress;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
@@ -66,6 +67,7 @@ public static Set<User> getChangeSetAuthors(final Collection<Run<?, ?>> runs, fi
         final Set<User> users = new HashSet<>();
         for (final Run<?, ?> run : runs) {
             debug.send("    build: %d", run.getNumber());
+            // TODO: core 2.60+, workflow-job 2.12+: Switch to checking if run is an instance of RunWithSCM and call getChangeSets directly.
             if (run instanceof AbstractBuild<?,?>) {
                 final ChangeLogSet<?> changeLogSet = ((AbstractBuild<?,?>)run).getChangeSet();
                 if (changeLogSet == null) {
@@ -74,6 +76,7 @@ public static Set<User> getChangeSetAuthors(final Collection<Run<?, ?>> runs, fi
                     addChangeSetUsers(changeLogSet, users, debug);
                 }
             } else {
+                // TODO: core 2.60+, workflow-job 2.12+: Decide whether to remove this logic since it won't be needed for Pipelines any more.
                 try {
                     Method getChangeSets = run.getClass().getMethod("getChangeSets");
                     if (List.class.isAssignableFrom(getChangeSets.getReturnType())) {

src/main/java/hudson/plugins/emailext/plugins/trigger/FailureTrigger.java

@@ -33,7 +33,11 @@ public FailureTrigger(boolean sendToList, boolean sendToDevs, boolean sendToRequ
     @Deprecated
     public static FailureTrigger createDefault() {
         DescriptorImpl descriptor = (DescriptorImpl) Jenkins.getActiveInstance().getDescriptor(FailureTrigger.class);
-        return (FailureTrigger) descriptor.createDefault();
+        if (descriptor != null) {
+            return (FailureTrigger) descriptor.createDefault();
+        } else {
+            return null;
+        }
     }
 
     @Override

src/test/java/hudson/plugins/emailext/plugins/recipients/CulpritsRecipientProviderTest.java

@@ -1,11 +1,13 @@
 package hudson.plugins.emailext.plugins.recipients;
 
 import hudson.model.FreeStyleBuild;
+import hudson.model.Job;
 import hudson.model.Result;
 import hudson.model.User;
 import hudson.plugins.emailext.ExtendedEmailPublisherDescriptor;
 import hudson.tasks.Mailer;
 import jenkins.model.Jenkins;
+import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.junit.Before;
 import org.junit.Test;
@@ -22,7 +24,9 @@
         Mailer.class,
         Mailer.DescriptorImpl.class,
         User.class,
-        WorkflowRun.class
+        WorkflowRun.class,
+        WorkflowJob.class,
+        Job.class
 })
 public class CulpritsRecipientProviderTest {
 
@@ -46,38 +50,42 @@ public void before() throws Exception {
 
     @Test
     public void testAddRecipients1() throws Exception {
-        final WorkflowRun build1 = PowerMockito.mock(WorkflowRun.class);
+        final WorkflowJob j = PowerMockito.mock(WorkflowJob.class);
+        final WorkflowRun build1 = PowerMockito.spy(new WorkflowRun(j));
         PowerMockito.when(build1.getResult()).thenReturn(Result.UNSTABLE);
         MockUtilities.addChangeSet(build1, "X", "V");
+        PowerMockito.doReturn(null).when(build1).getPreviousBuild();
 
-        final WorkflowRun build2 = PowerMockito.mock(WorkflowRun.class);
+        final WorkflowRun build2 = PowerMockito.spy(new WorkflowRun(j));
         PowerMockito.when(build2.getResult()).thenReturn(Result.SUCCESS);
         MockUtilities.addChangeSet(build2, "Z", "V");
-        PowerMockito.when(build2.getPreviousCompletedBuild()).thenReturn(build1);
+        PowerMockito.doReturn(build1).when(build2).getPreviousCompletedBuild();
 
-        final WorkflowRun build3 = PowerMockito.mock(WorkflowRun.class);
+        final WorkflowRun build3 = PowerMockito.spy(new WorkflowRun(j));
         PowerMockito.when(build3.getResult()).thenReturn(Result.UNSTABLE);
         MockUtilities.addChangeSet(build3, "A");
-        PowerMockito.when(build3.getPreviousCompletedBuild()).thenReturn(build2);
+        PowerMockito.doReturn(build2).when(build3).getPreviousCompletedBuild();
 
-        final WorkflowRun build4 = PowerMockito.mock(WorkflowRun.class);
+        final WorkflowRun build4 = PowerMockito.spy(new WorkflowRun(j));
         PowerMockito.when(build4.getResult()).thenReturn(Result.UNSTABLE);
         MockUtilities.addChangeSet(build4, "B");
-        PowerMockito.when(build4.getPreviousCompletedBuild()).thenReturn(build3);
+        PowerMockito.doReturn(build3).when(build4).getPreviousCompletedBuild();
 
         TestUtilities.checkRecipients(build4, new CulpritsRecipientProvider(), "A", "B");
     }
 
     @Test
     public void testAddRecipients2() throws Exception {
-        final WorkflowRun build1 = PowerMockito.mock(WorkflowRun.class);
+        final WorkflowJob j = PowerMockito.mock(WorkflowJob.class);
+        final WorkflowRun build1 = PowerMockito.spy(new WorkflowRun(j));
         PowerMockito.when(build1.getResult()).thenReturn(Result.UNSTABLE);
         MockUtilities.addChangeSet(build1, "X", "V");
+        PowerMockito.doReturn(null).when(build1).getPreviousBuild();
 
-        final WorkflowRun build2 = PowerMockito.mock(WorkflowRun.class);
+        final WorkflowRun build2 = PowerMockito.spy(new WorkflowRun(j));
         PowerMockito.when(build2.getResult()).thenReturn(Result.SUCCESS);
         MockUtilities.addChangeSet(build2, "Z", "V");
-        PowerMockito.when(build2.getPreviousCompletedBuild()).thenReturn(build1);
+        PowerMockito.doReturn(build1).when(build2).getPreviousCompletedBuild();
 
         TestUtilities.checkRecipients(build2, new CulpritsRecipientProvider(), "X", "V", "Z");
     }

src/test/java/hudson/plugins/emailext/plugins/recipients/DevelopersRecipientProviderTest.java

@@ -1,11 +1,13 @@
 package hudson.plugins.emailext.plugins.recipients;
 
 import hudson.model.FreeStyleBuild;
+import hudson.model.FreeStyleProject;
 import hudson.model.Result;
 import hudson.model.User;
 import hudson.plugins.emailext.ExtendedEmailPublisherDescriptor;
 import hudson.tasks.Mailer;
 import jenkins.model.Jenkins;
+import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.junit.Before;
 import org.junit.Test;
@@ -22,7 +24,9 @@
         Mailer.class,
         Mailer.DescriptorImpl.class,
         User.class,
-        WorkflowRun.class
+        WorkflowRun.class,
+        WorkflowJob.class,
+        FreeStyleProject.class
 })
 public class DevelopersRecipientProviderTest {
 
@@ -46,14 +50,16 @@ public void before() throws Exception {
 
     @Test
     public void testAddRecipients() throws Exception {
-        final FreeStyleBuild build1 = PowerMockito.mock(FreeStyleBuild.class);
-        PowerMockito.when(build1.getResult()).thenReturn(Result.UNSTABLE);
+        final FreeStyleProject p = PowerMockito.mock(FreeStyleProject.class);
+        final FreeStyleBuild build1 = PowerMockito.spy(new FreeStyleBuild(p));
+        PowerMockito.doReturn(Result.UNSTABLE).when(build1).getResult();
         MockUtilities.addRequestor(build1, "A");
         MockUtilities.addChangeSet(build1, "X", "V");
         TestUtilities.checkRecipients(build1, new DevelopersRecipientProvider(), "X", "V");
 
-        final WorkflowRun build2 = PowerMockito.mock(WorkflowRun.class);
-        PowerMockito.when(build2.getResult()).thenReturn(Result.UNSTABLE);
+        final WorkflowJob j = PowerMockito.mock(WorkflowJob.class);
+        final WorkflowRun build2 = PowerMockito.spy(new WorkflowRun(j));
+        PowerMockito.doReturn(Result.UNSTABLE).when(build2).getResult();
         MockUtilities.addChangeSet(build2, "X", "V");
         TestUtilities.checkRecipients(build2, new DevelopersRecipientProvider(), "X", "V");
     }