]> source.dussan.org Git - poi.git/commitdiff
[bug-66242] Fix issue with orphaned (in package) images and notes post slide removal...
authorPJ Fanning <fanningpj@apache.org>
Sun, 28 Aug 2022 09:35:48 +0000 (09:35 +0000)
committerPJ Fanning <fanningpj@apache.org>
Sun, 28 Aug 2022 09:35:48 +0000 (09:35 +0000)
git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1903727 13f79535-47bb-0310-9956-ffa450edef68

poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java
poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFNotes.java
poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFSlide.java
poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFSlideShow.java

index 5d861393d2627af8ea64dca1e340477058ca070b..3392f3a02becc004e7b6f26dc2e64668ec9c3216 100644 (file)
@@ -190,7 +190,7 @@ public class XMLSlideShow extends POIXMLDocument
             _masters.clear();
             if (_presentation.isSetSldMasterIdLst()) {
                 _presentation.getSldMasterIdLst().getSldMasterIdList().forEach(
-                   id -> _masters.add(masterMap.get(id.getId2()))
+                        id -> _masters.add(masterMap.get(id.getId2()))
                 );
             }
 
@@ -248,10 +248,10 @@ public class XMLSlideShow extends POIXMLDocument
      */
     public XSLFSlide createSlide(XSLFSlideLayout layout) {
         CTSlideIdList slideList = _presentation.isSetSldIdLst()
-            ? _presentation.getSldIdLst() : _presentation.addNewSldIdLst();
+                ? _presentation.getSldIdLst() : _presentation.addNewSldIdLst();
 
         OptionalLong maxId = Stream.of(slideList.getSldIdArray())
-            .mapToLong(CTSlideIdListEntry::getId).max();
+                .mapToLong(CTSlideIdListEntry::getId).max();
 
         final XSLFRelation relationType = XSLFRelation.SLIDE;
         final int slideNumber = (int)(Math.max(maxId.orElse(0),255)+1);
@@ -466,6 +466,15 @@ public class XMLSlideShow extends POIXMLDocument
         sldIdLst.setSldIdArray(entries);
     }
 
+    /**
+     * Remove a slide from this presentation.
+     *
+     * @param index The slide number to remove.
+     * @return The slide that was removed.
+     *
+     * @throws RuntimeException a number of runtime exceptions can be thrown, especially if there are problems with the
+     * input format
+     */
     public XSLFSlide removeSlide(int index) {
         XSLFSlide slide = _slides.remove(index);
         removeRelation(slide);
@@ -478,11 +487,38 @@ public class XMLSlideShow extends POIXMLDocument
             } else if (p instanceof XSLFSlideLayout) {
                 XSLFSlideLayout layout = (XSLFSlideLayout) p;
                 slide.removeLayoutRelation(layout);
+            } else if (p instanceof XSLFNotes) {
+                XSLFNotes notes = slide.removeNotes(_notesMaster);
+                removeRelation(notes);
+            } else if (p instanceof XSLFPictureData) {
+                XSLFPictureData picture = (XSLFPictureData) p;
+                removePictureRelations(slide, picture);
+                _pictures.remove(picture);
             }
         }
         return slide;
     }
 
+    private void removePictureRelations(XSLFSlide slide, XSLFPictureData picture) {
+        removePictureRelations(slide, slide, picture);
+    }
+
+    private void removePictureRelations(XSLFSlide slide, XSLFShapeContainer container, XSLFPictureData picture) {
+        for (XSLFShape shape : container.getShapes()) {
+            // Find either group shapes (and recurse) ...
+            if (shape instanceof XSLFGroupShape) {
+                removePictureRelations(slide, (XSLFGroupShape)shape, picture);
+            }
+            // ... or the picture shape with this picture data and remove it's relation to the picture data.
+            if (shape instanceof XSLFPictureShape) {
+                XSLFPictureShape pic = (XSLFPictureShape) shape;
+                if (pic.getPictureData() == picture) {
+                    slide.removePictureRelation(pic);
+                }
+            }
+        }
+    }
+
     @Override
     public Dimension getPageSize() {
         CTSlideSize sz = _presentation.getSldSz();
index 2abbc340b0031291003e44e8c7f92cbad1365079..ce0bfecf631a68d4c071eeed9d4c7b07feeef4a5 100644 (file)
@@ -111,4 +111,9 @@ implements Notes<XSLFShape,XSLFTextParagraph> {
     String mapSchemeColor(String schemeColor) {
         return mapSchemeColor(_notes.getClrMapOvr(), schemeColor);
     }
+
+    void removeRelations(XSLFSlide slide, XSLFNotesMaster master) {
+        super.removeRelation(slide);
+        super.removeRelation(master);
+    }
 }
index f7f4e8dc1de57ce7f626c9a2b37b846cda409dca..c48c8d162f6c77a02517a963214e92e9a5afa208 100644 (file)
@@ -239,6 +239,21 @@ implements Slide<XSLFShape,XSLFTextParagraph> {
         return _notes;
     }
 
+    public XSLFNotes removeNotes(XSLFNotesMaster master) {
+        XSLFNotes notesForSlide = getNotes();
+        if (notesForSlide == null) {
+            // No notes to remove.
+            return null;
+        }
+
+        notesForSlide.removeRelations(this, master);
+        removeRelation(notesForSlide);
+
+        _notes = null;
+
+        return notesForSlide;
+    }
+
     @Override
     public String getTitle(){
         XSLFTextShape txt = getTextShapeByType(Placeholder.TITLE);
index adcbf3f34b7d2aaf46a69f95039b27ad6e746808..45e2aa31c2d8fa57bf15af22f3edfe42e03026e9 100644 (file)
@@ -21,126 +21,310 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 
 import java.awt.Dimension;
+import java.awt.Rectangle;
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.util.List;
+import java.util.regex.Pattern;
 
 import org.apache.poi.ooxml.POIXMLDocumentPart;
+import org.apache.poi.openxml4j.opc.PackagePart;
+import org.apache.poi.sl.usermodel.PictureData;
+import org.apache.poi.sl.usermodel.Placeholder;
 import org.apache.poi.xslf.XSLFTestDataSamples;
 import org.junit.jupiter.api.Test;
 
 class TestXSLFSlideShow {
     @Test
     void testCreateSlide() throws IOException {
-        XMLSlideShow  ppt = new XMLSlideShow();
-        assertEquals(0, ppt.getSlides().size());
+        try (XMLSlideShow ppt = new XMLSlideShow()) {
+            assertEquals(0, ppt.getSlides().size());
 
-        XSLFSlide slide1 = ppt.createSlide();
-        assertEquals(1, ppt.getSlides().size());
-        assertSame(slide1, ppt.getSlides().get(0));
+            XSLFSlide slide1 = ppt.createSlide();
+            assertEquals(1, ppt.getSlides().size());
+            assertSame(slide1, ppt.getSlides().get(0));
 
-        List<POIXMLDocumentPart> rels =  slide1.getRelations();
-        assertEquals(1, rels.size());
-        assertEquals(slide1.getSlideMaster().getLayout(SlideLayout.BLANK), rels.get(0));
+            List<POIXMLDocumentPart> rels =  slide1.getRelations();
+            assertEquals(1, rels.size());
+            assertEquals(slide1.getSlideMaster().getLayout(SlideLayout.BLANK), rels.get(0));
 
-        XSLFSlide slide2 = ppt.createSlide();
-        assertEquals(2, ppt.getSlides().size());
-        assertSame(slide2, ppt.getSlides().get(1));
+            XSLFSlide slide2 = ppt.createSlide();
+            assertEquals(2, ppt.getSlides().size());
+            assertSame(slide2, ppt.getSlides().get(1));
 
-        ppt.setSlideOrder(slide2, 0);
-        assertSame(slide2, ppt.getSlides().get(0));
-        assertSame(slide1, ppt.getSlides().get(1));
+            ppt.setSlideOrder(slide2, 0);
+            assertSame(slide2, ppt.getSlides().get(0));
+            assertSame(slide1, ppt.getSlides().get(1));
 
-        XMLSlideShow ppt2 = XSLFTestDataSamples.writeOutAndReadBack(ppt);
-        assertEquals(2, ppt2.getSlides().size());
-        rels =  ppt2.getSlides().get(0).getRelations();
-
-        ppt2.close();
-        ppt.close();
+            try (XMLSlideShow ppt2 = XSLFTestDataSamples.writeOutAndReadBack(ppt)) {
+                assertEquals(2, ppt2.getSlides().size());
+                rels = ppt2.getSlides().get(0).getRelations();
+                assertNotNull(rels);
+            }
+        }
     }
 
     @Test
     void testRemoveSlide() throws IOException {
-        XMLSlideShow  ppt = new XMLSlideShow();
-        assertEquals(0, ppt.getSlides().size());
+        try (XMLSlideShow ppt = new XMLSlideShow()) {
+            assertEquals(0, ppt.getSlides().size());
+
+            XSLFSlide slide1 = ppt.createSlide();
+            XSLFSlide slide2 = ppt.createSlide();
+
+            assertEquals(2, ppt.getSlides().size());
+            assertSame(slide1, ppt.getSlides().get(0));
+            assertSame(slide2, ppt.getSlides().get(1));
+
+            XSLFSlide removedSlide = ppt.removeSlide(0);
+            assertSame(slide1, removedSlide);
+
+            assertEquals(1, ppt.getSlides().size());
+            assertSame(slide2, ppt.getSlides().get(0));
+
+            try (XMLSlideShow ppt2 = XSLFTestDataSamples.writeOutAndReadBack(ppt)) {
+                assertEquals(1, ppt2.getSlides().size());
+
+                // Check that the slide is actually removed from the package.
+                String slidePartRegEx = "/ppt/slides/slide[0-9]+\\.xml";
+                List<PackagePart> slideParts = ppt2.getPackage().getPartsByName(Pattern.compile(slidePartRegEx));
+                assertEquals(1, slideParts.size());
+            }
+        }
+    }
+
+
+    /**
+     * This test ensures that if a slide (with notes) is removed, that it
+     * is ACTUALLY removed (including the notes), and not left orphaned
+     * when the PPTX is later written.
+     *
+     * @throws IOException If there is an I/O issue during the test.
+     */
+    @Test
+    void testRemoveSlideThatHasNotes() throws IOException {
+        try (XMLSlideShow ppt = new XMLSlideShow()) {
+            assertEquals(0, ppt.getSlides().size());
 
-        XSLFSlide slide1 = ppt.createSlide();
-        XSLFSlide slide2 = ppt.createSlide();
+            XSLFSlide slide1 = ppt.createSlide();
+            XSLFSlide slide2 = ppt.createSlide();
 
-        assertEquals(2, ppt.getSlides().size());
-        assertSame(slide1, ppt.getSlides().get(0));
-        assertSame(slide2, ppt.getSlides().get(1));
+            XSLFNotes note = ppt.getNotesSlide(slide1);
+            for (XSLFTextShape shape : note.getPlaceholders()) {
+                if (shape.getTextType() == Placeholder.BODY) {
+                    shape.setText("Some notes");
+                    break;
+                }
+            }
 
-        XSLFSlide removedSlide = ppt.removeSlide(0);
-        assertSame(slide1, removedSlide);
+            assertEquals(2, ppt.getSlides().size());
+            assertSame(slide1, ppt.getSlides().get(0));
+            assertSame(slide2, ppt.getSlides().get(1));
 
-        assertEquals(1, ppt.getSlides().size());
-        assertSame(slide2, ppt.getSlides().get(0));
+            XSLFSlide removedSlide = ppt.removeSlide(0);
+            assertSame(slide1, removedSlide);
 
-        XMLSlideShow ppt2 = XSLFTestDataSamples.writeOutAndReadBack(ppt);
-        assertEquals(1, ppt2.getSlides().size());
+            assertEquals(1, ppt.getSlides().size());
+            assertSame(slide2, ppt.getSlides().get(0));
 
-        ppt2.close();
-        ppt.close();
+            try (XMLSlideShow ppt2 = XSLFTestDataSamples.writeOutAndReadBack(ppt)) {
+                assertEquals(1, ppt2.getSlides().size());
+
+                // Check that the slide is actually removed from the package.
+                String slidePartRegEx = "/ppt/slides/slide[0-9]+\\.xml";
+                List<PackagePart> slideParts = ppt2.getPackage().getPartsByName(Pattern.compile(slidePartRegEx));
+                assertEquals(1, slideParts.size());
+
+                // Check that there is no note slide part.
+                String notePartRegEx = "/ppt/notesSlides/notesSlide[0-9]+\\.xml";
+                List<PackagePart> noteParts = ppt2.getPackage().getPartsByName(Pattern.compile(notePartRegEx));
+                assertEquals(0, noteParts.size());
+            }
+        }
+    }
+
+
+    /**
+     * This test ensures that if a slide (with notes and images) is removed, that it
+     * is ACTUALLY removed (including the notes and images), and not left orphaned
+     * when the PPTX is later written.
+     *
+     * @throws IOException If there is an I/O issue during the test.
+     */
+    @Test
+    void testRemoveSlideThatHasNotesAndImages() throws IOException {
+        try (XMLSlideShow ppt = new XMLSlideShow()) {
+            assertEquals(0, ppt.getSlides().size());
+
+            XSLFSlide slide1 = ppt.createSlide();
+            XSLFSlide slide2 = ppt.createSlide();
+
+            // NOTE: This image is INVALID but this doesnt matter for THIS test.
+            XSLFPictureData pictData = ppt.addPicture(
+                    new ByteArrayInputStream(new byte[] { 00, 01, 02 }), PictureData.PictureType.PNG);
+            XSLFPictureShape picShape = slide1.createPicture(pictData);
+            picShape.setAnchor(new Rectangle(10, 10, 200, 100));
+
+            XSLFNotes note = ppt.getNotesSlide(slide1);
+            for (XSLFTextShape shape : note.getPlaceholders()) {
+                if (shape.getTextType() == Placeholder.BODY) {
+                    shape.setText("Some notes");
+                    break;
+                }
+            }
+
+            assertEquals(2, ppt.getSlides().size());
+            assertSame(slide1, ppt.getSlides().get(0));
+            assertSame(slide2, ppt.getSlides().get(1));
+
+            XSLFSlide removedSlide = ppt.removeSlide(0);
+            assertSame(slide1, removedSlide);
+
+            assertEquals(1, ppt.getSlides().size());
+            assertSame(slide2, ppt.getSlides().get(0));
+
+            try (XMLSlideShow ppt2 = XSLFTestDataSamples.writeOutAndReadBack(ppt)) {
+                assertEquals(1, ppt2.getSlides().size());
+
+                // Check that the slide is actually removed from the package.
+                String slidePartRegEx = "/ppt/slides/slide[0-9]+\\.xml";
+                List<PackagePart> slideParts = ppt2.getPackage().getPartsByName(Pattern.compile(slidePartRegEx));
+                assertEquals(1, slideParts.size());
+
+                // Check that there is no note slide part.
+                String notePartRegEx = "/ppt/notesSlides/notesSlide[0-9]+\\.xml";
+                List<PackagePart> noteParts = ppt2.getPackage().getPartsByName(Pattern.compile(notePartRegEx));
+                assertEquals(0, noteParts.size());
+
+                // Check that there is no image slide part.
+                String imagePartRegEx = "/ppt/media/image[0-9]+\\.png";
+                List<PackagePart> imageParts = ppt2.getPackage().getPartsByName(Pattern.compile(imagePartRegEx));
+                assertEquals(0, imageParts.size());
+            }
+        }
+    }
+
+    /**
+     * This test ensures that if a slide (with notes and images [inside a group])
+     * is removed, that it is ACTUALLY removed (including the notes and images),
+     * and not left orphaned when the PPTX is later written.
+     *
+     * @throws IOException If there is an I/O issue during the test.
+     */
+    @Test
+    void testRemoveSlideThatHasNotesAndImagesInsideAGroup() throws IOException {
+        try (XMLSlideShow  ppt = new XMLSlideShow()) {
+            assertEquals(0, ppt.getSlides().size());
+
+            XSLFSlide slide1 = ppt.createSlide();
+            XSLFSlide slide2 = ppt.createSlide();
+
+            XSLFGroupShape group = slide1.createGroup();
+
+            // NOTE: This image is INVALID but this doesnt matter for THIS test.
+            XSLFPictureData pictData = ppt.addPicture(
+                    new ByteArrayInputStream(new byte[] { 00, 01, 02 }), PictureData.PictureType.PNG);
+            XSLFPictureShape picShape = group.createPicture(pictData);
+            picShape.setAnchor(new Rectangle(10, 10, 200, 100));
+
+            XSLFNotes note = ppt.getNotesSlide(slide1);
+            for (XSLFTextShape shape : note.getPlaceholders()) {
+                if (shape.getTextType() == Placeholder.BODY) {
+                    shape.setText("Some notes");
+                    break;
+                }
+            }
+
+            assertEquals(2, ppt.getSlides().size());
+            assertSame(slide1, ppt.getSlides().get(0));
+            assertSame(slide2, ppt.getSlides().get(1));
+
+            XSLFSlide removedSlide = ppt.removeSlide(0);
+            assertSame(slide1, removedSlide);
+
+            assertEquals(1, ppt.getSlides().size());
+            assertSame(slide2, ppt.getSlides().get(0));
+
+            try (XMLSlideShow ppt2 = XSLFTestDataSamples.writeOutAndReadBack(ppt)) {
+                assertEquals(1, ppt2.getSlides().size());
+
+                // Check that the slide is actually removed from the package.
+                String slidePartRegEx = "/ppt/slides/slide[0-9]+\\.xml";
+                List<PackagePart> slideParts = ppt2.getPackage().getPartsByName(Pattern.compile(slidePartRegEx));
+                assertEquals(1, slideParts.size());
+
+                // Check that there is no note slide part.
+                String notePartRegEx = "/ppt/notesSlides/notesSlide[0-9]+\\.xml";
+                List<PackagePart> noteParts = ppt2.getPackage().getPartsByName(Pattern.compile(notePartRegEx));
+                assertEquals(0, noteParts.size());
+
+                // Check that there is no image slide part.
+                String imagePartRegEx = "/ppt/media/image[0-9]+\\.png";
+                List<PackagePart> imageParts = ppt2.getPackage().getPartsByName(Pattern.compile(imagePartRegEx));
+                imageParts.forEach(System.out::println);
+                assertEquals(0, imageParts.size());
+            }
+        }
     }
 
     @Test
     void testDimension() throws IOException {
-        XMLSlideShow  ppt = new XMLSlideShow();
-        Dimension sz = ppt.getPageSize();
-        assertEquals(720, sz.width);
-        assertEquals(540, sz.height);
-        ppt.setPageSize(new Dimension(792, 612));
-        sz = ppt.getPageSize();
-        assertEquals(792, sz.width);
-        assertEquals(612, sz.height);
-        ppt.close();
+        try (XMLSlideShow  ppt = new XMLSlideShow()) {
+            Dimension sz = ppt.getPageSize();
+            assertEquals(720, sz.width);
+            assertEquals(540, sz.height);
+            ppt.setPageSize(new Dimension(792, 612));
+            sz = ppt.getPageSize();
+            assertEquals(792, sz.width);
+            assertEquals(612, sz.height);
+        }
     }
 
     @Test
     void testSlideMasters() throws IOException {
-        XMLSlideShow  ppt = new XMLSlideShow();
-        List<XSLFSlideMaster> masters = ppt.getSlideMasters();
-        assertEquals(1, masters.size());
+        try (XMLSlideShow  ppt = new XMLSlideShow()) {
+            List<XSLFSlideMaster> masters = ppt.getSlideMasters();
+            assertEquals(1, masters.size());
 
-        XSLFSlide slide = ppt.createSlide();
-        assertSame(masters.get(0), slide.getSlideMaster());
-        ppt.close();
+            XSLFSlide slide = ppt.createSlide();
+            assertSame(masters.get(0), slide.getSlideMaster());
+        }
     }
 
     @Test
     void testSlideLayout() throws IOException {
-        XMLSlideShow  ppt = new XMLSlideShow();
-        List<XSLFSlideMaster> masters = ppt.getSlideMasters();
-        assertEquals(1, masters.size());
+        try (XMLSlideShow  ppt = new XMLSlideShow()) {
+            List<XSLFSlideMaster> masters = ppt.getSlideMasters();
+            assertEquals(1, masters.size());
 
-        XSLFSlide slide = ppt.createSlide();
-        XSLFSlideLayout layout = slide.getSlideLayout();
-        assertNotNull(layout);
+            XSLFSlide slide = ppt.createSlide();
+            XSLFSlideLayout layout = slide.getSlideLayout();
+            assertNotNull(layout);
 
-        assertSame(masters.get(0), layout.getSlideMaster());
-        ppt.close();
+            assertSame(masters.get(0), layout.getSlideMaster());
+        }
     }
 
     @Test
     void testSlideLayoutNames() throws IOException {
         final String[] names = {
-                "Blank", "Title Only", "Section Header", "Picture with Caption", "Title and Content"
-                , "Title Slide", "Title and Vertical Text", "Vertical Title and Text", "Comparison"
-                , "Two Content", "Content with Caption"
-        };
-        XMLSlideShow ppt = XSLFTestDataSamples.openSampleDocument("layouts.pptx");
-        for (String name : names) {
-            assertNotNull(ppt.findLayout(name));
-        }
-        final SlideLayout[] layTypes = {
-                SlideLayout.BLANK, SlideLayout.TITLE_ONLY, SlideLayout.SECTION_HEADER
-                , SlideLayout.PIC_TX, SlideLayout.TITLE_AND_CONTENT, SlideLayout.TITLE
-                , SlideLayout.VERT_TX, SlideLayout.VERT_TITLE_AND_TX, SlideLayout.TWO_TX_TWO_OBJ
-                , SlideLayout.TWO_OBJ, SlideLayout.OBJ_TX
+                "Blank", "Title Only", "Section Header", "Picture with Caption", "Title and Content",
+                "Title Slide", "Title and Vertical Text", "Vertical Title and Text", "Comparison",
+                "Two Content", "Content with Caption"
         };
-        for (SlideLayout sl : layTypes){
-            assertNotNull(ppt.getSlideMasters().get(0).getLayout(sl));
+        try (XMLSlideShow ppt = XSLFTestDataSamples.openSampleDocument("layouts.pptx")) {
+            for (String name : names) {
+                assertNotNull(ppt.findLayout(name));
+            }
+            final SlideLayout[] layTypes = {
+                    SlideLayout.BLANK, SlideLayout.TITLE_ONLY, SlideLayout.SECTION_HEADER,
+                    SlideLayout.PIC_TX, SlideLayout.TITLE_AND_CONTENT, SlideLayout.TITLE,
+                    SlideLayout.VERT_TX, SlideLayout.VERT_TITLE_AND_TX, SlideLayout.TWO_TX_TWO_OBJ,
+                    SlideLayout.TWO_OBJ, SlideLayout.OBJ_TX
+            };
+            for (SlideLayout sl : layTypes){
+                assertNotNull(ppt.getSlideMasters().get(0).getLayout(sl));
+            }
         }
-        ppt.close();
     }
 }