Fix JENKINS-20524

Changed to Multimap from just a normal Map so that triggers don't get overwritten.
jenkinsci · Nov 12, 2013 · 80f5bbd · 80f5bbd
1 parent b7edb86
commit 80f5bbd
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 15 deletions.
diff --git a/src/main/java/hudson/plugins/emailext/ExtendedEmailPublisher.java b/src/main/java/hudson/plugins/emailext/ExtendedEmailPublisher.java
@@ -1,5 +1,7 @@
 package hudson.plugins.emailext;
 
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableMultimap;
 import hudson.EnvVars;
 import hudson.Launcher;
 import hudson.matrix.MatrixAggregatable;
@@ -52,11 +54,9 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Random;
 import java.util.Set;
 import java.util.StringTokenizer;
@@ -79,6 +79,8 @@
 import javax.mail.internet.MimeMultipart;
 import org.jenkinsci.plugins.tokenmacro.TokenMacro;
 import org.kohsuke.stapler.DataBoundConstructor;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.ListMultimap;
 
 /**
  * {@link Publisher} that sends notification e-mail.
@@ -239,7 +241,7 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
     private boolean _perform(AbstractBuild<?, ?> build, BuildListener listener, boolean forPreBuild) {
         boolean emailTriggered = false;
         debug(listener.getLogger(), "Checking if email needs to be generated");
-        final Map<String, EmailTrigger> triggered = new HashMap<String, EmailTrigger>();
+        final Multimap<String, EmailTrigger> triggered = ArrayListMultimap.create();
 
         for (EmailTrigger trigger : getConfiguredTriggers()) {
             if (trigger.isPreBuild() == forPreBuild && trigger.trigger(build, listener)) {
@@ -253,12 +255,15 @@ private boolean _perform(AbstractBuild<?, ?> build, BuildListener listener, bool
         //Go through and remove triggers that are replaced by others
         List<String> replacedTriggers = new ArrayList<String>();
 
-        for (String triggerName : triggered.keySet()) {
-            replacedTriggers.addAll(triggered.get(triggerName).getDescriptor().getTriggerReplaceList());
+        for (Object tName : triggered.keySet()) {
+            String triggerName = (String)tName;
+            for(EmailTrigger trigger : (Collection<EmailTrigger>)triggered.get(triggerName)) {
+                replacedTriggers.addAll(trigger.getDescriptor().getTriggerReplaceList());
+            }
         }
 
-        for (String triggerName : replacedTriggers) {
-            triggered.remove(triggerName);
+        for (String triggerName : replacedTriggers) {           
+            triggered.removeAll(triggerName);
             listener.getLogger().println("Trigger " + triggerName + " was overridden by another trigger and will not send an email.");
         }
 
@@ -270,12 +275,14 @@ private boolean _perform(AbstractBuild<?, ?> build, BuildListener listener, bool
             return true;
         }
 
-        for (final String triggerName : triggered.keySet()) {
-            listener.getLogger().println("Sending email for trigger: " + triggerName);
-            final ExtendedEmailPublisherContext context = new ExtendedEmailPublisherContext(this, build, listener);
-            context.setTriggered(triggered);
-            context.setTrigger(triggered.get(triggerName));
-            sendMail(context);
+        for (String triggerName : triggered.keySet()) {
+            for(EmailTrigger trigger : triggered.get(triggerName)) {
+                listener.getLogger().println("Sending email for trigger: " + triggerName);            
+                final ExtendedEmailPublisherContext context = new ExtendedEmailPublisherContext(this, build, listener);
+                context.setTriggered(triggered);
+                context.setTrigger(trigger);
+                sendMail(context);
+            }
         }
 
         return true;
@@ -394,7 +401,7 @@ private boolean executePresendScript(ExtendedEmailPublisherContext context, Mime
             binding.setVariable("logger", context.getListener().getLogger());
             binding.setVariable("cancel", cancel);
             binding.setVariable("trigger", context.getTrigger());
-            binding.setVariable("triggered", Collections.unmodifiableMap(context.getTriggered()));
+            binding.setVariable("triggered", ImmutableMultimap.copyOf(context.getTriggered()));
 
             GroovyShell shell = new GroovyShell(cl, binding, cc);
             StringWriter out = new StringWriter();

diff --git a/src/test/java/hudson/plugins/emailext/ExtendedEmailPublisherTest.java b/src/test/java/hudson/plugins/emailext/ExtendedEmailPublisherTest.java
@@ -766,6 +766,26 @@ public Void call() throws Exception {
         });
     }
 
+    @Bug(20524)
+    @Test
+    public void testMultipleTriggersOfSameType() 
+            throws Exception {
+        FreeStyleProject prj = j.createFreeStyleProject("JENKINS-20524");
+        prj.getPublishersList().add(publisher);
+
+        publisher.recipientList = "mickey@disney.com";
+        publisher.configuredTriggers.add(new SuccessTrigger(true, false, false, false, "$DEFAULT_RECIPIENTS",
+                "$DEFAULT_REPLYTO", "$DEFAULT_SUBJECT", "$DEFAULT_CONTENT", "", 0, "project"));
+        publisher.configuredTriggers.add(new SuccessTrigger(true, false, false, false, "$DEFAULT_RECIPIENTS",
+                "$DEFAULT_REPLYTO", "$DEFAULT_SUBJECT", "$DEFAULT_CONTENT", "", 0, "project"));
+
+        FreeStyleBuild build = prj.scheduleBuild2(0).get();
+        j.assertBuildStatusSuccess(build);
+
+        assertEquals(2, Mailbox.get("mickey@disney.com").size());        
+    }
+
+
     /* Need to find out why this gets a 404 on the fileprovider.js file
     @Test
     @Bug(15442)
@@ -785,7 +805,9 @@ public void testConfiguredStateNoTriggers()
 	*/
 
     @Bug(16376)
-    @Test public void concurrentBuilds() throws Exception {
+    @Test 
+    public void testConcurrentBuilds() 
+            throws Exception {
         publisher.configuredTriggers.add(new RegressionTrigger(false, false, false, false, "", "", "", "", "", 0, ""));
         project.setConcurrentBuild(true);
         project.getBuildersList().add(new SleepOnceBuilder());