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

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1764863 13f79535-47bb-0310-9956-ffa450edef68

src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java

@@ -39,6 +39,7 @@ import org.apache.poi.openxml4j.opc.TargetMode;
 import org.apache.poi.util.Internal;
 import org.apache.poi.util.POILogFactory;
 import org.apache.poi.util.POILogger;
+import org.apache.poi.xssf.usermodel.XSSFRelation;
 
 /**
  * Represents an entry of a OOXML package.
@@ -538,6 +539,55 @@ public class POIXMLDocumentPart {
         return createRelationship(descriptor, factory, idx, false).getDocumentPart();
     }
     
+    /**
+     * 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 final int getNextPartNumber(POIXMLRelation descriptor, int minIdx) {
+        OPCPackage pkg = packagePart.getPackage();
+        
+        try {
+            if (descriptor.getDefaultFileName().equals(descriptor.getFileName(9999))) {
+                // Non-index based, check if default is free
+                PackagePartName ppName = PackagingURIHelper.createPartName(descriptor.getDefaultFileName());
+                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
      *

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java

@@ -554,8 +554,9 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
             return getDrawingPatriarch();
         }
         
-        //drawingNumber = #drawings.size() + 1
+        // Default drawingNumber = #drawings.size() + 1
         int drawingNumber = getPackagePart().getPackage().getPartsByContentType(XSSFRelation.DRAWINGS.getContentType()).size() + 1;
+        drawingNumber = getNextPartNumber(XSSFRelation.DRAWINGS, drawingNumber);
         RelationPart rp = createRelationship(XSSFRelation.DRAWINGS, XSSFFactory.getInstance(), drawingNumber, false);
         XSSFDrawing drawing = rp.getDocumentPart();
         String relId = rp.getRelationship().getId();

src/ooxml/testcases/org/apache/poi/TestPOIXMLDocument.java

@@ -44,6 +44,8 @@ import org.apache.poi.util.PackageHelper;
 import org.apache.poi.util.TempFile;
 import org.apache.poi.xslf.usermodel.XMLSlideShow;
 import org.apache.poi.xslf.usermodel.XSLFShape;
+import org.apache.poi.xssf.usermodel.XSSFRelation;
+import org.apache.poi.xwpf.usermodel.XWPFRelation;
 import org.junit.Test;
 
 /**
@@ -228,6 +230,39 @@ public final class TestPOIXMLDocument {
         }
     }
     
+    @Test
+    public void testGetNextPartNumber() throws Exception {
+        POIDataSamples pds = POIDataSamples.getDocumentInstance();
+        @SuppressWarnings("resource")
+        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
+            assertEquals(-1, doc.getNextPartNumber(XWPFRelation.DOCUMENT, 0));
+            assertEquals(-1, doc.getNextPartNumber(XWPFRelation.DOCUMENT, -1));
+            assertEquals(-1, doc.getNextPartNumber(XWPFRelation.DOCUMENT, 99));
+            assertEquals(0, doc.getNextPartNumber(XSSFRelation.WORKBOOK, 0));
+            assertEquals(0, doc.getNextPartNumber(XSSFRelation.WORKBOOK, -1));
+            assertEquals(0, doc.getNextPartNumber(XSSFRelation.WORKBOOK, 99));
+            
+            // Indexed parts:
+            // Has 2 headers
+            assertEquals(0, doc.getNextPartNumber(XWPFRelation.HEADER, 0));
+            assertEquals(3, doc.getNextPartNumber(XWPFRelation.HEADER, -1));
+            assertEquals(3, doc.getNextPartNumber(XWPFRelation.HEADER, 1));
+            assertEquals(8, doc.getNextPartNumber(XWPFRelation.HEADER, 8));
+            
+            // Has no Excel Sheets
+            assertEquals(0, doc.getNextPartNumber(XSSFRelation.WORKSHEET, 0));
+            assertEquals(1, doc.getNextPartNumber(XSSFRelation.WORKSHEET, -1));
+            assertEquals(1, doc.getNextPartNumber(XSSFRelation.WORKSHEET, 1));
+        } finally {
+            doc.close();
+        }
+    }
+
     @Test
     public void testCommitNullPart() throws IOException, InvalidFormatException {
         POIXMLDocumentPart part = new POIXMLDocumentPart();

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java

@@ -3128,4 +3128,47 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
         
         wb.close();
     }
+    
+    /**
+     * 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)
+     */
+    @Test
+    public void drawingNumbersAlreadyTaken_60255() throws Exception {
+        Workbook wb = XSSFTestDataSamples.openSampleWorkbook("60255_extra_drawingparts.xlsx");
+        assertEquals(4, wb.getNumberOfSheets());
+        
+        // Sheet 3 starts with a drawing
+        Sheet sheet = wb.getSheetAt(0);
+        assertNull(sheet.getDrawingPatriarch());
+        sheet = wb.getSheetAt(1);
+        assertNull(sheet.getDrawingPatriarch());
+        sheet = wb.getSheetAt(2);
+        assertNotNull(sheet.getDrawingPatriarch());
+        sheet = wb.getSheetAt(3);
+        assertNull(sheet.getDrawingPatriarch());
+        
+        // Add another sheet, and give it a drawing
+        sheet = wb.createSheet();
+        assertNull(sheet.getDrawingPatriarch());
+        sheet.createDrawingPatriarch();
+        assertNotNull(sheet.getDrawingPatriarch());
+        
+        // Save and check
+        wb = XSSFTestDataSamples.writeOutAndReadBack(wb);
+        assertEquals(5, wb.getNumberOfSheets());
+        
+        // Sheets 3 and 5 now
+        sheet = wb.getSheetAt(0);
+        assertNull(sheet.getDrawingPatriarch());
+        sheet = wb.getSheetAt(1);
+        assertNull(sheet.getDrawingPatriarch());
+        sheet = wb.getSheetAt(2);
+        assertNotNull(sheet.getDrawingPatriarch());
+        sheet = wb.getSheetAt(3);
+        assertNull(sheet.getDrawingPatriarch());
+        sheet = wb.getSheetAt(4);
+        assertNotNull(sheet.getDrawingPatriarch());
+    }
 }