Add a disabled test for bug #51850, and tweak the OPC package exceptions to make it easier to spot when a part name is already used

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

6 files changed, 76 insertions, 12 deletions

diff --git a/src/ooxml/java/org/apache/poi/openxml4j/exceptions/InvalidOperationException.java b/src/ooxml/java/org/apache/poi/openxml4j/exceptions/InvalidOperationException.java
index 814edd1179..a2538a6d19 100644
--- a/src/ooxml/java/org/apache/poi/openxml4j/exceptions/InvalidOperationException.java
+++ b/src/ooxml/java/org/apache/poi/openxml4j/exceptions/InvalidOperationException.java
@@ -19,13 +19,9 @@ package org.apache.poi.openxml4j.exceptions;
 
 /**
  * Throw when an invalid operation is done.
- *
- * @author Julien Chable
- * @version 1.0
  */
 @SuppressWarnings("serial")
-public final class InvalidOperationException extends OpenXML4JRuntimeException{
-
+public class InvalidOperationException extends OpenXML4JRuntimeException{
 	public InvalidOperationException(String message){
 		super(message);
 	}
diff --git a/src/ooxml/java/org/apache/poi/openxml4j/exceptions/PartAlreadyExistsException.java b/src/ooxml/java/org/apache/poi/openxml4j/exceptions/PartAlreadyExistsException.java
new file mode 100644
index 0000000000..e469ace568
--- /dev/null
+++ b/src/ooxml/java/org/apache/poi/openxml4j/exceptions/PartAlreadyExistsException.java
@@ -0,0 +1,31 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.openxml4j.exceptions;
+
+import org.apache.poi.openxml4j.opc.PackagePart;
+
+/**
+ * Throw when trying to create a {@link PackagePart} but one
+ *  already exists with that name.
+ */
+@SuppressWarnings("serial")
+public final class PartAlreadyExistsException extends InvalidOperationException {
+	public PartAlreadyExistsException(String message){
+		super(message);
+	}
+}
diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
index 6ec685cb4e..541700a136 100644
--- a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
+++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java
@@ -40,6 +40,7 @@ import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.exceptions.InvalidOperationException;
 import org.apache.poi.openxml4j.exceptions.OpenXML4JException;
 import org.apache.poi.openxml4j.exceptions.OpenXML4JRuntimeException;
+import org.apache.poi.openxml4j.exceptions.PartAlreadyExistsException;
 import org.apache.poi.openxml4j.opc.internal.ContentType;
 import org.apache.poi.openxml4j.opc.internal.ContentTypeManager;
 import org.apache.poi.openxml4j.opc.internal.PackagePropertiesPart;
@@ -716,7 +717,7 @@ public abstract class OPCPackage implements RelationshipSource, Closeable {
 		// Check if the specified part name already exists
 		if (partList.containsKey(partName)
 				&& !partList.get(partName).isDeleted()) {
-			throw new InvalidOperationException(
+			throw new PartAlreadyExistsException(
 					"A part with the name '"
 							+ partName.getName()
 							+ "' already exists : Packages shall not contain equivalent part names and package implementers shall neither create nor recognize packages with equivalent part names. [M1.12]");
diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFDrawing.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFDrawing.java
index 446e0ecea7..803fdf0142 100644
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFDrawing.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFDrawing.java
@@ -37,15 +37,14 @@ import org.apache.poi.util.Internal;
 import org.apache.poi.xssf.model.CommentsTable;
 import org.apache.xmlbeans.XmlException;
 import org.apache.xmlbeans.XmlOptions;
-import org.openxmlformats.schemas.drawingml.x2006.main.CTNonVisualDrawingProps;
 import org.openxmlformats.schemas.drawingml.x2006.spreadsheetDrawing.CTConnector;
 import org.openxmlformats.schemas.drawingml.x2006.spreadsheetDrawing.CTDrawing;
+import org.openxmlformats.schemas.drawingml.x2006.spreadsheetDrawing.CTGraphicalObjectFrame;
 import org.openxmlformats.schemas.drawingml.x2006.spreadsheetDrawing.CTGroupShape;
 import org.openxmlformats.schemas.drawingml.x2006.spreadsheetDrawing.CTPicture;
 import org.openxmlformats.schemas.drawingml.x2006.spreadsheetDrawing.CTShape;
 import org.openxmlformats.schemas.drawingml.x2006.spreadsheetDrawing.CTTwoCellAnchor;
 import org.openxmlformats.schemas.drawingml.x2006.spreadsheetDrawing.STEditAs;
-import org.openxmlformats.schemas.drawingml.x2006.spreadsheetDrawing.CTGraphicalObjectFrame;
 import org.openxmlformats.schemas.officeDocument.x2006.relationships.STRelationshipId;
 
 /**
diff --git a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/compliance/TestOPCCompliancePackageModel.java b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/compliance/TestOPCCompliancePackageModel.java
index e3c54f367e..a39edd3a62 100644
--- a/src/ooxml/testcases/org/apache/poi/openxml4j/opc/compliance/TestOPCCompliancePackageModel.java
+++ b/src/ooxml/testcases/org/apache/poi/openxml4j/opc/compliance/TestOPCCompliancePackageModel.java
@@ -17,20 +17,20 @@
 
 package org.apache.poi.openxml4j.opc.compliance;
 
-import java.io.File;
 import java.io.IOException;
 
 import junit.framework.TestCase;
 
+import org.apache.poi.POIDataSamples;
 import org.apache.poi.openxml4j.exceptions.InvalidFormatException;
 import org.apache.poi.openxml4j.exceptions.InvalidOperationException;
+import org.apache.poi.openxml4j.exceptions.PartAlreadyExistsException;
 import org.apache.poi.openxml4j.opc.ContentTypes;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 import org.apache.poi.openxml4j.opc.PackagePartName;
 import org.apache.poi.openxml4j.opc.PackageRelationshipTypes;
 import org.apache.poi.openxml4j.opc.PackagingURIHelper;
 import org.apache.poi.openxml4j.opc.TargetMode;
-import org.apache.poi.POIDataSamples;
 
 /**
  * Test Open Packaging Convention package model compliance.
@@ -102,7 +102,7 @@ public class TestOPCCompliancePackageModel extends TestCase {
 		pkg.createPart(name1, ContentTypes.XML);
 		try {
 			pkg.createPart(name2, ContentTypes.XML);
-		} catch (InvalidOperationException e) {
+		} catch (PartAlreadyExistsException e) {
 			return;
 		}
 		fail("Packages shall not contain equivalent part names and package implementers shall neither create nor recognize packages with equivalent part names. [M1.12]");
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 364e4120d6..045e043fc6 100644
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java
@@ -23,7 +23,6 @@ import java.util.List;
 
 import org.apache.poi.POIXMLDocumentPart;
 import org.apache.poi.hssf.usermodel.HSSFWorkbook;
-import org.apache.poi.hssf.util.PaneInformation;
 import org.apache.poi.openxml4j.opc.OPCPackage;
 import org.apache.poi.openxml4j.opc.PackagePart;
 import org.apache.poi.openxml4j.opc.PackagingURIHelper;
@@ -31,7 +30,11 @@ import org.apache.poi.ss.usermodel.BaseTestBugzillaIssues;
 import org.apache.poi.ss.usermodel.Cell;
 import org.apache.poi.ss.usermodel.CellStyle;
 import org.apache.poi.ss.usermodel.CellValue;
+import org.apache.poi.ss.usermodel.ClientAnchor;
+import org.apache.poi.ss.usermodel.Comment;
+import org.apache.poi.ss.usermodel.CreationHelper;
 import org.apache.poi.ss.usermodel.DataFormatter;
+import org.apache.poi.ss.usermodel.Drawing;
 import org.apache.poi.ss.usermodel.Font;
 import org.apache.poi.ss.usermodel.FormulaError;
 import org.apache.poi.ss.usermodel.FormulaEvaluator;
@@ -1169,4 +1172,38 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues {
 
         assertEquals(rels0.get(0).getPackageRelationship(), rels1.get(0).getPackageRelationship());
     }
+    
+    /**
+     * Add comments to Sheet 1, when Sheet 2 already has
+     *  comments (so /xl/comments1.xml is taken)
+     */
+    public void DISABLEDtest51850() {
+       XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("51850.xlsx");
+       XSSFSheet sh1 = wb.getSheetAt(0);
+       XSSFSheet sh2 = wb.getSheetAt(1);
+ 
+       // Sheet 2 has comments
+       assertNotNull(sh2.getCommentsTable(false));
+       
+       // Sheet 1 doesn't (yet)
+       assertNull(sh1.getCommentsTable(false));
+       
+       // Try to add comments to Sheet 1
+       CreationHelper factory = wb.getCreationHelper();
+       Drawing drawing = sh1.createDrawingPatriarch();
+
+       ClientAnchor anchor = factory.createClientAnchor();
+       anchor.setCol1(0);
+       anchor.setCol2(4);
+       anchor.setRow1(0);
+       anchor.setRow2(1);
+
+       Comment excelComment = drawing.createCellComment(anchor);
+       excelComment.setString(
+             factory.createRichTextString("I like this cell. It's my favourite."));
+       excelComment.setAuthor("Bob T. Fish");
+
+       Cell c = sh1.getRow(0).getCell(4);
+       c.setCellComment(excelComment);
+    }
 }