poi: #60255 When creating a XSSF drawing, find the next available doc…

…ument part, even if another type has pinched the next number

ooxml/POIXMLDocumentPart.cs

@@ -573,6 +573,65 @@ public POIXMLDocumentPart CreateRelationship(POIXMLRelation descriptor, POIXMLFa
  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 @@ public IDrawing CreateDrawingPatriarch()
  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 @@ public void TestRelationOrder()
  }
 
  }
+ [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 @@ public void Test55076_collapseColumnGroups()
  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()