Change XWPFDocument#getNextPicNameNumber slot selection implementation

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<Format, Index>}. For example, a package with 2 JPEGs and 1 PNG may organize the images like:
+ * <pre>
+ * {
+ * JPEG: [1.jpeg, 2.jpeg],
+ * PNG: [1.png]
+ * }
+ * </pre>.
+ *
+ * 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());
  }
 
  /**

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);
+ }
+ }
 }