FOP-2903: Do not delete files on syntax errors using command line by Dave Roxburgh

3 files changed, 104 insertions, 12 deletions

diff --git a/fop-core/src/main/java/org/apache/fop/cli/CommandLineOptions.java b/fop-core/src/main/java/org/apache/fop/cli/CommandLineOptions.java
index f62b38670..8d824b960 100644
--- a/fop-core/src/main/java/org/apache/fop/cli/CommandLineOptions.java
+++ b/fop-core/src/main/java/org/apache/fop/cli/CommandLineOptions.java
@@ -102,6 +102,8 @@ public class CommandLineOptions {
     private File iffile;
     /* area tree input file */
     private File imagefile;
+    /* output filename */
+    private String outfilename;
     /* output file */
     private File outfile;
     /* input mode */
@@ -403,6 +405,9 @@ public class CommandLineOptions {
                 System.exit(1);
             }
         }
+        if (outfilename != null) {
+            outfile = new File(outfilename);
+        }
         return true;
     }    // end parseOptions
 
@@ -527,7 +532,7 @@ public class CommandLineOptions {
         if (isSystemInOutFile(filename)) {
             this.useStdOut = true;
         } else {
-            outfile = new File(filename);
+            outfilename = filename;
         }
     }
 
@@ -1351,15 +1356,15 @@ public class CommandLineOptions {
             log.info("not set");
         } else if (MimeConstants.MIME_FOP_AWT_PREVIEW.equals(outputmode)) {
             log.info("awt on screen");
-            if (outfile != null) {
+            if (outfilename != null) {
                 log.error("awt mode, but outfile is set:");
-                log.error("out file: " + outfile.toString());
+                log.error("out file: " + outfilename);
             }
         } else if (MimeConstants.MIME_FOP_PRINT.equals(outputmode)) {
             log.info("print directly");
-            if (outfile != null) {
+            if (outfilename != null) {
                 log.error("print mode, but outfile is set:");
-                log.error("out file: " + outfile.toString());
+                log.error("out file: " + outfilename);
             }
         } else if (MimeConstants.MIME_FOP_AREA_TREE.equals(outputmode)) {
             log.info("area tree");
@@ -1369,17 +1374,17 @@ public class CommandLineOptions {
             if (isOutputToStdOut()) {
                 log.info("output file: to stdout");
             } else {
-                log.info("output file: " + outfile.toString());
+                log.info("output file: " + outfilename);
             }
         } else if (MimeConstants.MIME_FOP_IF.equals(outputmode)) {
             log.info("intermediate format");
-            log.info("output file: " + outfile.toString());
+            log.info("output file: " + outfilename);
         } else {
             log.info(outputmode);
             if (isOutputToStdOut()) {
                 log.info("output file: to stdout");
             } else {
-                log.info("output file: " + outfile.toString());
+                log.info("output file: " + outfilename);
             }
         }
 
diff --git a/fop-core/src/main/java/org/apache/fop/cli/Main.java b/fop-core/src/main/java/org/apache/fop/cli/Main.java
index b8ba68df9..a14d20c78 100644
--- a/fop-core/src/main/java/org/apache/fop/cli/Main.java
+++ b/fop-core/src/main/java/org/apache/fop/cli/Main.java
@@ -146,14 +146,19 @@ public final class Main {
         }
     }
 
+    public static void startFOP(String[] args) {
+        startFOP(args, new SystemWrapper());
+    }
+
     /**
      * Executes FOP with the given arguments. If no argument is provided, returns its
      * version number as well as a short usage statement; if '-v' is provided, returns its
      * version number alone; if '-h' is provided, returns its short help message.
      *
      * @param args command-line arguments
+     * @param systemWrapper Object on which exit() is to be called.
      */
-    public static void startFOP(String[] args) {
+    public static void startFOP(String[] args, SystemWrapper systemWrapper) {
         //System.out.println("static CCL: "
         //    + Thread.currentThread().getContextClassLoader().toString());
         //System.out.println("static CL: " + Fop.class.getClassLoader().toString());
@@ -165,7 +170,7 @@ public final class Main {
             options = new CommandLineOptions();
             if (!options.parse(args)) {
                 // @SuppressFBWarnings("DM_EXIT")
-                System.exit(0);
+                systemWrapper.exit(0);
             }
 
             foUserAgent = options.getFOUserAgent();
@@ -192,7 +197,7 @@ public final class Main {
             // AWTRenderer closes with window shutdown, so exit() should not
             // be called here
             if (!MimeConstants.MIME_FOP_AWT_PREVIEW.equals(outputFormat)) {
-                System.exit(0);
+                systemWrapper.exit(0);
             }
         } catch (Exception e) {
             if (options != null) {
@@ -201,7 +206,7 @@ public final class Main {
                     options.getOutputFile().delete();
                 }
             }
-            System.exit(1);
+            systemWrapper.exit(1);
         }
     }
 
@@ -217,4 +222,12 @@ public final class Main {
         }
     }
 
+    /**
+     * Wrapper to support dependency injection.
+     */
+    public static class SystemWrapper {
+        public void exit(int status) {
+            System.exit(status);
+        }
+    }
 }
diff --git a/fop-core/src/test/java/org/apache/fop/cli/CommandLineOptions2TestCase.java b/fop-core/src/test/java/org/apache/fop/cli/CommandLineOptions2TestCase.java
new file mode 100644
index 000000000..518069154
--- /dev/null
+++ b/fop-core/src/test/java/org/apache/fop/cli/CommandLineOptions2TestCase.java
@@ -0,0 +1,74 @@
+/*
+ * 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.fop.cli;
+
+import java.io.File;
+import java.io.IOException;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import org.apache.fop.apps.FOPException;
+
+public class CommandLineOptions2TestCase {
+
+    @Rule
+    public ExpectedException exceptionRule = ExpectedException.none();
+
+    /**
+     * Check that the parser detects the case where the -xsl switch has been omitted.
+     * This detection prevents the XSL file being deleted.
+     * @throws FOPException
+     * @throws IOException
+     */
+    @Test
+    public void testXslSwitchMissing() throws FOPException, IOException {
+        final String xslFilename = "../fop/examples/embedding/xml/xslt/projectteam2fo.xsl";
+        final String outputFilename = "out.pdf";
+        exceptionRule.expect(FOPException.class);
+        exceptionRule.expectMessage("Don't know what to do with " + outputFilename);
+        // -xsl switch omitted.
+        String cmdLine = "-xml ../fop/examples/embedding/xml/xml/projectteam.xml " + xslFilename + " " + outputFilename;
+        String[] args = cmdLine.split(" ");
+        CommandLineOptions clo = new CommandLineOptions();
+        clo.parse(args);
+    }
+
+    /**
+     * Check that the XSL file is not deleted in the case where the -xsl switch has been omitted.
+     * @throws FOPException
+     * @throws IOException
+     */
+    @Test
+    public void testXslFileNotDeleted() {
+        Main.SystemWrapper mockSystemWrapper = mock(Main.SystemWrapper.class);
+        final String xslFilename = "../fop/examples/embedding/xml/xslt/projectteam2fo.xsl";
+        // -xsl switch omitted.
+        String cmdLine = "-xml ../fop/examples/embedding/xml/xml/projectteam.xml " + xslFilename + " out.pdf";
+        String[] args = cmdLine.split(" ");
+        Main.startFOP(args, mockSystemWrapper);
+        verify(mockSystemWrapper).exit(1);
+        File file = new File(xslFilename);
+        assertTrue(file.exists());
+    }
+}