FindBugs: Cleanup issues after the core dependency upgrade

pom.xml

@@ -15,7 +15,7 @@
   <url>http://wiki.jenkins-ci.org/display/JENKINS/Promoted+Builds+Plugin</url>
 
   <properties>
-    <findbugs-maven-plugin.version>3.0.1</findbugs-maven-plugin.version>
+    <findbugs-maven-plugin.version>3.0.3</findbugs-maven-plugin.version>
     <findbugs.failOnError>true</findbugs.failOnError>
   </properties>
 

src/main/java/hudson/plugins/promoted_builds/Promotion.java

@@ -20,6 +20,7 @@
 import hudson.model.Run;
 import hudson.model.User;
 import hudson.plugins.promoted_builds.conditions.ManualCondition;
+import hudson.plugins.promoted_builds.util.JenkinsHelper;
 import hudson.security.Permission;
 import hudson.security.PermissionGroup;
 import hudson.security.PermissionScope;
@@ -53,8 +54,7 @@
  *
  * @author Kohsuke Kawaguchi
  */
-public class Promotion extends AbstractBuild<PromotionProcess,Promotion> 
-	implements Comparable<Promotion>{
+public class Promotion extends AbstractBuild<PromotionProcess,Promotion> {
 
     public Promotion(PromotionProcess job) throws IOException {
         super(job);
@@ -102,7 +102,7 @@ public EnvVars getEnvironment(TaskListener listener) throws IOException, Interru
         EnvVars e = super.getEnvironment(listener);
 
         // Augment environment with target build's information
-        String rootUrl = Jenkins.getInstance().getRootUrl();
+        String rootUrl = JenkinsHelper.getInstance().getRootUrl();
         AbstractBuild<?, ?> target = getTarget();
         if(rootUrl!=null)
             e.put("PROMOTED_URL",rootUrl+target.getUrl());
@@ -183,7 +183,7 @@ public String getUserName() {
         for (PromotionBadge badget : getStatus().getBadges()) {
             if (badget instanceof ManualCondition.Badge) {
                 final String nameFromBadge = ((ManualCondition.Badge) badget).getUserName();
-                if (nameFromBadge != null) {
+                if (!nameFromBadge.equals(ManualCondition.MISSING_USER_ID_DISPLAY_STRING)) {
                     return nameFromBadge;
                 }
             }
@@ -221,7 +221,7 @@ public String getUserId() {
         for (PromotionBadge badget : getStatus().getBadges()) {
             if (badget instanceof ManualCondition.Badge) {
                 final String idFromBadge = ((ManualCondition.Badge) badget).getUserId();
-                if (idFromBadge != null) {
+                if (!idFromBadge.equals(ManualCondition.MISSING_USER_ID_DISPLAY_STRING)) {
                     return idFromBadge;
                 }
             }
@@ -287,7 +287,13 @@ protected Lease decideWorkspace(Node n, WorkspaceList wsl) throws InterruptedExc
                 return Lease.createDummyLease(
                         rootPath.child(getEnvironment(listener).expand(customWorkspace)));
             }
-            return wsl.acquire(n.getWorkspaceFor((TopLevelItem)getTarget().getProject()),true);
+
+            TopLevelItem item = (TopLevelItem)getTarget().getProject();
+            FilePath workspace = n.getWorkspaceFor(item);
+            if (workspace == null) {
+                throw new IOException("Cannot retrieve workspace for " + item + " on the node " + n);
+            }
+            return wsl.acquire(workspace, true);
         }
 
         protected Result doRun(BuildListener listener) throws Exception {
@@ -353,7 +359,7 @@ protected void post2(BuildListener listener) throws Exception {
                 }
 
                 // tickle PromotionTriggers
-                for (AbstractProject<?,?> p : Jenkins.getInstance().getAllItems(AbstractProject.class)) {
+                for (AbstractProject<?,?> p : JenkinsHelper.getInstance().getAllItems(AbstractProject.class)) {
                     PromotionTrigger pt = p.getTrigger(PromotionTrigger.class);
                     if (pt!=null)
                         pt.consider(Promotion.this);
@@ -403,11 +409,6 @@ private boolean preBuild(BuildListener listener, List<BuildStep> steps) {
     public static final PermissionGroup PERMISSIONS = new PermissionGroup(Promotion.class, Messages._Promotion_Permissions_Title());
     public static final Permission PROMOTE = new Permission(PERMISSIONS, "Promote", Messages._Promotion_PromotePermission_Description(), Jenkins.ADMINISTER, PermissionScope.RUN);
 
-    @Override
-    public int compareTo(Promotion that) {
-    	return Integer.compare(that.getNumber(),this.getNumber());
-    }
-
     @Override
     public int hashCode() {
         return this.getId().hashCode();

src/main/java/hudson/plugins/promoted_builds/PromotionCondition.java

@@ -6,9 +6,11 @@
 import hudson.model.AbstractProject;
 import hudson.model.Describable;
 import hudson.model.Hudson;
+import hudson.plugins.promoted_builds.util.JenkinsHelper;
 
 import java.util.ArrayList;
 import java.util.List;
+import jenkins.model.Jenkins;
 
 /**
  * Extension point for defining a promotion criteria.
@@ -49,14 +51,14 @@ public PromotionBadge isMet(PromotionProcess promotionProcess, AbstractBuild<?,?
     }
 
     public PromotionConditionDescriptor getDescriptor() {
-        return (PromotionConditionDescriptor)Hudson.getInstance().getDescriptor(getClass());
+        return (PromotionConditionDescriptor)JenkinsHelper.getInstance().getDescriptor(getClass());
     }
 
     /**
      * Returns all the registered {@link PromotionConditionDescriptor}s.
      */
     public static DescriptorExtensionList<PromotionCondition,PromotionConditionDescriptor> all() {
-        return Hudson.getInstance().<PromotionCondition,PromotionConditionDescriptor>getDescriptorList(PromotionCondition.class);
+        return JenkinsHelper.getInstance().<PromotionCondition,PromotionConditionDescriptor>getDescriptorList(PromotionCondition.class);
     }
 
     /**

src/main/java/hudson/plugins/promoted_builds/PromotionProcess.java

@@ -1,6 +1,7 @@
 package hudson.plugins.promoted_builds;
 
 import antlr.ANTLRException;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.BulkChange;
 import hudson.Extension;
 import hudson.Util;
@@ -17,7 +18,6 @@
 import hudson.model.Descriptor.FormException;
 import hudson.model.Failure;
 import hudson.model.FreeStyleProject;
-import hudson.model.Hudson;
 import hudson.model.ItemGroup;
 import hudson.model.JDK;
 import hudson.model.Job;
@@ -30,6 +30,7 @@
 import hudson.model.labels.LabelAtom;
 import hudson.model.labels.LabelExpression;
 import hudson.plugins.promoted_builds.conditions.ManualCondition.ManualApproval;
+import hudson.plugins.promoted_builds.util.JenkinsHelper;
 import hudson.security.ACL;
 import hudson.tasks.BuildStep;
 import hudson.tasks.BuildStepDescriptor;
@@ -96,7 +97,7 @@ public final class PromotionProcess extends AbstractProject<PromotionProcess,Pro
     public static PromotionProcess fromJson(StaplerRequest req, JSONObject o) throws FormException, IOException {
         String name = o.getString("name");
         try {
-            Hudson.checkGoodName(name);
+            Jenkins.checkGoodName(name);
         } catch (Failure f) {
             throw new Descriptor.FormException(f.getMessage(), name);
         }
@@ -212,7 +213,7 @@ public String getAssignedLabelString() {
         // not just the same label.. but at least this works if job is tied to one node:
         if (assignedLabel == null) return getOwner().getAssignedLabel();
 
-        return Hudson.getInstance().getLabel(assignedLabel);
+        return JenkinsHelper.getInstance().getLabel(assignedLabel);
     }
 
     @Override public JDK getJDK() {
@@ -435,7 +436,7 @@ public Future<Promotion> scheduleBuild2(AbstractBuild<?,?> build, Cause cause) {
     }
 
     public boolean isInQueue(AbstractBuild<?,?> build) {
-        for (Item item : Hudson.getInstance().getQueue().getItems(this))
+        for (Item item : JenkinsHelper.getInstance().getQueue().getItems(this))
             if (item.getAction(PromotionTargetAction.class).resolve(this)==build)
                 return true;
         return false;
@@ -496,7 +497,7 @@ public String getId() {
     }
 
     public DescriptorImpl getDescriptor() {
-        return (DescriptorImpl)Jenkins.getInstance().getDescriptorOrDie(getClass());
+        return (DescriptorImpl)JenkinsHelper.getInstance().getDescriptorOrDie(getClass());
     }
 
     @Override
@@ -526,14 +527,14 @@ public FormValidation doCheckLabelString(@QueryParameter String value) {
                         Messages.JobPropertyImpl_LabelString_InvalidBooleanExpression(e.getMessage()));
             }
             // TODO: if there's an atom in the expression that is empty, report it
-            if (Hudson.getInstance().getLabel(value).isEmpty())
+            if (JenkinsHelper.getInstance().getLabel(value).isEmpty())
                 return FormValidation.warning(Messages.JobPropertyImpl_LabelString_NoMatch());
             return FormValidation.ok();
         }
 
         public AutoCompletionCandidates doAutoCompleteAssignedLabelString(@QueryParameter String value) {
             AutoCompletionCandidates c = new AutoCompletionCandidates();
-            Set<Label> labels = Hudson.getInstance().getLabels();
+            Set<Label> labels = JenkinsHelper.getInstance().getLabels();
             List<String> queries = new AutoCompleteSeeder(value).getSeeds();
 
             for (String term : queries) {
@@ -602,7 +603,7 @@ public List<Descriptor<? extends BuildStep>> getApplicableBuildSteps() {
             return PromotionProcess.getAll();
         }
 
-        // exposed for Jelly
+        @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD", justification = "exposed for Jelly")
         public final Class<PromotionProcess> promotionProcessType = PromotionProcess.class;
 
         public FormValidation doCheckName(@QueryParameter String name) {
@@ -612,7 +613,7 @@ public FormValidation doCheckName(@QueryParameter String name) {
             }
 
             try {
-                Hudson.checkGoodName(name);
+                Jenkins.checkGoodName(name);
             } catch (Failure f) {
                 return FormValidation.error(f.getMessage());
             }

src/main/java/hudson/plugins/promoted_builds/PromotionTargetAction.java

@@ -2,8 +2,8 @@
 
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
-import hudson.model.Hudson;
 import hudson.model.InvisibleAction;
+import hudson.plugins.promoted_builds.util.JenkinsHelper;
 
 /**
  * Remembers what build it's promoting. Attached to {@link Promotion}.
@@ -20,7 +20,7 @@ public PromotionTargetAction(AbstractBuild<?,?> build) {
     }
 
     public AbstractBuild<?,?> resolve() {
-        AbstractProject<?,?> j = Hudson.getInstance().getItemByFullName(jobName, AbstractProject.class);
+        AbstractProject<?,?> j = JenkinsHelper.getInstance().getItemByFullName(jobName, AbstractProject.class);
         if (j==null)    return null;
         return j.getBuildByNumber(number);
     }

src/main/java/hudson/plugins/promoted_builds/PromotionTrigger.java

@@ -5,6 +5,7 @@
 import hudson.model.AutoCompletionCandidates;
 import hudson.model.Item;
 import hudson.model.Job;
+import hudson.plugins.promoted_builds.util.JenkinsHelper;
 import hudson.triggers.Trigger;
 import hudson.triggers.TriggerDescriptor;
 import hudson.util.FormValidation;
@@ -46,8 +47,9 @@ public boolean appliesTo(PromotionProcess proc) {
     }
 
     public void consider(Promotion p) {
-        if (appliesTo(p.getParent()))
+        if (appliesTo(p.getParent()) && job != null) {
             job.scheduleBuild2(job.getQuietPeriod());
+        }
     }
 
     @Extension
@@ -72,19 +74,21 @@ public FormValidation doCheckJobName(@AncestorInPath Item project, @QueryParamet
             project.checkPermission(Item.CONFIGURE);
 
             if (StringUtils.isNotBlank(value)) {
-                AbstractProject p = Jenkins.getInstance().getItem(value,project,AbstractProject.class);
-                if(p==null)
-                    return FormValidation.error(hudson.tasks.Messages.BuildTrigger_NoSuchProject(value,
-                            AbstractProject.findNearest(value, project.getParent()).getRelativeNameFrom(project)));
-
+                AbstractProject p = JenkinsHelper.getInstance().getItem(value,project,AbstractProject.class);
+                if(p==null) {
+                    AbstractProject nearest = AbstractProject.findNearest(value, project.getParent());
+                    return FormValidation.error( nearest != null 
+                            ? hudson.tasks.Messages.BuildTrigger_NoSuchProject(value, nearest.getRelativeNameFrom(project))
+                            : Messages.Shared_noSuchProject(value));
+                }
             }
 
             return FormValidation.ok();
         }
 
         public AutoCompletionCandidates doAutoCompleteJobName(@QueryParameter String value) {
             AutoCompletionCandidates candidates = new AutoCompletionCandidates();
-            List<AbstractProject> jobs = Jenkins.getInstance().getItems(AbstractProject.class);
+            List<AbstractProject> jobs = JenkinsHelper.getInstance().getItems(AbstractProject.class);
             for (AbstractProject job: jobs) {
                 if (job.getFullName().startsWith(value)) {
                     if (job.hasPermission(Item.READ)) {
@@ -106,7 +110,7 @@ public ListBoxModel doFillProcessItems(@AncestorInPath Item defaultJob, @QueryPa
 
             AbstractProject<?,?> j = null;
             if (jobName!=null)
-                j = Jenkins.getInstance().getItem(jobName,defaultJob,AbstractProject.class);
+                j = JenkinsHelper.getInstance().getItem(jobName,defaultJob,AbstractProject.class);
 
             ListBoxModel r = new ListBoxModel();
             if (j!=null) {

src/main/java/hudson/plugins/promoted_builds/conditions/DownstreamPassCondition.java

@@ -13,7 +13,6 @@
 import hudson.model.Cause.UpstreamCause;
 import hudson.model.Fingerprint;
 import hudson.model.Fingerprint.BuildPtr;
-import hudson.model.Hudson;
 import hudson.model.InvisibleAction;
 import hudson.model.Item;
 import hudson.model.ItemGroup;
@@ -26,6 +25,7 @@
 import hudson.plugins.promoted_builds.PromotionCondition;
 import hudson.plugins.promoted_builds.PromotionConditionDescriptor;
 import hudson.plugins.promoted_builds.PromotionProcess;
+import hudson.plugins.promoted_builds.util.JenkinsHelper;
 import jenkins.model.Jenkins;
 import net.sf.json.JSONObject;
 import org.kohsuke.stapler.AncestorInPath;
@@ -118,7 +118,7 @@ public PromotionBadge isMet(PromotionProcess promotionProcess, AbstractBuild<?,?
     public List<AbstractProject<?,?>> getJobList(ItemGroup context) {
         List<AbstractProject<?,?>> r = new ArrayList<AbstractProject<?,?>>();
         for (String name : Util.tokenize(jobs,",")) {
-            AbstractProject job = Hudson.getInstance().getItem(name.trim(), context, AbstractProject.class);
+            AbstractProject job = JenkinsHelper.getInstance().getItem(name.trim(), context, AbstractProject.class);
             if(job!=null)   r.add(job);
         }
         return r;
@@ -173,7 +173,7 @@ public PromotionCondition newInstance(StaplerRequest req, JSONObject formData) t
 
         public AutoCompletionCandidates doAutoCompleteJobs(@QueryParameter String value, @AncestorInPath AbstractProject project) {
             List<AbstractProject> downstreams = project.getDownstreamProjects();
-            List<Item> all = Jenkins.getInstance().getItems(Item.class);
+            List<Item> all = JenkinsHelper.getInstance().getItems(Item.class);
             List<String> candidatesDownstreams = Lists.newArrayList();
             List<String> candidatesOthers = Lists.newArrayList();
             for (Item i : all) {
@@ -219,7 +219,7 @@ public RunListenerImpl() {
         @Override
         public void onCompleted(AbstractBuild<?,?> build, TaskListener listener) {
             // this is not terribly efficient,
-            for(AbstractProject<?,?> j : Hudson.getInstance().getAllItems(AbstractProject.class)) {
+            for(AbstractProject<?,?> j : JenkinsHelper.getInstance().getAllItems(AbstractProject.class)) {
                 boolean warned = false; // used to avoid warning for the same project more than once.
 
                 JobPropertyImpl jp = j.getProperty(JobPropertyImpl.class);

src/main/java/hudson/plugins/promoted_builds/conditions/ManualCondition.java

@@ -30,6 +30,7 @@
 import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import javax.annotation.Nonnull;
 
 import javax.servlet.ServletException;
 
@@ -51,6 +52,11 @@
 public class ManualCondition extends PromotionCondition {
     private String users;
     private List<ParameterDefinition> parameterDefinitions = new ArrayList<ParameterDefinition>();
+
+    /**
+     * 
+     */
+    public final static String MISSING_USER_ID_DISPLAY_STRING = "N/A";
 
     public ManualCondition() {
     }
@@ -240,27 +246,36 @@ public Badge(List<ParameterValue> values) {
             this.values = values;
         }
 
+        /**
+         * Gets name of the user, who has promoted the build.
+         * @return User name or {@link #MISSING_USER_ID_DISPLAY_STRING} if the user cannot be determined
+         */
         @Exported
+        @Nonnull
         public String getUserName() {
             if (authenticationName == null)
-                return "N/A";
+                return MISSING_USER_ID_DISPLAY_STRING;
 
             User u = User.get(authenticationName, false, null);
             return u != null ? u.getDisplayName() : authenticationName;
         }
 
+        /**
+         * Gets ID of the user, who has promoted the build.
+         * @return User id or {@link #MISSING_USER_ID_DISPLAY_STRING} if the user cannot be determined
+         */
         @Exported
         public String getUserId() {
             if (authenticationName == null) {
-                return "N/A";
+                return MISSING_USER_ID_DISPLAY_STRING;
             }
 
             return authenticationName;
         }
 
         @Exported
         public List<ParameterValue> getParameterValues() {
-            return values != null ? values : Collections.EMPTY_LIST;
+            return values != null ? values : Collections.<ParameterValue>emptyList();
         }
 
         @Override