Make fields for BranchFilter and WebHookTriggerHandler transient and initialize them with readResolve in the GitLabPushTrigger

Author: coder-hugo

src/main/java/com/dabsquared/gitlabjenkins/GitLabPushTrigger.java

@@ -11,19 +11,16 @@
 import com.dabsquared.gitlabjenkins.trigger.filter.BranchFilter;
 import com.dabsquared.gitlabjenkins.trigger.filter.BranchFilterFactory;
 import com.dabsquared.gitlabjenkins.trigger.filter.BranchFilterType;
-import com.dabsquared.gitlabjenkins.trigger.handler.WebHookTriggerConfig;
 import com.dabsquared.gitlabjenkins.trigger.handler.merge.MergeRequestHookTriggerHandler;
-import com.dabsquared.gitlabjenkins.trigger.handler.merge.MergeRequestHookTriggerHandlerFactory;
 import com.dabsquared.gitlabjenkins.trigger.handler.push.PushHookTriggerHandler;
-import com.dabsquared.gitlabjenkins.trigger.handler.push.PushHookTriggerHandlerFactory;
 import com.dabsquared.gitlabjenkins.webhook.GitLabWebHook;
 import hudson.Extension;
 import hudson.Util;
 import hudson.init.InitMilestone;
 import hudson.init.Initializer;
+import hudson.model.AbstractProject;
 import hudson.model.AutoCompletionCandidates;
 import hudson.model.Item;
-import hudson.model.AbstractProject;
 import hudson.model.Job;
 import hudson.triggers.Trigger;
 import hudson.triggers.TriggerDescriptor;
@@ -35,7 +32,6 @@
 import jenkins.model.ParameterizedJobMixIn;
 import jenkins.triggers.SCMTriggerItem.SCMTriggerItems;
 import net.sf.json.JSONObject;
-import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.Ancestor;
 import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
@@ -44,32 +40,36 @@
 import org.kohsuke.stapler.StaplerRequest;
 
 import java.io.IOException;
+import java.io.ObjectStreamException;
 
 import static com.dabsquared.gitlabjenkins.trigger.filter.BranchFilterConfig.BranchFilterConfigBuilder.branchFilterConfig;
+import static com.dabsquared.gitlabjenkins.trigger.handler.merge.MergeRequestHookTriggerHandlerFactory.newMergeRequestHookTriggerHandler;
+import static com.dabsquared.gitlabjenkins.trigger.handler.push.PushHookTriggerHandlerFactory.newPushHookTriggerHandler;
 
 
 /**
  * Triggers a build when we receive a GitLab WebHook.
  *
  * @author Daniel Brooks
  */
-public class GitLabPushTrigger extends Trigger<Job<?, ?>> implements WebHookTriggerConfig {
-	private transient boolean triggerOnPush = true;
-    private transient boolean triggerOnMergeRequest = true;
+public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
+	private boolean triggerOnPush = true;
+    private boolean triggerOnMergeRequest = true;
     private final TriggerOpenMergeRequest triggerOpenMergeRequestOnPush;
     private boolean ciSkip = true;
     private boolean setBuildDescription = true;
     private boolean addNoteOnMergeRequest = true;
     private boolean addCiMessage = false;
     private boolean addVoteOnMergeRequest = true;
     private transient boolean allowAllBranches = false;
-    private transient String branchFilterName;
-    private transient String includeBranchesSpec;
-    private transient String excludeBranchesSpec;
-    private transient String targetBranchRegex;
-    private BranchFilter branchFilter;
-    private PushHookTriggerHandler pushHookTriggerHandler;
-    private MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler;
+    private transient BranchFilterType branchFilterName;
+    private BranchFilterType branchFilterType;
+    private String includeBranchesSpec;
+    private String excludeBranchesSpec;
+    private String targetBranchRegex;
+    private transient BranchFilter branchFilter;
+    private transient PushHookTriggerHandler pushHookTriggerHandler;
+    private transient MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler;
     private boolean acceptMergeRequestOnSuccess = false;
 
 
@@ -78,28 +78,30 @@ public GitLabPushTrigger(boolean triggerOnPush, boolean triggerOnMergeRequest, T
                              boolean ciSkip, boolean setBuildDescription, boolean addNoteOnMergeRequest, boolean addCiMessage,
                              boolean addVoteOnMergeRequest, boolean acceptMergeRequestOnSuccess, BranchFilterType branchFilterType,
                              String includeBranchesSpec, String excludeBranchesSpec, String targetBranchRegex) {
-        mergeRequestHookTriggerHandler = MergeRequestHookTriggerHandlerFactory.newMergeRequestHookTriggerHandler(triggerOnMergeRequest, triggerOpenMergeRequestOnPush);
-        pushHookTriggerHandler = PushHookTriggerHandlerFactory.newPushHookTriggerHandler(triggerOnPush, triggerOpenMergeRequestOnPush);
+        this.triggerOnPush = triggerOnPush;
+        this.triggerOnMergeRequest = triggerOnMergeRequest;
         this.triggerOpenMergeRequestOnPush = triggerOpenMergeRequestOnPush;
         this.ciSkip = ciSkip;
         this.setBuildDescription = setBuildDescription;
         this.addNoteOnMergeRequest = addNoteOnMergeRequest;
         this.addCiMessage = addCiMessage;
         this.addVoteOnMergeRequest = addVoteOnMergeRequest;
-        this.branchFilter = BranchFilterFactory.newBranchFilter(branchFilterConfig()
-                .withIncludeBranchesSpec(includeBranchesSpec)
-                .withExcludeBranchesSpec(excludeBranchesSpec)
-                .withTargetBranchRegex(targetBranchRegex)
-                .build(branchFilterType));
+        this.branchFilterType = branchFilterType;
+        this.includeBranchesSpec = includeBranchesSpec;
+        this.excludeBranchesSpec = excludeBranchesSpec;
+        this.targetBranchRegex = targetBranchRegex;
         this.acceptMergeRequestOnSuccess = acceptMergeRequestOnSuccess;
+
+        initializeTriggerHandler();
+        initializeBranchFilter();
     }
 
     public boolean getTriggerOnPush() {
-    	return pushHookTriggerHandler.isEnabled();
+    	return triggerOnPush;
     }
 
     public boolean getTriggerOnMergeRequest() {
-    	return mergeRequestHookTriggerHandler.isEnabled();
+    	return triggerOnMergeRequest;
     }
 
     public TriggerOpenMergeRequest getTriggerOpenMergeRequestOnPush() {
@@ -122,14 +124,24 @@ public boolean getAcceptMergeRequestOnSuccess() {
         return acceptMergeRequestOnSuccess;
     }
 
-    @Override
     public boolean getCiSkip() {
         return ciSkip;
     }
 
-    @Override
-    public BranchFilter getBranchFilter() {
-        return branchFilter;
+    public BranchFilterType getBranchFilterType() {
+        return branchFilterType;
+    }
+
+    public String getIncludeBranchesSpec() {
+        return includeBranchesSpec;
+    }
+
+    public String getExcludeBranchesSpec() {
+        return excludeBranchesSpec;
+    }
+
+    public String getTargetBranchRegex() {
+        return targetBranchRegex;
     }
 
     // executes when the Trigger receives a push request
@@ -142,6 +154,26 @@ public void onPost(final MergeRequestHook hook) {
         mergeRequestHookTriggerHandler.handle(job, hook, ciSkip, branchFilter);
     }
 
+    private void initializeTriggerHandler() {
+        mergeRequestHookTriggerHandler = newMergeRequestHookTriggerHandler(triggerOnMergeRequest, triggerOpenMergeRequestOnPush);
+        pushHookTriggerHandler = newPushHookTriggerHandler(triggerOnPush, triggerOpenMergeRequestOnPush);
+    }
+
+    private void initializeBranchFilter() {
+        branchFilter = BranchFilterFactory.newBranchFilter(branchFilterConfig()
+                .withIncludeBranchesSpec(includeBranchesSpec)
+                .withExcludeBranchesSpec(excludeBranchesSpec)
+                .withTargetBranchRegex(targetBranchRegex)
+                .build(branchFilterType));
+    }
+
+    @Override
+    protected Object readResolve() throws ObjectStreamException {
+        initializeTriggerHandler();
+        initializeBranchFilter();
+        return super.readResolve();
+    }
+
     @Extension
     public static class DescriptorImpl extends TriggerDescriptor {
 
@@ -247,20 +279,8 @@ public static void migrateJobs() throws IOException {
                     if (trigger.addCiMessage) {
                         project.getPublishersList().add(new GitLabCommitStatusPublisher());
                     }
-                    if (trigger.branchFilter == null) {
-                        String name = StringUtils.isNotEmpty(trigger.branchFilterName) ? trigger.branchFilterName : "All";
-                        trigger.branchFilter = BranchFilterFactory.newBranchFilter(branchFilterConfig()
-                                .withIncludeBranchesSpec(trigger.includeBranchesSpec)
-                                .withExcludeBranchesSpec(trigger.excludeBranchesSpec)
-                                .withTargetBranchRegex(trigger.targetBranchRegex)
-                                .build(BranchFilterType.valueOf(name)));
-                    }
-                    if (trigger.pushHookTriggerHandler == null) {
-                        trigger.pushHookTriggerHandler = PushHookTriggerHandlerFactory.newPushHookTriggerHandler(trigger.triggerOnPush, trigger.triggerOpenMergeRequestOnPush);
-                    }
-                    if (trigger.mergeRequestHookTriggerHandler == null) {
-                        trigger.mergeRequestHookTriggerHandler =
-                                MergeRequestHookTriggerHandlerFactory.newMergeRequestHookTriggerHandler(trigger.triggerOnMergeRequest, trigger.triggerOpenMergeRequestOnPush);
+                    if (trigger.branchFilterType == null) {
+                        trigger.branchFilterType = trigger.branchFilterName;
                     }
                     project.addProperty(new GitLabConnectionProperty(defaultConnectionName));
                     project.save();

src/main/java/com/dabsquared/gitlabjenkins/trigger/filter/AllBranchesFilter.java

@@ -8,9 +8,4 @@ class AllBranchesFilter implements BranchFilter {
     public boolean isBranchAllowed(String branchName) {
         return true;
     }
-
-    @Override
-    public BranchFilterConfig getConfig() {
-        return BranchFilterConfig.BranchFilterConfigBuilder.branchFilterConfig().build(BranchFilterType.All);
-    }
 }

src/main/java/com/dabsquared/gitlabjenkins/trigger/filter/BranchFilter.java

@@ -6,6 +6,4 @@
 public interface BranchFilter {
 
     boolean isBranchAllowed(String branchName);
-
-    BranchFilterConfig getConfig();
 }

src/main/java/com/dabsquared/gitlabjenkins/trigger/filter/BranchFilterConfig.java

@@ -21,15 +21,15 @@ public BranchFilterType getType() {
         return type;
     }
 
-    public String getIncludeBranchesSpec() {
+    String getIncludeBranchesSpec() {
         return includeBranchesSpec;
     }
 
-    public String getExcludeBranchesSpec() {
+    String getExcludeBranchesSpec() {
         return excludeBranchesSpec;
     }
 
-    public String getTargetBranchRegex() {
+    String getTargetBranchRegex() {
         return targetBranchRegex;
     }
 

src/main/java/com/dabsquared/gitlabjenkins/trigger/filter/NameBasedFilter.java

@@ -1,14 +1,11 @@
 package com.dabsquared.gitlabjenkins.trigger.filter;
 
-import com.google.common.base.Joiner;
 import com.google.common.base.Splitter;
 import org.springframework.util.AntPathMatcher;
 
 import java.util.ArrayList;
 import java.util.List;
 
-import static com.dabsquared.gitlabjenkins.trigger.filter.BranchFilterConfig.BranchFilterConfigBuilder.branchFilterConfig;
-
 /**
  * @author Robin Müller
  */
@@ -27,14 +24,6 @@ public boolean isBranchAllowed(String branchName) {
         return hasNoBranchSpecs() || (isBranchNotExcluded(branchName) && isBranchIncluded(branchName));
     }
 
-    @Override
-    public BranchFilterConfig getConfig() {
-        return branchFilterConfig()
-                .withIncludeBranchesSpec(Joiner.on(", ").join(includedBranches))
-                .withExcludeBranchesSpec(Joiner.on(", ").join(excludedBranches))
-                .build(BranchFilterType.NameBasedFilter);
-    }
-
     private boolean hasNoBranchSpecs() {
         return includedBranches.isEmpty() && excludedBranches.isEmpty();
     }
@@ -60,7 +49,7 @@ private boolean isBranchIncluded(String branchName) {
     }
 
     private List<String> convert(String commaSeparatedString) {
-        ArrayList<String> result = new ArrayList<String>();
+        ArrayList<String> result = new ArrayList<>();
         for (String s : Splitter.on(',').omitEmptyStrings().trimResults().split(commaSeparatedString)) {
             result.add(s);
         }

src/main/java/com/dabsquared/gitlabjenkins/trigger/filter/RegexBasedFilter.java

@@ -2,8 +2,6 @@
 
 import org.apache.commons.lang.StringUtils;
 
-import static com.dabsquared.gitlabjenkins.trigger.filter.BranchFilterConfig.BranchFilterConfigBuilder.branchFilterConfig;
-
 /**
  * @author Robin Müller
  */
@@ -19,10 +17,4 @@ public RegexBasedFilter(String regex) {
     public boolean isBranchAllowed(String branchName) {
         return StringUtils.isEmpty(branchName) || StringUtils.isEmpty(regex) || branchName.matches(regex);
     }
-
-    @Override
-    public BranchFilterConfig getConfig() {
-        return branchFilterConfig().withTargetBranchRegex(regex).build(BranchFilterType.RegexBasedFilter);
-    }
-
 }

src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/WebHookTriggerConfig.java

@@ -10,6 +10,4 @@
 public interface WebHookTriggerHandler<H extends WebHook> {
 
     void handle(Job<?, ?> job, H hook, boolean ciSkip, BranchFilter branchFilter);
-
-    boolean isEnabled();
 }

src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/WebHookTriggerHandler.java

@@ -40,11 +40,6 @@ public void handle(Job<?, ?> job, MergeRequestHook hook, boolean ciSkip, BranchF
         }
     }
 
-    @Override
-    public boolean isEnabled() {
-        return true;
-    }
-
     @Override
     protected boolean isCiSkip(MergeRequestHook hook) {
         return hook.getObjectAttributes() != null

src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImpl.java

@@ -12,9 +12,4 @@ class NopMergeRequestHookTriggerHandler implements MergeRequestHookTriggerHandle
     public void handle(Job<?, ?> job, MergeRequestHook hook, boolean ciSkip, BranchFilter branchFilter) {
         // nothing to do
     }
-
-    @Override
-    public boolean isEnabled() {
-        return false;
-    }
 }