From: Yegor Kozlov Date: Wed, 10 Oct 2012 10:48:10 +0000 (+0000) Subject: Bugzilla 53568: Fixed null returned by XSSFPicture.getPictureData() X-Git-Tag: 3.10-beta1~129 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=95366195ff6f2960b8a912bd02858f50b02e573e;p=poi.git Bugzilla 53568: Fixed null returned by XSSFPicture.getPictureData() git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1396539 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 69870b19c5..139ea60030 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 53568 - Fixed null returned by XSSFPicture.getPictureData() 53950 - fixed setForceFormulaRecalculation to reset workbook-level "manual" flag 52211 - avoid unnessary re-coverting content types to US-ASCII, it can cause exceptions on ibm mainframes 53568 - Set shapes anchors in XSSF when reading from existing drawings diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPicture.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPicture.java index ee8bbe7e49..47402bdc35 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPicture.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFPicture.java @@ -290,13 +290,7 @@ public final class XSSFPicture extends XSSFShape implements Picture { */ public XSSFPictureData getPictureData() { String blipId = ctPicture.getBlipFill().getBlip().getEmbed(); - for (POIXMLDocumentPart part : getDrawing().getRelations()) { - if(part.getPackageRelationship().getId().equals(blipId)){ - return (XSSFPictureData)part; - } - } - logger.log(POILogger.WARN, "Picture data was not found for blipId=" + blipId); - return null; + return (XSSFPictureData)getDrawing().getRelationById(blipId); } protected CTShapeProperties getShapeProperties(){ diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java index f50c42a150..d7a0e00b98 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java @@ -23,13 +23,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashMap; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.regex.Pattern; import javax.xml.namespace.QName; @@ -56,6 +50,7 @@ import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.ss.util.CellReference; import org.apache.poi.ss.util.WorkbookUtil; import org.apache.poi.util.*; +import org.apache.poi.xslf.usermodel.XSLFPictureData; import org.apache.poi.xssf.model.*; import org.apache.poi.xssf.usermodel.helpers.XSSFFormulaUtils; import org.apache.xmlbeans.XmlException; @@ -694,27 +689,18 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable getAllPictures() { - if(pictures == null) { - //In OOXML pictures are referred to in sheets, - //dive into sheet's relations, select drawings and their images - pictures = new ArrayList(); - for(XSSFSheet sh : sheets){ - for(POIXMLDocumentPart dr : sh.getRelations()){ - if(dr instanceof XSSFDrawing){ - for(POIXMLDocumentPart img : dr.getRelations()){ - if(img instanceof XSSFPictureData){ - pictures.add((XSSFPictureData)img); - } - } - } - } + if(pictures == null){ + List mediaParts = getPackage().getPartsByName(Pattern.compile("/xl/media/.*?")); + pictures = new ArrayList(mediaParts.size()); + for(PackagePart part : mediaParts){ + pictures.add(new XSSFPictureData(part, null)); } } - return pictures; + return pictures; //YK: should return Collections.unmodifiableList(pictures); } /** - * gGet the cell style object at the given index + * Get the cell style object at the given index * * @param idx index within the set of styles * @return XSSFCellStyle object at the index diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFPicture.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFPicture.java index 2d0575d80f..0aa42349b7 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFPicture.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFPicture.java @@ -20,6 +20,7 @@ package org.apache.poi.xssf.usermodel; import org.apache.poi.ss.usermodel.ClientAnchor; import org.apache.poi.ss.usermodel.BaseTestPicture; import org.apache.poi.xssf.XSSFITestDataProvider; +import org.apache.poi.xssf.XSSFTestDataSamples; import org.openxmlformats.schemas.drawingml.x2006.spreadsheetDrawing.CTTwoCellAnchor; import org.openxmlformats.schemas.drawingml.x2006.spreadsheetDrawing.STEditAs; @@ -92,4 +93,52 @@ public final class TestXSSFPicture extends BaseTestPicture { XSSFPicture shape2 = drawing.createPicture(anchor, jpegIdx); assertEquals(2, shape2.getCTPicture().getNvPicPr().getCNvPr().getId()); } + + /** + * same image refrerred by mulitple sheets + */ + public void testMultiRelationShips(){ + XSSFWorkbook wb = new XSSFWorkbook(); + + byte[] pic1Data = "test jpeg data".getBytes(); + byte[] pic2Data = "test png data".getBytes(); + + List pictures = wb.getAllPictures(); + assertEquals(0, pictures.size()); + + int pic1 = wb.addPicture(pic1Data, XSSFWorkbook.PICTURE_TYPE_JPEG); + int pic2 = wb.addPicture(pic2Data, XSSFWorkbook.PICTURE_TYPE_PNG); + + XSSFSheet sheet1 = wb.createSheet(); + XSSFDrawing drawing1 = sheet1.createDrawingPatriarch(); + XSSFPicture shape1 = drawing1.createPicture(new XSSFClientAnchor(), pic1); + XSSFPicture shape2 = drawing1.createPicture(new XSSFClientAnchor(), pic2); + + XSSFSheet sheet2 = wb.createSheet(); + XSSFDrawing drawing2 = sheet2.createDrawingPatriarch(); + XSSFPicture shape3 = drawing2.createPicture(new XSSFClientAnchor(), pic2); + XSSFPicture shape4 = drawing2.createPicture(new XSSFClientAnchor(), pic1); + + assertEquals(2, pictures.size()); + + wb = XSSFTestDataSamples.writeOutAndReadBack(wb); + pictures = wb.getAllPictures(); + assertEquals(2, pictures.size()); + + sheet1 = wb.getSheetAt(0); + drawing1 = sheet1.createDrawingPatriarch(); + XSSFPicture shape11 = (XSSFPicture)drawing1.getShapes().get(0); + assertTrue(Arrays.equals(shape1.getPictureData().getData(), shape11.getPictureData().getData())); + XSSFPicture shape22 = (XSSFPicture)drawing1.getShapes().get(1); + assertTrue(Arrays.equals(shape2.getPictureData().getData(), shape22.getPictureData().getData())); + + sheet2 = wb.getSheetAt(1); + drawing2 = sheet2.createDrawingPatriarch(); + XSSFPicture shape33 = (XSSFPicture)drawing2.getShapes().get(0); + assertTrue(Arrays.equals(shape3.getPictureData().getData(), shape33.getPictureData().getData())); + XSSFPicture shape44 = (XSSFPicture)drawing2.getShapes().get(1); + assertTrue(Arrays.equals(shape4.getPictureData().getData(), shape44.getPictureData().getData())); + + } + } 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 55677e565e..85fe049936 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFPictureData.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFPictureData.java @@ -102,4 +102,28 @@ public final class TestXSSFPictureData extends TestCase { assertTrue(Arrays.equals(pngData, pictures2.get(pngIdx).getData())); } + + /** + * Bug 53568: XSSFPicture.getPictureData() can return null. + */ + public void test53568(){ + XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("53568.xlsx"); + List pictures = wb.getAllPictures(); + + XSSFSheet sheet1 = wb.getSheetAt(0); + List shapes1 = sheet1.createDrawingPatriarch().getShapes(); + + for(int i = 0; i < wb.getNumberOfSheets(); i++){ + XSSFSheet sheet = wb.getSheetAt(i); + XSSFDrawing drawing = sheet.createDrawingPatriarch(); + for(XSSFShape shape : drawing.getShapes()){ + if(shape instanceof XSSFPicture){ + XSSFPicture pic = (XSSFPicture)shape; + XSSFPictureData picData = pic.getPictureData(); + assertNotNull(picData); + } + } + } + + } } 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 60ad9fbbdb..3d80aa7fb9 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java @@ -277,7 +277,7 @@ public final class TestXSSFWorkbook extends BaseTestWorkbook { public void testBug47668() throws Exception { XSSFWorkbook workbook = XSSFTestDataSamples.openSampleWorkbook("47668.xlsx"); List allPictures = workbook.getAllPictures(); - assertEquals(2, allPictures.size()); + assertEquals(1, allPictures.size()); PackagePartName imagePartName = PackagingURIHelper .createPartName("/xl/media/image1.jpeg"); diff --git a/test-data/spreadsheet/53568.xlsx b/test-data/spreadsheet/53568.xlsx new file mode 100644 index 0000000000..082333b5bb Binary files /dev/null and b/test-data/spreadsheet/53568.xlsx differ