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