[FIXED JENKINS-42762] Allow multiple conditions in when directly

Reverts changes made in JENKINS-41185 fix. Does not affect the core of the changes in JENKINS-41185.
jenkinsci · Mar 14, 2017 · 3bc41e0 · 3bc41e0
1 parent 6ead78c
commit 3bc41e0
Show file tree

Hide file tree

Showing 10 changed files with 122 additions and 41 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/...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/whenMultiple.groovy b/pipeline-model-definition/src/test/resources/whenMultiple.groovy
@@ -0,0 +1,52 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2017, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+pipeline {
+    agent any
+    environment {
+        BRANCH_NAME = "master"
+    }
+    stages {
+        stage("One") {
+            steps {
+                echo "Hello"
+            }
+        }
+        stage("Two") {
+            when {
+                branch "master"
+                expression {
+                    "foo" == "bar"
+                }
+            }
+            steps {
+                script {
+                    echo "World"
+                    echo "Heal it"
+                }
+
+            }
+        }
+    }
+}