Add setters to the StashNotifier.DescriptorImpl class #201
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,25 +3,17 @@ | |
| <parent> | ||
| <groupId>org.jenkins-ci.plugins</groupId> | ||
| <artifactId>plugin</artifactId> | ||
| <version>3.40</version> | ||
| <relativePath /> | ||
| </parent> | ||
| <packaging>hpi</packaging> | ||
| <properties> | ||
| <revision>1.18</revision> | ||
| <changelist>-SNAPSHOT</changelist> | ||
| <!-- Baseline Jenkins version you use to build the plugin. Users must have this version or newer to run. --> | ||
| <jenkins.version>2.60.3</jenkins.version> | ||
| <!-- Java Level to use. Java 7 required when using core >= 1.612 --> | ||
| <java.level>8</java.level> | ||
| </properties> | ||
| <artifactId>stashNotifier</artifactId> | ||
| <version>${revision}${changelist}</version> | ||
|
|
@@ -143,6 +135,18 @@ | |
| <version>2.0</version> | ||
| <type>jar</type> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.jenkins-ci.plugins</groupId> | ||
| <artifactId>matrix-project</artifactId> | ||
|
There was a problem hiding this comment. Why was the matrix project dependency added? The commit description makes no mention of it. I don't see the matrix plugin being used in the code. I see that it's only for testing, but I still don't see those new tests. There was a problem hiding this comment. That's a There was a problem hiding this comment. Sorry, I missed to add that. I added the Matrix project test dependency because the JenkinsRule requires it and it has been removed from core since 1.561. I added the JenkinsRule in the test that verifies the configuration as code setup, according to JCasC documentation. (import and export yaml). (this is the issue where I found more information about the NoClassDefFoundError I got while using the JenkinsRule: https://issues.jenkins-ci.org/browse/JENKINS-27826) There was a problem hiding this comment. Thank you for the explanation! |
||
| <version>1.9</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>io.jenkins</groupId> | ||
| <artifactId>configuration-as-code</artifactId> | ||
| <version>1.8</version> | ||
| <scope>test</scope> | ||
| </dependency> | ||
| </dependencies> | ||
| <build> | ||
| <pluginManagement> | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,10 +21,7 @@ | |
| import com.cloudbees.plugins.credentials.common.*; | ||
| import com.cloudbees.plugins.credentials.common.UsernamePasswordCredentials; | ||
| import com.cloudbees.plugins.credentials.domains.DomainRequirement; | ||
| import hudson.*; | ||
|
There was a problem hiding this comment. I don't think wildcards make the code better. There are plenty of tools to keep an up-to-date list of used imports. There was a problem hiding this comment. For me, using wildcard imports or not is a project decision. There are as always advantages and disadvantages. And nowadays tools should be able to handle both. There was a problem hiding this comment. I agree on all counts, I just wanted to make sure it was an intentional change and not somebody's sloppiness that everybody missed. There was a problem hiding this comment. I'm not in general a Java developer and I adopted (ocassionaly, huh) this plugin with worse code conventions than it have today, but there is still no CSC strictly defined here today. We can adopt one, I think, if it make life easier. |
||
| import hudson.model.*; | ||
| import hudson.plugins.git.Revision; | ||
| import hudson.plugins.git.util.BuildData; | ||
|
|
@@ -69,10 +66,7 @@ | |
| import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider; | ||
| import org.jenkinsci.plugins.tokenmacro.MacroEvaluationException; | ||
| import org.jenkinsci.plugins.tokenmacro.TokenMacro; | ||
| import org.kohsuke.stapler.*; | ||
|
|
||
| import javax.annotation.Nonnull; | ||
| import javax.net.ssl.SSLContext; | ||
|
|
@@ -311,10 +305,6 @@ private boolean perform(Run<?, ?> run, | |
| private String getRootUrl() { | ||
| Jenkins instance = Jenkins.getInstance(); | ||
|
|
||
scaytrase marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return (instance.getRootUrl() != null) ? instance.getRootUrl() : globalConfig.getUrl(); | ||
| } | ||
|
|
||
|
|
@@ -531,10 +521,6 @@ public boolean isTrusted(X509Certificate[] chain, String authType) | |
|
|
||
| private void configureProxy(HttpClientBuilder builder, URL url) { | ||
| Jenkins jenkins = Jenkins.getInstance(); | ||
scaytrase marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ProxyConfiguration proxyConfig = jenkins.proxy; | ||
| if (proxyConfig == null) { | ||
| return; | ||
|
|
@@ -584,14 +570,14 @@ public static final class DescriptorImpl | |
| * If you don't want fields to be persisted, use <tt>transient</tt>. | ||
| */ | ||
|
|
||
| private boolean considerUnstableAsSuccess; | ||
| private String credentialsId; | ||
| private boolean disableInprogressNotification; | ||
| private boolean ignoreUnverifiedSsl; | ||
| private boolean includeBuildNumberInKey; | ||
|
There was a problem hiding this comment. @scaytrase I am wondring why There was a problem hiding this comment. Not sure for it really, may be a copy-paste coincident. We can try to remove it, cannot image when project key should be declared on global level There was a problem hiding this comment. if it's non blocking for this PR we can try it as a separate change |
||
| private boolean prependParentProjectKey; | ||
| private String projectKey; | ||
| private String stashRootUrl; | ||
|
|
||
| public DescriptorImpl() { | ||
| this(true); | ||
|
|
@@ -629,42 +615,78 @@ public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item project) { | |
| return new StandardListBoxModel(); | ||
| } | ||
|
|
||
| public boolean isConsiderUnstableAsSuccess() { | ||
| return considerUnstableAsSuccess; | ||
| } | ||
|
|
||
| @DataBoundSetter | ||
| public void setConsiderUnstableAsSuccess(boolean considerUnstableAsSuccess) { | ||
| this.considerUnstableAsSuccess = considerUnstableAsSuccess; | ||
| } | ||
|
|
||
| public String getCredentialsId() { | ||
| return credentialsId; | ||
| } | ||
|
|
||
| @DataBoundSetter | ||
| public void setCredentialsId(String credentialsId) { | ||
| this.credentialsId = StringUtils.trimToNull(credentialsId); | ||
| } | ||
|
|
||
| public boolean isDisableInprogressNotification() { | ||
| return disableInprogressNotification; | ||
| } | ||
|
|
||
| @DataBoundSetter | ||
| public void setDisableInprogressNotification(boolean disableInprogressNotification) { | ||
| this.disableInprogressNotification = disableInprogressNotification; | ||
| } | ||
|
|
||
| public boolean isIgnoreUnverifiedSsl() { | ||
| return ignoreUnverifiedSsl; | ||
| } | ||
|
|
||
| @DataBoundSetter | ||
| public void setIgnoreUnverifiedSsl(boolean ignoreUnverifiedSsl) { | ||
| this.ignoreUnverifiedSsl = ignoreUnverifiedSsl; | ||
| } | ||
|
|
||
| public boolean isIncludeBuildNumberInKey() { | ||
| return includeBuildNumberInKey; | ||
| } | ||
|
|
||
| @DataBoundSetter | ||
| public void setIncludeBuildNumberInKey(boolean includeBuildNumberInKey) { | ||
| this.includeBuildNumberInKey = includeBuildNumberInKey; | ||
| } | ||
|
|
||
| public boolean isPrependParentProjectKey() { | ||
| return prependParentProjectKey; | ||
| } | ||
|
|
||
| @DataBoundSetter | ||
| public void setPrependParentProjectKey(boolean prependParentProjectKey) { | ||
| this.prependParentProjectKey = prependParentProjectKey; | ||
| } | ||
|
|
||
| public String getProjectKey() { | ||
| return projectKey; | ||
| } | ||
|
|
||
| @DataBoundSetter | ||
| public void setProjectKey(String projectKey) { | ||
|
There was a problem hiding this comment. It seems you missed a |
||
| this.projectKey = StringUtils.trimToNull(projectKey); | ||
| } | ||
|
|
||
| public String getStashRootUrl() { | ||
scaytrase marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return stashRootUrl; | ||
| } | ||
|
|
||
| @DataBoundSetter | ||
| public void setStashRootUrl(String stashRootUrl) { | ||
| this.stashRootUrl = StringUtils.trimToNull(stashRootUrl); | ||
| } | ||
|
|
||
| public FormValidation doCheckCredentialsId(@QueryParameter String value, @AncestorInPath Item project) | ||
| throws IOException, ServletException { | ||
|
|
||
|
|
@@ -720,26 +742,20 @@ public boolean configure( | |
| StaplerRequest req, | ||
| JSONObject formData) throws FormException { | ||
|
|
||
| this.considerUnstableAsSuccess = false; | ||
| this.credentialsId = null; | ||
| this.disableInprogressNotification = false; | ||
| this.ignoreUnverifiedSsl = false; | ||
| this.includeBuildNumberInKey = false; | ||
| this.prependParentProjectKey = false; | ||
| this.projectKey = null; | ||
| this.stashRootUrl = null; | ||
|
|
||
| req.bindJSON(this, formData); | ||
|
|
||
| save(); | ||
|
|
||
| return true; | ||
| } | ||
| } | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| package org.jenkinsci.plugins.stashNotifier; | ||
|
|
||
| import hudson.util.IOUtils; | ||
| import io.jenkins.plugins.casc.ConfigurationAsCode; | ||
| import org.junit.Rule; | ||
| import org.junit.Test; | ||
| import org.jvnet.hudson.test.JenkinsRule; | ||
|
|
||
| import java.io.ByteArrayOutputStream; | ||
| import java.io.InputStream; | ||
| import java.net.URL; | ||
|
|
||
| import static org.hamcrest.CoreMatchers.containsString; | ||
| import static org.hamcrest.Matchers.equalTo; | ||
| import static org.junit.Assert.assertThat; | ||
|
|
||
| public class ConfigAsCodeTest { | ||
| @Rule | ||
| public JenkinsRule rule = new JenkinsRule(); | ||
|
|
||
| @Test | ||
| public void should_support_jcasc_from_yaml() throws Exception { | ||
| URL configFileUrl = ConfigAsCodeTest.class.getResource(getClass().getSimpleName() + "/configuration-as-code.yml"); | ||
| ConfigurationAsCode.get().configure(configFileUrl.toString()); | ||
| StashNotifier.DescriptorImpl stashNotifierConfig = rule.jenkins.getDescriptorByType(StashNotifier.DescriptorImpl.class); | ||
|
|
||
| assertThat(stashNotifierConfig.isConsiderUnstableAsSuccess(), equalTo(true)); | ||
| assertThat(stashNotifierConfig.getCredentialsId(), equalTo("bitbucket-credentials")); | ||
| assertThat(stashNotifierConfig.isDisableInprogressNotification(), equalTo(true)); | ||
| assertThat(stashNotifierConfig.isIgnoreUnverifiedSsl(), equalTo(true)); | ||
| assertThat(stashNotifierConfig.isIncludeBuildNumberInKey(), equalTo(true)); | ||
| assertThat(stashNotifierConfig.isPrependParentProjectKey(), equalTo(true)); | ||
| assertThat(stashNotifierConfig.getProjectKey(), equalTo("JEN")); | ||
| assertThat(stashNotifierConfig.getStashRootUrl(), equalTo("https://my.company.intranet/bitbucket")); | ||
| } | ||
|
There was a problem hiding this comment. Verifying |
||
|
|
||
| @Test | ||
| public void should_support_jcasc_to_yaml() throws Exception { | ||
| StashNotifier.DescriptorImpl stashNotifierConfig = rule.jenkins.getDescriptorByType(StashNotifier.DescriptorImpl.class); | ||
|
|
||
| stashNotifierConfig.setConsiderUnstableAsSuccess(true); | ||
| stashNotifierConfig.setCredentialsId("bitbucket-credentials"); | ||
| stashNotifierConfig.setDisableInprogressNotification(true); | ||
| stashNotifierConfig.setIgnoreUnverifiedSsl(true); | ||
| stashNotifierConfig.setIncludeBuildNumberInKey(true); | ||
| stashNotifierConfig.setPrependParentProjectKey(true); | ||
| stashNotifierConfig.setProjectKey("JEN"); | ||
| stashNotifierConfig.setStashRootUrl("https://my.company.intranet/bitbucket"); | ||
|
|
||
| ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); | ||
| ConfigurationAsCode.get().export(outputStream); | ||
| String exportedYaml = outputStream.toString("UTF-8"); | ||
|
|
||
| InputStream yamlStream = getClass().getResourceAsStream(getClass().getSimpleName() + "/configuration-as-code.yml"); | ||
| String expectedYaml = IOUtils.toString(yamlStream, "UTF-8") | ||
| .replaceAll("\r\n?", "\n") | ||
| .replace("unclassified:\n", ""); | ||
|
|
||
| assertThat(exportedYaml, containsString(expectedYaml)); | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to reduce it to the following snippet:
The rest is already covered by the parent pom nowadays.
(This can be verified with
mvn help:effective-pom.)