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 2013/08/22 00:08:20 UTC

svn commit: r1516313 - in /poi/trunk: src/java/org/apache/poi/hssf/model/InternalWorkbook.java src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java test-data/spreadsheet/50298.xls

Author: centic
Date: Wed Aug 21 22:08:20 2013
New Revision: 1516313

URL: http://svn.apache.org/r1516313
Log:
Bug 50298: Fix corruption of Workbook when setting sheet order. The
boundssheets themselves were adjusted, but not the corresponding
records.

Added:
    poi/trunk/test-data/spreadsheet/50298.xls
Modified:
    poi/trunk/src/java/org/apache/poi/hssf/model/InternalWorkbook.java
    poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java

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=1516313&r1=1516312&r2=1516313&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 Wed Aug 21 22:08:20 2013
@@ -26,61 +26,8 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
 
-import org.apache.poi.ddf.EscherBSERecord;
-import org.apache.poi.ddf.EscherBoolProperty;
-import org.apache.poi.ddf.EscherContainerRecord;
-import org.apache.poi.ddf.EscherDgRecord;
-import org.apache.poi.ddf.EscherDggRecord;
-import org.apache.poi.ddf.EscherOptRecord;
-import org.apache.poi.ddf.EscherProperties;
-import org.apache.poi.ddf.EscherRGBProperty;
-import org.apache.poi.ddf.EscherRecord;
-import org.apache.poi.ddf.EscherSimpleProperty;
-import org.apache.poi.ddf.EscherSpRecord;
-import org.apache.poi.ddf.EscherSplitMenuColorsRecord;
-import org.apache.poi.hssf.record.BOFRecord;
-import org.apache.poi.hssf.record.BackupRecord;
-import org.apache.poi.hssf.record.BookBoolRecord;
-import org.apache.poi.hssf.record.BoundSheetRecord;
-import org.apache.poi.hssf.record.CodepageRecord;
-import org.apache.poi.hssf.record.CountryRecord;
-import org.apache.poi.hssf.record.DSFRecord;
-import org.apache.poi.hssf.record.DateWindow1904Record;
-import org.apache.poi.hssf.record.DrawingGroupRecord;
-import org.apache.poi.hssf.record.EOFRecord;
-import org.apache.poi.hssf.record.EscherAggregate;
-import org.apache.poi.hssf.record.ExtSSTRecord;
-import org.apache.poi.hssf.record.ExtendedFormatRecord;
-import org.apache.poi.hssf.record.ExternSheetRecord;
-import org.apache.poi.hssf.record.FileSharingRecord;
-import org.apache.poi.hssf.record.FnGroupCountRecord;
-import org.apache.poi.hssf.record.FontRecord;
-import org.apache.poi.hssf.record.FormatRecord;
-import org.apache.poi.hssf.record.HideObjRecord;
-import org.apache.poi.hssf.record.HyperlinkRecord;
-import org.apache.poi.hssf.record.InterfaceEndRecord;
-import org.apache.poi.hssf.record.InterfaceHdrRecord;
-import org.apache.poi.hssf.record.MMSRecord;
-import org.apache.poi.hssf.record.NameCommentRecord;
-import org.apache.poi.hssf.record.NameRecord;
-import org.apache.poi.hssf.record.PaletteRecord;
-import org.apache.poi.hssf.record.PasswordRecord;
-import org.apache.poi.hssf.record.PasswordRev4Record;
-import org.apache.poi.hssf.record.PrecisionRecord;
-import org.apache.poi.hssf.record.ProtectRecord;
-import org.apache.poi.hssf.record.ProtectionRev4Record;
-import org.apache.poi.hssf.record.RecalcIdRecord;
-import org.apache.poi.hssf.record.Record;
-import org.apache.poi.hssf.record.RefreshAllRecord;
-import org.apache.poi.hssf.record.SSTRecord;
-import org.apache.poi.hssf.record.StyleRecord;
-import org.apache.poi.hssf.record.SupBookRecord;
-import org.apache.poi.hssf.record.TabIdRecord;
-import org.apache.poi.hssf.record.UseSelFSRecord;
-import org.apache.poi.hssf.record.WindowOneRecord;
-import org.apache.poi.hssf.record.WindowProtectRecord;
-import org.apache.poi.hssf.record.WriteAccessRecord;
-import org.apache.poi.hssf.record.WriteProtectRecord;
+import org.apache.poi.ddf.*;
+import org.apache.poi.hssf.record.*;
 import org.apache.poi.hssf.record.common.UnicodeString;
 import org.apache.poi.hssf.util.HSSFColor;
 import org.apache.poi.ss.formula.EvaluationWorkbook.ExternalName;
@@ -405,14 +352,14 @@ public final class InternalWorkbook {
         }
 
         for (int k = 0; k < 21; k++) {
-            records.add(retval.createExtendedFormat(k));
+            records.add(InternalWorkbook.createExtendedFormat(k));
             retval.numxfs++;
         }
         retval.records.setXfpos( records.size() - 1 );
         for (int k = 0; k < 6; k++) {
-            records.add(retval.createStyle(k));
+            records.add(InternalWorkbook.createStyle(k));
         }
-        records.add(retval.createUseSelFS());
+        records.add(InternalWorkbook.createUseSelFS());
 
         int nBoundSheets = 1; // now just do 1
         for (int k = 0; k < nBoundSheets; k++) {
@@ -422,13 +369,13 @@ public final class InternalWorkbook {
             retval.boundsheets.add(bsr);
             retval.records.setBspos(records.size() - 1);
         }
-        records.add( retval.createCountry() );
+        records.add( InternalWorkbook.createCountry() );
         for ( int k = 0; k < nBoundSheets; k++ ) {
             retval.getOrCreateLinkTable().checkExternSheet(k);
         }
         retval.sst = new SSTRecord();
         records.add(retval.sst);
-        records.add(retval.createExtendedSST());
+        records.add(InternalWorkbook.createExtendedSST());
 
         records.add(EOFRecord.instance);
         if (log.check( POILogger.DEBUG ))
@@ -628,9 +575,15 @@ public final class InternalWorkbook {
      */
 
     public void setSheetOrder(String sheetname, int pos ) {
-    int sheetNumber = getSheetIndex(sheetname);
-    //remove the sheet that needs to be reordered and place it in the spot we want
-    boundsheets.add(pos, boundsheets.remove(sheetNumber));
+        int sheetNumber = getSheetIndex(sheetname);
+        //remove the sheet that needs to be reordered and place it in the spot we want
+        boundsheets.add(pos, boundsheets.remove(sheetNumber));
+        
+        // also adjust order of Records, calculate the position of the Boundsheets via getBspos()...
+        int pos0 = records.getBspos() - (boundsheets.size() - 1);
+        Record removed = records.get(pos0 + sheetNumber);
+        records.remove(pos0 + sheetNumber);
+		records.add(pos0 + pos, removed);
     }
 
     /**
@@ -1087,11 +1040,13 @@ public final class InternalWorkbook {
             Record record = records.get( k );
             if (record instanceof SSTRecord)
                 sst = (SSTRecord)record;
+
             if (record.getSid() == ExtSSTRecord.sid && sst != null)
                 retval += sst.calcExtSSTRecordSize();
             else
                 retval += record.getRecordSize();
         }
+
         return retval;
     }
 
@@ -2320,10 +2275,9 @@ public final class InternalWorkbook {
      * @param password to set
      */
     public void writeProtectWorkbook( String password, String username ) {
-        int protIdx = -1;
         FileSharingRecord frec = getFileSharing();
         WriteAccessRecord waccess = getWriteAccess();
-        WriteProtectRecord wprotect = getWriteProtect();
+        /* WriteProtectRecord wprotect =*/ getWriteProtect();
         frec.setReadOnly((short)1);
         frec.setPassword(FileSharingRecord.hashPassword(password));
         frec.setUsername(username);

Modified: poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java?rev=1516313&r1=1516312&r2=1516313&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java Wed Aug 21 22:08:20 2013
@@ -17,29 +17,40 @@
 
 package org.apache.poi.hssf.usermodel;
 
-import java.io.*;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
 import java.util.List;
 
 import junit.framework.AssertionFailedError;
 
-import org.apache.poi.hssf.HSSFTestDataSamples;
+import org.apache.poi.POIDataSamples;
+import org.apache.poi.ddf.EscherBSERecord;
+import org.apache.poi.hpsf.ClassID;
 import org.apache.poi.hssf.HSSFITestDataProvider;
+import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.model.HSSFFormulaParser;
-import org.apache.poi.hssf.model.InternalWorkbook;
 import org.apache.poi.hssf.model.InternalSheet;
-import org.apache.poi.hssf.record.*;
-import org.apache.poi.ss.formula.ptg.Area3DPtg;
-import org.apache.poi.util.LittleEndian;
-import org.apache.poi.util.TempFile;
-import org.apache.poi.ss.usermodel.BaseTestWorkbook;
-import org.apache.poi.ss.util.CellRangeAddress;
+import org.apache.poi.hssf.model.InternalWorkbook;
+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;
 import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
-import org.apache.poi.POIDataSamples;
-import org.apache.poi.ddf.EscherBSERecord;
-import org.apache.poi.hpsf.ClassID;
+import org.apache.poi.ss.formula.ptg.Area3DPtg;
+import org.apache.poi.ss.usermodel.BaseTestWorkbook;
+import org.apache.poi.ss.util.CellRangeAddress;
+import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.TempFile;
 
 /**
  * Tests for {@link HSSFWorkbook}
@@ -455,13 +466,16 @@ public final class TestHSSFWorkbook exte
         public BadlyBehavedRecord() {
             //
         }
-        public short getSid() {
+        @Override
+		public short getSid() {
             return 0x777;
         }
-        public int serialize(int offset, byte[] data) {
+        @Override
+		public int serialize(int offset, byte[] data) {
             return 4;
         }
-        public int getRecordSize() {
+        @Override
+		public int getRecordSize() {
             return 8;
         }
     }
@@ -598,6 +612,8 @@ public final class TestHSSFWorkbook exte
        wb = HSSFTestDataSamples.writeOutAndReadBack(wb);
        assertEquals(3, wb.getNumberOfSheets());
        assertEquals("Root xls", wb.getSheetAt(0).getRow(0).getCell(0).getStringCellValue());
+       
+       fs.close();
     }
    
     public void testCellStylesLimit() {
@@ -606,12 +622,12 @@ public final class TestHSSFWorkbook exte
         int MAX_STYLES = 4030;
         int limit = MAX_STYLES - numBuiltInStyles;
         for(int i=0; i < limit; i++){
-            HSSFCellStyle style = wb.createCellStyle();
+            /* HSSFCellStyle style =*/ wb.createCellStyle();
         }
 
         assertEquals(MAX_STYLES, wb.getNumCellStyles());
         try {
-            HSSFCellStyle style = wb.createCellStyle();
+            /*HSSFCellStyle style =*/ wb.createCellStyle();
             fail("expected exception");
         } catch (IllegalStateException e){
             assertEquals("The maximum number of cell styles was exceeded. " +
@@ -877,4 +893,31 @@ public final class TestHSSFWorkbook exte
         wb.unwriteProtectWorkbook();
         assertFalse(wb.isWriteProtected());
     }
+
+	public void testBug50298() throws Exception {
+		HSSFWorkbook wb = HSSFTestDataSamples.openSampleWorkbook("50298.xls");
+
+		
+		HSSFSheet sheet = wb.cloneSheet(0);
+
+		wb.setSheetName(wb.getSheetIndex(sheet), "copy");
+
+		wb.setSheetOrder("copy", 0);
+
+		wb.removeSheetAt(0);
+		
+		// check that the overall workbook serializes with its correct size
+		int expected = wb.getWorkbook().getSize();
+		int written = wb.getWorkbook().serialize(0, new byte[expected*2]);
+		
+		assertEquals("Did not have the expected size when writing the workbook: written: " + written + ", but expected: " + expected,
+				expected, written);
+		
+		FileOutputStream fileOut = new FileOutputStream("/tmp/workbook.xls");
+		try {
+			wb.write(fileOut);
+		} finally {
+			fileOut.close();
+		}
+	}
 }

Added: poi/trunk/test-data/spreadsheet/50298.xls
URL: http://svn.apache.org/viewvc/poi/trunk/test-data/spreadsheet/50298.xls?rev=1516313&view=auto
==============================================================================
Files poi/trunk/test-data/spreadsheet/50298.xls (added) and poi/trunk/test-data/spreadsheet/50298.xls Wed Aug 21 22:08:20 2013 differ



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


Re: svn commit: r1516313 - in /poi/trunk: src/java/org/apache/poi/hssf/model/InternalWorkbook.java src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java test-data/spreadsheet/50298.xls

Posted by Dominik Stadler <do...@gmx.at>.
Hi,

Ok, I will adjust the Eclipse project settings that I use, btw, any reason
why we don't have one checked in? Then we can define all formatting
settings there and anybody using Eclipse will automatically use the correct
formatting?!

Any objections to me checking in Eclipse .project/.classpath/.settings
files?

Thanks... Dominik.



On Thu, Aug 22, 2013 at 1:20 AM, Nick Burch <ap...@gagravarr.org> wrote:

> On Wed, 21 Aug 2013, centic@apache.org wrote:
>
>> -import org.apache.poi.ddf.**EscherBSERecord;
>> -import org.apache.poi.ddf.**EscherBoolProperty;
>> -import org.apache.poi.ddf.**EscherContainerRecord;
>>
> ...
>
>> -import org.apache.poi.hssf.record.**BOFRecord;
>> -import org.apache.poi.hssf.record.**BackupRecord;
>> -import org.apache.poi.hssf.record.**BookBoolRecord;
>>
> ....
>
>> +import org.apache.poi.ddf.*;
>> +import org.apache.poi.hssf.record.*;
>>
>
> I think we've generally tended to prefer explict imports to wildcard ones.
> It generally makes it easier to check where a file comes from, and avoids
> issues when we have multiple classes in different packages with the same
> name (eg Sheet)
>
> Cheers
> Nick
>
> ------------------------------**------------------------------**---------
> To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
> For additional commands, e-mail: dev-help@poi.apache.org
>
>

Re: svn commit: r1516313 - in /poi/trunk: src/java/org/apache/poi/hssf/model/InternalWorkbook.java src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java test-data/spreadsheet/50298.xls

Posted by Nick Burch <ap...@gagravarr.org>.
On Wed, 21 Aug 2013, centic@apache.org wrote:
> -import org.apache.poi.ddf.EscherBSERecord;
> -import org.apache.poi.ddf.EscherBoolProperty;
> -import org.apache.poi.ddf.EscherContainerRecord;
...
> -import org.apache.poi.hssf.record.BOFRecord;
> -import org.apache.poi.hssf.record.BackupRecord;
> -import org.apache.poi.hssf.record.BookBoolRecord;
....
> +import org.apache.poi.ddf.*;
> +import org.apache.poi.hssf.record.*;

I think we've generally tended to prefer explict imports to wildcard ones. 
It generally makes it easier to check where a file comes from, and avoids 
issues when we have multiple classes in different packages with the same 
name (eg Sheet)

Cheers
Nick

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