From d2c866c611fca7c39adc3d3841efcb29ee1cfdcb Mon Sep 17 00:00:00 2001 From: Mads Nielsen Date: Wed, 23 Nov 2016 08:24:58 +0100 Subject: [PATCH] Fixed various issue preventing job configuration from being saved. 1. You cannot store 'state' information about the current build in your build wrapper. So i've removed the 'StatusMessage' you've been using to set the build description. 2. I've fixed the loading, it was all down to wrong variable names. Remember, the construtor parameter name MUST match the value in the jelly form. This actually meant your values were saved correctly, but failed to load on refresh, since that happens through databinding. 3. Various cleanups... That should be good. --- .../AbstractSCMBridge.java | 24 ++--------- .../PretestedIntegrationBuildWrapper.java | 43 ++++--------------- .../PretestedIntegrationJobDslExtension.java | 1 + .../PretestedIntegrationPostCheckout.java | 7 +-- .../scm/git/GitBridge.java | 27 +++++------- .../scm/git/GitIntegrationStrategy.java | 1 + .../PretestedIntegrationAsGitPluginExt.java | 10 ++--- ...egrationAsGitPluginExtJobDslExtension.java | 2 +- .../scm/git/SquashCommitStrategy.java | 4 +- .../scm/git/GitBridge/config.jelly | 11 ++--- 10 files changed, 39 insertions(+), 91 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/pretestedintegration/AbstractSCMBridge.java b/src/main/java/org/jenkinsci/plugins/pretestedintegration/AbstractSCMBridge.java index bc49e914..3ed07e3d 100644 --- a/src/main/java/org/jenkinsci/plugins/pretestedintegration/AbstractSCMBridge.java +++ b/src/main/java/org/jenkinsci/plugins/pretestedintegration/AbstractSCMBridge.java @@ -9,8 +9,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; -import java.util.logging.Logger; - import jenkins.model.Jenkins; import org.jenkinsci.plugins.pretestedintegration.exceptions.*; @@ -18,13 +16,10 @@ * Abstract class representing an SCM bridge. */ public abstract class AbstractSCMBridge implements Describable, ExtensionPoint { - private static final Logger LOGGER = Logger.getLogger(AbstractSCMBridge.class.getName()); - /** * Information about the result of the integration (Unknown, Conflict, Build, Push). */ - protected String resultInfo = "Unknown"; protected abstract String getIntegrationBranch(); /** @@ -216,20 +211,6 @@ public static List> getDescriptors() { return descriptors; } - /** - * @return The information of the result of the Phlow - */ - public String getResultInfo() { - return resultInfo; - } - - /** - * @param resultInfo - */ - public void setResultInfo(String resultInfo) { - this.resultInfo = resultInfo; - } - /** * @param environment environment * @return The Integration Branch name as variable expanded if possible - otherwise return integrationBranch @@ -238,7 +219,6 @@ public String getExpandedIntegrationBranch(EnvVars environment) { return environment.expand(getIntegrationBranch()); } - /** * @param environment The environment to expand the integrationBranch in * @return The Integration Branch name, expanded using given EnvVars. @@ -250,4 +230,8 @@ public String getExpandedIntegrationBranch(EnvVars environment) { public Result getRequiredResult() { return Result.SUCCESS; } + + void setIntegrationFailedStatusUnstable(boolean integrationFailedStatusUnstable) { + throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates. + } } diff --git a/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationBuildWrapper.java b/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationBuildWrapper.java index dba0789a..837f2109 100644 --- a/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationBuildWrapper.java +++ b/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationBuildWrapper.java @@ -20,9 +20,7 @@ import org.jenkinsci.plugins.pretestedintegration.exceptions.IntegrationFailedException; import org.jenkinsci.plugins.pretestedintegration.exceptions.NothingToDoException; import org.jenkinsci.plugins.pretestedintegration.exceptions.UnsupportedConfigurationException; -import org.jenkinsci.plugins.pretestedintegration.scm.git.GitBridge; import org.kohsuke.stapler.DataBoundConstructor; -import org.kohsuke.stapler.DataBoundSetter; /** * The build wrapper determines what will happen before the build will run. @@ -40,36 +38,19 @@ public class PretestedIntegrationBuildWrapper extends BuildWrapper { /** * The SCM Bridge used for this project. */ - public final GitBridge scmBridge; - + public final AbstractSCMBridge scmBridge; /** * Constructor for the Build Wrapper. * DataBound to work with Jenkins UI. * @param scmBridge the SCM bridge + * @param integrationFailedStatusUnstable + * @param allowedNoCommits */ @DataBoundConstructor - public PretestedIntegrationBuildWrapper(final GitBridge scmBridge) { + public PretestedIntegrationBuildWrapper(final AbstractSCMBridge scmBridge) { this.scmBridge = scmBridge; } - @DataBoundSetter - public void setIntegrationFailedStatusUnstable(boolean integrationFailedStatusUnstable){ - this.scmBridge.setIntegrationFailedStatusUnstable(integrationFailedStatusUnstable); - } - public boolean getIntegrationFailedStatusUnstable(){ - if ( scmBridge == null ) { - return false; - } else { - return scmBridge.getIntegrationFailedStatusUnstable(); - } - } - public String getAllowedNoCommits(){ - if ( scmBridge == null ) { - return ""; - } else { - return Integer.toString(scmBridge.getAllowedNoCommits()); - } - } public String getIntegrationBranch(){ if ( scmBridge == null ) { return "master"; @@ -143,14 +124,6 @@ public String getVersion(){ */ @Extension public static class DescriptorImpl extends BuildWrapperDescriptor { - - /** - * Constructor for the Descriptor - */ - public DescriptorImpl() { - load(); - } - /** * {@inheritDoc} */ @@ -171,11 +144,13 @@ public List> getSCMBridges() { */ @Override public boolean isApplicable(AbstractProject arg0) { - if (arg0 instanceof FreeStyleProject) + if (arg0 instanceof FreeStyleProject) { return true; - if (arg0 instanceof MatrixProject) + } else if (arg0 instanceof MatrixProject) { return true; - return false; + } else { + return false; + } } } diff --git a/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationJobDslExtension.java b/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationJobDslExtension.java index db4227d1..02c73682 100644 --- a/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationJobDslExtension.java +++ b/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationJobDslExtension.java @@ -67,6 +67,7 @@ public Object pretestedIntegration(String strategy, String branch, String reposi break; } + GitBridge bridge = new GitBridge(integrationStrategy, branch, repository, Integer.getInteger(allowedNoCommits)); return new PretestedIntegrationBuildWrapper(new GitBridge(integrationStrategy, branch, repository, Integer.getInteger(allowedNoCommits))); } diff --git a/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationPostCheckout.java b/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationPostCheckout.java index 1fd37c63..e3e84163 100644 --- a/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationPostCheckout.java +++ b/src/main/java/org/jenkinsci/plugins/pretestedintegration/PretestedIntegrationPostCheckout.java @@ -99,7 +99,7 @@ private AbstractSCMBridge getScmBridge(AbstractBuild build, BuildListener public boolean perform(AbstractBuild build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException { AbstractProject proj = build.getProject(); GitSCM client = (GitSCM)build.getProject().getScm(); - if ( ! (client instanceof GitSCM) || client == null ) { + if ( ! (client instanceof GitSCM) ) { listener.getLogger().println(PretestedIntegrationBuildWrapper.LOG_PREFIX + "Unsupported SCM type: expects Git"); return false; } @@ -116,27 +116,23 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen } catch (NullPointerException | IllegalArgumentException e) { listener.getLogger().println(String.format("Caught %s during post-checkout. Failing build.", e.getClass().getSimpleName())); e.printStackTrace(listener.getLogger()); - bridge.setResultInfo("Unknown"); bridge.updateBuildDescription((Run)build); throw new AbortException("Unexpected error. Trace written to log."); } catch (IOException e) { //All our known errors are IOExceptions. Just print the message, log the error. listener.getLogger().println(e.getMessage()); LOGGER.log(Level.SEVERE, "IOException in post checkout", e); - bridge.setResultInfo("Unknown"); bridge.updateBuildDescription(build, launcher, listener); throw new AbortException(e.getMessage()); } } if ( build.getResult() == Result.SUCCESS ) { - bridge.setResultInfo("Ok"); } bridge.updateBuildDescription((Run)build); return true; } else { /* */ if (proj instanceof MatrixConfiguration) { listener.getLogger().println(PretestedIntegrationBuildWrapper.LOG_PREFIX + "MatrixConfiguration/sub - skipping publisher - leaving it to root job"); - bridge.setResultInfo("Ok"); bridge.updateBuildDescription(build, launcher, listener); } else { listener.getLogger().println(PretestedIntegrationBuildWrapper.LOG_PREFIX + "Performing pre-verified post build steps"); @@ -153,7 +149,6 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen throw new AbortException(e.getMessage()); } } - bridge.setResultInfo("Ok"); bridge.updateBuildDescription(build, launcher, listener); return true; } diff --git a/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/GitBridge.java b/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/GitBridge.java index eb001649..aec48831 100644 --- a/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/GitBridge.java +++ b/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/GitBridge.java @@ -36,6 +36,7 @@ import org.jenkinsci.plugins.pretestedintegration.PretestedIntegrationBuildWrapper; import org.jenkinsci.plugins.pretestedintegration.SCMBridgeDescriptor; import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.DataBoundSetter; /** * The Git SCM Bridge. @@ -67,15 +68,15 @@ public class GitBridge extends AbstractSCMBridge { * Constructor for GitBridge. * DataBound for use in the UI. * @param integrationStrategy The selected IntegrationStrategy - * @param branch The Integration Branch name - * @param repositoryName The Integration Repository name + * @param integrationBranch The Integration Branch name + * @param repoName The Integration Repository name * @param allowedNoCommits The Integration Repository name */ @DataBoundConstructor - public GitBridge(IntegrationStrategy integrationStrategy, final String branch, final String repositoryName, final Integer allowedNoCommits) { + public GitBridge(IntegrationStrategy integrationStrategy, String integrationBranch, String repoName, final Integer allowedNoCommits) { super(integrationStrategy); - this.integrationBranch = branch; - this.repoName = repositoryName; + this.integrationBranch = integrationBranch; + this.repoName = repoName; this.allowedNoCommits = allowedNoCommits; } @@ -271,7 +272,7 @@ public void updateBuildDescription(Run run) throws IOException { */ @Override public String createBuildDescription(String triggerBranchName) throws NothingToDoException, UnsupportedConfigurationException, IOException, InterruptedException { - return String.format("%s", "(" + getResultInfo() + "):" + triggerBranchName + " -> " + integrationBranch); + return String.format("%s", "(" + "REPLACEDME" + "):" + triggerBranchName + " -> " + integrationBranch); } @@ -360,9 +361,7 @@ public void handlePostBuild(AbstractBuild build, Launcher launcher, BuildL Result result = build.getResult(); if (result != null && result.isBetterOrEqualTo(getRequiredResult())) { - setResultInfo("Push"); pushToIntegrationBranch(build, listener); - setResultInfo("CleanUpRemote"); deleteIntegratedBranch(build, launcher, listener); } else { LOGGER.log(Level.WARNING, "Build result not satisfied - skipped post-build step."); @@ -411,6 +410,7 @@ public boolean getIntegrationFailedStatusUnstable() { return this.integrationFailedStatusUnstable; } + @DataBoundSetter public void setIntegrationFailedStatusUnstable( boolean integrationFailedStatusUnstable) { this.integrationFailedStatusUnstable = integrationFailedStatusUnstable; } @@ -419,13 +419,15 @@ public Integer getAllowedNoCommits() { return this.allowedNoCommits; } - public void setAllowedNoCommits( Integer allowedNoCommits) { + @DataBoundSetter + public void setAllowedNoCommits(int allowedNoCommits) { this.allowedNoCommits = allowedNoCommits; } /** * @param repositoryName the repositoryName to set */ + @DataBoundSetter public void setRepoName(String repositoryName) { this.repoName = repositoryName; } @@ -444,13 +446,6 @@ public String getExpandedRepository(EnvVars environment) { @Extension public static final class DescriptorImpl extends SCMBridgeDescriptor { - /** - * Constructor for the Descriptor - */ - public DescriptorImpl() { - load(); - } - /** * {@inheritDoc } */ diff --git a/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/GitIntegrationStrategy.java b/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/GitIntegrationStrategy.java index 774bc9c0..fb278956 100644 --- a/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/GitIntegrationStrategy.java +++ b/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/GitIntegrationStrategy.java @@ -92,6 +92,7 @@ protected boolean tryRebase(ObjectId commitId, GitClient client, String branch ) * @param commitId The commit * @param logger The logger for console logging * @param client The GitClient + * @param commitCount * @return true if the FF merge was a success, false if the integrationBranch isn't * suitable for a FF merge. * @throws IntegrationFailedException When commit counting fails diff --git a/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/PretestedIntegrationAsGitPluginExt.java b/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/PretestedIntegrationAsGitPluginExt.java index 256b2f6f..3bdfdf88 100644 --- a/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/PretestedIntegrationAsGitPluginExt.java +++ b/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/PretestedIntegrationAsGitPluginExt.java @@ -51,8 +51,8 @@ public class PretestedIntegrationAsGitPluginExt extends GitSCMExtension { * @param allowedNoCommits The amount of commits allowed for integration */ @DataBoundConstructor - public PretestedIntegrationAsGitPluginExt(IntegrationStrategy gitIntegrationStrategy, final String integrationBranch, final String repoName, String allowedNoCommits) { - this.gitBridge = new GitBridge(gitIntegrationStrategy,integrationBranch,repoName, Integer.valueOf(allowedNoCommits)); + public PretestedIntegrationAsGitPluginExt(IntegrationStrategy gitIntegrationStrategy, final String integrationBranch, final String repoName, int allowedNoCommits) { + this.gitBridge = new GitBridge(gitIntegrationStrategy,integrationBranch,repoName, allowedNoCommits); } @DataBoundSetter @@ -140,7 +140,7 @@ public Revision decorateRevisionToBuild(GitSCM scm, Run run, GitClient git } catch (IOException ex) { LOGGER.log(Level.FINE, "Failed to update build description", ex); } - gitBridge.setResultInfo("NothingToDo"); + gitBridge.updateBuildDescription(run); throw new GitException("Nothing to do.."); } catch (IntegrationFailedException| UnsupportedConfigurationException e) { @@ -149,7 +149,6 @@ public Revision decorateRevisionToBuild(GitSCM scm, Run run, GitClient git } else { run.setResult(Result.FAILURE); } - gitBridge.setResultInfo("MergeFailed"); String logMessage = String.format("%s - decorateRevisionToBuild() - %s - %s", LOG_PREFIX, e.getClass().getSimpleName(), e.getMessage()); listener.getLogger().println(logMessage); LOGGER.log(Level.SEVERE, logMessage, e); @@ -165,14 +164,12 @@ public Revision decorateRevisionToBuild(GitSCM scm, Run run, GitClient git } else { run.setResult(Result.FAILURE); } - gitBridge.setResultInfo("NoCommits"); String logMessage = String.format("%s - decorateRevisionToBuild() - %s - %s", LOG_PREFIX, e.getClass().getSimpleName(), e.getMessage()); listener.getLogger().println(logMessage); LOGGER.log(Level.SEVERE, logMessage, e); } catch (IOException e) { run.setResult(Result.FAILURE); - gitBridge.setResultInfo("UNKNOWN"); gitBridge.updateBuildDescription(run); String logMessage = String.format("%s - Unexpected error. %n%s", LOG_PREFIX, e.getMessage()); LOGGER.log(Level.SEVERE, logMessage, e); @@ -191,7 +188,6 @@ public Revision decorateRevisionToBuild(GitSCM scm, Run run, GitClient git if ( run.getResult() == null || run.getResult() == Result.SUCCESS ) { Revision mergeRevision = new GitUtils(listener,git).getRevisionForSHA1(git.revParse(HEAD)); - gitBridge.setResultInfo("Build"); mergeRevision.getBranches().add( new Branch(gitBridge.getExpandedIntegrationBranch(environment), triggeredRevision.getSha1())); return mergeRevision; diff --git a/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/PretestedIntegrationAsGitPluginExtJobDslExtension.java b/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/PretestedIntegrationAsGitPluginExtJobDslExtension.java index 47acbb08..2899f680 100644 --- a/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/PretestedIntegrationAsGitPluginExtJobDslExtension.java +++ b/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/PretestedIntegrationAsGitPluginExtJobDslExtension.java @@ -68,7 +68,7 @@ public Object pretestedIntegrationAsGitPluginExt(String strategy, String branch, integrationStrategy = new SquashCommitStrategy(); break; } - return new PretestedIntegrationAsGitPluginExt((GitIntegrationStrategy)integrationStrategy, branch, repository); + return new PretestedIntegrationAsGitPluginExt(integrationStrategy, branch, repository, Integer.MAX_VALUE); } /** diff --git a/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/SquashCommitStrategy.java b/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/SquashCommitStrategy.java index 301a2d7c..5fa23060 100644 --- a/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/SquashCommitStrategy.java +++ b/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/SquashCommitStrategy.java @@ -79,7 +79,7 @@ public void integrate(AbstractBuild build, Launcher launcher, BuildListene expandedIntegrationBranch = gitbridge.getIntegrationBranch(); } - if(tryFastForward(buildData.lastBuild.getSHA1(), listener.getLogger(), client, expandedIntegrationBranch )) return; + if(tryFastForward(buildData.lastBuild.getSHA1(), listener.getLogger(), client, gitbridge.getAllowedNoCommits() )) return; if(tryRebase(buildData.lastBuild.getSHA1(), client, expandedIntegrationBranch )) return; try { @@ -181,7 +181,7 @@ public void integrateAsGitPluginExt(GitSCM scm, Run build, GitClient clien expandedIntegrationBranch = gitbridge.getIntegrationBranch(); } - if(tryFastForward(buildData.lastBuild.getSHA1(), listener.getLogger(), client, expandedIntegrationBranch )) return; + if(tryFastForward(buildData.lastBuild.getSHA1(), listener.getLogger(), client, gitbridge.getAllowedNoCommits() )) return; if(tryRebase(buildData.lastBuild.getSHA1(), client, expandedIntegrationBranch )) return; String expandedBranchName; diff --git a/src/main/resources/org/jenkinsci/plugins/pretestedintegration/scm/git/GitBridge/config.jelly b/src/main/resources/org/jenkinsci/plugins/pretestedintegration/scm/git/GitBridge/config.jelly index 738ddf92..294f1f0c 100644 --- a/src/main/resources/org/jenkinsci/plugins/pretestedintegration/scm/git/GitBridge/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/pretestedintegration/scm/git/GitBridge/config.jelly @@ -1,20 +1,20 @@ - - + + + - + - + - @@ -26,5 +26,6 @@ +