Merge pull request #73 from jenkinsci/fix_spotbugs_issues

Fix spotbugs issues
jenkinsci · Feb 9, 2023 · 360a1d2 · 360a1d2
2 parents 6b37073 + 74dad74
commit 360a1d2
Show file tree

Hide file tree

Showing 13 changed files with 87 additions and 84 deletions.
diff --git a/src/main/java/hudson/plugins/disk_usage/BuildDiskUsageAction.java b/src/main/java/hudson/plugins/disk_usage/BuildDiskUsageAction.java
@@ -1,5 +1,6 @@
 package hudson.plugins.disk_usage;
 
+import hudson.FilePath;
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
 import hudson.model.Action;
@@ -40,11 +41,11 @@ public String getIconFileName() {
     }
 
     public String getDisplayName() {
-        return Messages.DisplayName();
+        return Messages.displayName();
     }
 
     public String getUrlName() {
-        return Messages.UrlName();
+        return Messages.urlName();
     }
 
     public void setDiskUsage(Long size) throws IOException {
@@ -123,9 +124,12 @@ public Object readResolve() {
             Node node = build.getBuiltOn();
             if(node != null && diskUsage.wsUsage != null && diskUsage.wsUsage > 0) {
                 DiskUsageProperty property = (DiskUsageProperty) build.getProject().getProperty(DiskUsageProperty.class);
-                AbstractProject project = build.getProject().getRootProject();
+                AbstractProject<?,?> project = build.getProject().getRootProject();
                 if(property != null && (project instanceof TopLevelItem)) {
-                    property.putAgentWorkspaceSize(node, node.getWorkspaceFor((TopLevelItem) project).getRemote(), diskUsage.wsUsage);
+                    final var workspaceFor = node.getWorkspaceFor((TopLevelItem) project);
+                    if (workspaceFor!=null){
+                        property.putAgentWorkspaceSize(node, workspaceFor.getRemote(), diskUsage.wsUsage);
+                    }
                 }
             }
             diskUsage = null;

diff --git a/src/main/java/hudson/plugins/disk_usage/DiskUsageBuildInformation.java b/src/main/java/hudson/plugins/disk_usage/DiskUsageBuildInformation.java
@@ -8,12 +8,13 @@
 import java.text.DateFormat;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
+import java.util.Objects;
 
 /**
  *
  * @author Lucie Votypkova
  */
-public class DiskUsageBuildInformation implements Serializable, Comparable {
+public class DiskUsageBuildInformation implements Serializable, Comparable<DiskUsageBuildInformation> {
 
     private static final long serialVersionUID = 1;
 
@@ -71,9 +72,15 @@ public boolean equals(Object o) {
         return false;
     }
 
-    public int compareTo(Object o) {
-        if(o instanceof DiskUsageBuildInformation) {
-            return id.compareTo(((DiskUsageBuildInformation) o).getId());
+    @Override
+    public int hashCode() {
+        return Objects.hashCode(id);
+    }
+
+    @Override
+    public int compareTo(DiskUsageBuildInformation o) {
+        if(o != null) {
+            return id.compareTo(o.getId());
         }
         throw new IllegalArgumentException("Can not compare with different type");
     }

diff --git a/src/main/java/hudson/plugins/disk_usage/DiskUsageManagement.java b/src/main/java/hudson/plugins/disk_usage/DiskUsageManagement.java
@@ -27,7 +27,7 @@ public String getIconFileName() {
     }
 
     public String getDisplayName() {
-        return Messages.DisplayName();
+        return Messages.displayName();
     }
 
     public String getUrlName() {
@@ -36,11 +36,10 @@ public String getUrlName() {
 
     @Override
     public String getDescription() {
-        return Messages.Description();
+        return Messages.description();
     }
 
     public void doIndex(StaplerRequest req, StaplerResponse res) throws ServletException, IOException {
-        DiskUsagePlugin plugin = Jenkins.getInstance().getPlugin(DiskUsagePlugin.class);
         res.sendRedirect(Jenkins.getInstance().getRootUrlFromRequest() + "plugin/disk-usage");
     }
 

diff --git a/src/main/java/hudson/plugins/disk_usage/DiskUsageProjectActionFactory.java b/src/main/java/hudson/plugins/disk_usage/DiskUsageProjectActionFactory.java
@@ -280,7 +280,7 @@ public String getValue(String size) {
 
         @Override
         public String getDisplayName() {
-            return Messages.DisplayName();
+            return Messages.displayName();
         }
 
 

diff --git a/src/main/java/hudson/plugins/disk_usage/DiskUsageProperty.java b/src/main/java/hudson/plugins/disk_usage/DiskUsageProperty.java
@@ -8,6 +8,7 @@
 import hudson.init.Initializer;
 import java.io.File;
 import java.io.IOException;
+import java.util.Map.Entry;
 import net.sf.json.JSONObject;
 import org.kohsuke.stapler.StaplerRequest;
 
@@ -215,9 +216,9 @@ public Long getWorkspaceSize(Boolean containdedInWorkspace) {
                 continue;
             }
             Map<String, Long> paths = getAgentWorkspaceUsage().get(nodeName);
-            for(String path: paths.keySet()) {
-                if(containdedInWorkspace.equals(path.startsWith(workspacePath))) {
-                    size += paths.get(path);
+            for(Entry<String, Long> entry : paths.entrySet()) {
+                if(containdedInWorkspace.equals(entry.getKey().startsWith(workspacePath))) {
+                    size += entry.getValue();
                 }
             }
         }
@@ -297,8 +298,10 @@ public void checkWorkspaces(boolean force) {
                             putAgentWorkspace(node, path.getRemote());
                         }
                     }
-                    catch (Exception e) {
-                        LOGGER.warning("Can not check if file " + path.getRemote() + " exists on node " + node.getNodeName());
+                    catch (IOException|InterruptedException e) {
+                        if (path != null) {
+                            LOGGER.warning("Can not check if file " + path.getRemote() + " exists on node " + node.getNodeName());
+                        }
                     }
                 }
             }
@@ -317,7 +320,7 @@ public void checkWorkspaces(boolean force) {
             }
             else {
                 // delete path which does not exists
-                if(node != null && node.toComputer() != null && node.getChannel() != null) {
+                if(node.toComputer() != null && node.getChannel() != null) {
                     Map<String, Long> workspaces = diskUsage.slaveWorkspacesUsage.get(nodeName);
                     Iterator<String> pathIterator = workspaces.keySet().iterator();
                     while(pathIterator.hasNext()) {
@@ -360,7 +363,7 @@ public Long getAllNonAgentOrCustomWorkspaceSize() {
                 continue;
             }
             Map<String, Long> paths = getAgentWorkspaceUsage().get(nodeName);
-            for(String path: paths.keySet()) {
+            for(Entry<String, Long> entry : paths.entrySet()) {
                 TopLevelItem item = null;
                 if(owner instanceof TopLevelItem) {
                     item = (TopLevelItem) owner;
@@ -369,8 +372,8 @@ public Long getAllNonAgentOrCustomWorkspaceSize() {
                     item = (TopLevelItem) owner.getParent();
                 }
                 try {
-                    if(!isContainedInWorkspace(item, node, path)) {
-                        size += paths.get(path);
+                    if(!isContainedInWorkspace(item, node, entry.getKey())) {
+                        size += entry.getValue();
                     }
                 }
                 catch (Exception e) {
@@ -405,13 +408,13 @@ private boolean isContainedInWorkspace(TopLevelItem item, Node node, String path
     public Long getAllWorkspaceSize() {
         Long size = 0L;
         for(String nodeName: getAgentWorkspaceUsage().keySet()) {
-            Node agent = Jenkins.getInstance().getNode(nodeName);
-            if(agent == null && !nodeName.isEmpty() && !(agent instanceof Jenkins)) {// agent does not exist
+            Node agent = Jenkins.get().getNode(nodeName);
+            if(agent == null && !nodeName.isEmpty()) {// agent does not exist
                 continue;
             }
             Map<String, Long> paths = getAgentWorkspaceUsage().get(nodeName);
-            for(String path: paths.keySet()) {
-                size += paths.get(path);
+            for(Entry<String, Long> entry : paths.entrySet()) {
+                size += entry.getValue();
             }
         }
         return size;
@@ -474,7 +477,6 @@ public void saveDiskUsage() {
     }
 
     public void loadDiskUsage() {
-        AbstractProject job = (AbstractProject) owner;
         diskUsage.load();
         // ensure that build was not removed without calling listener - badly removed, or badly saved (without build.xml)
         for(DiskUsageBuildInformation information: diskUsage.getBuildDiskUsage(false)) {
@@ -507,7 +509,7 @@ public DiskUsageDescriptor() {
 
         @Override
         public String getDisplayName() {
-            return Messages.DisplayName();
+            return Messages.displayName();
         }
 
         public boolean showGraph() {

diff --git a/src/main/java/hudson/plugins/disk_usage/DiskUsageUtil.java b/src/main/java/hudson/plugins/disk_usage/DiskUsageUtil.java
@@ -326,7 +326,7 @@ public static void calculationDiskUsageOfBuild(AbstractBuild build, TaskListener
                 property.saveDiskUsage();
             }
         }
-        catch (Exception ex) {
+        catch (IOException | InterruptedException ex) {
             listener.getLogger().println("Disk usage plugin fails during calculation disk usage of this build.");
             Logger.getLogger(DiskUsageUtil.class.getName()).log(Level.WARNING, "Disk usage plugin fails during build calculation disk space of job " + build.getParent().getDisplayName(), ex);
         }

diff --git a/src/main/java/hudson/plugins/disk_usage/ProjectDiskUsage.java b/src/main/java/hudson/plugins/disk_usage/ProjectDiskUsage.java
@@ -65,7 +65,6 @@ public boolean isBuildsLoaded() {
 
     public Set<DiskUsageBuildInformation> getBuildDiskUsage(boolean needAll) {
         Set<DiskUsageBuildInformation> information = new HashSet<>();
-        AbstractProject p = (AbstractProject) job;
         if(needAll && !allBuildsLoaded) {
             try {
                 loadAllBuilds();
@@ -185,12 +184,6 @@ public synchronized void load() {
         }
         try {
             file.unmarshal(this);
-            if(buildDiskUsage instanceof HashSet) {
-                // saved collection is not serialized in previous versions.
-                Set<DiskUsageBuildInformation> informations = new CopyOnWriteArraySet<>();
-                informations.addAll(buildDiskUsage);
-                buildDiskUsage = informations;
-            }
         } catch (IOException e) {
             Logger.getLogger(getClass().getName()).log(Level.WARNING, "Failed to load " + file, e);
         }

diff --git a/src/main/java/hudson/plugins/disk_usage/ProjectDiskUsageAction.java b/src/main/java/hudson/plugins/disk_usage/ProjectDiskUsageAction.java
@@ -43,11 +43,11 @@ public String getIconFileName() {
     }
 
     public String getDisplayName() {
-        return Messages.DisplayName();
+        return Messages.displayName();
     }
 
     public String getUrlName() {
-        return Messages.UrlName();
+        return Messages.urlName();
     }
 
     public Long getDiskUsageWorkspace() {
@@ -230,42 +230,42 @@ public Map<String, Long> getBuildsDiskUsage(Date older, Date yonger) throws IOEx
         Long buildsDiskUsage = 0L;
         Long locked = 0L;
         Long notLoaded = 0L;
-        if(project != null) {
-            if(project instanceof ItemGroup) {
-                ItemGroup group = (ItemGroup) project;
-                Map<String, Long> sizes = getBuildsDiskUsageAllSubItems(group, older, yonger);
-                buildsDiskUsage += sizes.get("all");
-                locked += sizes.get("locked");
-                notLoaded += sizes.get("notLoaded");
+
+        if(project instanceof ItemGroup) {
+            ItemGroup group = (ItemGroup) project;
+            Map<String, Long> sizes = getBuildsDiskUsageAllSubItems(group, older, yonger);
+            buildsDiskUsage += sizes.get("all");
+            locked += sizes.get("locked");
+            notLoaded += sizes.get("notLoaded");
+        }
+        Set<DiskUsageBuildInformation> informations = property.getDiskUsageOfBuilds();
+        for(DiskUsageBuildInformation information: informations) {
+            Date date = new Date(information.getTimestamp());
+            if(older != null && !date.before(older)) {
+                continue;
             }
-            Set<DiskUsageBuildInformation> informations = property.getDiskUsageOfBuilds();
-            for(DiskUsageBuildInformation information: informations) {
-                Date date = new Date(information.getTimestamp());
-                if(older != null && !date.before(older)) {
-                    continue;
-                }
-                if(yonger != null && !date.after(yonger)) {
-                    continue;
-                }
-                Long size = information.getSize();
-                buildsDiskUsage += size;
-                Collection<AbstractBuild> loadedBuilds = (Collection<AbstractBuild>) project._getRuns().getLoadedBuilds().values();
-                AbstractBuild build = null;
-                for(AbstractBuild b: loadedBuilds) {
-                    if(b.getId().equals(information.getId())) {
-                        build = b;
-                    }
-                }
-                if(build != null) {
-                    if(build.isKeepLog()) {
-                        locked += size;
-                    }
+            if(yonger != null && !date.after(yonger)) {
+                continue;
+            }
+            Long size = information.getSize();
+            buildsDiskUsage += size;
+            Collection<AbstractBuild> loadedBuilds = (Collection<AbstractBuild>) project._getRuns().getLoadedBuilds().values();
+            AbstractBuild build = null;
+            for(AbstractBuild b: loadedBuilds) {
+                if(b.getId().equals(information.getId())) {
+                    build = b;
                 }
-                else {
-                    notLoaded += size;
+            }
+            if(build != null) {
+                if(build.isKeepLog()) {
+                    locked += size;
                 }
             }
+            else {
+                notLoaded += size;
+            }
         }
+
         diskUsage.put("all", buildsDiskUsage);
         diskUsage.put("locked", locked);
         diskUsage.put("notLoaded", notLoaded);

diff --git a/src/main/resources/hudson/plugins/disk_usage/Messages.properties b/src/main/resources/hudson/plugins/disk_usage/Messages.properties
@@ -1,6 +1,6 @@
-DisplayName=Disk Usage
-UrlName=diskUsage
-Description=Displays per-project disk usage
+displayName=Disk Usage
+urlName=diskUsage
+description=Displays per-project disk usage
 
 DiskUsage.Graph.JobDiskUsageAxis=job, builds
 DiskUsage.Graph.WorkspaceDiskUsageAxis=workspaces

diff --git a/src/main/resources/hudson/plugins/disk_usage/Messages_fr.properties b/src/main/resources/hudson/plugins/disk_usage/Messages_fr.properties
@@ -1,4 +1,3 @@
-DisplayName=Utilisation du disque
-UrlName=diskUsage
-Description=Affiche l''utilisation du disque pour chaque projet
-ProjectDiskUsage=Utilisation du disque
+displayName=Utilisation du disque
+urlName=diskUsage
+description=Affiche l''utilisation du disque pour chaque projet
diff --git a/src/main/resources/hudson/plugins/disk_usage/Messages_ja.properties b/src/main/resources/hudson/plugins/disk_usage/Messages_ja.properties
@@ -20,7 +20,6 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
 
-DisplayName=\u30c7\u30a3\u30b9\u30af\u4f7f\u7528\u91cf
-UrlName=diskUsage
-Description=\u30c7\u30a3\u30b9\u30af\u4f7f\u7528\u91cf\u3092\u30d7\u30ed\u30b8\u30a7\u30af\u30c8\u3054\u3068\u306b\u8868\u793a\u3057\u307e\u3059\u3002
-ProjectDiskUsage=\u30c7\u30a3\u30b9\u30af\u4f7f\u7528\u91cf
+displayName=\u30c7\u30a3\u30b9\u30af\u4f7f\u7528\u91cf
+urlName=diskUsage
+description=\u30c7\u30a3\u30b9\u30af\u4f7f\u7528\u91cf\u3092\u30d7\u30ed\u30b8\u30a7\u30af\u30c8\u3054\u3068\u306b\u8868\u793a\u3057\u307e\u3059\u3002
diff --git a/src/main/resources/hudson/plugins/disk_usage/Messages_zh_TW.properties b/src/main/resources/hudson/plugins/disk_usage/Messages_zh_TW.properties
@@ -20,7 +20,6 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
 
-DisplayName=\u78c1\u789f\u7528\u91cf
-UrlName=diskUsage
-Description=\u986f\u793a\u500b\u5225\u5c08\u6848\u7684\u78c1\u789f\u7528\u91cf
-ProjectDiskUsage=\u78c1\u789f\u7528\u91cf
+displayName=\u78c1\u789f\u7528\u91cf
+urlName=diskUsage
+description=\u986f\u793a\u500b\u5225\u5c08\u6848\u7684\u78c1\u789f\u7528\u91cf
diff --git a/src/test/java/hudson/plugins/disk_usage/integration/DiskUsagePropertyTest.java b/src/test/java/hudson/plugins/disk_usage/integration/DiskUsagePropertyTest.java
@@ -12,6 +12,7 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
 import java.lang.annotation.Annotation;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRecipe;
 import hudson.XmlFile;
 import java.util.logging.Level;
@@ -529,7 +530,7 @@ private void checkForConcurrencyException(Exception exception) {
         fail("Checking of thread safety caused Exception which is not connected with thread safety problem.");
     }
 
-    // JENKINS-29143
+    @Issue("JENKINS-29143")
     @Test
     public void testThreadSaveOperationUnderSetOfDiskUsageBuildInformation() throws Exception {
         final FreeStyleProject project = j.createFreeStyleProject();