[bug-68183] SXSSFWorkbook now removes temp files when closed - removing need for a separate dispose call. Thanks to Clayton Bodendein. This closes #586

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

3 files changed, 37 insertions, 3 deletions

diff --git a/poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java b/poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java
index 84b6fba166..da64a880f2 100644
--- a/poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java
+++ b/poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java
@@ -100,6 +100,10 @@ import org.apache.poi.xssf.usermodel.XSSFWorkbook;
  *
  * Carefully review your memory budget and compatibility needs before deciding
  * whether to enable shared strings or not.
+ *
+ * <p>To release resources used by this workbook (including disposing of the temporary
+ * files backing this workbook on disk) {@link #close} should be called directly or a
+ * try-with-resources statement should be used.</p>
  */
 public class SXSSFWorkbook implements Workbook {
     /**
@@ -904,8 +908,9 @@ public class SXSSFWorkbook implements Workbook {
     }
 
     /**
-     * Closes the underlying {@link XSSFWorkbook} and {@link OPCPackage}
-     *  on which this Workbook is based, if any.
+     * Disposes of the temporary files backing this workbook on disk and closes the
+     * underlying {@link XSSFWorkbook} and {@link OPCPackage} on which this Workbook is
+     * based, if any.
      *
      * <p>Once this has been called, no further
      *  operations, updates or reads should be performed on the
@@ -923,6 +928,8 @@ public class SXSSFWorkbook implements Workbook {
             }
         }
 
+        // Dispose of any temporary files backing this workbook on disk
+        dispose();
 
         // Tell the base workbook to close, does nothing if
         //  it's a newly created one
@@ -1001,8 +1008,14 @@ public class SXSSFWorkbook implements Workbook {
     /**
      * Dispose of temporary files backing this workbook on disk.
      * Calling this method will render the workbook unusable.
+     *
+     * <p>The {@link #close()} method will also dispose of the temporary files so
+     * explicitly calling this method is unnecessary if the workbook will get closed.</p>
+     *
      * @return true if all temporary files were deleted successfully.
+     * @deprecated use {@link #close()} to close the workbook instead which also disposes of the temporary files
      */
+    @Deprecated
     public boolean dispose() {
         boolean success = true;
         for (SXSSFSheet sheet : _sxFromXHash.keySet()) {
diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java
index 1f57111839..6b723ff259 100644
--- a/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java
+++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java
@@ -356,6 +356,27 @@ public final class TestSXSSFWorkbook extends BaseTestXWorkbook {
     }
 
     @Test
+    void workbookTempFilesAreDisposedWhenClosingWorkbook() throws IOException {
+        SXSSFWorkbook wb = new SXSSFWorkbook();
+
+        // Closing / auto-closing the workbook should clean up the temp files
+        try {
+            populateData(wb);
+
+            for (int i = 0; i < wb.getNumberOfSheets(); i++) {
+                assertTrue(wb.getSheetAt(i).getSheetDataWriter().getTempFile().exists());
+            }
+            // Not calling SXSSFWorkbook#dispose since closing the workbook should clean up the temp files
+        } finally {
+            wb.close();
+        }
+
+        for (int i = 0; i < wb.getNumberOfSheets(); i++) {
+            assertFalse(wb.getSheetAt(i).getSheetDataWriter().getTempFile().exists());
+        }
+    }
+
+    @Test
     void bug53515() throws Exception {
         try (Workbook wb1 = new SXSSFWorkbook(10)) {
             populateWorkbook(wb1);
diff --git a/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbookWithCustomZipEntrySource.java b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbookWithCustomZipEntrySource.java
index d89ffbb2b3..26b683fdd0 100644
--- a/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbookWithCustomZipEntrySource.java
+++ b/poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbookWithCustomZipEntrySource.java
@@ -114,7 +114,6 @@ public final class TestSXSSFWorkbookWithCustomZipEntrySource {
         SXSSFCell cell1 = row1.createCell(1);
         cell1.setCellValue(cellValue);
         workbook.write(NullOutputStream.INSTANCE);
-        workbook.close();
         List<File> tempFiles = workbook.getTempFiles();
         assertEquals(1, tempFiles.size());
         File tempFile = tempFiles.get(0);
@@ -127,5 +126,6 @@ public final class TestSXSSFWorkbookWithCustomZipEntrySource {
         }
         workbook.dispose();
         assertFalse(tempFile.exists(), "tempFile deleted after dispose?");
+        workbook.close();
     }
 }
\ No newline at end of file