feat: Add option to preserve directory structure for attachments (#165)

src/main/java/hudson/plugins/junitattachments/AttachmentPublisher.java

@@ -31,6 +31,7 @@ public class AttachmentPublisher extends TestDataPublisher {
 
     private Boolean showAttachmentsAtClassLevel = true;
     private Boolean showAttachmentsInStdOut = true;
+    private Boolean keepAttachmentsDirectories = false;
 
     @DataBoundConstructor
     public AttachmentPublisher() {
@@ -44,6 +45,10 @@ public boolean isShowAttachmentsInStdOut() {
         return showAttachmentsInStdOut != null ? showAttachmentsInStdOut : true;
     }
 
+    public boolean isKeepAttachmentsDirectories() {
+        return keepAttachmentsDirectories != null ? keepAttachmentsDirectories : false;
+    }
+
     @DataBoundSetter
     public void setShowAttachmentsAtClassLevel(Boolean showAttachmentsAtClassLevel) {
         this.showAttachmentsAtClassLevel = showAttachmentsAtClassLevel;
@@ -54,6 +59,11 @@ public void setShowAttachmentsInStdOut(Boolean showAttachmentsInStdOut) {
         this.showAttachmentsInStdOut = showAttachmentsInStdOut;
     }
 
+    @DataBoundSetter
+    public void setKeepAttachmentsDirectories(Boolean keepAttachmentsDirectories) {
+        this.keepAttachmentsDirectories = keepAttachmentsDirectories;
+    }
+
     public static FilePath getAttachmentPath(Run<?, ?> build) {
         return new FilePath(new File(build.getRootDir().getAbsolutePath()))
                 .child("junit-attachments");
@@ -76,13 +86,17 @@ public Data contributeTestData(Run<?, ?> build, FilePath workspace, Launcher lau
                                    TaskListener listener, TestResult testResult) throws IOException,
             InterruptedException {
         final GetTestDataMethodObject methodObject = new GetTestDataMethodObject(build, workspace, launcher, listener, testResult);
-        Map<String, Map<String, List<String>>> attachments = methodObject.getAttachments();
+        Map<String, Map<String, List<String>>> attachments = methodObject.getAttachments(isKeepAttachmentsDirectories());
 
         if (attachments.isEmpty()) {
             return null;
         }
 
-        return new Data(attachments, isShowAttachmentsAtClassLevel(), isShowAttachmentsInStdOut());
+        return new Data(
+            attachments,
+            isShowAttachmentsAtClassLevel(),
+            isShowAttachmentsInStdOut(),
+            isKeepAttachmentsDirectories());
     }
 
     public static class Data extends TestResultAction.Data {
@@ -92,6 +106,7 @@ public static class Data extends TestResultAction.Data {
         private Map<String, Map<String, List<String>>> attachmentsMap;
         private Boolean showAttachmentsAtClassLevel;
         private Boolean showAttachmentsInStdOut;
+        private Boolean keepAttachmentsDirectories;
 
         /**
          * @param attachmentsMap { fully-qualified test class name → { test method name → [ attachment file name ] } }
@@ -100,10 +115,12 @@ public static class Data extends TestResultAction.Data {
         public Data(
                 Map<String, Map<String, List<String>>> attachmentsMap,
                 Boolean showAttachmentsAtClassLevel,
-                Boolean showAttachmentsInStdOut) {
+                Boolean showAttachmentsInStdOut,
+                Boolean keepAttachmentsDirectories) {
             this.attachmentsMap = attachmentsMap;
             this.showAttachmentsAtClassLevel = showAttachmentsAtClassLevel;
             this.showAttachmentsInStdOut = showAttachmentsInStdOut;
+            this.keepAttachmentsDirectories = keepAttachmentsDirectories;
         }
 
         @Override
@@ -186,6 +203,10 @@ private Object readResolve() {
                 this.showAttachmentsInStdOut = true;
             }
 
+            if (this.keepAttachmentsDirectories == null) {
+                this.keepAttachmentsDirectories = false;
+            }
+
             if (attachments != null && attachmentsMap == null) {
                 // Migrate from the flat list per test class to a map of <test method, attachments>
                 attachmentsMap = new HashMap<String, Map<String, List<String>>>();

src/main/java/hudson/plugins/junitattachments/GetTestDataMethodObject.java

@@ -18,11 +18,17 @@
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -94,22 +100,22 @@
      * Returns a Map of classname vs. the stored attachments in a directory named as the test class.
      *
      * @return the map
-     * @throws InterruptedException
      * @throws IOException
      * @throws IllegalStateException
      * @throws InterruptedException
      *
      */
-    public Map<String, Map<String, List<String>>> getAttachments() throws IllegalStateException, IOException, InterruptedException {
+    public Map<String, Map<String, List<String>>> getAttachments(
+            boolean keepAttachmentsDirectories) throws IllegalStateException, IOException, InterruptedException {
         // build a map of className -> result xml file
-        Map<String, String> reports = getReports();
+        Map<String, String> reports = getReports(keepAttachmentsDirectories);
         LOG.fine("reports: " + reports);
         for (Map.Entry<String, String> report : reports.entrySet()) {
             final String className = report.getKey();
             final FilePath reportFile = workspace.child(report.getValue());
             final FilePath target = AttachmentPublisher.getAttachmentPath(attachmentsStorage, className, null);
             attachFilesForReport(className, reportFile, target);
-            attachStdInAndOut(className, reportFile);
+            attachStdInAndOut(className, reportFile, keepAttachmentsDirectories);
         }
         return attachments;
     }
@@ -136,7 +142,9 @@
     /**
      * Creates a map of the all classNames to their corresponding result file.
      */
-    private Map<String,String> getReports() throws IOException, InterruptedException {
+    private Map<String,String> getReports(boolean keepAttachmentsDirectories)
+            throws IOException, InterruptedException {
+
         Map<String,String> reports = new HashMap<String, String>();
         for (SuiteResult suiteResult : testResult.getSuites()) {
             String f = suiteResult.getFile();
@@ -160,12 +168,27 @@
 
                 // Add a newline so that we detect attachments if stdout has no trailing newline
                 // and stderr is null (as otherwise we'd try and parse "[[ATTACHMENT|foo]]null")
-                findAttachmentsInOutput(cr.getClassName(), cr.getName(), caseStdout + "\n" + caseStderr);
+                var testClassName = cr.getClassName();
+                var testCaseName = cr.getName();
+                captureAttachments(
+                    testClassName,
+                    testCaseName,
+                    findAttachmentsInOutput(testClassName, caseStdout + "\n" + caseStderr),
+                    keepAttachmentsDirectories);
             }
 
             // Capture stdout and stderr for the testsuite as a whole, if they exist
-            findAttachmentsInOutput(suiteResult.getName(), null, suiteStdout);
-            findAttachmentsInOutput(suiteResult.getName(), null, suiteStderr);
+            var suiteName = suiteResult.getName();
+
+            captureAttachments(
+                suiteName,
+                findAttachmentsInOutput(suiteName, suiteStdout),
+                keepAttachmentsDirectories);
+
+            captureAttachments(
+                suiteName,
+                findAttachmentsInOutput(suiteName, suiteStderr),
+                keepAttachmentsDirectories);
         }
         return reports;
     }
@@ -174,9 +197,12 @@
      * Finds attachments from a test's stdout/stderr, i.e. instances of:
      * <pre>[[ATTACHMENT|/path/to/attached-file.xyz|...reserved...]]</pre>
      */
-    private void findAttachmentsInOutput(String className, String testName, String output) throws IOException, InterruptedException {
+    private HashSet<FilePath> findAttachmentsInOutput(String className, String output) throws IOException, InterruptedException {
+
+        var outputAttachments = new LinkedHashSet<FilePath>();
+
         if (Util.fixEmpty(output) == null) {
-            return;
+            return outputAttachments;
         }
 
         Matcher matcher = ATTACHMENT_PATTERN.matcher(output);
@@ -190,64 +216,80 @@
             }
 
             String fileName = line;
-            if (fileName != null) {
-                FilePath src = workspace.child(fileName); // even though we use child(), this should be absolute
-                if (src.isDirectory()) {
-                    listener.getLogger().println("Attachment " + fileName + " was referenced from the test '" + className + "' but it is a directory, not a file. Skipping.");
-                } else if (src.exists()) {
-                    captureAttachment(className, testName, src);
-                } else {
-                    listener.getLogger().println("Attachment "+fileName+" was referenced from the test '"+className+"' but it doesn't exist. Skipping.");
-                }
+            FilePath src = workspace.child(fileName); // even though we use child(), this should be absolute
+            if (src.isDirectory()) {
+                listener.getLogger().println("Attachment " + fileName + " was referenced from the test '" + className + "' but it is a directory, not a file. Skipping.");
+            } else if (src.exists()) {
+                outputAttachments.add(src);
+            } else {
+                listener.getLogger().println("Attachment "+fileName+" was referenced from the test '"+className+"' but it doesn't exist. Skipping.");
             }
         }
+
+        return outputAttachments;
     }
 
     private static final String PREFIX = "[[ATTACHMENT|";
     private static final String SUFFIX = "]]";
     private static final Pattern ATTACHMENT_PATTERN = Pattern.compile("\\[\\[ATTACHMENT\\|.+\\]\\]");
 
     @SuppressFBWarnings(value = "NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE", justification = "TODO needs triage")
-    private void attachStdInAndOut(String className, FilePath reportFile)
+    private void attachStdInAndOut(String className, FilePath reportFile, boolean keepAttachmentsDirectories)
             throws IOException, InterruptedException {
-        final FilePath stdInAndOut = reportFile.getParent().child(
-                className + "-output.txt");
+        final FilePath stdInAndOut = reportFile.getParent().child(className + "-output.txt");
         LOG.fine("stdInAndOut: " + stdInAndOut.absolutize());
         if (stdInAndOut.exists()) {
-            captureAttachment(className, stdInAndOut);
+            captureAttachments(
+                    className,
+                    new HashSet<FilePath>(List.of(stdInAndOut)),
+                    keepAttachmentsDirectories);
         }
     }
 
     /**
      * Captures a single file as an attachment by copying it and recording it.
      *
-     * @param src
+     * @param filePaths
      *      File on the build workspace to be copied back to the controller and captured.
      */
-    private void captureAttachment(String className, FilePath src) throws IOException, InterruptedException {
-        captureAttachment(className, null, src);
+    private void captureAttachments(
+            String className,
+            Set<FilePath> filePaths,
+            boolean keepAttachmentsDirectories) throws IOException, InterruptedException {
+        captureAttachments(className, null, filePaths, keepAttachmentsDirectories);
     }
 
-    private void captureAttachment(String className, String testName, FilePath src) throws IOException, InterruptedException {
-        Map<String, List<String>> tests = attachments.get(className);
-        if (tests == null) {
-            tests = new HashMap<String, List<String>>();
-            attachments.put(className, tests);
-        }
-        List<String> testFiles = tests.get(Util.fixNull(testName));
-        if (testFiles == null) {
-            testFiles = new ArrayList<String>();
-            tests.put(Util.fixNull(testName), testFiles);
+    private void captureAttachments(
+            String className,
+            String testName,
+            Set<FilePath> filePaths,
+            boolean keepAttachmentsDirectories) throws IOException, InterruptedException {
+
+        if (filePaths == null || filePaths.isEmpty()) {
+            return;
         }
 
-        String filename = src.getName();
-        if (!testFiles.contains(filename)) {
-            // Only need to copy the file if it hasn't already been handled for this test class
-            FilePath target = AttachmentPublisher.getAttachmentPath(attachmentsStorage, className, testName);
-            target.mkdirs();
-            FilePath dst = new FilePath(target, filename);
-            src.copyTo(dst);
-            testFiles.add(filename);
+        Map<String, List<String>> tests = attachments.computeIfAbsent(className, k -> new HashMap<String, List<String>>());
+        List<String> testFiles = tests.computeIfAbsent(Util.fixNull(testName), k -> new ArrayList<String>());
+
+        var baseDirectory = keepAttachmentsDirectories ?
+                getCommonBaseDirectory(filePaths) :
+                null;
+
+        FilePath target = AttachmentPublisher.getAttachmentPath(attachmentsStorage, className, testName);
+        target.mkdirs();
+
+        for (FilePath filePath : filePaths) {
+            String relativeFilePath = keepAttachmentsDirectories ?
+                    getRelativePath(baseDirectory, filePath) :
+                    filePath.getName();
+
+            if (!testFiles.contains(relativeFilePath)) {
+                // Only need to copy the file if it hasn't already been handled for this test case
+                FilePath destinationPath = new FilePath(target, relativeFilePath);
+                filePath.copyTo(destinationPath);
+                testFiles.add(relativeFilePath);
+            }
         }
     }
 
@@ -261,4 +303,58 @@
         return false;
     }
 
+    private static String getRelativePath(FilePath base, FilePath target) {
+        Path basePath = Paths.get(base.getRemote()).toAbsolutePath().normalize();
+        Path targetPath = Paths.get(target.getRemote()).toAbsolutePath().normalize();
+
+        return basePath.relativize(targetPath).toString();
+    }
+
+    private static FilePath getCommonBaseDirectory(Set<FilePath> filePaths) {
+        if (filePaths == null || filePaths.isEmpty()) {
+            return null;
+        }
+
+        Iterator<FilePath> iterator = filePaths.iterator();
+
+        Path commonBase = Paths.get(iterator.next().getRemote()).toAbsolutePath().normalize();
+        if (filePaths.size() == 1) {
+            Path parent = commonBase.getParent();
+            if (parent == null) {
+                throw new IllegalStateException("Cannot determine base directory because the file path is a root path: " + commonBase);
+            }
+            return new FilePath(parent.toFile());
+        }
+
+        while (iterator.hasNext()) {
+            Path current = Paths.get(iterator.next().getRemote()).toAbsolutePath().normalize();
+            commonBase = commonPrefix(commonBase, current);
+
+            if (commonBase == null) {
+                break;
+            }
+        }
+
+        return commonBase == null ? null : new FilePath(commonBase.toFile());
+    }
+
+    private static Path commonPrefix(Path p1, Path p2) {
+        int minCount = Math.min(p1.getNameCount(), p2.getNameCount());
+        int i = 0;
+        while (i < minCount && p1.getName(i).equals(p2.getName(i))) {
+            i++;
+        }
+
+        if (i == 0) {
+            return null;
+        }
+
+        Path root = p1.getRoot();
+        if (root == null) {
+            throw new IllegalArgumentException("Expected absolute paths but got a relative path: " + p1);
+        }
+
+        return root.resolve(p1.subpath(0, i));
+    }
+
 }