Remove unused parameters, use CachingWriter, simplify a bit (#79)

Co-authored-by: Elliotte Rusty Harold <elharo@ibiblio.org>

src/main/java/org/apache/maven/shared/archiver/MavenArchiver.java

@@ -540,14 +540,7 @@ public void doCreateArchive(Session session, Project project, MavenArchiveConfig
 
             new PomPropertiesUtil()
                     .createPomProperties(
-                            session,
-                            groupId,
-                            artifactId,
-                            version,
-                            archiver,
-                            customPomPropertiesFile,
-                            pomPropertiesFile,
-                            forced);
+                            groupId, artifactId, version, archiver, customPomPropertiesFile, pomPropertiesFile);
         }
 
         // ----------------------------------------------------------------------

src/main/java/org/apache/maven/shared/archiver/PomPropertiesUtil.java

@@ -18,23 +18,19 @@
  */
 package org.apache.maven.shared.archiver;
 
-import java.io.BufferedReader;
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.PrintWriter;
-import java.io.StringReader;
-import java.io.StringWriter;
+import java.io.Writer;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
 import java.util.Properties;
+import java.util.stream.Collectors;
 
 import org.apache.maven.api.Project;
-import org.apache.maven.api.Session;
 import org.codehaus.plexus.archiver.Archiver;
+import org.codehaus.plexus.util.io.CachingWriter;
 
 /**
  * This class is responsible for creating the <code>pom.properties</code> file
@@ -52,92 +48,54 @@ private Properties loadPropertiesFile(Path file) throws IOException {
         }
     }
 
-    private boolean sameContents(Properties props, Path file) throws IOException {
-        if (!Files.isRegularFile(file)) {
-            return false;
-        }
-
-        Properties fileProps = loadPropertiesFile(file);
-        return fileProps.equals(props);
-    }
-
-    private void createPropertiesFile(Properties properties, Path outputFile, boolean forceCreation)
-            throws IOException {
+    private void createPropertiesFile(Properties properties, Path outputFile) throws IOException {
         Path outputDir = outputFile.getParent();
-        if (outputDir != null && !Files.isDirectory(outputDir)) {
+        if (outputDir != null) {
             Files.createDirectories(outputDir);
         }
-        if (!forceCreation && sameContents(properties, outputFile)) {
-            return;
-        }
-
-        try (PrintWriter pw = new PrintWriter(outputFile.toFile(), StandardCharsets.ISO_8859_1.name());
-                StringWriter sw = new StringWriter()) {
-
-            properties.store(sw, null);
-
-            List<String> lines = new ArrayList<>();
-            try (BufferedReader r = new BufferedReader(new StringReader(sw.toString()))) {
-                String line;
-                while ((line = r.readLine()) != null) {
-                    if (!line.startsWith("#")) {
-                        lines.add(line);
-                    }
-                }
-            }
-
-            Collections.sort(lines);
-            for (String l : lines) {
-                pw.println(l);
-            }
+        // For reproducible builds, sort the properties and drop comments.
+        // The java.util.Properties class doesn't guarantee order so we have
+        // to write the file using a Writer.
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        properties.store(baos, null);
+        // The encoding can be either UTF-8 or ISO-8859-1, as any non ascii character
+        // is transformed into a \\uxxxx sequence anyway
+        String output = baos.toString(StandardCharsets.ISO_8859_1)
+                .lines()
+                .filter(line -> !line.startsWith("#"))
+                .sorted()
+                .collect(Collectors.joining("\n", "", "\n")); // system independent new line
+        try (Writer writer = new CachingWriter(outputFile, StandardCharsets.ISO_8859_1)) {
+            writer.write(output);
         }
     }
 
     /**
      * Creates the pom.properties file.
      *
-     * @param session {@link org.apache.maven.api.Session}
      * @param project {@link org.apache.maven.api.Project}
      * @param archiver {@link org.codehaus.plexus.archiver.Archiver}
      * @param customPomPropertiesFile optional custom pom properties file
      * @param pomPropertiesFile The pom properties file.
-     * @param forceCreation force creation true/false
      * @throws org.codehaus.plexus.archiver.ArchiverException archiver exception.
      * @throws java.io.IOException IO exception.
      */
     public void createPomProperties(
-            Session session,
-            Project project,
-            Archiver archiver,
-            Path customPomPropertiesFile,
-            Path pomPropertiesFile,
-            boolean forceCreation)
+            Project project, Archiver archiver, Path customPomPropertiesFile, Path pomPropertiesFile)
             throws IOException {
         final String groupId = project.getGroupId();
         final String artifactId = project.getArtifactId();
         final String version = project.getVersion();
-        createPomProperties(
-                session,
-                groupId,
-                artifactId,
-                version,
-                archiver,
-                customPomPropertiesFile,
-                pomPropertiesFile,
-                forceCreation);
+        createPomProperties(groupId, artifactId, version, archiver, customPomPropertiesFile, pomPropertiesFile);
     }
 
-    // CHECKSTYLE_OFF: ParameterNumber
     public void createPomProperties(
-            Session session,
             String groupId,
             String artifactId,
             String version,
             Archiver archiver,
             Path customPomPropertiesFile,
-            Path pomPropertiesFile,
-            boolean forceCreation)
-            // CHECKSTYLE_ON
+            Path pomPropertiesFile)
             throws IOException {
         Properties p;
 
@@ -153,7 +111,7 @@ public void createPomProperties(
 
         p.setProperty("version", version);
 
-        createPropertiesFile(p, pomPropertiesFile, forceCreation);
+        createPropertiesFile(p, pomPropertiesFile);
 
         archiver.addFile(
                 pomPropertiesFile.toFile(), "META-INF/maven/" + groupId + "/" + artifactId + "/pom.properties");

src/test/java/org/apache/maven/shared/archiver/PomPropertiesUtilTest.java

@@ -0,0 +1,119 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.shared.archiver;
+
+import java.io.IOException;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Properties;
+
+import org.codehaus.plexus.archiver.jar.JarArchiver;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+class PomPropertiesUtilTest {
+
+    private PomPropertiesUtil util = new PomPropertiesUtil();
+
+    @TempDir
+    Path tempDirectory;
+
+    @Test
+    void testCreatePomProperties() throws IOException {
+        Path pomPropertiesFile = tempDirectory.resolve("bar.properties");
+        util.createPomProperties("org.foo", "bar", "2.1.5", new JarArchiver(), null, pomPropertiesFile);
+
+        assertThat(pomPropertiesFile).exists();
+        Properties actual = new Properties();
+        actual.load(Files.newInputStream(pomPropertiesFile));
+        assertEquals("org.foo", actual.getProperty("groupId"));
+        assertEquals("bar", actual.getProperty("artifactId"));
+        assertEquals("2.1.5", actual.getProperty("version"));
+
+        // Now read the file directly to check for alphabetical order
+        List<String> contents = Files.readAllLines(pomPropertiesFile, StandardCharsets.ISO_8859_1);
+        assertEquals("artifactId=bar", contents.get(0));
+        assertEquals("groupId=org.foo", contents.get(1));
+        assertEquals("version=2.1.5", contents.get(2));
+        assertEquals(3, contents.size());
+    }
+
+    @Test
+    void testUnicodeEscape() throws IOException {
+        Path pomPropertiesFile = tempDirectory.resolve("bar.properties");
+        util.createPomProperties("org.foo", "こんにちは", "2.1.5", new JarArchiver(), null, pomPropertiesFile);
+
+        assertThat(pomPropertiesFile).exists();
+        Properties actual = new Properties();
+        actual.load(Files.newInputStream(pomPropertiesFile));
+        assertEquals("org.foo", actual.getProperty("groupId"));
+        assertEquals("こんにちは", actual.getProperty("artifactId"));
+        assertEquals("2.1.5", actual.getProperty("version"));
+
+        // Now read the file directly to check for alphabetical order and encoding
+        List<String> contents = Files.readAllLines(pomPropertiesFile, StandardCharsets.ISO_8859_1);
+        assertEquals("artifactId=\\u3053\\u3093\\u306B\\u3061\\u306F", contents.get(0));
+        assertEquals("groupId=org.foo", contents.get(1));
+        assertEquals("version=2.1.5", contents.get(2));
+        assertEquals(3, contents.size());
+    }
+
+    @Test
+    void testWhitespaceEscape() throws IOException {
+        Path pomPropertiesFile = tempDirectory.resolve("bar.properties");
+        Path customPomPropertiesFile = tempDirectory.resolve("custom.properties");
+        try (Writer out = Files.newBufferedWriter(customPomPropertiesFile, StandardCharsets.ISO_8859_1)) {
+            out.write("a\\u0020key\\u0020with\\u0009whitespace=value\\u0020with\\u0009whitespace\n");
+            out.write("zkey=value with \\\\ not at end of line\n");
+            out.write("ykey=\\tvalue with tab at beginning\n");
+            out.write("xkey=\\ value with whitespace at beginning\n");
+            out.write("wkey=\\u00E9\\u00FC\\u00E5\n");
+        }
+
+        util.createPomProperties(
+                "org.foo", "こんにちは", "2.1.5", new JarArchiver(), customPomPropertiesFile, pomPropertiesFile);
+        assertThat(pomPropertiesFile).exists();
+
+        Properties actual = new Properties();
+        actual.load(Files.newInputStream(pomPropertiesFile));
+        assertEquals("value with\twhitespace", actual.getProperty("a key with\twhitespace"));
+        assertEquals("value with \\ not at end of line", actual.getProperty("zkey"));
+        assertEquals("\tvalue with tab at beginning", actual.getProperty("ykey"));
+        assertEquals(" value with whitespace at beginning", actual.getProperty("xkey"));
+        assertEquals("éüå", actual.getProperty("wkey"));
+
+        // Now read the file directly to check for alphabetical order and encoding
+        List<String> contents = Files.readAllLines(pomPropertiesFile, StandardCharsets.ISO_8859_1);
+        assertEquals(8, contents.size());
+        assertEquals("a\\ key\\ with\\twhitespace=value with\\twhitespace", contents.get(0));
+        assertEquals("artifactId=\\u3053\\u3093\\u306B\\u3061\\u306F", contents.get(1));
+        assertEquals("groupId=org.foo", contents.get(2));
+        assertEquals("version=2.1.5", contents.get(3));
+        assertEquals("wkey=\\u00E9\\u00FC\\u00E5", contents.get(4));
+        assertEquals("xkey=\\ value with whitespace at beginning", contents.get(5));
+        assertEquals("ykey=\\tvalue with tab at beginning", contents.get(6));
+        assertEquals("zkey=value with \\\\ not at end of line", contents.get(7));
+    }
+}