diff --git a/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java b/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java index 4c9796871d6..cd127354e22 100644 --- a/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java +++ b/src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java @@ -1496,22 +1496,33 @@ public String addPictureData(InputStream is, int format) throws InvalidFormatExc } /** - * get the next free ImageNumber + * Calculates the index within the package of the next image of a given format. * - * @param format - * @return the next free ImageNumber - * @throws InvalidFormatException If the format of the picture is not known. + * Images within the package are indexed by image format. Loosely, this can be thought of as a + * {@code MultiMap}. For example, a package with 2 JPEGs and 1 PNG may organize the images like: + *
+     * {
+     *     JPEG: [1.jpeg, 2.jpeg],
+     *     PNG: [1.png]
+     * }
+     * 
. + * + * The index returned is not guaranteed to be the highest index. For example, a package could already have + * {@code [1.png, 3.png]}, in which case this method may return {@code 2}. + * + * @param format Image format identifier. See {@code PICTURE_TYPE_*} constants in {@link Document} for allowed + * values. + * @return the index of the next next slot for the given format. + * @throws InvalidFormatException If the specified format is unknown. */ public int getNextPicNameNumber(int format) throws InvalidFormatException { - int img = getAllPackagePictures().size() + 1; - String proposal = XWPFPictureData.RELATIONS[format].getFileName(img); - PackagePartName createPartName = PackagingURIHelper.createPartName(proposal); - while (this.getPackage().getPart(createPartName) != null) { - img++; - proposal = XWPFPictureData.RELATIONS[format].getFileName(img); - createPartName = PackagingURIHelper.createPartName(proposal); + POIXMLRelation relation; + try { + relation = XWPFPictureData.RELATIONS[format]; + } catch (ArrayIndexOutOfBoundsException e) { + throw new InvalidFormatException("Unknown image format " + format); } - return img; + return getPackage().getUnusedPartIndex(relation.getDefaultFileName()); } /** diff --git a/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFDocument.java b/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFDocument.java index ed5c7ea32d9..e2732868531 100644 --- a/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFDocument.java +++ b/src/ooxml/testcases/org/apache/poi/xwpf/usermodel/TestXWPFDocument.java @@ -17,14 +17,6 @@ Licensed to the Apache Software Foundation (ASF) under one or more package org.apache.poi.xwpf.usermodel; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import java.io.IOException; import java.io.OutputStream; import java.util.Arrays; @@ -47,6 +39,8 @@ Licensed to the Apache Software Foundation (ASF) under one or more import org.junit.Test; import org.openxmlformats.schemas.wordprocessingml.x2006.main.CTP; +import static org.junit.Assert.*; + public final class TestXWPFDocument { @Test @@ -490,4 +484,49 @@ public void testWriteFromReadOnlyOPC() throws Exception { assertEquals(origText, ext.getText()); } } + + @Test + public void getNextPicNameNumberThrowsInvalidFormatExceptionForUnknownFormat() throws IOException { + try (XWPFDocument doc = new XWPFDocument()) { + try { + doc.getNextPicNameNumber(Integer.MAX_VALUE); + fail("Expected InvalidFormatException"); + } catch (InvalidFormatException expected) { + } + } + } + + @Test + public void getNextPicNameNumberReturnsAvailableIndex() throws IOException, InvalidFormatException { + try (XWPFDocument doc = new XWPFDocument()) { + + // For an empty document, slot 1 makes sense, but any slot will result in a valid document. + int slot1 = doc.getNextPicNameNumber(Document.PICTURE_TYPE_JPEG); + assertTrue(slot1 > 0); + + // For a document with 1 image in it, slot 2 makes sense, but any slot will result in a valid document as + // long as the new slot is different than the first slot. + doc.addPictureData(new byte[1], Document.PICTURE_TYPE_JPEG); + int slot2 = doc.getNextPicNameNumber(Document.PICTURE_TYPE_JPEG); + assertNotEquals(slot1, slot2); + assertTrue(slot2 > 0); + + // For a document with 2 images that are separated by a gap, as in [1.jpeg, 3.jpeg], it makes sense for the + // gap to be filled, but any slot will result in a valid document as long as the new slot is different than + // the existing slots. + String image2 = doc.addPictureData(new byte[2], Document.PICTURE_TYPE_JPEG); + int slot3 = doc.getNextPicNameNumber(Document.PICTURE_TYPE_JPEG); + doc.addPictureData(new byte[3], Document.PICTURE_TYPE_JPEG); + doc.getPackage().removePart(doc.getPartById(image2)); + slot2 = doc.getNextPicNameNumber(Document.PICTURE_TYPE_JPEG); + assertNotEquals(slot1, slot2); + assertNotEquals(slot3, slot2); + assertTrue(slot2 > 0); + + // When adding an image of a different format, it makes sense to start the indexing from 1, but any slot + // will result in a valid document. + int pngSlot1 = doc.getNextPicNameNumber(Document.PICTURE_TYPE_PNG); + assertTrue(pngSlot1 > 0); + } + } }