poi: #60255 When creating a XSSF drawing, find the next available document part, even if another type has pinched the next number

ooxml/POIXMLDocumentPart.cs

@@ -573,6 +573,65 @@ using System.Xml;
             return CreateRelationship(descriptor, factory, idx, false).DocumentPart;
         }
 
+        /**
+         * Identifies the next available part number for a part of the given type,
+         *  if possible, otherwise -1 if none are available.
+         * The found (valid) index can then be safely given to
+         *  {@link #createRelationship(POIXMLRelation, POIXMLFactory, int)} or
+         *  {@link #createRelationship(POIXMLRelation, POIXMLFactory, int, boolean)}
+         *  without naming clashes.
+         * If parts with other types are already claiming a name for this relationship
+         *  type (eg a {@link XSSFRelation#CHART} using the drawing part namespace 
+         *  normally used by {@link XSSFRelation#DRAWINGS}), those will be considered
+         *  when finding the next spare number.
+         *
+         * @param descriptor The relationship type to find the part number for
+         * @param minIdx The minimum free index to assign, use -1 for any
+         * @return The next free part number, or -1 if none available
+         */
+        protected internal int GetNextPartNumber(POIXMLRelation descriptor, int minIdx)
+        {
+            OPCPackage pkg = packagePart.Package;
+
+            try
+            {
+                if (descriptor.DefaultFileName.Equals(descriptor.GetFileName(9999)))
+                {
+                    // Non-index based, check if default is free
+                    PackagePartName ppName = PackagingUriHelper.CreatePartName(descriptor.DefaultFileName);
+                    if (pkg.ContainPart(ppName))
+                    {
+                        // Default name already taken, not index based, nothing free
+                        return -1;
+                    }
+                    else
+                    {
+                        // Default name free
+                        return 0;
+                    }
+                }
+
+                // Default to searching from 1, unless they asked for 0+
+                int idx = minIdx;
+                if (minIdx < 0) idx = 1;
+                while (idx < 1000)
+                {
+                    String name = descriptor.GetFileName(idx);
+                    if (!pkg.ContainPart(PackagingUriHelper.CreatePartName(name)))
+                    {
+                        return idx;
+                    }
+                    idx++;
+                }
+            }
+            catch (InvalidFormatException e)
+            {
+                // Give a general wrapped exception for the problem
+                throw new POIXMLException(e);
+            }
+            return -1;
+        }
+
         /**
          * Create a new child POIXMLDocumentPart
          *

ooxml/XSSF/UserModel/XSSFSheet.cs

@@ -1743,13 +1743,14 @@ namespace NPOI.XSSF.UserModel
                 return GetDrawingPatriarch();
             }
 
-            //drawingNumber = #drawings.Count + 1
-            int DrawingNumber = GetPackagePart()
+            // Default drawingNumber = #drawings.Count + 1
+            int drawingNumber = GetPackagePart()
                 .Package.GetPartsByContentType(XSSFRelation.DRAWINGS.ContentType).Count + 1;
+            drawingNumber = GetNextPartNumber(XSSFRelation.DRAWINGS, drawingNumber);
             RelationPart rp = CreateRelationship(
                 XSSFRelation.DRAWINGS,
                 XSSFFactory.GetInstance(),
-                DrawingNumber,
+                drawingNumber,
                 false);
             XSSFDrawing drawing = rp.DocumentPart as XSSFDrawing;
             string relId = rp.Relationship.Id;

testcases/ooxml/TestPOIXMLDocument.cs

@@ -21,6 +21,8 @@ namespace TestCases.OOXML
     using NPOI.OpenXml4Net.Exceptions;
     using NPOI.OpenXml4Net.OPC;
     using NPOI.Util;
+    using NPOI.XSSF.UserModel;
+    using NPOI.XWPF.UserModel;
     using NUnit.Framework;
     using System;
     using System.Collections.Generic;
@@ -241,7 +243,42 @@ namespace TestCases.OOXML
             }
 
         }
+        [Test]
+        public void TestGetNextPartNumber()
+        {
 
+            POIDataSamples pds = POIDataSamples.GetDocumentInstance();
+            OPCPackage pkg = PackageHelper.Open(pds.OpenResourceAsStream("WordWithAttachments.docx"));
+            OPCParser doc = new OPCParser(pkg);
+            try
+            {
+                doc.Parse(new TestFactory());
+
+                // Non-indexed parts: Word is taken, Excel is not
+                Assert.AreEqual(-1, doc.GetNextPartNumber(XWPFRelation.DOCUMENT, 0));
+                Assert.AreEqual(-1, doc.GetNextPartNumber(XWPFRelation.DOCUMENT, -1));
+                Assert.AreEqual(-1, doc.GetNextPartNumber(XWPFRelation.DOCUMENT, 99));
+                Assert.AreEqual(0, doc.GetNextPartNumber(XSSFRelation.WORKBOOK, 0));
+                Assert.AreEqual(0, doc.GetNextPartNumber(XSSFRelation.WORKBOOK, -1));
+                Assert.AreEqual(0, doc.GetNextPartNumber(XSSFRelation.WORKBOOK, 99));
+
+                // Indexed parts:
+                // Has 2 headers
+                Assert.AreEqual(0, doc.GetNextPartNumber(XWPFRelation.HEADER, 0));
+                Assert.AreEqual(3, doc.GetNextPartNumber(XWPFRelation.HEADER, -1));
+                Assert.AreEqual(3, doc.GetNextPartNumber(XWPFRelation.HEADER, 1));
+                Assert.AreEqual(8, doc.GetNextPartNumber(XWPFRelation.HEADER, 8));
+
+                // Has no Excel Sheets
+                Assert.AreEqual(0, doc.GetNextPartNumber(XSSFRelation.WORKSHEET, 0));
+                Assert.AreEqual(1, doc.GetNextPartNumber(XSSFRelation.WORKSHEET, -1));
+                Assert.AreEqual(1, doc.GetNextPartNumber(XSSFRelation.WORKSHEET, 1));
+            }
+            finally
+            {
+                doc.Close();
+            }
+        }
         [Test]
         public void TestCommitNullPart()
         {

testcases/ooxml/XSSF/UserModel/TestXSSFBugs.cs

@@ -3406,6 +3406,51 @@ namespace TestCases.XSSF.UserModel
             wb.Close();
         }
 
+        /// <summary>
+        /// Other things, including charts, may end up taking Drawing part
+        /// numbers. (Uses a test file hand-crafted with an extra non-drawing
+        /// part with a part number)
+        /// </summary>
+        [Test]
+        public void DrawingNumbersAlreadyTaken_60255()
+        {
+
+            IWorkbook wb = XSSFTestDataSamples.OpenSampleWorkbook("60255_extra_drawingparts.xlsx");
+            Assert.AreEqual(4, wb.NumberOfSheets);
+
+            // Sheet 3 starts with a Drawing
+            ISheet sheet = wb.GetSheetAt(0);
+            Assert.IsNull(sheet.DrawingPatriarch);
+            sheet = wb.GetSheetAt(1);
+            Assert.IsNull(sheet.DrawingPatriarch);
+            sheet = wb.GetSheetAt(2);
+            Assert.IsNotNull(sheet.DrawingPatriarch);
+            sheet = wb.GetSheetAt(3);
+            Assert.IsNull(sheet.DrawingPatriarch);
+
+            // Add another sheet, and give it a Drawing
+            sheet = wb.CreateSheet();
+            Assert.IsNull(sheet.DrawingPatriarch);
+            sheet.CreateDrawingPatriarch();
+            Assert.IsNotNull(sheet.DrawingPatriarch);
+
+            // Save and check
+            wb = XSSFTestDataSamples.WriteOutAndReadBack(wb);
+            Assert.AreEqual(5, wb.NumberOfSheets);
+
+            // Sheets 3 and 5 now
+            sheet = wb.GetSheetAt(0);
+            Assert.IsNull(sheet.DrawingPatriarch);
+            sheet = wb.GetSheetAt(1);
+            Assert.IsNull(sheet.DrawingPatriarch);
+            sheet = wb.GetSheetAt(2);
+            Assert.IsNotNull(sheet.DrawingPatriarch);
+            sheet = wb.GetSheetAt(3);
+            Assert.IsNull(sheet.DrawingPatriarch);
+            sheet = wb.GetSheetAt(4);
+            Assert.IsNotNull(sheet.DrawingPatriarch);
+        }
+
         [Test]
         [Ignore("TODO FIX CI TESTS")]
         public void Bug61063()