From d0513f2ca676f2a05adfccd4644368d382efe619 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Thu, 31 Mar 2016 13:32:05 +0000 Subject: [PATCH] Apply patch from bug 58909 - Add a cloneSheet() which directly sets the sheetname to allow to avoid a costly renaming of sheets. Combine related unit-tests into Base-Test-Classes to run them for all types of Workbook/Sheet/... git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1737237 13f79535-47bb-0310-9956-ffa450edef68 --- .../poi/hssf/usermodel/HSSFWorkbook.java | 63 +++----- .../poi/xssf/usermodel/XSSFWorkbook.java | 38 ++++- .../org/apache/poi/xssf/AllXSSFTests.java | 3 +- .../apache/poi/xssf/TestXSSFCloneSheet.java | 62 +++++++ .../poi/xssf/streaming/TestSXSSFWorkbook.java | 15 ++ .../poi/hssf/usermodel/TestCloneSheet.java | 60 ++----- .../poi/hssf/usermodel/TestHSSFSheet.java | 2 +- .../poi/hssf/usermodel/TestHSSFWorkbook.java | 152 +++++------------- .../ss/formula/BaseTestExternalFunctions.java | 2 +- .../poi/ss/usermodel/BaseTestCloneSheet.java | 100 ++++++++++++ .../poi/ss/usermodel/BaseTestWorkbook.java | 68 +++++++- 11 files changed, 364 insertions(+), 201 deletions(-) create mode 100644 src/ooxml/testcases/org/apache/poi/xssf/TestXSSFCloneSheet.java create mode 100644 src/testcases/org/apache/poi/ss/usermodel/BaseTestCloneSheet.java diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index 20f6c37c5..9c080cac1 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java @@ -246,8 +246,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss public static String getWorkbookDirEntryName(DirectoryNode directory) { - for (int i = 0; i < WORKBOOK_DIR_ENTRY_NAMES.length; i++) { - String wbName = WORKBOOK_DIR_ENTRY_NAMES[i]; + for (String wbName : WORKBOOK_DIR_ENTRY_NAMES) { try { directory.getEntry(wbName); return wbName; @@ -547,14 +546,14 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss } public void setSelectedTabs(int[] indexes) { - for (int i = 0; i < indexes.length; i++) { - validateSheetIndex(indexes[i]); + for (int index : indexes) { + validateSheetIndex(index); } int nSheets = _sheets.size(); for (int i=0; inot hide, select or focus sheets. + * It just sets the scroll position in the tab-bar. + * + * @param index the sheet index of the tab that will become the first in the tab-bar */ @Override public void setFirstVisibleTab(int index) { @@ -651,7 +652,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss } if (workbook.doesContainsSheetName(name, sheetIx)) { - throw new IllegalArgumentException("The workbook already contains a sheet with this name"); + throw new IllegalArgumentException("The workbook already contains a sheet named '" + name + "'"); } validateSheetIndex(sheetIx); workbook.setSheetName(sheetIx, name); @@ -896,7 +897,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss } if (workbook.doesContainsSheetName( sheetname, _sheets.size() )) - throw new IllegalArgumentException( "The workbook already contains a sheet of this name" ); + throw new IllegalArgumentException("The workbook already contains a sheet named '" + sheetname + "'"); HSSFSheet sheet = new HSSFSheet(this); @@ -907,7 +908,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss sheet.setActive(isOnlySheet); return sheet; } - + /** * Returns an iterator of the sheets in the workbook * in sheet order. Includes hidden and very hidden sheets. @@ -1099,8 +1100,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss { BackupRecord backupRecord = workbook.getBackupRecord(); - return (backupRecord.getBackup() == 0) ? false - : true; + return backupRecord.getBackup() != 0; } int findExistingBuiltinNameRecordIdx(int sheetIndex, byte builtinCode) { @@ -1253,9 +1253,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss } ExtendedFormatRecord xfr = workbook.createCellXF(); short index = (short) (getNumCellStyles() - 1); - HSSFCellStyle style = new HSSFCellStyle(index, xfr, this); - - return style; + return new HSSFCellStyle(index, xfr, this); } /** @@ -1277,9 +1275,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss public HSSFCellStyle getCellStyleAt(int idx) { ExtendedFormatRecord xfr = workbook.getExFormatAt(idx); - HSSFCellStyle style = new HSSFCellStyle((short)idx, xfr, this); - - return style; + return new HSSFCellStyle((short)idx, xfr, this); } /** @@ -1369,9 +1365,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss } public int serialize(int offset, byte[] data) { int result = 0; - int nRecs = _list.size(); - for(int i=0; i escherRecords = r.getEscherRecords(); PrintWriter w = new PrintWriter(new OutputStreamWriter(System.out, Charset.defaultCharset())); - for ( Iterator iterator = escherRecords.iterator(); iterator.hasNext(); ) - { - EscherRecord escherRecord = iterator.next(); + for (EscherRecord escherRecord : escherRecords) { if (fat) System.out.println(escherRecord.toString()); else @@ -1836,12 +1825,8 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss { // The drawing group record always exists at the top level, so we won't need to do this recursively. List pictures = new ArrayList(); - Iterator recordIter = workbook.getRecords().iterator(); - while (recordIter.hasNext()) - { - Record r = recordIter.next(); - if (r instanceof AbstractEscherHolderRecord) - { + for (Record r : workbook.getRecords()) { + if (r instanceof AbstractEscherHolderRecord) { ((AbstractEscherHolderRecord) r).decode(); List escherRecords = ((AbstractEscherHolderRecord) r).getEscherRecords(); searchForPictures(escherRecords, pictures); 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 8bd34905b..078cfa22d 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFWorkbook.java @@ -534,19 +534,40 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { * Create an XSSFSheet from an existing sheet in the XSSFWorkbook. * The cloned sheet is a deep copy of the original. * + * @param sheetNum The index of the sheet to clone * @return XSSFSheet representing the cloned sheet. * @throws IllegalArgumentException if the sheet index in invalid * @throws POIXMLException if there were errors when cloning */ @Override public XSSFSheet cloneSheet(int sheetNum) { + return cloneSheet(sheetNum, null); + } + + /** + * Create an XSSFSheet from an existing sheet in the XSSFWorkbook. + * The cloned sheet is a deep copy of the original but with a new given + * name. + * + * @param sheetNum The index of the sheet to clone + * @param newName The name to set for the newly created sheet + * @return XSSFSheet representing the cloned sheet. + * @throws IllegalArgumentException if the sheet index or the sheet + * name is invalid + * @throws POIXMLException if there were errors when cloning + */ + public XSSFSheet cloneSheet(int sheetNum, String newName) { validateSheetIndex(sheetNum); - XSSFSheet srcSheet = sheets.get(sheetNum); - String srcName = srcSheet.getSheetName(); - String clonedName = getUniqueSheetName(srcName); - XSSFSheet clonedSheet = createSheet(clonedName); + if (newName == null) { + String srcName = srcSheet.getSheetName(); + newName = getUniqueSheetName(srcName); + } else { + validateSheetName(newName); + } + + XSSFSheet clonedSheet = createSheet(newName); // copy sheet's relations List rels = srcSheet.getRelationParts(); @@ -785,8 +806,7 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { throw new IllegalArgumentException("sheetName must not be null"); } - if (containsSheet( sheetname, sheets.size() )) - throw new IllegalArgumentException( "The workbook already contains a sheet of this name"); + validateSheetName(sheetname); // YK: Mimic Excel and silently truncate sheet names longer than 31 characters if(sheetname.length() > 31) sheetname = sheetname.substring(0, 31); @@ -827,6 +847,12 @@ public class XSSFWorkbook extends POIXMLDocument implements Workbook { return wrapper; } + private void validateSheetName(final String sheetName) throws IllegalArgumentException { + if (containsSheet( sheetName, sheets.size() )) { + throw new IllegalArgumentException("The workbook already contains a sheet named '" + sheetName + "'"); + } + } + protected XSSFDialogsheet createDialogsheet(String sheetname, CTDialogsheet dialogsheet) { XSSFSheet sheet = createSheet(sheetname); String sheetRelId = getRelationId(sheet); diff --git a/src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java b/src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java index 38d840174..c6e2f7eca 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java @@ -45,7 +45,8 @@ import org.junit.runners.Suite; //TestCellReference.class, //converted to junit4 TestCTColComparator.class, TestNumericRanges.class, - TestCellFormatPart.class + TestCellFormatPart.class, + TestXSSFCloneSheet.class }) public final class AllXSSFTests { } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/TestXSSFCloneSheet.java b/src/ooxml/testcases/org/apache/poi/xssf/TestXSSFCloneSheet.java new file mode 100644 index 000000000..91883a3df --- /dev/null +++ b/src/ooxml/testcases/org/apache/poi/xssf/TestXSSFCloneSheet.java @@ -0,0 +1,62 @@ +/* ==================================================================== + 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.xssf; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import org.apache.poi.hssf.HSSFITestDataProvider; +import org.apache.poi.ss.usermodel.BaseTestCloneSheet; +import org.apache.poi.xssf.usermodel.XSSFSheet; +import org.apache.poi.xssf.usermodel.XSSFWorkbook; +import org.junit.Before; +import org.junit.Test; + +public class TestXSSFCloneSheet extends BaseTestCloneSheet { + public TestXSSFCloneSheet() { + super(HSSFITestDataProvider.instance); + } + + private static final String OTHER_SHEET_NAME = "Another"; + private static final String VALID_SHEET_NAME = "Sheet01"; + private XSSFWorkbook wb; + + @Before + public void setUp() { + wb = new XSSFWorkbook(); + wb.createSheet(VALID_SHEET_NAME); + } + + @Test + public void testCloneSheetIntStringValidName() { + XSSFSheet cloned = wb.cloneSheet(0, OTHER_SHEET_NAME); + assertEquals(OTHER_SHEET_NAME, cloned.getSheetName()); + assertEquals(2, wb.getNumberOfSheets()); + } + + @Test + public void testCloneSheetIntStringInvalidName() { + try { + wb.cloneSheet(0, VALID_SHEET_NAME); + fail("Should fail"); + } catch (IllegalArgumentException e) { + // expected here + } + assertEquals(1, wb.getNumberOfSheets()); + } +} diff --git a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java index e9611891d..f8816a406 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java @@ -32,6 +32,7 @@ import java.io.IOException; import org.apache.poi.POIDataSamples; import org.apache.poi.POITestCase; +import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.apache.poi.openxml4j.opc.OPCPackage; import org.apache.poi.ss.SpreadsheetVersion; @@ -77,6 +78,20 @@ public final class TestSXSSFWorkbook extends BaseTestWorkbook { } } + /** + * cloning of sheets is not supported in SXSSF + */ + @Override + @Test + public void sheetClone() throws IOException { + try { + super.sheetClone(); + fail("expected exception"); + } catch (RuntimeException e){ + assertEquals("NotImplemented", e.getMessage()); + } + } + /** * Skip this test, as SXSSF doesn't update formulas on sheet name * changes. diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java b/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java index 3bcd66bd4..5bdfe6227 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java @@ -17,18 +17,18 @@ package org.apache.poi.hssf.usermodel; -import static org.junit.Assert.assertArrayEquals; +import org.apache.poi.ddf.EscherDgRecord; +import org.apache.poi.ddf.EscherSpRecord; +import org.apache.poi.hssf.HSSFITestDataProvider; +import org.apache.poi.hssf.record.CommonObjectDataSubRecord; +import org.apache.poi.hssf.record.EscherAggregate; +import org.apache.poi.ss.usermodel.BaseTestCloneSheet; +import org.junit.Test; import java.io.IOException; import java.util.Arrays; -import junit.framework.TestCase; - -import org.apache.poi.ddf.EscherDgRecord; -import org.apache.poi.ddf.EscherSpRecord; -import org.apache.poi.hssf.record.CommonObjectDataSubRecord; -import org.apache.poi.hssf.record.EscherAggregate; -import org.apache.poi.ss.util.CellRangeAddress; +import static org.junit.Assert.*; /** * Test the ability to clone a sheet. @@ -36,40 +36,12 @@ import org.apache.poi.ss.util.CellRangeAddress; * add that record to the sheet in the testCloneSheetBasic method. * @author avik */ -public final class TestCloneSheet extends TestCase { +public final class TestCloneSheet extends BaseTestCloneSheet { + public TestCloneSheet() { + super(HSSFITestDataProvider.instance); + } - public void testCloneSheetBasic() throws IOException{ - HSSFWorkbook b = new HSSFWorkbook(); - HSSFSheet s = b.createSheet("Test"); - s.addMergedRegion(new CellRangeAddress(0, 1, 0, 1)); - HSSFSheet clonedSheet = b.cloneSheet(0); - - assertEquals("One merged area", 1, clonedSheet.getNumMergedRegions()); - - b.close(); - } - - /** - * Ensures that pagebreak cloning works properly - * @throws IOException - */ - public void testPageBreakClones() throws IOException { - HSSFWorkbook b = new HSSFWorkbook(); - HSSFSheet s = b.createSheet("Test"); - s.setRowBreak(3); - s.setColumnBreak((short) 6); - - HSSFSheet clone = b.cloneSheet(0); - assertTrue("Row 3 not broken", clone.isRowBroken(3)); - assertTrue("Column 6 not broken", clone.isColumnBroken((short) 6)); - - s.removeRowBreak(3); - - assertTrue("Row 3 still should be broken", clone.isRowBroken(3)); - - b.close(); - } - + @Test public void testCloneSheetWithoutDrawings(){ HSSFWorkbook b = new HSSFWorkbook(); HSSFSheet s = b.createSheet("Test"); @@ -79,7 +51,8 @@ public final class TestCloneSheet extends TestCase { assertNull(s2.getDrawingPatriarch()); assertEquals(HSSFTestHelper.getSheetForTest(s).getRecords().size(), HSSFTestHelper.getSheetForTest(s2).getRecords().size()); } - + + @Test public void testCloneSheetWithEmptyDrawingAggregate(){ HSSFWorkbook b = new HSSFWorkbook(); HSSFSheet s = b.createSheet("Test"); @@ -114,7 +87,8 @@ public final class TestCloneSheet extends TestCase { assertEquals(agg1.toXml(""), agg2.toXml("")); assertArrayEquals(agg1.serialize(), agg2.serialize()); } - + + @Test public void testCloneComment() throws IOException { HSSFWorkbook wb = new HSSFWorkbook(); HSSFSheet sh = wb.createSheet(); diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java index 56fdc2ef2..ed43b4264 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java @@ -964,7 +964,7 @@ public final class TestHSSFSheet extends BaseTestSheet { wb.createSheet(SAME_PREFIX + "Dyyyy"); // identical up to the 32nd char fail("Expected exception not thrown"); } catch (IllegalArgumentException e) { - assertEquals("The workbook already contains a sheet of this name", e.getMessage()); + assertEquals("The workbook already contains a sheet named 'A123456789B123456789C123456789Dyyyy'", e.getMessage()); } wb.createSheet(SAME_PREFIX + "Exxxx"); // OK - differs in the 31st char wb.close(); diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java index a5f48eec2..167e41adf 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java +++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java @@ -48,7 +48,6 @@ import org.apache.poi.hssf.record.CFRuleRecord; import org.apache.poi.hssf.record.NameRecord; import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.RecordBase; -import org.apache.poi.hssf.record.RecordFormatException; import org.apache.poi.hssf.record.WindowOneRecord; import org.apache.poi.poifs.filesystem.DirectoryEntry; import org.apache.poi.poifs.filesystem.DirectoryNode; @@ -56,13 +55,10 @@ import org.apache.poi.poifs.filesystem.NPOIFSFileSystem; import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.ss.SpreadsheetVersion; import org.apache.poi.ss.formula.ptg.Area3DPtg; -import org.apache.poi.ss.usermodel.BaseTestWorkbook; -import org.apache.poi.ss.usermodel.Name; -import org.apache.poi.ss.usermodel.Row; -import org.apache.poi.ss.usermodel.Sheet; -import org.apache.poi.ss.usermodel.Workbook; +import org.apache.poi.ss.usermodel.*; import org.apache.poi.ss.util.CellRangeAddress; import org.apache.poi.util.LittleEndian; +import org.apache.poi.util.RecordFormatException; import org.apache.poi.util.TempFile; import org.junit.Test; @@ -87,7 +83,7 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { /** * Tests for {@link HSSFWorkbook#isHidden()} etc - * @throws IOException + * @throws IOException */ @Test public void hidden() throws IOException { @@ -112,36 +108,14 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { wbBack.setHidden(false); assertEquals(false, wbBack.isHidden()); assertEquals(false, w1.getHidden()); - + wbBack.close(); wb.close(); } - @Test - public void sheetClone() throws IOException { - // First up, try a simple file - HSSFWorkbook b = new HSSFWorkbook(); - assertEquals(0, b.getNumberOfSheets()); - b.createSheet("Sheet One"); - b.createSheet("Sheet Two"); - - assertEquals(2, b.getNumberOfSheets()); - b.cloneSheet(0); - assertEquals(3, b.getNumberOfSheets()); - - // Now try a problem one with drawing records in it - HSSFWorkbook bBack = HSSFTestDataSamples.openSampleWorkbook("SheetWithDrawing.xls"); - assertEquals(1, bBack.getNumberOfSheets()); - bBack.cloneSheet(0); - assertEquals(2, bBack.getNumberOfSheets()); - - bBack.close(); - b.close(); - } - @Test public void readWriteWithCharts() throws IOException { - HSSFSheet s; + Sheet s; // Single chart, two sheets HSSFWorkbook b1 = HSSFTestDataSamples.openSampleWorkbook("44010-SingleChart.xls"); @@ -196,7 +170,7 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { // So, start again HSSFWorkbook b5 = HSSFTestDataSamples.openSampleWorkbook("44010-TwoCharts.xls"); - HSSFWorkbook b6 = HSSFTestDataSamples.writeOutAndReadBack(b5); + Workbook b6 = HSSFTestDataSamples.writeOutAndReadBack(b5); b5.close(); assertEquals(3, b6.getNumberOfSheets()); @@ -281,7 +255,7 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { assertEquals(false, sheet1.isActive()); assertEquals(true, sheet3.isActive()); - if (false) { // helpful if viewing this workbook in excel: + /*{ // helpful if viewing this workbook in excel: sheet1.createRow(0).createCell(0).setCellValue(new HSSFRichTextString("Sheet1")); sheet2.createRow(0).createCell(0).setCellValue(new HSSFRichTextString("Sheet2")); sheet3.createRow(0).createCell(0).setCellValue(new HSSFRichTextString("Sheet3")); @@ -295,7 +269,7 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { } catch (IOException e) { throw new RuntimeException(e); } - } + }*/ wb.close(); } @@ -674,18 +648,17 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { HSSFWorkbook hw = new HSSFWorkbook(root, true); List objects = hw.getAllEmbeddedObjects(); boolean found = false; - for (int i = 0; i < objects.size(); i++) { - HSSFObjectData embeddedObject = objects.get(i); - if (embeddedObject.hasDirectoryEntry()) { - DirectoryEntry dir = embeddedObject.getDirectory(); - if (dir instanceof DirectoryNode) { - DirectoryNode dNode = (DirectoryNode)dir; - if (hasEntry(dNode,"WordDocument")) { - found = true; + for (HSSFObjectData embeddedObject : objects) { + if (embeddedObject.hasDirectoryEntry()) { + DirectoryEntry dir = embeddedObject.getDirectory(); + if (dir instanceof DirectoryNode) { + DirectoryNode dNode = (DirectoryNode) dir; + if (hasEntry(dNode, "WordDocument")) { + found = true; + } } - } - } - } + } + } assertTrue(found); hw.close(); @@ -731,7 +704,7 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { @Test public void cellStylesLimit() throws IOException { - HSSFWorkbook wb = new HSSFWorkbook(); + Workbook wb = new HSSFWorkbook(); int numBuiltInStyles = wb.getNumCellStyles(); int MAX_STYLES = 4030; int limit = MAX_STYLES - numBuiltInStyles; @@ -754,38 +727,38 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { @Test public void setSheetOrderHSSF() throws IOException{ - HSSFWorkbook wb = new HSSFWorkbook(); - HSSFSheet s1 = wb.createSheet("first sheet"); - HSSFSheet s2 = wb.createSheet("other sheet"); + Workbook wb = new HSSFWorkbook(); + Sheet s1 = wb.createSheet("first sheet"); + Sheet s2 = wb.createSheet("other sheet"); - HSSFName name1 = wb.createName(); + Name name1 = wb.createName(); name1.setNameName("name1"); name1.setRefersToFormula("'first sheet'!D1"); - HSSFName name2 = wb.createName(); + Name name2 = wb.createName(); name2.setNameName("name2"); name2.setRefersToFormula("'other sheet'!C1"); - HSSFRow s1r1 = s1.createRow(2); - HSSFCell c1 = s1r1.createCell(3); + Row s1r1 = s1.createRow(2); + Cell c1 = s1r1.createCell(3); c1.setCellValue(30); - HSSFCell c2 = s1r1.createCell(2); + Cell c2 = s1r1.createCell(2); c2.setCellFormula("SUM('other sheet'!C1,'first sheet'!C1)"); - HSSFRow s2r1 = s2.createRow(0); - HSSFCell c3 = s2r1.createCell(1); + Row s2r1 = s2.createRow(0); + Cell c3 = s2r1.createCell(1); c3.setCellFormula("'first sheet'!D3"); - HSSFCell c4 = s2r1.createCell(2); + Cell c4 = s2r1.createCell(2); c4.setCellFormula("'other sheet'!D3"); // conditional formatting - HSSFSheetConditionalFormatting sheetCF = s1.getSheetConditionalFormatting(); + SheetConditionalFormatting sheetCF = s1.getSheetConditionalFormatting(); - HSSFConditionalFormattingRule rule1 = sheetCF.createConditionalFormattingRule( + ConditionalFormattingRule rule1 = sheetCF.createConditionalFormattingRule( CFRuleRecord.ComparisonOperator.BETWEEN, "'first sheet'!D1", "'other sheet'!D1"); - HSSFConditionalFormattingRule [] cfRules = { rule1 }; + ConditionalFormattingRule [] cfRules = { rule1 }; CellRangeAddress[] regions = { new CellRangeAddress(2, 4, 0, 0), // A3:A5 @@ -804,10 +777,10 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { assertEquals("'other sheet'!D3", c4.getCellFormula()); // conditional formatting - HSSFConditionalFormatting cf = sheetCF.getConditionalFormattingAt(0); + ConditionalFormatting cf = sheetCF.getConditionalFormattingAt(0); assertEquals("'first sheet'!D1", cf.getRule(0).getFormula1()); assertEquals("'other sheet'!D1", cf.getRule(0).getFormula2()); - + wb.close(); } @@ -901,47 +874,6 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { wb.close(); } - @Test - public void addSheetTwice() throws IOException { - HSSFWorkbook wb=new HSSFWorkbook(); - HSSFSheet sheet1 = wb.createSheet("Sheet1"); - assertNotNull(sheet1); - try { - wb.createSheet("Sheet1"); - fail("Should fail if we add the same sheet twice"); - } catch (IllegalArgumentException e) { - assertTrue(e.getMessage(), e.getMessage().contains("already contains a sheet of this name")); - } - - wb.close(); - } - - @Test - public void getSheetIndex() throws IOException { - HSSFWorkbook wb=new HSSFWorkbook(); - HSSFSheet sheet1 = wb.createSheet("Sheet1"); - HSSFSheet sheet2 = wb.createSheet("Sheet2"); - HSSFSheet sheet3 = wb.createSheet("Sheet3"); - HSSFSheet sheet4 = wb.createSheet("Sheet4"); - - assertEquals(0, wb.getSheetIndex(sheet1)); - assertEquals(1, wb.getSheetIndex(sheet2)); - assertEquals(2, wb.getSheetIndex(sheet3)); - assertEquals(3, wb.getSheetIndex(sheet4)); - - // remove sheets - wb.removeSheetAt(0); - wb.removeSheetAt(2); - - // ensure that sheets are moved up and removed sheets are not found any more - assertEquals(-1, wb.getSheetIndex(sheet1)); - assertEquals(0, wb.getSheetIndex(sheet2)); - assertEquals(1, wb.getSheetIndex(sheet3)); - assertEquals(-1, wb.getSheetIndex(sheet4)); - - wb.close(); - } - @SuppressWarnings("deprecation") @Test public void getExternSheetIndex() throws IOException { @@ -1143,24 +1075,26 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { n.setRefersToFormula(sheetName + "!A1"); assertSheetOrder(wb, "Sheet1", "Sheet2", "Sheet3", "ASheet"); - assertEquals("ASheet!A1", wb.getName(nameName).getRefersToFormula()); + final HSSFName name = wb.getName(nameName); + assertNotNull(name); + assertEquals("ASheet!A1", name.getRefersToFormula()); ByteArrayOutputStream stream = new ByteArrayOutputStream(); wb.write(stream); assertSheetOrder(wb, "Sheet1", "Sheet2", "Sheet3", "ASheet"); - assertEquals("ASheet!A1", wb.getName(nameName).getRefersToFormula()); + assertEquals("ASheet!A1", name.getRefersToFormula()); wb.removeSheetAt(1); assertSheetOrder(wb, "Sheet1", "Sheet3", "ASheet"); - assertEquals("ASheet!A1", wb.getName(nameName).getRefersToFormula()); + assertEquals("ASheet!A1", name.getRefersToFormula()); ByteArrayOutputStream stream2 = new ByteArrayOutputStream(); wb.write(stream2); assertSheetOrder(wb, "Sheet1", "Sheet3", "ASheet"); - assertEquals("ASheet!A1", wb.getName(nameName).getRefersToFormula()); + assertEquals("ASheet!A1", name.getRefersToFormula()); HSSFWorkbook wb2 = new HSSFWorkbook(new ByteArrayInputStream(stream.toByteArray())); expectName(wb2, nameName, "ASheet!A1"); @@ -1172,7 +1106,9 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook { } private void expectName(HSSFWorkbook wb, String name, String expect) { - assertEquals(expect, wb.getName(name).getRefersToFormula()); + final HSSFName hssfName = wb.getName(name); + assertNotNull(hssfName); + assertEquals(expect, hssfName.getRefersToFormula()); } @Test diff --git a/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java b/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java index 321968d09..0043187c0 100644 --- a/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java +++ b/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java @@ -38,7 +38,7 @@ import org.junit.Test; /** * Test setting / evaluating of Analysis Toolpack and user-defined functions */ -public class BaseTestExternalFunctions { +public abstract class BaseTestExternalFunctions { // define two custom user-defined functions private static class MyFunc implements FreeRefFunction { public MyFunc() { diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestCloneSheet.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestCloneSheet.java new file mode 100644 index 000000000..f6c47af84 --- /dev/null +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestCloneSheet.java @@ -0,0 +1,100 @@ +/* ==================================================================== + 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.ss.usermodel; + +import org.apache.poi.ss.ITestDataProvider; +import org.apache.poi.ss.util.CellRangeAddress; +import org.junit.Test; + +import java.io.IOException; + +import static org.junit.Assert.*; + +/** + * Common superclass for testing implementations of + * Workbook.cloneSheet() + */ +public abstract class BaseTestCloneSheet { + + private final ITestDataProvider _testDataProvider; + + protected BaseTestCloneSheet(ITestDataProvider testDataProvider) { + _testDataProvider = testDataProvider; + } + + @Test + public void testCloneSheetBasic() throws IOException{ + Workbook b = _testDataProvider.createWorkbook(); + Sheet s = b.createSheet("Test"); + s.addMergedRegion(new CellRangeAddress(0, 1, 0, 1)); + Sheet clonedSheet = b.cloneSheet(0); + + assertEquals("One merged area", 1, clonedSheet.getNumMergedRegions()); + + b.close(); + } + + /** + * Ensures that pagebreak cloning works properly + * @throws IOException + */ + @Test + public void testPageBreakClones() throws IOException { + Workbook b = _testDataProvider.createWorkbook(); + Sheet s = b.createSheet("Test"); + s.setRowBreak(3); + s.setColumnBreak((short) 6); + + Sheet clone = b.cloneSheet(0); + assertTrue("Row 3 not broken", clone.isRowBroken(3)); + assertTrue("Column 6 not broken", clone.isColumnBroken((short) 6)); + + s.removeRowBreak(3); + + assertTrue("Row 3 still should be broken", clone.isRowBroken(3)); + + b.close(); + } + + @Test + public void testCloneSheetIntValid() { + Workbook wb = _testDataProvider.createWorkbook(); + wb.createSheet("Sheet01"); + wb.cloneSheet(0); + assertEquals(2, wb.getNumberOfSheets()); + try { + wb.cloneSheet(2); + fail("ShouldFail"); + } catch (IllegalArgumentException e) { + // expected here + } + } + + @Test + public void testCloneSheetIntInvalid() { + Workbook wb = _testDataProvider.createWorkbook(); + wb.createSheet("Sheet01"); + try { + wb.cloneSheet(1); + fail("Should Fail"); + } catch (IllegalArgumentException e) { + // expected here + } + assertEquals(1, wb.getNumberOfSheets()); + } +} diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java index 346340082..2169db3d5 100644 --- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java +++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java @@ -161,7 +161,7 @@ public abstract class BaseTestWorkbook { fail("should have thrown exceptiuon due to duplicate sheet name"); } catch (IllegalArgumentException e) { // expected during successful test - assertEquals("The workbook already contains a sheet of this name", e.getMessage()); + assertEquals("The workbook already contains a sheet named 'sHeeT3'", e.getMessage()); } //names cannot be blank or contain any of /\*?[] @@ -255,7 +255,7 @@ public abstract class BaseTestWorkbook { fail("expected exception"); } catch (IllegalArgumentException e) { // expected during successful test - assertEquals("The workbook already contains a sheet of this name", e.getMessage()); + assertEquals("The workbook already contains a sheet named 'My very long sheet name which is longer than 31 chars and sheetName2.substring(0, 31) == sheetName1.substring(0, 31)'", e.getMessage()); } String sheetName3 = "POI allows creating sheets with names longer than 31 characters"; @@ -843,4 +843,68 @@ public abstract class BaseTestWorkbook { assertArrayEquals(filename + " sample file was modified as a result of closing the workbook", before, after); } + + @Test + public void sheetClone() throws IOException { + // First up, try a simple file + final Workbook b = _testDataProvider.createWorkbook(); + assertEquals(0, b.getNumberOfSheets()); + b.createSheet("Sheet One"); + b.createSheet("Sheet Two"); + + assertEquals(2, b.getNumberOfSheets()); + b.cloneSheet(0); + assertEquals(3, b.getNumberOfSheets()); + + // Now try a problem one with drawing records in it + Workbook bBack = HSSFTestDataSamples.openSampleWorkbook("SheetWithDrawing.xls"); + assertEquals(1, bBack.getNumberOfSheets()); + bBack.cloneSheet(0); + assertEquals(2, bBack.getNumberOfSheets()); + + bBack.close(); + b.close(); + } + + @Test + public void getSheetIndex() throws IOException { + final Workbook wb = _testDataProvider.createWorkbook(); + Sheet sheet1 = wb.createSheet("Sheet1"); + Sheet sheet2 = wb.createSheet("Sheet2"); + Sheet sheet3 = wb.createSheet("Sheet3"); + Sheet sheet4 = wb.createSheet("Sheet4"); + + assertEquals(0, wb.getSheetIndex(sheet1)); + assertEquals(1, wb.getSheetIndex(sheet2)); + assertEquals(2, wb.getSheetIndex(sheet3)); + assertEquals(3, wb.getSheetIndex(sheet4)); + + // remove sheets + wb.removeSheetAt(0); + wb.removeSheetAt(2); + + // ensure that sheets are moved up and removed sheets are not found any more + assertEquals(-1, wb.getSheetIndex(sheet1)); + assertEquals(0, wb.getSheetIndex(sheet2)); + assertEquals(1, wb.getSheetIndex(sheet3)); + assertEquals(-1, wb.getSheetIndex(sheet4)); + + wb.close(); + } + + @Test + public void addSheetTwice() throws IOException { + final Workbook wb = _testDataProvider.createWorkbook(); + Sheet sheet1 = wb.createSheet("Sheet1"); + assertNotNull(sheet1); + try { + wb.createSheet("Sheet1"); + fail("Should fail if we add the same sheet twice"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage(), e.getMessage().contains("already contains a sheet named 'Sheet1'")); + } + + wb.close(); + } + }