Merge pull request #449 from jtnord/misc-cleanup

Misc cleanup
jenkinsci · Mar 15, 2023 · ef62610 · ef62610
2 parents a1733b7 + 59998d1
commit ef62610
Show file tree

Hide file tree

Showing 13 changed files with 196 additions and 155 deletions.
diff --git a/.mvn/jvm.config b/.mvn/jvm.config
@@ -0,0 +1 @@
+-Xmx256m
diff --git a/pom.xml b/pom.xml
@@ -152,11 +152,6 @@
         </exclusion>
       </exclusions>
     </dependency>
-    <dependency>
-      <groupId>com.google.guava</groupId>
-      <artifactId>guava</artifactId>
-      <version>31.1-jre</version>
-    </dependency>
     <dependency>
       <groupId>com.sun.codemodel</groupId>
       <artifactId>codemodel</artifactId>

diff --git a/src/main/java/org/jenkinsci/maven/plugins/hp/util/Utils.java b/src/main/java/org/jenkinsci/maven/plugins/hp/util/Utils.java
@@ -0,0 +1,21 @@
+package org.jenkinsci.maven.plugins.hp.util;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+public class Utils {
+
+    /**
+     * Return an unmodifiable set whose contents are the unions of all the specified sets.
+     */
+    @SafeVarargs
+    public static <T> Set<T> unionOf(Set<T>... sets) {
+        Set<T> unionSet = new HashSet<>(sets[0]);
+        for (int i = 1; i < sets.length; i++) {
+            unionSet.addAll(sets[i]);
+        }
+        return Collections.unmodifiableSet(unionSet);
+    }
+
+}
diff --git a/src/main/java/org/jenkinsci/maven/plugins/hpi/AbstractHpiMojo.java b/src/main/java/org/jenkinsci/maven/plugins/hpi/AbstractHpiMojo.java
@@ -16,20 +16,16 @@
  * limitations under the License.
  */
 
-import com.google.common.collect.Sets;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import edu.umd.cs.findbugs.annotations.CheckForNull;
 import hudson.Extension;
-import java.io.BufferedReader;
 import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
 import java.io.IOException;
-import java.io.InputStreamReader;
-import java.io.OutputStreamWriter;
+import java.io.InputStream;
 import java.io.Reader;
 import java.io.Writer;
 import java.net.URL;
 import java.net.URLClassLoader;
+import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.util.ArrayList;
@@ -40,8 +36,6 @@
 import java.util.Properties;
 import java.util.Set;
 import java.util.TreeSet;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 import jenkins.YesNoMaybe;
 import net.java.sezpoz.Index;
 import net.java.sezpoz.IndexItem;
@@ -63,6 +57,7 @@
 import org.codehaus.plexus.util.InterpolationFilterReader;
 import org.codehaus.plexus.util.PropertyUtils;
 import org.codehaus.plexus.util.StringUtils;
+import org.jenkinsci.maven.plugins.hp.util.Utils;
 
 public abstract class AbstractHpiMojo extends AbstractJenkinsMojo {
     /**
@@ -509,7 +504,7 @@ public void buildWebapp(MavenProject project, File webappDirectory)
         // List up IDs of Jenkins plugin dependencies
         Set<String> jenkinsPlugins = new HashSet<>();
         Set<String> excludedArtifacts = new HashSet<>();
-        for (MavenArtifact artifact : Sets.union(artifacts, dependencyArtifacts)) {
+        for (MavenArtifact artifact : Utils.unionOf(artifacts, dependencyArtifacts)) {
             if (artifact.isPluginBestEffort(getLog()))
                 jenkinsPlugins.add(artifact.getId());
             // Exclude dependency if it comes from test or provided trail.
@@ -791,37 +786,19 @@ public Reader getReader(Reader fileReader, Properties filterProperties) {
     private static void copyFilteredFile(File from, File to, String encoding, FilterWrapper[] wrappers,
                                          Properties filterProperties)
         throws IOException {
-        // buffer so it isn't reading a byte at a time!
-        Reader fileReader = null;
-        Writer fileWriter = null;
-        try {
-            // fix for MWAR-36, ensures that the parent dir are created first
-            Files.createDirectories(to.toPath().getParent());
-
-            if (encoding == null || encoding.length() < 1) {
-                fileReader = Files.newBufferedReader(from.toPath(), StandardCharsets.UTF_8);
-                fileWriter = Files.newBufferedWriter(to.toPath(), StandardCharsets.UTF_8);
-            } else {
-                FileInputStream instream = new FileInputStream(from);
-
-                FileOutputStream outstream = new FileOutputStream(to);
+        // fix for MWAR-36, ensures that the parent dir are created first
+        Files.createDirectories(to.toPath().getParent());
 
-                fileReader = new BufferedReader(new InputStreamReader(instream, encoding));
-
-                fileWriter = new OutputStreamWriter(outstream, encoding);
-            }
+        Charset cs = (encoding == null || encoding.length() < 1) ? StandardCharsets.UTF_8 : Charset.forName(encoding);
+        try (Reader fileReader = Files.newBufferedReader(from.toPath(), cs);
+             Writer fileWriter = Files.newBufferedWriter(to.toPath(), cs)) {
 
             Reader reader = fileReader;
             for (FilterWrapper wrapper : wrappers) {
                 reader = wrapper.getReader(reader, filterProperties);
             }
-
             IOUtil.copy(reader, fileWriter);
         }
-        finally {
-            IOUtil.close(fileReader);
-            IOUtil.close(fileWriter);
-        }
     }
 
     /**
@@ -842,7 +819,10 @@ public String getGitHeadSha1() {
         try {
             Process p = new ProcessBuilder("git", "-C", git.getAbsolutePath(), "rev-parse", "HEAD").redirectErrorStream(true).start();
             p.getOutputStream().close();
-            String v = IOUtils.toString(p.getInputStream()).trim();
+            String v;
+            try (InputStream is = p.getInputStream()) {
+                v = IOUtils.toString(is, Charset.defaultCharset()).trim();
+            }
             if (p.waitFor()!=0)
                 return null;    // git rev-parse failed to run
 
@@ -851,7 +831,7 @@ public String getGitHeadSha1() {
 
             return v.substring(0,8);
         } catch (IOException | InterruptedException e) {
-            LOGGER.log(Level.FINE, "Failed to run git rev-parse HEAD",e);
+            getLog().debug("Failed to run git rev-parse HEAD", e);
             return null;
         }
     }
@@ -869,21 +849,25 @@ private interface FilterWrapper {
      * False, if the answer is known to be "No". Otherwise null, if there are some extensions
      * we don't know we can dynamic load. Otherwise, if everything is known to be dynamic loadable, return true.
      */
-    @SuppressFBWarnings(value = "NP_BOOLEAN_RETURN_NULL", justification = "TODO needs triage")
+    @CheckForNull
     protected Boolean isSupportDynamicLoading() throws IOException {
-        URLClassLoader cl = new URLClassLoader(new URL[]{
-                new File(project.getBuild().getOutputDirectory()).toURI().toURL()
-        }, getClass().getClassLoader());
+        try (URLClassLoader cl = new URLClassLoader(new URL[]{
+                    new File(project.getBuild().getOutputDirectory()).toURI().toURL()
+            }, getClass().getClassLoader())) {
 
-        EnumSet<YesNoMaybe> e = EnumSet.noneOf(YesNoMaybe.class);
-        for (IndexItem<Extension,Object> i : Index.load(Extension.class, Object.class, cl)) {
-            e.add(i.annotation().dynamicLoadable());
-        }
+            EnumSet<YesNoMaybe> e = EnumSet.noneOf(YesNoMaybe.class);
+            for (IndexItem<Extension,Object> i : Index.load(Extension.class, Object.class, cl)) {
+                e.add(i.annotation().dynamicLoadable());
+            }
 
-        if (e.contains(YesNoMaybe.NO))  return false;
-        if (e.contains(YesNoMaybe.MAYBE))   return null;
-        return true;
+            if (e.contains(YesNoMaybe.NO)) {
+                return Boolean.FALSE;
+            }
+            if (e.contains(YesNoMaybe.MAYBE)) {
+                return null;
+            }
+            return Boolean.TRUE;
+        }
     }
 
-    private static final Logger LOGGER = Logger.getLogger(AbstractHpiMojo.class.getName());
 }
diff --git a/src/main/java/org/jenkinsci/maven/plugins/hpi/AbstractJenkinsManifestMojo.java b/src/main/java/org/jenkinsci/maven/plugins/hpi/AbstractJenkinsManifestMojo.java
@@ -42,8 +42,6 @@
 import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.List;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 /**
  * Abstract class for Mojo implementations, which produce Jenkins-style manifests.
@@ -56,8 +54,6 @@
  */
 public abstract class AbstractJenkinsManifestMojo extends AbstractHpiMojo {
 
-    private static final Logger LOGGER = Logger.getLogger(AbstractJenkinsManifestMojo.class.getName());
-
     /**
      * Optional - the oldest version of this plugin which the current version is
      * configuration-compatible with.
@@ -116,11 +112,10 @@ protected void generateManifest(MavenArchiveConfiguration archive, File manifest
     protected void setAttributes(Manifest.ExistingSection mainSection) throws MojoExecutionException, ManifestException, IOException {
         File pluginImpl = new File(project.getBuild().getOutputDirectory(), "META-INF/services/hudson.Plugin");
         if(pluginImpl.exists()) {
-            BufferedReader in = new BufferedReader(new InputStreamReader(new FileInputStream(pluginImpl), StandardCharsets.UTF_8));
-            String pluginClassName = in.readLine();
-            in.close();
-
-            mainSection.addAttributeAndCheck(new Manifest.Attribute("Plugin-Class",pluginClassName));
+            try (BufferedReader in = new BufferedReader(new InputStreamReader(new FileInputStream(pluginImpl), StandardCharsets.UTF_8))) {
+                String pluginClassName = in.readLine();
+                mainSection.addAttributeAndCheck(new Manifest.Attribute("Plugin-Class",pluginClassName));
+            }
         }
 
         mainSection.addAttributeAndCheck(new Manifest.Attribute("Group-Id",project.getGroupId()));

diff --git a/src/main/java/org/jenkinsci/maven/plugins/hpi/Artifacts.java b/src/main/java/org/jenkinsci/maven/plugins/hpi/Artifacts.java
@@ -14,6 +14,9 @@
  * @author Kohsuke Kawaguchi
  */
 public class Artifacts extends ArrayList<Artifact> {
+
+    private static final long serialVersionUID = 1L;
+
     public Artifacts() {
     }
 

diff --git a/src/main/java/org/jenkinsci/maven/plugins/hpi/ListPluginDependenciesMojo.java b/src/main/java/org/jenkinsci/maven/plugins/hpi/ListPluginDependenciesMojo.java
@@ -33,9 +33,7 @@ public class ListPluginDependenciesMojo extends AbstractHpiMojo {
 
     @Override
     public void execute() throws MojoExecutionException, MojoFailureException {
-        try {
-            Writer w = outputFile==null ? new NullWriter() : new OutputStreamWriter(new FileOutputStream(outputFile), StandardCharsets.UTF_8);
-
+        try (Writer w = outputFile==null ? new NullWriter() : new OutputStreamWriter(new FileOutputStream(outputFile), StandardCharsets.UTF_8)) {
             for (MavenArtifact a : getDirectDependencyArtfacts()) {
                 if(!a.isPlugin())
                     continue;
@@ -45,8 +43,6 @@ public void execute() throws MojoExecutionException, MojoFailureException {
                 w.write('\n');
                 getLog().info(line);
             }
-
-            w.close();
         } catch (IOException e) {
             throw new MojoExecutionException("Failed to list plugin dependencies",e);
         }

diff --git a/src/main/java/org/jenkinsci/maven/plugins/hpi/ReservedName.java b/src/main/java/org/jenkinsci/maven/plugins/hpi/ReservedName.java
@@ -1,6 +1,5 @@
 package org.jenkinsci.maven.plugins.hpi;
 
-import java.util.HashSet;
 import java.util.Set;
 
 /**

diff --git a/src/main/java/org/jenkinsci/maven/plugins/hpi/RunMojo.java b/src/main/java/org/jenkinsci/maven/plugins/hpi/RunMojo.java
@@ -449,17 +449,12 @@ private void copyPlugin(File src, File pluginsDir, String shortName) throws IOEx
         Files.writeString(pluginsDir.toPath().resolve(shortName + ".jpi.pinned"), "pinned", StandardCharsets.US_ASCII);
         Files.deleteIfExists(new File(pluginsDir, shortName + ".jpl").toPath()); // in case we used to have a snapshot dependency
     }
+
     private VersionNumber versionOfPlugin(File p) throws IOException {
         if (!p.isFile()) {
             return new VersionNumber("0.0");
         }
-        JarFile j;
-        try {
-            j = new JarFile(p);
-        } catch (IOException x) {
-            throw new IOException("not a valid JarFile: " + p, x);
-        }
-        try {
+        try (JarFile j = new JarFile(p)) {
             String v = j.getManifest().getMainAttributes().getValue("Plugin-Version");
             if (v == null) {
                 throw new IOException("no Plugin-Version in " + p);
@@ -469,8 +464,9 @@ private VersionNumber versionOfPlugin(File p) throws IOException {
             } catch (IllegalArgumentException x) {
                 throw new IOException("malformed Plugin-Version in " + p + ": " + x, x);
             }
-        } finally {
-            j.close();
+        }
+        catch (IOException x) {
+            throw new IOException("not a valid JarFile: " + p, x);
         }
     }
 
@@ -553,33 +549,25 @@ private boolean isExtractedWebAppDirStale(File extractedWebAppDir, File webApp)
             getLog().warn("no such file " + extractedPath);
             return false;
         }
-        InputStream is = new FileInputStream(extractedPath);
         String extractedVersion;
-        try {
+        try (InputStream is = new FileInputStream(extractedPath)) {
             extractedVersion = loadVersion(is);
-        } finally {
-            is.close();
         }
         if (extractedVersion == null) {
             getLog().warn("no " + VERSION_PROP + " in " + extractedPath);
             return false;
         }
-        ZipFile zip = new ZipFile(webApp);
         String originalVersion;
-        try {
+        try (ZipFile zip = new ZipFile(webApp)) {
             ZipEntry entry = zip.getEntry(VERSION_PATH);
             if (entry == null) {
                 getLog().warn("no " + VERSION_PATH + " in " + webApp);
                 return false;
             }
-            is = zip.getInputStream(entry);
-            try {
+
+            try (InputStream is = zip.getInputStream(entry);){
                 originalVersion = loadVersion(is);
-            } finally {
-                is.close();
             }
-        } finally {
-            zip.close();
         }
         if (originalVersion == null) {
             getLog().warn("no " + VERSION_PROP + " in jar:" + webApp.toURI() + "!/" + VERSION_PATH);