From: Javen O'Neal <onealj@apache.org>
Date: Mon, 4 Jul 2016 01:07:52 +0000 (+0000)
Subject: bug 59166: suggest alternative implementations for TempFileCreationStrategy
X-Git-Tag: REL_3_15_BETA3~213
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=b8ff07d06461b31ee8e6867daec51abfc87906d9;p=poi.git

bug 59166: suggest alternative implementations for TempFileCreationStrategy

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1751190 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/src/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java b/src/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java
index 0ed08c15d7..7e35d7e5d2 100644
--- a/src/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java
+++ b/src/java/org/apache/poi/util/DefaultTempFileCreationStrategy.java
@@ -9,13 +9,25 @@ import java.security.SecureRandom;
  * Files are collected into one directory and by default are deleted on exit from the VM.
  * Files may be manually deleted by user prior to JVM exit.
  * Files can be kept by defining the system property {@link #KEEP_FILES}.
+ * 
+ * Each file is registered for deletion with the JVM and the temporary directory is not deleted
+ * after the JVM exits. Files that are created in the poifiles directory outside
+ * the control of DefaultTempFileCreationStrategy are not deleted.
+ * See {@link TempFileCreationStrategy} for better strategies for long-running
+ * processes or limited temporary storage.
  */
 public class DefaultTempFileCreationStrategy implements TempFileCreationStrategy {
-    /** Define a constant for this property as it is sometimes mistypes as "tempdir" otherwise */
+    /** Define a constant for this property as it is sometimes mistypes as "tempdir" otherwise
+     * This is private to avoid having two public-visible constants ({@link TempFile#JAVA_IO_TMPDIR}). */
     private static final String JAVA_IO_TMPDIR = TempFile.JAVA_IO_TMPDIR;
+    /*package*/ static final String POIFILES = "poifiles";
+    
     /** To keep files after JVM exit, set the <code>-Dpoi.keep.tmp.files</code> JVM property */
     public static final String KEEP_FILES = "poi.keep.tmp.files";
     
+    /** random number generator to generate unique filenames */
+    private static final SecureRandom random = new SecureRandom();
+    
     /** The directory where the temporary files will be created (<code>null</code> to use the default directory). */
     private File dir;
     
@@ -47,12 +59,18 @@ public class DefaultTempFileCreationStrategy implements TempFileCreationStrategy
             if (tmpDir == null) {
                 throw new IOException("Systems temporary directory not defined - set the -D"+JAVA_IO_TMPDIR+" jvm property!");
             }
-            dir = new File(tmpDir, "poifiles");
+            dir = new File(tmpDir, POIFILES);
         }
         
         createTempDirectory(dir);
     }
     
+    /**
+     * Attempt to create a directory
+     *
+     * @param directory
+     * @throws IOException
+     */
     private void createTempDirectory(File directory) throws IOException {
         if (!(directory.exists() || directory.mkdirs()) || !directory.isDirectory()) {
             throw new IOException("Could not create temporary directory '" + directory + "'");
@@ -75,8 +93,8 @@ public class DefaultTempFileCreationStrategy implements TempFileCreationStrategy
         // All done
         return newFile;
     }
-    
-    private static final SecureRandom random = new SecureRandom();
+
+    /* (non-JavaDoc) Created directory path is <JAVA_IO_TMPDIR>/poifiles/prefix0123456789 */
     @Override
     public File createTempDirectory(String prefix) throws IOException {
         // Identify and create our temp dir, if needed
diff --git a/src/java/org/apache/poi/util/TempFileCreationStrategy.java b/src/java/org/apache/poi/util/TempFileCreationStrategy.java
index 28feca0c1d..d576f08857 100644
--- a/src/java/org/apache/poi/util/TempFileCreationStrategy.java
+++ b/src/java/org/apache/poi/util/TempFileCreationStrategy.java
@@ -22,6 +22,43 @@ import java.io.IOException;
 
 /**
  * Interface used by the {@link TempFile} utility class to create temporary files.
+ * 
+ * Classes that implement a TempFileCreationStrategy attempt to handle the cleanup
+ * of temporary files.
+ * 
+ * Examples include:
+ * <ul>
+ *   <li>{@link DefaultTempFileCreationStrategy} deletes temporary files when
+ *       the JVM exits.
+ *       This may not be suitable for long-running applications that never
+ *       shut down the JVM since the list of registered files and disk space
+ *       usage would grow for as long as the JVM is running.
+ *       You may wish to implement your own strategy that meets the needs of
+ *       your situation.
+ *   </li>
+ *   <li>A strategy that keeps the <code>n</code> most-recent files, discarding
+ *       older files on a first-in, first-out basis.
+ *       A java.util.Deque or org.apache.commons.collections4.queue.CircularFifoQueue
+ *       may be helpful for achieving this.
+ *   </li>
+ *   <li>A strategy that keeps track of every temporary file that has been
+ *       created by the class or instance and provides a method to explicitly
+ *       delete the temporary files in the reverse order that they were created.
+ *       This is the same as DefaultTempFileCreationStrategy, except the strategy
+ *       class would maintain the list of files to delete rather than or in
+ *       addition to {@link java.io.DeleteOnExitHook} maintaining the list, and
+ *       the files could be deleted before the JVM exit.
+ *   </li>
+ *   <li>A strategy that creates a directory that is deleted on JVM exit.
+ *       Any files inside the directory do not need to be registered since the
+ *       entire directory will be deleted at exit.
+ *       This could be dangerous if files were added to the temporary directory
+ *       outside of this TempFileCreationStrategy's control.
+ *       This could be accomplished with {@link #createTempDirectory(String)} and
+ *       creating regular (unregistered) files in the temp directory.
+ *   </li>
+ * </ul>
+ * 
  */
 public interface TempFileCreationStrategy {
     /**
@@ -44,6 +81,8 @@ public interface TempFileCreationStrategy {
      * @return The path to the newly created and empty temporary directory.
      * 
      * @throws IOException If no temporary directory could be created.
+     * 
+     * @since POI 3.15 beta 3.
      */
     File createTempDirectory(String prefix) throws IOException;
 }
diff --git a/src/testcases/org/apache/poi/util/TestTempFile.java b/src/testcases/org/apache/poi/util/TestTempFile.java
index 498b1daf8c..e893f00c52 100644
--- a/src/testcases/org/apache/poi/util/TestTempFile.java
+++ b/src/testcases/org/apache/poi/util/TestTempFile.java
@@ -20,7 +20,6 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 import java.io.File;
 import java.io.FileOutputStream;
@@ -57,7 +56,7 @@ public class TestTempFile {
         String[] files = tempDir.list();
         // can have the "poifiles" subdir
         if(files.length == 1) {
-            assertEquals("Had: " + Arrays.toString(files), "poifiles", files[0]);
+            assertEquals("Had: " + Arrays.toString(files), DefaultTempFileCreationStrategy.POIFILES, files[0]);
             files = new File(tempDir, files[0]).list();
             assertEquals("Had: " + Arrays.toString(files), 0, files.length);
         } else {
@@ -96,7 +95,7 @@ public class TestTempFile {
         assertTrue("temp file's name should end with .txt",
                 tempFile.getName().endsWith(".txt"));
         assertEquals("temp file is saved in poifiles directory",
-                "poifiles", tempFile.getParentFile().getName());
+                DefaultTempFileCreationStrategy.POIFILES, tempFile.getParentFile().getName());
 
         // Can't think of a good way to check whether a file is actually deleted since it would require the VM to stop.
         // Solution: set TempFileCreationStrategy to something that the unit test can trigger a deletion"
@@ -119,7 +118,7 @@ public class TestTempFile {
         assertTrue("testDir's name starts with testDir",
                 tempDir.getName().startsWith("testDir"));
         assertEquals("tempDir is saved in poifiles directory",
-                "poifiles", tempDir.getParentFile().getName());
+                DefaultTempFileCreationStrategy.POIFILES, tempDir.getParentFile().getName());
 
         // Can't think of a good way to check whether a directory is actually deleted since it would require the VM to stop.
         // Solution: set TempFileCreationStrategy to something that the unit test can trigger a deletion"