#60499 - Deleting a picture that is used twice on a slide corrupt the slide

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

src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java

@@ -231,15 +231,35 @@ public class POIXMLDocumentPart {
      * @return the target part of the relation, or null, if none exists
      */
     public final POIXMLDocumentPart getRelationById(String id) {
-        RelationPart rp = relations.get(id);
+        RelationPart rp = getRelationPartById(id);
         return (rp == null) ? null : rp.getDocumentPart();
     }
 
     /**
-     * Returns the {@link PackageRelationship#getId()} of the
+     * Returns the target {@link RelationPart}, where a
+     * {@link PackageRelationship} is set from the {@link PackagePart} of this
+     * {@link POIXMLDocumentPart} to the {@link PackagePart} of the target
+     * {@link POIXMLDocumentPart} with a {@link PackageRelationship#getId()}
+     * matching the given parameter value.
+     *
+     * @param id
+     *            The relation id to look for
+     * @return the target relation part, or null, if none exists
+     * 
+     * @since 4.0.0
+     */
+    public final RelationPart getRelationPartById(String id) {
+        return relations.get(id);
+    }
+
+    /**
+     * Returns the first {@link PackageRelationship#getId()} of the
      * {@link PackageRelationship}, that sources from the {@link PackagePart} of
      * this {@link POIXMLDocumentPart} to the {@link PackagePart} of the given
-     * parameter value.
+     * parameter value.<p>
+     * 
+     * There can be multiple references to the given {@link POIXMLDocumentPart}
+     * and only the first in the order of creation is returned.
      *
      * @param part
      *            The {@link POIXMLDocumentPart} for which the according
@@ -292,7 +312,11 @@ public class POIXMLDocumentPart {
 
     /**
      * Remove the relation to the specified part in this package and remove the
-     * part, if it is no longer needed.
+     * part, if it is no longer needed.<p>
+     * 
+     * If there are multiple relationships to the same part, this will only
+     * remove the first relationship in the order of creation. The removal
+     * via the part id ({@link #removeRelation(String)} is preferred.
      * 
      * @param part the part which relation is to be removed from this document
      */
@@ -302,7 +326,11 @@ public class POIXMLDocumentPart {
 
     /**
      * Remove the relation to the specified part in this package and remove the
-     * part, if it is no longer needed and flag is set to true.
+     * part, if it is no longer needed and flag is set to true.<p>
+     *
+     * If there are multiple relationships to the same part, this will only
+     * remove the first relationship in the order of creation. The removal
+     * via the part id ({@link #removeRelation(String,boolean)} is preferred.
      *
      * @param part
      *            The related part, to which the relation shall be removed.
@@ -311,18 +339,53 @@ public class POIXMLDocumentPart {
      *            needed any longer.
      * @return true, if the relation was removed
      */
-    protected final boolean removeRelation(POIXMLDocumentPart part, boolean removeUnusedParts){
+    protected final boolean removeRelation(POIXMLDocumentPart part, boolean removeUnusedParts) {
         String id = getRelationId(part);
-        if (id == null) {
+        return removeRelation(id, removeUnusedParts);
+    }
+
+    /**
+     * Remove the relation to the specified part in this package and remove the
+     * part, if it is no longer needed.<p>
+     * 
+     * If there are multiple relationships to the same part, this will only
+     * remove the first relationship in the order of creation. The removal
+     * via the part id ({@link #removeRelation(String)} is preferred.
+     * 
+     * @param partId the part id which relation is to be removed from this document
+     * 
+     * @since 4.0.0
+     */
+    protected final void removeRelation(String partId) {
+        removeRelation(partId, true);
+    }
+
+    /**
+     * Remove the relation to the specified part in this package and remove the
+     * part, if it is no longer needed and flag is set to true.<p>
+     *
+     * @param partId
+     *            The related part id, to which the relation shall be removed.
+     * @param removeUnusedParts
+     *            true, if the part shall be removed from the package if not
+     *            needed any longer.
+     * @return true, if the relation was removed
+     * 
+     * @since 4.0.0
+     */
+    private final boolean removeRelation(String partId, boolean removeUnusedParts) {
+        RelationPart rp = relations.get(partId);
+        if (rp == null) {
             // part is not related with this POIXMLDocumentPart
             return false;
         }
+        POIXMLDocumentPart part = rp.getDocumentPart();
         /* decrement usage counter */
         part.decrementRelationCounter();
         /* remove packagepart relationship */
-        getPackagePart().removeRelationship(id);
+        getPackagePart().removeRelationship(partId);
         /* remove POIXMLDocument from relations */
-        relations.remove(id);
+        relations.remove(partId);
 
         if (removeUnusedParts) {
             /* if last relation to target part was removed, delete according target part */
@@ -338,6 +401,8 @@ public class POIXMLDocumentPart {
         return true;
     }
 
+    
+    
     /**
      * Returns the parent POIXMLDocumentPart. All parts except root have not-null parent.
      *

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

@@ -612,7 +612,6 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
      * @param pictureShape the picture shapes whose relation is to be removed
      */
     void removePictureRelation(XSLFPictureShape pictureShape) {
-        POIXMLDocumentPart pd = getRelationById(pictureShape.getBlipId());
+        removeRelation(pictureShape.getBlipId());
-        removeRelation(pd);
     }
 }

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

@@ -243,7 +243,7 @@ implements Slide<XSLFShape,XSLFTextParagraph> {
         if (bgThis != null) {
             if (bgThis.isSetBgPr() && bgThis.getBgPr().isSetBlipFill()) {
                 String oldId = bgThis.getBgPr().getBlipFill().getBlip().getEmbed();
-                removeRelation(getRelationById(oldId));
+                removeRelation(oldId);
             }
             _slide.getCSld().unsetBg();
         }

src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java

@@ -280,7 +280,7 @@ public final class TestPOIXMLDocument {
         assertNull(part.getRelationById(null));
         assertNull(part.getRelationId(null));
         assertFalse(part.removeRelation(null, true));
-        part.removeRelation(null);
+        part.removeRelation((POIXMLDocumentPart)null);
         assertEquals("",part.toString());
         part.onDocumentCreate();
         //part.getTargetPart(null);

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

@@ -20,6 +20,7 @@ import static org.apache.poi.POITestCase.assertContains;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -30,17 +31,24 @@ import java.awt.RenderingHints;
 import java.awt.geom.AffineTransform;
 import java.awt.geom.Rectangle2D;
 import java.awt.image.BufferedImage;
+import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.URI;
 import java.util.Collection;
+import java.util.Optional;
+import java.util.stream.Collectors;
 
 import javax.imageio.ImageIO;
 
 import org.apache.poi.POIDataSamples;
 import org.apache.poi.POIXMLDocumentPart;
 import org.apache.poi.POIXMLDocumentPart.RelationPart;
+import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.opc.OPCPackage;
+import org.apache.poi.openxml4j.opc.PackagePartName;
+import org.apache.poi.openxml4j.opc.PackagingURIHelper;
 import org.apache.poi.sl.draw.DrawPaint;
 import org.apache.poi.sl.usermodel.PaintStyle;
 import org.apache.poi.sl.usermodel.PaintStyle.SolidPaint;
@@ -49,6 +57,7 @@ import org.apache.poi.sl.usermodel.PictureData;
 import org.apache.poi.sl.usermodel.PictureData.PictureType;
 import org.apache.poi.sl.usermodel.ShapeType;
 import org.apache.poi.sl.usermodel.VerticalAlignment;
+import org.apache.poi.util.IOUtils;
 import org.apache.poi.xslf.extractor.XSLFPowerPointExtractor;
 import org.apache.poi.xslf.usermodel.XMLSlideShow;
 import org.apache.poi.xslf.usermodel.XSLFAutoShape;
@@ -76,6 +85,50 @@ import org.openxmlformats.schemas.presentationml.x2006.main.CTShape;
 public class TestXSLFBugs {
     private static final POIDataSamples slTests = POIDataSamples.getSlideShowInstance();
 
+    @Test
+    public void bug60499() throws IOException, InvalidFormatException {
+        InputStream is = slTests.openResourceAsStream("bug60499.pptx");
+        byte buf[] = IOUtils.toByteArray(is);
+        is.close();
+
+        PackagePartName ppn = PackagingURIHelper.createPartName("/ppt/media/image1.png");
+        
+        XMLSlideShow ppt1 = new XMLSlideShow(new ByteArrayInputStream(buf));
+        XSLFSlide slide1 = ppt1.getSlides().get(0);
+        
+        Optional<XSLFShape> shapeToDelete1 =
+            slide1.getShapes().stream().filter(s -> s instanceof XSLFPictureShape).findFirst();
+        
+        assertTrue(shapeToDelete1.isPresent());
+        slide1.removeShape(shapeToDelete1.get());
+        slide1.getRelationParts().stream()
+            .allMatch(rp -> "rId1,rId3".contains(rp.getRelationship().getId()) ); 
+        
+        assertNotNull(ppt1.getPackage().getPart(ppn));
+        ppt1.close();
+
+        XMLSlideShow ppt2 = new XMLSlideShow(new ByteArrayInputStream(buf));
+        XSLFSlide slide2 = ppt2.getSlides().get(0);
+
+        Optional<XSLFShape> shapeToDelete2 =
+            slide2.getShapes().stream().filter(s -> s instanceof XSLFPictureShape).skip(1).findFirst();
+        assertTrue(shapeToDelete2.isPresent());
+        slide2.removeShape(shapeToDelete2.get());
+        slide2.getRelationParts().stream()
+            .allMatch(rp -> "rId1,rId2".contains(rp.getRelationship().getId()) ); 
+        assertNotNull(ppt2.getPackage().getPart(ppn));
+        ppt2.close();
+
+        XMLSlideShow ppt3 = new XMLSlideShow(new ByteArrayInputStream(buf));
+        XSLFSlide slide3 = ppt3.getSlides().get(0);
+        slide3.getShapes().stream()
+            .filter(s -> s instanceof XSLFPictureShape)
+            .collect(Collectors.toList())
+            .forEach(s -> slide3.removeShape(s));
+        assertNull(ppt3.getPackage().getPart(ppn));
+        ppt3.close();
+    }
+    
     @Test
     public void bug51187() throws Exception {
        XMLSlideShow ss1 = XSLFTestDataSamples.openSampleDocument("51187.pptx");