From 7fd339952b0622eb760f17a0b73077f0b8d269a8 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 28 Aug 2022 09:35:48 +0000 Subject: [PATCH] [bug-66242] Fix issue with orphaned (in package) images and notes post slide removal. Thanks to gffloodg. This closes #377 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1903727 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/xslf/usermodel/XMLSlideShow.java | 42 ++- .../apache/poi/xslf/usermodel/XSLFNotes.java | 5 + .../apache/poi/xslf/usermodel/XSLFSlide.java | 15 + .../poi/xslf/usermodel/TestXSLFSlideShow.java | 332 ++++++++++++++---- 4 files changed, 317 insertions(+), 77 deletions(-) diff --git a/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java b/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java index 5d861393d2..3392f3a02b 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java @@ -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(); diff --git a/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFNotes.java b/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFNotes.java index 2abbc340b0..ce0bfecf63 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFNotes.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFNotes.java @@ -111,4 +111,9 @@ implements Notes { String mapSchemeColor(String schemeColor) { return mapSchemeColor(_notes.getClrMapOvr(), schemeColor); } + + void removeRelations(XSLFSlide slide, XSLFNotesMaster master) { + super.removeRelation(slide); + super.removeRelation(master); + } } diff --git a/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFSlide.java b/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFSlide.java index f7f4e8dc1d..c48c8d162f 100644 --- a/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFSlide.java +++ b/poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFSlide.java @@ -239,6 +239,21 @@ implements Slide { 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); diff --git a/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFSlideShow.java b/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFSlideShow.java index adcbf3f34b..45e2aa31c2 100644 --- a/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFSlideShow.java +++ b/poi-ooxml/src/test/java/org/apache/poi/xslf/usermodel/TestXSLFSlideShow.java @@ -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 rels = slide1.getRelations(); - assertEquals(1, rels.size()); - assertEquals(slide1.getSlideMaster().getLayout(SlideLayout.BLANK), rels.get(0)); + List 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(); + 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)); + 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); + XSLFSlide removedSlide = ppt.removeSlide(0); + assertSame(slide1, removedSlide); - assertEquals(1, ppt.getSlides().size()); - assertSame(slide2, ppt.getSlides().get(0)); + assertEquals(1, ppt.getSlides().size()); + assertSame(slide2, ppt.getSlides().get(0)); - XMLSlideShow ppt2 = XSLFTestDataSamples.writeOutAndReadBack(ppt); - assertEquals(1, ppt2.getSlides().size()); + try (XMLSlideShow ppt2 = XSLFTestDataSamples.writeOutAndReadBack(ppt)) { + assertEquals(1, ppt2.getSlides().size()); - ppt2.close(); - ppt.close(); + // Check that the slide is actually removed from the package. + String slidePartRegEx = "/ppt/slides/slide[0-9]+\\.xml"; + List 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(); + + 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 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 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 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 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 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 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 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 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 masters = ppt.getSlideMasters(); - assertEquals(1, masters.size()); + try (XMLSlideShow ppt = new XMLSlideShow()) { + List 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 masters = ppt.getSlideMasters(); - assertEquals(1, masters.size()); + try (XMLSlideShow ppt = new XMLSlideShow()) { + List 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" + "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)); + 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)); + } } - 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(); } }