From 5e0612eabdb40360e2bbbd3488e949f11e13163e Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sun, 31 Jul 2016 17:19:17 +0000 Subject: [PATCH] IDE warnings and fix a few places where we do not close resources in tests git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1754673 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/poi/openxml4j/opc/OPCPackage.java | 8 ++-- .../opc/internal/ContentTypeManager.java | 7 +-- .../poi/xslf/usermodel/XSLFGroupShape.java | 6 +-- .../poi/xssf/extractor/XSSFExportToXml.java | 47 ++++++------------- .../apache/poi/xssf/streaming/SXSSFCell.java | 1 + .../opc/internal/TestContentTypeManager.java | 24 ++++++---- .../poi/sl/usermodel/BaseTestSlideShow.java | 22 +++++---- .../apache/poi/ss/util/BaseTestCellUtil.java | 29 +++++++----- 8 files changed, 68 insertions(+), 76 deletions(-) diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java index a438c061f..e029d7dec 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java @@ -382,8 +382,7 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { } // Creates a new package - OPCPackage pkg = null; - pkg = new ZipPackage(); + OPCPackage pkg = new ZipPackage(); pkg.originalPackagePath = file.getAbsolutePath(); configurePackage(pkg); @@ -391,8 +390,7 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { } public static OPCPackage create(OutputStream output) { - OPCPackage pkg = null; - pkg = new ZipPackage(); + OPCPackage pkg = new ZipPackage(); pkg.originalPackagePath = null; pkg.output = output; @@ -542,7 +540,7 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { // Create the thumbnail part name String contentType = ContentTypes .getContentTypeFromFileExtension(filename); - PackagePartName thumbnailPartName = null; + PackagePartName thumbnailPartName; try { thumbnailPartName = PackagingURIHelper.createPartName("/docProps/" + filename); diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ContentTypeManager.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ContentTypeManager.java index 537dd15e9..750f9cd71 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ContentTypeManager.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/ContentTypeManager.java @@ -29,10 +29,7 @@ import java.util.TreeMap; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.openxml4j.exceptions.InvalidOperationException; import org.apache.poi.openxml4j.exceptions.OpenXML4JRuntimeException; -import org.apache.poi.openxml4j.opc.OPCPackage; -import org.apache.poi.openxml4j.opc.PackagePart; -import org.apache.poi.openxml4j.opc.PackagePartName; -import org.apache.poi.openxml4j.opc.PackagingURIHelper; +import org.apache.poi.openxml4j.opc.*; import org.apache.poi.util.DocumentHelper; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -54,7 +51,7 @@ public abstract class ContentTypeManager { /** * Content type namespace */ - public static final String TYPES_NAMESPACE_URI = "http://schemas.openxmlformats.org/package/2006/content-types"; + public static final String TYPES_NAMESPACE_URI = PackageNamespaces.CONTENT_TYPES; /* Xml elements in content type part */ diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFGroupShape.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFGroupShape.java index 3652b89b1..08022e976 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFGroupShape.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFGroupShape.java @@ -304,13 +304,13 @@ implements XSLFShapeContainer, GroupShape { @Override public boolean getFlipHorizontal(){ CTGroupTransform2D xfrm = getXfrm(); - return (xfrm == null || !xfrm.isSetFlipH()) ? false : xfrm.getFlipH(); + return !(xfrm == null || !xfrm.isSetFlipH()) && xfrm.getFlipH(); } @Override public boolean getFlipVertical(){ CTGroupTransform2D xfrm = getXfrm(); - return (xfrm == null || !xfrm.isSetFlipV()) ? false : xfrm.getFlipV(); + return !(xfrm == null || !xfrm.isSetFlipV()) && xfrm.getFlipV(); } @Override @@ -333,7 +333,7 @@ implements XSLFShapeContainer, GroupShape { // recursively update each shape for(XSLFShape shape : gr.getShapes()) { - XSLFShape newShape = null; + XSLFShape newShape; if (shape instanceof XSLFTextBox) { newShape = createTextBox(); } else if (shape instanceof XSLFAutoShape) { diff --git a/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExportToXml.java b/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExportToXml.java index abba81430..2277d600a 100644 --- a/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExportToXml.java +++ b/src/ooxml/java/org/apache/poi/xssf/extractor/XSSFExportToXml.java @@ -41,7 +41,6 @@ import javax.xml.validation.Schema; import javax.xml.validation.SchemaFactory; import javax.xml.validation.Validator; -import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.ss.usermodel.CellType; import org.apache.poi.ss.usermodel.DateUtil; import org.apache.poi.util.DocumentHelper; @@ -55,7 +54,6 @@ import org.apache.poi.xssf.usermodel.XSSFSheet; import org.apache.poi.xssf.usermodel.XSSFTable; import org.apache.poi.xssf.usermodel.helpers.XSSFSingleXmlCell; import org.apache.poi.xssf.usermodel.helpers.XSSFXmlColumnPr; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.STXmlDataType; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.NamedNodeMap; @@ -117,8 +115,7 @@ public class XSSFExportToXml implements Comparator{ * @param validate if true, validates the XML againts the XML Schema * @throws SAXException * @throws ParserConfigurationException - * @throws TransformerException - * @throws InvalidFormatException + * @throws TransformerException */ public void exportToXML(OutputStream os, String encoding, boolean validate) throws SAXException, ParserConfigurationException, TransformerException{ List singleXMLCells = map.getRelatedSingleXMLCell(); @@ -128,10 +125,10 @@ public class XSSFExportToXml implements Comparator{ Document doc = DocumentHelper.createDocument(); - Element root = null; + final Element root; if (isNamespaceDeclared()) { - root=doc.createElementNS(getNamespace(),rootElement); + root = doc.createElementNS(getNamespace(),rootElement); } else { root = doc.createElementNS("", rootElement); } @@ -152,7 +149,6 @@ public class XSSFExportToXml implements Comparator{ tableMappings.put(commonXPath, table); } - Collections.sort(xpaths,this); for(String xpath : xpaths) { @@ -167,8 +163,7 @@ public class XSSFExportToXml implements Comparator{ XSSFCell cell = simpleXmlCell.getReferencedCell(); if (cell!=null) { Node currentNode = getNodeByXPath(xpath,doc.getFirstChild(),doc,false); - STXmlDataType.Enum dataType = simpleXmlCell.getXmlDataType(); - mapCellOnNode(cell,currentNode,dataType); + mapCellOnNode(cell,currentNode); //remove nodes which are empty in order to keep the output xml valid if("".equals(currentNode.getTextContent()) && currentNode.getParentNode() != null) { @@ -202,22 +197,15 @@ public class XSSFExportToXml implements Comparator{ XSSFXmlColumnPr pointer = tableColumns.get(j-startColumnIndex); String localXPath = pointer.getLocalXPath(); Node currentNode = getNodeByXPath(localXPath,tableRootNode,doc,false); - STXmlDataType.Enum dataType = pointer.getXmlDataType(); - - mapCellOnNode(cell,currentNode,dataType); + mapCellOnNode(cell,currentNode); } - } - } - - - } - } else { + } /*else { // TODO: implement filtering management in xpath - } + }*/ } boolean isValid = true; @@ -225,8 +213,6 @@ public class XSSFExportToXml implements Comparator{ isValid =isValid(doc); } - - if (isValid) { ///////////////// @@ -275,7 +261,7 @@ public class XSSFExportToXml implements Comparator{ } - private void mapCellOnNode(XSSFCell cell, Node node, STXmlDataType.Enum outputDataType) { + private void mapCellOnNode(XSSFCell cell, Node node) { String value =""; switch (cell.getCellTypeEnum()) { @@ -349,11 +335,7 @@ public class XSSFExportToXml implements Comparator{ } currentNode = selectedNode; } else { - - - Node attribute = createAttribute(doc, currentNode, axisName); - - currentNode = attribute; + currentNode = createAttribute(doc, currentNode, axisName); } } return currentNode; @@ -421,12 +403,11 @@ public class XSSFExportToXml implements Comparator{ for(int i =1;i { }if ( leftIndex > rightIndex) { return 1; } - } else { + } /*else { // NOTE: the xpath doesn't match correctly in the schema - } + }*/ } } @@ -483,7 +464,7 @@ public class XSSFExportToXml implements Comparator{ // Note: we expect that all the complex types are defined at root level Node complexTypeNode = null; if (!"".equals(complexTypeName)) { - complexTypeNode = getComplexTypeNodeFromSchemaChildren(xmlSchema, complexTypeNode, complexTypeName); + complexTypeNode = getComplexTypeNodeFromSchemaChildren(xmlSchema, null, complexTypeName); } return complexTypeNode; diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFCell.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFCell.java index e6994b8de..6174af54b 100644 --- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFCell.java +++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFCell.java @@ -994,6 +994,7 @@ public class SXSSFCell implements Cell { ((FormulaValue)_value)._value = ((FormulaValue)prevValue)._value; } } + //TODO: implement this correctly @NotImplemented /*package*/ CellType computeTypeFromFormula(String formula) diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/internal/TestContentTypeManager.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/internal/TestContentTypeManager.java index 9acf0d10b..936d0f9e8 100644 --- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/internal/TestContentTypeManager.java +++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/internal/TestContentTypeManager.java @@ -18,6 +18,7 @@ package org.apache.poi.openxml4j.opc.internal; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; @@ -44,16 +45,21 @@ public final class TestContentTypeManager { // Retrieves core properties part OPCPackage p = OPCPackage.open(filepath, PackageAccess.READ); - PackageRelationshipCollection rels = p.getRelationshipsByType(PackageRelationshipTypes.CORE_PROPERTIES); - PackageRelationship corePropertiesRelationship = rels.getRelationship(0); - PackagePart coreDocument = p.getPart(corePropertiesRelationship); - - assertEquals("application/vnd.openxmlformats-package.core-properties+xml", coreDocument.getContentType()); + try { + PackageRelationshipCollection rels = p.getRelationshipsByType(PackageRelationshipTypes.CORE_PROPERTIES); + PackageRelationship corePropertiesRelationship = rels.getRelationship(0); + PackagePart coreDocument = p.getPart(corePropertiesRelationship); - // TODO - finish writing this test - assumeTrue("finish writing this test", false); - - ContentTypeManager ctm = new ZipContentTypeManager(coreDocument.getInputStream(), p); + assertEquals("application/vnd.openxmlformats-package.core-properties+xml", coreDocument.getContentType()); + + // TODO - finish writing this test + assumeTrue("finish writing this test", false); + + ContentTypeManager ctm = new ZipContentTypeManager(coreDocument.getInputStream(), p); + assertNotNull(ctm); + } finally { + p.close(); + } } /** diff --git a/src/testcases/org/apache/poi/sl/usermodel/BaseTestSlideShow.java b/src/testcases/org/apache/poi/sl/usermodel/BaseTestSlideShow.java index 404e0da07..e02260844 100644 --- a/src/testcases/org/apache/poi/sl/usermodel/BaseTestSlideShow.java +++ b/src/testcases/org/apache/poi/sl/usermodel/BaseTestSlideShow.java @@ -50,14 +50,20 @@ public abstract class BaseTestSlideShow { @Test public void addPicture_Stream() throws IOException { SlideShow show = createSlideShow(); - InputStream stream = slTests.openResourceAsStream("clock.jpg"); - - assertEquals(0, show.getPictureData().size()); - PictureData picture = show.addPicture(stream, PictureType.JPEG); - assertEquals(1, show.getPictureData().size()); - assertSame(picture, show.getPictureData().get(0)); - - show.close(); + try { + InputStream stream = slTests.openResourceAsStream("clock.jpg"); + try { + assertEquals(0, show.getPictureData().size()); + PictureData picture = show.addPicture(stream, PictureType.JPEG); + assertEquals(1, show.getPictureData().size()); + assertSame(picture, show.getPictureData().get(0)); + + } finally { + stream.close(); + } + } finally { + show.close(); + } } @Test diff --git a/src/testcases/org/apache/poi/ss/util/BaseTestCellUtil.java b/src/testcases/org/apache/poi/ss/util/BaseTestCellUtil.java index ed0c1da7e..a63ce5a60 100644 --- a/src/testcases/org/apache/poi/ss/util/BaseTestCellUtil.java +++ b/src/testcases/org/apache/poi/ss/util/BaseTestCellUtil.java @@ -78,14 +78,16 @@ public class BaseTestCellUtil { @Test(expected=RuntimeException.class) public void setCellStylePropertyWithInvalidValue() throws IOException { Workbook wb = _testDataProvider.createWorkbook(); - Sheet s = wb.createSheet(); - Row r = s.createRow(0); - Cell c = r.createCell(0); + try { + Sheet s = wb.createSheet(); + Row r = s.createRow(0); + Cell c = r.createCell(0); - // An invalid BorderStyle constant - CellUtil.setCellStyleProperty(c, CellUtil.BORDER_BOTTOM, 42); - - wb.close(); + // An invalid BorderStyle constant + CellUtil.setCellStyleProperty(c, CellUtil.BORDER_BOTTOM, 42); + } finally { + wb.close(); + } } @Test() @@ -352,10 +354,8 @@ public class BaseTestCellUtil { CellUtil.setFont(A1, font2); fail("setFont not allowed if font belongs to a different workbook"); } catch (final IllegalArgumentException e) { - if (e.getMessage().startsWith("Font does not belong to this workbook")) { - // expected - } - else { + // one specific message is expected + if (!e.getMessage().startsWith("Font does not belong to this workbook")) { throw e; } } finally { @@ -371,7 +371,7 @@ public class BaseTestCellUtil { */ // bug 55555 @Test - public void setFillForegroundColorBeforeFillBackgroundColor() { + public void setFillForegroundColorBeforeFillBackgroundColor() throws IOException { Workbook wb1 = _testDataProvider.createWorkbook(); Cell A1 = wb1.createSheet().createRow(0).createCell(0); Map properties = new HashMap(); @@ -386,13 +386,14 @@ public class BaseTestCellUtil { assertEquals("fill pattern", CellStyle.BRICKS, style.getFillPattern()); assertEquals("fill foreground color", IndexedColors.BLUE, IndexedColors.fromInt(style.getFillForegroundColor())); assertEquals("fill background color", IndexedColors.RED, IndexedColors.fromInt(style.getFillBackgroundColor())); + wb1.close(); } /** * bug 55555 * @since POI 3.15 beta 3 */ @Test - public void setFillForegroundColorBeforeFillBackgroundColorEnum() { + public void setFillForegroundColorBeforeFillBackgroundColorEnum() throws IOException { Workbook wb1 = _testDataProvider.createWorkbook(); Cell A1 = wb1.createSheet().createRow(0).createCell(0); Map properties = new HashMap(); @@ -407,5 +408,7 @@ public class BaseTestCellUtil { assertEquals("fill pattern", FillPatternType.BRICKS, style.getFillPatternEnum()); assertEquals("fill foreground color", IndexedColors.BLUE, IndexedColors.fromInt(style.getFillForegroundColor())); assertEquals("fill background color", IndexedColors.RED, IndexedColors.fromInt(style.getFillBackgroundColor())); + + wb1.close(); } }