Merge pull request #89 from basil/improvements

Code cleanup
jenkinsci · Jul 8, 2020 · d12dd07 · d12dd07
2 parents 388dede + 853fab9
commit d12dd07
Show file tree

Hide file tree

Showing 15 changed files with 62 additions and 87 deletions.
diff --git a/...java/org/jenkinsci/plugins/workflow/multibranch/AbstractWorkflowBranchProjectFactory.java b/...java/org/jenkinsci/plugins/workflow/multibranch/AbstractWorkflowBranchProjectFactory.java
@@ -50,7 +50,7 @@ public abstract class AbstractWorkflowBranchProjectFactory extends BranchProject
     protected abstract SCMSourceCriteria getSCMSourceCriteria(SCMSource source);
 
     @Override public WorkflowJob newInstance(Branch branch) {
-        WorkflowJob job = new WorkflowJob((WorkflowMultiBranchProject) getOwner(), branch.getEncodedName());
+        WorkflowJob job = new WorkflowJob(getOwner(), branch.getEncodedName());
         setBranch(job, branch);
         return job;
     }

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/multibranch/DurabilityHintBranchProperty.java b/src/main/java/org/jenkinsci/plugins/workflow/multibranch/DurabilityHintBranchProperty.java
@@ -14,6 +14,7 @@
 import org.jenkinsci.plugins.workflow.flow.FlowDurabilityHint;
 import org.jenkinsci.plugins.workflow.flow.GlobalDefaultFlowDurabilityLevel;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
+import org.jenkinsci.plugins.workflow.job.properties.DurabilityHintJobProperty;
 import org.jenkinsci.Symbol;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
@@ -68,21 +69,22 @@ public static FlowDurabilityHint getDefaultDurabilityHint() {
             return GlobalDefaultFlowDurabilityLevel.getDefaultDurabilityHint();
         }
 
+        /** Lower ordinal than {@link DurabilityHintJobProperty} so those can override. */
         @Override
-        /** Lower ordinal than {@link org.jenkinsci.plugins.workflow.job.properties.DurabilityHintJobProperty} so those can override. */
         public int ordinal() {
             return 200;
         }
 
+        /**
+         * Dynamically fetch the property with each build, because the {@link BranchPropertyStrategy} does not re-evaluate.
+         * @see <a href="https://issues.jenkins-ci.org/browse/JENKINS-48826">JENKINS-48826</a>
+         */
         @CheckForNull
         @Override
-        /**
-         * Dynamically fetch the property with each build, because the {@link BranchPropertyStrategy} does not re-evaluate,
-         * resulting in {@see <a href="https://issues.jenkins-ci.org/browse/JENKINS-48826">JENKINS-48826</a>}. */
         public FlowDurabilityHint suggestFor(@Nonnull Item x) {
             // BranchJobProperty *should* be present if it's a child of a MultiBranchProject but we double-check for safety
             if (x instanceof WorkflowJob && x.getParent() instanceof MultiBranchProject && ((WorkflowJob)x).getProperty(BranchJobProperty.class) != null) {
-                MultiBranchProject mp  = (MultiBranchProject)(x.getParent());
+                MultiBranchProject mp = (MultiBranchProject) x.getParent();
                 WorkflowJob job = (WorkflowJob)x;
                 BranchJobProperty bjp = job.getProperty(BranchJobProperty.class);
 

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java b/src/main/java/org/jenkinsci/plugins/workflow/multibranch/JobPropertyStep.java
@@ -24,7 +24,6 @@
 
 package org.jenkinsci.plugins.workflow.multibranch;
 
-import com.google.common.util.concurrent.ListenableFuture;
 import hudson.AbortException;
 import hudson.BulkChange;
 import hudson.Extension;
@@ -56,7 +55,6 @@
 import org.jenkinsci.plugins.workflow.graphanalysis.DepthFirstScanner;
 import org.jenkinsci.plugins.workflow.graphanalysis.NodeStepTypePredicate;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
-import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl;
 import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl;
 import org.jenkinsci.plugins.workflow.steps.AbstractSynchronousStepExecution;
@@ -184,7 +182,7 @@ public DescriptorImpl() {
             // A modified version of RequestImpl.TypePair.convertJSON.
             // Works around the fact that Stapler does not call back into Descriptor.newInstance for nested objects (JENKINS-31458);
             // and propertiesMap virtual field name; and null values for unselected properties.
-            List<JobProperty> properties = new ArrayList<JobProperty>();
+            List<JobProperty> properties = new ArrayList<>();
             ClassLoader cl = req.getStapler().getWebApp().getClassLoader();
             @SuppressWarnings("unchecked") Set<Map.Entry<String,Object>> entrySet = formData.getJSONObject("propertiesMap").entrySet();
             for (Map.Entry<String,Object> e : entrySet) {
@@ -209,7 +207,7 @@ public DescriptorImpl() {
 
         @Restricted(DoNotUse.class) // f:repeatableHeteroProperty
         public Collection<? extends Descriptor<?>> getPropertyDescriptors() {
-            List<Descriptor<?>> result = new ArrayList<Descriptor<?>>();
+            List<Descriptor<?>> result = new ArrayList<>();
             for (JobPropertyDescriptor p : ExtensionList.lookup(JobPropertyDescriptor.class)) {
                 if (p.isApplicable(WorkflowJob.class)) {
                     result.add(p);

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/multibranch/ResolveScmStep.java b/src/main/java/org/jenkinsci/plugins/workflow/multibranch/ResolveScmStep.java
@@ -25,25 +25,21 @@
 
 package org.jenkinsci.plugins.workflow.multibranch;
 
-import edu.umd.cs.findbugs.annotations.CheckForNull;
-import edu.umd.cs.findbugs.annotations.NonNull;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 import hudson.AbortException;
 import hudson.Extension;
 import hudson.model.Descriptor;
-import hudson.model.Executor;
 import hudson.model.TaskListener;
 import hudson.scm.SCM;
 import hudson.util.FormValidation;
 import java.io.PrintStream;
-import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import jenkins.model.CauseOfInterruption;
 import jenkins.scm.api.SCMHead;
 import jenkins.scm.api.SCMHeadObserver;
 import jenkins.scm.api.SCMRevision;
@@ -61,8 +57,6 @@
 import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
 
-import static hudson.model.Result.ABORTED;
-
 /**
  * Resolves an {@link SCM} from a {@link SCMSource} using a priority list of target branch names.
  *
@@ -73,13 +67,13 @@ public class ResolveScmStep extends Step {
     /**
      * The {@link SCMSource}
      */
-    @NonNull
+    @Nonnull
     private final SCMSource source;
 
     /**
      * The {@link SCMSource}
      */
-    @NonNull
+    @Nonnull
     private final List<String> targets;
 
     /**
@@ -95,17 +89,17 @@ public class ResolveScmStep extends Step {
      * @param targets The {@link SCMSource}
      */
     @DataBoundConstructor
-    public ResolveScmStep(@NonNull SCMSource source, @NonNull List<String> targets) {
+    public ResolveScmStep(@Nonnull SCMSource source, @Nonnull List<String> targets) {
         this.source = source;
-        this.targets = new ArrayList<String>(targets);
+        this.targets = new ArrayList<>(targets);
     }
 
     /**
      * Gets the {@link SCMSource} to resolve from.
      *
      * @return the {@link SCMSource} to resolve from.
      */
-    @NonNull
+    @Nonnull
     public SCMSource getSource() {
         return source;
     }
@@ -115,7 +109,7 @@ public SCMSource getSource() {
      *
      * @return the {@link SCMHead} names to try and resolve.
      */
-    @NonNull
+    @Nonnull
     public List<String> getTargets() {
         return Collections.unmodifiableList(targets);
     }
@@ -170,7 +164,7 @@ public static class DescriptorImpl extends StepDescriptor {
          */
         @Override
         public Set<Class<?>> getRequiredContext() {
-            return Collections.<Class<?>>singleton(TaskListener.class);
+            return Collections.singleton(TaskListener.class);
         }
 
         /**
@@ -190,7 +184,7 @@ public String getDisplayName() {
         }
 
         @Override
-        public Step newInstance(@CheckForNull StaplerRequest req, @NonNull JSONObject formData)
+        public Step newInstance(@CheckForNull StaplerRequest req, @Nonnull JSONObject formData)
                 throws FormException {
             assert req != null : "see contract for method, it's never null but has to claim it could be";
             // roll our own because we want the groovy api to be easier than the jelly form binding would have us
@@ -238,13 +232,13 @@ public static class Execution extends SynchronousStepExecution<SCM> {
         /**
          * The {@link SCMSource}
          */
-        @NonNull
+        @Nonnull
         private transient final SCMSource source;
 
         /**
          * The {@link SCMSource}
          */
-        @NonNull
+        @Nonnull
         private final List<String> targets;
 
         /**
@@ -269,6 +263,7 @@ public static class Execution extends SynchronousStepExecution<SCM> {
         /**
          * {@inheritDoc}
          */
+        @Override
         protected SCM run() throws Exception {
             StepContext context = getContext();
             TaskListener listener = context.get(TaskListener.class);
@@ -297,14 +292,14 @@ private static class ObserverImpl extends SCMHeadObserver {
         /**
          * The heads we are looking for
          */
-        private final Map<String, SCMRevision> revision = new LinkedHashMap<String, SCMRevision>();
+        private final Map<String, SCMRevision> revision = new LinkedHashMap<>();
 
         /**
          * Constructor.
          *
          * @param heads the {@link SCMHead#getName()} to get the {@link SCMRevision} of.
          */
-        public ObserverImpl(@NonNull List<String> heads) {
+        public ObserverImpl(@Nonnull List<String> heads) {
             heads.getClass(); // fail fast if null
             for (String head : heads) {
                 if (StringUtils.isNotBlank(head)) {
@@ -332,7 +327,7 @@ public SCMRevision result() {
          * {@inheritDoc}
          */
         @Override
-        public void observe(@NonNull SCMHead head, @NonNull SCMRevision revision) {
+        public void observe(@Nonnull SCMHead head, @Nonnull SCMRevision revision) {
             if (this.revision.containsKey(head.getName())) {
                 this.revision.put(head.getName(), revision);
             }

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProject.java b/src/main/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProject.java
@@ -40,7 +40,6 @@
 
 import jenkins.branch.Branch;
 import jenkins.branch.BranchProjectFactory;
-import jenkins.branch.BranchProperty;
 import jenkins.branch.BranchPropertyStrategy;
 import jenkins.branch.BranchSource;
 import jenkins.branch.MultiBranchProject;
@@ -131,7 +130,7 @@ private BranchJobProperty reconstructBranchJobProperty(@Nonnull WorkflowJob job,
 
         final BranchPropertyStrategy strategy = source.getStrategy();
         return new BranchJobProperty(new Branch(sourceId, head, source.getSource().build(head),
-                strategy != null ? strategy.getPropertiesFor(head) : Collections.<BranchProperty>emptyList()));
+                strategy != null ? strategy.getPropertiesFor(head) : Collections.emptyList()));
     }
 
     @Extension public static class DescriptorImpl extends MultiBranchProjectDescriptor implements IconSpec {
@@ -140,11 +139,11 @@ private BranchJobProperty reconstructBranchJobProperty(@Nonnull WorkflowJob job,
             return Messages.WorkflowMultiBranchProject_DisplayName();
         }
 
-        public String getDescription() {
+        @Override public String getDescription() {
             return Messages.WorkflowMultiBranchProject_Description();
         }
 
-        public String getIconFilePathPattern() {
+        @Override public String getIconFilePathPattern() {
             return "plugin/workflow-multibranch/images/:size/pipelinemultibranchproject.png";
         }
 

diff --git a/...in/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectFactory.java b/...in/java/org/jenkinsci/plugins/workflow/multibranch/WorkflowMultiBranchProjectFactory.java
@@ -38,7 +38,7 @@
  * Defines organization folders by {@link WorkflowBranchProjectFactory}.
  */
 public class WorkflowMultiBranchProjectFactory extends AbstractWorkflowMultiBranchProjectFactory {
-    private String scriptPath = WorkflowBranchProjectFactory.SCRIPT;;
+    private String scriptPath = WorkflowBranchProjectFactory.SCRIPT;
 
     public Object readResolve() {
         if (this.scriptPath == null) {

diff --git a/.../org/jenkinsci/plugins/workflow/multibranch/DurabilityHintBranchPropertyWorkflowTest.java b/.../org/jenkinsci/plugins/workflow/multibranch/DurabilityHintBranchPropertyWorkflowTest.java
@@ -39,15 +39,12 @@
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.jenkinsci.plugins.workflow.job.properties.DurabilityHintJobProperty;
 import org.junit.Assert;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 
 import static org.jenkinsci.plugins.workflow.multibranch.WorkflowMultiBranchProjectTest.scheduleAndFindBranchProject;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
 
 /**
  * Integration test for {@link NoTriggerBranchProperty} and {@link NoTriggerOrganizationFolderProperty}.
@@ -65,7 +62,7 @@ public void configRoundtrip() throws Exception {
         mp.getSourcesList().add(bs);
         bs.setStrategy(new DefaultBranchPropertyStrategy(new BranchProperty[]{new DurabilityHintBranchProperty(FlowDurabilityHint.SURVIVABLE_NONATOMIC)}));
         r.configRoundtrip(mp);
-        DefaultBranchPropertyStrategy strat = (DefaultBranchPropertyStrategy)(mp.getBranchPropertyStrategy(mp.getSCMSources().get(0)));
+        DefaultBranchPropertyStrategy strat = (DefaultBranchPropertyStrategy) mp.getBranchPropertyStrategy(mp.getSCMSources().get(0));
         DurabilityHintBranchProperty prop = null;
         for (BranchProperty bp : strat.getProps()) {
             if (bp instanceof  DurabilityHintBranchProperty) {

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/multibranch/GitDirectorySCMNavigator.java b/src/test/java/org/jenkinsci/plugins/workflow/multibranch/GitDirectorySCMNavigator.java
@@ -24,7 +24,7 @@
 
 package org.jenkinsci.plugins.workflow.multibranch;
 
-import edu.umd.cs.findbugs.annotations.NonNull;
+import javax.annotation.Nonnull;
 import hudson.Extension;
 import hudson.Launcher;
 import hudson.Proc;
@@ -60,7 +60,7 @@ public String getDirectory() {
         return directory;
     }
 
-    @NonNull
+    @Nonnull
     @Override
     protected String id() {
         return directory;