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

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++) {
+        for (String wbName : WORKBOOK_DIR_ENTRY_NAMES) {
-            String wbName = WORKBOOK_DIR_ENTRY_NAMES[i];
             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++) {
+        for (int index : indexes) {
-            validateSheetIndex(indexes[i]);
+            validateSheetIndex(index);
         }
         int nSheets = _sheets.size();
         for (int i=0; i<nSheets; i++) {
             boolean bSelect = false;
-            for (int j = 0; j < indexes.length; j++) {
+            for (int index : indexes) {
-                if (indexes[j] == i) {
+                if (index == i) {
                     bSelect = true;
                     break;
                 }
@@ -603,8 +602,10 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
 
     /**
      * sets the first tab that is displayed in the list of tabs
-     * in excel.
+     * in excel. This method does <b>not</b> hide, select or focus sheets.
-     * @param index
+     * 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);
 
@@ -1099,8 +1100,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
     {
         BackupRecord backupRecord = workbook.getBackupRecord();
 
-        return (backupRecord.getBackup() == 0) ? false
+        return backupRecord.getBackup() != 0;
-                : true;
     }
 
     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 new HSSFCellStyle(index, xfr, this);
-
-        return style;
     }
 
     /**
@@ -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 new HSSFCellStyle((short)idx, xfr, this);
-
-        return style;
     }
 
     /**
@@ -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 (Record rec : _list) {
-            for(int i=0; i<nRecs; i++) {
-                Record rec = _list.get(i);
                 result += rec.serialize(offset + result, data);
             }
             return result;
@@ -1399,9 +1393,9 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
         // before getting the workbook size we must tell the sheets that
         // serialization is about to occur.
         workbook.preSerialize();
-        for (int i = 0; i < nSheets; i++) {
+        for (HSSFSheet sheet : sheets) {
-            sheets[i].getSheet().preSerialize();
+            sheet.getSheet().preSerialize();
-            sheets[i].preSerialize();
+            sheet.preSerialize();
         }
 
         int totalsize = workbook.getSize();
@@ -1460,8 +1454,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
 
     @Override
     public int getNumberOfNames(){
-        int result = names.size();
+        return names.size();
-        return result;
     }
 
     @Override
@@ -1507,9 +1500,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
      * @return named range name
      */
     public String getNameName(int index){
-        String result = getNameAt(index).getNameName();
+        return getNameAt(index).getNameName();
-
-        return result;
     }
 
     /**
@@ -1718,9 +1709,7 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
         r.decode();
         List<EscherRecord> escherRecords = r.getEscherRecords();
         PrintWriter w = new PrintWriter(new OutputStreamWriter(System.out, Charset.defaultCharset()));
-        for ( Iterator<EscherRecord> iterator = escherRecords.iterator(); iterator.hasNext(); )
+        for (EscherRecord escherRecord : escherRecords) {
-        {
-            EscherRecord escherRecord = iterator.next();
             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<HSSFPictureData> pictures = new ArrayList<HSSFPictureData>();
-        Iterator<Record> recordIter = workbook.getRecords().iterator();
+        for (Record r : workbook.getRecords()) {
-        while (recordIter.hasNext())
+            if (r instanceof AbstractEscherHolderRecord) {
-        {
-            Record r = recordIter.next();
-            if (r instanceof AbstractEscherHolderRecord)
-            {
                 ((AbstractEscherHolderRecord) r).decode();
                 List<EscherRecord> escherRecords = ((AbstractEscherHolderRecord) r).getEscherRecords();
                 searchForPictures(escherRecords, pictures);

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<RelationPart> 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() ))
+        validateSheetName(sheetname);
-               throw new IllegalArgumentException( "The workbook already contains a sheet of this name");
 
         // 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);

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 {
 }

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());
+    }
+}

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.

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 static org.junit.Assert.*;
-
-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;
 
 /**
  * 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() {
-	public void testCloneSheetBasic() throws IOException{
+        super(HSSFITestDataProvider.instance);
-		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");
@@ -80,6 +52,7 @@ public final class TestCloneSheet extends TestCase {
         assertEquals(HSSFTestHelper.getSheetForTest(s).getRecords().size(), HSSFTestHelper.getSheetForTest(s2).getRecords().size());
     }
 
+    @Test
     public void testCloneSheetWithEmptyDrawingAggregate(){
         HSSFWorkbook b = new HSSFWorkbook();
         HSSFSheet s = b.createSheet("Test");
@@ -115,6 +88,7 @@ public final class TestCloneSheet extends TestCase {
         assertArrayEquals(agg1.serialize(), agg2.serialize());
     }
 
+    @Test
     public void testCloneComment() throws IOException {
         HSSFWorkbook wb = new HSSFWorkbook();
         HSSFSheet sh = wb.createSheet();

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();

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.*;
-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.util.CellRangeAddress;
 import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.RecordFormatException;
 import org.apache.poi.util.TempFile;
 import org.junit.Test;
 
@@ -117,31 +113,9 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         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,13 +648,12 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
                   HSSFWorkbook hw = new HSSFWorkbook(root, true);
                   List<HSSFObjectData> objects = hw.getAllEmbeddedObjects();
                   boolean found = false;
-                  for (int i = 0; i < objects.size(); i++) {
+                   for (HSSFObjectData embeddedObject : objects) {
-                     HSSFObjectData embeddedObject = objects.get(i);
                        if (embeddedObject.hasDirectoryEntry()) {
                            DirectoryEntry dir = embeddedObject.getDirectory();
                            if (dir instanceof DirectoryNode) {
-                           DirectoryNode dNode = (DirectoryNode)dir;
+                               DirectoryNode dNode = (DirectoryNode) dir;
-                           if (hasEntry(dNode,"WordDocument")) {
+                               if (hasEntry(dNode, "WordDocument")) {
                                    found = true;
                                }
                            }
@@ -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();
+        Workbook wb = new HSSFWorkbook();
-        HSSFSheet s1 = wb.createSheet("first sheet");
+        Sheet s1 = wb.createSheet("first sheet");
-        HSSFSheet s2 = wb.createSheet("other 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);
+        Row s1r1 = s1.createRow(2);
-        HSSFCell c1 = s1r1.createCell(3);
+        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);
+        Row s2r1 = s2.createRow(0);
-        HSSFCell c3 = s2r1.createCell(1);
+        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,7 +777,7 @@ 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());
 
@@ -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

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() {

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());
+    }
+}

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();
+    }
+
 }