Merge branch 'master' into JENKINS-40370

Conflicts: pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidator.java pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java
jenkinsci · Jan 5, 2017 · 729dfab · 729dfab
2 parents 289e89e + 85b8005
commit 729dfab
Show file tree

Hide file tree

Showing 30 changed files with 113 additions and 608 deletions.
diff --git a/SYNTAX.md b/SYNTAX.md
@@ -296,8 +296,7 @@ parameters {
 ```
 
 ### Options
-* *Description*: Other options exclusive to Declarative, such as `skipDefaultCheckout`, and traditional `JobProperty`s, such as 
-build discarding, limiting concurrent builds, and more.
+* *Description*: Traditional `JobProperty`s, such as `buildDiscarder` or `disableConcurrentBuilds`, Declarative-specifc options, such as `skipDefaultCheckout`, and "wrappers" that should wrap the entire build, such as `timeout`.
 * *Required*: No
 * *Allowed In*: Top-level `pipeline` closure only.
 * *Parameters*: None

diff --git a/...src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTPipelineDef.java b/...src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTPipelineDef.java
@@ -19,7 +19,6 @@ public final class ModelASTPipelineDef extends ModelASTElement {
     private ModelASTOptions options;
     private ModelASTBuildParameters parameters;
     private ModelASTTriggers triggers;
-    private ModelASTWrappers wrappers;
 
     public ModelASTPipelineDef(Object sourceLocation) {
         super(sourceLocation);
@@ -48,11 +47,6 @@ public JSONObject toJSON() {
         } else {
             a.put("triggers", null);
         }
-        if (wrappers != null && !wrappers.getWrappers().isEmpty()) {
-            a.put("wrappers", wrappers.toJSON());
-        } else {
-            a.put("wrappers", null);
-        }
         return new JSONObject().accumulate("pipeline", a);
     }
 
@@ -84,9 +78,6 @@ public void validate(ModelValidator validator) {
         if (triggers != null) {
             triggers.validate(validator);
         }
-        if (wrappers != null) {
-            wrappers.validate(validator);
-        }
     }
 
     @Override
@@ -117,9 +108,6 @@ public String toGroovy() {
         if (triggers != null && !triggers.getTriggers().isEmpty()) {
             result.append(triggers.toGroovy()).append('\n');
         }
-        if (wrappers != null && !wrappers.getWrappers().isEmpty()) {
-            result.append(wrappers.toGroovy()).append('\n');
-        }
 
         result.append("}\n");
         return result.toString();
@@ -191,9 +179,6 @@ public void removeSourceLocation() {
         if (triggers != null) {
             triggers.removeSourceLocation();
         }
-        if (wrappers != null) {
-            wrappers.removeSourceLocation();
-        }
     }
 
     private String indent(int count) {
@@ -264,13 +249,6 @@ public void setTriggers(ModelASTTriggers triggers) {
         this.triggers = triggers;
     }
 
-    public ModelASTWrappers getWrappers() {
-        return wrappers;
-    }
-
-    public void setWrappers(ModelASTWrappers wrappers) {
-        this.wrappers = wrappers;
-    }
 
     @Override
     public String toString() {
@@ -283,7 +261,6 @@ public String toString() {
                 ", options=" + options +
                 ", parameters=" + parameters +
                 ", triggers=" + triggers +
-                ", wrappers=" + wrappers +
                 "}";
     }
 
@@ -326,10 +303,7 @@ public boolean equals(Object o) {
         if (getParameters() != null ? !getParameters().equals(that.getParameters()) : that.getParameters() != null) {
             return false;
         }
-        if (getTriggers() != null ? !getTriggers().equals(that.getTriggers()) : that.getTriggers() != null) {
-            return false;
-        }
-        return getWrappers() != null ? getWrappers().equals(that.getWrappers()) : that.getWrappers() == null;
+        return getTriggers() != null ? getTriggers().equals(that.getTriggers()) : that.getTriggers() == null;
 
     }
 
@@ -344,7 +318,6 @@ public int hashCode() {
         result = 31 * result + (getOptions() != null ? getOptions().hashCode() : 0);
         result = 31 * result + (getParameters() != null ? getParameters().hashCode() : 0);
         result = 31 * result + (getTriggers() != null ? getTriggers().hashCode() : 0);
-        result = 31 * result + (getWrappers() != null ? getWrappers().hashCode() : 0);
         return result;
     }
 }
diff --git a/...api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTWrapper.java b/...api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTWrapper.java
diff --git a/...pi/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTWrappers.java b/...pi/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTWrappers.java
diff --git a/...rc/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidator.java b/...rc/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidator.java
@@ -45,8 +45,6 @@
 import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTTrigger;
 import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTTriggers;
 import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTWhen;
-import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTWrapper;
-import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTWrappers;
 
 
 public interface ModelValidator {
@@ -90,9 +88,5 @@ public interface ModelValidator {
 
     boolean validateElement(ModelASTStages stages);
 
-    boolean validateElement(ModelASTWrapper wrapper);
-
-    boolean validateElement(ModelASTWrappers wrappers);
-
     boolean validateWhenCondition(ModelASTStep condition);
 }
diff --git a/pipeline-model-api/src/main/resources/ast-schema.json b/pipeline-model-api/src/main/resources/ast-schema.json
@@ -117,7 +117,7 @@
       ]
     },
     "options": {
-      "description": "One or more options (including job properties)",
+      "description": "One or more options (including job properties, wrappers, and options specific to Declarative Pipelines)",
       "type": "object",
       "properties": {
         "options": {
@@ -158,20 +158,6 @@
       },
       "additionalProperties": false
     },
-    "wrappers": {
-      "description": "One or more wrappers",
-      "type": "object",
-      "properties": {
-        "wrappers": {
-          "type": "array",
-          "items": {
-            "$ref": "#/definitions/methodCall"
-          },
-          "minItems": 1
-        }
-      },
-      "additionalProperties": false
-    },
     "step": {
       "description": "A single step with parameters",
       "type": "object",
@@ -402,9 +388,6 @@
         },
         "parameters": {
           "$ref": "#/definitions/parameters"
-        },
-        "wrappers": {
-          "$ref": "#/definitions/wrappers"
         }
       },
       "required": [

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
@@ -53,6 +53,7 @@ import org.jenkinsci.plugins.workflow.cps.nodes.StepEndNode
 import org.jenkinsci.plugins.workflow.cps.nodes.StepStartNode
 import org.jenkinsci.plugins.workflow.graph.FlowNode
 import org.jenkinsci.plugins.workflow.job.WorkflowRun
+import org.jenkinsci.plugins.workflow.steps.StepDescriptor
 import org.jenkinsci.plugins.workflow.support.steps.StageStep
 
 import javax.annotation.Nullable
@@ -313,13 +314,13 @@ public class Utils {
      * @return A {@link LoadingCache} for looking up types from symbols.
      */
     static generateTypeCache(Class<? extends Descriptor> type, boolean includeClassNames = false,
-                             List<String> excludedSymbols = []) {
+                             List<String> excludedSymbols = [], Closure<Boolean> filter = null) {
         return CacheBuilder.newBuilder()
             .expireAfterWrite(10, TimeUnit.MINUTES)
             .build(new CacheLoader<Object, Map<String, String>>() {
             @Override
             Map<String, String> load(Object key) throws Exception {
-                return populateTypeCache(type, includeClassNames, excludedSymbols)
+                return populateTypeCache(type, includeClassNames, excludedSymbols, filter)
             }
         })
     }
@@ -334,18 +335,26 @@ public class Utils {
      */
     private static Map<String,String> populateTypeCache(Class<? extends Descriptor> type,
                                                         boolean includeClassNames = false,
-                                                        List<String> excludedSymbols = []) {
+                                                        List<String> excludedSymbols = [],
+                                                        Closure<Boolean> filter = null) {
         Map<String,String> knownTypes = [:]
 
         ExtensionList.lookup(type).each { t ->
-            Set<String> symbolValue = SymbolLookup.getSymbolValue(t)
-            if (!symbolValue.isEmpty() && !symbolValue.any { excludedSymbols.contains(it) }) {
-                knownTypes.put(symbolValue.iterator().next(), t.clazz.getName())
-            }
+            if (filter == null || filter.call(t)) {
+                // Have to special-case StepDescriptor since it doesn't actually have symbols!
+                if (t instanceof StepDescriptor) {
+                    knownTypes.put(t.functionName, t.clazz.getName())
+                } else {
+                    Set<String> symbolValue = SymbolLookup.getSymbolValue(t)
+                    if (!symbolValue.isEmpty() && !symbolValue.any { excludedSymbols.contains(it) }) {
+                        knownTypes.put(symbolValue.iterator().next(), t.clazz.getName())
+                    }
+                }
 
-            if (includeClassNames) {
-                // Add the class name mapping even if we also found the symbol, for backwards compatibility reasons.
-                knownTypes.put(t.clazz.getName(), t.clazz.getName())
+                if (includeClassNames) {
+                    // Add the class name mapping even if we also found the symbol, for backwards compatibility reasons.
+                    knownTypes.put(t.clazz.getName(), t.clazz.getName())
+                }
             }
         }