From 73ae5a04cb99d0fcccdc56feddc010023a3c08e0 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Fri, 23 Oct 2015 12:11:40 +0000 Subject: [PATCH] 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 --- .../poi/hssf/model/InternalWorkbook.java | 6 +- .../poi/hssf/record/ExternSheetRecord.java | 9 +- .../poi/hssf/usermodel/TestWorkbook.java | 153 ++++++++++++++---- 3 files changed, 123 insertions(+), 45 deletions(-) diff --git a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java b/src/java/org/apache/poi/hssf/model/InternalWorkbook.java index c92c897094..8631833653 100644 --- a/src/java/org/apache/poi/hssf/model/InternalWorkbook.java +++ b/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! - for(int i = sheetIndex+1;i < getNumSheets()+1;i++) { - linkTable.removeSheet(i); + //index hasn't change in the linktable + if (linkTable != null) { + linkTable.removeSheet(sheetIndex); } } } diff --git a/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java b/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java index 636b6b0da0..61dfc93e0f 100644 --- a/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java +++ b/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); - } } /** diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestWorkbook.java b/src/testcases/org/apache/poi/hssf/usermodel/TestWorkbook.java index 7fdb4bd752..e29b35b7ec 100644 --- a/src/testcases/org/apache/poi/hssf/usermodel/TestWorkbook.java +++ b/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,45 +198,44 @@ public final class TestWorkbook extends TestCase { * */ - public void testWriteDataFormat() - throws IOException - { - File file = TempFile.createTempFile("testWriteDataFormat", - ".xls"); - FileOutputStream out = new FileOutputStream(file); - HSSFWorkbook wb = new HSSFWorkbook(); - HSSFSheet s = wb.createSheet(); - HSSFRow r = null; - HSSFCell c = null; - HSSFDataFormat format = wb.createDataFormat(); - HSSFCellStyle cs = wb.createCellStyle(); + public void testWriteDataFormat() throws IOException { + File file = TempFile.createTempFile("testWriteDataFormat", ".xls"); + FileOutputStream out = new FileOutputStream(file); + HSSFWorkbook wb = new HSSFWorkbook(); + HSSFSheet s = wb.createSheet(); + HSSFRow r = null; + HSSFCell c = null; + HSSFDataFormat format = wb.createDataFormat(); + HSSFCellStyle cs = wb.createCellStyle(); - short df = format.getFormat("0.0"); - cs.setDataFormat(df); + short df = format.getFormat("0.0"); + cs.setDataFormat(df); - r = s.createRow(0); - c = r.createCell(0); - c.setCellStyle(cs); - c.setCellValue(1.25); + r = s.createRow(0); + c = r.createCell(0); + c.setCellStyle(cs); + c.setCellValue(1.25); wb.write(out); out.close(); - FileInputStream stream = new FileInputStream(file); - POIFSFileSystem fs = new POIFSFileSystem(stream); - HSSFWorkbook workbook = new HSSFWorkbook(fs); - HSSFSheet sheet = workbook.getSheetAt(0); - HSSFCell cell = - sheet.getRow(0).getCell(0); - format = workbook.createDataFormat(); + FileInputStream stream = new FileInputStream(file); + POIFSFileSystem fs = new POIFSFileSystem(stream); + HSSFWorkbook workbook = new HSSFWorkbook(fs); + HSSFSheet sheet = workbook.getSheetAt(0); + HSSFCell cell = sheet.getRow(0).getCell(0); + format = workbook.createDataFormat(); - assertEquals(1.25,cell.getNumericCellValue(), 1e-10); + assertEquals(1.25, cell.getNumericCellValue(), 1e-10); - assertEquals(format.getFormat(df), "0.0"); + assertEquals(format.getFormat(df), "0.0"); - assertEquals(format, workbook.createDataFormat()); + 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() { + public void testAddMergedRegionWithRegion() throws IOException { 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(); } }