From a2b71304cc341129bfd7ad149480c7e345a064ee Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 17 Sep 2015 17:31:02 -0400 Subject: [PATCH 01/12] Experimenting with JobPropertyStep. --- multibranch/pom.xml | 7 ++ .../workflow/multibranch/JobPropertyStep.java | 106 ++++++++++++++++++ ...flowParameterDefinitionBranchProperty.java | 56 --------- .../multibranch/JobPropertyStep/config.jelly | 33 ++++++ .../multibranch/JobPropertyStep/help.html | 4 + ...ertyTest.java => JobPropertyStepTest.java} | 54 ++++----- .../WorkflowMultiBranchProjectTest.java | 21 ++++ 7 files changed, 198 insertions(+), 83 deletions(-) create mode 100644 multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java delete mode 100644 multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowParameterDefinitionBranchProperty.java create mode 100644 multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly create mode 100644 multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/help.html rename multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/{WorkflowParameterDefinitionBranchPropertyTest.java => JobPropertyStepTest.java} (61%) diff --git a/multibranch/pom.xml b/multibranch/pom.xml index cbc6ead02..ec1697907 100644 --- a/multibranch/pom.xml +++ b/multibranch/pom.xml @@ -66,6 +66,13 @@ THE SOFTWARE. ${workflow.version} test + + ${project.groupId} + workflow-step-api + ${workflow.version} + tests + test + ${project.groupId} workflow-support diff --git a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java new file mode 100644 index 000000000..bcc497f61 --- /dev/null +++ b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java @@ -0,0 +1,106 @@ +/* + * The MIT License + * + * Copyright 2015 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. + */ + +package org.jenkinsci.plugins.workflow.multibranch; + +import hudson.AbortException; +import hudson.BulkChange; +import hudson.Extension; +import hudson.model.Job; +import hudson.model.JobProperty; +import hudson.model.Run; +import java.util.List; +import javax.inject.Inject; +import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl; +import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl; +import org.jenkinsci.plugins.workflow.steps.AbstractSynchronousStepExecution; +import org.jenkinsci.plugins.workflow.steps.StepContextParameter; +import org.kohsuke.stapler.DataBoundConstructor; + +/** + * Resets the properties of the current job. + */ +public class JobPropertyStep extends AbstractStepImpl { + + private final List> properties; + + @DataBoundConstructor public JobPropertyStep(List> properties) { + this.properties = properties; + } + + public List> getProperties() { + return properties; + } + + public static class Execution extends AbstractSynchronousStepExecution { + + @Inject transient JobPropertyStep step; + @StepContextParameter Run build; + + @SuppressWarnings({"unchecked", "rawtypes"}) // untypable + @Override protected Void run() throws Exception { + Job job = build.getParent(); + for (JobProperty prop : step.properties) { + if (!prop.getDescriptor().isApplicable(job.getClass())) { + throw new AbortException("cannot apply " + prop.getDescriptor().getId() + " to a " + job.getClass().getSimpleName()); + } + } + BulkChange bc = new BulkChange(job); + try { + for (JobProperty prop : job.getAllProperties()) { + if (prop instanceof BranchJobProperty) { + // TODO do we need to define an API for other properties which should not be removed? + continue; + } + job.removeProperty(prop); + } + for (JobProperty prop : step.properties) { + job.addProperty(prop); + } + bc.commit(); + } finally { + bc.abort(); + } + return null; + } + + } + + @Extension public static class DescriptorImpl extends AbstractStepDescriptorImpl { + + public DescriptorImpl() { + super(Execution.class); + } + + @Override public String getFunctionName() { + return "properties"; + } + + @Override public String getDisplayName() { + return "Set job properties"; + } + + } + +} diff --git a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowParameterDefinitionBranchProperty.java b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowParameterDefinitionBranchProperty.java deleted file mode 100644 index a9534a794..000000000 --- a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowParameterDefinitionBranchProperty.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * The MIT License - * - * Copyright 2015 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. - */ - -package org.jenkinsci.plugins.workflow.multibranch; - -import hudson.Extension; -import hudson.model.Job; -import hudson.model.Run; -import jenkins.branch.BranchPropertyDescriptor; -import jenkins.branch.MultiBranchProjectDescriptor; -import jenkins.branch.ParameterDefinitionBranchProperty; -import org.jenkinsci.plugins.workflow.job.WorkflowJob; -import org.kohsuke.stapler.DataBoundConstructor; - -public class WorkflowParameterDefinitionBranchProperty extends ParameterDefinitionBranchProperty { - - @DataBoundConstructor public WorkflowParameterDefinitionBranchProperty() {} - - @Override protected

, B extends Run> boolean isApplicable(Class

clazz) { - return clazz == WorkflowJob.class; - } - - @Extension public static class DescriptorImpl extends BranchPropertyDescriptor { - - @Override protected boolean isApplicable(MultiBranchProjectDescriptor projectDescriptor) { - return WorkflowMultiBranchProject.class.isAssignableFrom(projectDescriptor.getClazz()); - } - - @Override - public String getDisplayName() { - return "Parameters"; - } - } - -} diff --git a/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly b/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly new file mode 100644 index 000000000..252780b9f --- /dev/null +++ b/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly @@ -0,0 +1,33 @@ + + + + + + + + + + + diff --git a/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/help.html b/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/help.html new file mode 100644 index 000000000..dd8353d80 --- /dev/null +++ b/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/help.html @@ -0,0 +1,4 @@ +

+ Updates the properties of the job which runs this step. + Mainly useful from multibranch workflows, so that Jenkinsfile itself can encode what would otherwise be static job configuration. +
diff --git a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowParameterDefinitionBranchPropertyTest.java b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java similarity index 61% rename from multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowParameterDefinitionBranchPropertyTest.java rename to multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java index 0fb3bc41a..09226964a 100644 --- a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowParameterDefinitionBranchPropertyTest.java +++ b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java @@ -24,25 +24,21 @@ package org.jenkinsci.plugins.workflow.multibranch; -import hudson.model.DescriptorVisibilityFilter; -import hudson.model.ParameterDefinition; +import hudson.model.BooleanParameterDefinition; +import hudson.model.JobProperty; import hudson.model.ParametersAction; -import hudson.model.StringParameterDefinition; +import hudson.model.ParametersDefinitionProperty; import hudson.model.StringParameterValue; -import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; -import java.util.Set; +import java.util.List; import jenkins.branch.BranchProperty; -import jenkins.branch.BranchPropertyDescriptor; import jenkins.branch.BranchSource; -import jenkins.branch.BuildRetentionBranchProperty; import jenkins.branch.DefaultBranchPropertyStrategy; -import jenkins.branch.RateLimitBranchProperty; import jenkins.plugins.git.GitSCMSource; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import static org.jenkinsci.plugins.workflow.multibranch.WorkflowMultiBranchProjectTest.findBranchProject; +import org.jenkinsci.plugins.workflow.steps.StepConfigTester; import org.jenkinsci.plugins.workflow.steps.scm.GitSampleRepoRule; import org.junit.Test; import static org.junit.Assert.*; @@ -52,42 +48,46 @@ import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; -public class WorkflowParameterDefinitionBranchPropertyTest { +public class JobPropertyStepTest { @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher(); @Rule public JenkinsRule r = new JenkinsRule(); @Rule public GitSampleRepoRule sampleRepo = new GitSampleRepoRule(); - @Test public void propertyVisible() throws Exception { - WorkflowMultiBranchProject p = r.jenkins.createProject(WorkflowMultiBranchProject.class, "p"); - Set> clazzes = new HashSet>(); - for (BranchPropertyDescriptor d : DescriptorVisibilityFilter.apply(p, BranchPropertyDescriptor.all())) { - clazzes.add(d.clazz); - } - @SuppressWarnings("unchecked") - Set> expected = new HashSet>(Arrays.asList( - WorkflowParameterDefinitionBranchProperty.class, - RateLimitBranchProperty.class, - BuildRetentionBranchProperty.class - /* UntrustedBranchProperty should not be here! */)); - assertEquals(expected, clazzes); + @Issue("JENKINS-30519") + @Test public void configRoundTrip() throws Exception { + StepConfigTester tester = new StepConfigTester(r); + // TODO fails (returns null) + assertEquals(Collections.emptyList(), tester.configRoundTrip(new JobPropertyStep(Collections.>emptyList())).getProperties()); + List> properties = tester.configRoundTrip(new JobPropertyStep(Collections.>singletonList(new ParametersDefinitionProperty(new BooleanParameterDefinition("flag", true, null))))).getProperties(); + assertEquals(1, properties.size()); + assertEquals(ParametersDefinitionProperty.class, properties.get(0)); + ParametersDefinitionProperty pdp = (ParametersDefinitionProperty) properties.get(0); + assertEquals(1, pdp.getParameterDefinitions().size()); + assertEquals(BooleanParameterDefinition.class, pdp.getParameterDefinitions().get(0).getClass()); + BooleanParameterDefinition bpd = (BooleanParameterDefinition) pdp.getParameterDefinitions().get(0); + assertEquals("flag", bpd.getName()); + assertTrue(bpd.isDefaultValue()); + // TODO Snippetizer fails with: NoStaplerConstructorException: There's no @DataBoundConstructor on any constructor of class hudson.model.ParametersDefinitionProperty + // TODO also JENKINS-29711 means it seems to omit the required () } @Issue("JENKINS-30206") @Test public void useParameter() throws Exception { sampleRepo.init(); - sampleRepo.write("Jenkinsfile", "echo \"received ${myparam}\""); + sampleRepo.write("Jenkinsfile", + "properties([[$class: 'ParametersDefinitionProperty', parameterDefinitions: [[$class: 'StringParameterDefinition', name: 'myparam', defaultValue: 'default value']]]])\n" + + "echo \"received ${myparam}\""); sampleRepo.git("add", "Jenkinsfile"); sampleRepo.git("commit", "--all", "--message=flow"); WorkflowMultiBranchProject mp = r.jenkins.createProject(WorkflowMultiBranchProject.class, "p"); - WorkflowParameterDefinitionBranchProperty prop = new WorkflowParameterDefinitionBranchProperty(); - prop.setParameterDefinitions(Collections.singletonList(new StringParameterDefinition("myparam", "default value"))); - mp.getSourcesList().add(new BranchSource(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", false), new DefaultBranchPropertyStrategy(new BranchProperty[] {prop}))); + mp.getSourcesList().add(new BranchSource(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", false), new DefaultBranchPropertyStrategy(new BranchProperty[0]))); WorkflowJob p = findBranchProject(mp, "master"); assertEquals(1, mp.getItems().size()); r.waitUntilNoActivity(); WorkflowRun b1 = p.getLastBuild(); assertEquals(1, b1.getNumber()); + // TODO fails due to JENKINS-28510 r.assertLogContains("received default value", b1); WorkflowRun b2 = r.assertBuildStatusSuccess(p.scheduleBuild2(0, new ParametersAction(new StringParameterValue("myparam", "special value")))); assertEquals(2, b2.getNumber()); diff --git a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java index 1502be297..7b6e1f754 100644 --- a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java +++ b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java @@ -24,10 +24,17 @@ package org.jenkinsci.plugins.workflow.multibranch; +import hudson.model.DescriptorVisibilityFilter; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; import javax.annotation.Nonnull; import jenkins.branch.BranchProperty; +import jenkins.branch.BranchPropertyDescriptor; import jenkins.branch.BranchSource; +import jenkins.branch.BuildRetentionBranchProperty; import jenkins.branch.DefaultBranchPropertyStrategy; +import jenkins.branch.RateLimitBranchProperty; import jenkins.plugins.git.GitSCMSource; import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; import org.jenkinsci.plugins.workflow.job.WorkflowJob; @@ -88,4 +95,18 @@ public class WorkflowMultiBranchProjectTest { return p; } + @Test public void visibleBranchProperties() throws Exception { + WorkflowMultiBranchProject p = r.jenkins.createProject(WorkflowMultiBranchProject.class, "p"); + Set> clazzes = new HashSet>(); + for (BranchPropertyDescriptor d : DescriptorVisibilityFilter.apply(p, BranchPropertyDescriptor.all())) { + clazzes.add(d.clazz); + } + @SuppressWarnings("unchecked") + Set> expected = new HashSet>(Arrays.asList( + RateLimitBranchProperty.class, + BuildRetentionBranchProperty.class + /* UntrustedBranchProperty should not be here! */)); + assertEquals(expected, clazzes); + } + } From 110c42e3a9e52df26d67c5900f7a520d08014efc Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 23 Oct 2015 13:45:59 -0400 Subject: [PATCH 02/12] Will need upstream fixes. --- multibranch/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multibranch/pom.xml b/multibranch/pom.xml index f38bf68bd..3385c0f13 100644 --- a/multibranch/pom.xml +++ b/multibranch/pom.xml @@ -28,7 +28,7 @@ THE SOFTWARE. org.jenkins-ci.plugins.workflow workflow-pom - 1.11-beta-1 + 1.11-SNAPSHOT 1.11-beta-2-SNAPSHOT From b5302ebc2e2479f03c538060460e43d0c1c0b47a Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 26 Oct 2015 15:26:08 -0400 Subject: [PATCH 03/12] JENKINS-26535 can be worked around using a collection of raw types. --- .../plugins/workflow/multibranch/JobPropertyStep.java | 9 +++++---- .../workflow/multibranch/JobPropertyStepTest.java | 6 +++--- .../plugins/workflow/structs/DescribableHelper.java | 4 +++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java index bcc497f61..a89779bb8 100644 --- a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java +++ b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java @@ -41,15 +41,16 @@ /** * Resets the properties of the current job. */ +@SuppressWarnings("rawtypes") // TODO JENKINS-26535: cannot bind List> public class JobPropertyStep extends AbstractStepImpl { - private final List> properties; + private final List properties; - @DataBoundConstructor public JobPropertyStep(List> properties) { + @DataBoundConstructor public JobPropertyStep(List properties) { this.properties = properties; } - public List> getProperties() { + public List getProperties() { return properties; } @@ -58,7 +59,7 @@ public static class Execution extends AbstractSynchronousStepExecution { @Inject transient JobPropertyStep step; @StepContextParameter Run build; - @SuppressWarnings({"unchecked", "rawtypes"}) // untypable + @SuppressWarnings("unchecked") // untypable @Override protected Void run() throws Exception { Job job = build.getParent(); for (JobProperty prop : step.properties) { diff --git a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java index f85b10284..f806e5dea 100644 --- a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java +++ b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java @@ -54,12 +54,13 @@ public class JobPropertyStepTest { @Rule public JenkinsRule r = new JenkinsRule(); @Rule public GitSampleRepoRule sampleRepo = new GitSampleRepoRule(); + @SuppressWarnings("rawtypes") @Issue("JENKINS-30519") @Test public void configRoundTrip() throws Exception { StepConfigTester tester = new StepConfigTester(r); // TODO fails (returns null) - assertEquals(Collections.emptyList(), tester.configRoundTrip(new JobPropertyStep(Collections.>emptyList())).getProperties()); - List> properties = tester.configRoundTrip(new JobPropertyStep(Collections.>singletonList(new ParametersDefinitionProperty(new BooleanParameterDefinition("flag", true, null))))).getProperties(); + assertEquals(Collections.emptyList(), tester.configRoundTrip(new JobPropertyStep(Collections.emptyList())).getProperties()); + List properties = tester.configRoundTrip(new JobPropertyStep(Collections.singletonList(new ParametersDefinitionProperty(new BooleanParameterDefinition("flag", true, null))))).getProperties(); assertEquals(1, properties.size()); assertEquals(ParametersDefinitionProperty.class, properties.get(0)); ParametersDefinitionProperty pdp = (ParametersDefinitionProperty) properties.get(0); @@ -87,7 +88,6 @@ public class JobPropertyStepTest { r.waitUntilNoActivity(); WorkflowRun b1 = p.getLastBuild(); assertEquals(1, b1.getNumber()); - // TODO fails due to JENKINS-28510 r.assertLogContains("received default value", b1); WorkflowRun b2 = r.assertBuildStatusSuccess(p.scheduleBuild2(0, new ParametersAction(new StringParameterValue("myparam", "special value")))); assertEquals(2, b2.getNumber()); diff --git a/step-api/src/main/java/org/jenkinsci/plugins/workflow/structs/DescribableHelper.java b/step-api/src/main/java/org/jenkinsci/plugins/workflow/structs/DescribableHelper.java index 5c29101cd..ff0938856 100644 --- a/step-api/src/main/java/org/jenkinsci/plugins/workflow/structs/DescribableHelper.java +++ b/step-api/src/main/java/org/jenkinsci/plugins/workflow/structs/DescribableHelper.java @@ -219,7 +219,7 @@ private static Object coerce(String context, Type type, @Nonnull Object o) throw Jenkins j = Jenkins.getInstance(); ClassLoader loader = j != null ? j.getPluginManager().uberClassLoader : DescribableHelper.class.getClassLoader(); clazz = loader.loadClass(clazzS); - } else { + } else if (type instanceof Class) { clazz = null; for (Class c : findSubtypes((Class) type)) { if (c.getSimpleName().equals(clazzS)) { @@ -232,6 +232,8 @@ private static Object coerce(String context, Type type, @Nonnull Object o) throw if (clazz == null) { throw new UnsupportedOperationException("no known implementation of " + type + " is named " + clazzS); } + } else { + throw new UnsupportedOperationException("JENKINS-26535: do not know how to handle " + type); } return instantiate(clazz.asSubclass((Class) type), m); } else if (o instanceof String && type instanceof Class && ((Class) type).isEnum()) { From c621774286e3e0209cf08c9236945e9b86f29e18 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 26 Oct 2015 15:53:36 -0400 Subject: [PATCH 04/12] Temporarily working around lack of @DataBoundConstructor on ParametersDefinitionProperty. --- .../multibranch/JobPropertyStepTest.java | 3 +- .../workflow/structs/DescribableHelper.java | 28 ++++++++++++++----- .../structs/DescribableHelperTest.java | 5 ++++ 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java index f806e5dea..766d18ea0 100644 --- a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java +++ b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java @@ -69,8 +69,7 @@ public class JobPropertyStepTest { BooleanParameterDefinition bpd = (BooleanParameterDefinition) pdp.getParameterDefinitions().get(0); assertEquals("flag", bpd.getName()); assertTrue(bpd.isDefaultValue()); - // TODO Snippetizer fails with: NoStaplerConstructorException: There's no @DataBoundConstructor on any constructor of class hudson.model.ParametersDefinitionProperty - // TODO also JENKINS-29711 means it seems to omit the required () + // TODO JENKINS-29711 means it seems to omit the required () } @Issue("JENKINS-30206") diff --git a/step-api/src/main/java/org/jenkinsci/plugins/workflow/structs/DescribableHelper.java b/step-api/src/main/java/org/jenkinsci/plugins/workflow/structs/DescribableHelper.java index ff0938856..bd9e15b54 100644 --- a/step-api/src/main/java/org/jenkinsci/plugins/workflow/structs/DescribableHelper.java +++ b/step-api/src/main/java/org/jenkinsci/plugins/workflow/structs/DescribableHelper.java @@ -30,6 +30,7 @@ import hudson.model.Descriptor; import hudson.model.ParameterDefinition; import hudson.model.ParameterValue; +import hudson.model.ParametersDefinitionProperty; import java.beans.Introspector; import java.lang.reflect.Array; import java.lang.reflect.Constructor; @@ -85,8 +86,7 @@ public class DescribableHelper { * and only one subtype is registered (as a {@link Descriptor}) with that simple name. */ public static T instantiate(Class clazz, Map arguments) throws Exception { - ClassDescriptor d = new ClassDescriptor(clazz); - String[] names = d.loadConstructorParamNames(); + String[] names = loadConstructorParamNames(clazz); Constructor c = findConstructor(clazz, names.length); Object[] args = buildArguments(clazz, arguments, c.getGenericParameterTypes(), names, true); T o = c.newInstance(args); @@ -103,10 +103,9 @@ public static T instantiate(Class clazz, Map argument public static Map uninstantiate(Object o) throws UnsupportedOperationException { Class clazz = o.getClass(); Map r = new TreeMap(); - ClassDescriptor d = new ClassDescriptor(clazz); String[] names; try { - names = d.loadConstructorParamNames(); + names = loadConstructorParamNames(clazz); } catch (NoStaplerConstructorException x) { throw new UnsupportedOperationException(x); } @@ -267,9 +266,24 @@ private static List mapList(String context, Type type, List list) thr return r; } - // copied from RequestImpl + private static String[] loadConstructorParamNames(Class clazz) { + if (clazz == ParametersDefinitionProperty.class) { // TODO pending core fix + return new String[] {"parameterDefinitions"}; + } + return new ClassDescriptor(clazz).loadConstructorParamNames(); + } + + // adapted from RequestImpl + @SuppressWarnings("unchecked") private static Constructor findConstructor(Class clazz, int length) { - @SuppressWarnings("unchecked") Constructor[] ctrs = (Constructor[]) clazz.getConstructors(); + try { // may work without this, but only if the JVM happens to return the right overload first + if (clazz == ParametersDefinitionProperty.class && length == 1) { // TODO pending core fix + return (Constructor) ParametersDefinitionProperty.class.getConstructor(List.class); + } + } catch (NoSuchMethodException x) { + throw new AssertionError(x); + } + Constructor[] ctrs = (Constructor[]) clazz.getConstructors(); for (Constructor c : ctrs) { if (c.getAnnotation(DataBoundConstructor.class) != null) { if (c.getParameterTypes().length != length) { @@ -320,7 +334,7 @@ private static void inspect(Map r, Object o, Class clazz, Str AtomicReference type = new AtomicReference(); Object value = inspect(o, clazz, field, type); try { - String[] names = new ClassDescriptor(clazz).loadConstructorParamNames(); + String[] names = loadConstructorParamNames(clazz); int idx = Arrays.asList(names).indexOf(field); if (idx >= 0) { Type ctorType = findConstructor(clazz, names.length).getGenericParameterTypes()[idx]; diff --git a/step-api/src/test/java/org/jenkinsci/plugins/workflow/structs/DescribableHelperTest.java b/step-api/src/test/java/org/jenkinsci/plugins/workflow/structs/DescribableHelperTest.java index 17601e8a6..c0ebcc67d 100644 --- a/step-api/src/test/java/org/jenkinsci/plugins/workflow/structs/DescribableHelperTest.java +++ b/step-api/src/test/java/org/jenkinsci/plugins/workflow/structs/DescribableHelperTest.java @@ -30,6 +30,7 @@ import hudson.model.BooleanParameterValue; import hudson.model.Descriptor; import hudson.model.ParameterValue; +import hudson.model.ParametersDefinitionProperty; import hudson.plugins.git.GitSCM; import hudson.plugins.git.extensions.impl.CleanBeforeCheckout; import java.net.URL; @@ -463,6 +464,10 @@ public static final class TakesParams { } } + @Test public void parametersDefinitionProperty() throws Exception { + roundTrip(ParametersDefinitionProperty.class, map("parameterDefinitions", Arrays.asList(map("$class", "BooleanParameterDefinition", "name", "flag", "defaultValue", false), map("$class", "StringParameterDefinition", "name", "text")))); + } + @Issue("JENKINS-26619") @Test public void getterDescribableList() throws Exception { roundTrip(GitSCM.class, map( From 4320015acd28c40e6cce462c15e86d4873762f55 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 26 Oct 2015 17:01:52 -0400 Subject: [PATCH 05/12] ParametersDefinitionProperty not set when the first build begins. Without JENKINS-27295, this makes it awkward to have a default parameter value. --- .../plugins/workflow/multibranch/JobPropertyStepTest.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java index 766d18ea0..b88f1d53c 100644 --- a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java +++ b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java @@ -35,6 +35,7 @@ import jenkins.branch.BranchSource; import jenkins.branch.DefaultBranchPropertyStrategy; import jenkins.plugins.git.GitSCMSource; +import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.steps.StepConfigTester; @@ -75,9 +76,10 @@ public class JobPropertyStepTest { @Issue("JENKINS-30206") @Test public void useParameter() throws Exception { sampleRepo.init(); + ScriptApproval.get().approveSignature("method groovy.lang.Binding hasVariable java.lang.String"); // TODO add to generic whitelist sampleRepo.write("Jenkinsfile", "properties([[$class: 'ParametersDefinitionProperty', parameterDefinitions: [[$class: 'StringParameterDefinition', name: 'myparam', defaultValue: 'default value']]]])\n" + - "echo \"received ${myparam}\""); + "echo \"received ${binding.hasVariable('myparam') ? myparam : 'undefined'}\""); sampleRepo.git("add", "Jenkinsfile"); sampleRepo.git("commit", "--all", "--message=flow"); WorkflowMultiBranchProject mp = r.jenkins.createProject(WorkflowMultiBranchProject.class, "p"); @@ -87,7 +89,8 @@ public class JobPropertyStepTest { r.waitUntilNoActivity(); WorkflowRun b1 = p.getLastBuild(); assertEquals(1, b1.getNumber()); - r.assertLogContains("received default value", b1); + // TODO not all that satisfactory since it means you cannot rely on a default value; would be a little easier given JENKINS-27295 + r.assertLogContains("received undefined", b1); WorkflowRun b2 = r.assertBuildStatusSuccess(p.scheduleBuild2(0, new ParametersAction(new StringParameterValue("myparam", "special value")))); assertEquals(2, b2.getNumber()); r.assertLogContains("received special value", b2); From a8455584fafb66d347e0146ceba23572ba3176fc Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 26 Oct 2015 19:44:32 -0400 Subject: [PATCH 06/12] Added BuildDiscarderProperty to replace [Workflow]Job.buildDiscarder. --- .../plugins/workflow/job/WorkflowJob.java | 26 ++++++++ .../properties/BuildDiscarderProperty.java | 56 +++++++++++++++++ .../job/properties/OptionalJobProperty.java | 61 +++++++++++++++++++ .../config-details.jelly | 29 +++++++++ .../BuildDiscarderProperty/help.html | 19 ++++++ .../OptionalJobProperty/config.jelly | 33 ++++++++++ 6 files changed, 224 insertions(+) create mode 100644 job/src/main/java/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty.java create mode 100644 job/src/main/java/org/jenkinsci/plugins/workflow/job/properties/OptionalJobProperty.java create mode 100644 job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty/config-details.jelly create mode 100644 job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty/help.html create mode 100644 job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/OptionalJobProperty/config.jelly diff --git a/job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowJob.java b/job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowJob.java index 471007868..94330e2a4 100644 --- a/job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowJob.java +++ b/job/src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowJob.java @@ -58,6 +58,7 @@ import hudson.search.SearchIndexBuilder; import hudson.security.ACL; import hudson.slaves.WorkspaceList; +import hudson.tasks.LogRotator; import hudson.triggers.SCMTrigger; import hudson.triggers.Trigger; import hudson.triggers.TriggerDescriptor; @@ -73,6 +74,7 @@ import java.util.Map; import javax.annotation.CheckForNull; import javax.servlet.ServletException; +import jenkins.model.BuildDiscarder; import jenkins.model.Jenkins; import jenkins.model.ParameterizedJobMixIn; import jenkins.model.lazy.LazyBuildMixIn; @@ -81,6 +83,7 @@ import net.sf.json.JSONObject; import org.acegisecurity.Authentication; import org.jenkinsci.plugins.workflow.flow.FlowDefinition; +import org.jenkinsci.plugins.workflow.job.properties.BuildDiscarderProperty; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.DoNotUse; import org.kohsuke.stapler.QueryParameter; @@ -521,6 +524,29 @@ public void addTrigger(Trigger trigger) { // TODO call SCM.processWorkspaceBeforeDeletion } + /** Actually it does, but we want to suppress this section of {@code Job/configure.jelly} in favor of {@link BuildDiscarderProperty}. */ + @Override public boolean supportsLogRotator() { + return false; + } + + @SuppressWarnings("deprecation") + @Override public LogRotator getLogRotator() { + BuildDiscarder buildDiscarder = getBuildDiscarder(); + return buildDiscarder instanceof LogRotator ? (LogRotator) buildDiscarder : null; + } + + @Override public void setBuildDiscarder(BuildDiscarder bd) throws IOException { + removeProperty(BuildDiscarderProperty.class); + if (bd != null) { + addProperty(new BuildDiscarderProperty(bd)); + } + } + + @Override public BuildDiscarder getBuildDiscarder() { + BuildDiscarderProperty prop = getProperty(BuildDiscarderProperty.class); + return prop != null ? prop.getStrategy() : /* settings compatibility */ super.getBuildDiscarder(); + } + @Initializer(before=InitMilestone.EXTENSIONS_AUGMENTED) public static void alias() { Items.XSTREAM2.alias("flow-definition", WorkflowJob.class); diff --git a/job/src/main/java/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty.java b/job/src/main/java/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty.java new file mode 100644 index 000000000..71d601579 --- /dev/null +++ b/job/src/main/java/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty.java @@ -0,0 +1,56 @@ +/* + * The MIT License + * + * Copyright 2015 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. + */ + +package org.jenkinsci.plugins.workflow.job.properties; + +import hudson.Extension; +import jenkins.model.BuildDiscarder; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.kohsuke.stapler.DataBoundConstructor; + +/** + * Defines a {@link BuildDiscarder}. + * TODO consider whether this should be moved upstream to core. + */ +public class BuildDiscarderProperty extends OptionalJobProperty { + + private final BuildDiscarder strategy; + + @DataBoundConstructor public BuildDiscarderProperty(BuildDiscarder strategy) { + this.strategy = strategy; + } + + public BuildDiscarder getStrategy() { + return strategy; + } + + @Extension public static class DescriptorImpl extends OptionalJobPropertyDescriptor { + + @Override public String getDisplayName() { + return "Discard Old Builds"; + } + + } + +} diff --git a/job/src/main/java/org/jenkinsci/plugins/workflow/job/properties/OptionalJobProperty.java b/job/src/main/java/org/jenkinsci/plugins/workflow/job/properties/OptionalJobProperty.java new file mode 100644 index 000000000..ac6a74f65 --- /dev/null +++ b/job/src/main/java/org/jenkinsci/plugins/workflow/job/properties/OptionalJobProperty.java @@ -0,0 +1,61 @@ +/* + * The MIT License + * + * Copyright 2015 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. + */ + +package org.jenkinsci.plugins.workflow.job.properties; + +import hudson.model.Job; +import hudson.model.JobProperty; +import hudson.model.JobPropertyDescriptor; +import hudson.model.ParametersDefinitionProperty; +import net.sf.json.JSONObject; +import org.kohsuke.stapler.StaplerRequest; + +/** + * Job property which may or may not be present. + * Must define {@code config-details.jelly} or {@code config-details.groovy}. + * TODO should be moved into core and implemented by {@link ParametersDefinitionProperty}. + */ +abstract class OptionalJobProperty> extends JobProperty { + + @Override + public OptionalJobPropertyDescriptor getDescriptor() { + return (OptionalJobPropertyDescriptor) super.getDescriptor(); + } + + public static abstract class OptionalJobPropertyDescriptor extends JobPropertyDescriptor { + + protected OptionalJobPropertyDescriptor(Class> clazz) { + super(clazz); + } + + protected OptionalJobPropertyDescriptor() {} + + @Override + public JobProperty newInstance(StaplerRequest req, JSONObject formData) throws FormException { + return formData.optBoolean("specified") ? super.newInstance(req, formData) : null; + } + + } + +} diff --git a/job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty/config-details.jelly b/job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty/config-details.jelly new file mode 100644 index 000000000..a5f3d0a4d --- /dev/null +++ b/job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty/config-details.jelly @@ -0,0 +1,29 @@ + + + + + + + diff --git a/job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty/help.html b/job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty/help.html new file mode 100644 index 000000000..3d1e66957 --- /dev/null +++ b/job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/BuildDiscarderProperty/help.html @@ -0,0 +1,19 @@ +
+ This controls the disk consumption of Jenkins by managing how long you'd like to keep + records of the builds (such as console output, build artifacts, and so on.) + Jenkins offers two criteria: + +
    +
  1. + Driven by age. You can have Jenkins delete a record if it reaches a certain age + (for example, 7 days old.) +
  2. + Driven by number. You can have Jenkins make sure that it only maintains up to + N build records. If a new build is started, the oldest record will + be simply removed. +
+ + Jenkins also allows you to mark an individual build as 'Keep this log forever', to + exclude certain important builds from being discarded automatically. + The last stable and last successful build are always kept as well. +
diff --git a/job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/OptionalJobProperty/config.jelly b/job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/OptionalJobProperty/config.jelly new file mode 100644 index 000000000..07345d446 --- /dev/null +++ b/job/src/main/resources/org/jenkinsci/plugins/workflow/job/properties/OptionalJobProperty/config.jelly @@ -0,0 +1,33 @@ + + + + + + + + + + + From 78305b6495ad764d0618c11eb66204a37849d89a Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 26 Oct 2015 19:45:04 -0400 Subject: [PATCH 07/12] Finally found a way to configure a List. --- .../workflow/multibranch/JobPropertyStep.java | 51 +++++++++++++++++++ .../multibranch/JobPropertyStep/config.jelly | 6 +-- .../multibranch/JobPropertyStepTest.java | 29 ++++++++++- 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java index a89779bb8..0a8dc2bb6 100644 --- a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java +++ b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java @@ -27,16 +27,30 @@ import hudson.AbortException; import hudson.BulkChange; import hudson.Extension; +import hudson.ExtensionList; +import hudson.model.Descriptor; import hudson.model.Job; import hudson.model.JobProperty; +import hudson.model.JobPropertyDescriptor; import hudson.model.Run; +import java.util.ArrayList; +import java.util.Collection; import java.util.List; +import java.util.Map; +import java.util.Set; import javax.inject.Inject; +import jenkins.model.Jenkins; +import net.sf.json.JSONObject; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl; import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl; import org.jenkinsci.plugins.workflow.steps.AbstractSynchronousStepExecution; +import org.jenkinsci.plugins.workflow.steps.Step; import org.jenkinsci.plugins.workflow.steps.StepContextParameter; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.DoNotUse; import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.StaplerRequest; /** * Resets the properties of the current job. @@ -102,6 +116,43 @@ public DescriptorImpl() { return "Set job properties"; } + @Override public Step newInstance(StaplerRequest req, JSONObject formData) throws FormException { + // A modified version of RequestImpl.TypePair.convertJSON. + // Works around the fact that Stapler does not call back into Descriptor.newInstance for nested objects. + List properties = new ArrayList(); + ClassLoader cl = req.getStapler().getWebApp().getClassLoader(); + @SuppressWarnings("unchecked") Set> entrySet = formData.getJSONObject("properties").entrySet(); + for (Map.Entry e : entrySet) { + if (e.getValue() instanceof JSONObject) { + String className = e.getKey().replace('-', '.'); // decode JSON-safe class name escaping + Class itemType; + try { + itemType = cl.loadClass(className).asSubclass(JobProperty.class); + } catch (ClassNotFoundException x) { + throw new FormException(x, "properties"); + } + JobPropertyDescriptor d = (JobPropertyDescriptor) Jenkins.getActiveInstance().getDescriptorOrDie(itemType); + JSONObject more = (JSONObject) e.getValue(); + JobProperty property = d.newInstance(req, more); + if (property != null) { + properties.add(property); + } + } + } + return new JobPropertyStep(properties); + } + + @Restricted(DoNotUse.class) // f:repeatableHeteroProperty + public Collection> getPropertyDescriptors() { + List> result = new ArrayList>(); + for (JobPropertyDescriptor p : ExtensionList.lookup(JobPropertyDescriptor.class)) { + if (p.isApplicable(WorkflowJob.class)) { + result.add(p); + } + } + return result; + } + } } diff --git a/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly b/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly index 252780b9f..50c0ce03d 100644 --- a/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly +++ b/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly @@ -25,9 +25,5 @@ THE SOFTWARE. - - - - - + diff --git a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java index b88f1d53c..bcd250ca0 100644 --- a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java +++ b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java @@ -29,15 +29,18 @@ import hudson.model.ParametersAction; import hudson.model.ParametersDefinitionProperty; import hudson.model.StringParameterValue; +import hudson.tasks.LogRotator; import java.util.Collections; import java.util.List; import jenkins.branch.BranchProperty; import jenkins.branch.BranchSource; import jenkins.branch.DefaultBranchPropertyStrategy; +import jenkins.model.BuildDiscarder; import jenkins.plugins.git.GitSCMSource; import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; +import org.jenkinsci.plugins.workflow.job.properties.BuildDiscarderProperty; import org.jenkinsci.plugins.workflow.steps.StepConfigTester; import org.jenkinsci.plugins.workflow.steps.scm.GitSampleRepoRule; import org.junit.Test; @@ -48,6 +51,7 @@ import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import static org.jenkinsci.plugins.workflow.multibranch.WorkflowMultiBranchProjectTest.scheduleAndFindBranchProject; +import org.junit.Ignore; public class JobPropertyStepTest { @@ -55,11 +59,11 @@ public class JobPropertyStepTest { @Rule public JenkinsRule r = new JenkinsRule(); @Rule public GitSampleRepoRule sampleRepo = new GitSampleRepoRule(); + @Ignore("TODO fails with TypeError: Cannot call method \"hasClassName\" of undefined (http://localhost:35782/jenkins/adjuncts/cb384db9/lib/form/hetero-list/hetero-list.js#16)") @SuppressWarnings("rawtypes") @Issue("JENKINS-30519") - @Test public void configRoundTrip() throws Exception { + @Test public void configRoundTripParameters() throws Exception { StepConfigTester tester = new StepConfigTester(r); - // TODO fails (returns null) assertEquals(Collections.emptyList(), tester.configRoundTrip(new JobPropertyStep(Collections.emptyList())).getProperties()); List properties = tester.configRoundTrip(new JobPropertyStep(Collections.singletonList(new ParametersDefinitionProperty(new BooleanParameterDefinition("flag", true, null))))).getProperties(); assertEquals(1, properties.size()); @@ -73,6 +77,27 @@ public class JobPropertyStepTest { // TODO JENKINS-29711 means it seems to omit the required () } + @Ignore("TODO fails with TypeError: Cannot call method \"hasClassName\" of undefined (http://localhost:35782/jenkins/adjuncts/cb384db9/lib/form/hetero-list/hetero-list.js#16)") + @SuppressWarnings("rawtypes") + @Issue("JENKINS-30519") + @Test public void configRoundTripBuildDiscarder() throws Exception { + StepConfigTester tester = new StepConfigTester(r); + assertEquals(Collections.emptyList(), tester.configRoundTrip(new JobPropertyStep(Collections.emptyList())).getProperties()); + List properties = tester.configRoundTrip(new JobPropertyStep(Collections.singletonList(new BuildDiscarderProperty(new LogRotator(1, 2, -1, 3))))).getProperties(); + assertEquals(1, properties.size()); + assertEquals(BuildDiscarderProperty.class, properties.get(0)); + BuildDiscarderProperty bdp = (BuildDiscarderProperty) properties.get(0); + BuildDiscarder strategy = bdp.getStrategy(); + assertNotNull(strategy); + assertEquals(LogRotator.class, strategy.getClass()); + LogRotator lr = (LogRotator) strategy; + assertEquals(1, lr.getDaysToKeep()); + assertEquals(2, lr.getNumToKeep()); + assertEquals(-1, lr.getArtifactDaysToKeep()); + assertEquals(3, lr.getArtifactNumToKeep()); + // TODO JENKINS-29711 means it seems to omit the required () + } + @Issue("JENKINS-30206") @Test public void useParameter() throws Exception { sampleRepo.init(); From b3a8824809b842c9a14fbedd448e67780c7f29da Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 27 Oct 2015 10:35:02 -0400 Subject: [PATCH 08/12] f:descriptorList does not actually accept a plain old List. --- .../plugins/workflow/multibranch/JobPropertyStep.java | 8 ++++++-- .../workflow/multibranch/JobPropertyStep/config.jelly | 2 +- .../workflow/multibranch/JobPropertyStepTest.java | 11 +++++------ 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java index 0a8dc2bb6..4024df6e9 100644 --- a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java +++ b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java @@ -68,6 +68,10 @@ public List getProperties() { return properties; } + public Map getPropertiesMap() { + return Descriptor.toMap(properties); + } + public static class Execution extends AbstractSynchronousStepExecution { @Inject transient JobPropertyStep step; @@ -121,7 +125,7 @@ public DescriptorImpl() { // Works around the fact that Stapler does not call back into Descriptor.newInstance for nested objects. List properties = new ArrayList(); ClassLoader cl = req.getStapler().getWebApp().getClassLoader(); - @SuppressWarnings("unchecked") Set> entrySet = formData.getJSONObject("properties").entrySet(); + @SuppressWarnings("unchecked") Set> entrySet = formData.getJSONObject("propertiesMap").entrySet(); for (Map.Entry e : entrySet) { if (e.getValue() instanceof JSONObject) { String className = e.getKey().replace('-', '.'); // decode JSON-safe class name escaping @@ -129,7 +133,7 @@ public DescriptorImpl() { try { itemType = cl.loadClass(className).asSubclass(JobProperty.class); } catch (ClassNotFoundException x) { - throw new FormException(x, "properties"); + throw new FormException(x, "propertiesMap"); } JobPropertyDescriptor d = (JobPropertyDescriptor) Jenkins.getActiveInstance().getDescriptorOrDie(itemType); JSONObject more = (JSONObject) e.getValue(); diff --git a/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly b/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly index 50c0ce03d..b669eeedf 100644 --- a/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly +++ b/multibranch/src/main/resources/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep/config.jelly @@ -25,5 +25,5 @@ THE SOFTWARE. - + diff --git a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java index bcd250ca0..253956d9d 100644 --- a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java +++ b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java @@ -59,7 +59,7 @@ public class JobPropertyStepTest { @Rule public JenkinsRule r = new JenkinsRule(); @Rule public GitSampleRepoRule sampleRepo = new GitSampleRepoRule(); - @Ignore("TODO fails with TypeError: Cannot call method \"hasClassName\" of undefined (http://localhost:35782/jenkins/adjuncts/cb384db9/lib/form/hetero-list/hetero-list.js#16)") + @Ignore("TODO fails with a JS TypeError, which goes away on new cores, maybe due to new HtmlUnit (1.627+); and needs patch to RateLimitBranchProperty") @SuppressWarnings("rawtypes") @Issue("JENKINS-30519") @Test public void configRoundTripParameters() throws Exception { @@ -67,17 +67,17 @@ public class JobPropertyStepTest { assertEquals(Collections.emptyList(), tester.configRoundTrip(new JobPropertyStep(Collections.emptyList())).getProperties()); List properties = tester.configRoundTrip(new JobPropertyStep(Collections.singletonList(new ParametersDefinitionProperty(new BooleanParameterDefinition("flag", true, null))))).getProperties(); assertEquals(1, properties.size()); - assertEquals(ParametersDefinitionProperty.class, properties.get(0)); + assertEquals(ParametersDefinitionProperty.class, properties.get(0).getClass()); ParametersDefinitionProperty pdp = (ParametersDefinitionProperty) properties.get(0); assertEquals(1, pdp.getParameterDefinitions().size()); assertEquals(BooleanParameterDefinition.class, pdp.getParameterDefinitions().get(0).getClass()); BooleanParameterDefinition bpd = (BooleanParameterDefinition) pdp.getParameterDefinitions().get(0); assertEquals("flag", bpd.getName()); assertTrue(bpd.isDefaultValue()); - // TODO JENKINS-29711 means it seems to omit the required () + // TODO JENKINS-29711 means it seems to omit the required () but we are not currently testing the Snippetizer output anyway } - @Ignore("TODO fails with TypeError: Cannot call method \"hasClassName\" of undefined (http://localhost:35782/jenkins/adjuncts/cb384db9/lib/form/hetero-list/hetero-list.js#16)") + @Ignore("TODO as above") @SuppressWarnings("rawtypes") @Issue("JENKINS-30519") @Test public void configRoundTripBuildDiscarder() throws Exception { @@ -85,7 +85,7 @@ public class JobPropertyStepTest { assertEquals(Collections.emptyList(), tester.configRoundTrip(new JobPropertyStep(Collections.emptyList())).getProperties()); List properties = tester.configRoundTrip(new JobPropertyStep(Collections.singletonList(new BuildDiscarderProperty(new LogRotator(1, 2, -1, 3))))).getProperties(); assertEquals(1, properties.size()); - assertEquals(BuildDiscarderProperty.class, properties.get(0)); + assertEquals(BuildDiscarderProperty.class, properties.get(0).getClass()); BuildDiscarderProperty bdp = (BuildDiscarderProperty) properties.get(0); BuildDiscarder strategy = bdp.getStrategy(); assertNotNull(strategy); @@ -95,7 +95,6 @@ public class JobPropertyStepTest { assertEquals(2, lr.getNumToKeep()); assertEquals(-1, lr.getArtifactDaysToKeep()); assertEquals(3, lr.getArtifactNumToKeep()); - // TODO JENKINS-29711 means it seems to omit the required () } @Issue("JENKINS-30206") From 37cf6d755583814bf6b539cde7af83883ff3b881 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 27 Oct 2015 10:45:52 -0400 Subject: [PATCH 09/12] Linking to https://github.com/jenkinsci/branch-api-plugin/pull/9. --- .../plugins/workflow/multibranch/JobPropertyStepTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java index 253956d9d..6606a8cfc 100644 --- a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java +++ b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java @@ -59,7 +59,7 @@ public class JobPropertyStepTest { @Rule public JenkinsRule r = new JenkinsRule(); @Rule public GitSampleRepoRule sampleRepo = new GitSampleRepoRule(); - @Ignore("TODO fails with a JS TypeError, which goes away on new cores, maybe due to new HtmlUnit (1.627+); and needs patch to RateLimitBranchProperty") + @Ignore("TODO fails with a JS TypeError, which goes away on new cores, maybe due to new HtmlUnit (1.627+); and needs https://github.com/jenkinsci/branch-api-plugin/pull/9") @SuppressWarnings("rawtypes") @Issue("JENKINS-30519") @Test public void configRoundTripParameters() throws Exception { From 7e08e342b51f54ece65b072ed26503046f780085 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 27 Oct 2015 11:56:38 -0400 Subject: [PATCH 10/12] Hiding RateLimitBranchProperty & BuildRetentionBranchProperty. --- .../workflow/multibranch/JobPropertyStep.java | 15 +++++++++++++++ .../WorkflowMultiBranchProjectTest.java | 13 ++++--------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java index 4024df6e9..ba06ef6e4 100644 --- a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java +++ b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java @@ -29,6 +29,7 @@ import hudson.Extension; import hudson.ExtensionList; import hudson.model.Descriptor; +import hudson.model.DescriptorVisibilityFilter; import hudson.model.Job; import hudson.model.JobProperty; import hudson.model.JobPropertyDescriptor; @@ -39,6 +40,8 @@ import java.util.Map; import java.util.Set; import javax.inject.Inject; +import jenkins.branch.BuildRetentionBranchProperty; +import jenkins.branch.RateLimitBranchProperty; import jenkins.model.Jenkins; import net.sf.json.JSONObject; import org.jenkinsci.plugins.workflow.job.WorkflowJob; @@ -159,4 +162,16 @@ public Collection> getPropertyDescriptors() { } + @Extension public static class HideSuperfluousBranchProperties extends DescriptorVisibilityFilter { + + @Override public boolean filter(Object context, Descriptor descriptor) { + if (context instanceof WorkflowMultiBranchProject && (descriptor.clazz == RateLimitBranchProperty.class || descriptor.clazz == BuildRetentionBranchProperty.class)) { + // These are both adequately handled by declarative job properties. + return false; + } + return true; + } + + } + } diff --git a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java index 1e58d1840..8f3c624eb 100644 --- a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java +++ b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectTest.java @@ -25,16 +25,14 @@ package org.jenkinsci.plugins.workflow.multibranch; import hudson.model.DescriptorVisibilityFilter; -import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.Set; import javax.annotation.Nonnull; import jenkins.branch.BranchProperty; import jenkins.branch.BranchPropertyDescriptor; import jenkins.branch.BranchSource; -import jenkins.branch.BuildRetentionBranchProperty; import jenkins.branch.DefaultBranchPropertyStrategy; -import jenkins.branch.RateLimitBranchProperty; import jenkins.plugins.git.GitSCMSource; import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; import org.jenkinsci.plugins.workflow.job.WorkflowJob; @@ -105,12 +103,9 @@ public class WorkflowMultiBranchProjectTest { for (BranchPropertyDescriptor d : DescriptorVisibilityFilter.apply(p, BranchPropertyDescriptor.all())) { clazzes.add(d.clazz); } - @SuppressWarnings("unchecked") - Set> expected = new HashSet>(Arrays.asList( - RateLimitBranchProperty.class, - BuildRetentionBranchProperty.class - /* UntrustedBranchProperty should not be here! */)); - assertEquals(expected, clazzes); + // RateLimitBranchProperty & BuildRetentionBranchProperty hidden by JobPropertyStep.HideSuperfluousBranchProperties. + // UntrustedBranchProperty hidden because it applies only to Project. + assertEquals(Collections.>emptySet(), clazzes); } } From 295ccd59b5abc934f14e69c96e0392c685f9adad Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 27 Oct 2015 13:15:49 -0400 Subject: [PATCH 11/12] useBuildDiscarder --- .../multibranch/JobPropertyStepTest.java | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java index 6606a8cfc..1274605f8 100644 --- a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java +++ b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStepTest.java @@ -53,6 +53,7 @@ import static org.jenkinsci.plugins.workflow.multibranch.WorkflowMultiBranchProjectTest.scheduleAndFindBranchProject; import org.junit.Ignore; +@Issue("JENKINS-30519") public class JobPropertyStepTest { @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher(); @@ -61,7 +62,6 @@ public class JobPropertyStepTest { @Ignore("TODO fails with a JS TypeError, which goes away on new cores, maybe due to new HtmlUnit (1.627+); and needs https://github.com/jenkinsci/branch-api-plugin/pull/9") @SuppressWarnings("rawtypes") - @Issue("JENKINS-30519") @Test public void configRoundTripParameters() throws Exception { StepConfigTester tester = new StepConfigTester(r); assertEquals(Collections.emptyList(), tester.configRoundTrip(new JobPropertyStep(Collections.emptyList())).getProperties()); @@ -79,7 +79,6 @@ public class JobPropertyStepTest { @Ignore("TODO as above") @SuppressWarnings("rawtypes") - @Issue("JENKINS-30519") @Test public void configRoundTripBuildDiscarder() throws Exception { StepConfigTester tester = new StepConfigTester(r); assertEquals(Collections.emptyList(), tester.configRoundTrip(new JobPropertyStep(Collections.emptyList())).getProperties()); @@ -120,4 +119,25 @@ public class JobPropertyStepTest { r.assertLogContains("received special value", b2); } + @SuppressWarnings("deprecation") // RunList.size + @Test public void useBuildDiscarder() throws Exception { + sampleRepo.init(); + sampleRepo.write("Jenkinsfile", "properties([[$class: 'BuildDiscarderProperty', strategy: [$class: 'LogRotator', numToKeepStr: '1']]])"); + sampleRepo.git("add", "Jenkinsfile"); + sampleRepo.git("commit", "--all", "--message=flow"); + WorkflowMultiBranchProject mp = r.jenkins.createProject(WorkflowMultiBranchProject.class, "p"); + mp.getSourcesList().add(new BranchSource(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", false), new DefaultBranchPropertyStrategy(new BranchProperty[0]))); + WorkflowJob p = scheduleAndFindBranchProject(mp, "master"); + assertEquals(1, mp.getItems().size()); + r.waitUntilNoActivity(); // #1 built automatically + assertEquals(1, p.getBuilds().size()); + r.assertBuildStatusSuccess(p.scheduleBuild2(0)); // #2 + assertEquals(1, p.getBuilds().size()); + r.assertBuildStatusSuccess(p.scheduleBuild2(0)); // #3 + assertEquals(1, p.getBuilds().size()); + WorkflowRun b3 = p.getLastBuild(); + assertEquals(3, b3.getNumber()); + assertNull(b3.getPreviousBuild()); + } + } From c96a666d651847d5b2d063285e718516e3ba460a Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 27 Oct 2015 13:42:38 -0400 Subject: [PATCH 12/12] Need to pick up https://github.com/jenkinsci/branch-api-plugin/pull/9. --- multibranch/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multibranch/pom.xml b/multibranch/pom.xml index 3385c0f13..fbd622d4f 100644 --- a/multibranch/pom.xml +++ b/multibranch/pom.xml @@ -58,7 +58,7 @@ THE SOFTWARE. org.jenkins-ci.plugins branch-api - 0.2-beta-5 + 0.2-beta-6-SNAPSHOT ${project.groupId}