Merge pull request #138 from abayer/jenkins-42762

[FIXED JENKINS-42762] Allow multiple conditions in when directly
jenkinsci · Mar 15, 2017 · 2b88323 · 2b88323
2 parents 6ead78c + 22dc056
commit 2b88323
Show file tree

Hide file tree

Showing 23 changed files with 186 additions and 113 deletions.
diff --git a/...el-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTWhen.java b/...el-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTWhen.java
@@ -37,51 +37,59 @@
  */
 public class ModelASTWhen extends ModelASTElement {
 
-    private ModelASTWhenContent condition;
+    private List<ModelASTWhenContent> conditions = new ArrayList<>();
 
     public ModelASTWhen(Object sourceLocation) {
         super(sourceLocation);
     }
 
-    public ModelASTWhenContent getCondition() {
-        return condition;
+    public List<ModelASTWhenContent> getConditions() {
+        return conditions;
     }
 
-    public void setCondition(ModelASTWhenContent condition) {
-        this.condition = condition;
+    public void setConditions(List<ModelASTWhenContent> conditions) {
+        this.conditions = conditions;
     }
 
     @Override
     public Object toJSON() {
-        return new JSONObject().accumulate("condition", condition.toJSON());
+        final JSONArray a = new JSONArray();
+        for (ModelASTWhenContent c : conditions) {
+            a.add(c.toJSON());
+        }
+        return new JSONObject().accumulate("conditions", a);
     }
 
     @Override
     public String toGroovy() {
         StringBuilder result = new StringBuilder("when {\n");
-        result.append(condition.toGroovy()).append("\n");
+        for (ModelASTWhenContent c : conditions) {
+            result.append(c.toGroovy()).append("\n");
+        }
         result.append("}\n");
         return result.toString();
     }
 
     @Override
     public void removeSourceLocation() {
         super.removeSourceLocation();
-        condition.removeSourceLocation();
+        for (ModelASTWhenContent c : conditions) {
+            c.removeSourceLocation();
+        }
     }
 
     @Override
     public String toString() {
         return "ModelASTWhen{" +
-                "condition=" + condition +
+                "conditions=" + conditions +
                 "}";
     }
 
     @Override
     public void validate(final ModelValidator validator) {
         validator.validateElement(this);
-        if (condition != null) {
-            condition.validate(validator);
+        for (ModelASTWhenContent c : conditions) {
+            c.validate(validator);
         }
     }
 }
diff --git a/pipeline-model-api/src/main/resources/ast-schema.json b/pipeline-model-api/src/main/resources/ast-schema.json
@@ -407,19 +407,23 @@
       "description": "Conditions to evaluate whether the stage should run or not",
       "type": "object",
       "properties": {
-        "condition": {
-          "anyOf": [
-            {
-              "$ref": "#/definitions/step"
-            },
-            {
-              "$ref": "#/definitions/nestedWhenCondition"
-            }
-          ]
+        "conditions": {
+          "type": "array",
+          "minItems": 1,
+          "items": {
+            "anyOf": [
+              {
+                "$ref": "#/definitions/step"
+              },
+              {
+                "$ref": "#/definitions/nestedWhenCondition"
+              }
+            ]
+          }
         }
       },
       "required": [
-        "condition"
+        "conditions"
       ],
       "additionalProperties": false
     },

diff --git a/...el-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/Utils.groovy b/...el-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/Utils.groovy
@@ -262,7 +262,12 @@ public class Utils {
         root.stages.stages.each { s ->
             ModelASTStage astStage = model.stages.stages.find { it.name == s.name }
             if (astStage.when != null) {
-                s.when(new StageConditionals(stageConditionalFromAST(astStage.when.condition)))
+                List<DeclarativeStageConditional<? extends DeclarativeStageConditional>> processedConditions =
+                    astStage.when.conditions.collect { c ->
+                        stageConditionalFromAST(c)
+                    }
+
+                s.when(new StageConditionals(processedConditions))
             }
             stagesWithWhen.add(s)
         }

diff --git a/...main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/StageConditionals.groovy b/...main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/StageConditionals.groovy
@@ -40,7 +40,7 @@ import org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable
 @ToString
 @EqualsAndHashCode
 @SuppressFBWarnings(value="SE_NO_SERIALVERSIONID")
-class StageConditionals implements Serializable {
+class StageConditionals implements MethodsToList<DeclarativeStageConditional<? extends DeclarativeStageConditional>>, Serializable {
     private static final Object NESTED_CACHE_KEY = new Object()
     private static final Object MULTIPLE_NESTED_CACHE_KEY = new Object()
 
@@ -66,10 +66,9 @@ class StageConditionals implements Serializable {
         return multipleNestedTypeCache.get(MULTIPLE_NESTED_CACHE_KEY)
     }
 
-    public DeclarativeStageConditional condition
+    public List<DeclarativeStageConditional> conditions = []
 
-    public StageConditionals(DeclarativeStageConditional<? extends DeclarativeStageConditional> c) {
-        this.condition = c
+    public StageConditionals(List<DeclarativeStageConditional<? extends DeclarativeStageConditional>> inList) {
+        conditions.addAll(inList)
     }
-
 }
diff --git a/...n/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/JSONParser.groovy b/...n/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/JSONParser.groovy
@@ -202,8 +202,11 @@ class JSONParser implements Parser {
     public @CheckForNull ModelASTWhen parseWhen(JsonTree j) {
         ModelASTWhen when = new ModelASTWhen(j)
 
-        JsonTree conditionTree = j.append(JsonPointer.of("condition"))
-        when.condition = parseWhenContent(conditionTree)
+        JsonTree conditionsTree = j.append(JsonPointer.of("conditions"))
+        conditionsTree.node.eachWithIndex { JsonNode entry, int i ->
+            JsonTree condTree = conditionsTree.append(JsonPointer.of(i))
+            when.conditions.add(parseWhenContent(condTree))
+        }
 
         return when
     }

diff --git a/.../src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy b/.../src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy
@@ -121,7 +121,7 @@ class ModelParser implements Parser {
         }
 
         if (pst==null) {
-            // Check if there's a 'pipeline' step somewhere children within the other statements and error out if that's the case.
+            // Check if there's a 'pipeline' step somewhere nexted within the other statements and error out if that's the case.
             src.statementBlock.statements.each { checkForNestedPipelineStep(it) }
             return null; // no 'pipeline', so this doesn't apply
         }
@@ -420,11 +420,8 @@ class ModelParser implements Parser {
         def stepsBlock = matchBlockStatement(statement)
         BlockStatement block = asBlock(stepsBlock.body.code)
         ModelASTWhen w = new ModelASTWhen(statement)
-        if (block.statements.size() != 1) {
-            errorCollector.error(w, Messages.ModelParser_WrongWhenCount())
-            return w
-        } else {
-            w.condition = parseWhenContent(block.statements.first())
+        block.statements.each { s ->
+            w.conditions.add(parseWhenContent(s))
         }
 
         return w

diff --git a/...groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy b/...groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy
@@ -175,7 +175,7 @@ class ModelValidatorImpl implements ModelValidator {
 
     public boolean validateElement(ModelASTWhen when) {
         boolean valid = true
-        if (when.condition == null) {
+        if (when.conditions.isEmpty()) {
             errorCollector.error(when, Messages.ModelValidatorImpl_EmptyWhen())
             valid = false
         }

diff --git a/...src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ModelInterpreter.groovy b/...src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ModelInterpreter.groovy
@@ -415,13 +415,17 @@ public class ModelInterpreter implements Serializable {
      *
      */
     def evaluateWhen(StageConditionals when) {
-        if (when != null) {
-            DeclarativeStageConditional c = when.condition
-            if (!c.getScript(script).evaluate()) {
-                return false
+        if (when == null) {
+            return true
+        } else {
+            for (int i = 0; i < when.conditions.size(); i++) {
+                DeclarativeStageConditional c = when.conditions.get(i)
+                if (!c.getScript(script).evaluate()) {
+                    return false
+                }
             }
+            return true
         }
-        return true
     }
 
     /**

diff --git a/...on/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java b/...on/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java
@@ -170,7 +170,7 @@ public static Iterable<Object[]> configsWithErrors() {
         result.add(new Object[]{"emptyJobProperties", Messages.JSONParser_TooFewItems(0, 1)});
         result.add(new Object[]{"emptyParameters", Messages.JSONParser_TooFewItems(0, 1)});
         result.add(new Object[]{"emptyTriggers", Messages.JSONParser_TooFewItems(0, 1)});
-        result.add(new Object[]{"emptyWhen", "instance failed to match at least one schema"});
+        result.add(new Object[]{"emptyWhen", Messages.JSONParser_TooFewItems(0, 1)});
         result.add(new Object[]{"mixedMethodArgs", Messages.ModelValidatorImpl_MixedNamedAndUnnamedParameters()});
 
         result.add(new Object[]{"rejectPropertiesStepInMethodCall",

diff --git a/...ition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java b/...ition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java
@@ -401,6 +401,15 @@ public void whenAnd() throws Exception {
                 .go();
     }
 
+    @Issue("JENKINS-42762")
+    @Test
+    public void whenMultiple() throws Exception {
+        expect("whenMultiple")
+                .logContains("[Pipeline] { (One)", "[Pipeline] { (Two)")
+                .logNotContains("World")
+                .go();
+    }
+
     @Test
     public void whenAndOrSingle() throws Exception {
         expect("whenAndOrSingle")

diff --git a/pipeline-model-definition/src/test/resources/json/basicWhen.json b/pipeline-model-definition/src/test/resources/json/basicWhen.json
@@ -31,13 +31,13 @@
           }]
         }]
       }],
-      "when": {"condition": {
+      "when": {"conditions": [      {
         "name": "expression",
-        "arguments": {
+        "arguments":         {
           "isLiteral": true,
           "value": "echo \"Should I run?\"\n                    return true"
         }
-      }}
+      }]}
     }
   ],
   "agent": {"type": "any"}

diff --git a/pipeline-model-definition/src/test/resources/json/errors/emptyWhen.json b/pipeline-model-definition/src/test/resources/json/errors/emptyWhen.json
@@ -11,7 +11,7 @@
         }
       }]
     }],
-    "when": {"condition": null}
+    "when": {"conditions": []}
   }],
   "agent": {"type": "none"}
 }}
diff --git a/pipeline-model-definition/src/test/resources/json/errors/unknownWhenConditional.json b/pipeline-model-definition/src/test/resources/json/errors/unknownWhenConditional.json
@@ -11,13 +11,13 @@
         }
       }]
     }],
-    "when": {"condition": {
+    "when": {"conditions": [    {
       "name": "banana",
-      "arguments": {
+      "arguments":       {
         "isLiteral": true,
         "value": "monkey"
       }
-    }}
+    }]}
   }],
   "agent": {"type": "none"}
 }}
diff --git a/pipeline-model-definition/src/test/resources/json/errors/whenInvalidParameterType.json b/pipeline-model-definition/src/test/resources/json/errors/whenInvalidParameterType.json
@@ -25,13 +25,13 @@
           }
         }]
       }],
-      "when": {"condition": {
+      "when": {"conditions": [      {
         "name": "branch",
-        "arguments": {
+        "arguments":         {
           "isLiteral": true,
           "value": 4
         }
-      }}
+      }]}
     }
   ],
   "agent": {"type": "any"}

diff --git a/pipeline-model-definition/src/test/resources/json/errors/whenMissingRequiredParameter.json b/pipeline-model-definition/src/test/resources/json/errors/whenMissingRequiredParameter.json
@@ -25,18 +25,16 @@
           }
         }]
       }],
-      "when": {"condition": {
+      "when": {"conditions": [      {
         "name": "environment",
-        "arguments": [
-          {
-            "key": "name",
-            "value": {
-              "isLiteral": true,
-              "value": "FOO"
-            }
+        "arguments": [        {
+          "key": "name",
+          "value":           {
+            "isLiteral": true,
+            "value": "FOO"
           }
-        ]
-      }}
+        }]
+      }]}
     }
   ],
   "environment": [  {

diff --git a/pipeline-model-definition/src/test/resources/json/errors/whenUnknownParameter.json b/pipeline-model-definition/src/test/resources/json/errors/whenUnknownParameter.json
@@ -25,32 +25,32 @@
           }
         }]
       }],
-      "when": {"condition": {
+      "when": {"conditions": [      {
         "name": "environment",
-        "arguments": [
+        "arguments":         [
           {
             "key": "name",
-            "value": {
+            "value":             {
               "isLiteral": true,
               "value": "FOO"
             }
           },
           {
             "key": "value",
-            "value": {
+            "value":             {
               "isLiteral": true,
               "value": "SOME_OTHER_VALUE"
             }
           },
           {
             "key": "banana",
-            "value": {
+            "value":             {
               "isLiteral": true,
               "value": "monkey"
             }
           }
         ]
-      }}
+      }]}
     }
   ],
   "environment": [  {

diff --git a/pipeline-model-definition/src/test/resources/json/skippedWhen.json b/pipeline-model-definition/src/test/resources/json/skippedWhen.json
@@ -31,13 +31,13 @@
           }]
         }]
       }],
-      "when": {"condition": {
+      "when": {"conditions": [      {
         "name": "expression",
-        "arguments": {
+        "arguments":         {
           "isLiteral": true,
           "value": "echo \"Should I run?\"\n                    return false"
         }
-      }}
+      }]}
     }
   ],
   "agent": {"type": "any"}