From 1e4c1770d7b767d2644588c721713d285dd49eef Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Thu, 26 Dec 2013 17:55:36 +0000 Subject: [PATCH] Bug 51158: clear out Memory-based part before writing to it, except for PictureData items, which keep the original image data in the Part-object directly. Add reproducer-unit tests and enhance some related unit tests git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1553525 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/poi/POIXMLDocumentPart.java | 25 +++++- .../apache/poi/openxml4j/opc/PackagePart.java | 12 ++- .../opc/internal/MemoryPackagePart.java | 3 +- .../poi/xslf/usermodel/XMLSlideShow.java | 36 +++++--- .../poi/xslf/usermodel/XSLFPictureData.java | 13 ++- .../poi/xssf/usermodel/XSSFPictureData.java | 9 ++ .../poi/xwpf/usermodel/XWPFPictureData.java | 9 ++ .../xslf/usermodel/TestXSLFPictureShape.java | 34 +++++--- .../xssf/model/TestSharedStringsTable.java | 18 ++-- .../poi/xssf/model/TestStylesTable.java | 16 +++- .../poi/xssf/usermodel/TestXSSFCellStyle.java | 30 ++++--- .../poi/xssf/usermodel/TestXSSFChart.java | 7 ++ .../poi/xssf/usermodel/TestXSSFComment.java | 3 + .../poi/xssf/usermodel/TestXSSFDrawing.java | 56 ++++++++---- .../xssf/usermodel/TestXSSFPictureData.java | 26 ++++-- .../poi/xssf/usermodel/TestXSSFWorkbook.java | 85 +++++++++++++++++++ .../xwpf/usermodel/TestXWPFPictureData.java | 30 ++++--- 17 files changed, 319 insertions(+), 93 deletions(-) diff --git a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java b/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java index fe147b6228..de889d9df1 100644 --- a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java +++ b/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java @@ -18,8 +18,14 @@ package org.apache.poi; import java.io.IOException; import java.net.URI; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; import java.util.Map.Entry; +import java.util.Set; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.openxml4j.exceptions.OpenXML4JException; @@ -310,6 +316,9 @@ public class POIXMLDocumentPart { * @param alreadySaved context set containing already visited nodes */ protected final void onSave(Set alreadySaved) throws IOException{ + // this usually clears out previous content in the part... + prepareForCommit(); + commit(); alreadySaved.add(this.getPackagePart()); for(POIXMLDocumentPart p : relations.values()){ @@ -319,6 +328,20 @@ public class POIXMLDocumentPart { } } + /** + * Ensure that a memory based package part does not have lingering data from previous + * commit() calls. + * + * Note: This is overwritten for some objects, as *PictureData seem to store the actual content + * in the part directly without keeping a copy like all others therefore we need to handle them differently. + */ + protected void prepareForCommit() { + PackagePart part = this.getPackagePart(); + if(part != null) { + part.clear(); + } + } + /** * Create a new child POIXMLDocumentPart * diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java index 56f6a66e91..e97349447f 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java @@ -522,11 +522,11 @@ public abstract class PackagePart implements RelationshipSource { // Create a memory part PackagePart part = _container.createPart(this._partName, this._contentType.toString(), false); - part._relationships = this._relationships; if (part == null) { - throw new InvalidOperationException( - "Can't create a temporary part !"); + throw new InvalidOperationException( + "Can't create a temporary part !"); } + part._relationships = this._relationships; outStream = part.getOutputStreamImpl(); } else { outStream = this.getOutputStreamImpl(); @@ -690,4 +690,10 @@ public abstract class PackagePart implements RelationshipSource { * respective buffer. */ public abstract void flush(); + + /** + * Allows sub-classes to clean up before new data is added. + */ + public void clear() { + } } diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/MemoryPackagePart.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/MemoryPackagePart.java index f38d7a26d8..f2d8c31151 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/MemoryPackagePart.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/internal/MemoryPackagePart.java @@ -102,7 +102,8 @@ public final class MemoryPackagePart extends PackagePart { return data == null ? 0 : data.length; } - public void clear() { + @Override + public void clear() { data = null; } diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java index 22f7ea4a32..de31fb2fd6 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java @@ -16,13 +16,33 @@ ==================================================================== */ package org.apache.poi.xslf.usermodel; +import java.awt.Dimension; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Pattern; + import org.apache.poi.POIXMLDocument; import org.apache.poi.POIXMLDocumentPart; import org.apache.poi.POIXMLException; import org.apache.poi.POIXMLRelation; import org.apache.poi.openxml4j.exceptions.OpenXML4JException; -import org.apache.poi.openxml4j.opc.*; -import org.apache.poi.util.*; +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.TargetMode; +import org.apache.poi.util.Beta; +import org.apache.poi.util.IOUtils; +import org.apache.poi.util.Internal; +import org.apache.poi.util.POILogFactory; +import org.apache.poi.util.POILogger; +import org.apache.poi.util.PackageHelper; +import org.apache.poi.util.Units; import org.apache.poi.xslf.XSLFSlideShow; import org.apache.xmlbeans.XmlException; import org.apache.xmlbeans.XmlObject; @@ -35,17 +55,6 @@ import org.openxmlformats.schemas.presentationml.x2006.main.CTSlideIdListEntry; import org.openxmlformats.schemas.presentationml.x2006.main.CTSlideSize; import org.openxmlformats.schemas.presentationml.x2006.main.PresentationDocument; -import java.awt.Dimension; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.regex.Pattern; - /** * High level representation of a ooxml slideshow. * This is the first object most users will construct whether @@ -163,6 +172,7 @@ public class XMLSlideShow extends POIXMLDocument { /** * Get the document's embedded files. */ + @Override public List getAllEmbedds() throws OpenXML4JException { return Collections.unmodifiableList( getPackage().getPartsByName(Pattern.compile("/ppt/embeddings/.*?")) diff --git a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFPictureData.java b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFPictureData.java index 2122e62d26..7ade4f948d 100644 --- a/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFPictureData.java +++ b/src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFPictureData.java @@ -19,6 +19,8 @@ package org.apache.poi.xslf.usermodel; +import java.io.IOException; + import org.apache.poi.POIXMLDocumentPart; import org.apache.poi.POIXMLException; import org.apache.poi.POIXMLRelation; @@ -27,8 +29,6 @@ import org.apache.poi.openxml4j.opc.PackageRelationship; import org.apache.poi.util.Beta; import org.apache.poi.util.IOUtils; -import java.io.IOException; - /** * Instantiates sub-classes of POIXMLDocumentPart depending on their relationship type * @@ -206,4 +206,13 @@ public final class XSLFPictureData extends POIXMLDocumentPart { } return checksum; } + + /** + * *PictureData objects store the actual content in the part directly without keeping a + * copy like all others therefore we need to handle them differently. + */ + @Override + protected void prepareForCommit() { + // do not clear the part here + } } \ No newline at end of file diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPictureData.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPictureData.java index 391db1ea4e..43599feb75 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPictureData.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPictureData.java @@ -129,4 +129,13 @@ public class XSSFPictureData extends POIXMLDocumentPart implements PictureData { public String getMimeType() { return getPackagePart().getContentType(); } + + /** + * *PictureData objects store the actual content in the part directly without keeping a + * copy like all others therefore we need to handle them differently. + */ + @Override + protected void prepareForCommit() { + // do not clear the part here + } } diff --git a/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFPictureData.java b/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFPictureData.java index 532d40b00f..e819da5dbb 100644 --- a/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFPictureData.java +++ b/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFPictureData.java @@ -234,4 +234,13 @@ public class XWPFPictureData extends POIXMLDocumentPart { public int hashCode() { return getChecksum().hashCode(); } + + /** + * *PictureData objects store the actual content in the part directly without keeping a + * copy like all others therefore we need to handle them differently. + */ + @Override + protected void prepareForCommit() { + // do not clear the part here + } } diff --git a/src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFPictureShape.java b/src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFPictureShape.java index 2cbaeed63e..955b273c1e 100644 --- a/src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFPictureShape.java +++ b/src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFPictureShape.java @@ -16,15 +16,17 @@ ==================================================================== */ package org.apache.poi.xslf.usermodel; -import junit.framework.TestCase; -import org.apache.poi.xslf.XSLFTestDataSamples; -import org.openxmlformats.schemas.presentationml.x2006.main.CTPicture; +import static org.junit.Assert.assertArrayEquals; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import junit.framework.TestCase; + +import org.apache.poi.xslf.XSLFTestDataSamples; +import org.openxmlformats.schemas.presentationml.x2006.main.CTPicture; + /** * @author Yegor Kozlov */ @@ -34,6 +36,7 @@ public class TestXSLFPictureShape extends TestCase { XMLSlideShow ppt = new XMLSlideShow(); assertEquals(0, ppt.getAllPictures().size()); byte[] data1 = new byte[100]; + for(int i = 0;i < 100;i++) { data1[i] = (byte)i; } int idx1 = ppt.addPicture(data1, XSLFPictureData.PICTURE_TYPE_JPEG); assertEquals(0, idx1); assertEquals(1, ppt.getAllPictures().size()); @@ -41,25 +44,26 @@ public class TestXSLFPictureShape extends TestCase { XSLFSlide slide = ppt.createSlide(); XSLFPictureShape shape1 = slide.createPicture(idx1); assertNotNull(shape1.getPictureData()); - assertTrue(Arrays.equals(data1, shape1.getPictureData().getData())); + assertArrayEquals(data1, shape1.getPictureData().getData()); byte[] data2 = new byte[200]; + for(int i = 0;i < 200;i++) { data2[i] = (byte)i; } int idx2 = ppt.addPicture(data2, XSLFPictureData.PICTURE_TYPE_PNG); XSLFPictureShape shape2 = slide.createPicture(idx2); assertNotNull(shape2.getPictureData()); assertEquals(1, idx2); assertEquals(2, ppt.getAllPictures().size()); - assertTrue(Arrays.equals(data2, shape2.getPictureData().getData())); + assertArrayEquals(data2, shape2.getPictureData().getData()); ppt = XSLFTestDataSamples.writeOutAndReadBack(ppt); List pics = ppt.getAllPictures(); assertEquals(2, pics.size()); - assertTrue(Arrays.equals(data1, pics.get(0).getData())); - assertTrue(Arrays.equals(data2, pics.get(1).getData())); + assertArrayEquals(data1, pics.get(0).getData()); + assertArrayEquals(data2, pics.get(1).getData()); XSLFShape[] shapes = ppt.getSlides()[0].getShapes(); - assertTrue(Arrays.equals(data1, ((XSLFPictureShape) shapes[0]).getPictureData().getData())); - assertTrue(Arrays.equals(data2, ((XSLFPictureShape) shapes[1]).getPictureData().getData())); + assertArrayEquals(data1, ((XSLFPictureShape) shapes[0]).getPictureData().getData()); + assertArrayEquals(data2, ((XSLFPictureShape) shapes[1]).getPictureData().getData()); } public void testCreateMultiplePictures() { @@ -79,7 +83,7 @@ public class TestXSLFPictureShape extends TestCase { // POI saves images as image1.png, image2.png, etc. String fileName = "image" + (elementIndex + 1) + ".png"; assertEquals(fileName, picture.getPictureData().getFileName()); - assertTrue(Arrays.equals(data, picture.getPictureData().getData())); + assertArrayEquals(data, picture.getPictureData().getData()); } // and then add next 20 images to a group @@ -92,7 +96,7 @@ public class TestXSLFPictureShape extends TestCase { assertEquals(pictureIndex, elementIndex); // added images have indexes 0,1,2....19 String fileName = "image" + (pictureIndex + 1) + ".png"; assertEquals(fileName, picture.getPictureData().getFileName()); - assertTrue(Arrays.equals(data, picture.getPictureData().getData())); + assertArrayEquals(data, picture.getPictureData().getData()); } // serialize, read back and check that all images are there @@ -110,7 +114,7 @@ public class TestXSLFPictureShape extends TestCase { XSLFPictureData data = pics.get(fileName); assertNotNull(data); assertEquals(fileName, data.getFileName()); - assertTrue(Arrays.equals(data1, data.getData())); + assertArrayEquals(data1, data.getData()); } } @@ -127,7 +131,9 @@ public class TestXSLFPictureShape extends TestCase { assertEquals(1, ppt.addPicture(img2, XSLFPictureData.PICTURE_TYPE_PNG)); XSLFSlide slide1 = ppt.createSlide(); + assertNotNull(slide1); XSLFSlide slide2 = ppt.createSlide(); + assertNotNull(slide2); } @@ -146,7 +152,7 @@ public class TestXSLFPictureShape extends TestCase { XSLFSlide slide2 = ppt2.createSlide().importContent(slide1); XSLFPictureShape shape2 = (XSLFPictureShape)slide2.getShapes()[0]; - assertTrue(Arrays.equals(data1, shape2.getPictureData().getData())); + assertArrayEquals(data1, shape2.getPictureData().getData()); CTPicture ctPic2 = (CTPicture)shape2.getXmlObject(); assertFalse(ctPic2.getNvPicPr().getNvPr().isSetCustDataLst()); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/model/TestSharedStringsTable.java b/src/ooxml/testcases/org/apache/poi/xssf/model/TestSharedStringsTable.java index 0298b05173..e06bd213f3 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/model/TestSharedStringsTable.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/model/TestSharedStringsTable.java @@ -17,19 +17,21 @@ package org.apache.poi.xssf.model; -import java.util.List; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; import java.util.ArrayList; -import java.io.*; +import java.util.List; import junit.framework.TestCase; +import org.apache.poi.POIDataSamples; +import org.apache.poi.POIXMLException; +import org.apache.poi.ss.usermodel.Sheet; +import org.apache.poi.ss.usermodel.Workbook; import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.usermodel.XSSFRichTextString; import org.apache.poi.xssf.usermodel.XSSFWorkbook; -import org.apache.poi.ss.usermodel.Workbook; -import org.apache.poi.ss.usermodel.Sheet; -import org.apache.poi.POIDataSamples; -import org.apache.poi.POIXMLException; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTRElt; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTRPrElt; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTRst; @@ -130,6 +132,8 @@ public final class TestSharedStringsTable extends TestCase { CTRst st2 = items2.get(i); assertEquals(st1.toString(), st2.toString()); } + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } /** @@ -159,6 +163,8 @@ public final class TestSharedStringsTable extends TestCase { String val = s.getRow(i++).getCell(0).getStringCellValue(); assertEquals(str, val); } + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(w)); } private List readStrings(String filename) throws IOException { diff --git a/src/ooxml/testcases/org/apache/poi/xssf/model/TestStylesTable.java b/src/ooxml/testcases/org/apache/poi/xssf/model/TestStylesTable.java index 73ec1d005f..01fa47fe69 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/model/TestStylesTable.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/model/TestStylesTable.java @@ -17,11 +17,11 @@ package org.apache.poi.xssf.model; +import junit.framework.TestCase; + +import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.usermodel.XSSFCellStyle; import org.apache.poi.xssf.usermodel.XSSFWorkbook; -import org.apache.poi.xssf.XSSFTestDataSamples; - -import junit.framework.TestCase; public final class TestStylesTable extends TestCase { private String testFile = "Formatting.xlsx"; @@ -51,6 +51,8 @@ public final class TestStylesTable extends TestCase { assertEquals(1, st._getXfsSize()); assertEquals(1, st._getStyleXfsSize()); assertEquals(0, st._getNumberFormatSize()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } public void testLoadExisting() { @@ -60,7 +62,10 @@ public final class TestStylesTable extends TestCase { StylesTable st = workbook.getStylesSource(); doTestExisting(st); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(workbook)); } + public void testLoadSaveLoad() { XSSFWorkbook workbook = XSSFTestDataSamples.openSampleWorkbook(testFile); assertNotNull(workbook.getStylesSource()); @@ -71,6 +76,7 @@ public final class TestStylesTable extends TestCase { st = XSSFTestDataSamples.writeOutAndReadBack(workbook).getStylesSource(); doTestExisting(st); } + public void doTestExisting(StylesTable st) { // Check contents assertNotNull(st.getCTStylesheet()); @@ -123,6 +129,8 @@ public final class TestStylesTable extends TestCase { assertEquals("yyyy-mm-dd", st.getNumberFormatAt(nf1)); assertEquals(nf1, st.putNumberFormat("yyyy-mm-dd")); assertEquals(nf2, st.putNumberFormat("yyyy-mm-DD")); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } public void testPopulateExisting() { @@ -147,5 +155,7 @@ public final class TestStylesTable extends TestCase { assertEquals("YYYY-mm-dd", st.getNumberFormatAt(nf1)); assertEquals(nf1, st.putNumberFormat("YYYY-mm-dd")); assertEquals(nf2, st.putNumberFormat("YYYY-mm-DD")); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(workbook)); } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java index 19e9a65a32..8d427be552 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFCellStyle.java @@ -31,16 +31,7 @@ import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.model.StylesTable; import org.apache.poi.xssf.usermodel.extensions.XSSFCellBorder; import org.apache.poi.xssf.usermodel.extensions.XSSFCellFill; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTBorder; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCellXfs; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFill; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTFont; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTStylesheet; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTXf; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.STBorderStyle; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.STHorizontalAlignment; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.STPatternType; -import org.openxmlformats.schemas.spreadsheetml.x2006.main.STVerticalAlignment; +import org.openxmlformats.schemas.spreadsheetml.x2006.main.*; public class TestXSSFCellStyle extends TestCase { private StylesTable stylesTable; @@ -568,6 +559,8 @@ public class TestXSSFCellStyle extends TestCase { assertEquals(IndexedColors.AUTOMATIC.getIndex(), style1.getFillBackgroundColor()); assertNull(style1.getFillBackgroundXSSFColor()); + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb1)); + //compatibility with HSSF HSSFWorkbook wb2 = new HSSFWorkbook(); HSSFCellStyle style2 = wb2.createCellStyle(); @@ -588,7 +581,6 @@ public class TestXSSFCellStyle extends TestCase { public void testGetFillForegroundColor() { - XSSFWorkbook wb = new XSSFWorkbook(); StylesTable styles = wb.getStylesSource(); assertEquals(1, wb.getNumCellStyles()); @@ -620,6 +612,8 @@ public class TestXSSFCellStyle extends TestCase { assertEquals(IndexedColors.BRIGHT_GREEN.getIndex(), style.getFillForegroundColor()); assertEquals(4, styles.getFills().size()); } + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } public void testGetFillPattern() { @@ -752,7 +746,10 @@ public class TestXSSFCellStyle extends TestCase { assertTrue(fnt == clone.getFont()); assertTrue(18 == clone.getDataFormat()); assertEquals(2, wb.getNumberOfFonts()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } + /** * Cloning one XSSFCellStyle onto Another, different XSSFWorkbooks */ @@ -820,6 +817,9 @@ public class TestXSSFCellStyle extends TestCase { assertEquals("TestingFont", reload.getFont().getFontName()); assertEquals(fmtClone.getFormat("Test##"), reload.getDataFormat()); assertFalse(fmtClone.getFormat("Test##") == fmt.getFormat("Test##")); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wbOrig)); + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wbClone)); } /** @@ -831,9 +831,10 @@ public class TestXSSFCellStyle extends TestCase { StylesTable st = workbook.getStylesSource(); assertEquals(0, st._getStyleXfsSize()); - XSSFCellStyle style = workbook.createCellStyle(); // no exception at this point assertNull(style.getStyleXf()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(workbook)); } /** @@ -848,6 +849,8 @@ public class TestXSSFCellStyle extends TestCase { // no exception at this point XSSFCellStyle style = workbook.getSheetAt(0).getRow(0).getCell(0).getCellStyle(); assertNull(style.getStyleXf()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(workbook)); } public void testShrinkToFit() { @@ -878,5 +881,8 @@ public class TestXSSFCellStyle extends TestCase { r = s.getRow(0); assertEquals(false, r.getCell(0).getCellStyle().getShrinkToFit()); assertEquals(true, r.getCell(1).getCellStyle().getShrinkToFit()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wbOrig)); } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFChart.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFChart.java index 00b0e36b2c..1c8732402e 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFChart.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFChart.java @@ -32,6 +32,8 @@ public final class TestXSSFChart extends TestCase { assertEquals(0, s1.getRelations().size()); assertEquals(1, s2.getRelations().size()); assertEquals(1, s3.getRelations().size()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } public void testGetCharts() throws Exception { @@ -54,6 +56,8 @@ public final class TestXSSFChart extends TestCase { chart = s3.createDrawingPatriarch().getCharts().get(0); assertEquals("Sheet 3 Chart with Title", chart.getTitle().getString()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } public void testAddChartsToNewWorkbook() throws Exception { @@ -69,6 +73,9 @@ public final class TestXSSFChart extends TestCase { XSSFClientAnchor a2 = new XSSFClientAnchor(0, 0, 0, 0, 1, 11, 10, 60); XSSFChart c2 = d1.createChart(a2); + assertNotNull(c2); assertEquals(2, d1.getCharts().size()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFComment.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFComment.java index 889c0d8e70..2ca3dbaa91 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFComment.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFComment.java @@ -22,6 +22,7 @@ import org.apache.poi.ss.usermodel.BaseTestCellComment; import org.apache.poi.ss.usermodel.IndexedColors; import org.apache.poi.ss.util.CellReference; import org.apache.poi.xssf.XSSFITestDataProvider; +import org.apache.poi.xssf.XSSFTestDataSamples; import org.apache.poi.xssf.model.CommentsTable; import org.apache.xmlbeans.XmlObject; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTComment; @@ -146,6 +147,8 @@ public final class TestXSSFComment extends BaseTestCellComment { assertEquals(8.5, rPr.getSzArray(0).getVal()); assertEquals(IndexedColors.BLUE_GREY.getIndex(), rPr.getColorArray(0).getIndexed()); assertEquals("Tahoma", rPr.getRFontArray(0).getVal()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } public void testAuthor() { diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDrawing.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDrawing.java index 77f48c74aa..3eea634e18 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDrawing.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFDrawing.java @@ -16,11 +16,7 @@ ==================================================================== */ package org.apache.poi.xssf.usermodel; -import java.awt.*; -import java.io.File; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.OutputStream; +import java.awt.Color; import java.util.Arrays; import java.util.List; @@ -68,6 +64,7 @@ public class TestXSSFDrawing extends TestCase { for(XSSFShape sh : shapes) assertNotNull(sh.getAnchor()); + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } public void testNew() throws Exception { @@ -94,6 +91,7 @@ public class TestXSSFDrawing extends TestCase { c1.setLineStyle(1); XSSFShapeGroup c2 = drawing.createGroup(new XSSFClientAnchor(0,0,0,0,0,0,5,5)); + assertNotNull(c2); XSSFSimpleShape c3 = drawing.createSimpleShape(new XSSFClientAnchor(0,0,0,0,2,2,3,4)); c3.setText(new XSSFRichTextString("Test String")); @@ -139,6 +137,8 @@ public class TestXSSFDrawing extends TestCase { String xml = ctDrawing.toString(); assertTrue(xml.contains("xmlns:xdr=\"http://schemas.openxmlformats.org/drawingml/2006/spreadsheetDrawing\"")); assertTrue(xml.contains("xmlns:a=\"http://schemas.openxmlformats.org/drawingml/2006/main\"")); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } public void testMultipleDrawings(){ @@ -146,9 +146,12 @@ public class TestXSSFDrawing extends TestCase { for (int i = 0; i < 3; i++) { XSSFSheet sheet = wb.createSheet(); XSSFDrawing drawing = sheet.createDrawingPatriarch(); + assertNotNull(drawing); } OPCPackage pkg = wb.getPackage(); assertEquals(3, pkg.getPartsByContentType(XSSFRelation.DRAWINGS.getContentType()).size()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } public void testClone() throws Exception{ @@ -175,12 +178,14 @@ public class TestXSSFDrawing extends TestCase { assertEquals(shapes1.size(), shapes2.size()); for(int i = 0; i < shapes1.size(); i++){ - XSSFShape sh1 = (XSSFShape)shapes1.get(i); - XSSFShape sh2 = (XSSFShape)shapes2.get(i); + XSSFShape sh1 = shapes1.get(i); + XSSFShape sh2 = shapes2.get(i); assertTrue(sh1.getClass() == sh2.getClass()); assertEquals(sh1.getShapeProperties().toString(), sh2.getShapeProperties().toString()); } + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } /** @@ -216,7 +221,8 @@ public class TestXSSFDrawing extends TestCase { assertTrue(Arrays.equals( new byte[]{0, (byte)128, (byte)128} , rPr.getSolidFill().getSrgbClr().getVal())); - + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } /** @@ -229,13 +235,16 @@ public class TestXSSFDrawing extends TestCase { XSSFClientAnchor anchor1 = new XSSFClientAnchor(0, 0, 0, 0, 2, 2, 3, 4); XSSFShape shape1 = drawing.createTextbox(anchor1); + assertNotNull(shape1); XSSFClientAnchor anchor2 = new XSSFClientAnchor(0, 0, 0, 0, 2, 2, 3, 5); XSSFShape shape2 = drawing.createTextbox(anchor2); + assertNotNull(shape2); int pictureIndex= wb.addPicture(new byte[]{}, XSSFWorkbook.PICTURE_TYPE_PNG); XSSFClientAnchor anchor3 = new XSSFClientAnchor(0, 0, 0, 0, 2, 2, 3, 6); XSSFShape shape3 = drawing.createPicture(anchor3, pictureIndex); + assertNotNull(shape3); wb = XSSFTestDataSamples.writeOutAndReadBack(wb); sheet = wb.getSheetAt(0); @@ -244,8 +253,8 @@ public class TestXSSFDrawing extends TestCase { assertEquals(shapes.get(0).getAnchor(), anchor1); assertEquals(shapes.get(1).getAnchor(), anchor2); assertEquals(shapes.get(2).getAnchor(), anchor3); - - + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } /** @@ -277,14 +286,14 @@ public class TestXSSFDrawing extends TestCase { assertTrue(Arrays.equals( new byte[]{0, (byte)128, (byte)128} , rPr.getSolidFill().getSrgbClr().getVal())); - + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } /** * Test setText single paragraph to ensure backwards compatibility */ public void testSetTextSingleParagraph() { - XSSFWorkbook wb = new XSSFWorkbook(); XSSFSheet sheet = wb.createSheet(); XSSFDrawing drawing = sheet.createDrawingPatriarch(); @@ -311,13 +320,14 @@ public class TestXSSFDrawing extends TestCase { assertTrue(Arrays.equals( new int[] { 0, 255, 255 } , new int[] { clr.getRed(), clr.getGreen(), clr.getBlue() })); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } /** * Test addNewTextParagraph */ public void testAddNewTextParagraph() { - XSSFWorkbook wb = new XSSFWorkbook(); XSSFSheet sheet = wb.createSheet(); XSSFDrawing drawing = sheet.createDrawingPatriarch(); @@ -333,13 +343,14 @@ public class TestXSSFDrawing extends TestCase { List runs = para.getTextRuns(); assertEquals(1, runs.size()); assertEquals("Line 1", runs.get(0).getText()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } /** * Test addNewTextParagraph using RichTextString */ public void testAddNewTextParagraphWithRTS() { - XSSFWorkbook wb = new XSSFWorkbook(); XSSFSheet sheet = wb.createSheet(); XSSFDrawing drawing = sheet.createDrawingPatriarch(); @@ -402,13 +413,14 @@ public class TestXSSFDrawing extends TestCase { assertTrue(Arrays.equals( new int[] { 0, 255, 255 } , new int[] { clr.getRed(), clr.getGreen(), clr.getBlue() })); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } /** * Test add multiple paragraphs and retrieve text */ public void testAddMultipleParagraphs() { - XSSFWorkbook wb = new XSSFWorkbook(); XSSFSheet sheet = wb.createSheet(); XSSFDrawing drawing = sheet.createDrawingPatriarch(); @@ -427,13 +439,14 @@ public class TestXSSFDrawing extends TestCase { List paras = shape.getTextParagraphs(); assertEquals(4, paras.size()); // this should be 4 as XSSFSimpleShape creates a default paragraph (no text), and then we added 3 paragraphs assertEquals("Line 1\nLine 2\nLine 3", shape.getText()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } /** * Test setting the text, then adding multiple paragraphs and retrieve text */ public void testSetAddMultipleParagraphs() { - XSSFWorkbook wb = new XSSFWorkbook(); XSSFSheet sheet = wb.createSheet(); XSSFDrawing drawing = sheet.createDrawingPatriarch(); @@ -451,6 +464,8 @@ public class TestXSSFDrawing extends TestCase { List paras = shape.getTextParagraphs(); assertEquals(3, paras.size()); // this should be 3 as we overwrote the default paragraph with setText, then added 2 new paragraphs assertEquals("Line 1\nLine 2\nLine 3", shape.getText()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } /** @@ -480,6 +495,8 @@ public class TestXSSFDrawing extends TestCase { XSSFSimpleShape textbox = (XSSFSimpleShape) shapes.get(4); assertEquals("Sheet with various pictures\n(jpeg, png, wmf, emf and pict)", textbox.getText()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } @@ -538,7 +555,10 @@ public class TestXSSFDrawing extends TestCase { assertTrue(Arrays.equals( new int[] { 0, 0, 255 } , new int[] { clr.getRed(), clr.getGreen(), clr.getBlue() })); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } + /** * Test adding and reading back paragraphs as bullet points */ @@ -644,6 +664,8 @@ public class TestXSSFDrawing extends TestCase { builder.append(paraString10); assertEquals(builder.toString(), sshape.getText()); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } /** @@ -668,5 +690,7 @@ public class TestXSSFDrawing extends TestCase { sb.append("\t\n\t\n\t\n\t"); assertEquals(sb.toString(), extracted); + + assertNotNull(XSSFTestDataSamples.writeOutAndReadBack(wb)); } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFPictureData.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFPictureData.java index 85fe049936..fe06afcd89 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFPictureData.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFPictureData.java @@ -17,7 +17,8 @@ package org.apache.poi.xssf.usermodel; -import java.util.Arrays; +import static org.junit.Assert.assertArrayEquals; + import java.util.List; import junit.framework.TestCase; @@ -52,7 +53,7 @@ public final class TestXSSFPictureData extends TestCase { assertEquals(pictures.size() - 1, idx); XSSFPictureData pict = pictures.get(idx); assertEquals("jpeg", pict.suggestFileExtension()); - assertTrue(Arrays.equals(pictureData, pict.getData())); + assertArrayEquals(pictureData, pict.getData()); } public void testNew(){ @@ -70,22 +71,25 @@ public final class TestXSSFPictureData extends TestCase { int jpegIdx = wb.addPicture(jpegData, XSSFWorkbook.PICTURE_TYPE_JPEG); assertEquals(1, pictures.size()); assertEquals("jpeg", pictures.get(jpegIdx).suggestFileExtension()); - assertTrue(Arrays.equals(jpegData, pictures.get(jpegIdx).getData())); + assertArrayEquals(jpegData, pictures.get(jpegIdx).getData()); int wmfIdx = wb.addPicture(wmfData, XSSFWorkbook.PICTURE_TYPE_WMF); assertEquals(2, pictures.size()); assertEquals("wmf", pictures.get(wmfIdx).suggestFileExtension()); - assertTrue(Arrays.equals(wmfData, pictures.get(wmfIdx).getData())); + assertArrayEquals(wmfData, pictures.get(wmfIdx).getData()); int pngIdx = wb.addPicture(pngData, XSSFWorkbook.PICTURE_TYPE_PNG); assertEquals(3, pictures.size()); assertEquals("png", pictures.get(pngIdx).suggestFileExtension()); - assertTrue(Arrays.equals(pngData, pictures.get(pngIdx).getData())); + assertArrayEquals(pngData, pictures.get(pngIdx).getData()); //TODO finish usermodel API for XSSFPicture XSSFPicture p1 = drawing.createPicture(new XSSFClientAnchor(), jpegIdx); + assertNotNull(p1); XSSFPicture p2 = drawing.createPicture(new XSSFClientAnchor(), wmfIdx); + assertNotNull(p2); XSSFPicture p3 = drawing.createPicture(new XSSFClientAnchor(), pngIdx); + assertNotNull(p3); //check that the added pictures are accessible after write wb = XSSFTestDataSamples.writeOutAndReadBack(wb); @@ -93,13 +97,13 @@ public final class TestXSSFPictureData extends TestCase { assertEquals(3, pictures2.size()); assertEquals("jpeg", pictures2.get(jpegIdx).suggestFileExtension()); - assertTrue(Arrays.equals(jpegData, pictures2.get(jpegIdx).getData())); + assertArrayEquals(jpegData, pictures2.get(jpegIdx).getData()); assertEquals("wmf", pictures2.get(wmfIdx).suggestFileExtension()); - assertTrue(Arrays.equals(wmfData, pictures2.get(wmfIdx).getData())); + assertArrayEquals(wmfData, pictures2.get(wmfIdx).getData()); assertEquals("png", pictures2.get(pngIdx).suggestFileExtension()); - assertTrue(Arrays.equals(pngData, pictures2.get(pngIdx).getData())); + assertArrayEquals(pngData, pictures2.get(pngIdx).getData()); } @@ -109,10 +113,14 @@ public final class TestXSSFPictureData extends TestCase { public void test53568(){ XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("53568.xlsx"); List pictures = wb.getAllPictures(); + assertNotNull(pictures); + assertEquals(4, pictures.size()); XSSFSheet sheet1 = wb.getSheetAt(0); List shapes1 = sheet1.createDrawingPatriarch().getShapes(); - + assertNotNull(shapes1); + assertEquals(5, shapes1.size()); + for(int i = 0; i < wb.getNumberOfSheets(); i++){ XSSFSheet sheet = wb.getSheetAt(i); XSSFDrawing drawing = sheet.createDrawingPatriarch(); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java index 0ea3f55811..4f09813b11 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java @@ -20,6 +20,7 @@ package org.apache.poi.xssf.usermodel; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileOutputStream; +import java.io.IOException; import java.io.OutputStream; import java.util.List; import java.util.zip.CRC32; @@ -31,6 +32,7 @@ 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.internal.MemoryPackagePart; import org.apache.poi.openxml4j.opc.internal.PackagePropertiesPart; import org.apache.poi.ss.usermodel.BaseTestWorkbook; import org.apache.poi.ss.usermodel.CellStyle; @@ -40,6 +42,7 @@ import org.apache.poi.ss.usermodel.RichTextString; import org.apache.poi.ss.usermodel.Row; import org.apache.poi.ss.usermodel.Sheet; import org.apache.poi.ss.usermodel.Workbook; +import org.apache.poi.util.IOUtils; import org.apache.poi.util.TempFile; import org.apache.poi.xssf.XSSFITestDataProvider; import org.apache.poi.xssf.XSSFTestDataSamples; @@ -574,4 +577,86 @@ public final class TestXSSFWorkbook extends BaseTestWorkbook { Workbook read = XSSFTestDataSamples.writeOutAndReadBack(workbook); assertSheetOrder(read, "Sheet2", "Sheet0", "Sheet1"); } + + public void testBug51158() throws IOException { + // create a workbook + final XSSFWorkbook workbook = new XSSFWorkbook(); + XSSFSheet sheet = workbook.createSheet("Test Sheet"); + XSSFRow row = sheet.createRow(2); + XSSFCell cell = row.createCell(3); + cell.setCellValue("test1"); + + //XSSFCreationHelper helper = workbook.getCreationHelper(); + //cell.setHyperlink(helper.createHyperlink(0)); + + XSSFComment comment = sheet.createDrawingPatriarch().createCellComment(new XSSFClientAnchor()); + assertNotNull(comment); + comment.setString("some comment"); + +// CellStyle cs = workbook.createCellStyle(); +// cs.setShrinkToFit(false); +// row.createCell(0).setCellStyle(cs); + + // write the first excel file + XSSFWorkbook readBack = XSSFTestDataSamples.writeOutAndReadBack(workbook); + assertNotNull(readBack); + assertEquals("test1", readBack.getSheetAt(0).getRow(2).getCell(3).getStringCellValue()); + assertNull(readBack.getSheetAt(0).getRow(2).getCell(4)); + + // add a new cell to the sheet + cell = row.createCell(4); + cell.setCellValue("test2"); + + // write the second excel file + readBack = XSSFTestDataSamples.writeOutAndReadBack(workbook); + assertNotNull(readBack); + assertEquals("test1", readBack.getSheetAt(0).getRow(2).getCell(3).getStringCellValue()); + assertEquals("test2", readBack.getSheetAt(0).getRow(2).getCell(4).getStringCellValue()); + } + + public void testBug51158a() throws IOException { + // create a workbook + final XSSFWorkbook workbook = new XSSFWorkbook(); + workbook.createSheet("Test Sheet"); + + XSSFSheet sheetBack = workbook.getSheetAt(0); + + // committing twice did add the XML twice without clearing the part in between + sheetBack.commit(); + + // ensure that a memory based package part does not have lingering data from previous commit() calls + if(sheetBack.getPackagePart() instanceof MemoryPackagePart) { + ((MemoryPackagePart)sheetBack.getPackagePart()).clear(); + } + + sheetBack.commit(); + + String str = new String(IOUtils.toByteArray(sheetBack.getPackagePart().getInputStream())); + System.out.println(str); + + assertEquals(1, countMatches(str, " pictures = doc.getAllPictures(); assertEquals(0,pictures.size()); @@ -113,20 +125,16 @@ public class TestXWPFPictureData extends TestCase { assertEquals("/word/media/image1.jpeg",jpegRel.getTargetURI().getPath()); XWPFPictureData pictureDataByID = doc.getPictureDataByID(jpegRel.getId()); - byte[] newJPEGData = pictureDataByID.getData(); - assertEquals(newJPEGData.length,jpegData.length); - for (int i = 0 ; i < newJPEGData.length ; i++) - { - assertEquals(newJPEGData[i],jpegData[i]); - } + assertArrayEquals(jpegData, pictureDataByID.getData()); // Save an re-load, check it appears doc = XWPFTestDataSamples.writeOutAndReadBack(doc); assertEquals(1,doc.getAllPictures().size()); assertEquals(1,doc.getAllPackagePictures().size()); - } - - public void testGetChecksum() { + + // verify the picture that we read back in + pictureDataByID = doc.getPictureDataByID(jpegRel.getId()); + assertArrayEquals(jpegData, pictureDataByID.getData()); } @@ -147,8 +155,4 @@ public class TestXWPFPictureData extends TestCase { } } - - private void process(XWPFParagraph paragraph){ - - } } -- 2.39.5