handle null from XStream readResolve and refactor to fluent interface

Author: rantoniuk

CONTRIBUTING.md

@@ -4,6 +4,7 @@ General rules:
 
 - check the [general Jenkins development guide](https://www.jenkins.io/doc/developer/book/)
 - make sure to provide tests
+- when adding new fields, make sure to [include backward-compatibility](https://www.jenkins.io/doc/developer/persistence/backward-compatibility/) and tests for that
 - mark the Pull Request as _draft_ initially, to make sure all the checks pass correctly, then convert it to non-draft.
 
 ## Setting up your environment
@@ -22,20 +23,19 @@ brew install pre-commit && pre-commit install --install-hooks
 
 Use [docker-compose](./docker-compose.yml) to run a local Jenkins instance with the plugin installed. The configuration includes local volumes for both: Jenkins and ssh-agent, so you can easily test the plugin in a clean environment.
 
-```bash
 
 ### Atlassian sources import
 
-To resolve some binary compatibility issues [JENKINS-48357](https://issues.jenkins-ci.org/browse/JENKINS-48357),
+To resolve [some binary compatibility issues](https://github.com/jenkinsci/jira-plugin/pull/140),
 the sources from the artifact [com.atlassian.httpclient:atlassian-httpclient-plugin:0.23](https://packages.atlassian.com/maven-external/com/atlassian/httpclient/atlassian-httpclient-plugin/0.23.0/)
 has been imported in the project to have control over http(s) protocol transport layer.
 The downloaded sources didn't have any license headers but based on the [pom](https://packages.atlassian.com/maven-external/com/atlassian/httpclient/atlassian-httpclient-plugin/0.23.0/atlassian-httpclient-plugin-0.23.0.pom)
-sources are Apache License (see pom in src/main/resources/atlassian-httpclient-plugin-0.23.0.pom)   
+sources are Apache License (see pom in src/main/resources/atlassian-httpclient-plugin-0.23.0.pom)
 
 ### Testing
 
 There is a [Jira Cloud](https://jenkins-jira-plugin.atlassian.net/) test instance that the changes can be tested against, official maintainers are admins that can grant access for testing to PR submitters on a need-to-have basis.
 
 ### Releasing the plugin
 
-Make sure you have your `~/.m2/settings.xml` configured accordingly - refer to [releasing Jenkins plugins](https://www.jenkins.io/doc/developer/publishing/releasing/).
+See [releasing Jenkins plugins](https://www.jenkins.io/doc/developer/publishing/releasing-manually/).

README.md

@@ -6,7 +6,9 @@
 [![Jenkins CI](https://ci.jenkins.io/buildStatus/icon?job=Plugins/jira-plugin/master)](https://ci.jenkins.io/job/Plugins/job/jira-plugin/)
 [![Contributors](https://img.shields.io/github/contributors/jenkinsci/jira-plugin.svg)](https://github.com/jenkinsci/jira-plugin/graphs/contributors)
 
-See user documentation at [https://jenkinsci.github.io/jira-plugin/](https://jenkinsci.github.io/jira-plugin/)
+1. See user documentation at [https://jenkinsci.github.io/jira-plugin/](https://jenkinsci.github.io/jira-plugin/).
+1. Use [Declarative pipelines](https://www.jenkins.io/doc/book/pipeline/#declarative-versus-scripted-pipeline-syntax).
+1. Check [jira plugin steps reference](https://www.jenkins.io/doc/pipeline/steps/jira/).
 
 ## i18n
 
@@ -22,10 +24,3 @@ See user documentation at [https://jenkinsci.github.io/jira-plugin/](https://jen
 
 This plugin uses [CrowdIn platform](https://jenkins.crowdin.com/jira-plugin) as the frontend to manage translations. If you would like to contribute translation of this plugin in your language,  you're most welcome! For details, see [jenkins.io CrowdIn introduction](https://www.jenkins.io/doc/developer/crowdin/translating-plugins/).
 
-## Contributing
-
-There have been many developers involved in the development of this plugin and there are many downstream users who depend on it. Tests help us assure that we're delivering a reliable plugin and that we've communicated our intent to other developers in a way that they can detect when they run tests.
-
-- each change should be covered by appropriate unit tests
-- in case it is not testable via a unit test, it should be tested against a real Jira instance - possibly both Jira Server and Jira Cloud. There is a [Jira Cloud test instance](https://jenkins-jira-plugin.atlassian.net/) that we are using for testing the plugin releases - let us know in the Pull Request in case you need access for testing
- 

src/main/java/hudson/plugins/jira/JiraVersionCreator.java

@@ -18,14 +18,15 @@
  * A build step which creates new Jira version
  *
  * @author Artem Koshelev [email protected]
- * @deprecated Replaced by {@link JiraVersionCreatorBuilder}. Read its description to see why.
- * Kept for backward compatibility.
+ * @deprecated Replaced by {@link JiraVersionCreatorBuilder}. Read its
+ * description to see why. Kept for backward compatibility.
  */
 @Deprecated
 public class JiraVersionCreator extends Notifier {
+
     private String jiraVersion;
     private String jiraProjectKey;
-    private boolean failIfAlreadyExists = true;
+    private Boolean failIfAlreadyExists = true;
 
     @DataBoundConstructor
     public JiraVersionCreator(String jiraVersion, String jiraProjectKey) {
@@ -67,14 +68,24 @@ public void setFailIfAlreadyExists(boolean failIfAlreadyExists) {
     public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) {
         VersionCreator versionCreator = new VersionCreator();
         versionCreator.setFailIfAlreadyExists(failIfAlreadyExists);
-        return versionCreator.perform(build.getProject(), jiraVersion, jiraProjectKey, build, listener);
+        versionCreator.setJiraVersion(jiraVersion);
+        versionCreator.setJiraProjectKey(jiraProjectKey);
+        return versionCreator.perform(build.getProject(), build, listener);
     }
 
     @Override
     public BuildStepDescriptor<Publisher> getDescriptor() {
         return DESCRIPTOR;
     }
 
+    protected Object readResolve() {
+        if (failIfAlreadyExists == null) {
+            setFailIfAlreadyExists(true);
+        }
+        
+        return this;
+    }
+
     @Extension
     public static final DescriptorImpl DESCRIPTOR = new DescriptorImpl();
 
@@ -103,5 +114,6 @@ public String getDisplayName() {
         public String getHelpFile() {
             return "/plugin/jira/help-version-create.html";
         }
+
     }
 }

src/main/java/hudson/plugins/jira/JiraVersionCreatorBuilder.java

@@ -26,7 +26,7 @@ public class JiraVersionCreatorBuilder extends Builder implements SimpleBuildSte
 
     private String jiraVersion;
     private String jiraProjectKey;
-    private boolean failIfAlreadyExists = true;
+    private Boolean failIfAlreadyExists = true;
 
     @DataBoundConstructor
     public JiraVersionCreatorBuilder(String jiraVersion, String jiraProjectKey) {
@@ -59,6 +59,14 @@ public boolean isFailIfAlreadyExists() {
         return failIfAlreadyExists;
     }
 
+    protected Object readResolve() {
+        if (failIfAlreadyExists == null) {
+            setFailIfAlreadyExists(true);
+        }
+        
+        return this;
+    }
+
     @DataBoundSetter
     public void setFailIfAlreadyExists(boolean failIfAlreadyExists) {
         this.failIfAlreadyExists = failIfAlreadyExists;
@@ -68,7 +76,9 @@ public void setFailIfAlreadyExists(boolean failIfAlreadyExists) {
     public void perform(Run<?, ?> run, EnvVars env, TaskListener listener) {
         VersionCreator versionCreator = new VersionCreator();
         versionCreator.setFailIfAlreadyExists(failIfAlreadyExists);
-        versionCreator.perform(run.getParent(), jiraVersion, jiraProjectKey, run, listener);
+        versionCreator.setJiraVersion(jiraVersion);
+        versionCreator.setJiraProjectKey(jiraProjectKey);
+        versionCreator.perform(run.getParent(), run, listener);
     }
 
     @Override

src/main/java/hudson/plugins/jira/VersionCreator.java

@@ -1,5 +1,10 @@
 package hudson.plugins.jira;
 
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.logging.Logger;
+
 import static org.apache.commons.lang.StringUtils.isEmpty;
 
 import hudson.model.BuildListener;
@@ -8,11 +13,6 @@
 import hudson.model.Run;
 import hudson.model.TaskListener;
 import hudson.plugins.jira.extension.ExtendedVersion;
-import java.util.Collections;
-import java.util.List;
-import java.util.Optional;
-import java.util.logging.Logger;
-import org.kohsuke.stapler.DataBoundSetter;
 
 /**
  * Performs an action which creates new Jira version.
@@ -23,17 +23,27 @@ class VersionCreator {
 
     private boolean failIfAlreadyExists = true;
 
-    @DataBoundSetter
-    public void setFailIfAlreadyExists(boolean failIfAlreadyExists) {
+    private String jiraVersion;
+
+    private String jiraProjectKey;
+    
+    public VersionCreator setFailIfAlreadyExists(boolean failIfAlreadyExists) {
         this.failIfAlreadyExists = failIfAlreadyExists;
+        return this;
     }
 
-    public boolean isFailIfAlreadyExists() {
-        return failIfAlreadyExists;
+    public VersionCreator setJiraVersion(String jiraVersion) {
+        this.jiraVersion = jiraVersion;
+        return this;
+    }
+
+    public VersionCreator setJiraProjectKey(String jiraProjectKey) {
+        this.jiraProjectKey = jiraProjectKey;
+        return this;
     }
 
     protected boolean perform(
-            Job<?, ?> project, String jiraVersion, String jiraProjectKey, Run<?, ?> build, TaskListener listener) {
+            Job<?, ?> project, Run<?, ?> build, TaskListener listener) {
         String realVersion = null;
         String realProjectKey = null;
 
@@ -75,9 +85,6 @@ protected boolean perform(
                     listener.fatalError("Unable to add version %s to Jira project %s", realVersion, realProjectKey, e));
         }
 
-        if (listener instanceof BuildListener) {
-            ((BuildListener) listener).finished(Result.FAILURE);
-        }
         return false;
     }
 

src/test/java/hudson/plugins/jira/VersionCreatorTest.java

@@ -1,35 +1,37 @@
 package hudson.plugins.jira;
 
+import java.io.IOException;
+import java.io.PrintStream;
+import java.util.Arrays;
+
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
+import org.joda.time.DateTime;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
 import static org.mockito.ArgumentMatchers.any;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.Mockito;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.mockito.stubbing.Answer;
 
 import com.atlassian.jira.rest.client.api.RestClientException;
+
 import hudson.EnvVars;
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
 import hudson.model.BuildListener;
 import hudson.model.Result;
 import hudson.plugins.jira.extension.ExtendedVersion;
-import java.io.IOException;
-import java.io.PrintStream;
-import java.util.Arrays;
-import org.joda.time.DateTime;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.ArgumentCaptor;
-import org.mockito.Captor;
-import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.junit.jupiter.MockitoExtension;
-import org.mockito.stubbing.Answer;
 
 @ExtendWith(MockitoExtension.class)
 class VersionCreatorTest {
@@ -96,7 +98,9 @@ void callsJiraWithSpecifiedParameters() throws InterruptedException, IOException
 
         // for new version, verify the addVersion method is called
         when(session.getVersions(JIRA_PRJ)).thenReturn(null);
-        boolean result = versionCreator.perform(project, JIRA_VER, JIRA_PRJ, build, listener);
+        versionCreator.setJiraProjectKey(JIRA_PRJ);
+        versionCreator.setJiraVersion(JIRA_VER);      
+        boolean result = versionCreator.perform(project, build, listener);
         verify(session, times(1)).addVersion(versionCaptor.capture(), projectCaptor.capture());
         assertThat(projectCaptor.getValue(), is(JIRA_PRJ));
         assertThat(versionCaptor.getValue(), is(JIRA_VER));
@@ -106,7 +110,9 @@ void callsJiraWithSpecifiedParameters() throws InterruptedException, IOException
         // for existing version, verify the addVersion method is not called
         reset(session);
         when(session.getVersions(JIRA_PRJ)).thenReturn(Arrays.asList(existingVersion));
-        result = versionCreator.perform(project, JIRA_VER, JIRA_PRJ, build, listener);
+        versionCreator.setJiraProjectKey(JIRA_PRJ);
+        versionCreator.setJiraVersion(JIRA_VER);        
+        result = versionCreator.perform(project, build, listener);
         verify(session, times(0)).addVersion(versionCaptor.capture(), projectCaptor.capture());
         verify(logger, times(1)).println(Messages.JiraVersionCreator_VersionExists(JIRA_VER, JIRA_PRJ));
         verify(listener).finished(Result.FAILURE);
@@ -120,7 +126,9 @@ void expandsEnvParameters() throws InterruptedException, IOException, RestClient
 
         // for new version, verify the addVersion method is called
         when(session.getVersions(JIRA_PRJ)).thenReturn(null);
-        boolean result = versionCreator.perform(project, JIRA_VER_PARAM, JIRA_PRJ_PARAM, build, listener);
+        versionCreator.setJiraProjectKey(JIRA_PRJ_PARAM);
+        versionCreator.setJiraVersion(JIRA_VER_PARAM);
+        boolean result = versionCreator.perform(project, build, listener);
         verify(session, times(1)).addVersion(versionCaptor.capture(), projectCaptor.capture());
         assertThat(projectCaptor.getValue(), is(JIRA_PRJ));
         assertThat(versionCaptor.getValue(), is(JIRA_VER));
@@ -129,7 +137,9 @@ void expandsEnvParameters() throws InterruptedException, IOException, RestClient
         // for existing version, verify the addVersion method is called
         reset(session);
         when(session.getVersions(JIRA_PRJ)).thenReturn(Arrays.asList(existingVersion));
-        result = versionCreator.perform(project, JIRA_VER_PARAM, JIRA_PRJ_PARAM, build, listener);
+        versionCreator.setJiraProjectKey(JIRA_PRJ_PARAM);
+        versionCreator.setJiraVersion(JIRA_VER_PARAM);
+        result = versionCreator.perform(project, build, listener);
         verify(session, times(0)).addVersion(versionCaptor.capture(), projectCaptor.capture());
         verify(logger, times(1)).println(Messages.JiraVersionCreator_VersionExists(JIRA_VER, JIRA_PRJ));
         verify(listener).finished(Result.FAILURE);
@@ -143,8 +153,9 @@ void buildDidNotFailWhenVersionExists() throws IOException, InterruptedException
                 new ExtendedVersion(null, ANY_ID, JIRA_VER, null, false, true, ANY_DATE, ANY_DATE);
         when(site.getSession(any())).thenReturn(session);
         when(session.getVersions(JIRA_PRJ)).thenReturn(Arrays.asList(releasedVersion));
-
-        versionCreator.perform(project, JIRA_VER_PARAM, JIRA_PRJ_PARAM, build, listener);
+        versionCreator.setJiraProjectKey(JIRA_PRJ_PARAM);
+        versionCreator.setJiraVersion(JIRA_VER_PARAM);
+        versionCreator.perform(project, build, listener);
         verify(session, times(0)).addVersion(any(), any());
     }
 
@@ -157,8 +168,9 @@ void buildDoesNotFailWhenVersionExistsAndFailIfAlreadyExistsIsFalse()
 
         // Set failIfAlreadyExists to false
         versionCreator.setFailIfAlreadyExists(false);
-
-        boolean result = versionCreator.perform(project, JIRA_VER, JIRA_PRJ, build, listener);
+        versionCreator.setJiraProjectKey(JIRA_PRJ);
+        versionCreator.setJiraVersion(JIRA_VER);
+        boolean result = versionCreator.perform(project, build, listener);
 
         // Verify that addVersion is not called (because version already exists)
         verify(session, times(0)).addVersion(any(), any());