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

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.9-beta1" date="2012-??-??">
+          <action dev="poi-developers" type="fix">53568 - Fixed null returned by XSSFPicture.getPictureData()</action>
           <action dev="poi-developers" type="fix">53950 - fixed setForceFormulaRecalculation to reset workbook-level "manual" flag</action>
           <action dev="poi-developers" type="fix">52211 - avoid unnessary re-coverting content types to US-ASCII, it can cause exceptions on ibm mainframes</action>
           <action dev="poi-developers" type="fix">53568 - Set shapes anchors in XSSF when reading from existing drawings</action>

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()) {
+        return  (XSSFPictureData)getDrawing().getRelationById(blipId);
-            if(part.getPackageRelationship().getId().equals(blipId)){
-                return (XSSFPictureData)part;
-            }
-        }
-        logger.log(POILogger.WARN, "Picture data was not found for blipId=" + blipId);
-        return null;
     }
 
     protected CTShapeProperties getShapeProperties(){

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.*;
-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.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<X
      * @see #addPicture(byte[], int)
      */
     public List<XSSFPictureData> getAllPictures() {
-        if(pictures == null) {
+        if(pictures == null){
-            //In OOXML pictures are referred to in sheets,
+            List<PackagePart> mediaParts = getPackage().getPartsByName(Pattern.compile("/xl/media/.*?"));
-            //dive into sheet's relations, select drawings and their images
+            pictures = new ArrayList<XSSFPictureData>(mediaParts.size());
-            pictures = new ArrayList<XSSFPictureData>();
+            for(PackagePart part : mediaParts){
-            for(XSSFSheet sh : sheets){
+                pictures.add(new XSSFPictureData(part, null));
-                for(POIXMLDocumentPart dr : sh.getRelations()){
-                    if(dr instanceof XSSFDrawing){
-                        for(POIXMLDocumentPart img : dr.getRelations()){
-                            if(img instanceof XSSFPictureData){
-                                pictures.add((XSSFPictureData)img);
-                            }
-                        }
-                    }
-                }
             }
         }
-        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

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<XSSFPictureData> 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()));
+
+    }
+
 }

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<XSSFPictureData> pictures = wb.getAllPictures();
+
+        XSSFSheet sheet1 = wb.getSheetAt(0);
+        List<XSSFShape> 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);
+                }
+            }
+        }
+
+    }
 }

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<XSSFPictureData> allPictures = workbook.getAllPictures();
-		assertEquals(2, allPictures.size());
+		assertEquals(1, allPictures.size());
 
 		PackagePartName imagePartName = PackagingURIHelper
 				.createPartName("/xl/media/image1.jpeg");