Bug 58085: removing sheet breaks other existing sheet references

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

src/java/org/apache/poi/hssf/model/InternalWorkbook.java

@@ -777,9 +777,9 @@ public final class InternalWorkbook {
         
         if (linkTable != null) {
             // also tell the LinkTable about the removed sheet
-            // +1 because we already removed it from the count of sheets!
+            //index hasn't change in the linktable
-            for(int i = sheetIndex+1;i < getNumSheets()+1;i++) {
+            if (linkTable != null) {
-                linkTable.removeSheet(i);
+                linkTable.removeSheet(sheetIndex);
             }
         }
     }

src/java/org/apache/poi/hssf/record/ExternSheetRecord.java

@@ -174,22 +174,17 @@ public class ExternSheetRecord extends StandardRecord {
 	
 	public void removeSheet(int sheetIdx) {
         int nItems = _list.size();
-        int toRemove = -1;
         for (int i = 0; i < nItems; i++) {
             RefSubRecord refSubRecord = _list.get(i);
             if(refSubRecord.getFirstSheetIndex() == sheetIdx && 
                     refSubRecord.getLastSheetIndex() == sheetIdx) {
-                toRemove = i;
+            	// removing the entry would mess up the sheet index in Formula of NameRecord
+            	_list.set(i, new RefSubRecord(refSubRecord.getExtBookIndex(), -1, -1));
             } else if (refSubRecord.getFirstSheetIndex() > sheetIdx && 
                     refSubRecord.getLastSheetIndex() > sheetIdx) {
                 _list.set(i, new RefSubRecord(refSubRecord.getExtBookIndex(), refSubRecord.getFirstSheetIndex()-1, refSubRecord.getLastSheetIndex()-1));
             }
         }
-        
-        // finally remove entries for sheet indexes that we remove
-        if(toRemove != -1) {
-            _list.remove(toRemove);
-        }
 	}
 	
 	/**

src/testcases/org/apache/poi/hssf/usermodel/TestWorkbook.java

@@ -22,8 +22,6 @@ import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
 
-import junit.framework.TestCase;
-
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.model.InternalWorkbook;
 import org.apache.poi.hssf.record.BackupRecord;
@@ -32,10 +30,15 @@ import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.ss.usermodel.Cell;
+import org.apache.poi.ss.usermodel.Name;
+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.ss.util.Region;
 import org.apache.poi.util.TempFile;
 
+import junit.framework.TestCase;
+
 /**
  * Class to test Workbook functionality
  *
@@ -43,6 +46,7 @@ import org.apache.poi.util.TempFile;
  * @author Greg Merrill
  * @author Siggi Cherem
  */
+@SuppressWarnings("deprecation")
 public final class TestWorkbook extends TestCase {
     private static final String LAST_NAME_KEY        = "lastName";
     private static final String FIRST_NAME_KEY       = "firstName";
@@ -194,11 +198,8 @@ public final class TestWorkbook extends TestCase {
      *
      */
 
-    public void testWriteDataFormat()
+    public void testWriteDataFormat() throws IOException {
-        throws IOException
+        File file = TempFile.createTempFile("testWriteDataFormat", ".xls");
-    {
-    File             file = TempFile.createTempFile("testWriteDataFormat",
-                                                    ".xls");
         FileOutputStream out = new FileOutputStream(file);
         HSSFWorkbook wb = new HSSFWorkbook();
         HSSFSheet s = wb.createSheet();
@@ -222,8 +223,7 @@ public final class TestWorkbook extends TestCase {
         POIFSFileSystem fs = new POIFSFileSystem(stream);
         HSSFWorkbook workbook = new HSSFWorkbook(fs);
         HSSFSheet sheet = workbook.getSheetAt(0);
-    HSSFCell    cell     =
+        HSSFCell cell = sheet.getRow(0).getCell(0);
-                     sheet.getRow(0).getCell(0);
         format = workbook.createDataFormat();
 
         assertEquals(1.25, cell.getNumericCellValue(), 1e-10);
@@ -233,6 +233,9 @@ public final class TestWorkbook extends TestCase {
         assertEquals(format, workbook.createDataFormat());
 
         stream.close();
+
+        workbook.close();
+        wb.close();
     }
 
     /**
@@ -447,8 +450,9 @@ public final class TestWorkbook extends TestCase {
 
     /**
      * Test the backup field gets set as expected.
+     * @throws IOException 
      */
-    public void testBackupRecord() {
+    public void testBackupRecord() throws IOException {
         HSSFWorkbook wb = new HSSFWorkbook();
 		wb.createSheet();
 		InternalWorkbook workbook = wb.getWorkbook();
@@ -462,6 +466,8 @@ public final class TestWorkbook extends TestCase {
         wb.setBackupFlag(false);
         assertEquals(0, record.getBackup());
         assertFalse(wb.getBackupFlag());
+        
+        wb.close();
     }
 
     private static final class RecordCounter implements RecordVisitor {
@@ -484,8 +490,9 @@ public final class TestWorkbook extends TestCase {
      * This tests is for bug [ #506658 ] Repeating output.
      *
      * We need to make sure only one LabelSSTRecord is produced.
+     * @throws IOException 
      */
-    public void testRepeatingBug() {
+    public void testRepeatingBug() throws IOException {
         HSSFWorkbook workbook = new HSSFWorkbook();
         HSSFSheet    sheet    = workbook.createSheet("Design Variants");
         HSSFRow      row      = sheet.createRow(2);
@@ -497,6 +504,8 @@ public final class TestWorkbook extends TestCase {
         RecordCounter rc = new RecordCounter();
         sheet.getSheet().visitContainedRecords(rc, 0);
         assertEquals(1, rc.getCount());
+        
+        workbook.close();
     }
 
 
@@ -550,9 +559,10 @@ public final class TestWorkbook extends TestCase {
         fileOut.close();
 
         assertTrue("file exists",file.exists());
+        
+        workbook.close();
     }
 
-    @SuppressWarnings("deprecation")
     public void testRepeatingColsRowsMinusOne() throws IOException
     {
         HSSFWorkbook workbook = new HSSFWorkbook();
@@ -573,10 +583,11 @@ public final class TestWorkbook extends TestCase {
         fileOut.close();
 
         assertTrue("file exists",file.exists());
+        
+        workbook.close();
     }
 
-    @SuppressWarnings("deprecation")
+    public void testAddMergedRegionWithRegion() throws IOException {
-    public void testAddMergedRegionWithRegion() {
         HSSFWorkbook     wb   = new HSSFWorkbook();
         HSSFSheet        s    = wb.createSheet();
 
@@ -603,5 +614,77 @@ public final class TestWorkbook extends TestCase {
 
         confirmRegion(new CellRangeAddress(0, 10, 0, 10), r1);
         confirmRegion(new CellRangeAddress(30, 40,5, 15), r2);
+        
+        wb.close();
+    }
+
+
+    public void testBug58085RemoveSheetWithNames() throws Exception {
+        reReadWithRemovedSheetWithName(writeWithRemovedSheetWithName());
+    }
+
+    private static HSSFWorkbook writeWithRemovedSheetWithName() throws IOException {
+        HSSFWorkbook workbook = new HSSFWorkbook();
+        Sheet sheet1 = workbook.createSheet("sheet1");
+        Sheet sheet2 = workbook.createSheet("sheet2");
+        Sheet sheet3 = workbook.createSheet("sheet3");
+
+        sheet1.createRow(0).createCell((short) 0).setCellValue("val1");
+        sheet2.createRow(0).createCell((short) 0).setCellValue("val2");
+        sheet3.createRow(0).createCell((short) 0).setCellValue("val3");
+        
+        Name namedCell1 = workbook.createName();
+        namedCell1.setNameName("name1");
+        String reference1 = "sheet1!$A$1";
+        namedCell1.setRefersToFormula(reference1);
+        
+        Name namedCell2= workbook.createName();
+        namedCell2.setNameName("name2");
+        String reference2 = "sheet2!$A$1";
+        namedCell2.setRefersToFormula(reference2);
+
+        Name namedCell3 = workbook.createName();
+        namedCell3.setNameName("name3");
+        String reference3 = "sheet3!$A$1";
+        namedCell3.setRefersToFormula(reference3);
+
+        return workbook;
+    }
+
+    private static void reReadWithRemovedSheetWithName(HSSFWorkbook workbookBefore) throws Exception {
+        Workbook workbook = HSSFTestDataSamples.writeOutAndReadBack(workbookBefore);
+        
+        System.out.println("Before removing sheet1...");
+        
+        Name nameCell = workbook.getName("name1");
+        System.out.println("name1: " + nameCell.getRefersToFormula());
+     
+        nameCell = workbook.getName("name2");
+        System.out.println("name2: " + nameCell.getRefersToFormula());
+        
+        nameCell = workbook.getName("name3");
+        System.out.println("name3: " + nameCell.getRefersToFormula());
+        
+        workbook.removeSheetAt(workbook.getSheetIndex("sheet1"));
+        
+        /*FileOutputStream fos = new FileOutputStream(AFTER_FILE);
+        try {
+            workbook.write(fos);
+        } finally {
+            fos.close();
+        }*/
+        
+        System.out.println("\nAfter removing sheet1...");
+        
+        nameCell = workbook.getName("name1");
+        System.out.println("name1: " + nameCell.getRefersToFormula());
+     
+        nameCell = workbook.getName("name2");
+        System.out.println("name2: " + nameCell.getRefersToFormula());
+        
+        nameCell = workbook.getName("name3");
+        System.out.println("name3: " + nameCell.getRefersToFormula());
+        
+        workbook.close();
     }
 }