Add behaviour options for counter factory in case of invalid options.

[FIXED JENKINS-13872] Add the possibility of skipping the triggering, or ignoring the parameters when the to, from and step elements do not form a contained set. Corrected an issue in the BuildTriggerConfig class where if a factory returned no parameters meant that no projects would be triggered in certain cases.
jenkinsci · Jul 30, 2012 · 89a1d23 · 89a1d23
1 parent f8f0998
commit 89a1d23
Show file tree

Hide file tree

Showing 6 changed files with 118 additions and 25 deletions.
diff --git a/src/main/java/hudson/plugins/parameterizedtrigger/BuildTriggerConfig.java b/src/main/java/hudson/plugins/parameterizedtrigger/BuildTriggerConfig.java
@@ -235,12 +235,12 @@ public ListMultimap<AbstractProject, Future<AbstractBuild>> perform2(AbstractBui
         try {
             if (getCondition().isMet(build.getResult())) {
                 ListMultimap<AbstractProject, Future<AbstractBuild>> futures = ArrayListMultimap.create();
-                
+
                 for (List<AbstractBuildParameters> addConfigs : getDynamicBuildParameters(build, listener)) {
                     List<Action> actions = getBaseActions(ImmutableList.<AbstractBuildParameters>builder().addAll(configs).addAll(addConfigs).build(), build, listener);
                     for (AbstractProject project : getProjectList(build.getProject().getParent(),env)) {
                         List<Action> list = getBuildActions(actions, project);
-                        
+
                         futures.put(project, schedule(build, project, list));
                     }
                 }
@@ -251,7 +251,7 @@ public ListMultimap<AbstractProject, Future<AbstractBuild>> perform2(AbstractBui
         }
         return ArrayListMultimap.create();
     }
-	
+
     /**
      * @return
      *      Inner list represents a set of build parameters used together for one invocation of a project,
@@ -266,16 +266,20 @@ private List<List<AbstractBuildParameters>> getDynamicBuildParameters(AbstractBu
             dynamicBuildParameters.add(Collections.<AbstractBuildParameters>emptyList());
             for (AbstractBuildParameterFactory configFactory : configFactories) {
                 List<List<AbstractBuildParameters>> newDynParameters = Lists.newArrayList();
-                for (AbstractBuildParameters config : configFactory.getParameters(build, listener)) {
-                    for (List<AbstractBuildParameters> dynamicBuildParameter : dynamicBuildParameters) {
-                        newDynParameters.add(
-                                ImmutableList.<AbstractBuildParameters>builder()
-                                        .addAll(dynamicBuildParameter)
-                                        .add(config)
-                                        .build());
+                List<AbstractBuildParameters> factoryParameters = configFactory.getParameters(build, listener);
+                // if factory returns 0 parameters we need to skip assigning newDynParameters to dynamicBuildParameters as we would add invalid list
+                if(factoryParameters.size() > 0) {
+                    for (AbstractBuildParameters config : factoryParameters) {
+                        for (List<AbstractBuildParameters> dynamicBuildParameter : dynamicBuildParameters) {
+                            newDynParameters.add(
+                                    ImmutableList.<AbstractBuildParameters>builder()
+                                            .addAll(dynamicBuildParameter)
+                                            .add(config)
+                                            .build());
+                        }
                     }
+                    dynamicBuildParameters = newDynParameters;
                 }
-                dynamicBuildParameters = newDynParameters;
             }
             return dynamicBuildParameters;
         }

diff --git a/src/main/java/hudson/plugins/parameterizedtrigger/CounterBuildParameterFactory.java b/src/main/java/hudson/plugins/parameterizedtrigger/CounterBuildParameterFactory.java
@@ -20,27 +20,67 @@
 
 /**
  * A BuildParameterFactory generating Predefined Parameters for a counter
+ *
+ * @author Chris Johnson
  */
 public class CounterBuildParameterFactory extends AbstractBuildParameterFactory {
 
     private final String from;
     private final String to;
     private final String step;
     private final String paramExpr;
+    private final SteppingValidationEnum validationFail;
+
+    public enum SteppingValidationEnum {
+        FAIL("Fail the build step"),    // previous behaviour (default)
+        SKIP("Don't trigger these projects"){
+            @Override
+            public void failCheck(TaskListener listener) throws AbstractBuildParameters.DontTriggerException {
+                listener.getLogger().println(Messages.CounterBuildParameterFactory_CountingWillNotTerminateSkipping());
+                throw new AbstractBuildParameters.DontTriggerException();
+        }},
+        NOPARMS("Skip these parameters"){
+            @Override
+            public void failCheck(TaskListener listener) throws AbstractBuildParameters.DontTriggerException {
+                listener.getLogger().println(Messages.CounterBuildParameterFactory_CountingWillNotTerminateIgnore());
+        }};
+
+        private String description;
+
+        public String getDescription() {
+            return description;
+        }
+        SteppingValidationEnum(String description) {
+            this.description = description;
+        }
 
+        public void failCheck(TaskListener listener) throws AbstractBuildParameters.DontTriggerException {
+            throw new RuntimeException(Messages.CounterBuildParameterFactory_CountingWillNotTerminate());
+        }
+    }
 
     public CounterBuildParameterFactory(long from, long to, long step, String paramExpr) {
         this(Long.toString(from), Long.toString(to), Long.toString(step), paramExpr);
     }
+    public CounterBuildParameterFactory(long from, long to, long step, String paramExpr, SteppingValidationEnum validationFail) {
+        this(Long.toString(from), Long.toString(to), Long.toString(step), paramExpr, validationFail);
+    }
 
-    @DataBoundConstructor
     public CounterBuildParameterFactory(String from, String to, String step, String paramExpr) {
+        // mimic old behaviour which failed job
+        this(from, to, step, paramExpr, SteppingValidationEnum.FAIL);
+    }
+
+    @DataBoundConstructor
+    public CounterBuildParameterFactory(String from, String to, String step, String paramExpr, SteppingValidationEnum validationFail) {
         this.from = from;
         this.to = to;
         this.step = step;
         this.paramExpr = paramExpr;
+        this.validationFail = validationFail;
     }
 
+
     @Override
     public List<AbstractBuildParameters> getParameters(AbstractBuild<?, ?> build, TaskListener listener) throws IOException, InterruptedException, AbstractBuildParameters.DontTriggerException {
         EnvVars envVars = build.getEnvironment(listener);
@@ -56,14 +96,13 @@ public List<AbstractBuildParameters> getParameters(AbstractBuild<?, ?> build, Ta
             params.add(getParameterForCount(fromNum));
         } else {
             if (stepNum == 0) {
-                throw new RuntimeException(Messages.CounterBuildParameterFactory_CountingWillNotTerminate());
-            }
-            if (upDown * stepNum < 0) {
-                throw new RuntimeException(Messages.CounterBuildParameterFactory_CountingWillNotTerminate());
-            }
-
-            for (Long i = fromNum; upDown * i <= upDown * toNum; i += stepNum) {
-                params.add(getParameterForCount(i));
+                validationFail.failCheck(listener);
+            } else if (upDown * stepNum < 0) {
+                validationFail.failCheck(listener);
+            } else {
+                for (Long i = fromNum; upDown * i <= upDown * toNum; i += stepNum) {
+                    params.add(getParameterForCount(i));
+                }
             }
         }
         return params;
@@ -127,6 +166,10 @@ public String getParamExpr() {
         return paramExpr;
     }
 
+    public SteppingValidationEnum getvalidationFail() {
+        return validationFail;
+    }
+
     private static final VariableResolver<String> EMPTY_STRING_VARIABLE_RESOLVER = new VariableResolver<String>() {
 
         @Override

diff --git a/...n/resources/hudson/plugins/parameterizedtrigger/CounterBuildParameterFactory/config.jelly b/...n/resources/hudson/plugins/parameterizedtrigger/CounterBuildParameterFactory/config.jelly
@@ -9,7 +9,9 @@
   <f:entry field="step" title="${%Step}">
     <f:textbox default="1"/>
   </f:entry>
-
+  <f:entry field="validationFail" title="${%Action to perform when stepping validation fails}">
+    <f:enum>${it.getDescription()}</f:enum>
+  </f:entry>
   <f:entry field="paramExpr" title="${%Parameters}">
     <f:textarea />
   </f:entry>

diff --git a/...hudson/plugins/parameterizedtrigger/CounterBuildParameterFactory/help-validationFail.html b/...hudson/plugins/parameterizedtrigger/CounterBuildParameterFactory/help-validationFail.html
@@ -0,0 +1,9 @@
+<div>
+This option determines the behaviour to take if the from,to and step values do not form a valid combination. One that gives a finite number of combinations.<br/>
+The actions that can be taken are:
+  <ul>
+  <li><b>Fail the build step</b> This will fail the buildstep with an RuntimeException. (This is the default, and occured in older versions)</li>
+  <li><b>Don't trigger these projects</b> This will cause the projects not to be triggered at all for this configuration.</li>
+  <li><b>Skip these parameters</b> This will allow the projects to be triggered without these parameters.</li>
+  </ul>
+</div>
diff --git a/src/main/resources/hudson/plugins/parameterizedtrigger/Messages.properties b/src/main/resources/hudson/plugins/parameterizedtrigger/Messages.properties
@@ -1,3 +1,6 @@
 CounterBuildParameterFactory.CounterBuildParameterFactory=Counter Parameter Factory
-CounterBuildParameterFactory.CountingWillNotTerminate=Counting with step size 0 will not terminate!
+CounterBuildParameterFactory.CountingWillNotTerminate=To, from and step form a infinite set, counting will not terminate!
+CounterBuildParameterFactory.CountingWillNotTerminateSkipping=To, from and step form a infinite set, counting will not terminate! Triggering Skipped.
+CounterBuildParameterFactory.CountingWillNotTerminateIgnore=To, from and step form a infinite set, counting will not terminate! Parameters Ignored.
+
 FileBuildParameterFactory.FileBuildParameterFactory=File Parameter Factory
diff --git a/...t/java/hudson/plugins/parameterizedtrigger/test/CounterBuildParameterFactoryUnitTest.java b/...t/java/hudson/plugins/parameterizedtrigger/test/CounterBuildParameterFactoryUnitTest.java
@@ -5,6 +5,7 @@
 import hudson.model.TaskListener;
 import hudson.plugins.parameterizedtrigger.AbstractBuildParameters;
 import hudson.plugins.parameterizedtrigger.CounterBuildParameterFactory;
+import hudson.plugins.parameterizedtrigger.CounterBuildParameterFactory.SteppingValidationEnum;
 import org.junit.Test;
 
 import java.io.IOException;
@@ -13,6 +14,10 @@
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.*;
 
+/**
+ *
+ * @author Chris Johnson
+ */
 public class CounterBuildParameterFactoryUnitTest {
 
     @Test
@@ -39,8 +44,10 @@ public void countingWithNoIncrementShouldNotWork() throws Exception {
     }
 
     @Test
-    public void countingWithNoIncrementOnlyOneElementShouldNotWork() throws Exception {
-        getParameters(1,1,0);
+    public void countingWithNoIncrementOnlyOneElement() throws Exception {
+        // step is ignored if from and to are equal
+        List<AbstractBuildParameters> parameters = getParameters(1,1,0);
+        assertEquals(1, parameters.size());
     }
 
     @Test
@@ -49,12 +56,37 @@ public void countingWhenFromToIsSameShouldWork() throws Exception {
         assertEquals(1, parameters.size());
     }
 
+    @Test(expected = RuntimeException.class)
+    public void countingWithNoIncrementShouldNotWork1() throws Exception {
+        getParameters(1,2,0,SteppingValidationEnum.FAIL);
+    }
+
+    @Test(expected = AbstractBuildParameters.DontTriggerException.class)
+    public void countingWithNoIncrementShouldNotWork2() throws Exception {
+        getParameters(1,2,0,SteppingValidationEnum.SKIP);
+
+    }
+
+    @Test
+    public void countingWithNoIncrementShouldNotWork3() throws Exception {
+        List<AbstractBuildParameters> parameters = getParameters(1,2,0,SteppingValidationEnum.NOPARMS);
+        assertEquals(0, parameters.size());
+    }
+
     private List<AbstractBuildParameters> getParameters(long from, long to, long step) throws IOException, InterruptedException, AbstractBuildParameters.DontTriggerException {
+        return getParameters(from, to, step, SteppingValidationEnum.FAIL);
+    }
+
+    private List<AbstractBuildParameters> getParameters(long from, long to, long step, SteppingValidationEnum validationFailure) throws IOException, InterruptedException, AbstractBuildParameters.DontTriggerException {
         AbstractBuild<?,?> build = mock(AbstractBuild.class);
         EnvVars vars = new EnvVars();
         TaskListener listener = mock(TaskListener.class);
         when(build.getEnvironment(listener)).thenReturn(vars);
-        CounterBuildParameterFactory counterFactory = new CounterBuildParameterFactory(from,to,step,"");
+        when(listener.getLogger()).thenReturn(System.out);
+        CounterBuildParameterFactory counterFactory = new CounterBuildParameterFactory(from,to,step,"",validationFailure);
+
         return counterFactory.getParameters(build, listener);
     }
+
+
 }