Use IOUtils.closeQuietly() in more places

Avoid two possible file-handle leaks when opening files fails with an exception
Make tests close resources properly to not spam the output when running with file-leak-detector

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

src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java

@@ -54,6 +54,7 @@ import org.apache.poi.openxml4j.opc.internal.unmarshallers.PackagePropertiesUnma
 import org.apache.poi.openxml4j.opc.internal.unmarshallers.UnmarshallContext;
 import org.apache.poi.openxml4j.util.Nullable;
 import org.apache.poi.openxml4j.util.ZipEntrySource;
+import org.apache.poi.util.IOUtils;
 import org.apache.poi.util.NotImplemented;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
@@ -221,18 +222,10 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
            // pack.originalPackagePath = file.getAbsolutePath();
            return pack;
        } catch (InvalidFormatException e) {
-           try {
-               pack.close();
-           } catch (IOException e1) {
-               throw new IllegalStateException(e);
-           }
+		   IOUtils.closeQuietly(pack);
            throw e;
        } catch (RuntimeException e) {
-           try {
-               pack.close();
-           } catch (IOException e1) {
-               throw new IllegalStateException(e);
-           }
+		   IOUtils.closeQuietly(pack);
            throw e;
        }
    }
@@ -277,6 +270,7 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
 				}
 			}
 		}
+
 		pack.originalPackagePath = new File(path).getAbsolutePath();
 		return pack;
 	}
@@ -310,18 +304,10 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
 		   pack.originalPackagePath = file.getAbsolutePath();
 		   return pack;
 	   } catch (InvalidFormatException e) {
-		   try {
-			   pack.close();
-		   } catch (IOException e1) {
-			   throw new IllegalStateException(e);
-		   }
+		   IOUtils.closeQuietly(pack);
 		   throw e;
 	   } catch (RuntimeException e) {
-		   try {
-			   pack.close();
-		   } catch (IOException e1) {
-			   throw new IllegalStateException(e);
-		   }
+		   IOUtils.closeQuietly(pack);
 		   throw e;
 	   }
    }
@@ -340,8 +326,16 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
 	public static OPCPackage open(InputStream in) throws InvalidFormatException,
 			IOException {
 		OPCPackage pack = new ZipPackage(in, PackageAccess.READ_WRITE);
-		if (pack.partList == null) {
-			pack.getParts();
+		try {
+			if (pack.partList == null) {
+				pack.getParts();
+			}
+		} catch (InvalidFormatException e) {
+			IOUtils.closeQuietly(pack);
+			throw e;
+		} catch (RuntimeException e) {
+			IOUtils.closeQuietly(pack);
+			throw e;
 		}
 		return pack;
 	}
@@ -357,13 +351,11 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
 	 *             Throws if the specified file exist and is not valid.
 	 */
 	public static OPCPackage openOrCreate(File file) throws InvalidFormatException {
-		OPCPackage retPackage = null;
 		if (file.exists()) {
-			retPackage = open(file.getAbsolutePath());
+			return open(file.getAbsolutePath());
 		} else {
-			retPackage = create(file);
+			return create(file);
 		}
-		return retPackage;
 	}
 
 	/**

src/scratchpad/testcases/org/apache/poi/hwpf/usermodel/TestHWPFWrite.java

@@ -17,11 +17,7 @@
 
 package org.apache.poi.hwpf.usermodel;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
+import java.io.*;
 
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.hwpf.HWPFDocument;
@@ -90,10 +86,17 @@ public final class TestHWPFWrite extends HWPFTestCase {
    public void testInPlaceWrite() throws Exception {
        // Setup as a copy of a known-good file
        final File file = TempFile.createTempFile("TestDocument", ".doc");
-       IOUtils.copy(
-               POIDataSamples.getDocumentInstance().openResourceAsStream("SampleDoc.doc"),
-               new FileOutputStream(file)
-       );
+       InputStream inputStream = POIDataSamples.getDocumentInstance().openResourceAsStream("SampleDoc.doc");
+       try {
+           FileOutputStream outputStream = new FileOutputStream(file);
+           try {
+               IOUtils.copy(inputStream, outputStream);
+           } finally {
+               outputStream.close();
+           }
+       } finally {
+           inputStream.close();
+       }
 
        // Open from the temp file in read-write mode
        HWPFDocument doc = new HWPFDocument(new NPOIFSFileSystem(file, false).getRoot());
@@ -108,7 +111,9 @@ public final class TestHWPFWrite extends HWPFTestCase {
        doc.close();
 
        doc = new HWPFDocument(new NPOIFSFileSystem(file).getRoot());
+       r = doc.getRange();
        assertEquals("X XX a test document\r", r.getParagraph(0).text());
+       doc.close();
    }
 
    @SuppressWarnings("resource")
@@ -121,7 +126,10 @@ public final class TestHWPFWrite extends HWPFTestCase {
        try {
            doc.write();
            fail("Shouldn't work for InputStream");
-       } catch (IllegalStateException e) {}
+       } catch (IllegalStateException e) {
+           // expected here
+       }
+       doc.close();
 
        // Can't work for OPOIFS
        OPOIFSFileSystem ofs = new OPOIFSFileSystem(
@@ -130,7 +138,10 @@ public final class TestHWPFWrite extends HWPFTestCase {
        try {
            doc.write();
            fail("Shouldn't work for OPOIFSFileSystem");
-       } catch (IllegalStateException e) {}
+       } catch (IllegalStateException e) {
+           // expected here
+       }
+       doc.close();
 
        // Can't work for Read-Only files
        NPOIFSFileSystem fs = new NPOIFSFileSystem(
@@ -139,6 +150,9 @@ public final class TestHWPFWrite extends HWPFTestCase {
        try {
            doc.write();
            fail("Shouldn't work for Read Only");
-       } catch (IllegalStateException e) {}
+       } catch (IllegalStateException e) {
+           // expected here
+       }
+       doc.close();
    }
 }

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java

@@ -1222,7 +1222,10 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         try {
             wb.write();
             fail("Shouldn't work for new files");
-        } catch (IllegalStateException e) {}
+        } catch (IllegalStateException e) {
+            // expected here
+        }
+        wb.close();
         
         // Can't work for InputStream opened files
         wb = new HSSFWorkbook(
@@ -1230,7 +1233,10 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         try {
             wb.write();
             fail("Shouldn't work for InputStream");
-        } catch (IllegalStateException e) {}
+        } catch (IllegalStateException e) {
+            // expected here
+        }
+        wb.close();
         
         // Can't work for OPOIFS
         OPOIFSFileSystem ofs = new OPOIFSFileSystem(
@@ -1239,7 +1245,10 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         try {
             wb.write();
             fail("Shouldn't work for OPOIFSFileSystem");
-        } catch (IllegalStateException e) {}
+        } catch (IllegalStateException e) {
+            // expected here
+        }
+        wb.close();
         
         // Can't work for Read-Only files
         NPOIFSFileSystem fs = new NPOIFSFileSystem(
@@ -1248,17 +1257,27 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         try {
             wb.write();
             fail("Shouldn't work for Read Only");
-        } catch (IllegalStateException e) {}
+        } catch (IllegalStateException e) {
+            // expected here
+        }
+        wb.close();
     }
     
     @Test
     public void inPlaceWrite() throws Exception {
         // Setup as a copy of a known-good file
         final File file = TempFile.createTempFile("TestHSSFWorkbook", ".xls");
-        IOUtils.copy(
-                POIDataSamples.getSpreadSheetInstance().openResourceAsStream("SampleSS.xls"),
-                new FileOutputStream(file)
-        );
+        InputStream inputStream = POIDataSamples.getSpreadSheetInstance().openResourceAsStream("SampleSS.xls");
+        try {
+            FileOutputStream outputStream = new FileOutputStream(file);
+            try {
+                IOUtils.copy(inputStream, outputStream);
+            } finally {
+                outputStream.close();
+            }
+        } finally {
+            inputStream.close();
+        }
         
         // Open from the temp file in read-write mode
         HSSFWorkbook wb = new HSSFWorkbook(new NPOIFSFileSystem(file, false));
@@ -1276,6 +1295,8 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         wb = new HSSFWorkbook(new NPOIFSFileSystem(file));
         assertEquals(1, wb.getNumberOfSheets());
         assertEquals("Changed!", wb.getSheetAt(0).getRow(0).getCell(0).toString());
+
+        wb.close();
     }
     
     @Test

src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSFileSystem.java

@@ -24,11 +24,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
 
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
+import java.io.*;
 import java.nio.ByteBuffer;
 import java.util.Iterator;
 
@@ -96,20 +92,25 @@ public final class TestNPOIFSFileSystem {
        HeaderBlock header = new HeaderBlock(new ByteArrayInputStream(baos.toByteArray()));
        return header;
    }
-   
-   protected static NPOIFSFileSystem writeOutAndReadBack(NPOIFSFileSystem original) throws IOException {
-       ByteArrayOutputStream baos = new ByteArrayOutputStream();
-       original.writeFilesystem(baos);
-       original.close();
-       return new NPOIFSFileSystem(new ByteArrayInputStream(baos.toByteArray()));
-   }
-   protected static NPOIFSFileSystem writeOutFileAndReadBack(NPOIFSFileSystem original) throws IOException {
-       final File file = TempFile.createTempFile("TestPOIFS", ".ole2");
-       final FileOutputStream fout = new FileOutputStream(file);
-       original.writeFilesystem(fout);
-       original.close();
-       return new NPOIFSFileSystem(file, false);
-   }
+
+    protected static NPOIFSFileSystem writeOutAndReadBack(NPOIFSFileSystem original) throws IOException {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        original.writeFilesystem(baos);
+        original.close();
+        return new NPOIFSFileSystem(new ByteArrayInputStream(baos.toByteArray()));
+    }
+
+    protected static NPOIFSFileSystem writeOutFileAndReadBack(NPOIFSFileSystem original) throws IOException {
+        final File file = TempFile.createTempFile("TestPOIFS", ".ole2");
+        final OutputStream fout = new FileOutputStream(file);
+        try {
+            original.writeFilesystem(fout);
+        } finally {
+            fout.close();
+        }
+        original.close();
+        return new NPOIFSFileSystem(file, false);
+    }
    
    @Test
    public void basicOpen() throws Exception {

src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java

@@ -1017,7 +1017,7 @@ public abstract class BaseTestCell {
     }
 
     @Test
-    public void primitiveToEnumReplacementDoesNotBreakBackwardsCompatibility() {
+    public void primitiveToEnumReplacementDoesNotBreakBackwardsCompatibility() throws IOException {
         // bug 59836
         // until we have changes POI from working on primitives (int) to enums,
         // we should make sure we minimize backwards compatibility breakages.
@@ -1046,5 +1046,7 @@ public abstract class BaseTestCell {
             default:
                 fail("unexpected cell type: " + cell.getCellType());
         }
+
+        wb.close();
     }
 }