Fix some cases where POI itself or the tests leaked file-handles

Also fix some IntelliJ warnings

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

diff --git a/src/ooxml/java/org/apache/poi/ss/usermodel/WorkbookFactory.java b/src/ooxml/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
index fe9af6cf5e84f9f7dd0c76d5a7cfe234a150f016..7f4d1f7971dd499c7abb953020a5e29a30b6f69d 100644 (file)
--- a/src/ooxml/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
+++ b/src/ooxml/java/org/apache/poi/ss/usermodel/WorkbookFactory.java
@@ -16,11 +16,7 @@
 ==================================================================== */
 package org.apache.poi.ss.usermodel;
 
-import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.PushbackInputStream;
+import java.io.*;
 import java.security.GeneralSecurityException;
 
 import org.apache.poi.EmptyFileException;
@@ -81,7 +77,7 @@ public class WorkbookFactory {
      *  @throws IOException if an error occurs while reading the data
      *  @throws InvalidFormatException if the contents of the file cannot be parsed into a {@link Workbook}
      */
-    private static Workbook create(NPOIFSFileSystem fs, String password) throws IOException, InvalidFormatException {
+    private static Workbook create(final NPOIFSFileSystem fs, String password) throws IOException, InvalidFormatException {
         DirectoryNode root = fs.getRoot();
 
         // Encrypted OOXML files go inside OLE2 containers, is this one?
@@ -99,7 +95,16 @@ public class WorkbookFactory {
                     passwordCorrect = true;
                 }
                 if (passwordCorrect) {
-                    stream = d.getDataStream(root);
+                    // wrap the stream in a FilterInputStream to close the NPOIFSFileSystem
+                    // as well when the resulting OPCPackage is closed
+                    stream = new FilterInputStream(d.getDataStream(root)) {
+                        @Override
+                        public void close() throws IOException {
+                            fs.close();
+
+                            super.close();
+                        }
+                    };
                 }
             } catch (GeneralSecurityException e) {
                 throw new IOException(e);

diff --git a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java
index 19bb7b9db82b8be9029eef587a9aff2071d00eb5..9d25d8e054c011f67e2358b1853144ac8108a104 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java
+++ b/src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java
@@ -150,6 +150,7 @@ public final class TestPOIXMLDocument {
             }
         } finally {
             doc.close();
+            pkg2.close();
         }
     }
 

diff --git a/src/ooxml/testcases/org/apache/poi/ss/TestWorkbookFactory.java b/src/ooxml/testcases/org/apache/poi/ss/TestWorkbookFactory.java
index 7c61e01ecb958ec612846522c4693720e1abb707..b088359b6982c192dc13dcae32c823874d502da3 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/ss/TestWorkbookFactory.java
+++ b/src/ooxml/testcases/org/apache/poi/ss/TestWorkbookFactory.java
@@ -313,6 +313,9 @@ public final class TestWorkbookFactory {
         );
         assertNotNull(wb);
         assertTrue(wb instanceof XSSFWorkbook);
+        assertTrue(wb.getNumberOfSheets() > 0);
+        assertNotNull(wb.getSheetAt(0));
+        assertNotNull(wb.getSheetAt(0).getRow(0));
         assertCloseDoesNotModifyFile(xlsx_prot[0], wb);
 
         // Protected, wrong password, throws Exception
@@ -322,7 +325,9 @@ public final class TestWorkbookFactory {
             );
             assertCloseDoesNotModifyFile(xls_prot[0], wb);
             fail("Shouldn't be able to open with the wrong password");
-        } catch (EncryptedDocumentException e) {}
+        } catch (EncryptedDocumentException e) {
+            // expected here
+        }
 
         try {
             wb = WorkbookFactory.create(
@@ -330,7 +335,9 @@ public final class TestWorkbookFactory {
             );
             assertCloseDoesNotModifyFile(xlsx_prot[0], wb);
             fail("Shouldn't be able to open with the wrong password");
-        } catch (EncryptedDocumentException e) {}
+        } catch (EncryptedDocumentException e) {
+            // expected here
+        }
     }
     
     /**

diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
index 278cd762ed3ae7187be0c029a43ea595c7387aee..8a5af76fa2b29ad4f10b6f0a2950091a99d7ef7d 100644 (file)
--- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
@@ -104,14 +104,10 @@ public abstract class BaseTestBugzillaIssues {
         Sheet sheet = wb1.createSheet();
         CreationHelper factory = wb1.getCreationHelper();
 
-        String tmp1 = null;
-        String tmp2 = null;
-        String tmp3 = null;
-
         for (int i = 0; i < num; i++) {
-            tmp1 = "Test1" + i;
-            tmp2 = "Test2" + i;
-            tmp3 = "Test3" + i;
+            String tmp1 = "Test1" + i;
+            String tmp2 = "Test2" + i;
+            String tmp3 = "Test3" + i;
 
             Row row = sheet.createRow(i);
 
@@ -127,9 +123,9 @@ public abstract class BaseTestBugzillaIssues {
 
         sheet = wb2.getSheetAt(0);
         for (int i = 0; i < num; i++) {
-            tmp1 = "Test1" + i;
-            tmp2 = "Test2" + i;
-            tmp3 = "Test3" + i;
+            String tmp1 = "Test1" + i;
+            String tmp2 = "Test2" + i;
+            String tmp3 = "Test3" + i;
 
             Row row = sheet.getRow(i);
 
@@ -216,15 +212,11 @@ public abstract class BaseTestBugzillaIssues {
      *test opening the resulting file in Excel*/
     @Test
     public final void bug22568() throws IOException {
-        int r=2000;int c=3;
-
         Workbook wb1 = _testDataProvider.createWorkbook();
         Sheet sheet = wb1.createSheet("ExcelTest") ;
 
-        int col_cnt=0, rw_cnt=0 ;
-
-        col_cnt = c;
-        rw_cnt = r;
+        int col_cnt = 3;
+        int rw_cnt = 2000;
 
         Row rw ;
         rw = sheet.createRow(0) ;
@@ -357,7 +349,7 @@ public abstract class BaseTestBugzillaIssues {
     }
 
     private static String createFunction(String name, int maxArgs){
-        StringBuffer fmla = new StringBuffer();
+        StringBuilder fmla = new StringBuilder();
         fmla.append(name);
         fmla.append("(");
         for(int i=0; i < maxArgs; i++){
@@ -468,14 +460,9 @@ public abstract class BaseTestBugzillaIssues {
     
     /**
      * Test if a > b. Fails if false.
-     *
-     * @param message
-     * @param a
-     * @param b
      */
     private void assertGreaterThan(String message, double a, double b) {
-        if (a > b) { // expected
-        } else {
+        if (a <= b) {
             String msg = "Expected: " + a + " > " + b;
             fail(message + ": " + msg);
         }
@@ -493,9 +480,9 @@ public abstract class BaseTestBugzillaIssues {
         AttributedString str = new AttributedString(txt);
         copyAttributes(font, str, 0, txt.length());
 
-        if (rt.numFormattingRuns() > 0) {
-            // TODO: support rich text fragments
-        }
+        // TODO: support rich text fragments
+        /*if (rt.numFormattingRuns() > 0) {
+        }*/
 
         TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext);
         double frameWidth = getFrameWidth(layout);
@@ -504,8 +491,7 @@ public abstract class BaseTestBugzillaIssues {
     
     private double getFrameWidth(TextLayout layout) {
         Rectangle2D bounds = layout.getBounds();
-        double frameWidth = bounds.getX() + bounds.getWidth();
-        return frameWidth;
+        return bounds.getX() + bounds.getWidth();
     }
 
     private double computeCellWidthFixed(Font font, String txt) {
@@ -514,8 +500,7 @@ public abstract class BaseTestBugzillaIssues {
         copyAttributes(font, str, 0, txt.length());
 
         TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext);
-        double frameWidth = getFrameWidth(layout);
-        return frameWidth;
+        return getFrameWidth(layout);
     }
 
     private static void copyAttributes(Font font, AttributedString str, int startIdx, int endIdx) {
@@ -682,7 +667,7 @@ public abstract class BaseTestBugzillaIssues {
             r1.createCell(i, Cell.CELL_TYPE_NUMERIC).setCellValue(1.2345);
         }
         for (int i=0; i<12; i+=3) {
-            r1.getCell(i+0).setCellStyle(iPercent);
+            r1.getCell(i).setCellStyle(iPercent);
             r1.getCell(i+1).setCellStyle(d1Percent);
             r1.getCell(i+2).setCellStyle(d2Percent);
         }
@@ -916,11 +901,15 @@ public abstract class BaseTestBugzillaIssues {
         try {
             evaluateCell(wb2, c1);
             fail("Shouldn't be able to evaluate without the other file");
-        } catch (Exception e) {}
+        } catch (Exception e) {
+            // expected here
+        }
         try {
             evaluateCell(wb2, c2);
             fail("Shouldn't be able to evaluate without the other file");
-        } catch (Exception e) {}
+        } catch (Exception e) {
+            // expected here
+        }
 
 
         // Set up references to the other file
@@ -1160,25 +1149,33 @@ public abstract class BaseTestBugzillaIssues {
         try {
             cn.getRichStringCellValue();
             fail();
-        } catch(IllegalStateException e) {}
+        } catch(IllegalStateException e) {
+            // expected here
+        }
 
         assertEquals("Testing", cs.getStringCellValue());
         try {
             cs.getNumericCellValue();
             fail();
-        } catch(IllegalStateException e) {}
+        } catch(IllegalStateException e) {
+            // expected here
+        }
 
         assertEquals(1.2, cfn.getNumericCellValue(), 0);
         try {
             cfn.getRichStringCellValue();
             fail();
-        } catch(IllegalStateException e) {}
+        } catch(IllegalStateException e) {
+            // expected here
+        }
 
         assertEquals("Testing", cfs.getStringCellValue());
         try {
             cfs.getNumericCellValue();
             fail();
-        } catch(IllegalStateException e) {}
+        } catch(IllegalStateException e) {
+            // expected here
+        }
         
         wb.close();
     }
@@ -1401,7 +1398,7 @@ public abstract class BaseTestBugzillaIssues {
     }
 
     @Test
-    public void test52684() {
+    public void test52684() throws IOException {
         Workbook wb = _testDataProvider.createWorkbook();
 
         Sheet sheet = wb.createSheet("test");
@@ -1422,6 +1419,8 @@ public abstract class BaseTestBugzillaIssues {
         DataFormatter formatter = new DataFormatter();
 
         assertEquals("12-312-345-123", formatter.formatCellValue(cell));
+
+        wb.close();
     }
     
     @Test
@@ -1434,7 +1433,7 @@ public abstract class BaseTestBugzillaIssues {
         final Workbook wb = _testDataProvider.createWorkbook(nrows+1);
         final Sheet sh = wb.createSheet();
         out.println(wb.getClass().getName() + " column autosizing timing...");
-        
+
         final long t0 = time();
         _testDataProvider.trackAllColumnsForAutosizing(sh);
         for (int r=0; r<nrows; r++) {
@@ -1477,9 +1476,9 @@ public abstract class BaseTestBugzillaIssues {
     }
     
     protected long time() {
-        final long currentTime = System.currentTimeMillis();
-        return currentTime;
+        return System.currentTimeMillis();
     }
+
     protected double delta(long startTimeMillis) {
         return time() - startTimeMillis;
     }