Bug 68183: SXSSFWorkbook should dispose of temporary files when close() is called

poi-ooxml/src/main/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java

@@ -100,6 +100,10 @@ Licensed to the Apache Software Foundation (ASF) under one or more
  *
  * 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 CellStyle getCellStyleAt(int idx) {
  }
 
  /**
- * 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 void close() throws IOException {
  }
  }
 
+ // 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 @@ protected void flushSheets() throws IOException {
  /**
  * 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()) {

poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java

@@ -355,6 +355,25 @@ void workbookDispose(boolean compressTempFiles) throws IOException {
  }
  }
 
+ @Test
+ void workbookTempFilesAreDisposedWhenClosingWorkbook() throws IOException {
+ SXSSFWorkbook wb = new SXSSFWorkbook();
+
+ // Closing / auto-closing the workbook should clean up the temp files
+ try (wb) {
+ 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
+ }
+
+ 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)) {

poi-ooxml/src/test/java/org/apache/poi/xssf/streaming/TestSXSSFWorkbookWithCustomZipEntrySource.java

@@ -114,7 +114,6 @@ void validateTempFilesAreEncrypted() throws IOException {
  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 @@ void validateTempFilesAreEncrypted() throws IOException {
  }
  workbook.dispose();
  assertFalse(tempFile.exists(), "tempFile deleted after dispose?");
+ workbook.close();
  }
 }