]> source.dussan.org Git - poi.git/commitdiff
#62051 - Two shapes have the same shapeId within the same slide.
authorAndreas Beeker <kiwiwings@apache.org>
Sun, 20 May 2018 22:00:14 +0000 (22:00 +0000)
committerAndreas Beeker <kiwiwings@apache.org>
Sun, 20 May 2018 22:00:14 +0000 (22:00 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1831947 13f79535-47bb-0310-9956-ffa450edef68

15 files changed:
src/java/org/apache/poi/sl/usermodel/Shape.java
src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFAutoShape.java
src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFConnectorShape.java
src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFDrawing.java
src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFFreeformShape.java
src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFGroupShape.java
src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFObjectShape.java
src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFPictureShape.java
src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFShape.java
src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java
src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTable.java
src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFTextBox.java
src/ooxml/testcases/org/apache/poi/xslf/TestXSLFBugs.java
src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFTable.java
src/scratchpad/src/org/apache/poi/hslf/usermodel/HSLFShape.java

index a8ed1cfaaa2e6162f397b3e2752d9ccbbd1aca22..4929f48bb8dc04b788b227005593e08c902437d7 100644 (file)
@@ -54,4 +54,18 @@ public interface Shape<
     *   if null, the bounds of the shape are used.
     */
    void draw(Graphics2D graphics, Rectangle2D bounds);
+
+
+   /**
+    * Returns a unique identifier for this shape within the current slide.
+    * This ID may be used to assist in uniquely identifying this object so that it can
+    * be referred to by other parts of the document.
+    * <p>
+    * If multiple objects within the same slide share the same id attribute value,
+    * then the document shall be considered non-conformant.
+    * </p>
+    *
+    * @return unique id of this shape
+    */
+   int getShapeId();
 }
index 9dfa62fa04018318f749172be475290f2815cd91..708dc9b745e272e2dd59028edba553f565ccbed9 100644 (file)
@@ -67,7 +67,7 @@ public class XSLFAutoShape extends XSLFTextShape
         CTShapeNonVisual nvSpPr = ct.addNewNvSpPr();
         CTNonVisualDrawingProps cnv = nvSpPr.addNewCNvPr();
         cnv.setName("AutoShape " + shapeId);
-        cnv.setId(shapeId + 1);
+        cnv.setId(shapeId);
         nvSpPr.addNewCNvSpPr();
         nvSpPr.addNewNvPr();
         CTShapeProperties spPr = ct.addNewSpPr();
index c7bf948a91af52734cb21dc5aaf739a6d89fc74c..346131c79d97a4df2d78981376e88cde11a9f266 100644 (file)
@@ -51,7 +51,7 @@ public class XSLFConnectorShape extends XSLFSimpleShape
         CTConnectorNonVisual nvSpPr = ct.addNewNvCxnSpPr();
         CTNonVisualDrawingProps cnv = nvSpPr.addNewCNvPr();
         cnv.setName("Connector " + shapeId);
-        cnv.setId(shapeId + 1);
+        cnv.setId(shapeId);
         nvSpPr.addNewCNvCxnSpPr();
         nvSpPr.addNewNvPr();
         CTShapeProperties spPr = ct.addNewSpPr();
index 42e375ae09a0bbaaa7eb3423175cc8140bd35464..f4cd0d8e1c03a5ef9e0e91236509571dfd431c3e 100644 (file)
@@ -18,26 +18,13 @@ package org.apache.poi.xslf.usermodel;
 
 import java.awt.Color;
 import java.awt.geom.Rectangle2D;
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.io.OutputStream;
 
-import org.apache.poi.POIXMLDocumentPart.RelationPart;
-import org.apache.poi.hpsf.ClassID;
-import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
-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.PackageRelationship;
-import org.apache.poi.openxml4j.opc.PackagingURIHelper;
-import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.util.Beta;
 import org.apache.xmlbeans.XmlObject;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTNonVisualDrawingProps;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTConnector;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTGraphicalObjectFrame;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTGroupShape;
-import org.openxmlformats.schemas.presentationml.x2006.main.CTOleObject;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTPicture;
 import org.openxmlformats.schemas.presentationml.x2006.main.CTShape;
 
@@ -45,7 +32,6 @@ import org.openxmlformats.schemas.presentationml.x2006.main.CTShape;
 @Beta
 public class XSLFDrawing {
     private XSLFSheet _sheet;
-    private int _shapeId = 1;
     private CTGroupShape _spTree;
 
     /*package*/ XSLFDrawing(XSLFSheet sheet, CTGroupShape spTree){
@@ -58,14 +44,14 @@ public class XSLFDrawing {
             // ignore them for now
             if (o instanceof CTNonVisualDrawingProps) {
                 CTNonVisualDrawingProps p = (CTNonVisualDrawingProps)o;
-                _shapeId = (int)Math.max(_shapeId, p.getId());
+                sheet.registerShapeId((int)p.getId());
             }
         }
     }
 
     public XSLFAutoShape createAutoShape(){
         CTShape sp = _spTree.addNewSp();
-        sp.set(XSLFAutoShape.prototype(_shapeId++));
+        sp.set(XSLFAutoShape.prototype(_sheet.allocateShapeId()));
         XSLFAutoShape shape = new XSLFAutoShape(sp, _sheet);
         shape.setAnchor(new Rectangle2D.Double());
         return shape;
@@ -73,7 +59,7 @@ public class XSLFDrawing {
 
     public XSLFFreeformShape createFreeform(){
         CTShape sp = _spTree.addNewSp();
-        sp.set(XSLFFreeformShape.prototype(_shapeId++));
+        sp.set(XSLFFreeformShape.prototype(_sheet.allocateShapeId()));
         XSLFFreeformShape shape = new XSLFFreeformShape(sp, _sheet);
         shape.setAnchor(new Rectangle2D.Double());
         return shape;
@@ -81,7 +67,7 @@ public class XSLFDrawing {
 
     public XSLFTextBox createTextBox(){
         CTShape sp = _spTree.addNewSp();
-        sp.set(XSLFTextBox.prototype(_shapeId++));
+        sp.set(XSLFTextBox.prototype(_sheet.allocateShapeId()));
         XSLFTextBox shape = new XSLFTextBox(sp, _sheet);
         shape.setAnchor(new Rectangle2D.Double());
         return shape;
@@ -89,7 +75,7 @@ public class XSLFDrawing {
 
     public XSLFConnectorShape createConnector(){
         CTConnector sp = _spTree.addNewCxnSp();
-        sp.set(XSLFConnectorShape.prototype(_shapeId++));
+        sp.set(XSLFConnectorShape.prototype(_sheet.allocateShapeId()));
         XSLFConnectorShape shape = new XSLFConnectorShape(sp, _sheet);
         shape.setAnchor(new Rectangle2D.Double());
         shape.setLineColor(Color.black);
@@ -98,33 +84,33 @@ public class XSLFDrawing {
     }
 
     public XSLFGroupShape createGroup(){
-        CTGroupShape obj = _spTree.addNewGrpSp();
-        obj.set(XSLFGroupShape.prototype(_shapeId++));
-        XSLFGroupShape shape = new XSLFGroupShape(obj, _sheet);
+        CTGroupShape sp = _spTree.addNewGrpSp();
+        sp.set(XSLFGroupShape.prototype(_sheet.allocateShapeId()));
+        XSLFGroupShape shape = new XSLFGroupShape(sp, _sheet);
         shape.setAnchor(new Rectangle2D.Double());
         return shape;
     }
 
     public XSLFPictureShape createPicture(String rel){
-        CTPicture obj = _spTree.addNewPic();
-        obj.set(XSLFPictureShape.prototype(_shapeId++, rel));
-        XSLFPictureShape shape = new XSLFPictureShape(obj, _sheet);
+        CTPicture sp = _spTree.addNewPic();
+        sp.set(XSLFPictureShape.prototype(_sheet.allocateShapeId(), rel));
+        XSLFPictureShape shape = new XSLFPictureShape(sp, _sheet);
         shape.setAnchor(new Rectangle2D.Double());
         return shape;
     }
 
     public XSLFTable createTable(){
-        CTGraphicalObjectFrame obj = _spTree.addNewGraphicFrame();
-        obj.set(XSLFTable.prototype(_shapeId++));
-        XSLFTable shape = new XSLFTable(obj, _sheet);
+        CTGraphicalObjectFrame sp = _spTree.addNewGraphicFrame();
+        sp.set(XSLFTable.prototype(_sheet.allocateShapeId()));
+        XSLFTable shape = new XSLFTable(sp, _sheet);
         shape.setAnchor(new Rectangle2D.Double());
         return shape;
     }
 
     public XSLFObjectShape createOleShape(String pictureRel) {
-        CTGraphicalObjectFrame obj = _spTree.addNewGraphicFrame();
-        obj.set(XSLFObjectShape.prototype(_shapeId++, pictureRel));
-        XSLFObjectShape shape = new XSLFObjectShape(obj, _sheet);
+        CTGraphicalObjectFrame sp = _spTree.addNewGraphicFrame();
+        sp.set(XSLFObjectShape.prototype(_sheet.allocateShapeId(), pictureRel));
+        XSLFObjectShape shape = new XSLFObjectShape(sp, _sheet);
         shape.setAnchor(new Rectangle2D.Double());
         return shape;
     }
index 92ec316a697962aa498016742258154bf68e7607..54acd2b7bacc757497f6278c0a5b8625d57929b0 100644 (file)
@@ -232,7 +232,7 @@ public class XSLFFreeformShape extends XSLFAutoShape
         CTShapeNonVisual nvSpPr = ct.addNewNvSpPr();
         CTNonVisualDrawingProps cnv = nvSpPr.addNewCNvPr();
         cnv.setName("Freeform " + shapeId);
-        cnv.setId(shapeId + 1);
+        cnv.setId(shapeId);
         nvSpPr.addNewCNvSpPr();
         nvSpPr.addNewNvPr();
         CTShapeProperties spPr = ct.addNewSpPr();
index adcdad21471d019f9284bfcd4669adeab9d87267..bcab1dabae1f1e06f91983467ddc00330594d1e2 100644 (file)
@@ -174,9 +174,12 @@ implements XSLFShapeContainer, GroupShape<XSLFShape,XSLFTextParagraph> {
     public boolean removeShape(XSLFShape xShape) {
         XmlObject obj = xShape.getXmlObject();
         CTGroupShape grpSp = (CTGroupShape)getXmlObject();
+        getSheet().deregisterShapeId(xShape.getShapeId());
         if(obj instanceof CTShape){
             grpSp.getSpList().remove(obj);
         } else if (obj instanceof CTGroupShape){
+            XSLFGroupShape gs = (XSLFGroupShape)xShape;
+            new ArrayList<>(gs.getShapes()).forEach(gs::removeShape);
             grpSp.getGrpSpList().remove(obj);
         } else if (obj instanceof CTConnector){
             grpSp.getCxnSpList().remove(obj);
@@ -203,7 +206,7 @@ implements XSLFShapeContainer, GroupShape<XSLFShape,XSLFTextParagraph> {
         CTGroupShapeNonVisual nvSpPr = ct.addNewNvGrpSpPr();
         CTNonVisualDrawingProps cnv = nvSpPr.addNewCNvPr();
         cnv.setName("Group " + shapeId);
-        cnv.setId(shapeId + 1);
+        cnv.setId(shapeId);
 
         nvSpPr.addNewCNvGrpSpPr();
         nvSpPr.addNewNvPr();
index ffa6b9eb192efb8e9c7238ab03c065b6b9f8022b..48ae87b122ccc49a10ea8edde844f3bdc391192c 100644 (file)
@@ -268,8 +268,8 @@ public class XSLFObjectShape extends XSLFGraphicFrame implements ObjectShape<XSL
         CTNonVisualDrawingProps cnv = nvGr.addNewCNvPr();
         // usually the shape name has its index based on the n-th embeding, but having
         // the prototype separate from the actual updating of the object, we use the shape id
-        cnv.setName("Object " + (shapeId + 1));
-        cnv.setId(shapeId + 1);
+        cnv.setName("Object " + shapeId);
+        cnv.setId(shapeId);
 
         // add empty property elements otherwise Powerpoint doesn't load the file ...
         nvGr.addNewCNvGraphicFramePr();
index da3cf45c8bd282034377e9e93849383b5c0a79ae..4c7486ddbcb7143a78e9e0c67c85554e952a86a0 100644 (file)
@@ -68,7 +68,7 @@ public class XSLFPictureShape extends XSLFSimpleShape
         CTPictureNonVisual nvSpPr = ct.addNewNvPicPr();
         CTNonVisualDrawingProps cnv = nvSpPr.addNewCNvPr();
         cnv.setName("Picture " + shapeId);
-        cnv.setId(shapeId + 1);
+        cnv.setId(shapeId);
         nvSpPr.addNewCNvPicPr().addNewPicLocks().setNoChangeAspect(true);
         nvSpPr.addNewNvPr();
 
index 82449d846a299c5de16ea74be96986c489ae87f7..66d9a67bfd76c509053937726e5ccfa31720032c 100644 (file)
@@ -102,17 +102,7 @@ public abstract class XSLFShape implements Shape<XSLFShape,XSLFTextParagraph> {
         return getCNvPr().getName();
     }
 
-    /**
-     * Returns a unique identifier for this shape within the current document.
-     * This ID may be used to assist in uniquely identifying this object so that it can
-     * be referred to by other parts of the document.
-     * <p>
-     * If multiple objects within the same document share the same id attribute value,
-     * then the document shall be considered non-conformant.
-     * </p>
-     *
-     * @return unique id of this shape
-     */
+    @Override
     public int getShapeId() {
         return (int)getCNvPr().getId();
     }
index 2d45cb2d134f8024486bffd341e377e2d6772bce..a6320d6c2e05e301aefd88a3537f47f4e90e8c94 100644 (file)
@@ -25,6 +25,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.ArrayList;
+import java.util.BitSet;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -79,6 +80,8 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
     private Map<Integer, XSLFSimpleShape> _placeholderByIdMap;
     private Map<Integer, XSLFSimpleShape> _placeholderByTypeMap;
 
+    private final BitSet shapeIds = new BitSet();
+
     public XSLFSheet() {
         super();
     }
@@ -105,6 +108,26 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
         throw new IllegalStateException("SlideShow was not found");
     }
 
+    protected int allocateShapeId() {
+        final int nextId = shapeIds.nextClearBit(1);
+        shapeIds.set(nextId);
+        return nextId;
+    }
+
+    protected void registerShapeId(final int shapeId) {
+        if (shapeIds.get(shapeId)) {
+            LOG.log(POILogger.WARN, "shape id "+shapeId+" has been already used.");
+        }
+        shapeIds.set(shapeId);
+    }
+
+    protected void deregisterShapeId(final int shapeId) {
+        if (!shapeIds.get(shapeId)) {
+            LOG.log(POILogger.WARN, "shape id "+shapeId+" hasn't been registered.");
+        }
+        shapeIds.clear(shapeId);
+    }
+
     protected static List<XSLFShape> buildShapes(CTGroupShape spTree, XSLFShapeContainer parent){
         final XSLFSheet sheet = (parent instanceof XSLFSheet) ? (XSLFSheet)parent : ((XSLFShape)parent).getSheet();
 
@@ -328,9 +351,12 @@ implements XSLFShapeContainer, Sheet<XSLFShape,XSLFTextParagraph> {
     public boolean removeShape(XSLFShape xShape) {
         XmlObject obj = xShape.getXmlObject();
         CTGroupShape spTree = getSpTree();
+        deregisterShapeId(xShape.getShapeId());
         if(obj instanceof CTShape){
             spTree.getSpList().remove(obj);
         } else if (obj instanceof CTGroupShape) {
+            XSLFGroupShape gs = (XSLFGroupShape)xShape;
+            new ArrayList<>(gs.getShapes()).forEach(gs::removeShape);
             spTree.getGrpSpList().remove(obj);
         } else if (obj instanceof CTConnector) {
             spTree.getCxnSpList().remove(obj);
index ba9fe227d8ebd1616001b0f3d6c47874f7aa6766..50cc38fb774cc47bcf37327804994a6b5820a845 100644 (file)
@@ -166,7 +166,7 @@ public class XSLFTable extends XSLFGraphicFrame implements Iterable<XSLFTableRow
 
         CTNonVisualDrawingProps cnv = nvGr.addNewCNvPr();
         cnv.setName("Table " + shapeId);
-        cnv.setId(shapeId + 1);
+        cnv.setId(shapeId);
         nvGr.addNewCNvGraphicFramePr().addNewGraphicFrameLocks().setNoGrp(true);
         nvGr.addNewNvPr();
 
index 2f172d2fdc6309d5d1eb584bd100094e04d6db87..6fcb94eedd0f3a770a81ef1a0c67b4eff3b7dfeb 100644 (file)
@@ -46,7 +46,7 @@ public class XSLFTextBox extends XSLFAutoShape
         CTShapeNonVisual nvSpPr = ct.addNewNvSpPr();
         CTNonVisualDrawingProps cnv = nvSpPr.addNewCNvPr();
         cnv.setName("TextBox " + shapeId);
-        cnv.setId(shapeId + 1);
+        cnv.setId(shapeId);
         nvSpPr.addNewCNvSpPr().setTxBox(true);
         nvSpPr.addNewNvPr();
         CTShapeProperties spPr = ct.addNewSpPr();
index a6a6467f9e103f405e3db7a845efd1bc6945350e..2c6d4eeed16091d0158d4a9450925e7f2bb1695f 100644 (file)
@@ -36,8 +36,11 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URI;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 import java.util.Optional;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import javax.imageio.ImageIO;
@@ -56,10 +59,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.Shape;
 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;
 import org.apache.poi.xslf.usermodel.XSLFGroupShape;
@@ -693,13 +696,6 @@ public class TestXSLFBugs {
             assertNotNull(notesSlide);
         }
 
-        /*OutputStream stream = new FileOutputStream("/tmp/test.pptx");
-        try {
-            ppt.write(stream);
-        } finally {
-            stream.close();
-        }*/
-
         ppt.close();
     }
 
@@ -727,4 +723,32 @@ public class TestXSLFBugs {
         }
         ppt.close();
     }
+
+    @Test
+    public void bug62051() throws IOException {
+        final Function<List<XSLFShape>, int[]> ids = (shapes) ->
+            shapes.stream().mapToInt(Shape::getShapeId).toArray();
+
+        try (final XMLSlideShow ppt = new XMLSlideShow()) {
+            final XSLFSlide slide = ppt.createSlide();
+            final List<XSLFShape> shapes = new ArrayList<>();
+            shapes.add(slide.createAutoShape());
+            final XSLFGroupShape g1 = slide.createGroup();
+            shapes.add(g1);
+            final XSLFGroupShape g2 = g1.createGroup();
+            shapes.add(g2);
+            shapes.add(g2.createAutoShape());
+            shapes.add(slide.createAutoShape());
+            shapes.add(g2.createAutoShape());
+            shapes.add(g1.createAutoShape());
+            assertArrayEquals(new int[]{ 2,3,4,5,6,7,8 }, ids.apply(shapes));
+            g1.removeShape(g2);
+            shapes.remove(5);
+            shapes.remove(3);
+            shapes.remove(2);
+            shapes.add(g1.createAutoShape());
+            assertArrayEquals(new int[]{ 2,3,6,8,4 }, ids.apply(shapes));
+
+        }
+    }
 }
index 736d276d922fb645b347cbc7b18cfaad5340e56f..1df72597d5eeb0029d0534b2183d7295e177168c 100644 (file)
@@ -94,7 +94,7 @@ public class TestXSLFTable {
         assertNotNull(tbl.getCTTable().getTblGrid());
         assertNotNull(tbl.getCTTable().getTblPr());
         assertTrue(tbl.getXmlObject() instanceof CTGraphicalObjectFrame);
-        assertEquals("Table 1", tbl.getShapeName());
+        assertEquals("Table 2", tbl.getShapeName());
         assertEquals(2, tbl.getShapeId());
         assertEquals(0, tbl.getRows().size());
         assertEquals(0, tbl.getCTTable().sizeOfTrArray());
index 4edcb53603708fb3e5c350725253069b4eb96a34..085d617eb85fe6108468cd8ccba052724884e2df 100644 (file)
@@ -538,9 +538,7 @@ public abstract class HSLFShape implements Shape<HSLFShape,HSLFTextParagraph> {
         return new Color(r, g, b);
     }
 
-    /**
-     * @return id for the shape.
-     */
+    @Override
     public int getShapeId(){
         EscherSpRecord spRecord = getEscherChild(EscherSpRecord.RECORD_ID);
         return spRecord == null ? 0 : spRecord.getShapeId();