From 949d7f954f84e02e833be66e8f65553a0918de65 Mon Sep 17 00:00:00 2001
From: Alan Hoyle <alan@alanhoyle.com>
Date: Fri, 30 Apr 2021 13:56:02 -0400
Subject: [PATCH] R Chart/PDF output filenames escape '%' with '%%' (#1671)

* Allow for R Chart generated files to have '%' characters in their filenames
* Added specific test case  for this condition
---
 build_push_docker.sh                          |  2 +-
 .../CollectAlignmentSummaryMetrics.java       |  6 ++---
 .../CollectBaseDistributionByCycle.java       |  6 ++---
 .../picard/analysis/CollectGcBiasMetrics.java |  2 +-
 .../analysis/CollectInsertSizeMetrics.java    |  2 +-
 .../picard/analysis/CollectRnaSeqMetrics.java |  2 +-
 .../picard/analysis/CollectRrbsMetrics.java   |  2 +-
 .../CollectWgsMetricsWithNonZeroCoverage.java |  2 +-
 .../picard/analysis/MeanQualityByCycle.java   |  2 +-
 .../analysis/QualityScoreDistribution.java    |  2 +-
 .../CollectInsertSizeMetricsTest.java         | 25 ++++++++++++++++---
 11 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/build_push_docker.sh b/build_push_docker.sh
index 86e74553eb..fe6d37520c 100755
--- a/build_push_docker.sh
+++ b/build_push_docker.sh
@@ -1,7 +1,7 @@
 #!/usr/bin/env bash
 
 # This script is used to build and deploy the GCR docker image for Picard
-# The dockerhub iamge is built using a dockerhub automated build: https://hub.docker.com/r/broadinstitute/picard/builds
+# The dockerhub image is built using a dockerhub automated build: https://hub.docker.com/r/broadinstitute/picard/builds
 
 if [[ "$1" == "" ]]
 then
diff --git a/src/main/java/picard/analysis/CollectAlignmentSummaryMetrics.java b/src/main/java/picard/analysis/CollectAlignmentSummaryMetrics.java
index e769a53f50..a69c15c51b 100644
--- a/src/main/java/picard/analysis/CollectAlignmentSummaryMetrics.java
+++ b/src/main/java/picard/analysis/CollectAlignmentSummaryMetrics.java
@@ -52,7 +52,7 @@
 import java.util.Set;
 
 /**
- * A command line tool to read a BAM file and produce standard alignment metrics that would be applicable to any alignment.  
+ * A command line tool to read a BAM file and produce standard alignment metrics that would be applicable to any alignment.
  * Metrics to include, but not limited to:
  * <ul>
  * <li>Total number of reads (total, period, no exclusions)</li>
@@ -77,7 +77,7 @@
  * <li>the paired end orientation is different that the expected orientation</li>
  * <li>the read contains an SA tag (chimeric alignment)</li>
  * </ul>
- * 
+ *
  * @author Doug Voet (dvoet at broadinstitute dot org)
  */
 @CommandLineProgramProperties(
@@ -192,7 +192,7 @@ protected void setup(final SAMFileHeader header, final File samFile) {
 
         if (HISTOGRAM_FILE != null) {
             final List<String> plotArgs = new ArrayList<>();
-            Collections.addAll(plotArgs, OUTPUT.getAbsolutePath(), HISTOGRAM_FILE.getAbsolutePath(), INPUT.getName());
+            Collections.addAll(plotArgs, OUTPUT.getAbsolutePath(), HISTOGRAM_FILE.getAbsolutePath().replaceAll("%", "%%"), INPUT.getName());
 
             final int rResult = RExecutor.executeFromClasspath(HISTOGRAM_R_SCRIPT, plotArgs.toArray(new String[0]));
             if (rResult != 0) {
diff --git a/src/main/java/picard/analysis/CollectBaseDistributionByCycle.java b/src/main/java/picard/analysis/CollectBaseDistributionByCycle.java
index 53144e251b..1dc127d80b 100644
--- a/src/main/java/picard/analysis/CollectBaseDistributionByCycle.java
+++ b/src/main/java/picard/analysis/CollectBaseDistributionByCycle.java
@@ -73,9 +73,9 @@ public class CollectBaseDistributionByCycle extends SinglePassSamProgram {
                 "of the Base Recalibration (BQSR) pre-processing step of the "+
                 "<a href='https://www.broadinstitute.org/gatk/guide/best-practices'>GATK Best Practices for Variant Discovery</a>, "+
                 "which aims to correct some types of systematic biases that affect the accuracy of base quality scores."+
-                
+
                 "<p>Note: Metrics labeled as percentages are actually expressed as fractions!</p>"+
-                
+
                 "<h4>Usage example:</h4>" +
                 "<pre>" +
                 "java -jar picard.jar CollectBaseDistributionByCycle \\<br />" +
@@ -140,7 +140,7 @@ protected void finish() {
         } else {
             final int rResult = RExecutor.executeFromClasspath("picard/analysis/baseDistributionByCycle.R",
                     OUTPUT.getAbsolutePath(),
-                    CHART_OUTPUT.getAbsolutePath(),
+                    CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
                     INPUT.getName(),
                     plotSubtitle);
             if (rResult != 0) {
diff --git a/src/main/java/picard/analysis/CollectGcBiasMetrics.java b/src/main/java/picard/analysis/CollectGcBiasMetrics.java
index 0ef279e400..21def4cc81 100644
--- a/src/main/java/picard/analysis/CollectGcBiasMetrics.java
+++ b/src/main/java/picard/analysis/CollectGcBiasMetrics.java
@@ -211,7 +211,7 @@ private void writeResultsToFiles() {
         RExecutor.executeFromClasspath(R_SCRIPT,
                 OUTPUT.getAbsolutePath(),
                 SUMMARY_OUTPUT.getAbsolutePath(),
-                CHART_OUTPUT.getAbsolutePath(),
+                CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
                 String.valueOf(SCAN_WINDOW_SIZE));
     }
 }
diff --git a/src/main/java/picard/analysis/CollectInsertSizeMetrics.java b/src/main/java/picard/analysis/CollectInsertSizeMetrics.java
index ea47e69601..d6d7916a8a 100644
--- a/src/main/java/picard/analysis/CollectInsertSizeMetrics.java
+++ b/src/main/java/picard/analysis/CollectInsertSizeMetrics.java
@@ -172,7 +172,7 @@ protected String[] customCommandLineValidation() {
             file.write(OUTPUT);
 
             final List<String> plotArgs = new ArrayList<>();
-            Collections.addAll(plotArgs, OUTPUT.getAbsolutePath(), Histogram_FILE.getAbsolutePath(), INPUT.getName());
+            Collections.addAll(plotArgs, OUTPUT.getAbsolutePath(), Histogram_FILE.getAbsolutePath().replaceAll("%", "%%"), INPUT.getName());
 
             if (HISTOGRAM_WIDTH != null) {
                 plotArgs.add(String.valueOf(HISTOGRAM_WIDTH));
diff --git a/src/main/java/picard/analysis/CollectRnaSeqMetrics.java b/src/main/java/picard/analysis/CollectRnaSeqMetrics.java
index 7b4d7b6c9c..1a00e0b68f 100644
--- a/src/main/java/picard/analysis/CollectRnaSeqMetrics.java
+++ b/src/main/java/picard/analysis/CollectRnaSeqMetrics.java
@@ -202,7 +202,7 @@ protected void finish() {
         if (CHART_OUTPUT != null && atLeastOneHistogram) {
             final int rResult = RExecutor.executeFromClasspath("picard/analysis/rnaSeqCoverage.R",
                                                                OUTPUT.getAbsolutePath(),
-                                                               CHART_OUTPUT.getAbsolutePath(),
+                                                               CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
                                                                INPUT.getName(),
                                                                this.plotSubtitle);
 
diff --git a/src/main/java/picard/analysis/CollectRrbsMetrics.java b/src/main/java/picard/analysis/CollectRrbsMetrics.java
index eedd0b0f90..708b6d0db5 100644
--- a/src/main/java/picard/analysis/CollectRrbsMetrics.java
+++ b/src/main/java/picard/analysis/CollectRrbsMetrics.java
@@ -186,7 +186,7 @@ protected int doWork() {
         }
         summaryFile.write(SUMMARY_OUT);
         detailsFile.write(DETAILS_OUT);
-        RExecutor.executeFromClasspath(R_SCRIPT, DETAILS_OUT.getAbsolutePath(), SUMMARY_OUT.getAbsolutePath(), PLOTS_OUT.getAbsolutePath());
+        RExecutor.executeFromClasspath(R_SCRIPT, DETAILS_OUT.getAbsolutePath(), SUMMARY_OUT.getAbsolutePath(), PLOTS_OUT.getAbsolutePath().replaceAll("%", "%%"));
         CloserUtil.close(samReader);
         return 0;
     }
diff --git a/src/main/java/picard/analysis/CollectWgsMetricsWithNonZeroCoverage.java b/src/main/java/picard/analysis/CollectWgsMetricsWithNonZeroCoverage.java
index b1e6e189e2..35ee673524 100644
--- a/src/main/java/picard/analysis/CollectWgsMetricsWithNonZeroCoverage.java
+++ b/src/main/java/picard/analysis/CollectWgsMetricsWithNonZeroCoverage.java
@@ -154,7 +154,7 @@ protected int doWork() {
         } else {
             final int rResult = RExecutor.executeFromClasspath("picard/analysis/wgsHistogram.R",
                     OUTPUT.getAbsolutePath(),
-                    CHART_OUTPUT.getAbsolutePath(),
+                    CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
                     INPUT.getName(),
                     plotSubtitle);
             if (rResult != 0) {
diff --git a/src/main/java/picard/analysis/MeanQualityByCycle.java b/src/main/java/picard/analysis/MeanQualityByCycle.java
index a855675b13..535045f8ec 100644
--- a/src/main/java/picard/analysis/MeanQualityByCycle.java
+++ b/src/main/java/picard/analysis/MeanQualityByCycle.java
@@ -213,7 +213,7 @@ protected void finish() {
             final int rResult = RExecutor.executeFromClasspath(
                     "picard/analysis/meanQualityByCycle.R",
                     OUTPUT.getAbsolutePath(),
-                    CHART_OUTPUT.getAbsolutePath(),
+                    CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
                     INPUT.getName(),
                     plotSubtitle);
 
diff --git a/src/main/java/picard/analysis/QualityScoreDistribution.java b/src/main/java/picard/analysis/QualityScoreDistribution.java
index f7aaba4e79..db867ad18c 100644
--- a/src/main/java/picard/analysis/QualityScoreDistribution.java
+++ b/src/main/java/picard/analysis/QualityScoreDistribution.java
@@ -164,7 +164,7 @@ protected void finish() {
             final int rResult = RExecutor.executeFromClasspath(
                     "picard/analysis/qualityScoreDistribution.R",
                     OUTPUT.getAbsolutePath(),
-                    CHART_OUTPUT.getAbsolutePath(),
+                    CHART_OUTPUT.getAbsolutePath().replaceAll("%", "%%"),
                     INPUT.getName(),
                     this.plotSubtitle);
 
diff --git a/src/test/java/picard/analysis/CollectInsertSizeMetricsTest.java b/src/test/java/picard/analysis/CollectInsertSizeMetricsTest.java
index 709067670d..8563813d7a 100755
--- a/src/test/java/picard/analysis/CollectInsertSizeMetricsTest.java
+++ b/src/test/java/picard/analysis/CollectInsertSizeMetricsTest.java
@@ -264,6 +264,25 @@ public void testMultipleOrientationsForHistogram() throws IOException {
         Assert.assertEquals(rResult, 0);
     }
 
+    @Test
+    public void testPercentCharInPdfFilename() throws IOException {
+        final File input = new File(TEST_DATA_DIR, "insert_size_metrics_test.sam");
+        final File outfile   = File.createTempFile("test_%_char", ".insert_size_metrics");
+        final File pdf = File.createTempFile("test_%_char", ".pdf");
+        pdf.deleteOnExit();
+
+        final String[] args = new String[] {
+            "INPUT="  + input.getAbsolutePath(),
+            "OUTPUT=" + outfile.getAbsolutePath(),
+            "Histogram_FILE=" + pdf.getAbsolutePath(),
+            "LEVEL=SAMPLE",
+            "LEVEL=LIBRARY",
+            "LEVEL=READ_GROUP"
+        };
+        Assert.assertEquals(runPicardCommandLine(args), 0);
+        Assert.assertTrue(pdf.exists());
+    }
+
     @Test
     public void testWidthOfMetrics() throws IOException {
         final File testSamFile = File.createTempFile("CollectInsertSizeMetrics", ".bam");
@@ -321,11 +340,11 @@ public void testWidthOfMetrics() throws IOException {
 
         final MetricsFile<InsertSizeMetrics, Comparable<?>> output = new MetricsFile<InsertSizeMetrics, Comparable<?>>();
         output.read(new FileReader(outfile));
-        
+
         final List<InsertSizeMetrics> metrics = output.getMetrics();
-        
+
         Assert.assertEquals(metrics.size(), 1);
-        
+
         final InsertSizeMetrics metric = metrics.get(0);
 
         Assert.assertEquals(metric.PAIR_ORIENTATION.name(), "FR");