From 81ae481815ca76ff3edbc904b4f30e0b5f360c25 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sun, 1 Mar 2015 19:56:56 +0000 Subject: [PATCH] Bug 57165: Avoid PartAlreadyExistsException when removing/cloning sheets git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1663153 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/poi/POIXMLDocumentPart.java | 20 +++++++++++ .../apache/poi/openxml4j/opc/OPCPackage.java | 4 +-- .../apache/poi/openxml4j/opc/PackagePart.java | 5 +++ .../poi/xssf/usermodel/XSSFWorkbook.java | 22 ++++++++++-- .../poi/xssf/usermodel/TestUnfixedBugs.java | 34 ------------------- .../poi/xssf/usermodel/TestXSSFBugs.java | 32 +++++++++++++++++ 6 files changed, 79 insertions(+), 38 deletions(-) diff --git a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java b/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java index 24b4af2389..4759ddbd3b 100644 --- a/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java +++ b/src/ooxml/java/org/apache/poi/POIXMLDocumentPart.java @@ -349,11 +349,27 @@ public class POIXMLDocumentPart { * @param descriptor the part descriptor * @param factory the factory that will create an instance of the requested relation * @return the created child POIXMLDocumentPart + * @throws PartAlreadyExistsException + * If rule M1.12 is not verified : Packages shall not contain + * equivalent part names and package implementers shall neither + * create nor recognize packages with equivalent part names. */ public final POIXMLDocumentPart createRelationship(POIXMLRelation descriptor, POIXMLFactory factory){ return createRelationship(descriptor, factory, -1, false); } + /** + * Create a new child POIXMLDocumentPart + * + * @param descriptor the part descriptor + * @param factory the factory that will create an instance of the requested relation + * @param idx part number + * @return the created child POIXMLDocumentPart + * @throws PartAlreadyExistsException + * If rule M1.12 is not verified : Packages shall not contain + * equivalent part names and package implementers shall neither + * create nor recognize packages with equivalent part names. + */ public final POIXMLDocumentPart createRelationship(POIXMLRelation descriptor, POIXMLFactory factory, int idx){ return createRelationship(descriptor, factory, idx, false); } @@ -366,6 +382,10 @@ public class POIXMLDocumentPart { * @param idx part number * @param noRelation if true, then no relationship is added. * @return the created child POIXMLDocumentPart + * @throws PartAlreadyExistsException + * If rule M1.12 is not verified : Packages shall not contain + * equivalent part names and package implementers shall neither + * create nor recognize packages with equivalent part names. */ protected final POIXMLDocumentPart createRelationship(POIXMLRelation descriptor, POIXMLFactory factory, int idx, boolean noRelation){ try { 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 e210be6f67..4f3d6ca650 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/OPCPackage.java @@ -739,7 +739,7 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { * @param contentType * Part content type. * @return The newly created part. - * @throws InvalidFormatException + * @throws PartAlreadyExistsException * If rule M1.12 is not verified : Packages shall not contain * equivalent part names and package implementers shall neither * create nor recognize packages with equivalent part names. @@ -762,7 +762,7 @@ public abstract class OPCPackage implements RelationshipSource, Closeable { * Specify if the existing relationship part, if any, logically * associated to the newly created part will be loaded. * @return The newly created part. - * @throws InvalidFormatException + * @throws PartAlreadyExistsException * If rule M1.12 is not verified : Packages shall not contain * equivalent part names and package implementers shall neither * create nor recognize packages with equivalent part names. diff --git a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java index 8b72770a77..8cfdb0fb36 100644 --- a/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java +++ b/src/ooxml/java/org/apache/poi/openxml4j/opc/PackagePart.java @@ -228,8 +228,13 @@ public abstract class PackagePart implements RelationshipSource { * Relationship unique id. * @return The newly created and added relationship * + * @throws InvalidOperationException + * If a writing operation is done on a read only package or + * invalid nested relations are created. * @throws InvalidFormatException * If the URI point to a relationship part URI. + * @throws IllegalArgumentException if targetPartName, targetMode + * or relationshipType are passed as null * @see org.apache.poi.openxml4j.opc.RelationshipSource#addRelationship(org.apache.poi.openxml4j.opc.PackagePartName, * org.apache.poi.openxml4j.opc.TargetMode, java.lang.String, java.lang.String) */ diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java index ae09af141d..41a6d613ff 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java @@ -751,8 +751,26 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook, Iterable try next one + sheetNumber++; + continue outerloop; + } + } + + // no duplicate found => use this one + break; } XSSFSheet wrapper = (XSSFSheet)createRelationship(XSSFRelation.WORKSHEET, XSSFFactory.getInstance(), sheetNumber); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java index c90ba80614..0e5864b365 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestUnfixedBugs.java @@ -17,8 +17,6 @@ package org.apache.poi.xssf.usermodel; -import static org.junit.Assert.assertEquals; - import java.io.IOException; import java.nio.charset.Charset; import java.util.Calendar; @@ -184,38 +182,6 @@ public final class TestUnfixedBugs extends TestCase { } } - - @Test - public void test57165() throws IOException { - XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("57171_57163_57165.xlsx"); - try { - removeAllSheetsBut(3, wb); - wb.cloneSheet(0); // Throws exception here - wb.setSheetName(1, "New Sheet"); - //saveWorkbook(wb, fileName); - - XSSFWorkbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(wb); - try { - - } finally { - wbBack.close(); - } - } finally { - wb.close(); - } - } - - private static void removeAllSheetsBut(int sheetIndex, Workbook wb) - { - int sheetNb = wb.getNumberOfSheets(); - // Move this sheet at the first position - wb.setSheetOrder(wb.getSheetName(sheetIndex), 0); - for (int sn = sheetNb - 1; sn > 0; sn--) - { - wb.removeSheetAt(sn); - } - } - // When this is fixed, the test case should go to BaseTestXCell with // adjustments to use _testDataProvider to also verify this for XSSF public void testBug57294() throws IOException { 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 0f8549562d..1ebb56178a 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFBugs.java @@ -2134,4 +2134,36 @@ public final class TestXSSFBugs extends BaseTestBugzillaIssues { assertEquals("~CIRCULAR~REF~", FormulaError.forInt(value.getErrorValue()).getString()); assertEquals("CIRCULAR_REF", FormulaError.forInt(value.getErrorValue()).toString()); } + + + @Test + public void test57165() throws IOException { + XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("57171_57163_57165.xlsx"); + try { + removeAllSheetsBut(3, wb); + wb.cloneSheet(0); // Throws exception here + wb.setSheetName(1, "New Sheet"); + //saveWorkbook(wb, fileName); + + XSSFWorkbook wbBack = XSSFTestDataSamples.writeOutAndReadBack(wb); + try { + + } finally { + wbBack.close(); + } + } finally { + wb.close(); + } + } + + private static void removeAllSheetsBut(int sheetIndex, Workbook wb) + { + int sheetNb = wb.getNumberOfSheets(); + // Move this sheet at the first position + wb.setSheetOrder(wb.getSheetName(sheetIndex), 0); + for (int sn = sheetNb - 1; sn > 0; sn--) + { + wb.removeSheetAt(sn); + } + } } -- 2.39.5