From 837c5a90f18332228235a9863526710d61da95c1 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Thu, 22 Sep 2011 10:37:54 +0000 Subject: [PATCH] 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 | 1 + .../org/apache/poi/POIXMLDocumentPart.java | 8 ++++- .../apache/poi/xssf/usermodel/XSSFSheet.java | 14 +++++++- .../poi/xssf/usermodel/TestXSSFBugs.java | 36 +++++++++++++++---- 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index ef7ad52d5..72f74129d 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 51850 - support creating comments in XSSF on an earlier slide when later ones already have them 51804 - optionally include Master Slide text in XSLF text extraction, as HSLF already offers New PackagePart method getRelatedPart(PackageRelationship) to simplify navigation of relations between OPC Parts 51832 - handle XLS files where the WRITEPROTECT record preceeds the FILEPASS one, rather than following as normal diff --git a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java b/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java index 880af2765..fe147b622 100644 --- a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java +++ b/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,8 +361,13 @@ 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){ - throw new POIXMLException(e); + // Give a general wrapped exception for the problem + throw new POIXMLException(e); } } diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java index a78162917..e87dd0daa 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java +++ b/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; } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java index 045e043fc..dd62f7d66 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/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); - excelComment.setString( + Comment comment1 = drawing.createCellComment(anchor); + comment1.setString( factory.createRichTextString("I like this cell. It's my favourite.")); - excelComment.setAuthor("Bob T. Fish"); + comment1.setAuthor("Bob T. Fish"); + + Comment comment2 = drawing.createCellComment(anchor); + comment2.setString( + factory.createRichTextString("This is much less fun...")); + comment2.setAuthor("Bob T. Fish"); - Cell c = sh1.getRow(0).getCell(4); - c.setCellComment(excelComment); + 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()); } }