#62587 - repeated call to XSLFSheet.removeShape leads to java.lang.IllegalArgumentException: partName

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

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

@@ -25,9 +25,7 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
-import org.apache.poi.openxml4j.opc.PackagePart;
+import org.apache.poi.ooxml.POIXMLDocumentPart.RelationPart;
-import org.apache.poi.openxml4j.opc.PackageRelationship;
-import org.apache.poi.openxml4j.opc.TargetMode;
 import org.apache.poi.sl.draw.DrawPictureShape;
 import org.apache.poi.sl.usermodel.GroupShape;
 import org.apache.poi.sl.usermodel.PictureData;
@@ -74,7 +72,7 @@ implements XSLFShapeContainer, GroupShape<XSLFShape,XSLFTextParagraph> {
         return _grpSpPr;
     }
     
-    protected CTGroupTransform2D getSafeXfrm() {
+    private CTGroupTransform2D getSafeXfrm() {
         CTGroupTransform2D xfrm = getXfrm();
         return (xfrm == null ? getGrpSpPr().addNewXfrm() : xfrm);
     }
@@ -267,13 +265,9 @@ implements XSLFShapeContainer, GroupShape<XSLFShape,XSLFTextParagraph> {
         if (!(pictureData instanceof XSLFPictureData)) {
             throw new IllegalArgumentException("pictureData needs to be of type XSLFPictureData");
         }
-        XSLFPictureData xPictureData = (XSLFPictureData)pictureData;
+        RelationPart rp = getSheet().addRelation(null, XSLFRelation.IMAGES, (XSLFPictureData)pictureData);
-        PackagePart pic = xPictureData.getPackagePart();
 
-        PackageRelationship rel = getSheet().getPackagePart().addRelationship(
+        XSLFPictureShape sh = getDrawing().createPicture(rp.getRelationship().getId());
-                pic.getPartName(), TargetMode.INTERNAL, XSLFRelation.IMAGES.getRelation());
-
-        XSLFPictureShape sh = getDrawing().createPicture(rel.getId());
         new DrawPictureShape(sh).resize();
         _shapes.add(sh);
         sh.setParent(this);
@@ -285,13 +279,10 @@ implements XSLFShapeContainer, GroupShape<XSLFShape,XSLFTextParagraph> {
         if (!(pictureData instanceof XSLFPictureData)) {
             throw new IllegalArgumentException("pictureData needs to be of type XSLFPictureData");
         }
-        XSLFPictureData xPictureData = (XSLFPictureData)pictureData;
-        PackagePart pic = xPictureData.getPackagePart();
 
-        PackageRelationship rel = getSheet().getPackagePart().addRelationship(
+        RelationPart rp = getSheet().addRelation(null, XSLFRelation.IMAGES, (XSLFPictureData)pictureData);
-                pic.getPartName(), TargetMode.INTERNAL, XSLFRelation.IMAGES.getRelation());
 
-        XSLFObjectShape sh = getDrawing().createOleShape(rel.getId());
+        XSLFObjectShape sh = getDrawing().createOleShape(rp.getRelationship().getId());
         CTOleObject oleObj = sh.getCTOleObject();
         Dimension dim = pictureData.getImageDimension();
         oleObj.setImgW(Units.toEMU(dim.getWidth()));

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

@@ -19,10 +19,9 @@ package org.apache.poi.xslf.usermodel;
 import java.net.URI;
 
 import org.apache.poi.common.usermodel.HyperlinkType;
+import org.apache.poi.ooxml.POIXMLDocumentPart.RelationPart;
 import org.apache.poi.openxml4j.opc.PackagePart;
-import org.apache.poi.openxml4j.opc.PackagePartName;
 import org.apache.poi.openxml4j.opc.PackageRelationship;
-import org.apache.poi.openxml4j.opc.TargetMode;
 import org.apache.poi.sl.usermodel.Hyperlink;
 import org.apache.poi.sl.usermodel.Slide;
 import org.apache.poi.util.Internal;
@@ -30,8 +29,8 @@ import org.apache.poi.util.Removal;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTHyperlink;
 
 public class XSLFHyperlink implements Hyperlink<XSLFShape,XSLFTextParagraph> {
-    final XSLFSheet _sheet;
+    private final XSLFSheet _sheet;
-    final CTHyperlink _link;
+    private final CTHyperlink _link;
 
     XSLFHyperlink(CTHyperlink link, XSLFSheet sheet){
         _sheet = sheet;
@@ -128,14 +127,12 @@ public class XSLFHyperlink implements Hyperlink<XSLFShape,XSLFTextParagraph> {
 
     @Override
     public void linkToSlide(Slide<XSLFShape,XSLFTextParagraph> slide) {
-        PackagePart thisPP = _sheet.getPackagePart();
-        PackagePartName otherPPN = ((XSLFSheet)slide).getPackagePart().getPartName();
         if (_link.isSetId() && !_link.getId().isEmpty()) {
-            thisPP.removeRelationship(_link.getId());
+            _sheet.getPackagePart().removeRelationship(_link.getId());
         }
-        PackageRelationship rel =
+
-            thisPP.addRelationship(otherPPN, TargetMode.INTERNAL, XSLFRelation.SLIDE.getRelation());
+        RelationPart rp = _sheet.addRelation(null, XSLFRelation.SLIDE, (XSLFSheet) slide);
-        _link.setId(rel.getId());
+        _link.setId(rp.getRelationship().getId());
         _link.setAction("ppaction://hlinksldjump");
     }
 

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

@@ -24,7 +24,6 @@ import java.net.URI;
 
 import javax.xml.namespace.QName;
 
-import org.apache.poi.ooxml.POIXMLException;
 import org.apache.poi.openxml4j.opc.PackagePart;
 import org.apache.poi.openxml4j.opc.PackageRelationship;
 import org.apache.poi.sl.usermodel.PictureShape;
@@ -95,10 +94,7 @@ public class XSLFPictureShape extends XSLFSimpleShape
      *  (image lives outside)?
      */
     public boolean isExternalLinkedPicture() {
-        if (getBlipId() == null && getBlipLink() != null) {
+        return getBlipId() == null && getBlipLink() != null;
-            return true;
-        }
-        return false;
     }
 
     /**
@@ -108,19 +104,10 @@ public class XSLFPictureShape extends XSLFSimpleShape
     public XSLFPictureData getPictureData() {
         if(_data == null){
             String blipId = getBlipId();
-            if (blipId == null) return null;
+            if (blipId == null) {
-
+                return null;
-            PackagePart p = getSheet().getPackagePart();
-            PackageRelationship rel = p.getRelationship(blipId);
-            if (rel != null) {
-                try {
-                    PackagePart imgPart = p.getRelatedPart(rel);
-                    _data = new XSLFPictureData(imgPart);
-                }
-                catch (Exception e) {
-                    throw new POIXMLException(e);
-                }
             }
+            _data = (XSLFPictureData)getSheet().getRelationById(blipId);
         }
         return _data;
     }
@@ -181,12 +168,14 @@ public class XSLFPictureShape extends XSLFSimpleShape
         return getBlipFill().getBlip();
     }
     
+    @SuppressWarnings("WeakerAccess")
     protected String getBlipLink(){
         String link = getBlip().getLink();
         if (link.isEmpty()) return null;
         return link;
     }
 
+    @SuppressWarnings("WeakerAccess")
     protected String getBlipId(){
         String id = getBlip().getEmbed();
         if (id.isEmpty()) return null;
@@ -210,7 +199,7 @@ public class XSLFPictureShape extends XSLFSimpleShape
             return;
         }
 
-        String relId = getSheet().importBlip(blipId, p.getSheet().getPackagePart());
+        String relId = getSheet().importBlip(blipId, p.getSheet());
 
         CTPicture ct = (CTPicture)getXmlObject();
         CTBlip blip = getBlipFill().getBlip();
@@ -224,13 +213,14 @@ public class XSLFPictureShape extends XSLFSimpleShape
         if(blip.isSetExtLst()) {
 
             CTOfficeArtExtensionList extLst = blip.getExtLst();
+            //noinspection deprecation
             for(CTOfficeArtExtension ext : extLst.getExtArray()){
                 String xpath = "declare namespace a14='http://schemas.microsoft.com/office/drawing/2010/main' $this//a14:imgProps/a14:imgLayer";
                 XmlObject[] obj = ext.selectPath(xpath);
                 if(obj != null && obj.length == 1){
                     XmlCursor c = obj[0].newCursor();
                     String id = c.getAttributeText(new QName("http://schemas.openxmlformats.org/officeDocument/2006/relationships", "embed"));//selectPath("declare namespace r='http://schemas.openxmlformats.org/officeDocument/2006/relationships' $this//[@embed]");
-                    String newId = getSheet().importBlip(id, p.getSheet().getPackagePart());
+                    String newId = getSheet().importBlip(id, p.getSheet());
                     c.setAttributeText(new QName("http://schemas.openxmlformats.org/officeDocument/2006/relationships", "embed"), newId);
                     c.dispose();
                 }

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

@@ -18,7 +18,6 @@ package org.apache.poi.xslf.usermodel;
 
 import static org.apache.poi.ooxml.POIXMLTypeLoader.DEFAULT_XML_OPTIONS;
 
-import javax.xml.namespace.QName;
 import java.awt.Dimension;
 import java.awt.Graphics2D;
 import java.io.IOException;
@@ -32,9 +31,10 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
+import javax.xml.namespace.QName;
+
 import org.apache.poi.ooxml.POIXMLDocumentPart;
 import org.apache.poi.ooxml.POIXMLException;
-import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 import org.apache.poi.openxml4j.opc.PackageNamespaces;
 import org.apache.poi.openxml4j.opc.PackagePart;
@@ -82,14 +82,14 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
 
     private final BitSet shapeIds = new BitSet();
 
-    public XSLFSheet() {
+    protected XSLFSheet() {
         super();
     }
 
     /**
      * @since POI 3.14-Beta1
      */
-    public XSLFSheet(PackagePart part) {
+    protected XSLFSheet(PackagePart part) {
         super(part);
     }
 
@@ -108,12 +108,14 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
         throw new IllegalStateException("SlideShow was not found");
     }
 
+    @SuppressWarnings("WeakerAccess")
     protected int allocateShapeId() {
         final int nextId = shapeIds.nextClearBit(1);
         shapeIds.set(nextId);
         return nextId;
     }
 
+    @SuppressWarnings("WeakerAccess")
     protected void registerShapeId(final int shapeId) {
         if (shapeIds.get(shapeId)) {
             LOG.log(POILogger.WARN, "shape id "+shapeId+" has been already used.");
@@ -121,6 +123,7 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
         shapeIds.set(shapeId);
     }
 
+    @SuppressWarnings("WeakerAccess")
     protected void deregisterShapeId(final int shapeId) {
         if (!shapeIds.get(shapeId)) {
             LOG.log(POILogger.WARN, "shape id "+shapeId+" hasn't been registered.");
@@ -128,6 +131,7 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
         shapeIds.clear(shapeId);
     }
 
+    @SuppressWarnings("WeakerAccess")
     protected static List<XSLFShape> buildShapes(CTGroupShape spTree, XSLFShapeContainer parent){
         final XSLFSheet sheet = (parent instanceof XSLFSheet) ? (XSLFSheet)parent : ((XSLFShape)parent).getSheet();
 
@@ -261,10 +265,8 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
         if (!(pictureData instanceof XSLFPictureData)) {
             throw new IllegalArgumentException("pictureData needs to be of type XSLFPictureData");
         }
-        XSLFPictureData xPictureData = (XSLFPictureData)pictureData;
-        PackagePart pic = xPictureData.getPackagePart();
 
-        RelationPart rp = addRelation(null, XSLFRelation.IMAGES, new XSLFPictureData(pic));
+        RelationPart rp = addRelation(null, XSLFRelation.IMAGES, (XSLFPictureData)pictureData);
 
         XSLFPictureShape sh = getDrawing().createPicture(rp.getRelationship().getId());
         new DrawPictureShape(sh).resize();
@@ -303,10 +305,7 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
         if (!(pictureData instanceof XSLFPictureData)) {
             throw new IllegalArgumentException("pictureData needs to be of type XSLFPictureData");
         }
-        XSLFPictureData xPictureData = (XSLFPictureData)pictureData;
+        RelationPart rp = addRelation(null, XSLFRelation.IMAGES, (XSLFPictureData)pictureData);
-        PackagePart pic = xPictureData.getPackagePart();
-
-        RelationPart rp = addRelation(null, XSLFRelation.IMAGES, new XSLFPictureData(pic));
         
         XSLFObjectShape sh = getDrawing().createOleShape(rp.getRelationship().getId());
         CTOleObject oleObj = sh.getCTOleObject();
@@ -386,6 +385,7 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
 
     protected abstract String getRootElementName();
 
+    @SuppressWarnings("WeakerAccess")
     protected CTGroupShape getSpTree(){
         if(_spTree == null) {
             XmlObject root = getXmlObject();
@@ -460,6 +460,7 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
      * @param src the source sheet
      * @return modified <code>this</code>.
      */
+    @SuppressWarnings("unused")
     public XSLFSheet appendContent(XSLFSheet src){
         int numShapes = getShapes().size();
         CTGroupShape spTree = getSpTree();
@@ -523,6 +524,7 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
         return null;
     }
 
+    @SuppressWarnings("WeakerAccess")
     protected XSLFTextShape getTextShapeByType(Placeholder type){
         for(XSLFShape shape : this.getShapes()){
             if(shape instanceof XSLFTextShape) {
@@ -535,6 +537,7 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
         return null;
     }
 
+    @SuppressWarnings("WeakerAccess")
     public XSLFSimpleShape getPlaceholder(Placeholder ph) {
         return getPlaceholderByType(ph.ooxmlId);
     }
@@ -600,19 +603,10 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
      *
      * @return all placeholder shapes in this sheet
      */
+    @SuppressWarnings("WeakerAccess")
     public XSLFTextShape[] getPlaceholders() {
         initPlaceholders();
-        return _placeholders.toArray(new XSLFTextShape[_placeholders.size()]);
+        return _placeholders.toArray(new XSLFTextShape[0]);
-    }
-
-    /**
-     * Checks if this <code>sheet</code> displays the specified shape.
-     *
-     * Subclasses can override it and skip certain shapes from drawings,
-     * for instance, slide masters and layouts don't display placeholders
-     */
-    protected boolean canDraw(XSLFShape shape){
-        return true;
     }
 
     /**
@@ -650,39 +644,35 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
      * Import a picture data from another document.
      *
      * @param blipId        ID of the package relationship to retrieve.
-     * @param packagePart   package part containing the data to import
+     * @param parent        parent document containing the data to import
      * @return ID of the created relationship
      */
-    String importBlip(String blipId, PackagePart packagePart) {
+    String importBlip(String blipId, POIXMLDocumentPart parent) {
-        PackageRelationship blipRel = packagePart.getRelationship(blipId);
+        final XSLFPictureData parData = parent.getRelationPartById(blipId).getDocumentPart();
-        PackagePart blipPart;
+        final XSLFPictureData pictureData;
-        try {
+        if (getPackagePart().getPackage() == parent.getPackagePart().getPackage()) {
-            blipPart = packagePart.getRelatedPart(blipRel);
+            // handle ref counter correct, if the parent document is the same as this
-        } catch (Exception e){
+            pictureData = parData;
-            throw new POIXMLException(e);
+        } else {
+            XMLSlideShow ppt = getSlideShow();
+            pictureData = ppt.addPicture(parData.getData(), parData.getType());
         }
-        XSLFPictureData data = new XSLFPictureData(blipPart);
-
-        XMLSlideShow ppt = getSlideShow();
-        XSLFPictureData pictureData = ppt.addPicture(data.getData(), data.getType());
-        PackagePart pic = pictureData.getPackagePart();
-
-        RelationPart rp = addRelation(blipId, XSLFRelation.IMAGES, new XSLFPictureData(pic));
 
+        RelationPart rp = addRelation(blipId, XSLFRelation.IMAGES, pictureData);
         return rp.getRelationship().getId();
     }
 
     /**
      * Import a package part into this sheet.
      */
-    PackagePart importPart(PackageRelationship srcRel, PackagePart srcPafrt) {
+    void importPart(PackageRelationship srcRel, PackagePart srcPafrt) {
         PackagePart destPP = getPackagePart();
         PackagePartName srcPPName = srcPafrt.getPartName();
 
         OPCPackage pkg = destPP.getPackage();
         if(pkg.containPart(srcPPName)){
             // already exists
-            return pkg.getPart(srcPPName);
+            return;
         }
 
         destPP.addRelationship(srcPPName, TargetMode.INTERNAL, srcRel.getRelationshipType());
@@ -697,7 +687,6 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
         } catch (IOException e){
             throw new POIXMLException(e);
         }
-        return part;
     }
 
     /**

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

@@ -216,13 +216,13 @@ public abstract class XSLFSimpleShape extends XSLFShape
     @Override
     public boolean getFlipHorizontal() {
         CTTransform2D xfrm = getXfrm(false);
-        return (xfrm == null || !xfrm.isSetFlipH()) ? false : xfrm.getFlipH();
+        return (xfrm != null && xfrm.isSetFlipH()) && xfrm.getFlipH();
     }
 
     @Override
     public boolean getFlipVertical() {
         CTTransform2D xfrm = getXfrm(false);
-        return (xfrm == null || !xfrm.isSetFlipV()) ? false : xfrm.getFlipV();
+        return (xfrm != null && xfrm.isSetFlipV()) && xfrm.getFlipV();
     }
 
 
@@ -232,7 +232,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
      *
      * @return line properties from the theme of null
      */
-    CTLineProperties getDefaultLineProperties() {
+    private CTLineProperties getDefaultLineProperties() {
         CTShapeStyle style = getSpStyle();
         if (style == null) {
             return null;
@@ -302,6 +302,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
      * @return the color of the shape outline or <code>null</code>
      * if outline is turned off
      */
+    @SuppressWarnings("WeakerAccess")
     public Color getLineColor() {
         PaintStyle ps = getLinePaint();
         if (ps instanceof SolidPaint) {
@@ -310,6 +311,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
         return null;
     }
 
+    @SuppressWarnings("WeakerAccess")
     protected PaintStyle getLinePaint() {
         XSLFSheet sheet = getSheet();
         final XSLFTheme theme = sheet.getTheme();
@@ -377,6 +379,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
      *
      * @param width line width in points. <code>0</code> means no line
      */
+    @SuppressWarnings("WeakerAccess")
     public void setLineWidth(double width) {
         CTLineProperties lnPr = getLn(this, true);
         if (lnPr == null) {
@@ -411,6 +414,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * @return line width in points. <code>0</code> means no line.
      */
+    @SuppressWarnings("WeakerAccess")
     public double getLineWidth() {
         PropertyFetcher<Double> fetcher = new PropertyFetcher<Double>() {
             @Override
@@ -451,6 +455,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * @param compound set the line compound style
      */
+    @SuppressWarnings("WeakerAccess")
     public void setLineCompound(LineCompound compound) {
         CTLineProperties ln = getLn(this, true);
         if (ln == null) {
@@ -487,6 +492,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * @return the line compound
      */
+    @SuppressWarnings("WeakerAccess")
     public LineCompound getLineCompound() {
         PropertyFetcher<Integer> fetcher = new PropertyFetcher<Integer>() {
             @Override
@@ -531,6 +537,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
      *
      * @param dash a preset line dashing scheme to stroke thr shape outline
      */
+    @SuppressWarnings("WeakerAccess")
     public void setLineDash(LineDash dash) {
         CTLineProperties ln = getLn(this, true);
         if (ln == null) {
@@ -549,6 +556,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * @return  a preset line dashing scheme to stroke the shape outline
      */
+    @SuppressWarnings("WeakerAccess")
     public LineDash getLineDash() {
 
         PropertyFetcher<LineDash> fetcher = new PropertyFetcher<LineDash>() {
@@ -579,6 +587,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
      *
      * @param cap the line end cap style
      */
+    @SuppressWarnings("WeakerAccess")
     public void setLineCap(LineCap cap) {
         CTLineProperties ln = getLn(this, true);
         if (ln == null) {
@@ -598,6 +607,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
      *
      * @return the line end cap style
      */
+    @SuppressWarnings("WeakerAccess")
     public LineCap getLineCap() {
         PropertyFetcher<LineCap> fetcher = new PropertyFetcher<LineCap>() {
             @Override
@@ -758,7 +768,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
             CTBlip blip = fp.getBlipFill().getBlip();
             String blipId = blip.getEmbed();
 
-            String relId = getSheet().importBlip(blipId, s.getSheet().getPackagePart());
+            String relId = getSheet().importBlip(blipId, s.getSheet());
             blip.setEmbed(relId);
         }
 
@@ -793,6 +803,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
      * 
      * @param style the line end docoration style
      */
+    @SuppressWarnings("WeakerAccess")
     public void setLineHeadDecoration(DecorationShape style) {
         CTLineProperties ln = getLn(this, true);
         if (ln == null) {
@@ -811,6 +822,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * @return the line end decoration shape
      */
+    @SuppressWarnings("WeakerAccess")
     public DecorationShape getLineHeadDecoration() {
         CTLineProperties ln = getLn(this, false);
         DecorationShape ds = DecorationShape.NONE;
@@ -825,6 +837,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
      * 
      * @param style the decoration width 
      */
+    @SuppressWarnings("WeakerAccess")
     public void setLineHeadWidth(DecorationSize style) {
         CTLineProperties ln = getLn(this, true);
         if (ln == null) {
@@ -843,6 +856,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * @return the line end decoration width
      */
+    @SuppressWarnings("WeakerAccess")
     public DecorationSize getLineHeadWidth() {
         CTLineProperties ln = getLn(this, false);
         DecorationSize ds = DecorationSize.MEDIUM;
@@ -855,6 +869,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * Specifies the line end width in relation to the line width.
      */
+    @SuppressWarnings("WeakerAccess")
     public void setLineHeadLength(DecorationSize style) {
         CTLineProperties ln = getLn(this, true);
         if (ln == null) {
@@ -874,6 +889,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * @return the line end decoration length
      */
+    @SuppressWarnings("WeakerAccess")
     public DecorationSize getLineHeadLength() {
         CTLineProperties ln = getLn(this, false);
         
@@ -887,6 +903,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * Specifies the line end decoration, such as a triangle or arrowhead.
      */
+    @SuppressWarnings("WeakerAccess")
     public void setLineTailDecoration(DecorationShape style) {
         CTLineProperties ln = getLn(this, true);
         if (ln == null) {
@@ -906,6 +923,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * @return the line end decoration shape
      */
+    @SuppressWarnings("WeakerAccess")
     public DecorationShape getLineTailDecoration() {
         CTLineProperties ln = getLn(this, false);
         
@@ -919,6 +937,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * specifies decorations which can be added to the tail of a line.
      */
+    @SuppressWarnings("WeakerAccess")
     public void setLineTailWidth(DecorationSize style) {
         CTLineProperties ln = getLn(this, true);
         if (ln == null) {
@@ -938,6 +957,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * @return the line end decoration width
      */
+    @SuppressWarnings("WeakerAccess")
     public DecorationSize getLineTailWidth() {
         CTLineProperties ln = getLn(this, false);
         DecorationSize ds = DecorationSize.MEDIUM;
@@ -950,6 +970,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * Specifies the line end width in relation to the line width.
      */
+    @SuppressWarnings("WeakerAccess")
     public void setLineTailLength(DecorationSize style) {
         CTLineProperties ln = getLn(this, true);
         if (ln == null) {
@@ -969,6 +990,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
     /**
      * @return the line end decoration length
      */
+    @SuppressWarnings("WeakerAccess")
     public DecorationSize getLineTailLength() {
         CTLineProperties ln = getLn(this, false);
         
@@ -984,6 +1006,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
         XSLFGeometryProperties gp = XSLFPropertiesDelegate.getGeometryDelegate(getShapeProperties());
         
         if (gp != null && gp.isSetPrstGeom() && gp.getPrstGeom().isSetAvLst()) {
+            //noinspection deprecation
             for (CTGeomGuide g : gp.getPrstGeom().getAvLst().getGdArray()) {
                 if (g.getName().equals(name)) {
                     return new Guide(g.getName(), g.getFmla());
@@ -1036,12 +1059,7 @@ public abstract class XSLFSimpleShape extends XSLFShape
      */
     @Override
     public FillStyle getFillStyle() {
-        return new FillStyle() {
+        return XSLFSimpleShape.this::getFillPaint;
-            @Override
-            public PaintStyle getPaint() {
-                return XSLFSimpleShape.this.getFillPaint();
-            }
-        };
     }
 
     @Override

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

@@ -128,10 +128,12 @@ implements Slide<XSLFShape,XSLFTextParagraph> {
         return "sld";
     }
 
+    @SuppressWarnings({"WeakerAccess", "ProtectedMemberInFinalClass"})
     protected void removeChartRelation(XSLFChart chart) {
         removeRelation(chart);
     }
 
+    @SuppressWarnings({"WeakerAccess", "ProtectedMemberInFinalClass"})
     protected void removeLayoutRelation(XSLFSlideLayout layout) {
         removeRelation(layout, false);
     }
@@ -164,6 +166,7 @@ implements Slide<XSLFShape,XSLFTextParagraph> {
      * @return the comments part or {@code null} if there weren't any comments
      * @since POI 4.0.0
      */
+    @SuppressWarnings("WeakerAccess")
     public XSLFComments getCommentsPart() {
         if(_comments == null) {
             for (POIXMLDocumentPart p : getRelations()) {
@@ -181,6 +184,7 @@ implements Slide<XSLFShape,XSLFTextParagraph> {
      * @return the comment authors part or {@code null} if there weren't any comments
      * @since POI 4.0.0
      */
+    @SuppressWarnings("WeakerAccess")
     public XSLFCommentAuthors getCommentAuthorsPart() {
         if(_commentAuthors == null) {
             // first scan the slide relations
@@ -209,6 +213,7 @@ implements Slide<XSLFShape,XSLFTextParagraph> {
         final XSLFComments xComments = getCommentsPart();
         final XSLFCommentAuthors xAuthors = getCommentAuthorsPart();
         if (xComments != null) {
+            //noinspection deprecation
             for (final CTComment xc : xComments.getCTCommentsList().getCmArray()) {
                 comments.add(new XSLFComment(xc, xAuthors));
             }
@@ -268,6 +273,7 @@ implements Slide<XSLFShape,XSLFTextParagraph> {
      *
      * @param value whether shapes on the master slide should be shown or not.
      */
+    @SuppressWarnings("WeakerAccess")
     public void setFollowMasterGraphics(boolean value){
         _slide.setShowMasterSp(value);
     }
@@ -310,7 +316,7 @@ implements Slide<XSLFShape,XSLFTextParagraph> {
 
         if(bgOther.isSetBgPr() && bgOther.getBgPr().isSetBlipFill()){
             String idOther = bgOther.getBgPr().getBlipFill().getBlip().getEmbed();
-            String idThis = importBlip(idOther, src.getPackagePart());
+            String idThis = importBlip(idOther, src);
             bgThis.getBgPr().getBlipFill().getBlip().setEmbed(idThis);
 
         }
@@ -358,7 +364,7 @@ implements Slide<XSLFShape,XSLFTextParagraph> {
     /**
      * Render this sheet into the supplied graphics object
      *
-     * @param graphics
+     * @param graphics the graphics context to draw to
      */
     @Override
     public void draw(Graphics2D graphics){

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

@@ -70,6 +70,7 @@ implements MasterSheet<XSLFShape,XSLFTextParagraph> {
      * @return slide master. Never null.
      * @throws IllegalStateException if slide master was not found
      */
+    @SuppressWarnings("WeakerAccess")
     public XSLFSlideMaster getSlideMaster() {
         if (_master == null) {
             for (POIXMLDocumentPart p : getRelations()) {
@@ -100,15 +101,6 @@ implements MasterSheet<XSLFShape,XSLFTextParagraph> {
         return _layout.getShowMasterSp();
     }
 
-    /**
-     * Render this sheet into the supplied graphics object
-     */
-    @Override
-    protected boolean canDraw(XSLFShape shape) {
-        return !(shape instanceof XSLFSimpleShape) || !shape.isPlaceholder();
-    }
-
-
     @Override
     public XSLFBackground getBackground() {
         CTBackground bg = _layout.getCSld().getBg();
@@ -124,6 +116,7 @@ implements MasterSheet<XSLFShape,XSLFTextParagraph> {
      *
      * @param slide destination slide
      */
+    @SuppressWarnings("WeakerAccess")
     public void copyLayout(XSLFSlide slide) {
         for (XSLFShape sh : getShapes()) {
             if (sh instanceof XSLFTextShape) {

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

@@ -153,15 +153,6 @@ import org.openxmlformats.schemas.presentationml.x2006.main.SldMasterDocument;
         return props;
     }
 
-    /**
-     * Render this sheet into the supplied graphics object
-     *
-     */
-    @Override
-    protected boolean canDraw(XSLFShape shape) {
-        return !(shape instanceof XSLFSimpleShape) || !shape.isPlaceholder();
-    }
-
     @Override
     public XSLFBackground getBackground() {
         CTBackground bg = _slide.getCSld().getBg();

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

@@ -32,6 +32,7 @@ import java.awt.geom.AffineTransform;
 import java.awt.geom.Rectangle2D;
 import java.awt.image.BufferedImage;
 import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
@@ -59,8 +60,10 @@ import org.apache.poi.sl.usermodel.PaintStyle.SolidPaint;
 import org.apache.poi.sl.usermodel.PaintStyle.TexturePaint;
 import org.apache.poi.sl.usermodel.PictureData;
 import org.apache.poi.sl.usermodel.PictureData.PictureType;
+import org.apache.poi.sl.usermodel.PictureShape;
 import org.apache.poi.sl.usermodel.Shape;
 import org.apache.poi.sl.usermodel.ShapeType;
+import org.apache.poi.sl.usermodel.Slide;
 import org.apache.poi.sl.usermodel.VerticalAlignment;
 import org.apache.poi.util.IOUtils;
 import org.apache.poi.xslf.usermodel.XMLSlideShow;
@@ -89,6 +92,57 @@ import org.openxmlformats.schemas.presentationml.x2006.main.CTShape;
 public class TestXSLFBugs {
     private static final POIDataSamples slTests = POIDataSamples.getSlideShowInstance();
 
+    @Test
+    public void bug62587() throws IOException {
+        ByteArrayOutputStream bos = new ByteArrayOutputStream();
+        try (XMLSlideShow ppt = new XMLSlideShow()) {
+            Slide slide = ppt.createSlide();
+            XSLFPictureData pd = ppt.addPicture(slTests.getFile("wrench.emf"), PictureType.EMF);
+            PictureShape ps = slide.createPicture(pd);
+            ps.setAnchor(new Rectangle2D.Double(100,100,100,100));
+            ppt.write(bos);
+        }
+
+        Object[][] pics = {
+            {"santa.wmf", PictureType.WMF, XSLFRelation.IMAGE_WMF},
+            {"tomcat.png",PictureType.PNG, XSLFRelation.IMAGE_PNG},
+            {"clock.jpg", PictureType.JPEG, XSLFRelation.IMAGE_JPEG}
+        };
+
+        try (XMLSlideShow ppt = new XMLSlideShow(new ByteArrayInputStream(bos.toByteArray()))) {
+            XSLFSlide s1 = ppt.getSlides().get(0);
+
+            for (Object[] p : pics) {
+                XSLFSlide s2 = ppt.createSlide();
+                s2.importContent(s1);
+
+                XSLFPictureData pd = ppt.addPicture(slTests.getFile((String)p[0]), (PictureType)p[1]);
+                XSLFPictureShape ps = (XSLFPictureShape) s2.getShapes().get(0);
+                Rectangle2D anchor = ps.getAnchor();
+                s2.removeShape(ps);
+                ps = s2.createPicture(pd);
+                ps.setAnchor(anchor);
+            }
+
+            bos.reset();
+            ppt.write(bos);
+        }
+
+        try (XMLSlideShow ppt = new XMLSlideShow(new ByteArrayInputStream(bos.toByteArray()))) {
+            for (XSLFSlide sl : ppt.getSlides()) {
+                List<RelationPart> rels = sl.getRelationParts();
+                assertEquals(2, rels.size());
+                RelationPart rel0 = rels.get(0);
+                assertEquals("rId1", rel0.getRelationship().getId());
+                assertEquals(XSLFRelation.SLIDE_LAYOUT.getRelation(), rel0.getRelationship().getRelationshipType());
+                RelationPart rel1 = rels.get(1);
+                assertEquals("rId2", rel1.getRelationship().getId());
+                assertEquals(XSLFRelation.IMAGES.getRelation(), rel1.getRelationship().getRelationshipType());
+            }
+        }
+    }
+
+
     @Test
     public void bug60499() throws IOException, InvalidFormatException {
         InputStream is = slTests.openResourceAsStream("bug60499.pptx");
@@ -314,8 +368,8 @@ public class TestXSLFBugs {
         ss.close();
     }
 
-    protected String getSlideText(XMLSlideShow ppt, XSLFSlide slide) throws IOException {
+    private String getSlideText(XMLSlideShow ppt, XSLFSlide slide) throws IOException {
-        try (SlideShowExtractor extr = new SlideShowExtractor(ppt)) {
+        try (SlideShowExtractor<XSLFShape,XSLFTextParagraph> extr = new SlideShowExtractor<>(ppt)) {
             // do not auto-close the slideshow
             extr.setFilesystem(null);
             extr.setSlidesByDefault(true);
@@ -369,7 +423,12 @@ public class TestXSLFBugs {
         assertEquals(1, slide.getShapes().size());
 
         assertEquals(1, slide.getRelations().size());
-        assertRelationEquals(XSLFRelation.SLIDE_LAYOUT, slide.getRelations().get(0));
+
+        final XSLFRelation expected = XSLFRelation.SLIDE_LAYOUT;
+        final POIXMLDocumentPart relation = slide.getRelations().get(0);
+
+        assertEquals(expected.getContentType(), relation.getPackagePart().getContentType());
+        assertEquals(expected.getFileName(expected.getFileNameIndex(relation)), relation.getPackagePart().getPartName().getName());
 
         // Some dummy pictures
         byte[][] pics = new byte[15][3];
@@ -472,11 +531,6 @@ public class TestXSLFBugs {
         }
     }
 
-    private void assertRelationEquals(XSLFRelation expected, POIXMLDocumentPart relation) {
-        assertEquals(expected.getContentType(), relation.getPackagePart().getContentType());
-        assertEquals(expected.getFileName(expected.getFileNameIndex(relation)), relation.getPackagePart().getPartName().getName());
-    }
-
     @Test
     public void bug58205() throws IOException {
         XMLSlideShow ss = XSLFTestDataSamples.openSampleDocument("themes.pptx");
@@ -726,25 +780,19 @@ public class TestXSLFBugs {
 
     @Test
     public void testAptia() throws IOException {
-        XMLSlideShow ppt = XSLFTestDataSamples.openSampleDocument("aptia.pptx");
+        try (XMLSlideShow ppt = XSLFTestDataSamples.openSampleDocument("aptia.pptx");
-        try {
+             XMLSlideShow saved = XSLFTestDataSamples.writeOutAndReadBack(ppt)) {
-            XMLSlideShow saved = XSLFTestDataSamples.writeOutAndReadBack(ppt);
+            assertEquals(ppt.getSlides().size(), saved.getSlides().size());
-        } catch (IOException e) {
-            fail("Could not read back saved presentation.");
         }
-        ppt.close();
     }
 
     @Ignore
     @Test
     public void testDivinoRevelado() throws IOException {
-        XMLSlideShow ppt = XSLFTestDataSamples.openSampleDocument("Divino_Revelado.pptx");
+        try (XMLSlideShow ppt = XSLFTestDataSamples.openSampleDocument("Divino_Revelado.pptx");
-        try {
+             XMLSlideShow saved = XSLFTestDataSamples.writeOutAndReadBack(ppt)){
-            XMLSlideShow saved = XSLFTestDataSamples.writeOutAndReadBack(ppt);
+            assertEquals(ppt.getSlides().size(), saved.getSlides().size());
-        } catch (IOException e) {
-            fail("Could not read back saved presentation.");
         }
-        ppt.close();
     }
 
     @Test