Change order of handling shape-types in copy() as FreeFormShape derives from AutoShape and thus would have not been copied correctly currently

Fix some Javadoc, some warnings and code duplications

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

src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java

@@ -146,7 +146,8 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
     }
 
     HSSFSheet cloneSheet(HSSFWorkbook workbook) {
-        this.getDrawingPatriarch();/**Aggregate drawing records**/
+        // Aggregate drawing records
+        this.getDrawingPatriarch();
         HSSFSheet sheet = new HSSFSheet(workbook, _sheet.cloneSheet());
         int pos = sheet._sheet.findFirstRecordLocBySid(DrawingRecord.sid);
         DrawingRecord dr = (DrawingRecord) sheet._sheet.findFirstRecordBySid(DrawingRecord.sid);

src/ooxml/java/org/apache/poi/ss/extractor/EmbeddedExtractor.java

@@ -167,13 +167,10 @@ public class EmbeddedExtractor implements Iterable<EmbeddedExtractor> {
     protected EmbeddedData extract(DirectoryNode dn) throws IOException {
         assert(canExtract(dn));
         ByteArrayOutputStream bos = new ByteArrayOutputStream(20000);
-        POIFSFileSystem dest = new POIFSFileSystem();
+        try (POIFSFileSystem dest = new POIFSFileSystem()) {
-        try {
             copyNodes(dn, dest.getRoot());
             // start with a reasonable big size
             dest.writeFilesystem(bos);
-        } finally {
-            dest.close();
         }
 
         return new EmbeddedData(dn.getName(), bos.toByteArray(), CONTENT_TYPE_BYTES);
@@ -369,11 +366,8 @@ public class EmbeddedExtractor implements Iterable<EmbeddedExtractor> {
                 destDir.setStorageClsid(srcDir.getStorageClsid());
                 copyNodes(srcDir, destDir);
             } else {
-                InputStream is = src.createDocumentInputStream(e);
+                try (InputStream is = src.createDocumentInputStream(e)) {
-                try {
                     dest.createDocument(e.getName(), is);
-                } finally {
-                    is.close();
                 }
             }
         }

src/ooxml/java/org/apache/poi/util/SAXHelper.java

@@ -66,19 +66,16 @@ public final class SAXHelper {
             saxFactory = SAXParserFactory.newInstance();
             saxFactory.setValidating(false);
             saxFactory.setNamespaceAware(true);
-        } catch (RuntimeException re) {
+        } catch (RuntimeException | Error re) {
+            // this also catches NoClassDefFoundError, which may be due to a local class path issue
+            // This may occur if the code is run inside a web container
+            // or a restricted JVM
+            // See bug 61170: https://bz.apache.org/bugzilla/show_bug.cgi?id=61170
             logger.log(POILogger.WARN, "Failed to create SAXParserFactory", re);
             throw re;
         } catch (Exception e) {
             logger.log(POILogger.WARN, "Failed to create SAXParserFactory", e);
             throw new RuntimeException("Failed to create SAXParserFactory", e);
-        } catch (Error e) {
-            // catches NoClassDefFoundError, which may be due to a local class path issue
-            // This may occur if the code is run inside a web container
-            // or a restricted JVM
-            // See bug 61170: https://bz.apache.org/bugzilla/show_bug.cgi?id=61170
-            logger.log(POILogger.WARN, "Failed to create SAXParserFactory", e);
-            throw e;
         }
     }
             

src/ooxml/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java

@@ -481,7 +481,6 @@ implements SlideShow<XSLFShape,XSLFTextParagraph> {
     @Override
     public XSLFPictureData addPicture(byte[] pictureData, PictureType format) {
         XSLFPictureData img = findPictureData(pictureData);
-
         if (img != null) {
             return img;
         }
@@ -491,13 +490,13 @@ implements SlideShow<XSLFShape,XSLFTextParagraph> {
         if (relType == null) {
             throw new IllegalArgumentException("Picture type "+format+" is not supported.");
         }
+
         img = createRelationship(relType, XSLFFactory.getInstance(), imageNumber + 1, true).getDocumentPart();
         img.setIndex(imageNumber);
         _pictures.add(img);
-        try {
+
-            OutputStream out = img.getPackagePart().getOutputStream();
+        try (OutputStream out = img.getPackagePart().getOutputStream()) {
             out.write(pictureData);
-            out.close();
         } catch (IOException e) {
             throw new POIXMLException(e);
         }
@@ -536,11 +535,8 @@ implements SlideShow<XSLFShape,XSLFTextParagraph> {
     {
         int length = (int) pict.length();
         byte[] data = IOUtils.safelyAllocate(length, MAX_RECORD_LENGTH);
-        FileInputStream is = new FileInputStream(pict);
+        try (InputStream is = new FileInputStream(pict)) {
-        try {
             IOUtils.readFully(is, data);
-        } finally {
-            is.close();
         }
         return addPicture(data, format);
     }

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFGroupShape.java

@@ -360,12 +360,12 @@ implements XSLFShapeContainer, GroupShape<XSLFShape,XSLFTextParagraph> {
                 XSLFShape newShape;
                 if (shape instanceof XSLFTextBox) {
                     newShape = createTextBox();
+                } else if (shape instanceof XSLFFreeformShape) {
+                    newShape = createFreeform();
                 } else if (shape instanceof XSLFAutoShape) {
                     newShape = createAutoShape();
                 } else if (shape instanceof XSLFConnectorShape) {
                     newShape = createConnector();
-                } else if (shape instanceof XSLFFreeformShape) {
-                    newShape = createFreeform();
                 } else if (shape instanceof XSLFPictureShape) {
                     XSLFPictureShape p = (XSLFPictureShape)shape;
                     XSLFPictureData pd = p.getPictureData();

src/ooxml/java/org/apache/poi/xslf/util/PPTX2PNG.java

@@ -116,8 +116,7 @@ public class PPTX2PNG {
         if (!quiet) {
             System.out.println("Processing " + file);
         }
-        SlideShow<?,?> ss = SlideShowFactory.create(file, null, true);
+        try (SlideShow<?, ?> ss = SlideShowFactory.create(file, null, true)) {
-        try {
             List<? extends Slide<?, ?>> slides = ss.getSlides();
 
             Set<Integer> slidenum = slideIndexes(slides.size(), slidenumStr);
@@ -160,12 +159,10 @@ public class PPTX2PNG {
                     File outfile = new File(outdir, outname);
                     ImageIO.write(img, format, outfile);
                 }
-                
+
                 graphics.dispose();
                 img.flush();
             }
-        } finally {
-            ss.close();
         }
 
         if (!quiet) {

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java

@@ -1114,7 +1114,7 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
     /**
      * Get the XSSFSheet object at the given index.
      *
-     * @param index of the sheet number (0-based physical & logical)
+     * @param index of the sheet number (0-based physical & logical)
      * @return XSSFSheet at the provided index
      * @throws IllegalArgumentException if the index is out of range (index
      *            < 0 || index >= getNumberOfSheets()).
@@ -1129,7 +1129,7 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook {
      * Returns the index of the sheet by his name (case insensitive match)
      *
      * @param name the sheet name
-     * @return index of the sheet (0 based) or <tt>-1</tt if not found
+     * @return index of the sheet (0 based) or <tt>-1</tt> if not found
      */
     @Override
     public int getSheetIndex(String name) {

src/ooxml/java/org/apache/poi/xssf/usermodel/helpers/XSSFRowColShifter.java

@@ -96,7 +96,7 @@ import java.util.List;
      * @param formulaShifter the formula shifting policy
      */
     /*package*/ static void updateRowFormulas(XSSFRow row, FormulaShifter formulaShifter) {
-        XSSFSheet sheet = (XSSFSheet) row.getSheet();
+        XSSFSheet sheet = row.getSheet();
         for (Cell c : row) {
             XSSFCell cell = (XSSFCell) c;
 

src/ooxml/testcases/org/apache/poi/xslf/TestXSLFBugs.java

@@ -101,8 +101,8 @@ public class TestXSLFBugs {
         
         assertTrue(shapeToDelete1.isPresent());
         slide1.removeShape(shapeToDelete1.get());
-        slide1.getRelationParts().stream()
+        assertTrue(slide1.getRelationParts().stream()
-            .allMatch(rp -> "rId1,rId3".contains(rp.getRelationship().getId()) ); 
+            .allMatch(rp -> "rId1,rId3".contains(rp.getRelationship().getId()) ));
         
         assertNotNull(ppt1.getPackage().getPart(ppn));
         ppt1.close();
@@ -114,8 +114,8 @@ public class TestXSLFBugs {
             slide2.getShapes().stream().filter(s -> s instanceof XSLFPictureShape).skip(1).findFirst();
         assertTrue(shapeToDelete2.isPresent());
         slide2.removeShape(shapeToDelete2.get());
-        slide2.getRelationParts().stream()
+        assertTrue(slide2.getRelationParts().stream()
-            .allMatch(rp -> "rId1,rId2".contains(rp.getRelationship().getId()) ); 
+            .allMatch(rp -> "rId1,rId2".contains(rp.getRelationship().getId()) ));
         assertNotNull(ppt2.getPackage().getPart(ppn));
         ppt2.close();
 
@@ -124,7 +124,7 @@ public class TestXSLFBugs {
         slide3.getShapes().stream()
             .filter(s -> s instanceof XSLFPictureShape)
             .collect(Collectors.toList())
-            .forEach(s -> slide3.removeShape(s));
+            .forEach(slide3::removeShape);
         assertNull(ppt3.getPackage().getPart(ppn));
         ppt3.close();
     }
@@ -370,16 +370,8 @@ public class TestXSLFBugs {
         }
 
         // Add a few pictures
-        for (int i=0; i<10; i++) {
+        addPictures(ss, slide, pics, 0, 10);
-            XSLFPictureData data = ss.addPicture(pics[i], PictureType.JPEG);
-            assertEquals(i, data.getIndex());
-            assertEquals(i+1, ss.getPictureData().size());
 
-            XSLFPictureShape shape = slide.createPicture(data);
-            assertNotNull(shape.getPictureData());
-            assertArrayEquals(pics[i], shape.getPictureData().getData());
-            assertEquals(i+2, slide.getShapes().size());
-        }
         // Re-fetch the pictures and check
         for (int i=0; i<10; i++) {
             XSLFPictureShape shape = (XSLFPictureShape)slide.getShapes().get(i+1);
@@ -388,16 +380,8 @@ public class TestXSLFBugs {
         }
 
         // Add past 10
-        for (int i=10; i<15; i++) {
+        addPictures(ss, slide, pics, 10, 15);
-            XSLFPictureData data = ss.addPicture(pics[i], PictureType.JPEG);
-            assertEquals(i, data.getIndex());
-            assertEquals(i+1, ss.getPictureData().size());
 
-            XSLFPictureShape shape = slide.createPicture(data);
-            assertNotNull(shape.getPictureData());
-            assertArrayEquals(pics[i], shape.getPictureData().getData());
-            assertEquals(i+2, slide.getShapes().size());
-        }
         // Check all pictures
         for (int i=0; i<15; i++) {
             XSLFPictureShape shape = (XSLFPictureShape)slide.getShapes().get(i+1);
@@ -446,6 +430,19 @@ public class TestXSLFBugs {
         ss.close();
     }
 
+    private void addPictures(XMLSlideShow ss, XSLFSlide slide, byte[][] pics, int start, int end) {
+        for (int i = start; i< end; i++) {
+            XSLFPictureData data = ss.addPicture(pics[i], PictureType.JPEG);
+            assertEquals(i, data.getIndex());
+            assertEquals(i+1, ss.getPictureData().size());
+
+            XSLFPictureShape shape = slide.createPicture(data);
+            assertNotNull(shape.getPictureData());
+            assertArrayEquals(pics[i], shape.getPictureData().getData());
+            assertEquals(i+2, slide.getShapes().size());
+        }
+    }
+
     private void validateSlides(XMLSlideShow ss, boolean saveAndReload, String... slideTexts) throws IOException {
         if (saveAndReload) {
             XMLSlideShow ss2 = XSLFTestDataSamples.writeOutAndReadBack(ss);

src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFTextParagraph.java

@@ -365,11 +365,10 @@ public class TestXSLFTextParagraph {
 
     @Test(expected = IllegalStateException.class)
     public void testLineBreak() throws IOException {
-        XMLSlideShow ppt = new XMLSlideShow();
+        try (XMLSlideShow ppt = new XMLSlideShow()) {
-        try {
             XSLFSlide slide = ppt.createSlide();
             XSLFTextShape sh = slide.createAutoShape();
-    
+
             XSLFTextParagraph p = sh.addNewTextParagraph();
             XSLFTextRun r1 = p.addNewTextRun();
             r1.setText("Hello,");
@@ -382,13 +381,11 @@ public class TestXSLFTextParagraph {
             r3.setFontSize(20.0);
             XSLFTextRun r4 = p.addLineBreak();
             assertEquals(20.0, r4.getFontSize(), 0);
-    
+
-            assertEquals("Hello,\nWorld!\n",sh.getText());
+            assertEquals("Hello,\nWorld!\n", sh.getText());
 
             // "You cannot change text of a line break, it is always '\\n'"
             r2.setText("aaa");
-        } finally {
-            ppt.close();
         }
     }
 }