Skip to content

Commit

Permalink
Merge pull request #215 from abayer/jenkins-47780-mk2
Browse files Browse the repository at this point in the history 
[FIXED JENKINS-47780] Fix instantiation time scoping of describables
  • Loading branch information
@abayer
abayer committed Nov 8, 2017
2 parents 73124e3 + 1a54eb8 commit f567261
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 15 deletions.
25 changes: 14 additions & 11 deletions ...c/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ASTParserUtils.groovy
View file
Expand Up @@ -251,17 +251,17 @@ class ASTParserUtils {

/**
* Takes a list of {@link ModelASTElement}s corresponding to {@link Describable}s (such as {@link JobProperty}s, etc),
* and transforms their Groovy AST nodes into AST from {@link #methodCallToDescribable(MethodCallExpression)}.
* and transforms their Groovy AST nodes into AST from {@link #methodCallToDescribable(MethodCallExpression,Class)}.
*/
@Nonnull
static Expression transformListOfDescribables(@CheckForNull List<ModelASTElement> children) {
static Expression transformListOfDescribables(@CheckForNull List<ModelASTElement> children, Class<? extends Describable> descClass) {
ListExpression descList = new ListExpression()

children?.each { d ->
if (d.sourceLocation instanceof Statement) {
MethodCallExpression m = matchMethodCall((Statement) d.sourceLocation)
if (m != null) {
descList.addExpression(methodCallToDescribable(m))
descList.addExpression(methodCallToDescribable(m,descClass))
} else {
throw new IllegalArgumentException("Expected a method call expression but received ${d.sourceLocation}")
}
Expand All @@ -279,13 +279,15 @@ class ASTParserUtils {
* @param children The children for the original element - passed as a separate argument since the getter will
* be different.
* @param containerClass The class we will be instantiating, i.e., {@link Parameters} or {@link Triggers}.
* @param descClass the describable class we're inheriting from
* @return The AST for instantiating the container and its contents.
*/
static Expression transformDescribableContainer(@CheckForNull ModelASTElement original,
@CheckForNull List<ModelASTElement> children,
@Nonnull Class containerClass) {
@CheckForNull List<ModelASTElement> children,
@Nonnull Class containerClass,
@Nonnull Class<? extends Describable> descClass) {
if (isGroovyAST(original) && !children?.isEmpty()) {
return ctorX(ClassHelper.make(containerClass), args(transformListOfDescribables(children)))
return ctorX(ClassHelper.make(containerClass), args(transformListOfDescribables(children, descClass)))
}
return constX(null)
}
Expand All @@ -305,7 +307,7 @@ class ASTParserUtils {

if (methCall != null) {
if (cond.children.isEmpty()) {
return methodCallToDescribable(methCall)
return methodCallToDescribable(methCall, null)
} else {
MapExpression argMap = new MapExpression()
if (parentDesc.allowedChildrenCount == 1) {
Expand Down Expand Up @@ -375,15 +377,15 @@ class ASTParserUtils {
singleArg.mapEntryExpressions.each { entry ->
if (entry.valueExpression instanceof MethodCallExpression) {
MethodCallExpression m = (MethodCallExpression) entry.valueExpression
tmpMap.addMapEntryExpression(entry.keyExpression, methodCallToDescribable(m))
tmpMap.addMapEntryExpression(entry.keyExpression, methodCallToDescribable(m, null))
} else {
tmpMap.addMapEntryExpression(entry)
}
}
} else {
if (singleArg instanceof MethodCallExpression) {
tmpMap.addMapEntryExpression(constX(UninstantiatedDescribable.ANONYMOUS_KEY),
methodCallToDescribable(singleArg))
methodCallToDescribable(singleArg, null))
} else {
tmpMap.addMapEntryExpression(constX(UninstantiatedDescribable.ANONYMOUS_KEY), singleArg)
}
Expand All @@ -407,16 +409,17 @@ class ASTParserUtils {
* Transforms a {@link MethodCallExpression} into either a map of name and arguments for steps, or a call to
* {@link Utils#instantiateDescribable(Class,Map)} that can be invoked at runtime to actually instantiated.
* @param expr A method call.
* @param descClass possibly null describable parent class
* @return The appropriate transformation, or the original expression if it didn't correspond to a Describable.
*/
@CheckForNull
static Expression methodCallToDescribable(MethodCallExpression expr) {
static Expression methodCallToDescribable(MethodCallExpression expr, Class<? extends Describable> descClass) {
def methodName = matchMethodName(expr)
List<Expression> methArgs = methodCallArgs(expr)

DescriptorLookupCache lookupCache = DescriptorLookupCache.getPublicCache()

Descriptor<? extends Describable> funcDesc = lookupCache.lookupFunction(methodName)
Descriptor<? extends Describable> funcDesc = lookupCache.lookupFunction(methodName, descClass)
StepDescriptor stepDesc = lookupCache.lookupStepDescriptor(methodName)
// This is the case where we've got a wrapper in options
if (stepDesc != null || (funcDesc != null && !StepDescriptor.metaStepsOf(methodName).isEmpty())) {
Expand Down
10 changes: 6 additions & 4 deletions ...groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/RuntimeASTTransformer.groovy
View file
Expand Up @@ -26,7 +26,9 @@ package org.jenkinsci.plugins.pipeline.modeldefinition.parser

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings
import hudson.model.JobProperty
import hudson.model.ParameterDefinition
import hudson.model.Run
import hudson.triggers.Trigger
import org.codehaus.groovy.ast.ClassHelper
import org.codehaus.groovy.ast.DynamicVariable
import org.codehaus.groovy.ast.expr.*
Expand Down Expand Up @@ -542,7 +544,7 @@ class RuntimeASTTransformer {
if (o.getSourceLocation() instanceof Statement) {
MethodCallExpression expr = matchMethodCall((Statement) o.getSourceLocation())
if (expr != null) {
optsMap.addMapEntryExpression(constX(o.name), methodCallToDescribable(expr))
optsMap.addMapEntryExpression(constX(o.name), methodCallToDescribable(expr, DeclarativeOption.class))
}
}
}
Expand All @@ -564,7 +566,7 @@ class RuntimeASTTransformer {
}

return ctorX(ClassHelper.make(Options.class),
args(transformListOfDescribables(jobProps), optsMap, wrappersMap))
args(transformListOfDescribables(jobProps, JobProperty.class), optsMap, wrappersMap))
}

return constX(null)
Expand All @@ -578,7 +580,7 @@ class RuntimeASTTransformer {
* cannot be transformed.
*/
Expression transformParameters(@CheckForNull ModelASTBuildParameters original) {
return transformDescribableContainer(original, original?.parameters, Parameters.class)
return transformDescribableContainer(original, original?.parameters, Parameters.class, ParameterDefinition.class)
}

/**
Expand Down Expand Up @@ -774,7 +776,7 @@ class RuntimeASTTransformer {
* cannot be transformed.
*/
Expression transformTriggers(@CheckForNull ModelASTTriggers original) {
return transformDescribableContainer(original, original?.triggers, Triggers.class)
return transformDescribableContainer(original, original?.triggers, Triggers.class, Trigger.class)
}

}
10 changes: 10 additions & 0 deletions ...definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/TriggersTest.java
View file
Expand Up @@ -135,4 +135,14 @@ public void externalTriggersNotRemoved() throws Exception {
assertNotNull(t);
assertTrue(t instanceof SCMTrigger);
}

@Issue("JENKINS-47780")
@Test
public void actualTriggerCorrectScope() throws Exception {
WorkflowRun b = getAndStartNonRepoBuild("simpleTriggers");
j.assertBuildStatusSuccess(j.waitForCompletion(b));

expect("actualTriggerCorrectScope")
.go();
}
}
40 changes: 40 additions & 0 deletions pipeline-model-definition/src/test/resources/actualTriggerCorrectScope.groovy
View file
@@ -0,0 +1,40 @@
/*
* 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 none
triggers {
upstream(upstreamProjects: "simpleTriggers", threshold: hudson.model.Result.SUCCESS)
}
stages {
stage("foo") {
steps {
echo "hello"
}
}
}
}



0 comments on commit f567261

Please sign in to comment.