You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@poi.apache.org by jo...@apache.org on 2008/06/06 00:24:05 UTC
svn commit: r663765 - in /poi/trunk/src: documentation/content/xdocs/
java/org/apache/poi/hssf/dev/ java/org/apache/poi/hssf/model/
java/org/apache/poi/hssf/record/aggregates/
testcases/org/apache/poi/hssf/model/
Author: josh
Date: Thu Jun 5 15:24:05 2008
New Revision: 663765
URL: http://svn.apache.org/viewvc?rev=663765&view=rev
Log:
Fix for bug 45145 - made sure RowRecordsAggregate comes before ValueRecordsAggregate. Also fixed BiffViewer to show correct record offsets
Modified:
poi/trunk/src/documentation/content/xdocs/changes.xml
poi/trunk/src/documentation/content/xdocs/status.xml
poi/trunk/src/java/org/apache/poi/hssf/dev/BiffViewer.java
poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java
poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java
poi/trunk/src/testcases/org/apache/poi/hssf/model/TestSheet.java
Modified: poi/trunk/src/documentation/content/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/changes.xml?rev=663765&r1=663764&r2=663765&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/changes.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/changes.xml Thu Jun 5 15:24:05 2008
@@ -37,6 +37,7 @@
<!-- Don't forget to update status.xml too! -->
<release version="3.1-final" date="2008-06-??">
+ <action dev="POI-DEVELOPERS" type="fix">45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate</action>
<action dev="POI-DEVELOPERS" type="fix">45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes</action>
<action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats like [Black]YYYY as being date based</action>
<action dev="POI-DEVELOPERS" type="add">45060 - Improved token class transformation during formula parsing</action>
Modified: poi/trunk/src/documentation/content/xdocs/status.xml
URL: http://svn.apache.org/viewvc/poi/trunk/src/documentation/content/xdocs/status.xml?rev=663765&r1=663764&r2=663765&view=diff
==============================================================================
--- poi/trunk/src/documentation/content/xdocs/status.xml (original)
+++ poi/trunk/src/documentation/content/xdocs/status.xml Thu Jun 5 15:24:05 2008
@@ -34,6 +34,7 @@
<!-- Don't forget to update changes.xml too! -->
<changes>
<release version="3.1-final" date="2008-06-??">
+ <action dev="POI-DEVELOPERS" type="fix">45145 - Fixed Sheet to always enforce RowRecordsAggregate before ValueRecordsAggregate</action>
<action dev="POI-DEVELOPERS" type="fix">45123 - Fixed SharedFormulaRecord.convertSharedFormulas() to propagate token operand classes</action>
<action dev="POI-DEVELOPERS" type="fix">45087 - Correctly detect date formats like [Black]YYYY as being date based</action>
<action dev="POI-DEVELOPERS" type="add">45060 - Improved token class transformation during formula parsing</action>
Modified: poi/trunk/src/java/org/apache/poi/hssf/dev/BiffViewer.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/dev/BiffViewer.java?rev=663765&r1=663764&r2=663765&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/dev/BiffViewer.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/dev/BiffViewer.java Thu Jun 5 15:24:05 2008
@@ -87,7 +87,8 @@
records.add(record);
if (activeRecord != null)
activeRecord.dump(ps);
- activeRecord = new RecordDetails(recStream.getSid(), recStream.getLength(), (int)recStream.getPos(), record);
+ int startPos = (int)(recStream.getPos()-recStream.getLength() - 4);
+ activeRecord = new RecordDetails(recStream.getSid(), recStream.getLength(), startPos, record);
}
if (dump) {
recStream.dumpBytes(ps);
Modified: poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java?rev=663765&r1=663764&r2=663765&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/model/Sheet.java Thu Jun 5 15:24:05 2008
@@ -66,7 +66,7 @@
protected ArrayList records = null;
int preoffset = 0; // offset of the sheet in a new file
int loc = 0;
- protected int dimsloc = 0;
+ protected int dimsloc = -1; // TODO - is it legal for dims record to be missing?
protected DimensionsRecord dims;
protected DefaultColWidthRecord defaultcolwidth = null;
protected DefaultRowHeightRecord defaultrowheight = null;
@@ -295,6 +295,8 @@
}
else if ( rec.getSid() == IndexRecord.sid )
{
+ // ignore INDEX record because it is only needed by Excel,
+ // and POI always re-calculates its contents
rec = null;
}
@@ -329,8 +331,8 @@
}
}
retval.records = records;
- retval.checkCells();
retval.checkRows();
+ retval.checkCells();
if (log.check( POILogger.DEBUG ))
log.log(POILogger.DEBUG, "sheet createSheet (existing file) exited");
return retval;
@@ -486,7 +488,15 @@
if (cells == null)
{
cells = new ValueRecordsAggregate();
- records.add(getDimsLoc() + 1, cells);
+ // In the worksheet stream, the row records always occur before the cell (value)
+ // records. Therefore POI's aggregates (RowRecordsAggregate, ValueRecordsAggregate)
+ // should follow suit. Some methods in this class tolerate either order, while
+ // others have been found to fail (see bug 45145).
+ int rraIndex = getDimsLoc() + 1;
+ if (records.get(rraIndex).getClass() != RowRecordsAggregate.class) {
+ throw new IllegalStateException("Cannot create value records before row records exist");
+ }
+ records.add(rraIndex+1, cells);
}
}
@@ -836,46 +846,61 @@
return pos-offset;
}
- private int serializeIndexRecord(final int BOFRecordIndex, final int offset, byte[] data) {
- IndexRecord index = new IndexRecord();
- index.setFirstRow(rows.getFirstRowNum());
- index.setLastRowAdd1(rows.getLastRowNum()+1);
- //Calculate the size of the records from the end of the BOF
- //and up to the RowRecordsAggregate...
- int sheetRecSize = 0;
- for (int j = BOFRecordIndex+1; j < records.size(); j++)
- {
- Record tmpRec = (( Record ) records.get(j));
- if (tmpRec instanceof UncalcedRecord) {
- continue;
+ /**
+ * @param indexRecordOffset also happens to be the end of the BOF record
+ * @return the size of the serialized INDEX record
+ */
+ private int serializeIndexRecord(final int bofRecordIndex, final int indexRecordOffset,
+ byte[] data) {
+ IndexRecord index = new IndexRecord();
+ index.setFirstRow(rows.getFirstRowNum());
+ index.setLastRowAdd1(rows.getLastRowNum() + 1);
+ // Calculate the size of the records from the end of the BOF
+ // and up to the RowRecordsAggregate...
+
+ // 'initial sheet records' are between INDEX and first ROW record.
+ int sizeOfInitialSheetRecords = 0;
+ // start just after BOF record (INDEX is not present in this list)
+ for (int j = bofRecordIndex + 1; j < records.size(); j++) {
+ Record tmpRec = ((Record) records.get(j));
+ if (tmpRec instanceof UncalcedRecord) {
+ continue;
+ }
+ if (tmpRec instanceof RowRecordsAggregate) {
+ break;
+ }
+ sizeOfInitialSheetRecords += tmpRec.getRecordSize();
}
- if (tmpRec instanceof RowRecordsAggregate) {
- break;
+ if (_isUncalced) {
+ sizeOfInitialSheetRecords += UncalcedRecord.getStaticRecordSize();
}
- sheetRecSize+= tmpRec.getRecordSize();
- }
- if (_isUncalced) {
- sheetRecSize += UncalcedRecord.getStaticRecordSize();
- }
- //Add the references to the DBCells in the IndexRecord (one for each block)
- int blockCount = rows.getRowBlockCount();
- //Calculate the size of this IndexRecord
- int indexRecSize = IndexRecord.getRecordSizeForBlockCount(blockCount);
-
- int rowBlockOffset = 0;
- int cellBlockOffset = 0;
- int dbCellOffset = 0;
- for (int block=0;block<blockCount;block++) {
- rowBlockOffset += rows.getRowBlockSize(block);
- cellBlockOffset += null == cells ? 0 : cells.getRowCellBlockSize(rows.getStartRowNumberForBlock(block),
- rows.getEndRowNumberForBlock(block));
- //Note: The offsets are relative to the Workbook BOF. Assume that this is
- //0 for now.....
- index.addDbcell(offset + indexRecSize + sheetRecSize + dbCellOffset + rowBlockOffset + cellBlockOffset);
- //Add space required to write the dbcell record(s) (whose references were just added).
- dbCellOffset += (8 + (rows.getRowCountForBlock(block) * 2));
- }
- return index.serialize(offset, data);
+
+ // Add the references to the DBCells in the IndexRecord (one for each block)
+ // Note: The offsets are relative to the Workbook BOF. Assume that this is
+ // 0 for now.....
+
+ int blockCount = rows.getRowBlockCount();
+ // Calculate the size of this IndexRecord
+ int indexRecSize = IndexRecord.getRecordSizeForBlockCount(blockCount);
+
+ int currentOffset = indexRecordOffset + indexRecSize + sizeOfInitialSheetRecords;
+
+ for (int block = 0; block < blockCount; block++) {
+ // each row-block has a DBCELL record.
+ // The offset of each DBCELL record needs to be updated in the INDEX record
+
+ // account for row records in this row-block
+ currentOffset += rows.getRowBlockSize(block);
+ // account for cell value records after those
+ currentOffset += null == cells ? 0 : cells.getRowCellBlockSize(rows
+ .getStartRowNumberForBlock(block), rows.getEndRowNumberForBlock(block));
+
+ // currentOffset is now the location of the DBCELL record for this row-block
+ index.addDbcell(currentOffset);
+ // Add space required to write the DBCELL record (whose reference was just added).
+ currentOffset += (8 + (rows.getRowCountForBlock(block) * 2));
+ }
+ return index.serialize(indexRecordOffset, data);
}
Modified: poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java?rev=663765&r1=663764&r2=663765&view=diff
==============================================================================
--- poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java (original)
+++ poi/trunk/src/java/org/apache/poi/hssf/record/aggregates/ValueRecordsAggregate.java Thu Jun 5 15:24:05 2008
@@ -37,7 +37,7 @@
public final class ValueRecordsAggregate
extends Record
{
- public final static short sid = -1000;
+ public final static short sid = -1001; // 1000 clashes with RowRecordsAggregate
int firstcell = -1;
int lastcell = -1;
CellValueRecordInterface[][] records;
Modified: poi/trunk/src/testcases/org/apache/poi/hssf/model/TestSheet.java
URL: http://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/hssf/model/TestSheet.java?rev=663765&r1=663764&r2=663765&view=diff
==============================================================================
--- poi/trunk/src/testcases/org/apache/poi/hssf/model/TestSheet.java (original)
+++ poi/trunk/src/testcases/org/apache/poi/hssf/model/TestSheet.java Thu Jun 5 15:24:05 2008
@@ -19,11 +19,15 @@
import junit.framework.AssertionFailedError;
import junit.framework.TestCase;
+
+import org.apache.poi.hssf.eventmodel.ERFListener;
+import org.apache.poi.hssf.eventmodel.EventRecordFactory;
import org.apache.poi.hssf.record.*;
import org.apache.poi.hssf.record.aggregates.ColumnInfoRecordsAggregate;
import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate;
import org.apache.poi.hssf.record.aggregates.ValueRecordsAggregate;
+import java.io.ByteArrayInputStream;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
@@ -34,8 +38,7 @@
* @author Glen Stampoultzis (glens at apache.org)
*/
public final class TestSheet extends TestCase {
- public void testCreateSheet() throws Exception
- {
+ public void testCreateSheet() {
// Check we're adding row and cell aggregates
List records = new ArrayList();
records.add( new BOFRecord() );
@@ -52,8 +55,7 @@
assertTrue( sheet.records.get(pos++) instanceof EOFRecord );
}
- public void testAddMergedRegion()
- {
+ public void testAddMergedRegion() {
Sheet sheet = Sheet.createSheet();
int regionsToAdd = 4096;
int startRecords = sheet.getRecords().size();
@@ -91,8 +93,7 @@
}
}
- public void testRemoveMergedRegion()
- {
+ public void testRemoveMergedRegion() {
Sheet sheet = Sheet.createSheet();
int regionsToAdd = 4096;
@@ -139,13 +140,11 @@
assertEquals("Should be no more merged regions", 0, sheet.getNumMergedRegions());
}
- public void testGetMergedRegionAt()
- {
+ public void testGetMergedRegionAt() {
//TODO
}
- public void testGetNumMergedRegions()
- {
+ public void testGetNumMergedRegions() {
//TODO
}
@@ -163,14 +162,13 @@
Sheet sheet = Sheet.createSheet(records, 0);
assertNotNull("Row [2] was skipped", sheet.getRow(2));
-
}
/**
* Make sure page break functionality works (in memory)
*
*/
- public void testRowPageBreaks(){
+ public void testRowPageBreaks() {
short colFrom = 0;
short colTo = 255;
@@ -226,7 +224,7 @@
* Make sure column pag breaks works properly (in-memory)
*
*/
- public void testColPageBreaks(){
+ public void testColPageBreaks() {
short rowFrom = 0;
short rowTo = (short)65535;
@@ -292,20 +290,20 @@
final short DEFAULT_IDX = 0xF; // 15
short xfindex = Short.MIN_VALUE;
Sheet sheet = Sheet.createSheet();
-
+
// without ColumnInfoRecord
xfindex = sheet.getXFIndexForColAt((short) 0);
assertEquals(DEFAULT_IDX, xfindex);
xfindex = sheet.getXFIndexForColAt((short) 1);
assertEquals(DEFAULT_IDX, xfindex);
-
+
ColumnInfoRecord nci = ( ColumnInfoRecord ) sheet.createColInfo();
sheet.columns.insertColumn(nci);
-
+
// single column ColumnInfoRecord
nci.setFirstColumn((short) 2);
nci.setLastColumn((short) 2);
- nci.setXFIndex(TEST_IDX);
+ nci.setXFIndex(TEST_IDX);
xfindex = sheet.getXFIndexForColAt((short) 0);
assertEquals(DEFAULT_IDX, xfindex);
xfindex = sheet.getXFIndexForColAt((short) 1);
@@ -318,7 +316,7 @@
// ten column ColumnInfoRecord
nci.setFirstColumn((short) 2);
nci.setLastColumn((short) 11);
- nci.setXFIndex(TEST_IDX);
+ nci.setXFIndex(TEST_IDX);
xfindex = sheet.getXFIndexForColAt((short) 1);
assertEquals(DEFAULT_IDX, xfindex);
xfindex = sheet.getXFIndexForColAt((short) 2);
@@ -333,7 +331,7 @@
// single column ColumnInfoRecord starting at index 0
nci.setFirstColumn((short) 0);
nci.setLastColumn((short) 0);
- nci.setXFIndex(TEST_IDX);
+ nci.setXFIndex(TEST_IDX);
xfindex = sheet.getXFIndexForColAt((short) 0);
assertEquals(TEST_IDX, xfindex);
xfindex = sheet.getXFIndexForColAt((short) 1);
@@ -342,7 +340,7 @@
// ten column ColumnInfoRecord starting at index 0
nci.setFirstColumn((short) 0);
nci.setLastColumn((short) 9);
- nci.setXFIndex(TEST_IDX);
+ nci.setXFIndex(TEST_IDX);
xfindex = sheet.getXFIndexForColAt((short) 0);
assertEquals(TEST_IDX, xfindex);
xfindex = sheet.getXFIndexForColAt((short) 7);
@@ -354,7 +352,7 @@
}
/**
- * Prior to bug 45066, POI would get the estimated sheet size wrong
+ * Prior to bug 45066, POI would get the estimated sheet size wrong
* when an <tt>UncalcedRecord</tt> was present.<p/>
*/
public void testUncalcSize_bug45066() {
@@ -363,7 +361,7 @@
records.add(new BOFRecord());
records.add(new UncalcedRecord());
records.add(new EOFRecord());
- Sheet sheet = Sheet.createSheet( records, 0, 0 );
+ Sheet sheet = Sheet.createSheet(records, 0, 0);
int estimatedSize = sheet.getSize();
int serializedSize = sheet.serialize(0, new byte[estimatedSize]);
@@ -372,5 +370,73 @@
}
assertEquals(50, serializedSize);
}
+
+ /**
+ * Prior to bug 45145 <tt>RowRecordsAggregate</tt> and <tt>ValueRecordsAggregate</tt> could
+ * sometimes occur in reverse order. This test reproduces one of those situations and makes
+ * sure that RRA comes before VRA.<br/>
+ *
+ * The code here represents a normal POI use case where a spreadsheet is created from scratch.
+ */
+ public void testRowValueAggregatesOrder_bug45145() {
+
+ Sheet sheet = Sheet.createSheet();
+
+ RowRecord rr = new RowRecord(5);
+ sheet.addRow(rr);
+
+ CellValueRecordInterface cvr = new BlankRecord();
+ cvr.setColumn((short)0);
+ cvr.setRow(5);
+ sheet.addValueRecord(5, cvr);
+
+
+ int dbCellRecordPos = getDbCellRecordPos(sheet);
+ if (dbCellRecordPos == 264) {
+ // The overt symptom of the bug
+ // DBCELL record pos is calculated wrong if VRA comes before RRA
+ throw new AssertionFailedError("Identified bug 45145");
+ }
+
+ // make sure that RRA and VRA are in the right place
+ int rraIx = sheet.getDimsLoc()+1;
+ List recs = sheet.getRecords();
+ assertEquals(RowRecordsAggregate.class, recs.get(rraIx).getClass());
+ assertEquals(ValueRecordsAggregate.class, recs.get(rraIx+1).getClass());
+
+ assertEquals(254, dbCellRecordPos);
+ }
+
+ /**
+ * @return the value calculated for the position of the first DBCELL record for this sheet.
+ * That value is found on the IndexRecord.
+ */
+ private static int getDbCellRecordPos(Sheet sheet) {
+ int size = sheet.getSize();
+ byte[] data = new byte[size];
+ sheet.serialize(0, data);
+ EventRecordFactory erf = new EventRecordFactory();
+ MyIndexRecordListener myIndexListener = new MyIndexRecordListener();
+ erf.registerListener(myIndexListener, new short[] { IndexRecord.sid, });
+ erf.processRecords(new ByteArrayInputStream(data));
+ IndexRecord indexRecord = myIndexListener.getIndexRecord();
+ int dbCellRecordPos = indexRecord.getDbcellAt(0);
+ return dbCellRecordPos;
+ }
+
+ private static final class MyIndexRecordListener implements ERFListener {
+
+ private IndexRecord _indexRecord;
+ public MyIndexRecordListener() {
+ // no-arg constructor
+ }
+ public boolean processRecord(Record rec) {
+ _indexRecord = (IndexRecord)rec;
+ return true;
+ }
+ public IndexRecord getIndexRecord() {
+ return _indexRecord;
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
For additional commands, e-mail: commits-help@poi.apache.org