Merge pull request #880 from evilmilo/master

[FIXED JENKINS-18813] Add/RemoveTrigger now works
jenkinsci · Jul 27, 2013 · ba432da · ba432da
2 parents 016baea + edf932b
commit ba432da
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 12 deletions.
diff --git a/core/src/main/java/hudson/model/AbstractProject.java b/core/src/main/java/hudson/model/AbstractProject.java
@@ -122,6 +122,7 @@
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Map;
@@ -1646,26 +1647,21 @@ public void removeTrigger(TriggerDescriptor trigger) throws IOException {
 
     protected final synchronized <T extends Describable<T>>
     void addToList( T item, List<T> collection ) throws IOException {
-        for( int i=0; i<collection.size(); i++ ) {
-            if(collection.get(i).getDescriptor()==item.getDescriptor()) {
-                // replace
-                collection.set(i,item);
-                save();
-                return;
-            }
-        }
-        // add
+        //No support to replace item in position, remove then add
+        removeFromList(item.getDescriptor(), collection);
         collection.add(item);
         save();
         updateTransientActions();
     }
 
     protected final synchronized <T extends Describable<T>>
     void removeFromList(Descriptor<T> item, List<T> collection) throws IOException {
-        for( int i=0; i< collection.size(); i++ ) {
-            if(collection.get(i).getDescriptor()==item) {
+        final Iterator<T> iCollection = collection.iterator();
+        while(iCollection.hasNext()) {
+            final T next = iCollection.next();
+            if(next.getDescriptor()==item) {
                 // found it
-                collection.remove(i);
+                iCollection.remove();
                 save();
                 updateTransientActions();
                 return;

diff --git a/test/src/test/groovy/hudson/model/AbstractProjectTest.groovy b/test/src/test/groovy/hudson/model/AbstractProjectTest.groovy
@@ -38,6 +38,8 @@ import hudson.Functions;
 import hudson.Util;
 import hudson.tasks.ArtifactArchiver
 import hudson.triggers.SCMTrigger;
+import hudson.triggers.TimerTrigger
+import hudson.triggers.TriggerDescriptor;
 import hudson.util.StreamTaskListener;
 import hudson.util.OneShotEvent
 import jenkins.model.Jenkins;
@@ -447,4 +449,38 @@ public class AbstractProjectTest extends HudsonTestCase {
         assert t.class==SCMTrigger.class;
         assert t.spec=="*/10 * * * *"
     }
+
+    @Bug(18813)
+    public void testRemoveTrigger() {
+        AbstractProject j = jenkins.createProjectFromXML("foo", getClass().getResourceAsStream("AbstractProjectTest/vectorTriggers.xml"))
+
+        TriggerDescriptor SCM_TRIGGER_DESCRIPTOR = Hudson.instance.getDescriptorOrDie(SCMTrigger.class)
+        j.removeTrigger(SCM_TRIGGER_DESCRIPTOR);
+        assert j.triggers().size()==0
+    }
+
+    @Bug(18813)
+    public void testAddTriggerSameType() {
+        AbstractProject j = jenkins.createProjectFromXML("foo", getClass().getResourceAsStream("AbstractProjectTest/vectorTriggers.xml"))
+
+        def newTrigger = new SCMTrigger("H/5 * * * *")
+        j.addTrigger(newTrigger);
+
+        assert j.triggers().size()==1
+        def t = j.triggers()[0]
+        assert t.class==SCMTrigger.class;
+        assert t.spec=="H/5 * * * *"
+    }
+
+    @Bug(18813)
+    public void testAddTriggerDifferentType() {
+        AbstractProject j = jenkins.createProjectFromXML("foo", getClass().getResourceAsStream("AbstractProjectTest/vectorTriggers.xml"))
+
+        def newTrigger = new TimerTrigger("20 * * * *")
+        j.addTrigger(newTrigger);
+
+        assert j.triggers().size()==2
+        def t = j.triggers()[1]
+        assert t == newTrigger
+    }
 }