Support @DataBoundSetter for triggers and publishers.

Currently jenkins jobs defined using the generated DSL for the `gitlab-plugin` are required to supply all possible values for any defined triggers or publishers due to the use of the `@DataBoundConstructor` annotation. This leads to the problem described in https://github.com/jenkinsci/gitlab-plugin/issues/613 where any modifications to the trigger or publisher constructor signatures cause existing job definitions to fail when run against a new version of the plugin.

Fix this by adding support for a setter driven approach to configure triggers and publishers with the `@DataBoundSetter` annotation. This allows all unimportant configuration to be ignored when defining jobs and prevents new functionality from breaking existing jobs when upgrading.
This commit is contained in:
g-dx 2017-09-01 23:52:10 +01:00
parent e9f0d27041
commit 03041b0899
3 changed files with 216 additions and 11 deletions

View File

@ -48,6 +48,7 @@ import org.jenkinsci.Symbol;
import org.kohsuke.stapler.Ancestor;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.Stapler;
import org.kohsuke.stapler.StaplerRequest;
@ -76,9 +77,9 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
private boolean triggerOnMergeRequest = true;
private boolean triggerOnAcceptedMergeRequest = false;
private boolean triggerOnClosedMergeRequest = false;
private final TriggerOpenMergeRequest triggerOpenMergeRequestOnPush;
private TriggerOpenMergeRequest triggerOpenMergeRequestOnPush;
private boolean triggerOnNoteRequest = true;
private final String noteRegex;
private String noteRegex = "";
private boolean ciSkip = true;
private boolean skipWorkInProgressMergeRequest;
private boolean setBuildDescription = true;
@ -91,7 +92,7 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
private String includeBranchesSpec;
private String excludeBranchesSpec;
private String targetBranchRegex;
private final MergeRequestLabelFilterConfig mergeRequestLabelFilterConfig;
private MergeRequestLabelFilterConfig mergeRequestLabelFilterConfig;
private volatile Secret secretToken;
private transient BranchFilter branchFilter;
@ -101,8 +102,10 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
private transient boolean acceptMergeRequestOnSuccess;
private transient MergeRequestLabelFilter mergeRequestLabelFilter;
@DataBoundConstructor
/**
* @deprecated use {@link #GitLabPushTrigger()} with setters to configure an instance of this class.
*/
@Deprecated
@GeneratePojoBuilder(intoPackage = "*.builder.generated", withFactoryMethod = "*")
public GitLabPushTrigger(boolean triggerOnPush, boolean triggerOnMergeRequest, boolean triggerOnAcceptedMergeRequest, boolean triggerOnClosedMergeRequest,
TriggerOpenMergeRequest triggerOpenMergeRequestOnPush, boolean triggerOnNoteRequest, String noteRegex,
@ -137,6 +140,9 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
initializeMergeRequestLabelFilter();
}
@DataBoundConstructor
public GitLabPushTrigger() { }
@Initializer(after = InitMilestone.JOB_LOADED)
public static void migrateJobs() throws IOException {
GitLabPushTrigger.DescriptorImpl oldConfig = Trigger.all().get(GitLabPushTrigger.DescriptorImpl.class);
@ -249,18 +255,150 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
return secretToken == null ? null : secretToken.getPlainText();
}
@DataBoundSetter
public void setTriggerOnPush(boolean triggerOnPush) {
this.triggerOnPush = triggerOnPush;
}
@DataBoundSetter
public void setTriggerOnMergeRequest(boolean triggerOnMergeRequest) {
this.triggerOnMergeRequest = triggerOnMergeRequest;
}
@DataBoundSetter
public void setTriggerOnAcceptedMergeRequest(boolean triggerOnAcceptedMergeRequest) {
this.triggerOnAcceptedMergeRequest = triggerOnAcceptedMergeRequest;
}
@DataBoundSetter
public void setTriggerOnClosedMergeRequest(boolean triggerOnClosedMergeRequest) {
this.triggerOnClosedMergeRequest = triggerOnClosedMergeRequest;
}
@DataBoundSetter
public void setTriggerOpenMergeRequestOnPush(TriggerOpenMergeRequest triggerOpenMergeRequestOnPush) {
this.triggerOpenMergeRequestOnPush = triggerOpenMergeRequestOnPush;
}
@DataBoundSetter
public void setTriggerOnNoteRequest(boolean triggerOnNoteRequest) {
this.triggerOnNoteRequest = triggerOnNoteRequest;
}
@DataBoundSetter
public void setNoteRegex(String noteRegex) {
this.noteRegex = noteRegex;
}
@DataBoundSetter
public void setCiSkip(boolean ciSkip) {
this.ciSkip = ciSkip;
}
@DataBoundSetter
public void setSkipWorkInProgressMergeRequest(boolean skipWorkInProgressMergeRequest) {
this.skipWorkInProgressMergeRequest = skipWorkInProgressMergeRequest;
}
@DataBoundSetter
public void setSetBuildDescription(boolean setBuildDescription) {
this.setBuildDescription = setBuildDescription;
}
@DataBoundSetter
public void setAddNoteOnMergeRequest(boolean addNoteOnMergeRequest) {
this.addNoteOnMergeRequest = addNoteOnMergeRequest;
}
@DataBoundSetter
public void setAddCiMessage(boolean addCiMessage) {
this.addCiMessage = addCiMessage;
}
@DataBoundSetter
public void setAddVoteOnMergeRequest(boolean addVoteOnMergeRequest) {
this.addVoteOnMergeRequest = addVoteOnMergeRequest;
}
@DataBoundSetter
public void setBranchFilterName(String branchFilterName) {
this.branchFilterName = branchFilterName;
}
@DataBoundSetter
public void setBranchFilterType(BranchFilterType branchFilterType) {
this.branchFilterType = branchFilterType;
}
@DataBoundSetter
public void setIncludeBranchesSpec(String includeBranchesSpec) {
this.includeBranchesSpec = includeBranchesSpec;
}
@DataBoundSetter
public void setExcludeBranchesSpec(String excludeBranchesSpec) {
this.excludeBranchesSpec = excludeBranchesSpec;
}
@DataBoundSetter
public void setTargetBranchRegex(String targetBranchRegex) {
this.targetBranchRegex = targetBranchRegex;
}
@DataBoundSetter
public void setMergeRequestLabelFilterConfig(MergeRequestLabelFilterConfig mergeRequestLabelFilterConfig) {
this.mergeRequestLabelFilterConfig = mergeRequestLabelFilterConfig;
}
@DataBoundSetter
public void setSecretToken(String secretToken) {
this.secretToken = Secret.fromString(secretToken);
}
@DataBoundSetter
public void setAcceptMergeRequestOnSuccess(boolean acceptMergeRequestOnSuccess) {
this.acceptMergeRequestOnSuccess = acceptMergeRequestOnSuccess;
}
// executes when the Trigger receives a push request
public void onPost(final PushHook hook) {
if (branchFilter == null) {
initializeBranchFilter();
}
if (mergeRequestLabelFilter == null) {
initializeMergeRequestLabelFilter();
}
if (pushHookTriggerHandler == null) {
initializeTriggerHandler();
}
pushHookTriggerHandler.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
}
// executes when the Trigger receives a merge request
public void onPost(final MergeRequestHook hook) {
if (branchFilter == null) {
initializeBranchFilter();
}
if (mergeRequestLabelFilter == null) {
initializeMergeRequestLabelFilter();
}
if (mergeRequestHookTriggerHandler == null) {
initializeTriggerHandler();
}
mergeRequestHookTriggerHandler.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
}
// executes when the Trigger receives a note request
public void onPost(final NoteHook hook) {
if (branchFilter == null) {
initializeBranchFilter();
}
if (mergeRequestLabelFilter == null) {
initializeMergeRequestLabelFilter();
}
if (noteHookTriggerHandler == null) {
initializeTriggerHandler();
}
noteHookTriggerHandler.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
}

View File

@ -11,6 +11,7 @@ import hudson.tasks.BuildStepDescriptor;
import hudson.tasks.Publisher;
import jenkins.model.Jenkins;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import javax.ws.rs.ProcessingException;
import javax.ws.rs.WebApplicationException;
@ -35,7 +36,10 @@ public class GitLabMessagePublisher extends MergeRequestNotifier {
private String abortNoteText;
private String unstableNoteText;
@DataBoundConstructor
/**
* @deprecated use {@link #GitLabMessagePublisher()} with setters to configure an instance of this class.
*/
@Deprecated
public GitLabMessagePublisher(boolean onlyForFailure, boolean replaceSuccessNote, boolean replaceFailureNote, boolean replaceAbortNote, boolean replaceUnstableNote,
String successNoteText, String failureNoteText, String abortNoteText, String unstableNoteText) {
this.onlyForFailure = onlyForFailure;
@ -49,6 +53,7 @@ public class GitLabMessagePublisher extends MergeRequestNotifier {
this.unstableNoteText = unstableNoteText;
}
@DataBoundConstructor
public GitLabMessagePublisher() { }
public boolean isOnlyForFailure() {
@ -87,6 +92,51 @@ public class GitLabMessagePublisher extends MergeRequestNotifier {
return this.unstableNoteText == null ? "" : this.unstableNoteText;
}
@DataBoundSetter
public void setOnlyForFailure(boolean onlyForFailure) {
this.onlyForFailure = onlyForFailure;
}
@DataBoundSetter
public void setReplaceSuccessNote(boolean replaceSuccessNote) {
this.replaceSuccessNote = replaceSuccessNote;
}
@DataBoundSetter
public void setReplaceFailureNote(boolean replaceFailureNote) {
this.replaceFailureNote = replaceFailureNote;
}
@DataBoundSetter
public void setReplaceAbortNote(boolean replaceAbortNote) {
this.replaceAbortNote = replaceAbortNote;
}
@DataBoundSetter
public void setReplaceUnstableNote(boolean replaceUnstableNote) {
this.replaceUnstableNote = replaceUnstableNote;
}
@DataBoundSetter
public void setSuccessNoteText(String successNoteText) {
this.successNoteText = successNoteText;
}
@DataBoundSetter
public void setFailureNoteText(String failureNoteText) {
this.failureNoteText = failureNoteText;
}
@DataBoundSetter
public void setAbortNoteText(String abortNoteText) {
this.abortNoteText = abortNoteText;
}
@DataBoundSetter
public void setUnstableNoteText(String unstableNoteText) {
this.unstableNoteText = unstableNoteText;
}
@Extension
public static class DescriptorImpl extends BuildStepDescriptor<Publisher> {

View File

@ -1,21 +1,28 @@
package com.dabsquared.gitlabjenkins.trigger.filter;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
/**
* @author Robin Müller
*/
public class MergeRequestLabelFilterConfig {
private final String include;
private final String exclude;
private String include;
private String exclude;
@DataBoundConstructor
/**
* @deprecated use {@link #MergeRequestLabelFilterConfig()} with setters to configure an instance of this class.
*/
@Deprecated
public MergeRequestLabelFilterConfig(String include, String exclude) {
this.include = include;
this.exclude = exclude;
}
@DataBoundConstructor
public MergeRequestLabelFilterConfig() { }
public String getInclude() {
return include;
}
@ -23,4 +30,14 @@ public class MergeRequestLabelFilterConfig {
public String getExclude() {
return exclude;
}
@DataBoundSetter
public void setInclude(String include) {
this.include = include;
}
@DataBoundSetter
public void setExclude(String exclude) {
this.exclude = exclude;
}
}