Fix bug #51850 - support creating comments in XSSF on an earlier slide when later ones already have them

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

src/documentation/content/xdocs/status.xml

@@ -34,6 +34,7 @@
 
     <changes>
         <release version="3.8-beta5" date="2011-??-??">
+           <action dev="poi-developers" type="fix">51850 - support creating comments in XSSF on an earlier slide when later ones already have them</action>
            <action dev="poi-developers" type="add">51804 - optionally include Master Slide text in XSLF text extraction, as HSLF already offers</action>
            <action dev="poi-developers" type="add">New PackagePart method getRelatedPart(PackageRelationship) to simplify navigation of relations between OPC Parts</action>
            <action dev="poi-developers" type="fix">51832 - handle XLS files where the WRITEPROTECT record preceeds the FILEPASS one, rather than following as normal</action>

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

@@ -23,6 +23,7 @@ import java.util.Map.Entry;
 
 import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.exceptions.OpenXML4JException;
+import org.apache.poi.openxml4j.exceptions.PartAlreadyExistsException;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 import org.apache.poi.openxml4j.opc.PackagePart;
 import org.apache.poi.openxml4j.opc.PackagePartName;
@@ -360,7 +361,12 @@ public class POIXMLDocumentPart {
                 addRelation(rel.getId(),doc);
             }
             return doc;
+        } catch (PartAlreadyExistsException pae) {
+           // Return the specific exception so the user knows
+           //  that the name is already taken
+           throw pae;
         } catch (Exception e){
+           // Give a general wrapped exception for the problem
            throw new POIXMLException(e);
         }
     }

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

@@ -35,6 +35,7 @@ import org.apache.poi.POIXMLException;
 import org.apache.poi.hssf.record.PasswordRecord;
 import org.apache.poi.hssf.util.PaneInformation;
 import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
+import org.apache.poi.openxml4j.exceptions.PartAlreadyExistsException;
 import org.apache.poi.openxml4j.opc.PackagePart;
 import org.apache.poi.openxml4j.opc.PackageRelationship;
 import org.apache.poi.openxml4j.opc.PackageRelationshipCollection;
@@ -2613,7 +2614,18 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet {
      */
     protected CommentsTable getCommentsTable(boolean create) {
         if(sheetComments == null && create){
-            sheetComments = (CommentsTable)createRelationship(XSSFRelation.SHEET_COMMENTS, XSSFFactory.getInstance(), (int)sheet.getSheetId());
+           // Try to create a comments table with the same number as
+           //  the sheet has (i.e. sheet 1 -> comments 1)
+           try {
+              sheetComments = (CommentsTable)createRelationship(
+                    XSSFRelation.SHEET_COMMENTS, XSSFFactory.getInstance(), (int)sheet.getSheetId());
+           } catch(PartAlreadyExistsException e) {
+              // Technically a sheet doesn't need the same number as
+              //  it's comments, and clearly someone has already pinched
+              //  our number! Go for the next available one instead
+              sheetComments = (CommentsTable)createRelationship(
+                    XSSFRelation.SHEET_COMMENTS, XSSFFactory.getInstance(), -1);
+           }
         }
         return sheetComments;
     }

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

@@ -1177,13 +1177,14 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
      * Add comments to Sheet 1, when Sheet 2 already has
      *  comments (so /xl/comments1.xml is taken)
      */
-    public void DISABLEDtest51850() {
+    public void test51850() {
        XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("51850.xlsx");
        XSSFSheet sh1 = wb.getSheetAt(0);
        XSSFSheet sh2 = wb.getSheetAt(1);
  
        // Sheet 2 has comments
        assertNotNull(sh2.getCommentsTable(false));
+       assertEquals(1, sh2.getCommentsTable(false).getNumberOfComments());
        
        // Sheet 1 doesn't (yet)
        assertNull(sh1.getCommentsTable(false));
@@ -1198,12 +1199,35 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
        anchor.setRow1(0);
        anchor.setRow2(1);
 
-       Comment excelComment = drawing.createCellComment(anchor);
+       Comment comment1 = drawing.createCellComment(anchor);
-       excelComment.setString(
+       comment1.setString(
              factory.createRichTextString("I like this cell. It's my favourite."));
-       excelComment.setAuthor("Bob T. Fish");
+       comment1.setAuthor("Bob T. Fish");
        
-       Cell c = sh1.getRow(0).getCell(4);
+       Comment comment2 = drawing.createCellComment(anchor);
-       c.setCellComment(excelComment);
+       comment2.setString(
+             factory.createRichTextString("This is much less fun..."));
+       comment2.setAuthor("Bob T. Fish");
+
+       Cell c1 = sh1.getRow(0).createCell(4);
+       c1.setCellValue(2.3);
+       c1.setCellComment(comment1);
+       
+       Cell c2 = sh1.getRow(0).createCell(5);
+       c2.setCellValue(2.1);
+       c2.setCellComment(comment2);
+       
+       
+       // Save and re-load
+       wb = XSSFTestDataSamples.writeOutAndReadBack(wb);
+       sh1 = wb.getSheetAt(0);
+       sh2 = wb.getSheetAt(1);
+       
+       // Check the comments
+       assertNotNull(sh2.getCommentsTable(false));
+       assertEquals(1, sh2.getCommentsTable(false).getNumberOfComments());
+       
+       assertNotNull(sh1.getCommentsTable(false));
+       assertEquals(2, sh1.getCommentsTable(false).getNumberOfComments());
     }
 }