Merge pull request #61 from jhonnymertz/fixing_bug

Fixing temporary files not being removed
jhonnymertz · Mar 15, 2020 · 6170a69 · 6170a69
2 parents 2c89ca9 + 2330c91
commit 6170a69
Show file tree

Hide file tree

Showing 13 changed files with 138 additions and 59 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -0,0 +1,10 @@
+Requirements
+------------
+**[wkhtmltopdf](http://wkhtmltopdf.org/) must be installed and working on your system.**
+
+Running tests
+------------
+
+This repo has two kinds of tests: integration and unit test.
+
+Use `mvn test -B` to run only the unit tests.
diff --git a/README.md b/README.md
@@ -26,13 +26,13 @@ In your `pom.xml`:
 <dependency>
     <groupId>com.github.jhonnymertz</groupId>
     <artifactId>java-wkhtmltopdf-wrapper</artifactId>
-    <version>1.1.11-RELEASE</version>
+    <version>1.1.12-RELEASE</version>
 </dependency>
 ```
 
 Usage and Examples
 ------------
-```
+```java
 Pdf pdf = new Pdf();
 
 pdf.addPageFromString("<html><head><meta charset=\"utf-8\"></head><h1>Müller</h1></html>");
@@ -55,7 +55,7 @@ pdf.saveAs("output.pdf");
 
 ### Xvfb Support
 
-```
+```java
 XvfbConfig xc = new XvfbConfig();
 xc.addParams(new Param("--auto-servernum"), new Param("--server-num=1"));
 
@@ -71,9 +71,9 @@ pdf.saveAs("output.pdf");
 ### wkhtmltopdf exit codes
 
 wkhtmltopdf may return non-zero exit codes to denote warnings, you can now set the Pdf object to allow this:
-```
 
-Pdf pdf = new Pdf(wc);
+```java
+Pdf pdf = new Pdf();
 pdf.addPageFromUrl("http://www.google.com");
 
 pdf.setAllowMissingAssets();
@@ -83,6 +83,15 @@ pdf.setSuccessValues(Arrays.asList(0, 1));
 pdf.saveAs("output.pdf");
 ```
 
+### Cleaning up temporary files
+
+After the PDF generation, the library automatically cleans up the temporary files created. However, there may be situations in which the `Pdf` object is created but no PDF is generated. To avoid increasing the temp folder size and having problems, you can force the deletion of all temporary files created by the library by:
+
+```java
+Pdf pdf = new Pdf();
+pdf.cleanAllTempFiles();
+```
+
 This is not an official Wkhtmltopdf product
 ------------
 This library is not an official Wkhtmltopdf product. Support is available on a best-effort basis via github issue tracking. Pull requests are welcomed.
@@ -97,6 +106,8 @@ Known issues
 **Output of wkhtmltopdf is being added to resulting pdf** ([Issue #19](https://github.com/jhonnymertz/java-wkhtmltopdf-wrapper/issues/19))
 - Starting from 1.1.10-RELEASE version, there is a method `saveAsDirect(String path)`, which executes wkhtmltopdf passing the `path` as output for wkhtmltopdf, instead of the standard input `-`. This saves the results directly to the specified file `path`.
 
+**Because this library relies on `wkhtmltopdf`, it does not support concurrent PDF generations.**
+
 License
 ------------
 This project is available under MIT Licence.
diff --git a/pom.xml b/pom.xml
@@ -39,8 +39,8 @@
     </developers>
 
     <properties>
-        <maven.compiler.source>1.7</maven.compiler.source>
-        <maven.compiler.target>1.7</maven.compiler.target>
+        <maven.compiler.source>1.8</maven.compiler.source>
+        <maven.compiler.target>1.8</maven.compiler.target>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
     </properties>
 

diff --git a/src/main/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/Pdf.java b/src/main/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/Pdf.java
@@ -1,5 +1,6 @@
 package com.github.jhonnymertz.wkhtmltopdf.wrapper;
 
+import com.github.jhonnymertz.wkhtmltopdf.wrapper.configurations.FilenameFilterConfig;
 import com.github.jhonnymertz.wkhtmltopdf.wrapper.configurations.WrapperConfig;
 import com.github.jhonnymertz.wkhtmltopdf.wrapper.exceptions.PDFExportException;
 import com.github.jhonnymertz.wkhtmltopdf.wrapper.page.Page;
@@ -22,11 +23,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.UUID;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
-import java.util.concurrent.TimeUnit;
+import java.util.concurrent.*;
 
 /**
  * Represents a Pdf file
@@ -127,9 +124,9 @@ public void setTimeout(int timeout) {
 
     /**
      * wkhtmltopdf often returns 1 to indicate some assets can't be found,
-     *  this can occur for protocol less links or in other cases. Sometimes you
-     *  may want to reject these with an exception which is the default, but in other
-     *  cases the PDF is fine for your needs.  Call this method allow return values of 1.
+     * this can occur for protocol less links or in other cases. Sometimes you
+     * may want to reject these with an exception which is the default, but in other
+     * cases the PDF is fine for your needs.  Call this method allow return values of 1.
      */
     public void setAllowMissingAssets() {
         if (!successValues.contains(1)) {
@@ -145,7 +142,7 @@ public boolean getAllowMissingAssets() {
      * In standard process returns 0 means "ok" and any other value is an error.  However, wkhtmltopdf
      * uses the return value to also return warning information which you may decide to ignore (@see setAllowMissingAssets)
      *
-     * @param successValues  The full list of process return values you will accept as a 'success'.
+     * @param successValues The full list of process return values you will accept as a 'success'.
      */
     public void setSuccessValues(List<Integer> successValues) {
         this.successValues = successValues;
@@ -154,6 +151,7 @@ public void setSuccessValues(List<Integer> successValues) {
     /**
      * Sets the temporary folder to store files during the process
      * Default is provided by #File.createTempFile()
+     *
      * @param tempDirectory
      */
     public void setTempDirectory(File tempDirectory) {
@@ -162,6 +160,7 @@ public void setTempDirectory(File tempDirectory) {
 
     /**
      * Executes the wkhtmltopdf into standard out and captures the results.
+     *
      * @param path The path to the file where the PDF will be saved.
      * @return
      * @throws IOException
@@ -176,12 +175,13 @@ public File saveAs(String path) throws IOException, InterruptedException {
 
     /**
      * Executes the wkhtmltopdf saving the results directly to the specified file path.
+     *
      * @param path The path to the file where the PDF will be saved.
      * @return
      * @throws IOException
      * @throws InterruptedException
      */
-    public File saveAsDirect(String path)throws IOException, InterruptedException  {
+    public File saveAsDirect(String path) throws IOException, InterruptedException {
         File file = new File(path);
         outputFilename = file.getAbsolutePath();
         getPDF();
@@ -202,7 +202,7 @@ public byte[] getPDF() throws IOException, InterruptedException, PDFExportExcept
 
             process.waitFor();
 
-            if (!successValues.contains( process.exitValue() )) {
+            if (!successValues.contains(process.exitValue())) {
                 byte[] errorStream = getFuture(outputStreamToByteArray);
                 logger.error("Error while generating pdf: {}", new String(errorStream));
                 throw new PDFExportException(command, process.exitValue(), errorStream, getFuture(inputStreamToByteArray));
@@ -239,6 +239,8 @@ protected String[] getCommandAsArray() throws IOException {
             if (page.getType().equals(PageType.htmlAsString)) {
                 //htmlAsString pages are first store into a temp file, then the location is passed as parameter to
                 // wkhtmltopdf, this is a workaround to avoid huge commands
+                if (page.getFilePath() != null)
+                    Files.deleteIfExists(Paths.get(page.getFilePath()));
                 File temp = File.createTempFile("java-wkhtmltopdf-wrapper" + UUID.randomUUID().toString(), ".html", tempDirectory);
                 FileUtils.writeStringToFile(temp, page.getSource(), "UTF-8");
                 page.setFilePath(temp.getAbsolutePath());
@@ -247,7 +249,7 @@ protected String[] getCommandAsArray() throws IOException {
                 commandLine.add(page.getSource());
             }
         }
-        commandLine.add( (null != outputFilename) ? outputFilename : STDINOUT);
+        commandLine.add((null != outputFilename) ? outputFilename : STDINOUT);
         logger.debug("Command generated: {}", commandLine.toString());
         return commandLine.toArray(new String[commandLine.size()]);
     }
@@ -268,6 +270,9 @@ private byte[] getFuture(Future<byte[]> future) {
         }
     }
 
+    /**
+     * CLeans up temporary files used to generate the wkhtmltopdf command
+     */
     private void cleanTempFiles() {
         logger.debug("Cleaning up temporary files...");
         for (Page page : pages) {
@@ -282,8 +287,24 @@ private void cleanTempFiles() {
         }
     }
 
+    /**
+     * Cleans up all the files generated by the library.
+     */
+    public void cleanAllTempFiles() {
+        logger.debug("Cleaning up temporary files...");
+        final File folder = new File(System.getProperty("java.io.tmpdir"));
+        final File[] files = folder.listFiles(new FilenameFilterConfig());
+        for (final File file : files) {
+            if (!file.delete()) {
+                logger.warn("Couldn't delete temp file " + file.getAbsolutePath());
+            }
+        }
+        logger.debug("{} temporary files removed.", files.length);
+    }
+
     /**
      * Gets the final wkhtmltopdf command as string
+     *
      * @return the generated command from params
      * @throws IOException
      */

diff --git a/.../java/com/github/jhonnymertz/wkhtmltopdf/wrapper/configurations/FilenameFilterConfig.java b/.../java/com/github/jhonnymertz/wkhtmltopdf/wrapper/configurations/FilenameFilterConfig.java
@@ -0,0 +1,14 @@
+package com.github.jhonnymertz.wkhtmltopdf.wrapper.configurations;
+
+import java.io.File;
+import java.io.FilenameFilter;
+
+/**
+ * Implements a filter to get files generated by the library
+ */
+public class FilenameFilterConfig implements FilenameFilter {
+    @Override
+    public boolean accept(File dir, String name) {
+        return name.matches("java-wkhtmltopdf-wrapper.*");
+    }
+}
diff --git a/src/main/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/configurations/WrapperConfig.java b/src/main/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/configurations/WrapperConfig.java
@@ -46,25 +46,6 @@ public WrapperConfig(String wkhtmltopdfCommand) {
         setWkhtmltopdfCommand(wkhtmltopdfCommand);
     }
 
-    /**
-     * Gets the wkhtmltopdf command to be used while calling wkhtmltopdf
-     * It's default is 'wkhtmltopdf'
-     *
-     * @return the wkhtmltopdf command
-     */
-    public String getWkhtmltopdfCommand() {
-        return wkhtmltopdfCommand;
-    }
-
-    /**
-     * Sets the configuration based on a provided wkhtmltopdf command to be used
-     *
-     * @param wkhtmltopdfCommand the wkhtmltopdf command
-     */
-    public void setWkhtmltopdfCommand(String wkhtmltopdfCommand) {
-        this.wkhtmltopdfCommand = wkhtmltopdfCommand;
-    }
-
     /**
      * Attempts to find the `wkhtmltopdf` executable in the system path.
      *
@@ -93,6 +74,25 @@ public static String findExecutable() {
         }
     }
 
+    /**
+     * Gets the wkhtmltopdf command to be used while calling wkhtmltopdf
+     * It's default is 'wkhtmltopdf'
+     *
+     * @return the wkhtmltopdf command
+     */
+    public String getWkhtmltopdfCommand() {
+        return wkhtmltopdfCommand;
+    }
+
+    /**
+     * Sets the configuration based on a provided wkhtmltopdf command to be used
+     *
+     * @param wkhtmltopdfCommand the wkhtmltopdf command
+     */
+    public void setWkhtmltopdfCommand(String wkhtmltopdfCommand) {
+        this.wkhtmltopdfCommand = wkhtmltopdfCommand;
+    }
+
     /**
      * Verify whether the Xvfb support is enabled and configured
      *

diff --git a/src/main/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/configurations/XvfbConfig.java b/src/main/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/configurations/XvfbConfig.java
@@ -11,8 +11,8 @@
  */
 public class XvfbConfig {
 
-    private String command;
     private final Params params = new Params();
+    private String command;
 
     public XvfbConfig() {
         this("xvfb-run");

diff --git a/.../github/jhonnymertz/wkhtmltopdf/wrapper/exceptions/WkhtmltopdfConfigurationException.java b/.../github/jhonnymertz/wkhtmltopdf/wrapper/exceptions/WkhtmltopdfConfigurationException.java
@@ -4,7 +4,7 @@
 /**
  * Exception to describe and track wrapper configuration errors
  */
-public class WkhtmltopdfConfigurationException extends RuntimeException  {
+public class WkhtmltopdfConfigurationException extends RuntimeException {
 
     public WkhtmltopdfConfigurationException(String s) {
         super(s);

diff --git a/src/main/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/page/Page.java b/src/main/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/page/Page.java
@@ -27,11 +27,11 @@ public void setType(PageType type) {
         this.type = type;
     }
 
-    public void setFilePath(String filePath) {
-        this.filePath = filePath;
-    }
-
     public String getFilePath() {
         return filePath;
     }
+
+    public void setFilePath(String filePath) {
+        this.filePath = filePath;
+    }
 }
diff --git a/src/main/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/params/Params.java b/src/main/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/params/Params.java
@@ -1,10 +1,11 @@
 package com.github.jhonnymertz.wkhtmltopdf.wrapper.params;
 
+import org.apache.commons.lang3.StringUtils;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
-import org.apache.commons.lang3.StringUtils;
 
 public class Params {
 
@@ -16,7 +17,7 @@ public Params() {
 
     public void add(Param param, Param... params) {
         this.params.add(param);
-        this.params.addAll( Arrays.asList( params ) );
+        this.params.addAll(Arrays.asList(params));
     }
 
     public List<String> getParamsAsStringList() {
@@ -37,7 +38,7 @@ public List<String> getParamsAsStringList() {
 
     @Override
     public String toString() {
-      return StringUtils.join(params, "");
+        return StringUtils.join(params, "");
     }
 
 }
diff --git a/...test/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/integration/PdfIntegrationTests.java b/...test/java/com/github/jhonnymertz/wkhtmltopdf/wrapper/integration/PdfIntegrationTests.java
@@ -22,8 +22,8 @@ public void findExecutable() throws Exception {
         WrapperConfig wc = new WrapperConfig();
         //see if executable is installed
         try {
-            wc.findExecutable();
-        }catch(RuntimeException ex){
+            WrapperConfig.findExecutable();
+        } catch (RuntimeException ex) {
             Assert.fail(ex.getMessage());
         }
     }
@@ -96,12 +96,12 @@ private String getPdfTextFromBytes(byte[] pdfBytes) throws IOException {
     }
 
     @Test
-    public void CleanUpTempFilesTest(){
+    public void CleanUpTempFilesTest() {
         Pdf pdf = new Pdf();
         pdf.addPageFromString("<!DOCTYPE html><head><title>title</title></head><body><p>TEST</p></body>");
         try {
             pdf.getPDF();
-        } catch(Exception ex){
+        } catch (Exception ex) {
             Assert.fail(ex.getMessage());
         }
     }