You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by ce...@apache.org on 2015/10/23 14:11:40 UTC

svn commit: r1710193 - in /poi: site/src/documentation/content/xdocs/ trunk/src/java/org/apache/poi/hssf/model/ trunk/src/java/org/apache/poi/hssf/record/ trunk/src/testcases/org/apache/poi/hssf/usermodel/

Author: centic
Date: Fri Oct 23 12:11:40 2015
New Revision: 1710193

URL: http://svn.apache.org/viewvc?rev=1710193&view=rev
Log:
Bug 58085: removing sheet breaks other existing sheet references

Modified:
    poi/site/src/documentation/content/xdocs/status.xml
    poi/trunk/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
    poi/trunk/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java
    poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestWorkbook.java

Modified: poi/site/src/documentation/content/xdocs/status.xml
URL: http://svn.apache.org/viewvc/poi/site/src/documentation/content/xdocs/status.xml?rev=1710193&r1=1710192&r2=1710193&view=diff
==============================================================================
--- poi/site/src/documentation/content/xdocs/status.xml (original)
+++ poi/site/src/documentation/content/xdocs/status.xml Fri Oct 23 12:11:40 2015
@@ -39,6 +39,7 @@
     </devs>
 
     <release version="3.14-beta1" date="2015-11-??">
+        <action dev="PD" type="fix" fixes-bug="58085">removing sheet breaks other existing sheet references</action>
         <action dev="PD" type="fix" fixes-bug="58480">Work around problem where on Windows systems a Mapped Buffer can still lock a file even if the Channel was closed properly</action>
         <action dev="PD" type="add">Update commons-logging to 1.2 and commons-codec to 1.10</action>
         <action dev="PD" type="fix">Removed deprecated mixed case getter/setter in XSSFColor</action>

Modified: poi/trunk/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/model/InternalWorkbook.java?rev=1710193&r1=1710192&r2=1710193&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/model/InternalWorkbook.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/model/InternalWorkbook.java Fri Oct 23 12:11:40 2015
@@ -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);
             }
         }
     }

Modified: poi/trunk/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java?rev=1710193&r1=1710192&r2=1710193&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/ExternSheetRecord.java Fri Oct 23 12:11:40 2015
@@ -174,22 +174,17 @@ public class ExternSheetRecord extends S
 	
 	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);
-        }
 	}
 	
 	/**

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestWorkbook.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestWorkbook.java?rev=1710193&r1=1710192&r2=1710193&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestWorkbook.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestWorkbook.java Fri Oct 23 12:11:40 2015
@@ -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
      *
      */
 
-    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);
-
-    r = s.createRow(0);
-    c = r.createCell(0);
-    c.setCellStyle(cs);
-    c.setCellValue(1.25);
+    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);
+
+        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
 
     /**
      * 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
         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
      * 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
         RecordCounter rc = new RecordCounter();
         sheet.getSheet().visitContainedRecords(rc, 0);
         assertEquals(1, rc.getCount());
+        
+        workbook.close();
     }
 
 
@@ -550,9 +559,10 @@ public final class TestWorkbook extends
         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
         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
 
         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();
     }
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org